From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liviu Dudau Subject: Re: [PATCH v10 07/10] OF: Introduce helper function for getting PCI domain_nr Date: Mon, 8 Sep 2014 15:54:59 +0100 Message-ID: <20140908145459.GO27864@e106497-lin.cambridge.arm.com> References: <1410184472-17630-1-git-send-email-Liviu.Dudau@arm.com> <1410184472-17630-8-git-send-email-Liviu.Dudau@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: Content-Disposition: inline Sender: linux-arch-owner@vger.kernel.org To: Rob Herring Cc: Bjorn Helgaas , Arnd Bergmann , Rob Herring , Jason Gunthorpe , Benjamin Herrenschmidt , Catalin Marinas , Will Deacon , Russell King , linux-pci , Linus Walleij , Tanmay Inamdar , Grant Likely , Sinan Kaya , Jingoo Han , Kukjin Kim , Suravee Suthikulanit , linux-arch , LKML , Device Tree ML , LAKML , "grant.likely@linaro.org" List-Id: devicetree@vger.kernel.org On Mon, Sep 08, 2014 at 03:27:56PM +0100, Rob Herring wrote: > On Mon, Sep 8, 2014 at 8:54 AM, Liviu Dudau wro= te: > > 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 > > Cc: Arnd Bergmann > > Cc: Grant Likely > > Cc: Rob Herring > > Reviewed-by: Catalin Marinas > > Signed-off-by: Liviu Dudau > > --- > > 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 *n= ode, struct resource *res) > > } > > EXPORT_SYMBOL_GPL(of_pci_parse_bus_range); > > > > +static atomic_t of_domain_nr =3D 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 th= at > > + * fails, a local allocator will be used. The local allocator can > > + * be requested to return a new domain_nr if the information is mi= ssing > > + * 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 n= umber > > + * if DT information is missing and @allocate_if_missing is true. = If > > + * @allocate_if_missing is false then the last allocated domain nu= mber > > + * will be returned. > > + */ > > +int of_pci_get_domain_nr(struct device_node *node, bool allocate_i= f_missing) > > +{ > > + int domain; > > + > > + domain =3D of_alias_get_id(node, "pci-domain"); > > + if (domain =3D=3D -ENODEV) { > > + if (allocate_if_missing) > > + domain =3D atomic_inc_return(&of_domain_nr)= ; > > + else > > + domain =3D atomic_read(&of_domain_nr); >=20 > This function seems a bit broken to me. It is overloaded with too man= y > 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. >=20 > Rob >=20 Rob, Would this version make more sense? int of_pci_get_domain_nr(struct device_node *node, bool allocate_if_mis= sing) { int domain; domain =3D of_alias_get_id(node, "pci-domain"); if (domain =3D=3D -ENODEV) { if (allocate_if_missing) domain =3D atomic_inc_return(&of_domain_nr); else domain =3D atomic_read(&of_domain_nr); } else { /* remember the largest value seen */ int d =3D 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 fil= ling 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 n= ot to make it too heavy weight. Best regards, Liviu --=20 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- =C2=AF\_(=E3=83=84)_/=C2=AF