On Thu, 1 Sep 2022, [email protected] wrote:
> > > + const unsigned int standard_baud_list[] = {50, 75, 110, 134, 150, 300,
> > > + 600, 1200, 1800, 2000, 2400, 3600,
> > > + 4800, 7200, 9600, 19200, 38400, 57600,
> > > + 115200, 125000, 136400, 150000, 166700,
> > > + 187500, 214300, 250000, 300000, 375000,
> > > + 500000, 750000,
> > > + 1000000, 1500000};
> >
> > Why?!
>
> The standard baud rates are handled within serial8250_do_set_termios
> which is invoked from within mchp_pci1xxxx_set_termios in first place.
> Hence if it matches with any of the standard baudrates,
> it can return immediately.
Care to explain why the baudrates in your table don't match those in
tty_baudrate.c? ...It makes no sense to me that you call these "standard
baud rates".
--
i.
> -----Original Message-----
> From: Ilpo J?rvinen <[email protected]>
> Sent: Thursday, September 1, 2022 7:12 PM
> To: Kumaravel Thiagarajan - I21417 <[email protected]>
> Cc: [email protected]; Greg Kroah-Hartman
> <[email protected]>; Jiri Slaby <[email protected]>; u.kleine-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Lukas Wunner <[email protected]>; LKML <[email protected]>;
> linux-serial <[email protected]>; UNGLinuxDriver
> <[email protected]>
> Subject: RE: [PATCH v1 tty-next 1/2] 8250: microchip: pci1xxxx: Add driver for
> the quad-uart function in the multi-function endpoint of pci1xxxx device.
>
>
> On Thu, 1 Sep 2022, [email protected] wrote:
>
> > > > + const unsigned int standard_baud_list[] = {50, 75, 110, 134, 150,
> 300,
> > > > + 600, 1200, 1800, 2000, 2400, 3600,
> > > > + 4800, 7200, 9600, 19200, 38400, 57600,
> > > > + 115200, 125000, 136400, 150000, 166700,
> > > > + 187500, 214300, 250000, 300000, 375000,
> > > > + 500000, 750000,
> > > > + 1000000, 1500000};
> > >
> > > Why?!
> >
> > The standard baud rates are handled within serial8250_do_set_termios
> > which is invoked from within mchp_pci1xxxx_set_termios in first place.
> > Hence if it matches with any of the standard baudrates, it can return
> > immediately.
>
> Care to explain why the baudrates in your table don't match those in
> tty_baudrate.c? ...It makes no sense to me that you call these "standard
> baud rates".
The baudrates in my table are from our legacy UART IP and these baudrates can be
generated by the hardware by updating UART_DLL & UART_DLM alone as done by the
serial8250_do_set_termios.
I noticed that some of the baud rates in tty_baudrate.c arenot listed in this table
but will still be handled by the mchp_pci1xxxx_set_termios.
I can rename standard_baud_list to simply baud_list. Please let me know.
Thank You.
Regards,
Kumaravel
On Fri, Sep 2, 2022 at 2:57 PM <[email protected]> wrote:
> > -----Original Message-----
> > From: Ilpo Järvinen <[email protected]>
> > Sent: Thursday, September 1, 2022 7:12 PM
> > On Thu, 1 Sep 2022, [email protected] wrote:
...
> > > > > + const unsigned int standard_baud_list[] = {50, 75, 110, 134, 150,
> > 300,
> > > > > + 600, 1200, 1800, 2000, 2400, 3600,
> > > > > + 4800, 7200, 9600, 19200, 38400, 57600,
> > > > > + 115200, 125000, 136400, 150000, 166700,
> > > > > + 187500, 214300, 250000, 300000, 375000,
> > > > > + 500000, 750000,
> > > > > + 1000000, 1500000};
> > > >
> > > > Why?!
> > >
> > > The standard baud rates are handled within serial8250_do_set_termios
> > > which is invoked from within mchp_pci1xxxx_set_termios in first place.
> > > Hence if it matches with any of the standard baudrates, it can return
> > > immediately.
> >
> > Care to explain why the baudrates in your table don't match those in
> > tty_baudrate.c? ...It makes no sense to me that you call these "standard
> > baud rates".
> The baudrates in my table are from our legacy UART IP and these baudrates can be
> generated by the hardware by updating UART_DLL & UART_DLM alone as done by the
> serial8250_do_set_termios.
> I noticed that some of the baud rates in tty_baudrate.c arenot listed in this table
> but will still be handled by the mchp_pci1xxxx_set_termios.
> I can rename standard_baud_list to simply baud_list. Please let me know.
No, the point is avoid repeating what standard APIs already do. Just
make sure you call it properly and provide _get/_set_divisor()
callbacks. Note, your driver can cope with BOTHER and there all
non-standard baud rates go.
--
With Best Regards,
Andy Shevchenko
> -----Original Message-----
> From: Andy Shevchenko <[email protected]>
> Sent: Friday, September 2, 2022 8:33 PM
> To: Kumaravel Thiagarajan - I21417 <[email protected]>
> Cc: Ilpo Järvinen <[email protected]>; Greg Kroah-Hartman
> <[email protected]>; Jiri Slaby <[email protected]>; Uwe
> Kleine-König <[email protected]>; Johan Hovold
> <[email protected]>; Wander Lairson Costa <[email protected]>; Eric
> Tremblay <[email protected]>; Maciej W. Rozycki
> <[email protected]>; Geert Uytterhoeven <[email protected]>;
> Jeremy Kerr <[email protected]>; Phil Edworthy <[email protected]>;
> Lukas Wunner <[email protected]>; Linux Kernel Mailing List <linux-
> [email protected]>; open list:SERIAL DRIVERS <linux-
> [email protected]>; UNGLinuxDriver
> <[email protected]>
> Subject: Re: [PATCH v1 tty-next 1/2] 8250: microchip: pci1xxxx: Add driver for
> the quad-uart function in the multi-function endpoint of pci1xxxx device.
>
> On Fri, Sep 2, 2022 at 2:57 PM <[email protected]>
> wrote:
> > > -----Original Message-----
> > > From: Ilpo Järvinen <[email protected]>
> > > Sent: Thursday, September 1, 2022 7:12 PM On Thu, 1 Sep 2022,
> > > [email protected] wrote:
>
> ...
>
> > > > > > + const unsigned int standard_baud_list[] = {50, 75,
> > > > > > + 110, 134, 150,
> > > 300,
> > > > > > + 600, 1200, 1800, 2000, 2400, 3600,
> > > > > > + 4800, 7200, 9600, 19200, 38400, 57600,
> > > > > > + 115200, 125000, 136400, 150000, 166700,
> > > > > > + 187500, 214300, 250000, 300000, 375000,
> > > > > > + 500000,
> > > > > > + 750000, 1000000, 1500000};
> > > > >
> > > > > Why?!
> > > >
> > > > The standard baud rates are handled within
> > > > serial8250_do_set_termios which is invoked from within
> mchp_pci1xxxx_set_termios in first place.
> > > > Hence if it matches with any of the standard baudrates, it can
> > > > return immediately.
> > >
> > > Care to explain why the baudrates in your table don't match those in
> > > tty_baudrate.c? ...It makes no sense to me that you call these
> > > "standard baud rates".
> > The baudrates in my table are from our legacy UART IP and these
> > baudrates can be generated by the hardware by updating UART_DLL &
> > UART_DLM alone as done by the serial8250_do_set_termios.
> > I noticed that some of the baud rates in tty_baudrate.c arenot listed
> > in this table but will still be handled by the mchp_pci1xxxx_set_termios.
> > I can rename standard_baud_list to simply baud_list. Please let me know.
>
> No, the point is avoid repeating what standard APIs already do. Just make
> sure you call it properly and provide _get/_set_divisor() callbacks. Note, your
> driver can cope with BOTHER and there all non-standard baud rates go.
I will review my driver again and get back to you if required.
Thank You.
Regards,
Kumaravel