# Re: \[PATCH v5 42/42\] pci/hotplug: PowerPC PowerNV PCI hotplug driver

- From: Benjamin Herrenschmidt \<benh@kernel.crashing.org\>
- Date: 2015-06-05 20:18:15 
- List: linuxppc-dev
- Page: https://ratatoskr.run/linuxppc-dev/2015/06/8108957
- Thread: https://ratatoskr.run/linux-devicetree/2015/06/8108957/t.md

```text
On Fri, 2015-06-05 at 15:11 -0500, Bjorn Helgaas wrote:

> You didn't add this, but "pcibios_add_pci_devices" doesn't seem like the
> right name.  "pcibios" generally refers to an arch-specific hook that's
> called by the generic PCI core.  In this case, pcibios_add_pci_devices()
> contains powerpc-specific code, and it's only called from powerpc code, so
> I think using "pcibios_" in the name is a bit misleading.

Maybe but just calling it pci_add_* makes it easy to confuse with a core
function and ppc_add_* is gross :-)

> > +	/* Remove all devices behind the slot */
> > +	pci_lock_rescan_remove();
> > +	pcibios_remove_pci_devices(slot->bus);
> 
> Same comment for pcibios_remove_pci_devices().  It would be better if the
> name didn't suggest that this was part of the pcibios_ interface between
> the PCI core and the arch code, because it's not.
> 
> > +	/* Slot indentifier */
> 
> s/indentifier/identifier/
> 
> > +	if (!php_slot_get_id(dn, &id))
> > +		return NULL;
> > +
> 
> > +	/* PCI bus */
> > +	bus = pcibios_find_pci_bus(dn);
> 
> And pcibios_find_pci_bus() (it's also powerpc-specific).

This one could actually move to of_pci.c and be generic, something like
of_pci_node_to_bus()

Ben.
```
