Re: [PATCH] Revert "serial: 8250: Don't touch RTS modem control while in rs485 mode"

From: Lukas Wunner <lukas@wunner.de>
To: Su Bao Cheng <baocheng_su@163.com>
Cc: Su Bao Cheng <baocheng.su@siemens.com>,
	linux-serial@vger.kernel.org, gregkh@linuxfoundation.org,
	jan.kiszka@siemens.com, chao.zeng@siemens.com
Subject: Re: [PATCH] Revert "serial: 8250: Don't touch RTS modem control while in rs485 mode"
Date: Sat, 20 Nov 2021 18:18:10 +0100	[thread overview]
Message-ID: <20211120171810.GA26621@wunner.de> (raw)
In-Reply-To: <e1a9b9bf-45a4-6e71-09f4-1ae730284778@163.com>

On Fri, Nov 12, 2021 at 02:14:11PM +0800, Su Bao Cheng wrote:
> On 2021/10/27 7:39, Lukas Wunner wrote:
> > On Wed, Oct 27, 2021 at 07:16:44PM +0800, Su Bao Cheng wrote:
> > > During tty_open, the uart_port_startup sets the MCR to 0, and then use
> > > set_mctrl to restore the MCR, so at this time, the MCR read does not
> > > reflect the desired value.

So only the *initial* value of MCR[1] is wrong and prevents receiving.
But once you've sent some data, RTS is deasserted correctly and you can
receive again.  Did I understand that correctly?

> The MCR is set to 0 at this line within uart_port_startup():
> 	retval = uport->ops->startup(uport);
> 
> On omap8250, the startup() points to omap_8250_startup(), within it:
> 	up->mcr = 0;
> 
> For software controlled RTS pin of RS485 half-duplex, when not in the
> transmitting, the MCR[1] should be constant to indicate the current
> direction is receiving. This is set in serial8250_em485_stop_tx().

I'm missing an important piece of information here:  Are you using
inverse polarity for RTS?  Normally MCR[1] should be 1 to transmit
and 0 to receive, per the figure on page 8734 of this document:

https://www.ti.com/lit/ug/spruid7e/spruid7e.pdf

Thus, setting up->mcr = 0 should be perfectly fine because it results
in RTS being deasserted, so the transceiver is in receive mode.

I suspect that you're using inverse polarity for RTS, so the desired
initial value of MCR[1] is 1 in your case.  Is that correct?

You write above that "the MCR[1] should be constant to indicate the
current direction is receiving".  That sentence is missing the desired
value, i.e. should MCR[1] be constant 1 or constant 0?

I suspect that the incorrect value in MCR[1] is evaluated by
omap8250_set_mctrl() via this call stack:
  omap_8250_set_termios()
    omap8250_restore_regs()
      up->port.ops->set_mctrl()

Could you confirm this please by inserting a dump_stack() in
omap8250_set_mctrl()?

I would also like to know if you have set UPSTAT_AUTORTS on the port
by enabling hardware flow-control (CRTSCTS) on the tty.  That would
cause omap8250_set_mctrl to fiddle with UART_EFR_RTS bit and I think
the user-visible result is that the transceiver is switched to
transmit mode when the "RX FIFO HALT trigger level" is reached.
We should probably stop it from doing that.

Thanks,

Lukas