On Dec 22, 2023 / 16:36, Andy Shevchenko wrote:
> 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.
Thanks. This will make p2sb_scan_and_cache() simpler. Will reflect it to v3
together with other your comments. I will also simplify
p2sb_scan_and_cache_devfn() to return void and skip p2sb_valid_resource() call.
