Hi Yijing, I wasn't quite sure I understood your comments, so I was trying to apply your patch series and test it, but patch 3 doesn't apply cleanly to 4.0-rc5 or master. Can you respin the series? Thanks, Daniel
Hi Daniel, thanks for your review and comments. We want to make a generic pci_host_bridge, which would hold the common host information, for example, pci domain is common info for pci host bridge, this series saved domain in pci_host_bridge, then we no need to extract out domain by pci_bus->sysdata by platform specific pci_domain_nr(). Also we store the sysdata in pci_host_bridge, and pci_bus_to_host() is the platform interface, I think use the common interface would be better.+ + /* Get probe mode and perform scan */ + if (hose->dn && ppc_md.pci_probe_mode) + mode = ppc_md.pci_probe_mode(bus); + + pr_debug(" probe mode: %d\n", mode); + if (mode == PCI_PROBE_DEVTREE) + of_scan_bus(hose->dn, bus); + + if (mode == PCI_PROBE_NORMAL) { + pci_bus_update_busn_res_end(bus, 255); + hose->last_busno = pci_scan_child_bus(bus); + pci_bus_update_busn_res_end(bus, hose->last_busno); + } + + return pci_bus_child_max_busnr(bus); +} +I'm having trouble convincing myself that this patch covers every variation within our PCI implementations. In particular, there's a stanza in of_scan_pci_bridge in kernel/pci_of_scan.c that's almost identical to this function. Does that implementation need to be cleaned up and replaced with this function too?This is a pci_host_bridge_ops hook function, which would be called in PCI core, and after applied this series, we only need to call pci_scan_host_bridge() to scan pci devices, and this function is also extracted from the pcibios_scan_phb(), it's not the redundant code.@@ -1641,9 +1655,9 @@ void pcibios_scan_phb(struct pci_controller *hose) ppc_md.pcibios_fixup_phb(hose); /* Configure PCI Express settings */ - if (bus && !pci_has_flag(PCI_PROBE_ONLY)) { + if (host->bus && !pci_has_flag(PCI_PROBE_ONLY)) { struct pci_bus *child; - list_for_each_entry(child, &bus->children, node) + list_for_each_entry(child, &host->bus->children, node) pcie_bus_configure_settings(child); } }Two things: Firstly, the function uses hose throughout, not host. Secondly, you're not deleting the bus variable: what's the purpose of this change?host is the common pci_host_bridge which is created by PCI core for pci host bridge driver, the hose is the platform data used in powerpc. The purpose of the patch/series is to simplify pci enumeration interface, and try to reduce the weak functions which were used to setup pci bus/devices during PCI enumeration.Regards, Daniel
Attachment:
signature.asc
Description: This is a digitally signed message part