RE: [PATCH 2/4] PCI: rcar-pcie: Remove dependency on ARM-specific struct hw_pci

From: Phil Edworthy
Date: Mon Oct 19 2015 - 04:54:30 EST


Hi Bjorn,

Thanks for the review.

On 16 October 2015 22:34, Bjorn wrote:
> On Fri, Oct 02, 2015 at 11:25:05AM +0100, Phil Edworthy wrote:
> > The R-Car PCIe host controller driver uses pci_common_init_dev(),
> > which is ARM-specific and requires the ARM struct hw_pci. The part of
> > pci_common_init_dev() that is needed is limited and can be done here
> > without using hw_pci.
> >
> > Note that the ARM pcibios functions expect the PCI sysdata to be a pointer
> > to a struct pci_sys_data. Add a struct pci_sys_data as the first element
> > in struct gen_pci so that when we use a gen_pci pointer as sysdata, it is
> > also a pointer to a struct pci_sys_data.
> >
> > Create and scan the root bus directly without using the ARM
> > pci_common_init_dev() interface.
> >
> > This change is based on commit
> <499733e0cc1a00523c5056a690f65dea7b9da140>
> > "PCI: generic: Remove dependency on ARM-specific struct hw_pci".
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
> > ---
<snip>

> > +
> > + if (!pci_has_flag(PCI_PROBE_ONLY)) {
> > + pci_bus_size_bridges(bus);
> > + pci_bus_assign_resources(bus);
> >
> > - pci_common_init_dev(&pdev->dev, &rcar_pci);
> > + list_for_each_entry(child, &bus->children, node)
> > + pcie_bus_configure_settings(child);
>
> I see this is exactly the same in 499733e0cc1a ("PCI: generic: Remove
> dependency on ARM-specific struct hw_pci"). But it seems like we should do
> pcie_bus_configure_settings() (MPS configuration) *always*, even if
> PCI_PROBE_ONLY. I expected PCI_PROBE_ONLY to mean "don't change any
> BARs"
> but I don't think it means we have to preserve *everything*.
I guess it could be interpreted both ways.

At the moment, we don't have any need for PCI_PROBE_ONLY as we always
probe. However, I purposely used the same code as the generic driver so as
to make any future refactoring easier to identify.

I'm happy to change it or leave it the same, your call.

Thanks
Phil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/