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:
>
> 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.
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.
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);
--
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
