On Tue, Aug 06, 2019 at 09:40:41AM -0400, Robert Middleton wrote: > On Mon, Aug 5, 2019 at 10:22 AM Andy Shevchenko > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > > > On Mon, Aug 05, 2019 at 09:49:24AM -0400, Robert Middleton wrote: > > > > Thanks for testing, my comments below. > > > > > Unfortunately this will re-introduce the bug that it was attempting to > > > solve, that is ensuring that the buffer in the kernel and the buffer > > > on the chip are clear before going into shutdown on the chip. > > > Breaking at the beginning of the loop means that the kernel has > > > written everything to the internal buffer on the chip, but until the > > > LSR bits are clear the bytes have not been transmitted yet. > > > > So, the difference here, that you have a long delay with mdelay(1) which > > "fixes" your issue. > > The important fix is really this part: > lsr = serial_in(up, UART_LSR); > if (lsr & (UART_LSR_TEMT | UART_LSR_THRE)) > > Both of those bits must be set before the shutdown can occur. The > msleep(1) is so that it is not a busy loop(maybe usleep_range would be > better?). > If the transmitter is not empty and the transmitter FIFO is > not empty, when the shutdown occurs the remaining data will be > discarded by the exar chip. Assuming the FIFO on the exar is full(256 > bytes to transmit), it would take ~250ms to transmit all of the bytes > at 9600 baud, so our check would happen ~250 times(assuming mdelay(1) > sleeps for 1ms and not up to 20). > > This is based off of the reference driver from Max Linear: > https://github.com/rm5248/exar-driver/blob/5dcbac6aa564cf5604512ecd6c9e02c577b70fa2/xr17v35x.c#L1282 I see, you explicitly force to continue in your case. Thanks for explanation. > > I guess the third patch in this series makes it again not-working. > > Can you check and confirm that? > > I don't see a third patch. It's here [1]. > > Or even better, replace entire loop with one usleep_range() call and play with > > numbers there, like (10, 20), (100, 150), (1000, 1100). Probably you can start > > with udelay(2) followed up by above list. > > > > If my theory is correct you will see at some point the problem will disappear. > > It would probably disappear, but if you remove the entire loop and LSR > check you can't be sure that the data has actually been transmitted > out of the exar chip. Likely this would result in a different number > of bytes being dropped depending on the baud rate and how large the > buffer was when shutdown occurs. I had sent v3 [2] without altering the algorithm, just one refactoring (patch 1) and switching to usleep_range() for delay (patch 2). Can you test that conversion still works for you? [1]: https://www.spinics.net/lists/linux-serial/msg35412.html [2]: https://www.spinics.net/lists/linux-serial/msg35444.html -- With Best Regards, Andy Shevchenko
