| Message ID | 20220924102718.2984-4-pali@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ftdi_sio driver improvements | expand |
On Sat, Sep 24, 2022 at 12:27:14PM +0200, Pali Rohár wrote:
> In preparation for following changes,
What do you mean by "following changes"? That doesn't work well when
looking in a changelog series. Spell out what you are going to do in a
future change, as to why this is necessary.
thanks,
greg k-h
On Saturday 24 September 2022 12:47:06 Greg Kroah-Hartman wrote: > On Sat, Sep 24, 2022 at 12:27:14PM +0200, Pali Rohár wrote: > > In preparation for following changes, > > What do you mean by "following changes"? That doesn't work well when > looking in a changelog series. Spell out what you are going to do in a > future change, as to why this is necessary. > > thanks, > > greg k-h Ok. Anything else is needed to address?
On Sunday 09 October 2022 14:17:07 Pali Rohár wrote: > On Saturday 24 September 2022 12:47:06 Greg Kroah-Hartman wrote: > > On Sat, Sep 24, 2022 at 12:27:14PM +0200, Pali Rohár wrote: > > > In preparation for following changes, > > > > What do you mean by "following changes"? That doesn't work well when > > looking in a changelog series. Spell out what you are going to do in a > > future change, as to why this is necessary. > > > > thanks, > > > > greg k-h > > Ok. Anything else is needed to address? Ping?
On Tue, Nov 01, 2022 at 11:50:57PM +0100, Pali Rohár wrote: > On Sunday 09 October 2022 14:17:07 Pali Rohár wrote: > > On Saturday 24 September 2022 12:47:06 Greg Kroah-Hartman wrote: > > > On Sat, Sep 24, 2022 at 12:27:14PM +0200, Pali Rohár wrote: > > > > In preparation for following changes, > > > > > > What do you mean by "following changes"? That doesn't work well when > > > looking in a changelog series. Spell out what you are going to do in a > > > future change, as to why this is necessary. > > > > > > thanks, > > > > > > greg k-h > > > > Ok. Anything else is needed to address? > > Ping? We have no idea, sorry, as we have no context here at all and it was thousands of patches ago in our reviews. Please just fix things up based on this review and resubmit, you never need to ask. greg k-h
On Tue, Nov 01, 2022 at 11:50:57PM +0100, Pali Rohár wrote: > On Sunday 09 October 2022 14:17:07 Pali Rohár wrote: > > On Saturday 24 September 2022 12:47:06 Greg Kroah-Hartman wrote: > > > On Sat, Sep 24, 2022 at 12:27:14PM +0200, Pali Rohár wrote: > > > > In preparation for following changes, > > > > > > What do you mean by "following changes"? That doesn't work well when > > > looking in a changelog series. Spell out what you are going to do in a > > > future change, as to why this is necessary. > > > > > > thanks, > > > > > > greg k-h > > > > Ok. Anything else is needed to address? > > Ping? I'll try to look at this again next week. I did notice that your SoB chains are wrong as Marek did not submit these changes and it's not clear whether he's an author at all. Johan
On Wednesday 02 November 2022 02:47:50 Greg Kroah-Hartman wrote: > On Tue, Nov 01, 2022 at 11:50:57PM +0100, Pali Rohár wrote: > > On Sunday 09 October 2022 14:17:07 Pali Rohár wrote: > > > On Saturday 24 September 2022 12:47:06 Greg Kroah-Hartman wrote: > > > > On Sat, Sep 24, 2022 at 12:27:14PM +0200, Pali Rohár wrote: > > > > > In preparation for following changes, > > > > > > > > What do you mean by "following changes"? That doesn't work well when > > > > looking in a changelog series. Spell out what you are going to do in a > > > > future change, as to why this is necessary. > > > > > > > > thanks, > > > > > > > > greg k-h > > > > > > Ok. Anything else is needed to address? > > > > Ping? > > We have no idea, sorry, as we have no context here at all and it was > thousands of patches ago in our reviews. Please just fix things up > based on this review and resubmit, you never need to ask. > > greg k-h It was already done in v3 and it contains only 7 patches, not thousands.
On Sat, Sep 24, 2022 at 12:27:14PM +0200, Pali Rohár wrote: > 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. Please spell out what you mean by "following changes". You're also now using the new helper to set the fallback rate, which should at least be mentioned as it looks like a separate and unnecessary change currently. > No functional change. > > Signed-off-by: Pali Rohár <pali@kernel.org> > Tested-by: Marek Behún <kabel@kernel.org> > Signed-off-by: Marek Behún <kabel@kernel.org> > --- > drivers/usb/serial/ftdi_sio.c | 45 ++++++++++++++++++++++++----------- > 1 file changed, 31 insertions(+), 14 deletions(-) > > diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c > index bfa601fc14fe..fe8a7c5fa0e9 100644 > --- a/drivers/usb/serial/ftdi_sio.c > +++ b/drivers/usb/serial/ftdi_sio.c > @@ -1314,23 +1342,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) { > + div_value = ftdi_sio_baud_to_divisor(baud); > + if (div_value == (u32)-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; This one too needs to be rebased, but you'll notice that. Johan
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c index bfa601fc14fe..fe8a7c5fa0e9 100644 --- a/drivers/usb/serial/ftdi_sio.c +++ b/drivers/usb/serial/ftdi_sio.c @@ -1155,6 +1155,34 @@ static struct usb_serial_driver * const serial_drivers[] = { * *************************************************************************** */ +static u32 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; + } +} + static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base) { unsigned short int divisor; @@ -1314,23 +1342,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) { + div_value = ftdi_sio_baud_to_divisor(baud); + if (div_value == (u32)-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;