On Fri, Jun 26, 2020 at 09:42:06AM +0200, Johan Hovold wrote:
> On Fri, Jun 26, 2020 at 04:24:20AM +0000, Phu Luu wrote:
> > From: Brant Merryman <brant.merryman@xxxxxxxxxx>
> >
> > CP210x hardware disables auto-RTS but leaves auto-CTS when
> > in hardware flow control mode and UART on cp210x hardware
> > is disabled. This allows data to flow out, but new data
> > will not come into the port. When re-opening the port, if
> > auto-CTS is enabled on the cp210x, then auto-RTS must be
> > re-enabled in the driver.
> >
> > Signed-off-by: Phu Luu <phu.luu@xxxxxxxxxx>
> > Signed-off-by: Brant Merryman <brant.merryman@xxxxxxxxxx>
>
> Please revisit these tags as well.
>
> > ---
> > 06/09/2020: Patch v3 2/2 Modified based on feedback from Johan Hovold <johan@xxxxxxxxxx>
> > 12/18/2019: Patch v2 Broken into two patches and modified based on feedback from Johan Hovold <johan@xxxxxxxxxx>
> > 12/09/2019: Initial submission of patch "Proper RTS control when buffers fill"
> >
> > drivers/usb/serial/cp210x.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> > index bcceb4ad8be0..091441b1c5b9 100644
> > --- a/drivers/usb/serial/cp210x.c
> > +++ b/drivers/usb/serial/cp210x.c
> > @@ -917,6 +917,7 @@ static void cp210x_get_termios_port(struct usb_serial_port *port,
> > u32 baud;
> > u16 bits;
> > u32 ctl_hs;
> > + u32 flow_repl;
> >
> > cp210x_read_u32_reg(port, CP210X_GET_BAUDRATE, &baud);
> >
> > @@ -1017,6 +1018,23 @@ static void cp210x_get_termios_port(struct usb_serial_port *port,
> > ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake);
> > if (ctl_hs & CP210X_SERIAL_CTS_HANDSHAKE) {
> > dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__);
> > + /*
> > + * When the port is closed, the CP210x hardware disables
> > + * auto-RTS and RTS is deasserted but it leaves auto-CTS when
> > + * in hardware flow control mode. This prevents new data from
> > + * being received by the port. When re-opening the port, if
> > + * auto-CTS is enabled on the cp210x, then auto-RTS must be
> > + * re-enabled in the driver.
> > + */
>
> I already asked this of Brant, but could you please be more specific
> about which state the RTS-line end up in when disabling the UART (e.g.
> 0x00: statically inactive)?
>
> The reason I ask is that after open() returns, the tty layer would raise
> RTS, which should again allow data to flow in in contrast to what the
> comment and changelog text claims.
I had another reason to look at this driver so I could determine that
the patch description and comment were indeed incorrect; the tty layer
asserts RTS when reopening the port just fine and data flows in.
> We still want to enable auto-RTS of course so the patch is otherwise
> correct.
I've fixed up the commit message and comment, reordered the SOB tags
and added your Co-developed-by tag before applying both of these now:
https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/log/?h=usb-next
Johan
