On Tue, 16 Apr 2024, Michael Pratt wrote: > On Tuesday, April 16th, 2024 at 14:57, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > > > unsigned int status, tmout = 10000; > > > > > > - /* Wait up to 10ms for the character(s) to be sent. / > > > + / Wait for a time relative to buffer size and baud */ > > > + if (up->fifo_enable && up->port.timeout) > > > + tmout = jiffies_to_usecs(up->port.timeout); > > > > > > Why do we still use that default? Can't we calculate timeout even for\ > > FIFO-less / FIFO-disabled devices? Yes we definitely should be able to. Unfortunately these patches just keep coming back not in the form that follows the review feedback, but they come up their own way of doing things which is way worse and ignores the given feedback. > Maybe it's possible that there is some kind of rare case where the LSR register > is not working or not configured properly for a device in which support > is being worked on...without a timeout, that would result in an infinite loop. "without a timeout" is not what Andy said. He said you should always have a timeout, regardless of there being FIFO or not. And that timeout should be derived in the same manner from baudrate and FIFO size (to address the cases w/o FIFO, the fifosize should be lower bounded to 1 while calculating the FIFO timeout). > AFAIK, when everything is working properly, there is no such thing as needing > a timeout for a uart device without fifo, as every single byte written would trigger > an interrupt anyway. While I agree the general principle, that this is backup that should not even be needed, the statement is partly incorrect. We don't get interrupts during console write because they're disabled. But LSR should still change and allow progress without the backup timeout. -- i.
