Re: [PATCH v2] tty: serial: imx: Handle RS485 DE signal active high

[Fabio Estevam <festevam@denx.de>]

Hi Marek,

Thanks for the detailed commit log.

Only some nits below:

On 29/09/2022 11:44, 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

s/behave/behaves

> 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

s/assering/asserting

> 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
>   >;
> };
> "
> 
> Tested-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Fabio Estevam <festevam@denx.de>