On Sat, Jun 06, 2015 at 06:18:15AM +1000, Benjamin Herrenschmidt wrote:
On Fri, 2015-06-05 at 15:11 -0500, Bjorn Helgaas wrote:
quoted
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 :-)
quoted
quoted
+ /* 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.
quoted
+ /* Slot indentifier */
s/indentifier/identifier/
quoted
+ if (!php_slot_get_id(dn, &id))
+ return NULL;
+
quoted
+ /* 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()
Thanks, Ben. I'll rename those functions as below if Bjorn won't object:
pcibios_add_pci_devices() pci_add_pci_devices()
pcibios_remove_pci_devices() pci_remove_pci_devices()
pcibios_find_pci_bus() of_node_to_pci_bus()
Thanks,
Gavin
Ben.