Hello, 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) Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
