Hello, TL;DR: The key question in this thread is, if a driver should drain fifo and transmitter in .set_termios or not. On Fri, May 31, 2019 at 07:15:21PM +0300, Sergey Organov wrote: > Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> writes: > > On Fri, May 31, 2019 at 09:14:56AM +0300, Sergey Organov wrote: > >> Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> writes: > >> > >> > On Thu, May 30, 2019 at 06:29:46PM +0300, Sergey Organov wrote: > >> >> imx_set_termios(): remove busy-waiting "drain Tx FIFO" loop. Worse > >> >> yet, it was potentially unbounded wait due to RTS/CTS (hardware) > >> >> handshake. > >> >> > >> >> Let user space ensure draining is done before termios change, if > >> >> draining is needed in the first place. > >> > > >> > I don't know for sure what the intended behaviour is here, but I tend to > >> > think that changing the unbounded wait to a timeout and then return > >> > -EBUSY (?) would be more suitable. > >> > >> No, please! Bytes in Tx FIFO are not an excuse to exit with error > >> instead of setting new termios as asked to. > > > > Well, my suggestion is more defensive. It at least tells the user that > > they do something wrong. If they already care for having the FIFO and > > transmitter empty before changing the baud rate the behaviour of both > > your and my approach are identical. With yours however it undefined if > > characters written to the device before the change are sent with the old > > or new settings. So my suggestions yields a deterministic behaviour > > which is good. And it tells the user when they do something wrong, which > > is good, too. > > > >> > With your change you're possibly breaking existent software. > >> > >> Well, I suspect the software is already broken then, as most widely used > >> drivers out there seem to do no Tx FIFO draining on set_termios() call, > >> or do they? > >> > >> I mean I tried to find similar code in some of the other drivers, to > >> replicate it, but I failed to find one. > > > > The first (and only) driver I checked does. (sa1100.c) > > I think I'd rather take 8250 as reference implementation, as being most > widely used. Can anybody please tell how 8250 code handles this? Does it > attempt to drain Tx FIFO on termios changes? Well, there are so many 8250 variants that the driver is rather complicated. Also the original 8250 doesn't have a FIFO at all. Given that it might not be so easy to judge if a given driver drains the FIFO and transmitter without consulting the reference manual I'd rather rely on an authority for the serial core. (Apart from that I bet we're finding examples for both variants.) @gregkh, rmk: What do you think? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
