| Message ID | 20170426111809.19922-6-lorenzo.pieralisi@arm.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, Apr 26, 2017 at 1:17 PM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > The introduction of pci_scan_root_bus_bridge() provides a PCI core > API to scan a PCI root bus backed by an already initialized > struct pci_host_bridge object, which simplifies the bus scan > interface and makes the PCI scan root bus interface easier to > generalize as members are added to the struct pci_host_bridge(). > > Convert ARM dove platform code to pci_scan_root_bus_bridge() to improve > the PCI root bus scanning interface. > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Is this patch required for one of the later steps in the series? As non-DT dove uses the traditional pci_common_init() helper rather than registering its own driver, I wonder if there is anything to gain here. Arnd
On Fri, Apr 28, 2017 at 2:38 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Wed, Apr 26, 2017 at 1:17 PM, Lorenzo Pieralisi > <lorenzo.pieralisi@arm.com> wrote: >> The introduction of pci_scan_root_bus_bridge() provides a PCI core >> API to scan a PCI root bus backed by an already initialized >> struct pci_host_bridge object, which simplifies the bus scan >> interface and makes the PCI scan root bus interface easier to >> generalize as members are added to the struct pci_host_bridge(). >> >> Convert ARM dove platform code to pci_scan_root_bus_bridge() to improve >> the PCI root bus scanning interface. >> >> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > Is this patch required for one of the later steps in the series? > > As non-DT dove uses the traditional pci_common_init() helper rather > than registering its own driver, I wonder if there is anything to gain here. Thinking about it some more, if we make the change to allocate from pcibios_init_hw(), we can also initialize most of the fields there and do the cleanup in common code when the scan callback fails, which in turn makes the changes in arch/arm/mach*/pci.c drivers very simple. Arnd
On Fri, Apr 28, 2017 at 02:38:48PM +0200, Arnd Bergmann wrote: > On Wed, Apr 26, 2017 at 1:17 PM, Lorenzo Pieralisi > <lorenzo.pieralisi@arm.com> wrote: > > The introduction of pci_scan_root_bus_bridge() provides a PCI core > > API to scan a PCI root bus backed by an already initialized > > struct pci_host_bridge object, which simplifies the bus scan > > interface and makes the PCI scan root bus interface easier to > > generalize as members are added to the struct pci_host_bridge(). > > > > Convert ARM dove platform code to pci_scan_root_bus_bridge() to improve > > the PCI root bus scanning interface. > > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > Is this patch required for one of the later steps in the series? > > As non-DT dove uses the traditional pci_common_init() helper rather > than registering its own driver, I wonder if there is anything to gain here. Well, the point is, the non-DT platforms I patched implement a custom .scan method in struct hw_pci. If we move the bridge allocation to pcibios_init_hw() we would end up initializing some struct pci_host_bridge fields in pcibios_init_hw() and some in the custom .scan method (ie custom pci_ops) which I found not very elegant but it could be done I reckon, I need to give it a go to see how the code looks like. Lorenzo
On Wed, May 3, 2017 at 12:31 PM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Fri, Apr 28, 2017 at 02:38:48PM +0200, Arnd Bergmann wrote: >> On Wed, Apr 26, 2017 at 1:17 PM, Lorenzo Pieralisi >> <lorenzo.pieralisi@arm.com> wrote: >> > The introduction of pci_scan_root_bus_bridge() provides a PCI core >> > API to scan a PCI root bus backed by an already initialized >> > struct pci_host_bridge object, which simplifies the bus scan >> > interface and makes the PCI scan root bus interface easier to >> > generalize as members are added to the struct pci_host_bridge(). >> > >> > Convert ARM dove platform code to pci_scan_root_bus_bridge() to improve >> > the PCI root bus scanning interface. >> > >> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >> >> Is this patch required for one of the later steps in the series? >> >> As non-DT dove uses the traditional pci_common_init() helper rather >> than registering its own driver, I wonder if there is anything to gain here. > > Well, the point is, the non-DT platforms I patched implement a custom > .scan method in struct hw_pci. If we move the bridge allocation to > pcibios_init_hw() we would end up initializing some struct > pci_host_bridge fields in pcibios_init_hw() and some in the custom .scan > method (ie custom pci_ops) which I found not very elegant but it could be > done I reckon, I need to give it a go to see how the code looks like. I don't see anything wrong with initializing the members in different places. The first set would give you a default that works for basic drivers, and the second set is a way to override the defaults for drivers that do something special. Conceptually we do this all the time in other places. Arnd
diff --git a/arch/arm/mach-dove/pcie.c b/arch/arm/mach-dove/pcie.c index 91fe971..999e465 100644 --- a/arch/arm/mach-dove/pcie.c +++ b/arch/arm/mach-dove/pcie.c @@ -155,13 +155,31 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL, PCI_ANY_ID, rc_pci_fixup); static struct pci_bus __init * dove_pcie_scan_bus(int nr, struct pci_sys_data *sys) { + struct pci_host_bridge *bridge; + int ret; + if (nr >= num_pcie_ports) { BUG(); return NULL; } - return pci_scan_root_bus(NULL, sys->busnr, &pcie_ops, sys, - &sys->resources); + bridge = pci_alloc_host_bridge(0); + if (!bridge) + return NULL; + + list_splice_init(&sys->resources, &bridge->windows); + bridge->dev.parent = NULL; + bridge->sysdata = sys; + bridge->busnr = sys->busnr; + bridge->ops = &pcie_ops; + + ret = pci_scan_root_bus_bridge(bridge); + if (ret < 0) { + pci_free_host_bridge(bridge); + return NULL; + } + + return bridge->bus; } static int __init dove_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
The introduction of pci_scan_root_bus_bridge() provides a PCI core API to scan a PCI root bus backed by an already initialized struct pci_host_bridge object, which simplifies the bus scan interface and makes the PCI scan root bus interface easier to generalize as members are added to the struct pci_host_bridge(). Convert ARM dove platform code to pci_scan_root_bus_bridge() to improve the PCI root bus scanning interface. Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Jason Cooper <jason@lakedaemon.net> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Russell King <linux@armlinux.org.uk> Cc: Andrew Lunn <andrew@lunn.ch> --- arch/arm/mach-dove/pcie.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)