-
Notifications
You must be signed in to change notification settings - Fork 86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FIX] Fix bcm pl011 UART #168
base: master
Are you sure you want to change the base?
Conversation
1e2a8ac
to
407b4d6
Compare
Just to clarify, it seem to me that when you enable the receive timeout interrupt and the FIFO, then the serial server should work again. You get an interrupt either when the FIFO has a certain level or - due to the receive timeout - when there is data in the FIFO, but no additional data is received for "a 32-bit period". So we should be able to merge this without breaking thins. I wonder if should still make the interrupt behavior configurable via the |
3c0db13
to
0b0c67b
Compare
0b0c67b
to
90395bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically I am fine with this, and I think this is the change what we discussed back then. I can't test this in detail at the moment - anybody?
Please rebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All look like reasonable fixes.
As the old code is known not to work and this probably does, I think we should just merge this.
The Gitlint and License Checks are stuck, but I can't re-trigger them. I also don't have an option to bring the branch up-to-date, so I guess a manual merge needs to be done. |
I will try to rebase it on monday |
b177ad8
to
90395bd
Compare
Using the PL011 as uart for data did not work: -Enabling FIFO as no irqs are raised otherwise -Added required interrupts -Fixing incorrect irq disable Signed-off-by: Felix Schladt <[email protected]>
Signed-off-by: Felix Schladt <[email protected]>
90395bd
to
e456c24
Compare
So I looked at the Broadcom UART documentation, but it doesn't explain what the receive time-out does. Does it trigger all the time or does it only trigger after receiving a character and nothing else arrives? |
As far as I understand, the receive timeout only triggers after a certain period of no incoming data |
Does that mean you get an interrupt all the time? Because that would make it "work", but not in the right way. |
@@ -227,14 +230,14 @@ static int pl011_uart_configure(ps_chardevice_t *dev) | |||
* | |||
*/ | |||
// Enable FIFO | |||
//pl011_uart_enable_fifo(dev); | |||
pl011_uart_enable_fifo(dev); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced that the problem mentioned in the comment is properly addressed. I am afraid that enabling FIFOs is just a work-around for an unfixed bug, probably caused by the original's code assumptions of the current state of the UART or initialisation.
Have you tried fully configuring the UART and setting all registers, and clearing any pending IRQs and errors at init?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please print something out at every receive interrupt and check that you don't get any when no input is given.
Or keep a counter and print out the counter value too.
I tried using the existing driver for data transmissions (with a hardware rng) and had the issue that without enabled FIFO, no interrupts are received.
As the note states, enabling the FIFO seems to cause some other trouble with the SerialServer.
I think this should be discussed, what makes the most sense and maybe a solution which fits all can be found.