Re: [PATCH v4 3/4] pmdomain: core: add genpd_for_each_child() helper

[Kevin Hilman]

Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx> writes:

> On 26-08-19 10:08:38, Kevin Hilman (TI) wrote:
>> Add a new internal helper function genpd_for_each_child() that recursively
>> iterates over all devices in a PM domain and its child domains (subdomains).
>> This helper is useful for governors and other core PM domain code that needs
>> to examine or apply operations to all devices within a domain hierarchy.
>> 
>> The function takes a callback that is invoked for each device, and supports
>> early termination if the callback returns a non-zero value.
>> 
>> The helper is defined in a new internal header drivers/pmdomain/core.h and
>> implemented in drivers/pmdomain/core.c, making it available to other PM
>> domain subsystem components.
>> 
>> The first user of this helper is the cpu_system_power_down_ok() governor
>> function, which uses it to check device QoS latency constraints across the
>> entire domain hierarchy.
>> 
>> Signed-off-by: Kevin Hilman (TI) <khilman@xxxxxxxxxxxx>
>> ---
>>  drivers/pmdomain/core.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/pmdomain/core.h | 17 +++++++++++++++++
>>  2 files changed, 62 insertions(+)
>> 
>> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
>> index 842c4169e290..ff27369d9a97 100644
>> --- a/drivers/pmdomain/core.c
>> +++ b/drivers/pmdomain/core.c
>> @@ -24,6 +24,8 @@
>>  #include <linux/cpu.h>
>>  #include <linux/debugfs.h>
>>  
>> +#include "core.h"
>> +
>>  /* Provides a unique ID for each genpd device */
>>  static DEFINE_IDA(genpd_ida);
>>  
>> @@ -276,6 +278,49 @@ static void genpd_sd_counter_inc(struct generic_pm_domain *genpd)
>>  	smp_mb__after_atomic();
>>  }
>>  
>> +/**
>> + * genpd_for_each_child - Recursively iterate over all devices
>> + *                        in a PM domain and its subdomains.
>> + * @genpd: PM domain to iterate over.
>> + * @fn: Callback function to invoke for each device.
>> + * @data: Data to pass to the callback function.
>> + *
>> + * This function recursively walks through all devices in the given PM domain
>> + * and all devices in its child PM domains (subdomains). For each device found,
>> + * the callback function @fn is invoked with the device and @data as arguments.
>> + *
>> + * Note: this function is inteded for use by the core and governors,
>> + * not for pmdomain providers.
>> + *
>> + * Returns: 0 on success, or the first non-zero value returned by @fn.
>> + */
>> +int genpd_for_each_child(struct generic_pm_domain *genpd,
>> +			 int (*fn)(struct device *dev, void *data),
>> +			 void *data)
>> +{
>> +	struct pm_domain_data *pdd;
>> +	struct gpd_link *link;
>> +	int ret;
>> +
>> +	/* First, iterate over all devices in this domain */
>> +	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
>> +		ret = fn(pdd->dev, data);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	/* Then, recursively iterate over all child domains (subdomains) */
>> +	list_for_each_entry(link, &genpd->parent_links, parent_node) {
>> +		struct generic_pm_domain *child_pd = link->child;
>> +
>> +		ret = genpd_for_each_child(child_pd, fn, data);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>
> This is usefull, I like it.

Thanks for the review & feedback.

>> +
>>  #ifdef CONFIG_DEBUG_FS
>>  static struct dentry *genpd_debugfs_dir;
>>  
>> diff --git a/drivers/pmdomain/core.h b/drivers/pmdomain/core.h
>> new file mode 100644
>> index 000000000000..7061891d31fb
>> --- /dev/null
>> +++ b/drivers/pmdomain/core.h
>> @@ -0,0 +1,17 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Internal header for PM domain core
>> + *
>> + * Copyright (C) 2011 Rafael J. Wysocki <rjw@xxxxxxx>, Renesas Electronics Corp.
>> + */
>
> Year and author are wrong.

Oops, copy/paste.

> With this addressed:
>
> Reviewed-by: Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx>

Thanks,

Kevin