Add of_genpd_add_subdomain_map() helper function to support
hierarchical PM domains defined by using power-domains-map
property (c.f. nexus node maps in DT spec, section 2.5.1).
This enables PM domain providers with #power-domain-cells > 0 to
establish subdomain relationships via the power-domain-map property,
which was not previously possible.
This new helper function
- uses an OF helper to iterate to over entries in power-domain-map
- For each mapped entry: extracts child specifier, resolves parent phandle,
extracts parent specifier args, and establishes subdomain relationship
- Uses genpd_add_subdomain() with proper gpd_list_lock mutex protection
Example from k3-am62l.dtsi:
scmi_pds: protocol@11 {
#power-domain-cells = <1>;
power-domain-map = <15 &MAIN_PD>, /* TIMER0 */
<19 &WKUP_PD>; /* WKUP_TIMER0 */
};
MAIN_PD: power-controller-main {
#power-domain-cells = <0>;
};
WKUP_PD: power-controller-main {
#power-domain-cells = <0>;
};
This allows SCMI power domain 15 to become a subdomain of MAIN_PD, and
domain 19 to become a subdomain of WKUP_PD.
Signed-off-by: Kevin Hilman (TI.com) <khilman@baylibre.com>
---
drivers/pmdomain/core.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/pm_domain.h | 9 +++++++
2 files changed, 73 insertions(+)
diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 61c2277c9ce3..592e9126896c 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -3483,6 +3483,70 @@ int of_genpd_parse_idle_states(struct device_node *dn,
}
EXPORT_SYMBOL_GPL(of_genpd_parse_idle_states);
+int of_genpd_add_subdomain_map(struct device_node *np,
+ struct genpd_onecell_data *data)
+{
+ struct generic_pm_domain *genpd, *parent_genpd;
+ struct of_phandle_args child_args, parent_args;
+ int index = 0;
+ int ret = 0;
+ u32 child_index;
+
+ if (!np || !data)
+ return -EINVAL;
+
+ /* Iterate through power-domain-map entries using the OF helper */
+ while (!of_parse_map_iter(np, "power-domain", &index,
+ &child_args, &parent_args)) {
+ /* Extract the child domain index from the child specifier */
+ if (child_args.args_count < 1) {
+ of_node_put(parent_args.np);
+ ret = -EINVAL;
+ break;
+ }
+ child_index = child_args.args[0];
+
+ /* Validate child domain index */
+ if (child_index >= data->num_domains) {
+ of_node_put(parent_args.np);
+ continue;
+ }
+
+ genpd = data->domains[child_index];
+ if (!genpd) {
+ of_node_put(parent_args.np);
+ continue;
+ }
+
+ /* Get parent power domain from provider and establish subdomain relationship */
+ mutex_lock(&gpd_list_lock);
+
+ parent_genpd = genpd_get_from_provider(&parent_args);
+ if (IS_ERR(parent_genpd)) {
+ mutex_unlock(&gpd_list_lock);
+ of_node_put(parent_args.np);
+ ret = PTR_ERR(parent_genpd);
+ dev_err(&genpd->dev, "failed to get parent domain: %d\n", ret);
+ break;
+ }
+
+ ret = genpd_add_subdomain(parent_genpd, genpd);
+ mutex_unlock(&gpd_list_lock);
+ of_node_put(parent_args.np);
+
+ if (ret) {
+ dev_err(&genpd->dev, "failed to add as subdomain of %s: %d\n",
+ parent_genpd->name, ret);
+ break;
+ }
+
+ dev_info(&genpd->dev, "added as subdomain of %s\n",
+ parent_genpd->name);
+ }
+
+ return ret;
+}
+
/**
* of_genpd_sync_state() - A common sync_state function for genpd providers
* @np: The device node the genpd provider is associated with.
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index f67a2cb7d781..3585acb89b83 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -462,6 +462,8 @@ int of_genpd_add_subdomain(const struct of_phandle_args *parent_spec,
int of_genpd_remove_subdomain(const struct of_phandle_args *parent_spec,
const struct of_phandle_args *subdomain_spec);
struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
+int of_genpd_add_subdomain_map(struct device_node *np,
+ struct genpd_onecell_data *data);
int of_genpd_parse_idle_states(struct device_node *dn,
struct genpd_power_state **states, int *n);
void of_genpd_sync_state(struct device_node *np);
@@ -504,6 +506,13 @@ static inline int of_genpd_remove_subdomain(const struct of_phandle_args *parent
return -ENODEV;
}
+static int of_genpd_add_subdomain_map(struct device_node *np,
+ struct generic_pm_domain *genpd,
+ int index)
+{
+ return -ENODEV;
+}
+
static inline int of_genpd_parse_idle_states(struct device_node *dn,
struct genpd_power_state **states, int *n)
{
--
2.51.0On Thu, 20 Nov 2025 at 01:58, Kevin Hilman (TI.com)
<khilman@baylibre.com> wrote:
>
> Add of_genpd_add_subdomain_map() helper function to support
> hierarchical PM domains defined by using power-domains-map
> property (c.f. nexus node maps in DT spec, section 2.5.1).
>
> This enables PM domain providers with #power-domain-cells > 0 to
> establish subdomain relationships via the power-domain-map property,
> which was not previously possible.
>
> This new helper function
> - uses an OF helper to iterate to over entries in power-domain-map
> - For each mapped entry: extracts child specifier, resolves parent phandle,
> extracts parent specifier args, and establishes subdomain relationship
> - Uses genpd_add_subdomain() with proper gpd_list_lock mutex protection
>
> Example from k3-am62l.dtsi:
>
> scmi_pds: protocol@11 {
> #power-domain-cells = <1>;
> power-domain-map = <15 &MAIN_PD>, /* TIMER0 */
> <19 &WKUP_PD>; /* WKUP_TIMER0 */
> };
>
> MAIN_PD: power-controller-main {
> #power-domain-cells = <0>;
> };
>
> WKUP_PD: power-controller-main {
> #power-domain-cells = <0>;
> };
>
> This allows SCMI power domain 15 to become a subdomain of MAIN_PD, and
> domain 19 to become a subdomain of WKUP_PD.
Nitpick:
As long as possible, please use the terminology "parent-domain" and
"child-domain" and avoid "subdomain". There are a couple of cases of
this, in the code too, can you please update all of them?
>
> Signed-off-by: Kevin Hilman (TI.com) <khilman@baylibre.com>
> ---
> drivers/pmdomain/core.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/pm_domain.h | 9 +++++++
> 2 files changed, 73 insertions(+)
>
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index 61c2277c9ce3..592e9126896c 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -3483,6 +3483,70 @@ int of_genpd_parse_idle_states(struct device_node *dn,
> }
> EXPORT_SYMBOL_GPL(of_genpd_parse_idle_states);
>
We need to add some description of the function here.
> +int of_genpd_add_subdomain_map(struct device_node *np,
Nitpick:
Hmm, either we should keep consistency with the name
"of_genpd_add_subdomain", according to what you propose - or we should
take the opportunity to move to use "child" in the name instead
(of_genpd_add_child_domain_map()).
Sooner or later it would be nice if we could rename
of_genpd_add_subdomain() (and friends) to of_genpd_add_child_domain().
No big deal at this point, I am fine with whatever name you decide to use.
> + struct genpd_onecell_data *data)
> +{
> + struct generic_pm_domain *genpd, *parent_genpd;
Maybe use "child" and "parent" as variable names instead. This should
make the code a bit more clear.
> + struct of_phandle_args child_args, parent_args;
> + int index = 0;
> + int ret = 0;
> + u32 child_index;
> +
> + if (!np || !data)
> + return -EINVAL;
> +
> + /* Iterate through power-domain-map entries using the OF helper */
> + while (!of_parse_map_iter(np, "power-domain", &index,
> + &child_args, &parent_args)) {
> + /* Extract the child domain index from the child specifier */
> + if (child_args.args_count < 1) {
This should be exactly 1, right?
> + of_node_put(parent_args.np);
> + ret = -EINVAL;
> + break;
If we fail here, we should remove child domains that we added for the
earlier indexes in the while loop, rather than just bailing out.
This applies to other error paths below too.
> + }
> + child_index = child_args.args[0];
> +
> + /* Validate child domain index */
> + if (child_index >= data->num_domains) {
> + of_node_put(parent_args.np);
> + continue;
I don't think we should just continue here, but instead treat this as an error.
> + }
> +
> + genpd = data->domains[child_index];
> + if (!genpd) {
> + of_node_put(parent_args.np);
> + continue;
Ditto.
> + }
> +
> + /* Get parent power domain from provider and establish subdomain relationship */
> + mutex_lock(&gpd_list_lock);
> +
> + parent_genpd = genpd_get_from_provider(&parent_args);
> + if (IS_ERR(parent_genpd)) {
> + mutex_unlock(&gpd_list_lock);
> + of_node_put(parent_args.np);
> + ret = PTR_ERR(parent_genpd);
> + dev_err(&genpd->dev, "failed to get parent domain: %d\n", ret);
Perhaps clarify the print by changing the text to state that we can't
find the parent's OF provider. If the print is needed at all.
> + break;
> + }
> +
> + ret = genpd_add_subdomain(parent_genpd, genpd);
> + mutex_unlock(&gpd_list_lock);
> + of_node_put(parent_args.np);
> +
> + if (ret) {
> + dev_err(&genpd->dev, "failed to add as subdomain of %s: %d\n",
> + parent_genpd->name, ret);
> + break;
> + }
> +
> + dev_info(&genpd->dev, "added as subdomain of %s\n",
> + parent_genpd->name);
> + }
> +
> + return ret;
> +}
Except for taking better care in the error path, it also looks like we
are missing a corresponding function to remove the child-domains that
was added with the above new function.
Perhaps that function can be used in the error paths too?
> +
> /**
> * of_genpd_sync_state() - A common sync_state function for genpd providers
> * @np: The device node the genpd provider is associated with.
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index f67a2cb7d781..3585acb89b83 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -462,6 +462,8 @@ int of_genpd_add_subdomain(const struct of_phandle_args *parent_spec,
> int of_genpd_remove_subdomain(const struct of_phandle_args *parent_spec,
> const struct of_phandle_args *subdomain_spec);
> struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
> +int of_genpd_add_subdomain_map(struct device_node *np,
> + struct genpd_onecell_data *data);
> int of_genpd_parse_idle_states(struct device_node *dn,
> struct genpd_power_state **states, int *n);
> void of_genpd_sync_state(struct device_node *np);
> @@ -504,6 +506,13 @@ static inline int of_genpd_remove_subdomain(const struct of_phandle_args *parent
> return -ENODEV;
> }
>
> +static int of_genpd_add_subdomain_map(struct device_node *np,
> + struct generic_pm_domain *genpd,
> + int index)
> +{
> + return -ENODEV;
> +}
> +
> static inline int of_genpd_parse_idle_states(struct device_node *dn,
> struct genpd_power_state **states, int *n)
> {
>
> --
> 2.51.0
>
Kind regards
Uffe
Ulf Hansson <ulf.hansson@linaro.org> writes:
> On Thu, 20 Nov 2025 at 01:58, Kevin Hilman (TI.com)
> <khilman@baylibre.com> wrote:
>>
>> Add of_genpd_add_subdomain_map() helper function to support
>> hierarchical PM domains defined by using power-domains-map
>> property (c.f. nexus node maps in DT spec, section 2.5.1).
>>
>> This enables PM domain providers with #power-domain-cells > 0 to
>> establish subdomain relationships via the power-domain-map property,
>> which was not previously possible.
>>
>> This new helper function
>> - uses an OF helper to iterate to over entries in power-domain-map
>> - For each mapped entry: extracts child specifier, resolves parent phandle,
>> extracts parent specifier args, and establishes subdomain relationship
>> - Uses genpd_add_subdomain() with proper gpd_list_lock mutex protection
>>
>> Example from k3-am62l.dtsi:
>>
>> scmi_pds: protocol@11 {
>> #power-domain-cells = <1>;
>> power-domain-map = <15 &MAIN_PD>, /* TIMER0 */
>> <19 &WKUP_PD>; /* WKUP_TIMER0 */
>> };
>>
>> MAIN_PD: power-controller-main {
>> #power-domain-cells = <0>;
>> };
>>
>> WKUP_PD: power-controller-main {
>> #power-domain-cells = <0>;
>> };
>>
>> This allows SCMI power domain 15 to become a subdomain of MAIN_PD, and
>> domain 19 to become a subdomain of WKUP_PD.
>
> Nitpick:
> As long as possible, please use the terminology "parent-domain" and
> "child-domain" and avoid "subdomain". There are a couple of cases of
> this, in the code too, can you please update all of them?
OK.
>>
>> Signed-off-by: Kevin Hilman (TI.com) <khilman@baylibre.com>
>> ---
>> drivers/pmdomain/core.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/pm_domain.h | 9 +++++++
>> 2 files changed, 73 insertions(+)
>>
>> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
>> index 61c2277c9ce3..592e9126896c 100644
>> --- a/drivers/pmdomain/core.c
>> +++ b/drivers/pmdomain/core.c
>> @@ -3483,6 +3483,70 @@ int of_genpd_parse_idle_states(struct device_node *dn,
>> }
>> EXPORT_SYMBOL_GPL(of_genpd_parse_idle_states);
>>
>
> We need to add some description of the function here.
OK.
>> +int of_genpd_add_subdomain_map(struct device_node *np,
>
> Nitpick:
> Hmm, either we should keep consistency with the name
> "of_genpd_add_subdomain", according to what you propose - or we should
> take the opportunity to move to use "child" in the name instead
> (of_genpd_add_child_domain_map()).
>
> Sooner or later it would be nice if we could rename
> of_genpd_add_subdomain() (and friends) to of_genpd_add_child_domain().
>
> No big deal at this point, I am fine with whatever name you decide to use.
I will update the changelogs/comments/descriptions etc. to use
parent/child, but I will leave subdomain in the function name since
that's what all the other APIs use. Then in a later cleanup step, we
could rename the subdomain APIs to child APIs.
>> + struct genpd_onecell_data *data)
>> +{
>> + struct generic_pm_domain *genpd, *parent_genpd;
>
> Maybe use "child" and "parent" as variable names instead. This should
> make the code a bit more clear.
OK.
>> + struct of_phandle_args child_args, parent_args;
>> + int index = 0;
>> + int ret = 0;
>> + u32 child_index;
>> +
>> + if (!np || !data)
>> + return -EINVAL;
>> +
>> + /* Iterate through power-domain-map entries using the OF helper */
>> + while (!of_parse_map_iter(np, "power-domain", &index,
>> + &child_args, &parent_args)) {
>> + /* Extract the child domain index from the child specifier */
>> + if (child_args.args_count < 1) {
>
> This should be exactly 1, right?
Hmm, I'm not sure exactly what you mean. Are you suggesting this check
should be "!= 1" instead of "< 1"?
I think args_count should match #power-domain-cells. So for SCMI, this
should indeed be 1. But if this function is used for other domains
where #power-domain-cells is > 1, then the current check for "< 1" is
correct.
>> + of_node_put(parent_args.np);
>> + ret = -EINVAL;
>> + break;
>
> If we fail here, we should remove child domains that we added for the
> earlier indexes in the while loop, rather than just bailing out.
>
> This applies to other error paths below too.
Yeah, the current error handling isn't really in place (hence the RFC)
but I will add it for the next version.
I'm planning to take the approach that all children in the map have to
be successfully added in order for this function to be considered
successful. If any of the children fail to get added (for any reason),
then they all should be removed.
This remove function will look *very* similar to the add function
because it will have to parse the map (again), finding parent and child
info and attempting to remove each child from the parent.
At first, this seems pretty inefficient, but I think it's better than
the add function being required to keep track of the state of which
domains were successfully added. Which gets even more complicated if
there are multiple domains which use power-domain-map.
So fora now, I plan to avoid tracking all that state, and have the
remove function be a simple reversal of the add, but looping through the
whole map, even if it fails to remove some items (because they may not
have been added in the first place.)
>> + }
>> + child_index = child_args.args[0];
>> +
>> + /* Validate child domain index */
>> + if (child_index >= data->num_domains) {
>> + of_node_put(parent_args.np);
>> + continue;
>
> I don't think we should just continue here, but instead treat this as an error.
Yes.
>> + }
>> +
>> + genpd = data->domains[child_index];
>> + if (!genpd) {
>> + of_node_put(parent_args.np);
>> + continue;
>
> Ditto.
Yes.
>> + }
>> +
>> + /* Get parent power domain from provider and establish subdomain relationship */
>> + mutex_lock(&gpd_list_lock);
>> +
>> + parent_genpd = genpd_get_from_provider(&parent_args);
>> + if (IS_ERR(parent_genpd)) {
>> + mutex_unlock(&gpd_list_lock);
>> + of_node_put(parent_args.np);
>> + ret = PTR_ERR(parent_genpd);
>> + dev_err(&genpd->dev, "failed to get parent domain: %d\n", ret);
>
> Perhaps clarify the print by changing the text to state that we can't
> find the parent's OF provider. If the print is needed at all.
Print probably isn't needed at all, but just useful for development
debug purposes.
>> + break;
>> + }
>> +
>> + ret = genpd_add_subdomain(parent_genpd, genpd);
>> + mutex_unlock(&gpd_list_lock);
>> + of_node_put(parent_args.np);
>> +
>> + if (ret) {
>> + dev_err(&genpd->dev, "failed to add as subdomain of %s: %d\n",
>> + parent_genpd->name, ret);
>> + break;
>> + }
>> +
>> + dev_info(&genpd->dev, "added as subdomain of %s\n",
>> + parent_genpd->name);
>> + }
>> +
>> + return ret;
>> +}
>
> Except for taking better care in the error path, it also looks like we
> are missing a corresponding function to remove the child-domains that
> was added with the above new function.
>
> Perhaps that function can be used in the error paths too?
Yeah, I as describe above. I will add that for the next version.
Kevin
© 2016 - 2026 Red Hat, Inc.