Re: 8250_omap: incorrect RS485 RTS on close during transmission

[Matthias Schiffer]

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?