| Message ID | 20180518132550.21749-2-johan@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 18, 2018 at 03:25:47PM +0200, Johan Hovold wrote: > Since forever this driver has had IXON and IXOFF mixed up, and has used > the latter rather than the former to enable hardware-assisted software > flow control on output. > > Signed-off-by: Johan Hovold <johan@kernel.org> > --- > drivers/usb/serial/ftdi_sio.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c > index 7ea221d42dba..62c99871863c 100644 > --- a/drivers/usb/serial/ftdi_sio.c > +++ b/drivers/usb/serial/ftdi_sio.c > @@ -2342,12 +2342,8 @@ static void ftdi_set_termios(struct tty_struct *tty, > } else { > /* > * Xon/Xoff code > - * > - * Check the IXOFF status in the iflag component of the > - * termios structure. If IXOFF is not set, the pre-xon/xoff > - * code is executed. > */ > - if (iflag & IXOFF) { > + if (iflag & IXON) { > dev_dbg(ddev, "%s request to enable xonxoff iflag=%04x\n", > __func__, iflag); > /* Try to enable the XON/XOFF on the ftdi_sio > @@ -2372,7 +2368,7 @@ static void ftdi_set_termios(struct tty_struct *tty, > } > } else { > /* else clause to only run if cflag ! CRTSCTS and iflag > - * ! XOFF. CHECKME Assuming XON/XOFF handled by tty > + * ! XON. CHECKME Assuming XON/XOFF handled by tty > * stack - not by device */ > dev_dbg(ddev, "%s Turning off hardware flow control\n", __func__); > if (usb_control_msg(dev, Ugh, that's a huge sign that no one actually uses IXON/IXOFF anymore :( Anyway, thanks for fixing this up, should also probably be backported to the stable kernels. Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 18, 2018 at 03:53:54PM +0200, Greg Kroah-Hartman wrote: > On Fri, May 18, 2018 at 03:25:47PM +0200, Johan Hovold wrote: > > Since forever this driver has had IXON and IXOFF mixed up, and has used > > the latter rather than the former to enable hardware-assisted software > > flow control on output. > Ugh, that's a huge sign that no one actually uses IXON/IXOFF anymore :( Could be that some applications set both IXON and IXOFF "just to be safe"; that appears to be the case with minicom at least. > Anyway, thanks for fixing this up, should also probably be backported to > the stable kernels. I considered that, but since its a change in behaviour I figured it was best that anyone affected hits this with a major (minor?) kernel version update (rather than a stable update). But I'll add a stable tag if you think it's warranted? > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Thanks for the review. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 18, 2018 at 04:33:50PM +0200, Johan Hovold wrote: > On Fri, May 18, 2018 at 03:53:54PM +0200, Greg Kroah-Hartman wrote: > > On Fri, May 18, 2018 at 03:25:47PM +0200, Johan Hovold wrote: > > > Since forever this driver has had IXON and IXOFF mixed up, and has used > > > the latter rather than the former to enable hardware-assisted software > > > flow control on output. > > > Ugh, that's a huge sign that no one actually uses IXON/IXOFF anymore :( > > Could be that some applications set both IXON and IXOFF "just to be > safe"; that appears to be the case with minicom at least. > > > Anyway, thanks for fixing this up, should also probably be backported to > > the stable kernels. > > I considered that, but since its a change in behaviour I figured it was > best that anyone affected hits this with a major (minor?) kernel version > update (rather than a stable update). But I'll add a stable tag if you > think it's warranted? Fair enough. If anyone notices we can easily backport it later on if needed. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c index 7ea221d42dba..62c99871863c 100644 --- a/drivers/usb/serial/ftdi_sio.c +++ b/drivers/usb/serial/ftdi_sio.c @@ -2342,12 +2342,8 @@ static void ftdi_set_termios(struct tty_struct *tty, } else { /* * Xon/Xoff code - * - * Check the IXOFF status in the iflag component of the - * termios structure. If IXOFF is not set, the pre-xon/xoff - * code is executed. */ - if (iflag & IXOFF) { + if (iflag & IXON) { dev_dbg(ddev, "%s request to enable xonxoff iflag=%04x\n", __func__, iflag); /* Try to enable the XON/XOFF on the ftdi_sio @@ -2372,7 +2368,7 @@ static void ftdi_set_termios(struct tty_struct *tty, } } else { /* else clause to only run if cflag ! CRTSCTS and iflag - * ! XOFF. CHECKME Assuming XON/XOFF handled by tty + * ! XON. CHECKME Assuming XON/XOFF handled by tty * stack - not by device */ dev_dbg(ddev, "%s Turning off hardware flow control\n", __func__); if (usb_control_msg(dev,
Since forever this driver has had IXON and IXOFF mixed up, and has used the latter rather than the former to enable hardware-assisted software flow control on output. Signed-off-by: Johan Hovold <johan@kernel.org> --- drivers/usb/serial/ftdi_sio.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)