On Fri, 2026-05-15 at 08:13 -0700, Dave Hansen wrote:
> On 5/15/26 07:22, Gerd Bayer wrote:
> >  static int __init pcibios_assign_resources(void)
> >  {
> > -   struct pci_bus *bus;
> > +   struct pci_bus *bus = NULL;
> >  
> >     if (!(pci_probe & PCI_ASSIGN_ROMS))
> > -           list_for_each_entry(bus, &pci_root_buses, node)
> > +           while ((bus = pci_find_next_bus(bus)) != NULL)
> >                     pcibios_allocate_rom_resources(bus);
> 
> What's with the 'bus = NULL'? I thought there was some crazy macro magic
> going on or something, but pci_find_next_bus() looks like a normal
> function that's just taking a pointer and not _modifying_ the pointer value.

Initializing 'bus = NULL" makes sure, that pci_find_next_bus() starts
at the list head; list_for_each_entry() did that implicitly. I didn't
want to rely on implicit zero-init for local var's on all the various
architectures. But I'm fine to drop it here, if you prefer.

> 
> Also, wouldn't this be a more readable way of writing what you have?
> 
>       while (bus = pci_find_next_bus(bus))

Yeah, another occasion of me being (overly?) verbose.
arch/sparc/kernel/pci.c was my blueprint. Again, something that I'm ok
to drop.

> 
> For that matter isn't the kernel idiom for these things:
> 
>       for_each_pci_bus(bus) {
>               // do bus stuff
>       }
> 
> I'm kinda surprised there isn't one of those already.

Just guessing: There was too little use of pci_find_next_bus() to
warrant that short-cut. But I can make a proposal in the next
iteration.

Thanks,
Gerd

Reply via email to