Received-SPF: pass (google.com: domain of linux-kernel+bounces-73172-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 20 Feb 2024 16:20:57 +0200
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Rengarajan.S@microchip.com
Cc: linux-serial@vger.kernel.org, jirislaby@kernel.org,
	gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	Kumaravel.Thiagarajan@microchip.com,
	Tharunkumar.Pasumarthi@microchip.com
Subject: Re: [PATCH v1 1/1] serial: 8250_pci1xxxx: Drop quirk from 8250_port
Message-ID: <ZdS1Se4bVvuKDd6-@smile.fi.intel.com>
References: <20240214135009.3299940-1-andriy.shevchenko@linux.intel.com>
 <063a1804732c619bc4a5c801c9881fedd92ad745.camel@microchip.com>
 <ZdN_npTcCfz0cI_g@smile.fi.intel.com>
 <9ae91cfc2ca24d23c5f3bc16208e5d59eccba076.camel@microchip.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <9ae91cfc2ca24d23c5f3bc16208e5d59eccba076.camel@microchip.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

On Tue, Feb 20, 2024 at 04:21:59AM +0000, Rengarajan.S@microchip.com wrote:
> On Mon, 2024-02-19 at 18:19 +0200, Andy Shevchenko wrote:
> > On Thu, Feb 15, 2024 at 09:26:21AM +0000,
> > Rengarajan.S@microchip.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.

v2 is here:

https://lore.kernel.org/r/20240219162917.2159736-1-andriy.shevchenko@linux.intel.com

-- 
With Best Regards,
Andy Shevchenko