Re: [RFC PATCH 1/2] serial: imx: Avoid busy polling for transmitter to become empty

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> >> 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.

> 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 :)

> 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

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [RT Stable]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]

  Powered by Linux