On Fri, Feb 16, 2024 at 4:17 PM Marek Vasut <marex@xxxxxxx> wrote:
>
> On 2/16/24 13:46, Rickard Andersson wrote:
> > From: Rickard x Andersson <rickaran@xxxxxxxx>
> >
> > When about to transmit the function imx_uart_start_tx is called and in
> > some RS485 configurations this function will call imx_uart_stop_rx. The
> > problem is that imx_uart_stop_rx will enable loopback and when loopback
> > is enabled transmitted data will just be looped to RX.
> >
> > This patch fixes the above problem by explicitly disabling loopback in
> > the case described above.
> >
> > Signed-off-by: Rickard x Andersson <rickaran@xxxxxxxx>
>
> Fixes: tag is missing.
Ok, I will add.
>
> > ---
> > drivers/tty/serial/imx.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > index 4aa72d5aeafb..899e331bdfc8 100644
> > --- a/drivers/tty/serial/imx.c
> > +++ b/drivers/tty/serial/imx.c
> > @@ -683,8 +683,15 @@ static void imx_uart_start_tx(struct uart_port *port)
> > imx_uart_writel(sport, ucr2, UCR2);
> >
> > if (!(port->rs485.flags & SER_RS485_RX_DURING_TX) &&
> > - !port->rs485_rx_during_tx_gpio)
> > + !port->rs485_rx_during_tx_gpio) {
> > imx_uart_stop_rx(port);
> > + /*
> > + * The function imx_uart_stop_rx right above
> > + * will enable loopback, but since we are just
> > + * about to transmit then disable loopback.
> > + */
> > + imx_uart_disable_loopback_rs485(sport);
> > + }
>
> Maybe it would be better to update the stop_rx and add parameter,
> whether it should/shouldn't enable the loopback ?
Since *stop_rx is part of struct uart_ops I can not add an input argument to the function.
Best regards,
Rickard
________________________________________
From: Marek Vasut <marex@xxxxxxx>
Sent: Friday, February 16, 2024 4:17 PM
To: Rickard X Andersson; linux-serial@xxxxxxxxxxxxxxx; rickard314.andersson@xxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; linux-imx@xxxxxxx; martin.fuzzey@flowbird.group; Christoph Niedermaier
Cc: kernel
Subject: Re: [PATCH] tty: serial: imx: Fix broken RS485
On 2/16/24 13:46, Rickard Andersson wrote:
> From: Rickard x Andersson <rickaran@xxxxxxxx>
>
> When about to transmit the function imx_uart_start_tx is called and in
> some RS485 configurations this function will call imx_uart_stop_rx. The
> problem is that imx_uart_stop_rx will enable loopback and when loopback
> is enabled transmitted data will just be looped to RX.
>
> This patch fixes the above problem by explicitly disabling loopback in
> the case described above.
>
> Signed-off-by: Rickard x Andersson <rickaran@xxxxxxxx>
Fixes: tag is missing.
> ---
> drivers/tty/serial/imx.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 4aa72d5aeafb..899e331bdfc8 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -683,8 +683,15 @@ static void imx_uart_start_tx(struct uart_port *port)
> imx_uart_writel(sport, ucr2, UCR2);
>
> if (!(port->rs485.flags & SER_RS485_RX_DURING_TX) &&
> - !port->rs485_rx_during_tx_gpio)
> + !port->rs485_rx_during_tx_gpio) {
> imx_uart_stop_rx(port);
> + /*
> + * The function imx_uart_stop_rx right above
> + * will enable loopback, but since we are just
> + * about to transmit then disable loopback.
> + */
> + imx_uart_disable_loopback_rs485(sport);
> + }
Maybe it would be better to update the stop_rx and add parameter,
whether it should/shouldn't enable the loopback ?
