> -----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? 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. > 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. > > > > + } > > Side note for those interested in this driver: > > > if (termios->c_iflag & IXON) > > flow |= SC16IS7XX_EFR_SWFLOW3_BIT; > > if (termios->c_iflag & IXOFF) > > flow |= SC16IS7XX_EFR_SWFLOW1_BIT; > > Looks like IXON and IXOFF auto soft flow control modes are backwards. > > IXON should enable comparison of rx with START/STOP chars, and IXOFF > should enable flow control based on rx fifo levels. > > I would seriously consider not enabling soft flow control for > IXOFF: only a remote with h/w-based soft flow control will be able to stop tx > before overflowing this uart's rx fifo. > I haven't looked at XON/XOFF flow control yet. Still I see no problem in fixing HW flow-control in a patch of its own. Several mail-threads about this driver are now active. Performance really s.... on the imx.6 we are currently using it at. I guess we'll see more patches in the future > > Lastly, according to the datasheet, the soft flow control should not be > enabled simultaneously with autoRTS/autoCTS, but I don't see any logic here > that prevents that. > > Regards, > Peter Hurley > > > > > > + /* Always set enable enhanced */ > > + flow |= SC16IS7XX_EFR_ENABLE_BIT; > > + > > sc16is7xx_port_write(port, SC16IS7XX_EFR_REG, flow); > > regcache_cache_bypass(s->regmap, false); > ��.n��������+%������w��{.n�����{��ǫ����{ay� ʇڙ���f���h������_�(�階�ݢj"��������G����?���&��
