On Tue, Nov 05, 2013 at 06:02:34PM +0100, Colin Leitner wrote:
> FTDI UARTs support only 7 or 8 data bits. Until now the ftdi_sio driver would
> only report this limitation for CS6 to dmesg and fail to reflect this fact to
> tcgetattr.
> 
> This patch reverts the unsupported CSIZE setting and reports the fact with 
> less
> severance to dmesg for both CS5 and CS6.
> 
> To test the patch it's sufficient to call
> 
>     stty -F /dev/ttyUSB0 cs5
> 
> which will succeed without the patch and report an error with the patch
> applied.
> 
> As an additional fix this patch ensures that the control request will always
> include a data bit size.

Great, all looks good now. Thanks for fixing this. Did you check if
there are even more (USB) serial drivers with similar problems (i.e.
testing C_CSIZE(tty) in set_termios)?

Greg, this could go into v3.13-rc2.
 
> Signed-off-by: Colin Leitner <colin.leit...@gmail.com>

Signed-off-by: Johan Hovold <jhov...@gmail.com>

> ---
>  drivers/usb/serial/ftdi_sio.c |   37 ++++++++++++++++++++++++-------------
>  1 file changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index b21d553..dccb4db 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -2115,6 +2115,20 @@ static void ftdi_set_termios(struct tty_struct *tty,
>               termios->c_cflag |= CRTSCTS;
>       }
> 
> +     /*
> +      * All FTDI UART chips are limited to CS7/8. We won't pretend to
> +      * support CS5/6 and revert the CSIZE setting instead.
> +      */
> +     if ((C_CSIZE(tty) != CS8) && (C_CSIZE(tty) != CS7)) {
> +             dev_warn(ddev, "requested CSIZE setting not supported\n");
> +
> +             termios->c_cflag &= ~CSIZE;
> +             if (old_termios)
> +                     termios->c_cflag |= old_termios->c_cflag & CSIZE;
> +             else
> +                     termios->c_cflag |= CS8;
> +     }
> +
>       cflag = termios->c_cflag;
> 
>       if (!old_termios)
> @@ -2151,19 +2165,16 @@ no_skip:
>       } else {
>               urb_value |= FTDI_SIO_SET_DATA_PARITY_NONE;
>       }
> -     if (cflag & CSIZE) {
> -             switch (cflag & CSIZE) {
> -             case CS7:
> -                     urb_value |= 7;
> -                     dev_dbg(ddev, "Setting CS7\n");
> -                     break;
> -             case CS8:
> -                     urb_value |= 8;
> -                     dev_dbg(ddev, "Setting CS8\n");
> -                     break;
> -             default:
> -                     dev_err(ddev, "CSIZE was set but not CS7-CS8\n");
> -             }
> +     switch (cflag & CSIZE) {
> +     case CS7:
> +             urb_value |= 7;
> +             dev_dbg(ddev, "Setting CS7\n");
> +             break;
> +     default:
> +     case CS8:
> +             urb_value |= 8;
> +             dev_dbg(ddev, "Setting CS8\n");
> +             break;
>       }
> 
>       /* This is needed by the break command since it uses the same command
> -- 
> 1.7.10.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to