Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> writes: > On 04.04.2024 13:04:27, Esben Haabendal wrote: >> Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> writes: >> >> > On 03.04.2024 17:22:52, Esben Haabendal wrote: >> >> Busy polling with readl() is a rather harsh way to wait for a potentially >> >> long time. >> > >> > This read_poll_timeout_atomic() is compiled to an >> > imx_uart_readl()/udelay()/cpu_relax() loop. Does the introduction of >> > udelay() bring any advantages? >> >> Good point. Probably not. I can set sleep_us 0 to go back to a tight >> loop. > > Sounds good I will do that for v2 then. >> >> While there, introduce a 10 ms timeout on this waiting, similar to what >> >> many other serial drivers do. >> > >> > But you don't handle the return value... >> >> True. But this is similar to all the different wait_for_xmitr() >> functions, which does basically the same. They are all void, so the >> timeout is handled in same happy-go-lucky style. > > IMHO the patch description should mention that the driver now ignores > the state of the transmitter after the timeout. Ok. Will do. >> I think the best we could do would be to show an error message. But >> maybe that is not the most sane thing to do to report a problem with >> writing error messages. I don't know, but maybe that is why most the >> other serial drivers are handling it like this. > > Writing an error message within the console driver could lead to a > positive feedback loop :) Yes, probably best to just silently ignore it. >> In fsl_lpuart.c and uartlite.c a warning message is printed if/when this >> timeout occurs. I am fine with doing that here as well... >> >> On a related note. I am unsure if 10 ms is a good choice for timeout. I >> picked it because it seems like a common value used in many/most >> drivers. But at least some drivers use something like 1 s, which to me >> sounds more sane given that we cannot do any meaningful error handling >> on timeout. > > Not having any experience with console drivers, I think the time to > empty the FIFO depends on the size of the TX FIFO and the speed of the > UART. > > With some numbers (FIFO size and UART speed) pulled out of thin air (and > neglecting start/stop/parity bits): > > 32 bytes * 8 bit/byte / 9600 bit/s = 26.7ms I assume that typical console usage will have messages much larger than 32 bytes. But on the other hand, most use cases will be 115200 bit/s. But in general, I would be more comofortable with a 1 second timeout. It should be more than large enough to handle all realistic cases. But will avoid spinning forever if uart for some reason does never clear the TXD bit. /Esben
