Re: [PATCH V8 5/9] pci, acpi: add acpi hook to assign domain number.

[Tomasz Nowicki]

On 10.06.2016 17:49, Lorenzo Pieralisi wrote:
> On Fri, Jun 10, 2016 at 04:14:58PM +0100, Lorenzo Pieralisi wrote:
> > Hi Bjorn, Tomasz,
> >
> > On Tue, Jun 07, 2016 at 07:15:59PM -0500, Bjorn Helgaas wrote:
> >
> > [...]
> >
> > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > > index eb431b5..2b52178 100644
> > > > --- a/drivers/pci/pci.c
> > > > +++ b/drivers/pci/pci.c
> > > > @@ -7,6 +7,7 @@
> > > >    *	Copyright 1997 -- 2000 Martin Mares <mj@xxxxxx>
> > > >    */
> > > >
> > > > +#include <linux/acpi.h>
> > > >   #include <linux/kernel.h>
> > > >   #include <linux/delay.h>
> > > >   #include <linux/init.h>
> > > > @@ -4941,7 +4942,7 @@ int pci_get_new_domain_nr(void)
> > > >   }
> > > >
> > > >   #ifdef CONFIG_PCI_DOMAINS_GENERIC
> > > > -void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent)
> > > > +static int of_pci_bus_domain_nr(struct device *parent)
> > >
> > > Can we do a little cleanup before this patch?
> > >
> > >    - pci_bus_assign_domain_nr() is only used inside drivers/pci, so
> > >      maybe we move the prototype to drivers/pci/pci.h?
> > >
> > >    - I don't really like the style of calling a function that
> > >      internally assigns bus->domain_nr.  Could we do something like
> > >      this instead?
> > >
> > >      int pci_bus_domain_nr(...)
> > >      {
> > >        ...
> > >        return domain;
> > >      }
> > >
> > >      ... pci_create_root_bus(...)
> > >      {
> > >        ...
> > >        b->domain_nr = pci_bus_domain_nr(...);
> >
> > We noticed while preparing v9, that this would force us to
> > write an empty pci_bus_domain_nr() prototype for
> > !PCI_DOMAINS_GENERIC (ie every arch but ARM/ARM64) that should
> > return 0 to keep current behaviour unchanged.
> >
> > That's why pci_bus_assign_domain_nr() was there, so that it
> > was compiled out on !PCI_DOMAINS_GENERIC.
> >
> > I really would like v9 to be final so let's fix it before posting it
> > shortly please.
> >
> > For the above we have three options:
> >
> > 1) Leave code as-is in v8
> >
> > 2) in pci_create_root_bus():
> > #ifdef CONFIG_PCI_DOMAINS_GENERIC
> >         b->domain_nr = pci_bus_domain_nr(...);
> > #endif
> >
> > + other changes requested above
> >
> > 3) in pci_create_root_bus()
> >
> > b->domain_nr = pci_bus_domain_nr(...);
> >
> > unguarded and a stub:
> >
> > #ifndef CONFIG_PCI_DOMAINS_GENERIC
> > static inline int pci_bus_domain_nr() { return 0; }
> > #endif
> >
> > + other changes requested above
>
> Actually, Tomasz made me notice that pci_bus.domain_nr is
> only declared for CONFIG_PCI_DOMAINS_GENERIC so (3) is not
> even an option and IMO (2) is not much nicer than code in
> v8 as-is with an ifdef in the middle of pci_create_root_bus().

To me (1) is nicer too. Bjorn what is your take on this? This is last 
bit before sending v9.

Thanks,
Tomasz