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
