| Message ID | 1430307599-20536-1-git-send-email-jchandra@broadcom.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Hi Jayachandran, On Wed, Apr 29, 2015 at 12:39:58PM +0100, Jayachandran C wrote: > The current code in pci-host-generic.c uses pci_common_init_dev() > from the arch/arm/ to do a part of the PCI initialization, and this > prevents it from being used on arm64. > > The initialization done by pci_common_init_dev() that is really > needed by pci-host-generic.c can be done in the same file without > using the hw_pci API of ARM. > > The ARM platform requires a pci_sys_data as sysdata for the PCI bus, > this is be handled by setting up 'struct gen_pci' to embed a > pci_sys_data variable as the first element on the ARM platform. > > Signed-off-by: Jayachandran C <jchandra@broadcom.com> > --- > > 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. You should be able to test using a 32-bit guest under KVM. > - There is only one ifdef, and that can be removed when arm64 gets > a sysdata, or when arm loses its sysdata. Anyway, I believe Lorenzo (CC'd) has already been working on this so I'll let him comment here. Will > diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c > index ba46e58..1631d34 100644 > --- a/drivers/pci/host/pci-host-generic.c > +++ b/drivers/pci/host/pci-host-generic.c > @@ -39,6 +39,9 @@ struct gen_pci_cfg_windows { > }; > > 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 +51,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 +66,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,11 +199,33 @@ 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) > +static int gen_pci_init(struct device *dev, struct gen_pci *pci) > { > - struct gen_pci *pci = sys->private_data; > - list_splice_init(&pci->resources, &sys->resources); > - return 1; > + struct pci_bus *bus; > + > + 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; > } > > static int gen_pci_probe(struct platform_device *pdev) > @@ -214,13 +237,6 @@ 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, > - }; > > if (!pci) > return -ENOMEM; > @@ -258,8 +274,7 @@ static int gen_pci_probe(struct platform_device *pdev) > return err; > } > > - pci_common_init_dev(dev, &hw); > - return 0; > + return gen_pci_init(dev, pci); > } > > static struct platform_driver gen_pci_driver = { > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 29 April 2015 17:09:58 Jayachandran C wrote: > The current code in pci-host-generic.c uses pci_common_init_dev() > from the arch/arm/ to do a part of the PCI initialization, and this > prevents it from being used on arm64. > > The initialization done by pci_common_init_dev() that is really > needed by pci-host-generic.c can be done in the same file without > using the hw_pci API of ARM. > > The ARM platform requires a pci_sys_data as sysdata for the PCI bus, > this is be handled by setting up 'struct gen_pci' to embed a > pci_sys_data variable as the first element on the ARM platform. > > Signed-off-by: Jayachandran C <jchandra@broadcom.com> This seems very useful > > -static int gen_pci_setup(int nr, struct pci_sys_data *sys) > +static int gen_pci_init(struct device *dev, struct gen_pci *pci) > { > - struct gen_pci *pci = sys->private_data; > - list_splice_init(&pci->resources, &sys->resources); > - return 1; > + struct pci_bus *bus; > + > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); Do we want to set that flag unconditionally? At least for servers, the resources should get assigned by firmware, and things might go wrong if Linux tries to reassign them. This is probably the case on kvmtool as well. > + 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); I thought this was done by default now. If it is not, can we make the of_irq swizzling the default? > + 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; > } We should also try to come up wtih a way to make that pcie_bus_configure_settings() automatic if it's required here. > static int gen_pci_probe(struct platform_device *pdev) > @@ -214,13 +237,6 @@ 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, > - }; > > if (!pci) > return -ENOMEM; > @@ -258,8 +274,7 @@ static int gen_pci_probe(struct platform_device *pdev) > return err; > } > > - pci_common_init_dev(dev, &hw); > - return 0; > + return gen_pci_init(dev, pci); How about moving the new code right into the probe() function? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 29 April 2015 19:55:55 Jayachandran C. wrote: > On Wed, Apr 29, 2015 at 02:34:20PM +0200, Arnd Bergmann wrote: > > On Wednesday 29 April 2015 17:09:58 Jayachandran C wrote: > > > The current code in pci-host-generic.c uses pci_common_init_dev() > > > from the arch/arm/ to do a part of the PCI initialization, and this > > > prevents it from being used on arm64. > > > > > > The initialization done by pci_common_init_dev() that is really > > > needed by pci-host-generic.c can be done in the same file without > > > using the hw_pci API of ARM. > > > > > > The ARM platform requires a pci_sys_data as sysdata for the PCI bus, > > > this is be handled by setting up 'struct gen_pci' to embed a > > > pci_sys_data variable as the first element on the ARM platform. > > > > > > Signed-off-by: Jayachandran C <jchandra@broadcom.com> > > > > This seems very useful > > I have tried to ensure that the new code is as much as possible > functionally equivalent to the call to pci_common_init_dev on ARM. > > I should have added this as a note to the patch. > > Ideally, I would like this patch to be functionally equivalent > to the existing code, and make further improvements with follow up > patches, but I can make the changes you suggest if it is needed. I believe the driver is only used in virtual machines at the moment, so it should be easy enough to test the relevant cases on arm32 and make it as simple as possible. For bisection purposes, it could however be better to start with your current patch and then follow it up with a second patch that removes the unnecessary parts again immediately. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 29, 2015 at 01:34:20PM +0100, Arnd Bergmann wrote: > On Wednesday 29 April 2015 17:09:58 Jayachandran C wrote: > > The current code in pci-host-generic.c uses pci_common_init_dev() > > from the arch/arm/ to do a part of the PCI initialization, and this > > prevents it from being used on arm64. > > > > The initialization done by pci_common_init_dev() that is really > > needed by pci-host-generic.c can be done in the same file without > > using the hw_pci API of ARM. > > > > The ARM platform requires a pci_sys_data as sysdata for the PCI bus, > > this is be handled by setting up 'struct gen_pci' to embed a > > pci_sys_data variable as the first element on the ARM platform. > > > > Signed-off-by: Jayachandran C <jchandra@broadcom.com> > > This seems very useful Yes, it is getting less awful, waiting for pci_sys_data to disappear. > > -static int gen_pci_setup(int nr, struct pci_sys_data *sys) > > +static int gen_pci_init(struct device *dev, struct gen_pci *pci) > > { > > - struct gen_pci *pci = sys->private_data; > > - list_splice_init(&pci->resources, &sys->resources); > > - return 1; > > + struct pci_bus *bus; > > + > > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); > > Do we want to set that flag unconditionally? At least for servers, > the resources should get assigned by firmware, and things might > go wrong if Linux tries to reassign them. This is probably the > case on kvmtool as well. I will give it a go on kvmtool, but at first glance concern is shared. > > + 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); > > I thought this was done by default now. If it is not, can > we make the of_irq swizzling the default? Possibly yes. > > + 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; > > } > > We should also try to come up wtih a way to make that > pcie_bus_configure_settings() automatic if it's required here. > > > static int gen_pci_probe(struct platform_device *pdev) > > @@ -214,13 +237,6 @@ 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, > > - }; > > > > if (!pci) > > return -ENOMEM; > > @@ -258,8 +274,7 @@ static int gen_pci_probe(struct platform_device *pdev) > > return err; > > } > > > > - pci_common_init_dev(dev, &hw); > > - return 0; > > + return gen_pci_init(dev, pci); > > How about moving the new code right into the probe() function? +1. I suspect the PCI_PROBE_ONLY case was not tested on arm64, since we still need a patch to prevent resources enablement there (in the pcibios_enable_device call - actually the check for PCI_PROBE_ONLY should be moved to the default pcibios_enable_device function in drivers/pci/pci.c). The other bit missing is MSI handling, that should still be implemented. Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> The other bit missing is MSI handling, that should still be implemented.
Hello! I decided to have a voice here.
I am currently working on a virtualization project, and i have made some
experimental additions to PCI-Generic code which enable PCI-X support. I
wanted to upstream this, but as far as i can see, you are refactoring
everything here.
Personally i think that pci_common_init_dev() is a good thing and it should
not be removed. Even more, perhaps it needs to be expanded with PCI-X
support. My current code adds a piece very similar to
mvebu_pcie_msi_enable(), which reads "msi-parent" property and then calls
of_pci_find_msi_chip_by_node().
MVEBU driver then fills in hw.msi_ctrl with the obtained pointer and calls
pci_common_init(), which is the same as pci_common_init_dev(NULL, hw). My
modified Generic driver now does the same. So, the question is - may be we
should extract "msi-parent" handling from MVEBU and move it into
pci_common_init_dev() ?
Just IMHO copy-pasting this function over and over again in all drivers is
a bad idea.
And one more. pci_common_init_dev() currently does not assign bus->msi,
therefore IRQ domain operations do not work. I have fixed this too, but
perhaps it's another story...
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 30 April 2015 09:40:10 Pavel Fedin wrote: > > The other bit missing is MSI handling, that should still be implemented. > > Hello! I decided to have a voice here. > I am currently working on a virtualization project, and i have made some > experimental additions to PCI-Generic code which enable PCI-X support. I > wanted to upstream this, but as far as i can see, you are refactoring > everything here. > Personally i think that pci_common_init_dev() is a good thing and it should > not be removed. Even more, perhaps it needs to be expanded with PCI-X > support. My current code adds a piece very similar to > mvebu_pcie_msi_enable(), which reads "msi-parent" property and then calls > of_pci_find_msi_chip_by_node(). > MVEBU driver then fills in hw.msi_ctrl with the obtained pointer and calls > pci_common_init(), which is the same as pci_common_init_dev(NULL, hw). My > modified Generic driver now does the same. So, the question is - may be we > should extract "msi-parent" handling from MVEBU and move it into > pci_common_init_dev() ? > Just IMHO copy-pasting this function over and over again in all drivers is > a bad idea. > > And one more. pci_common_init_dev() currently does not assign bus->msi, > therefore IRQ domain operations do not work. I have fixed this too, but > perhaps it's another story... Hi Pavel, There are a few problems with pci_common_init_dev() that we are solving by not calling it: - The interface is ARM specific, and not easily extended to other architectures that have requirements it does not handle today, and we definitely want to share host drivers between arm, arm64, mips and microblaze at least, probably most others in the long run. - The lack of return code and error handling means it is not good for pci host drivers in loadable modules. - The way that the interface registers multiple host bridges at the same time means that implementing that error handling is not possible without major changes. The plan forward at this point is to move over all drivers in drivers/pci/host to one of the architecture-independent interfaces. There is a lot of work going on at the moment to improve those interfaces to reduce code duplication and to make it as easy as possible to bring up a PCI host controller though, and there are probably more things that could be included there. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 30, 2015 at 10:59:50AM +0100, Jayachandran C. wrote: > On Wed, Apr 29, 2015 at 06:43:57PM +0100, Lorenzo Pieralisi wrote: > > On Wed, Apr 29, 2015 at 01:34:20PM +0100, Arnd Bergmann wrote: > > > On Wednesday 29 April 2015 17:09:58 Jayachandran C wrote: > > > > The current code in pci-host-generic.c uses pci_common_init_dev() > > > > from the arch/arm/ to do a part of the PCI initialization, and this > > > > prevents it from being used on arm64. > > > > > > > > The initialization done by pci_common_init_dev() that is really > > > > needed by pci-host-generic.c can be done in the same file without > > > > using the hw_pci API of ARM. > > > > > > > > The ARM platform requires a pci_sys_data as sysdata for the PCI bus, > > > > this is be handled by setting up 'struct gen_pci' to embed a > > > > pci_sys_data variable as the first element on the ARM platform. > > > > > > > > Signed-off-by: Jayachandran C <jchandra@broadcom.com> > > > > > > This seems very useful > > > > Yes, it is getting less awful, waiting for pci_sys_data to disappear. > > > > > > -static int gen_pci_setup(int nr, struct pci_sys_data *sys) > > > > +static int gen_pci_init(struct device *dev, struct gen_pci *pci) > > > > { > > > > - struct gen_pci *pci = sys->private_data; > > > > - list_splice_init(&pci->resources, &sys->resources); > > > > - return 1; > > > > + struct pci_bus *bus; > > > > + > > > > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); > > > > > > Do we want to set that flag unconditionally? At least for servers, > > > the resources should get assigned by firmware, and things might > > > go wrong if Linux tries to reassign them. This is probably the > > > case on kvmtool as well. > > I can skip setting this flag if PCI_PROBE_ONLY is set. Adding another > device tree parameter for this may not be needed. I think that's reasonable for the time being. > > I will give it a go on kvmtool, but at first glance concern is > > shared. > > > > > > + 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); > > > > > > I thought this was done by default now. If it is not, can > > > we make the of_irq swizzling the default? > > > > Possibly yes. > > I am not sure I understand this, I thought pci_fixup_irqs needs to be > called at this point. What we meant is that passing of_irq_parse_and_map_pci as a mapping function is the most common option, but I do not think there is much you can do at the moment apart from adding a pci_of_fixup_irqs function to the generic PCI layer and calling that instead, I do not see where the improvement is, so I think you can leave it as it is. > > > > + 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; > > > > } > > > > > > We should also try to come up wtih a way to make that > > > pcie_bus_configure_settings() automatic if it's required here. > > > > > > > static int gen_pci_probe(struct platform_device *pdev) > > > > @@ -214,13 +237,6 @@ 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, > > > > - }; > > > > > > > > if (!pci) > > > > return -ENOMEM; > > > > @@ -258,8 +274,7 @@ static int gen_pci_probe(struct platform_device *pdev) > > > > return err; > > > > } > > > > > > > > - pci_common_init_dev(dev, &hw); > > > > - return 0; > > > > + return gen_pci_init(dev, pci); > > > > > > How about moving the new code right into the probe() function? > > > > +1. > > I will add this to patch v2. > > > I suspect the PCI_PROBE_ONLY case was not tested on arm64, since we > > still need a patch to prevent resources enablement there (in the > > pcibios_enable_device call - actually the check for PCI_PROBE_ONLY > > should be moved to the default pcibios_enable_device function in > > drivers/pci/pci.c). > > > > The other bit missing is MSI handling, that should still be implemented. > > I think we should parse msi-parent and set ->msi on the root bus in > pci-host-generic.c. The implementation of pcibios_msi_controller() for > arm/arm64 can go up the bus hierarchy and return the first msi > controller it finds. This would be trivial to add to arm/arm64. > I can post a follow up patch for this if you are interested. I do not want to see a pcibios_msi_controller implementation on arm64, or put it differently what you are proposing has nothing in it arm64 specific, we need to find a solution that goes into generic code, there is already, pending further discussion: http://lwn.net/Articles/628872/ Thanks, Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4/29/15 12:43, Lorenzo Pieralisi wrote: > On Wed, Apr 29, 2015 at 01:34:20PM +0100, Arnd Bergmann wrote: >> >On Wednesday 29 April 2015 17:09:58 Jayachandran C wrote: >>> > >The current code in pci-host-generic.c uses pci_common_init_dev() >>> > >from the arch/arm/ to do a part of the PCI initialization, and this >>> > >prevents it from being used on arm64. >>> > > >>> > >The initialization done by pci_common_init_dev() that is really >>> > >needed by pci-host-generic.c can be done in the same file without >>> > >using the hw_pci API of ARM. >>> > > >>> > >The ARM platform requires a pci_sys_data as sysdata for the PCI bus, >>> > >this is be handled by setting up 'struct gen_pci' to embed a >>> > >pci_sys_data variable as the first element on the ARM platform. >>> > > >>> > >Signed-off-by: Jayachandran C<jchandra@broadcom.com> >> > >> >This seems very useful > Yes, it is getting less awful, waiting for pci_sys_data to disappear. > Lorenzo, A while back, you mentioned here (https://lkml.org/lkml/2015/2/16/364) that the ARM32 pcibios_align_resource() implementation requires pci_sys_data, so we _still_ rely on pci_common_init_dev to create one for us. Is this still the case? I am looking at the arch/arm32/kernel/bios32.c: pcibios_init_hw() and see that it setup the pci_sys_data.align_resource to hw_pci.align_resource (see here http://lxr.free-electrons.com/source/arch/arm/kernel/bios32.c#L471). However it seems that the hw_pci.align_resource is never setup in the pci-host-generic.c. Am I missing something here? Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c index ba46e58..1631d34 100644 --- a/drivers/pci/host/pci-host-generic.c +++ b/drivers/pci/host/pci-host-generic.c @@ -39,6 +39,9 @@ struct gen_pci_cfg_windows { }; 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 +51,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 +66,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,11 +199,33 @@ 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) +static int gen_pci_init(struct device *dev, struct gen_pci *pci) { - struct gen_pci *pci = sys->private_data; - list_splice_init(&pci->resources, &sys->resources); - return 1; + struct pci_bus *bus; + + 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; } static int gen_pci_probe(struct platform_device *pdev) @@ -214,13 +237,6 @@ 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, - }; if (!pci) return -ENOMEM; @@ -258,8 +274,7 @@ static int gen_pci_probe(struct platform_device *pdev) return err; } - pci_common_init_dev(dev, &hw); - return 0; + return gen_pci_init(dev, pci); } static struct platform_driver gen_pci_driver = {
The current code in pci-host-generic.c uses pci_common_init_dev() from the arch/arm/ to do a part of the PCI initialization, and this prevents it from being used on arm64. The initialization done by pci_common_init_dev() that is really needed by pci-host-generic.c can be done in the same file without using the hw_pci API of ARM. The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this is be handled by setting up 'struct gen_pci' to embed a pci_sys_data variable as the first element on the ARM platform. Signed-off-by: Jayachandran C <jchandra@broadcom.com> --- 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 | 49 ++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 17 deletions(-)