[PATCH] tty: serial: imx: Handle RS485 DE signal active high
Christoph Niedermaier
cniedermaier at dh-electronics.com
Thu Sep 29 07:18:12 PDT 2022
From: Marek Vasut [mailto:marex at 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 at denx.de>
> ---
> Cc: Christoph Niedermaier <cniedermaier at dh-electronics.com>
> Cc: Fabio Estevam <festevam at denx.de>
> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> Cc: Jiri Slaby <jirislaby at kernel.org>
> Cc: NXP Linux Team <linux-imx at nxp.com>
> Cc: Peng Fan <peng.fan at nxp.com>
> Cc: Sascha Hauer <s.hauer at pengutronix.de>
> Cc: Shawn Guo <shawnguo at kernel.org>
> Cc: kernel at dh-electronics.com
> Cc: kernel at pengutronix.de
> Cc: linux-arm-kernel at lists.infradead.org
> To: linux-serial at 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 at dh-electronics.com>
Regards
Christoph
More information about the linux-arm-kernel
mailing list