| Message ID | 20220928044037.605217-1-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tty: serial: imx: Handle RS485 DE signal active high | expand |
On Wed, Sep 28, 2022 at 06:40:37AM +0200, Marek Vasut wrote: > 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 > >; > }; > " An alternative work around is to use the GPIO as RTS also in default, isn't it? > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com> > Cc: Fabio Estevam <festevam@denx.de> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Jiri Slaby <jirislaby@kernel.org> > Cc: NXP Linux Team <linux-imx@nxp.com> > Cc: Peng Fan <peng.fan@nxp.com> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: kernel@dh-electronics.com > Cc: kernel@pengutronix.de > Cc: linux-arm-kernel@lists.infradead.org > To: linux-serial@vger.kernel.org > --- > 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) { IMHO this needs a comment. The later hunks might benefit from a comment two. I'd explain it once in more detail and refer to that description from the other relevant code sections. Best regards Uwe
On 9/28/22 09:43, Uwe Kleine-König wrote: > On Wed, Sep 28, 2022 at 06:40:37AM +0200, Marek Vasut wrote: >> 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 >> >; >> }; >> " > > An alternative work around is to use the GPIO as RTS also in default, > isn't it? No, see paragraph 6 in the wall of text above. The CTSC/CTS is synchronous to the internal UART IP clock. The GPIO workaround has downsides due to switching lag which breaks various deployments. Here the GPIO is only used to keep the CTS line LOW on boot ; after the driver takes over, the driver controls the CTS line all the time. >> @@ -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) { > > IMHO this needs a comment. The later hunks might benefit from a comment > two. I'd explain it once in more detail and refer to that description > from the other relevant code sections. Yes, I can add some abbreviated version of the commit message as a comment.
From: Marek Vasut [mailto:marex@denx.de] 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@denx.de> > --- > Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com> > Cc: Fabio Estevam <festevam@denx.de> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Jiri Slaby <jirislaby@kernel.org> > Cc: NXP Linux Team <linux-imx@nxp.com> > Cc: Peng Fan <peng.fan@nxp.com> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: kernel@dh-electronics.com > Cc: kernel@pengutronix.de > Cc: linux-arm-kernel@lists.infradead.org > To: linux-serial@vger.kernel.org > --- > 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@dh-electronics.com> Regards Christoph
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
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@denx.de> --- Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com> Cc: Fabio Estevam <festevam@denx.de> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Jiri Slaby <jirislaby@kernel.org> Cc: NXP Linux Team <linux-imx@nxp.com> Cc: Peng Fan <peng.fan@nxp.com> Cc: Sascha Hauer <s.hauer@pengutronix.de> Cc: Shawn Guo <shawnguo@kernel.org> Cc: kernel@dh-electronics.com Cc: kernel@pengutronix.de Cc: linux-arm-kernel@lists.infradead.org To: linux-serial@vger.kernel.org --- drivers/tty/serial/imx.c | 51 +++++++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 6 deletions(-)