On Mon, Sep 22, 2014 at 12:55 PM, Liviu Dudau [off-list ref] wrote:

On Sat, Sep 20, 2014 at 01:28:38AM +0100, Rob Herring wrote:

quoted

On 09/19/2014 04:06 PM, Bjorn Helgaas wrote:

quoted

On Thu, Sep 18, 2014 at 02:30:23AM +0100, Liviu Dudau wrote:

quoted

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 <bhelgaas@google.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Grant Likely <redacted>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---

[...]

quoted

quoted

+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 = OF_BAD_ADDR;
+
+  bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL);

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.

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 already
and it will clean up. If it is successful, then it is the job of the caller
to free the list, as mentioned in the comment associated with the function.

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 passing
it to pci_scan_root_bus() for example.

Okay, as long as the caller has a single clean-up responsibility, I
guess it is fine.

Rob