Re: [PATCH v3 2/2] tty: imx serial: Implement support for reversing TX and RX polarity

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

On Wed, Feb 12, 2020 at 10:35:38AM -0600, George Hilliard wrote:
> The peripheral has support for inverting its input and/or output
> signals.  This is useful if the hardware flips polarity of the
> peripheral's signal, such as swapped +/- pins on an RS-422 transceiver,
> or an inverting level shifter.  Add support for these control registers
> via the device tree binding.
> 
> Signed-off-by: George Hilliard <ghilliard@kopismobile.com>
> ---
> v1..v2: Remove confidentiality spam
> v2..v3: Set *and* clear register, and do it before TX enable
> 
>  drivers/tty/serial/imx.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 0c6c63166250..205627bcad66 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -195,6 +195,8 @@ struct imx_port {
>  	unsigned int		have_rtscts:1;
>  	unsigned int		have_rtsgpio:1;
>  	unsigned int		dte_mode:1;
> +	unsigned int		inverted_tx:1;
> +	unsigned int		inverted_rx:1;
>  	struct clk		*clk_ipg;
>  	struct clk		*clk_per;
>  	const struct imx_uart_data *devdata;
> @@ -1335,7 +1337,7 @@ static int imx_uart_startup(struct uart_port *port)
>  	int retval, i;
>  	unsigned long flags;
>  	int dma_is_inited = 0;
> -	u32 ucr1, ucr2, ucr4;
> +	u32 ucr1, ucr2, ucr3, ucr4;
>  
>  	retval = clk_prepare_enable(sport->clk_per);
>  	if (retval)
> @@ -1390,8 +1392,22 @@ static int imx_uart_startup(struct uart_port *port)
>  	ucr4 = imx_uart_readl(sport, UCR4) & ~UCR4_OREN;
>  	if (!sport->dma_is_enabled)
>  		ucr4 |= UCR4_OREN;
> +	if (sport->inverted_rx)
> +		ucr4 |= UCR4_INVR;
> +	else
> +		ucr4 &= ~UCR4_INVR;

Maybe clear UCR4_INVR in the same way as UCR4_OREN is cleared just
above?

>  	imx_uart_writel(sport, ucr4, UCR4);
>  
> +	/*
> +	 * configure tx polarity before enabling tx
> +	 */
> +	ucr3 = imx_uart_readl(sport, UCR3);
> +	if (sport->inverted_tx)
> +		ucr3 |= UCR3_INVT;
> +	else
> +		ucr3 &= ~UCR3_INVT;
> +	imx_uart_writel(sport, ucr3, UCR3);
> +
>  	ucr2 = imx_uart_readl(sport, UCR2) & ~UCR2_ATEN;
>  	ucr2 |= (UCR2_RXEN | UCR2_TXEN);
>  	if (!sport->have_rtscts)
> @@ -1405,10 +1421,6 @@ static int imx_uart_startup(struct uart_port *port)
>  	imx_uart_writel(sport, ucr2, UCR2);
>  
>  	if (!imx_uart_is_imx1(sport)) {

If this complete if block would be moved up, you only need to write this
register once.

> -		u32 ucr3;
> -
> -		ucr3 = imx_uart_readl(sport, UCR3);
> -
>  		ucr3 |= UCR3_DTRDEN | UCR3_RI | UCR3_DCD;
>  
>  		if (sport->dte_mode)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |