On Wed, Jul 29, 2015 at 04:28:00PM +0100, Jayachandran C wrote: > The current code in pci-host-generic.c uses pci_common_init_dev() > from ARM platform to do some of the PCI initializations, and this > prevents it from being used in ARM64. > > The initialization done by pci_common_init_dev() that is needed > by pci-host-generic.c is really limited, and can be done easily > in the same file without using hw_pci API. The ARM platform > requires a pci_sys_data as sysdata for the PCI bus, this can be > handled by setting up gen_pci to have a pci_sys_data variable as > the first element. Ok, I still do not like leaving pci_sys_data there, and to remove it there is only one snag (+ one patch to prevent resources enablement on ARM64 PROBE_ONLY systems to have a consolidated ARM/ARM64 host generic), which consists in removing the align_resource pointer from pci_sys_data. A proposed solution (maybe not ideal, we can consider it a temporary step to make progress) here: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/359273.html Comments very appreciated on the patch above, I do not have major concerns with this patch other than the pci_sys_data hack, because that's what it is. Thanks, Lorenzo > Signed-off-by: Jayachandran C <jchandra@xxxxxxxxxxxx> > --- > Here's v3 of the patchset. > > v2-v3 > - rebase to 4.2-rc > - fix PCI_PROBE_ONLY check before calling pcie configure > - added a comment above sysdata > - updated the commit message > > v1->v2 > - Address comments from Arnd Bergmann and Lorenzo Pieralisi > - move contents of gen_pci_init to gen_pci_probe > - assign resources only when !probe_only > - tested on ARM32 with qemu option -M virt > > Notes: > - passing a zeroed out pci_sys_data for ARM looks ok, but I haven't > tested it on ARM. > - tested it on ARM64 fast model > - Any information on how this can be tested on arm is welcome. > - There is only one ifdef, and that can be removed when arm64 gets > a sysdata, or when arm loses its sysdata. > > drivers/pci/host/pci-host-generic.c | 55 ++++++++++++++++++++++++------------- > 1 file changed, 36 insertions(+), 19 deletions(-) > > diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c > index ba46e58..e9d9c80 100644 > --- a/drivers/pci/host/pci-host-generic.c > +++ b/drivers/pci/host/pci-host-generic.c > @@ -38,7 +38,15 @@ struct gen_pci_cfg_windows { > const struct gen_pci_cfg_bus_ops *ops; > }; > > +/* > + * ARM needs platform specific pci_sys_data as the sysdata for PCI. > + * We add the sys as the first field below to handle this. sys will > + * set to 0, so that the pci functions in do the right thing. > + */ > struct gen_pci { > +#ifdef CONFIG_ARM > + struct pci_sys_data sys; > +#endif > struct pci_host_bridge host; > struct gen_pci_cfg_windows cfg; > struct list_head resources; > @@ -48,8 +56,7 @@ static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus, > unsigned int devfn, > int where) > { > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = bus->sysdata; > resource_size_t idx = bus->number - pci->cfg.bus_range->start; > > return pci->cfg.win[idx] + ((devfn << 8) | where); > @@ -64,8 +71,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus, > unsigned int devfn, > int where) > { > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = bus->sysdata; > resource_size_t idx = bus->number - pci->cfg.bus_range->start; > > return pci->cfg.win[idx] + ((devfn << 12) | where); > @@ -198,13 +204,6 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci) > return 0; > } > > -static int gen_pci_setup(int nr, struct pci_sys_data *sys) > -{ > - struct gen_pci *pci = sys->private_data; > - list_splice_init(&pci->resources, &sys->resources); > - return 1; > -} > - > static int gen_pci_probe(struct platform_device *pdev) > { > int err; > @@ -214,13 +213,7 @@ static int gen_pci_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct device_node *np = dev->of_node; > struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL); > - struct hw_pci hw = { > - .nr_controllers = 1, > - .private_data = (void **)&pci, > - .setup = gen_pci_setup, > - .map_irq = of_irq_parse_and_map_pci, > - .ops = &gen_pci_ops, > - }; > + struct pci_bus *bus; > > if (!pci) > return -ENOMEM; > @@ -258,7 +251,31 @@ static int gen_pci_probe(struct platform_device *pdev) > return err; > } > > - pci_common_init_dev(dev, &hw); > + /* do not reassign resource if probe only */ > + if (!pci_has_flag(PCI_PROBE_ONLY)) > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); > + > + bus = pci_scan_root_bus(dev, 0, &gen_pci_ops, pci, &pci->resources); > + if (!bus) { > + dev_err(dev, "Scanning rootbus failed"); > + return -ENODEV; > + } > + > + pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci); > + > + if (!pci_has_flag(PCI_PROBE_ONLY)) { > + pci_bus_size_bridges(bus); > + pci_bus_assign_resources(bus); > + } > + pci_bus_add_devices(bus); > + > + /* Configure PCI Express settings */ > + if (!pci_has_flag(PCI_PROBE_ONLY)) { > + struct pci_bus *child; > + > + list_for_each_entry(child, &bus->children, node) > + pcie_bus_configure_settings(child); > + } > return 0; > } > > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
