Re: [PATCH v1 1/1] serial: 8250_pci1xxxx: Drop quirk from 8250_port
From: Andy Shevchenko
Date: Mon Feb 19 2024 - 11:21:17 EST
On Thu, Feb 15, 2024 at 09:26:21AM +0000, Rengarajan.S@xxxxxxxxxxxxx wrote:
> On Wed, 2024-02-14 at 15:50 +0200, Andy Shevchenko wrote:
..
> > + /*
> > + * 8250 core considers prescaller value to be always 16.
> > + * The MCHP ports support downscaled mode and hence the
> > + * functional UART clock can be lower, i.e. 62.5MHz, than
> > + * software expects in order to support higher baud rates.
> > + * Assign here 64MHz to support 4Mbps.
> > + *
> > + * The value itself is not really used anywhere except baud
> > + * rate calculations, so we can mangle it as we wish.
> > + */
> > + port->port.uartclk = 64 * HZ_PER_MHZ;
>
> As per internal MCHP DOS, PCI1XXXX driver uses a simple method of
> converting "legacy 16 bit baud rate generator" to a "32 bit fractional
> baud rate generator" which enables generation of an acceptable baud
> rate from any valuable frequency.
>
> This is applicable only when the baud clock selected is 62.5 MHz, so
> when we configure the baud clock to 64 MHz(as above) will it be
> downscaled to 62.5 MHz, thus supporting the above feature?
I specifically added the above comment. If you look closer, your driver does
not use this value at all, the 8250 port code uses it in several places:
- 8250_rsa case (not applicable to your driver)
- probe_baud() call (applicable iff the kernel command line misses the
baudrate, but even without this patch it's broken for your driver)
- serial8250_update_uartclk() call (not applicable to your driver)
- serial8250_get_baud_rate() call (only to get max and min range;
my change will have an effect on min (max is exactly what your
quirk is doing right no), so 62500000/16/65535 ~= 59.6, while
with my change 64000000/16/65535 ~= 61.0, but standard baudrate
here is 50 and 75, the former isn't supported by the existing
code either
- serial8250_do_get_divisor() call when magic_multiplier supplied
(not applicable to your driver)
- autoconfig_16550a() call (not applicable to your driver)
Hope this clarifies the case.
Of course if you able to test, will be even better.
But wait for v2 where I update what Greg caught.
..
> Acked-by: Rengarajan S <rengarajan.s@xxxxxxxxxxxxx>
Thank you!
--
With Best Regards,
Andy Shevchenko