Shell with UART is dropping characters

[keith-zephyr]

Shell with UART is dropping characters

[keith-zephyr]

@robhancocksed - I'm not able to assign this to you, but this should be fixed before relanding #72599

[robhancocksed]

@keith-zephyr Looking at the code in the uart_ns16550 driver's uart_ns16550_fifo_fill function, it seems to assume that whenever it is called, the entire FIFO size of the UART is available to write data and so it will write up to fifo_size bytes if they are provided. With the change to the shell UART backend, it's possible that uart_fifo_fill is called twice for a given UART interrupt, such as if the shell's ring buffer wraps around and there is data that needs writing at both the "tail" and "head" ends. That behavior seems valid according to the UART API definition as far as I can tell. With the way uart_ns16550_fifo_fill is implemented, this could easily result in the UART's FIFO overflowing and dropping data.

Other UART drivers that I have looked at are either checking for FIFO full status after each byte is written, or possibly checking the FIFO occupancy before writing bytes to ensure it is not going to overflow, in their fifo_fill function. It seems like either that is the proper fix here. Or if indeed it's not valid to call uart_fifo_fill multiple times on one UART interrupt, then the patch I had should be reverted, but the documentation should be updated to make that clear..

[robhancocksed]

It looks like the 16550 UART doesn't actually provide a way to directly check the TX FIFO occupancy or even if it is full. All you know is that when you get an interrupt that indicates the TX FIFO is empty, you can write up to the TX FIFO size without overflow. So if it needs to support multiple uart_fifo_calls per interrupt, it would need to keep track internally of how many bytes were already written on the current interrupt and ensure it didn't allow more than that many bytes to be written in total.

[aescolar]

@keith-zephyr did the revert in #72599 resolve this issue? if so, could you please close this bug report.

[aescolar]

Closing, please reopen if you disagree