On Mon, Oct 10, 2022 at 03:17:14PM +0900, Dominique MARTINET wrote:
> +Cc stable
>
> Lukas Wunner wrote on Thu, Sep 22, 2022 at 06:27:33PM +0200:
> > When a UART port is newly registered, uart_configure_port() seeks to
> > deassert RS485 Transmit Enable by setting the RTS bit in port->mctrl.
> > However a number of UART drivers interpret a set RTS bit as *assertion*
> > instead of deassertion: Affected drivers include those using
> > serial8250_em485_config() (except 8250_bcm2835aux.c) and some using
> > mctrl_gpio (e.g. imx.c).
> >
> > Since the interpretation of the RTS bit is driver-specific, it is not
> > suitable as a means to centrally deassert Transmit Enable in the serial
> > core. Instead, the serial core must call on drivers to deassert it in
> > their driver-specific way. One way to achieve that is to call
> > ->rs485_config(). It implicitly deasserts Transmit Enable.
> >
> > So amend uart_configure_port() and uart_resume_port() to invoke
> > uart_rs485_config(). That allows removing calls to uart_rs485_config()
> > from drivers' ->probe() hooks and declaring the function static.
> >
> > Skip any invocation of ->set_mctrl() if RS485 is enabled. RS485 has no
> > hardware flow control, so the modem control lines are irrelevant and
> > need not be touched. When leaving RS485 mode, reset the modem control
> > lines to the state stored in port->mctrl. That way, UARTs which are
> > muxed between RS485 and RS232 transceivers drive the lines correctly
> > when switched to RS232. (serial8250_do_startup() historically raises
> > the OUT1 modem signal because otherwise interrupts are not signaled on
> > ancient PC UARTs, but I believe that no longer applies to modern,
> > RS485-capable UARTs and is thus safe to be skipped.)
> >
> > imx.c modifies port->mctrl whenever Transmit Enable is asserted and
> > deasserted. Stop it from doing that so port->mctrl reflects the RS232
> > line state.
> >
> > 8250_omap.c deasserts Transmit Enable on ->runtime_resume() by calling
> > ->set_mctrl(). Because that is now a no-op in RS485 mode, amend the
> > function to call serial8250_em485_stop_tx().
> >
> > fsl_lpuart.c retrieves and applies the RS485 device tree properties
> > after registering the UART port. Because applying now happens on
> > registration in uart_configure_port(), move retrieval of the properties
> > ahead of uart_add_one_port().
> >
> > Fixes: d3b3404df318 ("serial: Fix incorrect rs485 polarity on uart open")
>
> Thanks for this fix!
> We also noticed rs485 DE was initially wrong last week and I noticed
> this when I was about to send a patch that just inverted the
> SER_RS485_RTS_AFTER_SEND check in uart_configure_port, but after reading
> the commit message here it's a lot more complicated than that depending
> on the serial driver...
> (fixing commit 2dd8a74fddd2 ("serial: core: Initialize rs485 RTS
> polarity already on probe"), but it's actually the same problem in the
> opposite direction)
>
>
> Unfortunately you've marked this for v4.14+ stable, but it doesn't even
> apply to 5.19.14 due to (at least) commit d8fcd9cfbde5 ("serial: core:
> move sanitizing of RS485 delays into own function"), so it hasn't been
> picked up yet; since quite a bit of code was cleaned up/moved one will
> need to pay a bit attention when doing that.
>
> What would you like to do for stable branches?
> Would you be able to send a patch that applies on older 5.10 and 5.15
> where commit d3b3404df318 ("serial: Fix incorrect rs485 polarity on uart
> open") has been backported?
>
> If that sounds too complicated we could probably just revert a handful
> of serial_core/rs485 commits, but going forward sounds more future-proof
> to me.
>
> Thanks!
> (nothing below, leaving quote for stable@)
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.
</formletter>
