Re: [PATCH v2 2/3] serial: 8250_exar: Refactor exar_shutdown()

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

 



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.

I guess the third patch in this series makes it again not-working.
Can you check and confirm that?

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.

> I'm not positive that the uart_circ_empty needs to be checked in the
> first place; I had put it in because the serial8250_tx_chars does that
> before stopping the tx, and I assume that there could be a potential
> race condition where the kernel has not yet written all the data to
> the exar, but the exar has finished transmitting all the data in its
> transmit buffer(I am not sure how likely this is to happen).

tty gets uninitialized before ->shutdown() happen, it also set's TTY IO error
condition, which has been checked in tty_write(). I'm sure new data will not
come at this point.

-- 
With Best Regards,
Andy Shevchenko





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux