Re: [PATCH v4] serial: Deassert Transmit Enable on probe in driver-specific way

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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>



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux