* [PATCH] serial: imx: support RS-485 Rx disable on Tx
@ 2016-02-28 9:25 Baruch Siach
2016-02-28 9:56 ` Uwe Kleine-König
0 siblings, 1 reply; 4+ messages in thread
From: Baruch Siach @ 2016-02-28 9:25 UTC (permalink / raw)
To: linux-arm-kernel
Some RS-232 to RS-485 transceivers require Rx to be disabled on Tx to
avoid echo of Tx data into the Rx buffer. Specifically, the XR3160E
RS-232/RS-485/RS-422 transceiver behaves this way.
This commit disables Rx on active Tx when SER_RS485_ENABLED is active and
SER_RS485_RX_DURING_TX is disabled.
Note that this is a change in behavior of the driver. Until now
SER_RS485_RX_DURING_TX was enabled unconditionally even when disabled in
the TIOCSRS485 ioctl serial_rs485 flags field.
Cc: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
drivers/tty/serial/imx.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 9362f54c816c..333d34ff358c 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -361,6 +361,8 @@ static void imx_stop_tx(struct uart_port *port)
imx_port_rts_inactive(sport, &temp);
else
imx_port_rts_active(sport, &temp);
+ if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
+ temp |= UCR2_RXEN;
writel(temp, port->membase + UCR2);
temp = readl(port->membase + UCR4);
@@ -568,6 +570,8 @@ static void imx_start_tx(struct uart_port *port)
imx_port_rts_inactive(sport, &temp);
else
imx_port_rts_active(sport, &temp);
+ if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
+ temp &= ~UCR2_RXEN;
writel(temp, port->membase + UCR2);
/* enable transmitter and shifter empty irq */
@@ -1614,7 +1618,6 @@ static int imx_rs485_config(struct uart_port *port,
/* unimplemented */
rs485conf->delay_rts_before_send = 0;
rs485conf->delay_rts_after_send = 0;
- rs485conf->flags |= SER_RS485_RX_DURING_TX;
/* RTS is required to control the transmitter */
if (!sport->have_rtscts)
--
2.7.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] serial: imx: support RS-485 Rx disable on Tx
2016-02-28 9:25 [PATCH] serial: imx: support RS-485 Rx disable on Tx Baruch Siach
@ 2016-02-28 9:56 ` Uwe Kleine-König
2016-02-28 10:23 ` Baruch Siach
0 siblings, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2016-02-28 9:56 UTC (permalink / raw)
To: linux-arm-kernel
Hello Baruch,
On Sun, Feb 28, 2016 at 11:25:51AM +0200, Baruch Siach wrote:
> Some RS-232 to RS-485 transceivers require Rx to be disabled on Tx to
> avoid echo of Tx data into the Rx buffer. Specifically, the XR3160E
> RS-232/RS-485/RS-422 transceiver behaves this way.
>
> This commit disables Rx on active Tx when SER_RS485_ENABLED is active and
> SER_RS485_RX_DURING_TX is disabled.
>
> Note that this is a change in behavior of the driver. Until now
But this change is a good one (assuming it does what it advertises :-).
Userspace got informed before that SER_RS485_RX_DURING_TX is enabled, so
this is not an incompatible change.
Best regards
Uwe
> SER_RS485_RX_DURING_TX was enabled unconditionally even when disabled in
> the TIOCSRS485 ioctl serial_rs485 flags field.
>
> Cc: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> drivers/tty/serial/imx.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 9362f54c816c..333d34ff358c 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -361,6 +361,8 @@ static void imx_stop_tx(struct uart_port *port)
> imx_port_rts_inactive(sport, &temp);
> else
> imx_port_rts_active(sport, &temp);
> + if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
> + temp |= UCR2_RXEN;
> writel(temp, port->membase + UCR2);
>
> temp = readl(port->membase + UCR4);
> @@ -568,6 +570,8 @@ static void imx_start_tx(struct uart_port *port)
> imx_port_rts_inactive(sport, &temp);
> else
> imx_port_rts_active(sport, &temp);
> + if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
> + temp &= ~UCR2_RXEN;
> writel(temp, port->membase + UCR2);
Can this happen:
- SER_RS485_RX_DURING_TX is off
- thread A starts sending (and so disables RX)
- thread B sets SER_RS485_RX_DURING_TX
- thread A finishes sending, and doesn't restore RXEN.
?
Even if this cannot happen it might be more robust to restore RXEN
unconditionally in imx_stop_tx?!
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] serial: imx: support RS-485 Rx disable on Tx
2016-02-28 9:56 ` Uwe Kleine-König
@ 2016-02-28 10:23 ` Baruch Siach
2016-02-29 8:51 ` Uwe Kleine-König
0 siblings, 1 reply; 4+ messages in thread
From: Baruch Siach @ 2016-02-28 10:23 UTC (permalink / raw)
To: linux-arm-kernel
Hi Uwe,
Thanks for your prompt response.
On Sun, Feb 28, 2016 at 10:56:01AM +0100, Uwe Kleine-K?nig wrote:
> On Sun, Feb 28, 2016 at 11:25:51AM +0200, Baruch Siach wrote:
> > Some RS-232 to RS-485 transceivers require Rx to be disabled on Tx to
> > avoid echo of Tx data into the Rx buffer. Specifically, the XR3160E
> > RS-232/RS-485/RS-422 transceiver behaves this way.
> >
> > This commit disables Rx on active Tx when SER_RS485_ENABLED is active and
> > SER_RS485_RX_DURING_TX is disabled.
> >
> > Note that this is a change in behavior of the driver. Until now
>
> But this change is a good one (assuming it does what it advertises :-).
> Userspace got informed before that SER_RS485_RX_DURING_TX is enabled, so
> this is not an incompatible change.
I thought it is a good idea to mention this fact in the commit log anyway. It
is not hard to imagine broken userspace being affected by this change.
> > SER_RS485_RX_DURING_TX was enabled unconditionally even when disabled in
> > the TIOCSRS485 ioctl serial_rs485 flags field.
> >
> > Cc: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> > drivers/tty/serial/imx.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > index 9362f54c816c..333d34ff358c 100644
> > --- a/drivers/tty/serial/imx.c
> > +++ b/drivers/tty/serial/imx.c
> > @@ -361,6 +361,8 @@ static void imx_stop_tx(struct uart_port *port)
> > imx_port_rts_inactive(sport, &temp);
> > else
> > imx_port_rts_active(sport, &temp);
> > + if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
> > + temp |= UCR2_RXEN;
> > writel(temp, port->membase + UCR2);
> >
> > temp = readl(port->membase + UCR4);
> > @@ -568,6 +570,8 @@ static void imx_start_tx(struct uart_port *port)
> > imx_port_rts_inactive(sport, &temp);
> > else
> > imx_port_rts_active(sport, &temp);
> > + if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
> > + temp &= ~UCR2_RXEN;
> > writel(temp, port->membase + UCR2);
>
> Can this happen:
>
> - SER_RS485_RX_DURING_TX is off
> - thread A starts sending (and so disables RX)
> - thread B sets SER_RS485_RX_DURING_TX
> - thread A finishes sending, and doesn't restore RXEN.
>
> ?
>
> Even if this cannot happen it might be more robust to restore RXEN
> unconditionally in imx_stop_tx?!
Sounds like a good idea. But if I take your comment to its logical conclusion,
thread B might just disable SER_RS485_ENABLED entirely. Would it make sense to
restore RXEN outside the 'if (port->rs485.flags & SER_RS485_ENABLED)' block?
Or maybe we should just set RXEN in imx_rs485_config() when
SER_RS485_RX_DURING_TX is enabled?
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] serial: imx: support RS-485 Rx disable on Tx
2016-02-28 10:23 ` Baruch Siach
@ 2016-02-29 8:51 ` Uwe Kleine-König
0 siblings, 0 replies; 4+ messages in thread
From: Uwe Kleine-König @ 2016-02-29 8:51 UTC (permalink / raw)
To: linux-arm-kernel
Hello Baruch,
On Sun, Feb 28, 2016 at 12:23:23PM +0200, Baruch Siach wrote:
> Thanks for your prompt response.
>
> On Sun, Feb 28, 2016 at 10:56:01AM +0100, Uwe Kleine-K?nig wrote:
> > On Sun, Feb 28, 2016 at 11:25:51AM +0200, Baruch Siach wrote:
> > > Some RS-232 to RS-485 transceivers require Rx to be disabled on Tx to
> > > avoid echo of Tx data into the Rx buffer. Specifically, the XR3160E
> > > RS-232/RS-485/RS-422 transceiver behaves this way.
> > >
> > > This commit disables Rx on active Tx when SER_RS485_ENABLED is active and
> > > SER_RS485_RX_DURING_TX is disabled.
> > >
> > > Note that this is a change in behavior of the driver. Until now
> >
> > But this change is a good one (assuming it does what it advertises :-).
> > Userspace got informed before that SER_RS485_RX_DURING_TX is enabled, so
> > this is not an incompatible change.
>
> I thought it is a good idea to mention this fact in the commit log anyway. It
> is not hard to imagine broken userspace being affected by this change.
>
> > > SER_RS485_RX_DURING_TX was enabled unconditionally even when disabled in
> > > the TIOCSRS485 ioctl serial_rs485 flags field.
> > >
> > > Cc: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > > ---
> > > drivers/tty/serial/imx.c | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > > index 9362f54c816c..333d34ff358c 100644
> > > --- a/drivers/tty/serial/imx.c
> > > +++ b/drivers/tty/serial/imx.c
> > > @@ -361,6 +361,8 @@ static void imx_stop_tx(struct uart_port *port)
> > > imx_port_rts_inactive(sport, &temp);
> > > else
> > > imx_port_rts_active(sport, &temp);
> > > + if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
> > > + temp |= UCR2_RXEN;
> > > writel(temp, port->membase + UCR2);
> > >
> > > temp = readl(port->membase + UCR4);
> > > @@ -568,6 +570,8 @@ static void imx_start_tx(struct uart_port *port)
> > > imx_port_rts_inactive(sport, &temp);
> > > else
> > > imx_port_rts_active(sport, &temp);
> > > + if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
> > > + temp &= ~UCR2_RXEN;
> > > writel(temp, port->membase + UCR2);
> >
> > Can this happen:
> >
> > - SER_RS485_RX_DURING_TX is off
> > - thread A starts sending (and so disables RX)
> > - thread B sets SER_RS485_RX_DURING_TX
> > - thread A finishes sending, and doesn't restore RXEN.
> >
> > ?
> >
> > Even if this cannot happen it might be more robust to restore RXEN
> > unconditionally in imx_stop_tx?!
>
> Sounds like a good idea. But if I take your comment to its logical conclusion,
> thread B might just disable SER_RS485_ENABLED entirely. Would it make sense to
> restore RXEN outside the 'if (port->rs485.flags & SER_RS485_ENABLED)' block?
> Or maybe we should just set RXEN in imx_rs485_config() when
> SER_RS485_RX_DURING_TX is enabled?
The latter sounds like the right thing to do.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-02-29 8:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-28 9:25 [PATCH] serial: imx: support RS-485 Rx disable on Tx Baruch Siach
2016-02-28 9:56 ` Uwe Kleine-König
2016-02-28 10:23 ` Baruch Siach
2016-02-29 8:51 ` Uwe Kleine-König
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox