From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4B65EC43219 for ; Thu, 29 Sep 2022 14:20:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:In-Reply-To:References: Message-ID:Date:Subject:CC:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=s4UMCuKDwmvawtx8YPRUKqTW+06fQV3ySncgZ3+HhmE=; b=S7Yj/bCn0xFWeW apoarK0afWPPrItAep4wWTXhD4++jwR+mIpQAabQ1vZBVk1hmHAD7lIrUXcuFwacaDY6LVnAA8iXr TChK1CPw7IsMu7PpbDVynKcX3AcPJtgKlq9TKQ9eREn6SFxhZp4EpEoBQsjn5NAQ5t1hM+VYOgg+4 lo+0gI0QmtyBuQOmfSqix51lTLqACorGT5YGGvNYu9JPgJPiD9O1steWQ/TKWk8UA2jfdBwQOngYe teXbkNes94QjoiwBDIpteINntLKjK7M7E6ixE3MZ8VUn7lZDe1DYJfiWS4XEYqqAe70+61JFRKarZ j2v1fVpRFr9K8ATva1Cw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oduNa-003WsV-GS; Thu, 29 Sep 2022 14:19:10 +0000 Received: from mx3.securetransport.de ([2a01:4f8:c0c:92be::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oduNV-003WqW-0b for linux-arm-kernel@lists.infradead.org; Thu, 29 Sep 2022 14:19:08 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dh-electronics.com; s=dhelectronicscom; t=1664461110; bh=AwiFn7ECY/ufwQvHxZeyUVtG3QY3QXt51r8FcaarO0U=; h=From:To:CC:Subject:Date:References:In-Reply-To:From; b=d+ORmbYpz6J55Yrq/pUiPLKAvNJ6U8hPIcuw9s2+uwwkLy0LcJfMLSuKop7S5T5Bk MglkYrN8tEs1FcpZjUh7zgWriEPVL2NylxNZgkiGblEZE+jwJolYO+m7+A6uxGRGya PoGepZmNdKZQ+t/2uQPAo5KWaE5m6Y0zOVX9WnhDMjNk+qBV1QOX3vdO3wHsoyGPMW v2DkLzXBw7nxo5oNFgc7hpUOctSU9yFOEcof/uNUIoVMYaZRxsFXEezTtrtVgPaumx UYRbK7lVgWHNnoDmo9vW116WNJ+UVcgXhjW4dZ8E+C7zg4jfNiw072ixlELFQ/1lJi DpU+lay+69JFQ== X-secureTransport-forwarded: yes From: Christoph Niedermaier Complaints-To: abuse@cubewerk.de To: Marek Vasut CC: "linux-serial@vger.kernel.org" , "Fabio Estevam" , Greg Kroah-Hartman , Jiri Slaby , NXP Linux Team , "Peng Fan" , Sascha Hauer , Shawn Guo , kernel , "kernel@pengutronix.de" , "linux-arm-kernel@lists.infradead.org" Subject: RE: [PATCH] tty: serial: imx: Handle RS485 DE signal active high Thread-Topic: [PATCH] tty: serial: imx: Handle RS485 DE signal active high Thread-Index: AQHY0vRzG7gh2J3cq0u4h1BlAPYZiq32dZqw Date: Thu, 29 Sep 2022 14:18:12 +0000 Message-ID: <91ecdcb2525e451cb47a1d238932e83f@dh-electronics.com> References: <20220928044037.605217-1-marex@denx.de> In-Reply-To: <20220928044037.605217-1-marex@denx.de> Accept-Language: de-DE, en-US Content-Language: de-DE X-MS-Has-Attach: X-MS-TNEF-Correlator: MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220929_071905_572310_7F09A02C X-CRM114-Status: GOOD ( 44.34 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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 > --- > Cc: Christoph Niedermaier > Cc: Fabio Estevam > Cc: Greg Kroah-Hartman > Cc: Jiri Slaby > Cc: NXP Linux Team > Cc: Peng Fan > Cc: Sascha Hauer > Cc: Shawn Guo > 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 Regards Christoph _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel