Re: [PATCH v2 0/2] Support BMG PMT features for Xe

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Wed, Nov 13, 2024 at 03:52:01PM +0200, Ilpo Järvinen wrote:
> On Wed, 13 Nov 2024, Andy Shevchenko wrote:
> > On Tue, Nov 12, 2024 at 11:30:33AM -0500, Michael J. Ruhl wrote:
> > > Updates for PMT to support user offsets from the sysfs API.
> > > 
> > > Addressed review comments for the Xe driver udpates.
> > 
> > FWIW,
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > If you have wish and time, there are problems with the drivers of different
> > severities (from "fine as is" to "good to be fixed, but okay as is") I have
> > noticed so far:
> > - it uses s*printf() instead of sysfs_emit*()
> > - it most likely never tested the corner cases. e.g.,
> > 
> > 	if (disc_res->start >= pci_resource_start(pci_dev, i) &&
> > 	    (disc_res->start <= pci_resource_end(pci_dev, i))) {
> > 
> >   what is this supposed to mean? Probably someone wanted resource_contains() or
> >   alike to be called here.
> > - slightly above the above piece the for-loop
> > 
> > 	for (i = 0; i < 6; i++)
> > 
> >   which probably want to use PCI_STD_RESOURCE_END)
> 
> While both work, in practice PCI_STD_NUM_BARS is way more common than 
> PCI_STD_RESOURCE_END.

Ah, indeed. I always forget that we have two.

-- 
With Best Regards,
Andy Shevchenko