On Mon, Sep 08, 2014 at 03:27:56PM +0100, Rob Herring wrote: > On Mon, Sep 8, 2014 at 8:54 AM, Liviu Dudau <Liviu.Dudau@xxxxxxx> wrote: > > Add of_pci_get_domain_nr() to retrieve the PCI domain number > > of a given device from DT. If the information is not present, > > the function can be requested to allocate a new domain number. > > > > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > Cc: Arnd Bergmann <arnd@xxxxxxxx> > > Cc: Grant Likely <grant.likely@xxxxxxxxxx> > > Cc: Rob Herring <robh+dt@xxxxxxxxxx> > > Reviewed-by: Catalin Marinas <catalin.marinas@xxxxxxx> > > Signed-off-by: Liviu Dudau <Liviu.Dudau@xxxxxxx> > > --- > > drivers/of/of_pci.c | 34 ++++++++++++++++++++++++++++++++++ > > include/linux/of_pci.h | 7 +++++++ > > 2 files changed, 41 insertions(+) > > > > diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c > > index 8481996..a107edb 100644 > > --- a/drivers/of/of_pci.c > > +++ b/drivers/of/of_pci.c > > @@ -89,6 +89,40 @@ int of_pci_parse_bus_range(struct device_node *node, struct resource *res) > > } > > EXPORT_SYMBOL_GPL(of_pci_parse_bus_range); > > > > +static atomic_t of_domain_nr = ATOMIC_INIT(-1); > > + > > +/** > > + * This function will try to obtain the host bridge domain number by > > + * using of_alias_get_id() call with "pci-domain" as a stem. If that > > + * fails, a local allocator will be used. The local allocator can > > + * be requested to return a new domain_nr if the information is missing > > + * from the device tree. > > + * > > + * @node: device tree node with the domain information > > + * @allocate_if_missing: if DT lacks information about the domain nr, > > + * allocate a new number. > > + * > > + * Returns the associated domain number from DT, or a new domain number > > + * if DT information is missing and @allocate_if_missing is true. If > > + * @allocate_if_missing is false then the last allocated domain number > > + * will be returned. > > + */ > > +int of_pci_get_domain_nr(struct device_node *node, bool allocate_if_missing) > > +{ > > + int domain; > > + > > + domain = of_alias_get_id(node, "pci-domain"); > > + if (domain == -ENODEV) { > > + if (allocate_if_missing) > > + domain = atomic_inc_return(&of_domain_nr); > > + else > > + domain = atomic_read(&of_domain_nr); > > This function seems a bit broken to me. It is overloaded with too many > different outcomes. Think about how this would work if you have > multiple PCI buses and a mixture of having pci-domain aliases or not. > Aren't domain numbers global? Allocation should then start outside of > the range of alias ids. > > Rob > Rob, Would this version make more sense? int of_pci_get_domain_nr(struct device_node *node, bool allocate_if_missing) { int domain; domain = of_alias_get_id(node, "pci-domain"); if (domain == -ENODEV) { if (allocate_if_missing) domain = atomic_inc_return(&of_domain_nr); else domain = atomic_read(&of_domain_nr); } else { /* remember the largest value seen */ int d = atomic_read(&of_domain_nr); atomic_set(&of_domain_nr, max(domain, d)); } return domain; } EXPORT_SYMBOL_GPL(of_pci_get_domain_nr); It would still create gaps and possible duplicates, but this is just a number and trying to create a new root bus in an existing domain should fail. I have no clue on how to generate unique values without parsing the DT and filling a sparse array with values found there and then checking for allocated values on new requests. This function gets called quite a lot and I'm trying not to make it too heavy weight. Best regards, Liviu -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯ -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html