On Mon, Feb 04, 2019 at 04:57:00PM +0100, Johan Hovold wrote:
> On Mon, Feb 04, 2019 at 12:21:30PM +0100, Johanna Abrahamsson wrote:
> > From: Johanna Abrahamsson <johanna.abrahamsson@xxxxxxxxxxxxx>
> >
> > This patch adds a minimum baud rate of 2400 for CP2105 SCI
> >
> > 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.
>
> Good to hear to you found the root cause of that failed baudrate
> request.
>
> > Since this is, as far as I can tell, the only cp210x chip with a minimum
> > baud rate higher than 300, I've added a special case to
> > cp210x_change_speed rather than adding a min_speed field in
> > cp210x_serial_private.
>
> But please do add a min_speed field instead of special casing which
> tends to make the code harder to read.
>
> > Signed-off-by: Johanna Abrahamsson <johanna.abrahamsson@xxxxxxxxxxxxx>
> > ---
> > drivers/usb/serial/cp210x.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> > index 336a3c0f9f2c..181abf7bb8c0 100644
> > --- a/drivers/usb/serial/cp210x.c
> > +++ b/drivers/usb/serial/cp210x.c
> > @@ -1111,9 +1111,12 @@ static void cp210x_change_speed(struct tty_struct *tty,
> > */
> > if (priv->use_actual_rate)
> > baud = cp210x_get_actual_rate(serial, baud);
> > - else if (baud < 1000000)
> > + else if (baud < 1000000) {
> > + if (priv->partnum == CP210X_PARTNUM_CP2105 &&
> > + cp210x_interface_num(serial) == 1)
> > + baud = clamp(baud, 2400u, priv->max_speed);
> > baud = cp210x_get_an205_rate(baud);
>
> We already have a clamp in place in cp210x_get_actual_rate() so just add
> a similar construct to cp210x_get_actual_rate() and amend
> cp210x_init_max_speed() as needed.
Or better still, move the clamp from cp210x_get_actual_rate() to above
these conditionals and make sure to honour min_speed. Then you can drop
the final else-if here too.
> > - else if (baud > priv->max_speed)
> > + } else if (baud > priv->max_speed)
> > baud = priv->max_speed;
> >
> > dev_dbg(&port->dev, "%s - setting baud rate to %u\n", __func__, baud);
Johan
