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@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) { 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 -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature