Re: [PATCH v7 4/6] pci: Introduce a domain number for pci_host_bridge.

[Liviu Dudau <Liviu.Dudau@arm.com>]

On Tue, Apr 08, 2014 at 05:28:39PM +0100, Bjorn Helgaas wrote:
> On Tue, Apr 8, 2014 at 4:20 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > On Mon, Apr 07, 2014 at 11:44:51PM +0100, Bjorn Helgaas wrote:
> 
> >> Let me try to explain my concern about the
> >> pci_create_root_bus_in_domain() interface.  We currently have these
> >> interfaces:
> >>
> >>   pci_scan_root_bus()
> >>   pci_scan_bus()
> >>   pci_scan_bus_parented()
> >>   pci_create_root_bus()
> >> ...
> 
> >> One alternative is to add an _in_domain() variant of each of these
> >> interfaces, but that doesn't seem very convenient either.  My idea of
> >> passing in a structure would also require adding variants, so there's
> >> not really an advantage there, but I am thinking of the next
> >> unification effort, e.g., for NUMA node info.  I don't really want to
> >> have to change all the _in_domain() interfaces to also take yet
> >> another parameter for the node number.
> >
> > OK, what about this: all the functions that you have mentioned take a
> > void *sysdata parameter. Should we convert this opaque pointer into a
> > specific structure that holds the domain_nr and (in future) the NUMA
> > node info?
> 
> I doubt if we can make sysdata itself generic because I suspect we
> need a way to have *some* arch-specific data.  But maybe the arch
> could supply a structure containing a struct device *, domain, struct
> pci_ops *, list of resources, aperture info, etc.  I wonder if struct
> pci_host_bridge would be a reasonable place to put this stuff, e.g.,
> something like this:
> 
>   struct pci_host_bridge {
>     int domain;
>     int node;
>     struct device *dev;
>     struct pci_ops *ops;
>     struct list_head resources;
>     void *sysdata;
>     struct pci_bus *bus;  /* filled in by core, not by arch */
>     ... /* other existing contents managed by core */
>   };
> 
>   struct pci_bus *pci_scan_host_bridge(struct pci_host_bridge *bridge);

I'm really reluctant to give the arches more rope to hang themselves. I
really dislike the use of xxxx_initcall() to do PCI initialisation ordering
that is currently in widespread use through the arch code. As I hope to
have proven with my arm64 code, you can have PCI support for an architecture
without having to provide any arch specific code. We have enough correct
code in the PCI framework, what would the architectures provide to the generic
code that we cannot get by following the standard?

Of course, there are always arch specific corners and they need their data
structures to make sense of those, but rather than having architectures
fill in a structure *before* we can setup host bridges I think we need
to reverse the order. Using your example structure, I don't think is
the arch's job to provide the list of resources or the domain number
before we can scan the host bridge. We should be able to get those from
somewhere else (like adding by default the ioport_resource and
iomem_resource and managing domain numbers inside the core framework).

Best regards,
Liviu

> --
> 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
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯