From: Marek Vasut [mailto:marex@xxxxxxx] Sent: Wednesday, September 28, 2022 6:41 AM > The default polarity of RS485 DE signal is active high. This driver does > not handle such case properly. Currently, when a pin is multiplexed as a > UART CTS_B on boot, this pin is pulled HIGH by the i.MX UART CTS circuit, > which activates DE signal on the RS485 transceiver and thus behave as if > the RS485 was transmitting data, so the system blocks the RS485 bus when > it starts and until user application takes over. This behavior is not OK. > The problem consists of two separate parts. > > First, the i.MX UART IP requires UCR1 UARTEN and UCR2 RXEN to be set for > UCR2 CTSC and CTS bits to have any effect. The UCR2 CTSC bit permits the > driver to set CTS (RTS_B or RS485 DE signal) to either level sychronous > to the internal UART IP clock. Compared to other options, like GPIO CTS > control, this has the benefit of being synchronous to the UART IP clock > and thus without glitches or bus delays. The reason for the CTS design > is likely because when the Receiver is disabled, the UART IP can never > indicate that it is ready to receive data by assering CTS signal, so > the CTS is always pulled HIGH by default. > > When the port is closed by user space, imx_uart_stop_rx() clears UCR2 > RXEN bit, and imx_uart_shutdown() clears UCR1 UARTEN bit. This disables > UART Receiver and UART itself, and forces CTS signal HIGH, which leads > to the RS485 bus being blocked because RS485 DE is incorrectly active. > > The proposed solution for this problem is to keep the Receiver running > even after the port is closed, but in loopback mode. This disconnects > the RX FIFO input from the RXD external signal, and since UCR2 TXEN is > cleared, the UART Transmitter is disabled, so nothing can feed data in > the RX FIFO. Because the Receiver is still enabled, the UCR2 CTSC and > CTS bits still have effect and the CTS (RS485 DE) control is retained. > > Note that in case of RS485 DE signal active low, there is no problem and > no special handling is necessary. The CTS signal defaults to HIGH, thus > the RS485 is by default set to Receive and the bus is not blocked. > > Note that while there is the possibility to control CTS using GPIO with > either CTS polarity, this has the downside of not being synchronous to > the UART IP clock and thus glitchy and susceptible to slow DE switching. > > Second, on boot, before the UART driver probe callback is called, the > driver core triggers pinctrl_init_done() and configures the IOMUXC to > default state. At this point, UCR1 UARTEN and UCR2 RXEN are both still > cleared, but UART CTS_B (RS485 DE) is configured as CTS function, thus > the RTS signal is pulled HIGH by the UART IP CTS circuit. > > One part of the solution here is to enable UCR1 UARTEN and UCR2 RXEN and > UTS loopback in this driver probe callback, thus unblocking the CTSC and > CTS control early on. But this is still too late, since the pin control > is already configured and CTS has been pulled HIGH for a short period > of time. > > When Linux kernel boots and this driver is bound, the pin control is set > to special "init" state if the state is available, and driver can switch > the "default" state afterward when ready. This state can be used to set > the CTS line as a GPIO in DT temporarily, and a GPIO hog can force such > GPIO to LOW, thus keeping the RS485 DE line LOW early on boot. Once the > driver takes over and UCR1 UARTEN and UCR2 RXEN and UTS loopback are all > enabled, the driver can switch to "default" pin control state and control > the CTS line as function instead. DT binding example is below: > > " > &gpio6 { > rts-init-hog { > gpio-hog; > gpios = <5 0>; > output-low; > line-name = "rs485-de"; > }; > }; > > &uart5 { /* DHCOM UART2 */ > pinctrl-0 = <&pinctrl_uart5>; > pinctrl-1 = <&pinctrl_uart5_init>; > pinctrl-names = "default", "init"; > ... > }; > pinctrl_uart5_init: uart5-init-grp { > fsl,pins = < > ... > MX6QDL_PAD_CSI0_DAT19__GPIO6_IO05 0x30b1 > >; > }; > > pinctrl_uart5: uart5-grp { > fsl,pins = < > ... > MX6QDL_PAD_CSI0_DAT19__UART5_CTS_B 0x30b1 > >; > }; > " > > Signed-off-by: Marek Vasut <marex@xxxxxxx> > --- > Cc: Christoph Niedermaier <cniedermaier@xxxxxxxxxxxxxxxxxx> > Cc: Fabio Estevam <festevam@xxxxxxx> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > Cc: Jiri Slaby <jirislaby@xxxxxxxxxx> > Cc: NXP Linux Team <linux-imx@xxxxxxx> > Cc: Peng Fan <peng.fan@xxxxxxx> > Cc: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> > Cc: Shawn Guo <shawnguo@xxxxxxxxxx> > Cc: kernel@xxxxxxxxxxxxxxxxxx > Cc: kernel@xxxxxxxxxxxxxx > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > To: linux-serial@xxxxxxxxxxxxxxx > --- > drivers/tty/serial/imx.c | 51 +++++++++++++++++++++++++++++++++++----- > 1 file changed, 45 insertions(+), 6 deletions(-) > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index 05b432dc7a85c..144f1cedd4b64 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -489,7 +489,7 @@ static void imx_uart_stop_tx(struct uart_port *port) > static void imx_uart_stop_rx(struct uart_port *port) > { > struct imx_port *sport = (struct imx_port *)port; > - u32 ucr1, ucr2, ucr4; > + u32 ucr1, ucr2, ucr4, uts; > > ucr1 = imx_uart_readl(sport, UCR1); > ucr2 = imx_uart_readl(sport, UCR2); > @@ -505,7 +505,17 @@ static void imx_uart_stop_rx(struct uart_port *port) > imx_uart_writel(sport, ucr1, UCR1); > imx_uart_writel(sport, ucr4, UCR4); > > - ucr2 &= ~UCR2_RXEN; > + if (port->rs485.flags & SER_RS485_ENABLED && > + port->rs485.flags & SER_RS485_RTS_ON_SEND && > + sport->have_rtscts && !sport->have_rtsgpio) { > + uts = imx_uart_readl(sport, imx_uart_uts_reg(sport)); > + uts |= UTS_LOOP; > + imx_uart_writel(sport, uts, imx_uart_uts_reg(sport)); > + ucr2 |= UCR2_RXEN; > + } else { > + ucr2 &= ~UCR2_RXEN; > + } > + > imx_uart_writel(sport, ucr2, UCR2); > } > > @@ -1393,7 +1403,7 @@ static int imx_uart_startup(struct uart_port *port) > int retval, i; > unsigned long flags; > int dma_is_inited = 0; > - u32 ucr1, ucr2, ucr3, ucr4; > + u32 ucr1, ucr2, ucr3, ucr4, uts; > > retval = clk_prepare_enable(sport->clk_per); > if (retval) > @@ -1498,6 +1508,10 @@ static int imx_uart_startup(struct uart_port *port) > imx_uart_writel(sport, ucr2, UCR2); > } > > + uts = imx_uart_readl(sport, imx_uart_uts_reg(sport)); > + uts &= ~UTS_LOOP; > + imx_uart_writel(sport, uts, imx_uart_uts_reg(sport)); > + > spin_unlock_irqrestore(&sport->port.lock, flags); > > return 0; > @@ -1507,7 +1521,7 @@ static void imx_uart_shutdown(struct uart_port *port) > { > struct imx_port *sport = (struct imx_port *)port; > unsigned long flags; > - u32 ucr1, ucr2, ucr4; > + u32 ucr1, ucr2, ucr4, uts; > > if (sport->dma_is_enabled) { > dmaengine_terminate_sync(sport->dma_chan_tx); > @@ -1551,7 +1565,17 @@ static void imx_uart_shutdown(struct uart_port *port) > spin_lock_irqsave(&sport->port.lock, flags); > > ucr1 = imx_uart_readl(sport, UCR1); > - ucr1 &= ~(UCR1_TRDYEN | UCR1_RRDYEN | UCR1_RTSDEN | UCR1_UARTEN | UCR1_RXDMAEN | > UCR1_ATDMAEN); > + ucr1 &= ~(UCR1_TRDYEN | UCR1_RRDYEN | UCR1_RTSDEN | UCR1_RXDMAEN | UCR1_ATDMAEN); > + if (port->rs485.flags & SER_RS485_ENABLED && > + port->rs485.flags & SER_RS485_RTS_ON_SEND && > + sport->have_rtscts && !sport->have_rtsgpio) { > + uts = imx_uart_readl(sport, imx_uart_uts_reg(sport)); > + uts |= UTS_LOOP; > + imx_uart_writel(sport, uts, imx_uart_uts_reg(sport)); > + ucr1 |= UCR1_UARTEN; > + } else { > + ucr1 &= ~UCR1_UARTEN; > + } > imx_uart_writel(sport, ucr1, UCR1); > > ucr4 = imx_uart_readl(sport, UCR4); > @@ -2213,7 +2237,7 @@ static int imx_uart_probe(struct platform_device *pdev) > void __iomem *base; > u32 dma_buf_conf[2]; > int ret = 0; > - u32 ucr1; > + u32 ucr1, ucr2, uts; > struct resource *res; > int txirq, rxirq, rtsirq; > > @@ -2350,6 +2374,21 @@ static int imx_uart_probe(struct platform_device *pdev) > ucr1 &= ~(UCR1_ADEN | UCR1_TRDYEN | UCR1_IDEN | UCR1_RRDYEN | UCR1_RTSDEN); > imx_uart_writel(sport, ucr1, UCR1); > > + if (sport->port.rs485.flags & SER_RS485_ENABLED && > + sport->have_rtscts && !sport->have_rtsgpio) { > + uts = imx_uart_readl(sport, imx_uart_uts_reg(sport)); > + uts |= UTS_LOOP; > + imx_uart_writel(sport, uts, imx_uart_uts_reg(sport)); > + > + ucr1 = imx_uart_readl(sport, UCR1); > + ucr1 |= UCR1_UARTEN; > + imx_uart_writel(sport, ucr1, UCR1); > + > + ucr2 = imx_uart_readl(sport, UCR2); > + ucr2 |= UCR2_RXEN; > + imx_uart_writel(sport, ucr2, UCR2); > + } > + > if (!imx_uart_is_imx1(sport) && sport->dte_mode) { > /* > * The DCEDTE bit changes the direction of DSR, DCD, DTR and RI > -- > 2.35.1 Tested-by: Christoph Niedermaier <cniedermaier@xxxxxxxxxxxxxxxxxx> Regards Christoph
