On Fri, Sep 15, 2017 at 04:44:56PM +0200, Uwe Kleine-König wrote:
> On Fri, Sep 15, 2017 at 02:56:35PM +0200, Clemens Gruber wrote:
> > On Thu, Sep 14, 2017 at 01:11:33PM +0100, Ian Jamison wrote:
> > > UART core function uart_update_mctrl relies on a cached value of
> > > modem control lines. This was used but not updated by local RTS
> > > control functions within imx.c. These are used for RS485 line
> > > driver enable signalling. Having an out-of-date value in the cached
> > > mctrl can result in the transmitter being enabled when it shouldn't
> > > be.
> > >
> > > Fix this by updating the mctrl value before applying it.
> > >
> > > Cc: Uwe-Kleine König <u.kleine-koenig@xxxxxxxxxxxxxx>
> > > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > Cc: Clemens Gruber <clemens.gruber@xxxxxxxxxxxx>
> > > Cc: Fabio Estevam <festevam@xxxxxxxxx>
> > > Cc: Fabio Estevam <fabio.estevam@xxxxxxx>
> > > Signed-off-by: Ian Jamison <ian.dev@xxxxxxxxxx>
> > > ---
> > > Compile tested only, since my devboard uses a monostable off the TXD
> > > line to enable the transmitter.
> > >
> > > Resending to CC Fabio and Clemens.
> > >
> > > drivers/tty/serial/imx.c | 6 ++++--
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > > index dfeff3951f93..3657d745e90f 100644
> > > --- a/drivers/tty/serial/imx.c
> > > +++ b/drivers/tty/serial/imx.c
> > > @@ -334,7 +334,8 @@ static void imx_port_rts_active(struct imx_port *sport, unsigned long *ucr2)
> > > {
> > > *ucr2 &= ~(UCR2_CTSC | UCR2_CTS);
> > >
> > > - mctrl_gpio_set(sport->gpios, sport->port.mctrl | TIOCM_RTS);
> > > + sport->port.mctrl |= TIOCM_RTS;
> > > + mctrl_gpio_set(sport->gpios, sport->port.mctrl);
> > > }
> > >
> > > static void imx_port_rts_inactive(struct imx_port *sport, unsigned long *ucr2)
> > > @@ -342,7 +343,8 @@ static void imx_port_rts_inactive(struct imx_port *sport, unsigned long *ucr2)
> > > *ucr2 &= ~UCR2_CTSC;
> > > *ucr2 |= UCR2_CTS;
> > >
> > > - mctrl_gpio_set(sport->gpios, sport->port.mctrl & ~TIOCM_RTS);
> > > + sport->port.mctrl &= ~TIOCM_RTS;
> > > + mctrl_gpio_set(sport->gpios, sport->port.mctrl);
> > > }
> > >
> > > static void imx_port_rts_auto(struct imx_port *sport, unsigned long *ucr2)
> > > --
> > > 2.14.1
> > >
> >
> > Looks good to me. I tested RS-485 communications with this patch applied
> > between an i.MX6Q board and my host PC (with a RS-485-to-232 converter).
> >
> > Tested-by: Clemens Gruber <clemens.gruber@xxxxxxxxxxxx>
>
> Did it actually repair someting? Do you use it in combination with my
> patch that masked RTS from uart_dtr_rts? Do you use a gpio for RTS or
> the native function?
Normally, I use the native CTS_B pin, but to test this patch, I remuxed
the pin to GPIO6_IO3 and added rts-gpios to the uart in the DT.
Then I did a few tests without the patch but I did not experience any
problems. The transmitter was only active when it should be.
Applying the patch did not repair anything for me, but it also did not
do any harm. Everything still works. Ian wrote "Compile tested only", so
I made sure it did not break anything for me.
No, I don't think I have your patch in my inbox. You can send it to me
if you want me to test it.
> I have this change in my wip state, too, and it improves the situation,
> but it's not working compeletely yet.
Any idea how to reproduce it? Can you be more specific about the
situation with and without this patch?
Cheers
Clemens
--
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
