Re: [PATCH v5.10] serial: core: Initialize rs485 RTS polarity already on probe

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Jun 23, 2022 at 09:58:58AM +0900, Dominique Martinet wrote:
> From: Lukas Wunner <lukas@wunner.de>
> 
> Core part of commit 2dd8a74fddd21b95dcc60a2d3c9eaec993419d69 upstream:
> the PL011 driver does not support RS485 in the 5.10 tree yet so drop
> that bit.
> 
> RTS polarity of rs485-enabled ports is currently initialized on uart
> open via:
> 
> tty_port_open()
>   tty_port_block_til_ready()
>     tty_port_raise_dtr_rts()  # if (C_BAUD(tty))
>       uart_dtr_rts()
>         uart_port_dtr_rts()
> 
> There's at least three problems here:
> 
> First, if no baud rate is set, RTS polarity is not initialized.
> That's the right thing to do for rs232, but not for rs485, which
> requires that RTS is deasserted unconditionally.
> 
> Second, if the DeviceTree property "linux,rs485-enabled-at-boot-time" is
> present, RTS should be deasserted as early as possible, i.e. on probe.
> Otherwise it may remain asserted until first open.
> 
> Third, even though RTS is deasserted on open and close, it may
> subsequently be asserted by uart_throttle(), uart_unthrottle() or
> uart_set_termios() because those functions aren't rs485-aware.
> (Only uart_tiocmset() is.)
> 
> To address these issues, move RTS initialization from uart_port_dtr_rts()
> to uart_configure_port().  Prevent subsequent modification of RTS
> polarity by moving the existing rs485 check from uart_tiocmget() to
> uart_update_mctrl().
> 
> That way, RTS is initialized on probe and then remains unmodified unless
> the uart transmits data.  If rs485 is enabled at runtime (instead of at
> boot) through a TIOCSRS485 ioctl(), RTS is initialized by the uart
> driver's ->rs485_config() callback and then likewise remains unmodified.
> 
> The PL011 driver initializes RTS on uart open and prevents subsequent
> modification in its ->set_mctrl() callback.  That code is obsoleted by
> the present commit, so drop it.
> 
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Su Bao Cheng <baocheng.su@siemens.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Link: https://lore.kernel.org/r/2d2acaf3a69e89b7bf687c912022b11fd29dfa1e.1642909284.git.lukas@wunner.de
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable@vger.kernel.org # 5.10
> Reported-by: Daisuke Mizobuchi <mizo@atmark-techno.com>
> Tested-by: Daisuke Mizobuchi <mizo@atmark-techno.com>
> Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
> ---
> Notes:
>  - as said in commit message, I've dropped the PL011 part of the
> original patch as it is orthogonal to this change. We need this
> serial core fix for the imx serial tty driver.
> 
> - I wasn't really sure what to do with tags in the commit message,
> everything below the 'Cc: stable' tag apply to the backport:
> Mizobuchi-san tested the backport on the 5.10 branch with the imx
> driver.
> 
> - I'm not quite sure how far back it is relevant, for imx I assume
> rs485 is broken since 58362d5be352 ("serial: imx: implement handshaking
> using gpios with the mctrl_gpio helper") (4.5), and we did have that
> problem all the way back in our older 4.9 product tree... but core
> support back then wasn't as extensive for RS485 so we have a different
> imx specific workaround there.
> 
>  - I do not use 5.15 but either version of this patch apply cleanly
> there; I'd assume it'd be more appropriate to get the original
> 2dd8a74fddd21b cherry-picked in this case for 5.15.

Now queued up, thanks.

greg k-h