On Tue, Nov 05, 2013 at 05:38:33PM +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, even in error cases.
>
> Signed-off-by: Colin Leitner <[email protected]>
> ---
> drivers/usb/serial/ftdi_sio.c | 38 +++++++++++++++++++++++++-------------
> 1 file changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index b21d553..033d9b5 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -2115,6 +2115,17 @@ 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. */
Almost right. It should be
/*
* 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)) {
> + termios->c_cflag &= ~CSIZE;
> + if (old_termios)
> + termios->c_cflag |= old_termios->c_cflag & CSIZE;
> + else
> + termios->c_cflag |= CS8;
> + dev_warn(ddev, "requested CSIZE setting not supported\n");
Much better. Perhaps you could move the debug statement first in the
block and put an empty line after as well (or at least insert one
before).
> + }
> +
> cflag = termios->c_cflag;
>
> if (!old_termios)
> @@ -2151,19 +2162,20 @@ 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;
> + case CS8:
> + urb_value |= 8;
> + dev_dbg(ddev, "Setting CS8\n");
> + break;
> + default:
> + /* It would actually be a driver bug if we ended up in here */
> + urb_value |= 8;
> + dev_err(ddev, "CSIZE was set but not CS7-CS8\n");
> + break;
Since we cannot end up here, why not simply merge the CS8 and default
cases and remove the comment and dev_err?
Thanks,
Johan
> }
>
> /* 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
