Re: Possible software flow problem in serial_core

Re: Possible software flow problem in serial_core

[Peter Hurley]

[ +cc linux-serial ]

On 03/20/2014 09:44 AM, Bollinger, Seth wrote:
> Hello All,
>
> We’ve recently run into a possible problem with software flow control
> handling in the serial_core layer.
>
> Here is the scenario:
>
> 1. Transmit from uart to remote device
> 2. Remote device sends us an XOFF
> 3. The tty layer receives the XOFF
> 4. stop_tty() - The uart transmitter is stopped (ops->stop_tx) just as the
> serial_core ring is cleared (this could trap a few bytes in the fifo).
> 5. Remote device sends us an XON
> 6. The tty layer receives the XON
> 7. start_tty() - However, the serial_core ring is empty, so the call to
> start the uart transmitter (uart_start:ops->start_tx) is skipped
>
> Any window, however small, could leave bytes stuck in the transmitter
> forever -- particularly if there will be no further transmission until
> receiving a response.
>
> I can't find any functionality in the drivers that accounts for this
> possibility.  Can you help me find how Linux serial drivers manage this
> eventuality?
>
> Thanks,
>
> Seth

What hardware is this?

A 16550 transmitter doesn't shut-off at uart->stop_tx(), so the remaining
data in the transmitter FIFO is still transmitted.

Some hardware has (relatively) big FIFOs and allows the transmitter to be
shutoff, but these should unconditionally re-enable the transmitter without
clearing the xmit fifo.

Are you seeing lost data? Maybe it's because soft flow-control is to
slow to shutoff the sender?

Regards,
Peter Hurley


--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Possible software flow problem in serial_core

[Bollinger, Seth]

>What hardware is this?

We¹re using the Freescale mxs-auart driver (i.mx28).
 
>A 16550 transmitter doesn't shut-off at uart->stop_tx(), so the remaining
>data in the transmitter FIFO is still transmitted.

Ok, I just reviewed the 8250 driver and you¹re correct, it appears the
transmitter is only disabled on the 16C950.  I misread before (incorrectly
saw 16C550)...

>Some hardware has (relatively) big FIFOs and allows the transmitter to be
>shutoff, but these should unconditionally re-enable the transmitter
>without
>clearing the xmit fifo.

Do you have a driver that would be a good example of this?  From looking
at serial_core.c:uart_start(), is seems that start_tx() is only called if
the serial_core ring buffer is not empty.  If we don¹t stop the
transmitter, we slew too many characters.

>Are you seeing lost data? Maybe it's because soft flow-control is to
>slow to shutoff the sender?

No dropped bytes, but we do see a few bytes trapped in the fifo because we
received an XOFF.  When we receive the XON, start_tx() is never called
because the serial_core ring buffer is empty, and we have no more data to
transmit.  When we enable the transmitter manually, these bytes will pop
out.

Thanks for all of your help!

Seth

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Possible software flow problem in serial_core

[Peter Hurley]

On 03/20/2014 07:34 PM, Bollinger, Seth wrote:
> Do you have a driver that would be a good example of this?  From looking
> at serial_core.c:uart_start(), is seems that start_tx() is only called if
> the serial_core ring buffer is not empty.  If we don¹t stop the
> transmitter, we slew too many characters.

Yeah, you're right; thanks for catching this.
(I was busy looking at the ll driver and completely missed the bug in
the serial core).

uart_start() should not be conditioning the call to start_tx() on
the ring buffer being empty; ll drivers should already be able to handle
that because CTS flow control change will start_tx regardless of the ring
buffer count.

Will you send a patch?

Regards,
Peter Hurley

Re: Possible software flow problem in serial_core

[Bollinger, Seth]

>Yeah, you're right; thanks for catching this.
>(I was busy looking at the ll driver and completely missed the bug in
>the serial core).
>
>uart_start() should not be conditioning the call to start_tx() on
>the ring buffer being empty; ll drivers should already be able to handle
>that because CTS flow control change will start_tx regardless of the ring
>buffer count.
>
>Will you send a patch?

Sure thing.  Just to be clear, you want to remove the ring buffer tests
and leave the tty tests, right?

This will have to wait for the morning though.  :)

Seth

Re: Possible software flow problem in serial_core

[Peter Hurley]

On 03/20/2014 08:48 PM, Bollinger, Seth wrote:
>
>> Yeah, you're right; thanks for catching this.
>> (I was busy looking at the ll driver and completely missed the bug in
>> the serial core).
>>
>> uart_start() should not be conditioning the call to start_tx() on
>> the ring buffer being empty; ll drivers should already be able to handle
>> that because CTS flow control change will start_tx regardless of the ring
>> buffer count.
>>
>> Will you send a patch?
>
> Sure thing.  Just to be clear, you want to remove the ring buffer tests
> and leave the tty tests, right?

Yes. I would remove the NULL ptr test for xmit.buf as well. That way
we can uncover any bugs in the ll drivers that mistakenly depend on it.

The !stopped and !hw_stopped tests must remain.

> This will have to wait for the morning though.  :)

Take your time. TTY/serial trees are closed until 3.15-rc1 anyway.
Please be sure to address the patch to the serial maintainers.
You can cc me in the patch.

Thanks again,
Peter Hurley