Re: [PATCH v2] platform/x86: p2sb: Allow p2sb_bar() calls during PCI device probe

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

On Fri, Dec 22, 2023 at 04:25:10PM +0200, Andy Shevchenko wrote:
> On Fri, Dec 22, 2023 at 04:20:54PM +0900, Shin'ichiro Kawasaki wrote:

...

> > +static int p2sb_scan_and_cache(struct pci_bus *bus, unsigned int devfn)
> > +{
> > +	unsigned int slot, fn;
> > +	int ret;
> > +
> > +	/* Scan the P2SB device and cache its BAR0 */
> > +	ret = p2sb_scan_and_cache_devfn(bus, devfn);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/*
> > +	 * When function number of the P2SB device is zero, scan other function
> > +	 * numbers.
> > +	 */
> > +	if (PCI_FUNC(devfn) != 0)
> > +		return 0;
> > +
> > +	/* If devices are available, cache their BAR0 */
> > +	slot = PCI_SLOT(devfn);
> > +	for (fn = 1; fn < NR_P2SB_RES_CACHE; fn++)
> > +		p2sb_scan_and_cache_devfn(bus, PCI_DEVFN(slot, fn));
> > +
> > +	return 0;
> > +}
> 
> So, we can actually do the for-loop from index 0.
> 
> 	if (PCI_FUNC(devfn) == 0) {
> 		/*
> 		 * When function number of the P2SB device is zero, scan it
> 		 * and other function numbers, and if devices are available,
> 		 * cache their BAR0s.
> 		 */
> 		slot = PCI_SLOT(devfn);
> 
> // This will allow us to have something like pci_dev_for_each_func() in the future.
> 
> 		for (fn = 0; fn < NR_P2SB_RES_CACHE; fn++)
> 			p2sb_scan_and_cache_devfn(bus, PCI_DEVFN(slot, fn));
> 	} else {
> 		/* Scan the P2SB device and cache its BAR0 */
> 		ret = p2sb_scan_and_cache_devfn(bus, devfn);
> 		if (ret)
> 			return ret;
> 	}

Even going further

	if (PCI_FUNC(devfn) == 0) {
		/*
		 * When function number of the P2SB device is zero, scan it
		 * and other function numbers, and if devices are available,
		 * cache their BAR0s.
		 */
		slot = PCI_SLOT(devfn);
		for (fn = 0; fn < NR_P2SB_RES_CACHE; fn++)
			p2sb_scan_and_cache_devfn(bus, PCI_DEVFN(slot, fn));
	} else {
		/* Scan the P2SB device and cache its BAR0 */
		p2sb_scan_and_cache_devfn(bus, devfn);
	}

	return p2sb_valid_resource(&p2sb_resources[PCI_FUNC(devfn)]);

// With this it's maybe p2sb_scan_and_cache_devfn() can be void.

-- 
With Best Regards,
Andy Shevchenko