linux-kernel - Re: [PATCH v1 1/1] serial: 8250_pci1xxxx: Drop quirk from 8250

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9ae91cfc2ca24d23c5f3bc16208e5d59eccba076.camel@microchip.com>
Date: Tue, 20 Feb 2024 04:21:59 +0000
From: <Rengarajan.S@...rochip.com>
To: <andriy.shevchenko@...ux.intel.com>
CC: <linux-serial@...r.kernel.org>, <jirislaby@...nel.org>,
	<gregkh@...uxfoundation.org>, <linux-kernel@...r.kernel.org>,
	<Kumaravel.Thiagarajan@...rochip.com>, <Tharunkumar.Pasumarthi@...rochip.com>
Subject: Re: [PATCH v1 1/1] serial: 8250_pci1xxxx: Drop quirk from 8250_port

On Mon, 2024-02-19 at 18:19 +0200, Andy Shevchenko wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Thu, Feb 15, 2024 at 09:26:21AM +0000,
> Rengarajan.S@...rochip.com 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.

Thanks for the clarification Andy. Will start with the testing after v2
patch.

> 
> ...
> 
> > Acked-by: Rengarajan S <rengarajan.s@...rochip.com>
> 
> Thank you!
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
>