2022-11-18 12:26:20

by Matthias Schiffer

[permalink] [raw]
Subject: 8250_omap: incorrect RS485 RTS on close during transmission

Hi Lukas,

I've discovered a new issue with EM485 in 8250_omap (and probably other
serial drivers as well?): When closing the TTY device while there is
still an ongoing transmission, the RTS pin will get stuck active until
the TTY is opened again. This can be easily reproduced by running `cat
/dev/zero > /dev/ttySx` and then stopping it using Ctrl-C.

The issue exists in current mainline (6.1-rc5+), and applying "serial:
8250: 8250_omap: Avoid RS485 RTS glitch on ->set_termios()" from next
doesn't solve it. I think it should work with "serial: 8250: 8250_omap:
Support native RS485", but we also need RS485 on AM57xx CPUs, which
don't have native RS485.

I intend to look into this myself next week, but as you've worked on
this code a lot lately, maybe you already have an idea how to fix it?

Regards,
Matthias



2022-11-22 09:26:16

by Matthias Schiffer

[permalink] [raw]
Subject: Re: 8250_omap: incorrect RS485 RTS on close during transmission

On Fri, 2022-11-18 at 12:57 +0100, Matthias Schiffer wrote:
> Hi Lukas,
>
> I've discovered a new issue with EM485 in 8250_omap (and probably other
> serial drivers as well?): When closing the TTY device while there is
> still an ongoing transmission, the RTS pin will get stuck active until
> the TTY is opened again. This can be easily reproduced by running `cat
> /dev/zero > /dev/ttySx` and then stopping it using Ctrl-C.
>
> The issue exists in current mainline (6.1-rc5+), and applying "serial:
> 8250: 8250_omap: Avoid RS485 RTS glitch on ->set_termios()" from next
> doesn't solve it. I think it should work with "serial: 8250: 8250_omap:
> Support native RS485", but we also need RS485 on AM57xx CPUs, which
> don't have native RS485.
>
> I intend to look into this myself next week, but as you've worked on
> this code a lot lately, maybe you already have an idea how to fix it?
>
> Regards,
> Matthias
>

Okay, I've narrowed down the issue to uart_shutdown() ->
uart_port_shutdown() - here, ops->shutdown() is called, which clears
the FIFOs (in the case of the default 8250_port and 8250_omap)
implementations, but doesn't wait for the clear to finish, and doesn't
call ops->stop_tx().

I believe a similar problem can occur in uart_suspend_port(). Here,
ops->stop_tx() is called before ops->shutdown(), but stop_tx() is a no-
op when the TX FIFO is not empty (again, for the 8250 case - I haven't
really looked at other drivers).

Given that EM485 is handled in 8250_port by __stop_tx(), which is
called both internally in the 8250 driver and from the serial core
through stop_tx(), it is not clear to me what the correct layer to fix
this is. Two ideas:

(1) Have ops->shutdown() wait until the TX FIFO is empty. Call
ops->stop_tx() after ops->shutdown() in serial core. For this to work,
calling stop_tx() after shutdown() must be allowed in all drivers; I'm
not sure if this is currently the case, and if it is desirable at all.

(2) Do the whole handling in the driver's ops->shutdown(), do not
further involve the serial core.

So far, I've implemented (1) locally for 8250_omap, and together with
"serial: 8250: 8250_omap: Avoid RS485 RTS glitch on ->set_termios()" it
fixes the issue on my test devices, but I'm not too happy about the
"shutdown then tx_stop" order of calls. So maybe (2) is the better
solution after all?