On Wed, Feb 06, 2019 at 09:56:15AM +0100, Johanna Abrahamsson wrote:
> From: Johanna Abrahamsson <johanna.abrahams...@afconsult.com>
> 
> This patch adds minimum baud rate to the cp210x driver.
> 
> According to the datasheet for CP2105, the SCI supports 2400 as the
> lowest baud rate. As this is not heeded in the current code, an error
> message 'failed set req 0x1e size 4 status: -32' when trying to set a
> lower baud rate such as 300.
> The other cp210x models to date supports a minimum baud rate of 300.
> 
> Signed-off-by: Johanna Abrahamsson <johanna.abrahams...@afconsult.com>
> ---

Thanks for the update.

Now applied with a few minor tweaks:

>  drivers/usb/serial/cp210x.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index 260c875dd263..ceff735e961c 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -245,6 +245,7 @@ struct cp210x_serial_private {
>       u8                      gpio_input;
>  #endif
>       u8                      partnum;
> +     speed_t                 min_speed;
>       speed_t                 max_speed;
>       bool                    use_actual_rate;
>  };
> @@ -1078,8 +1079,6 @@ static speed_t cp210x_get_actual_rate(struct usb_serial 
> *serial, speed_t baud)
>       unsigned int prescale = 1;
>       unsigned int div;
>  
> -     baud = clamp(baud, 300u, priv->max_speed);

priv is now unused in this function and thus triggered a compilation
warning. I dropped priv along with the serial pointer.

> -
>       if (baud <= 365)
>               prescale = 4;
>  
> @@ -1122,7 +1121,7 @@ static void cp210x_change_speed(struct tty_struct *tty,
>       struct cp210x_serial_private *priv = usb_get_serial_data(serial);
>       u32 baud;
>  
> -     baud = tty->termios.c_ospeed;
> +     baud = clamp(tty->termios.c_ospeed, priv->min_speed, priv->max_speed);

I moved this clamp after the comment since this construct now makes sure
we honour max_speed.

>       /*
>        * This maps the requested rate to the actual rate, a valid rate on
> @@ -1134,8 +1133,6 @@ static void cp210x_change_speed(struct tty_struct *tty,
>               baud = cp210x_get_actual_rate(serial, baud);
>       else if (baud < 1000000)
>               baud = cp210x_get_an205_rate(baud);
> -     else if (baud > priv->max_speed)
> -             baud = priv->max_speed;
>  
>       dev_dbg(&port->dev, "%s - setting baud rate to %u\n", __func__, baud);
>       if (cp210x_write_u32_reg(port, CP210X_SET_BAUDRATE, baud)) {
> @@ -1797,28 +1794,35 @@ static void cp210x_init_max_speed(struct usb_serial 
> *serial)
>  {
>       struct cp210x_serial_private *priv = usb_get_serial_data(serial);
>       bool use_actual_rate = false;
> +     speed_t min;

And I initialised min to 300 here which allows most initialisations
below to be removed.

>       speed_t max;
>  
>       switch (priv->partnum) {
>       case CP210X_PARTNUM_CP2101:
> +             min = 300;
>               max = 921600;
>               break;
>       case CP210X_PARTNUM_CP2102:
>       case CP210X_PARTNUM_CP2103:
> +             min = 300;
>               max = 1000000;
>               break;
>       case CP210X_PARTNUM_CP2104:
>               use_actual_rate = true;
> +             min = 300;
>               max = 2000000;
>               break;
>       case CP210X_PARTNUM_CP2108:
> +             min = 300;
>               max = 2000000;
>               break;
>       case CP210X_PARTNUM_CP2105:
>               if (cp210x_interface_num(serial) == 0) {
>                       use_actual_rate = true;
> +                     min = 300;
>                       max = 2000000;  /* ECI */
>               } else {
> +                     min = 2400;
>                       max = 921600;   /* SCI */
>               }
>               break;
> @@ -1826,13 +1830,16 @@ static void cp210x_init_max_speed(struct usb_serial 
> *serial)
>       case CP210X_PARTNUM_CP2102N_QFN24:
>       case CP210X_PARTNUM_CP2102N_QFN20:
>               use_actual_rate = true;
> +             min = 300;
>               max = 3000000;
>               break;
>       default:
> +             min = 300;
>               max = 2000000;
>               break;
>       }
>  
> +     priv->min_speed = min;
>       priv->max_speed = max;
>       priv->use_actual_rate = use_actual_rate;
>  }

Thanks,
Johan

Reply via email to