linux-kernel - Re: [PATCH RFC v3 2/2] pmdomain: core: add support for subdomains using power-domain-map

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPDyKFrO9rb0eDb2qO+EGaVjOFG=7emgca8511XACDhWY=dt5g@mail.gmail.com>
Date: Mon, 16 Jun 2025 15:48:08 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Kevin Hilman <khilman@...libre.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, devicetree@...r.kernel.org, 
	linux-pm@...r.kernel.org, arm-scmi@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC v3 2/2] pmdomain: core: add support for subdomains
 using power-domain-map

On Sat, 14 Jun 2025 at 00:39, Kevin Hilman <khilman@...libre.com> wrote:
>
> Currently, PM domains can only support hierarchy for simple
> providers (e.g. ones with #power-domain-cells = 0).
>
> Add more generic support for hierarchy by using nexus node
> maps (c.f. section 2.5.1 of the DT spec.)
>
> For example, we could describe SCMI PM domains with multiple parents
> domains (MAIN_PD and WKUP_PD) like this:
>
>     scmi_pds: protocol@11 {
>         reg = <0x11>;
>         #power-domain-cells = <1>;
>
>         power-domain-map = <15 &MAIN_PD>,
>                            <19 &WKUP_PD>;
>     };
>
> which should mean that <&scmi_pds 15> is a subdomain of MAIN_PD and
> <&scmi_pds 19> is a subdomain of WKUP_PD.
>
> IOW, given an SCMI device which uses SCMI PM domains:
>
>    main_timer0: timer@...0000 {
>       power-domains = <&scmi_pds 15>;
>    };
>
> it already implies that main_timer0 is PM domain <&scmi_pds 15>
>
> With the new map, this *also* now implies <&scmi_pds 15> is a
> subdomain of MAIN_PD.
>
> Signed-off-by: Kevin Hilman <khilman@...libre.com>
> ---
>  drivers/pmdomain/core.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index d6c1ddb807b2..adf022b45d95 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -2998,8 +2998,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
>                                  unsigned int index, unsigned int num_domains,
>                                  bool power_on)
>  {
> -       struct of_phandle_args pd_args;
> -       struct generic_pm_domain *pd;
> +       struct of_phandle_args pd_args, parent_args;
> +       struct generic_pm_domain *pd, *parent_pd = NULL;
>         int ret;
>
>         ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
> @@ -3039,6 +3039,22 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
>                         goto err;
>         }
>
> +       /*
> +        * Check for power-domain-map, which implies the primary
> +        * power-doamin is a subdomain of the parent found in the map.
> +        */
> +       ret = of_parse_phandle_with_args_map(dev->of_node, "power-domains",
> +                                            "power-domain", index, &parent_args);
> +       if (!ret && (pd_args.np != parent_args.np)) {
> +               parent_pd = genpd_get_from_provider(&parent_args);
> +               of_node_put(parent_args.np);
> +
> +               ret = pm_genpd_add_subdomain(parent_pd, pd);
> +               if (!ret)
> +                       dev_dbg(dev, "adding PM domain %s as subdomain of %s\n",
> +                               pd->name, parent_pd->name);
> +       }

Please move the above new code to a separate shared genpd helper
function, that genpd providers can call build the topology. This, to
be consistent with the current way for how we usually add
parent/child-domains in genpd (see of_genpd_add_subdomain).

Moreover, we also need a corresponding "cleanup" helper function to
remove the child-domain (subdomain) correctly, similar to
of_genpd_remove_subdomain().

> +
>         ret = genpd_set_required_opp(dev, index);
>         if (ret)
>                 goto err;
> @@ -3056,6 +3072,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
>                         dev_gpd_data(dev)->default_pstate = 0;
>                 }
>
> +               if (parent_pd)
> +                       pm_genpd_remove_subdomain(parent_pd, pd);
>                 genpd_remove_device(pd, dev);
>                 return -EPROBE_DEFER;
>         }
> @@ -3063,6 +3081,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
>         return 1;
>
>  err:
> +       if (parent_pd)
> +               pm_genpd_remove_subdomain(parent_pd, pd);
>         genpd_remove_device(pd, dev);
>         return ret;
>  }
>
> --
> 2.49.0
>

Kind regards
Uffe