Bug fix for UART issue on LPC54608 - issue #7398

[dreemkiller]

Description

When the LPC54608 receives a short UART message, the interrupt never got serviced as it was only servicing it when the hardware Rx FIFO was full.
This PR changes that so it services the hardware Rx FIFO when it is "not empty".
This is a solution to issue #7398

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

[kjbracey]

Looking at that, do we not also want "transmit fifo not full"?

"empty" will work, but won't sustain throughput. "not full" isn't ideal, as it will tend to produce lots more interrupts and consume more CPU time, but given the choice of two, that's the one to go for.

The ideal is to have FIFO threshold values for both TX and RX IRQs to process several bytes per interrupt, and timeout if the threshold isn't reached. Real PC UARTs have always had this, but these embedded devices tend to omit it. If the LPC could possibly do that, would be ideal.

[dreemkiller]

I tend to agree with "transmit fifo not full", and was wondering the same thing. I imagine short messages going out would cause the same problem as short messages coming in. However, I haven't seen this issue, so I was hesitant to make a change that I could not validate solved an actual issue.

I, too, was wondering about the efficiency problem.

Note that the test project I created, https://github.com/dreemkiller/lpc54608_test, is probably not a great test for all of these interrupt efficiency issues, as the only thing it does is initialize the WizFi board and then connect to a wifi network. It doesn't send data across the link. It could probably be modified to send and receive packets of various sizes, but that's probably not a very good way to test this.
We probably need to find or write a dedicated UART test that uses a test shield and that sends and receives UART messages of various sizes to really make sure all of this works.
It would also probably be beneficial to run some benchmarks to see if there really is a performance/power issue with using "Not Empty" for both of them.
Perhaps that's a bigger job than this bug merits, though.

[kjbracey]

The transmit wouldn't be wrong - and for short messages it makes no difference at all as the interrupt won't be enabled anyway.

The difference occurs as soon as you have more than a FIFO full - do you wake the driver as soon as there is 1 byte of space available, or only when it's fully empty? Both are fairly rubbish answers - you really want to be woken when it's nearly empty, so you get to fill in nearly a FIFO's worth in one go, without the FIFO ever actually running out.

In this issue #4722 we experimented with "transmit register empty" versus "transmit complete" for a chip with a single data register, and found that "transmit complete" produced better throughput, despite the resulting inter-byte gap, because it meant we got to do 2 bytes per interrupt (first byte moves straight from transmit register to shift register if not currently transmitting, second byte sits and waits in the transmit register).

So maybe don't tweak that without an actual test.

[mmahadevan108]

Thank you.
The RX FIFO Theshold is set to 1. However the FIFO request is raised only when the FIFO data count reaches greater than 1 which was a miss from my part.

[cmonr]

@kjbracey-arm Thoughts? It seems like this could fixa bug, but it also sounds like we don't have a good way to test this.

@dreemkiller Would it be possible for you to run greentea tests using this target locally? It's something.

[cmonr]

@dreemkiller Do you have bandwidth to locally run tests sometime soon?

[0xc0170]

CI started

[mbed-ci]

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

• jenkins-ci/mbed-os-ci_greentea-test

[cmonr]

Retesting since CI failure appears to be unrelated to PR.