On Thu, Aug 18, 2022 at 04:11:40PM +0200, Pali Rohár wrote:
> On Sunday 24 July 2022 14:06:38 Johan Hovold wrote:
> > On Tue, Jul 12, 2022 at 01:53:02PM +0200, Marek Behún wrote:
> > > From: Pali Rohár <pali@xxxxxxxxxx>
> > >
> > > In preparation for following changes, extract divisor code for SIO chip
> > > into new function ftdi_sio_baud_to_divisor(), as is done for other
> > > chips.
> > >
> > > No functional change.
> > >
> > > Signed-off-by: Pali Rohár <pali@xxxxxxxxxx>
> > > Tested-by: Marek Behún <kabel@xxxxxxxxxx>
> > > Signed-off-by: Marek Behún <kabel@xxxxxxxxxx>
> > > ---
> > > drivers/usb/serial/ftdi_sio.c | 36 ++++++++++++++++++++---------------
> > > 1 file changed, 21 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> > > index 717b97f4e094..5b109714612f 100644
> > > --- a/drivers/usb/serial/ftdi_sio.c
> > > +++ b/drivers/usb/serial/ftdi_sio.c
> > > @@ -1150,6 +1150,23 @@ static struct usb_serial_driver * const serial_drivers[] = {
> > > * ***************************************************************************
> > > */
> > >
> > > +static int ftdi_sio_baud_to_divisor(int baud)
> > > +{
> > > + switch (baud) {
> > > + case 300: return ftdi_sio_b300;
> > > + case 600: return ftdi_sio_b600;
> > > + case 1200: return ftdi_sio_b1200;
> > > + case 2400: return ftdi_sio_b2400;
> > > + case 4800: return ftdi_sio_b4800;
> > > + case 9600: return ftdi_sio_b9600;
> > > + case 19200: return ftdi_sio_b19200;
> > > + case 38400: return ftdi_sio_b38400;
> > > + case 57600: return ftdi_sio_b57600;
> > > + case 115200: return ftdi_sio_b115200;
> > > + default: return -1;
> >
> > Please add a line break after the colons.
> >
> > Adding another enum for the invalid value might be preferable.
>
> Enum describe HW values, so it cannot be added to hw list.
Of course you can, but perhaps you'll figure out some other way to do
this.
> > > + }
> > > +}
> > > +
> > > static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base)
> > > {
> > > unsigned short int divisor;
> > > @@ -1286,7 +1303,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,
> > > {
> > > struct ftdi_private *priv = usb_get_serial_port_data(port);
> > > struct device *dev = &port->dev;
> > > - u32 div_value = 0;
> > > + int div_value = 0;
> >
> > And don't change the type here.
>
> This type change was explicitly asked during v1 review. v1 had u32.
Ok, but don't. This variable is used in other parts of this function.
> > > int div_okay = 1;
> > > int baud;
> > >
> > > @@ -1309,23 +1326,12 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,
> > > baud = 9600;
> > > switch (priv->chip_type) {
> > > case SIO: /* SIO chip */
> > > - switch (baud) {
> > > - case 300: div_value = ftdi_sio_b300; break;
> > > - case 600: div_value = ftdi_sio_b600; break;
> > > - case 1200: div_value = ftdi_sio_b1200; break;
> > > - case 2400: div_value = ftdi_sio_b2400; break;
> > > - case 4800: div_value = ftdi_sio_b4800; break;
> > > - case 9600: div_value = ftdi_sio_b9600; break;
> > > - case 19200: div_value = ftdi_sio_b19200; break;
> > > - case 38400: div_value = ftdi_sio_b38400; break;
> > > - case 57600: div_value = ftdi_sio_b57600; break;
> > > - case 115200: div_value = ftdi_sio_b115200; break;
> > > - } /* baud */
> > > - if (div_value == 0) {
Looks like there's another bug here in the current driver as b300 is
defined as 0.
I'll fix that up separately first.
> > > + div_value = ftdi_sio_baud_to_divisor(baud);
> > > + if (div_value == -1) {
> > > dev_dbg(dev, "%s - Baudrate (%d) requested is not supported\n",
> > > __func__, baud);
> > > - div_value = ftdi_sio_b9600;
> > > baud = 9600;
> > > + div_value = ftdi_sio_baud_to_divisor(baud);
> > > div_okay = 0;
> > > }
> > > break;
Johan
