From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liviu Dudau Subject: Re: [PATCH v12 08/12] PCI: OF: Introduce helper function for retrieving PCI domain numbers Date: Fri, 26 Sep 2014 23:47:52 +0100 Message-ID: <20140926224752.GB22030@e106497-lin.cambridge.arm.com> References: <1411498874-9864-1-git-send-email-Liviu.Dudau@arm.com> <1411498874-9864-9-git-send-email-Liviu.Dudau@arm.com> <20140926212052.GA6030@borg.dudau.co.uk> 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-pci-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 Fri, Sep 26, 2014 at 10:53:26PM +0100, Rob Herring wrote: > On Fri, Sep 26, 2014 at 4:20 PM, wrote: > > On Fri, Sep 26, 2014 at 01:20:39PM -0500, Rob Herring wrote: > >> On Tue, Sep 23, 2014 at 2:01 PM, Liviu Dudau = wrote: > >> > Add pci_get_new_domain_nr() to allocate a new domain number > >> > and of_get_pci_domain_nr() to retrieve the PCI domain number > >> > of a given device from DT. Host bridge drivers or architecture > >> > specific code can choose to implement their PCI domain number > >> > policy using these two functions. > >> > > >> > Using of_get_pci_domain_nr() guarantees a stable PCI domain > >> > number on every boot provided that all host bridge controllers > >> > are assigned a number in the device tree using "linux,pci-domain= " > >> > property. Mix use of pci_get_new_domain_nr() and of_get_pci_doma= in_nr() > >> > is not recommended as it can lead to potentially conflicting > >> > domain numbers being assigned to root busses behind different > >> > host bridges. > >> > > >> > Cc: Bjorn Helgaas > >> > Cc: Arnd Bergmann > >> > Cc: Grant Likely > >> > Cc: Rob Herring > >> > Cc: Catalin Marinas > >> > Signed-off-by: Liviu Dudau > >> > >> Looks pretty good, but a couple of comments that can be addressed = as > >> follow-up patches. > >> > >> > --- > >> > drivers/of/of_pci.c | 25 +++++++++++++++++++++++++ > >> > drivers/pci/pci.c | 9 +++++++++ > >> > include/linux/of_pci.h | 7 +++++++ > >> > include/linux/pci.h | 3 +++ > >> > 4 files changed, 44 insertions(+) > >> > > >> > diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c > >> > index 8481996..82d172f 100644 > >> > --- a/drivers/of/of_pci.c > >> > +++ b/drivers/of/of_pci.c > >> > @@ -89,6 +89,31 @@ int of_pci_parse_bus_range(struct device_node= *node, struct resource *res) > >> > } > >> > EXPORT_SYMBOL_GPL(of_pci_parse_bus_range); > >> > > >> > +/** > >> > + * This function will try to obtain the host bridge domain numb= er by > >> > + * finding a property called "linux,pci-domain" of the given de= vice node. > >> > + * > >> > + * @node: device tree node with the domain information > >> > + * > >> > + * Returns the associated domain number from DT in the range [0= -0xffff], or > >> > + * a negative value if the required property is not found. > >> > + */ > >> > +int of_get_pci_domain_nr(struct device_node *node) > >> > +{ > >> > + const __be32 *value; > >> > + int len; > >> > + u16 domain; > >> > + > >> > + value =3D of_get_property(node, "linux,pci-domain", &len= ); > >> > >> This needs to be documented. > > > > I would say needs agreed on *and* documented. >=20 > I thought Bjorn already applied the series. Who's agreement are you > waiting for specifically. Looks like he only applied about 60% of the series. I'm not waiting on anyones specifig agreement, just general agreement. Jason Gunthrope have initially suggested using pci-domain alias which you don't like, now I'm using "linux,pci-domain" property which you've ACKed, I wonder if I need to give people more time to catch up to the fact? >=20 > >> > >> > + if (!value || len < sizeof(*value)) > >> > + return -EINVAL; > >> > + > >> > + domain =3D (u16)be32_to_cpup(value); > >> > >> of_property_read_u32 would not work here? > > > > It should, I guess, but I want to limit the domain number range to = 16 bits. >=20 > Okay, but it would still be fewer lines to use of_property_read_u32 > and then cast it. You could use the u16 variant and make the property > be defined to be 16-bit in the documentation. True. Will do. Best regards, Liviu >=20 > Rob >=20 --=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