Hello Ian,
On Wed, Sep 13, 2017 at 11:26:02PM +0100, Ian Arkver wrote:
> On 13/09/17 22:11, Uwe Kleine-König wrote:
> > RS485 uses a single differential signal pair for both sending and
> > receiving. So there is an additional signal necessary to switch between
> > sending and receiving. Usually this signal is RTS, there are a few
> > pieces of evidence:
> >
> > - the rs485 device tree binding calls this signal RTS;
> > - all drivers I checked use RTS for direction switching
> > (atmel_serial, crisv10, imx, fsl_lpuart, omap-serial, sc16is7xx.c);
> > - nearly all machines I saw called this signal RTS in their schematic;
> > and
> > - the members of struct serial_rs485 and related defines use RTS in
> > their names.
> >
> > With such a setup using the imx driver I have the problem that opening a
> > serial device that is in RS485 mode for reading enables RTS via:
> >
> > tty_port_open() -> tty_port_block_til_ready() ->
> > tty_port_raise_dtr_rts() -> uart_dtr_rts() ->
> > uart_update_mctrl()
> >
> > and so disables receiving. The imx driver supports two different ways to
> > drive RTS: a) a native mode using UCR2_CTS and UCR2_CTSC (Freescale
> > named the UART signals from the modem POV, so what we call RTS is CTS in
> > the registers) and b) a gpio.
> >
> > For case a) imx_set_mctrl already ignores TIOCM_RTS if the uart is in
> > RS485 mode. For case b) however it doesn't and so data may be missed to
> > read.
> >
> > As I expect all drivers have the same problem it makes sense to fix this
> > in serial_core instead of teaching the imx driver to also ignore RTS for
> > the gpio case.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> > ---
> > drivers/tty/serial/serial_core.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> > index 2b74ad9bfc64..8a52f28cf1ce 100644
> > --- a/drivers/tty/serial/serial_core.c
> > +++ b/drivers/tty/serial/serial_core.c
> > @@ -1662,15 +1662,23 @@ static void uart_dtr_rts(struct tty_port *port, int onoff)
> > {
> > struct uart_state *state = container_of(port, struct uart_state, port);
> > struct uart_port *uport;
> > + unsigned int dtrrts = TIOCM_DTR | TIOCM_RTS;
> > uport = uart_port_ref(state);
> > if (!uport)
> > return;
> > + /*
> > + * If RS485 is active, don't mess with RTS because that is supposed to
> > + * enable the transmitter
> > + */
> > + if (uport->rs485.flags & SER_RS485_ENABLED)
> > + dtrrts &= ~TIOCM_RTS;
> > +
> > if (onoff)
> > - uart_set_mctrl(uport, TIOCM_DTR | TIOCM_RTS);
> > + uart_set_mctrl(uport, dtrrts);
> > else
> > - uart_clear_mctrl(uport, TIOCM_DTR | TIOCM_RTS);
> > + uart_clear_mctrl(uport, dtrrts);
> > uart_port_deref(uport);
> > }
>
> I haven't tested this but I think, from inspection...
>
> Although you've removed the TIOCM_RTS from the set/clear, these call
> uart_update_mctrl which uses a cached value of modem control lines in
> port->mctrl. This cached value is not updated by the imx.c functions
> imx_port_rts_active and imx_port_rts_inactive, which is a bit nasty.
Right. I'm sure if you dig deeper you will find still more nasty corners
with rs485. :-\
> If the uart is connected to a line driver with active low TX enable, the
> flag port->rs485.flags & SER_RS485_RTS_AFTER_SEND would be set, so the RTS
> line should be high for receive, but the cached port->mctrl would not
> include TIOCM_RTS so uart_update_mctrl will still set it low.
You're right. That is a problem in imx.c though and so orthogonal to
this patch. Having said that I don't don't feel confident with all the
handshaking and interactions with serial_core and the requirements of
POSIX.
> I think for your patch to work in this case, the imx_port_rts_(in)active
> functions would need fixing to update the mctrl cache.
My test setup still doesn't work completely, maybe this is part of the
remaining issue.
Thanks for your thoughts
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
