Hello Peter, ----- Original Message ----- From: Peter Hurley [mailto:peter@xxxxxxxxxxxxxxxxxx] To: Maarten Brock [mailto:m.brock@xxxxxxxxxxxxx], Kim Bøndergaard Poulsen [mailto:Kim.BondergaardPoulsen@xxxxxxxxx], linux-serial@xxxxxxxxxxxxxxx, joshc@xxxxxx, kubakici@xxxxx Cc: gregkh@xxxxxxxxxxxxxxxxxxx Sent: Mon, 14 Mar 2016 18:12:23 +0100 Subject: Re: [PATCH] HW Flocontrol fix for SC16IS7XX UART driver > Hi Maarten, > > On 03/14/2016 09:43 AM, Maarten Brock wrote: > > ----- Original Message ----- > > From: Peter Hurley [mailto:peter@xxxxxxxxxxxxxxxxxx] > > To: Kim Bøndergaard Poulsen [mailto:Kim.BondergaardPoulsen@xxxxxxxxx], > linux-serial@xxxxxxxxxxxxxxx [mailto:linux-serial@xxxxxxxxxxxxxxx], > joshc@xxxxxx [mailto:joshc@xxxxxx], kubakici@xxxxx [mailto:kubakici@xxxxx] > > Cc: gregkh@xxxxxxxxxxxxxxxxxxx [mailto:gregkh@xxxxxxxxxxxxxxxxxxx] > > Sent: Mon, 14 Mar 2016 16:25:37 +0100 > > Subject: Re: [PATCH] HW Flocontrol fix for SC16IS7XX UART driver > > Please fix your mail client or use a different one. > None of your replies are threaded. > It is already cumbersome enough to use Kerio Mini to please this mailing list not to send HTTP replies. I cannot force my company to use a different email client. > > >> On 03/14/2016 02:07 AM, Kim Bøndergaard Poulsen wrote: > >>> > >>> > >>>> -----Original Message----- > >>>> From: Peter Hurley [mailto:peter@xxxxxxxxxxxxxxxxxx] > >>>> Sent: 11. marts 2016 02:44 > >>>> To: Kim Bøndergaard Poulsen; linux-serial@xxxxxxxxxxxxxxx; > joshc@xxxxxx; > >>>> kubakici@xxxxx > >>>> Cc: gregkh@xxxxxxxxxxxxxxxxxxx > >>>> Subject: Re: [PATCH] HW Flocontrol fix for SC16IS7XX UART driver > >>>> > >>>> Hi Kim, > >>>> > >>>> On 03/08/2016 04:30 AM, Kim Bøndergaard Poulsen wrote: > >>>>> Following patch fixes HW flowcontrol issues with UARTs based in > >>>>> SC16IS7XX > >>>>> > >>>>> I've only tested the patch with an SC16IS750. > >>>>> > >>>>> commit 9390e7e2952cac5e0119a656a0d8a4e8efe33d07 > >>>>> Author: Kim Bøndergaard <kibo@xxxxxxxxx> > >>>>> Date: Tue Mar 8 12:31:21 2016 +0100 > >>>>> > >>>>> sc16is7xx: Fix HW flow control > >>>>> > >>>>> Following problems fixed: > >>>>> > >>>>> In sc16is7xx_set_baud() what ever was already set in > >>>>> EFR.CTS and EFT.RTS was cleared. > >>>>> > >>>>> In sc16is7xx_set_termios() enhanced functions where disabled > >>>>> > >>>>> In sc16is6xx_set_termios() upper layer is informed that this > driver > >>>>> support automatic HW flowcontrol. Without this knowledge the two > >>>>> layers eventually ended up in different states, causing > >>>>> communication to stop > >>>> > >>>> Missing Signed-off-by: > >>> > >>> I'll add it > >>> > >>>> > >>>>> > >>>>> diff --git a/drivers/tty/serial/sc16is7xx.c > >>>>> b/drivers/tty/serial/sc16is7xx.c index 13f8d5f..5a51024 100644 > >>>>> --- a/drivers/tty/serial/sc16is7xx.c > >>>>> +++ b/drivers/tty/serial/sc16is7xx.c > >>>>> @@ -511,8 +511,9 @@ static int sc16is7xx_set_baud(struct uart_port > >>>>> *port, int baud) > >>>>> > >>>>> /* Enable enhanced features */ > >>>>> regcache_cache_bypass(s->regmap, true); > >>>>> - sc16is7xx_port_write(port, SC16IS7XX_EFR_REG, > >>>>> - SC16IS7XX_EFR_ENABLE_BIT); > >>>>> + sc16is7xx_port_update(port, SC16IS7XX_EFR_REG, > >>>>> + SC16IS7XX_EFR_ENABLE_BIT, > >>>>> + SC16IS7XX_EFR_ENABLE_BIT); > >>>>> regcache_cache_bypass(s->regmap, false); > >>>>> > >>>>> /* Put LCR back to the normal mode */ @@ -901,14 +902,19 @@ > >>>>> static void sc16is7xx_set_termios(struct uart_port *port, > >>>>> regcache_cache_bypass(s->regmap, true); > >>>>> sc16is7xx_port_write(port, SC16IS7XX_XON1_REG, termios- > >>>>> c_cc[VSTART]); > >>>>> sc16is7xx_port_write(port, SC16IS7XX_XOFF1_REG, termios- > >>>>> c_cc[VSTOP]); > >>>>> - if (termios->c_cflag & CRTSCTS) > >>>>> + if (termios->c_cflag & CRTSCTS) { > >>>>> flow |= SC16IS7XX_EFR_AUTOCTS_BIT | > >>>>> SC16IS7XX_EFR_AUTORTS_BIT; > >>>>> + port->status |= UPSTAT_AUTOCTS; > >>>> > >>>> This is correct. > >>>> > >>>> > >>>> However a quick read of the SC16IS740 datasheet suggests that RTS > >>>> handling is broken when autoRTS is enabled. > >>>> > >>> > >>> > >>> Where do you see that? > >> > >> http://www.nxp.com/documents/data_sheet/SC16IS740_750_760.pdf > >> > >> page 27/63, 8.6 Modem Control Register, > >> "If Auto RTS is enabled, the RTS output is controlled by hardware flow > >> control" > >> > >> It's possible I'm misreading that, in that the statement only applies if > >> MCR[1] == 1. > >> > >>> Like written I've only tested at SC16IS750 but the datasheet I've used > >> cover the whole range 740/750/760 > >>> I think I have to get that information to understand your next > statements. > >> Can't really see if it relates to my fix. > >> > >> If MCR[1] == 0 disables RTS even when autoRTS is enabled, then please > ignore > >> my comments regarding TIOCMSET ioctl and set_mctrl() method below. > >> > >> > >> > >>>> AFAICT, RTS state in MCR is _ignored_ when autoRTS is enabled. > >>>> > >>>> This means that if userspace has specifically disabled RTS with > TIOCMSET > >>>> ioctl and CRTSCTS is enabled, input will not be disabled. > >>>> > >>>> To make this work properly requires disabling/restoring autoRTS in the > >>>> set_mctrl() method when TIOCM_RTS is changed. > >>>> > >>>> > >>>>> + } > > > > If the device uses autoRTS and then userspace tries to force RTS one > should > > expect this not to work. Userspace should first disable autoRTS. > > How? By dropping CRTSCTS in termios? > > No, that's exactly what I don't want; ioctl(TIOCMSET) should just work. > From the POV of userspace with CRTSCTS enabled, disabling RTS via > ioctl(TIOCMSET) is halting input and reenabling RTS via ioctl(TIOCMSET) > is resuming input. > > > > Alternatively the driver could automagically disable autoRTS. But then it > > should never reenable autoRTS as that AFAICT would override the forced > > setting again. > > The driver would restore (rather than re-enable) the autoRTS state when RTS > is raised. > > The 8250 driver does this with OMAP, and both OMAP and SC16IS7xx are based > on the 16C750. > > > > I see nothing broken though. > > As long as sc16is7xx_set_mctrl(~TIOCM_RTS) disables RTS regardless of > autoRTS state. > > Regards, > Peter Hurley Can I assume then that this behavior that you describe is documented somewhere as the way it is supposed to work. And that it is not just what you prefer. Maarten -- 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
