On 2018-05-15 09:54, Vladimir Zapolskiy wrote:
> Hi Jan,
>
> On 05/15/2018 08:58 AM, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>>
>> of_pci_get_host_bridge_resources() allocates the resource structures it
>> fills dynamically, but none of its callers care to release them so far.
>> Rather than requiring everyone to do this explicitly, convert the
>> existing function to a managed version.
>>
>> CC: Jingoo Han <jingoohan1@xxxxxxxxx>
>> CC: Joao Pinto <Joao.Pinto@xxxxxxxxxxxx>
>> CC: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
>> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>
> [snip]
>
>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>> index 4f21514cb4e4..00f42389aa56 100644
>> --- a/drivers/pci/of.c
>> +++ b/drivers/pci/of.c
>> @@ -244,7 +244,8 @@ EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
>>
>> #if defined(CONFIG_OF_ADDRESS)
>> /**
>> - * of_pci_get_host_bridge_resources - Parse PCI host bridge resources from DT
>> + * devm_of_pci_get_host_bridge_resources() - Resource-managed parsing of PCI
>> + * host bridge resources from DT
>> * @dev: host bridge device
>> * @busno: bus number associated with the bridge root bus
>> * @bus_max: maximum number of buses for this bridge
>> @@ -253,8 +254,6 @@ EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
>> * address for the start of the I/O range. Can be NULL if the caller doesn't
>> * expect I/O ranges to be present in the device tree.
>> *
>> - * It is the caller's job to free the @resources list.
>> - *
>> * This function will parse the "ranges" property of a PCI host bridge device
>> * node and setup the resource mapping based on its content. It is expected
>> * that the property conforms with the Power ePAPR document.
>> @@ -262,12 +261,11 @@ EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
>> * It returns zero if the range parsing has been successful or a standard error
>> * value if it failed.
>> */
>> -int of_pci_get_host_bridge_resources(struct device *dev,
>> +int devm_of_pci_get_host_bridge_resources(struct device *dev,
>> unsigned char busno, unsigned char bus_max,
>> struct list_head *resources, resource_size_t *io_base)
>> {
>> struct device_node *dev_node = dev->of_node;
>> - struct resource_entry *window;
>> struct resource *res;
>> struct resource *bus_range;
>> struct of_pci_range range;
>> @@ -278,7 +276,7 @@ int of_pci_get_host_bridge_resources(struct device *dev,
>> if (io_base)
>> *io_base = (resource_size_t)OF_BAD_ADDR;
>>
>> - bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL);
>> + bus_range = devm_kzalloc(dev, sizeof(*bus_range), GFP_KERNEL);
>> if (!bus_range)
>> return -ENOMEM;
>>
>> @@ -300,7 +298,7 @@ int of_pci_get_host_bridge_resources(struct device *dev,
>> /* Check for ranges property */
>> err = of_pci_range_parser_init(&parser, dev_node);
>> if (err)
>> - goto parse_failed;
>> + return err;
>
> In my opinion allocated by pci_add_resource() and pci_add_resource_offset()
> resource entries are leaked on error paths, and pci_free_resource_list() should
> be called.
Indeed, I overshot with removing also pci_free_resource_list. v4 will
follow.
Thanks,
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
