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