Re: [PATCH 3/3] pmdomain: arm_scmi: add support for domain hierarchies

Dhruva Gole
2026-03-13T12:07:13Z

On Mar 10, 2026 at 17:19:25 -0700, Kevin Hilman (TI) wrote:
> After primary SCMI pmdomain is created, use new of_genpd helper which
> checks for child domain mappings defined in power-domains-child-ids.
>
> Also remove any child domain mappings when SCMI domain is removed.
>
> Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
> ---

Again, since it worked fine on my AM62L,
Tested-by: Dhruva Gole <d-gole@ti.com>

But I had some thoughts further down...

> drivers/pmdomain/arm/scmi_pm_domain.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pmdomain/arm/scmi_pm_domain.c b/drivers/pmdomain/arm/scmi_pm_domain.c
> index b5e2ffd5ea64..9d8faef44aa9 100644
> --- a/drivers/pmdomain/arm/scmi_pm_domain.c
> +++ b/drivers/pmdomain/arm/scmi_pm_domain.c
> @@ -114,6 +114,14 @@ static int scmi_pm_domain_probe(struct scmi_device *sdev)
>
> dev_set_drvdata(dev, scmi_pd_data);
>
> + /*
> + * Parse (optional) power-domains-child-ids property to
> + * establish parent-child relationships
> + */
> + ret = of_genpd_add_child_ids(np, scmi_pd_data);
> + if (ret < 0 && ret != -ENOENT)
> + pr_err("Failed to parse power-domains-child-ids for %pOF: %d\n", np, ret);

Nit: I think the style of this driver is to use dev_err than pr_err

Also, maybe a dev_warn makes more sense since we're not even returning
the error or doing anything different if we get certain error path.

I am wondering if it makes sense to just abort the whole idea of
creating power-domain child ids if anything goes wrong?

Basically just of_genpd_remove_child_ids if we face a condition where we
have different number of parents/ children or id > num etc...

All are error cases where the system behaviour can go on to become very
unpredictable if we end up making a false/ incomplete parent-child ID
map.

Thoughts?

PS.
If we go on to do a of_genpd_remove_child_ids here incase of failure then it makes sense to
scream a dev_err here.

> +
> return 0;
> err_rm_genpds:
> for (i = num_domains - 1; i >= 0; i--)
> @@ -129,9 +137,13 @@ static void scmi_pm_domain_remove(struct scmi_device *sdev)
> struct device *dev = &sdev->dev;
> struct device_node *np = dev->of_node;
>
> + scmi_pd_data = dev_get_drvdata(dev);
> +
> + /* Remove any parent-child relationships established at probe time */
> + of_genpd_remove_child_ids(np, scmi_pd_data);
> +
> of_genpd_del_provider(np);
>
> - scmi_pd_data = dev_get_drvdata(dev);
> for (i = 0; i < scmi_pd_data->num_domains; i++) {
> if (!scmi_pd_data->domains[i])
> continue;
>
> --
> 2.51.0
>
>

--
Best regards,
Dhruva Gole
Texas Instruments Incorporated