| Message ID | 20190204112125.GA1478@fedora-21-dvm (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | USB: serial: cp210x: add minimun baud rate for CP2105 SCI | expand |
On Mon, Feb 04, 2019 at 12:21:30PM +0100, Johanna Abrahamsson wrote: > From: Johanna Abrahamsson <johanna.abrahamsson@afconsult.com> > > 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@afconsult.com> > --- > 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. > - 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
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@afconsult.com> > > > > 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@afconsult.com> > > --- > > 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
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); - 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);