Re: [Qemu-devel] [PATCH 1/2] serial: fix retry logic

[Stefano Stabellini]

On Mon, 2 Apr 2012, Anthony Liguori wrote:
> I'm not sure if the retry logic has ever worked when not using FIFO mode.  I
> found this while writing a test case although code inspection confirms it is
> definitely broken.
> 
> The TSR retry logic will never actually happen because it is guarded by an
> 'if (s->tsr_rety > 0)' but this is the only place that can ever make the
> variable greater than zero.  That effectively makes the retry logic an 'if 
> (0)'.
> 
> I believe this is a typo and the intention was >= 0.

I agree if you, I don't think there can be another explanation.


> Once this is fixed though,
> I see double transmits with my test case.  This is because in the non FIFO
> case, serial_xmit may get invoked while LSR.THRE is still high because the
> character was processed but the retransmit timer was still active.

If that is the case then this problem must be independent from the tsr_retry
bug, considering that the code path you are changing is only taken when
tsr_retry <= 0, right?


> We can handle this by simply checking for LSR.THRE and returning early.
> It's
> possible that the FIFO paths also need some attention.

The manual states: "In the FIFO mode this bit is set when the XMIT FIFO
is empty; it is cleared when at least 1 byte is written to the XMIT
FIFO", therefore I would return early if UART_LSR_THRE is set no matter
if we are in FIFO mode or not.