From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liviu Dudau Subject: Re: [PATCH v11 08/10] OF: PCI: Add support for parsing PCI host bridge resources from DT Date: Mon, 22 Sep 2014 18:55:06 +0100 Message-ID: <20140922175505.GH1994@e106497-lin.cambridge.arm.com> References: <1411003825-21521-1-git-send-email-Liviu.Dudau@arm.com> <1411003825-21521-9-git-send-email-Liviu.Dudau@arm.com> <20140919210610.GI19374@google.com> <541CCA36.8010606@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <541CCA36.8010606@gmail.com> 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 Sat, Sep 20, 2014 at 01:28:38AM +0100, Rob Herring wrote: > On 09/19/2014 04:06 PM, Bjorn Helgaas wrote: > > On Thu, Sep 18, 2014 at 02:30:23AM +0100, Liviu Dudau wrote: > >> Provide a function to parse the PCI DT ranges that can be used to > >> create a pci_host_bridge structure together with its associated > >> bus. > >> > >> Cc: Bjorn Helgaas > >> Cc: Arnd Bergmann > >> Cc: Grant Likely > >> Cc: Rob Herring > >> Cc: Catalin Marinas > >> Signed-off-by: Liviu Dudau > >> --- >=20 > [...] >=20 > >> +int of_pci_get_host_bridge_resources(struct device_node *dev, > >> + unsigned char busno, unsigned char bus_max, > >> + struct list_head *resources, resource_size_t *io_base) > >> +{ > >> + struct resource *res; > >> + struct resource *bus_range; > >> + struct of_pci_range range; > >> + struct of_pci_range_parser parser; > >> + char range_type[4]; > >> + int err; > >> + > >> + if (!io_base) > >> + return -EINVAL; > >> + *io_base =3D OF_BAD_ADDR; > >> + > >> + bus_range =3D kzalloc(sizeof(*bus_range), GFP_KERNEL); >=20 > This function does a lot of kalloc's but there is not an easy way to > undo those allocations. Hot unplug of a host bridge or probe error > handling would leak memory. >=20 > You could pass in struct device and use the devm_ variant (also > addressing Bjorn's comment), but not having an uninit/remove function > make what clean-up drivers have to do error prone. For example, on > uninit a driver needs to call pci_free_resource_list. If the function fails to parse the ranges for whatever reason it will call pci_free_resource_list on the resources that have been added alrea= dy and it will clean up. If it is successful, then it is the job of the ca= ller to free the list, as mentioned in the comment associated with the funct= ion. The reason why I am reluctant to use devm_ here is that the ranges are = only parsed here, no filtering is applied. Architectures and/or host bridge drivers can/could impose additional restrictions to the list before pas= sing it to pci_scan_root_bus() for example. 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