On Thu, 12 Mar 2026 at 13:46, AngeloGioacchino Del Regno
[off-list ref] wrote:

Il 11/03/26 18:29, Ulf Hansson ha scritto:

quoted

On Fri, 6 Mar 2026 at 10:50, AngeloGioacchino Del Regno
[off-list ref] wrote:

quoted

Il 12/02/26 12:34, Ulf Hansson ha scritto:

quoted

On Tue, 10 Feb 2026 at 06:40, Adam Ford [off-list ref] wrote:

quoted

The wrong value of the number of domains is wrong which leads to
failures when trying to enumerate nested power domains.

   PM: genpd_xlate_onecell: invalid domain index 0
   PM: genpd_xlate_onecell: invalid domain index 1
   PM: genpd_xlate_onecell: invalid domain index 3
   PM: genpd_xlate_onecell: invalid domain index 4
   PM: genpd_xlate_onecell: invalid domain index 5
   PM: genpd_xlate_onecell: invalid domain index 13
   PM: genpd_xlate_onecell: invalid domain index 14

Attempts to use these power domains fail, so fix this by
using the correct value of calculated power domains.

Signed-off-by: Adam Ford <redacted>

We should have a fixes tag for this too I think:

Fixes: 88914db077b6 ("pmdomain: mediatek: Add support for Hardware
Voter power domains")

quoted

---
   drivers/pmdomain/mediatek/mtk-pm-domains.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c b/drivers/pmdomain/mediatek/mtk-pm-domains.c
index 58648f4f689b..d2b8d0332951 100644
--- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
+++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c

@@ -1228,7 +1228,7 @@ static int scpsys_probe(struct platform_device *pdev)
          scpsys->soc_data = soc;

          scpsys->pd_data.domains = scpsys->domains;
-       scpsys->pd_data.num_domains = soc->num_domains;
+       scpsys->pd_data.num_domains = num_domains;

Not sure this is the complete fix, as scpsys_add_one_domain() seems to
be using the wrong value of "num_domains" too, no?

No, scpsys_add_one_domain() uses num_domains from the soc_data for DIRECT_CTL,
which is expected. Maybe that can be improved, but it's how it is supposed to work.

So yeah, this patch is resolving an issue, and it is a complete fix.

I had a closer look and unfortunately, it still looks weird to me.

More precisely, we are allocating and registering a number of genpds,
that corresponds to the sum of "soc->num_domains +
soc->num_hwv_domains". The index each genpd gets in the array (struct
genpd_onecell_data) must correspond to the index specified in DT, for
both a consumer-node and a child-domain-node, right?

It seems like adding them dynamically changes the index. In other
words, the index specified using the "reg" property in a
child-domain-node, may not match what a consumer-node should specify
in its "power-domains" property. Isn't that a problem? Perhaps not,
because we never have both "scpsys_domain_data" and
"scpsys_hwv_domain_data", but then why are we adding them in ->probe()
with "soc->num_domains + soc->num_hwv_domains"?

As you understood, having both is not really supported right now, and the
intention there was to have the flexibility to do so without restructuring
the entire thing in the future.

Thanks for clarifying!

Although, having both seems problematic for the reasons I pointed out
above. At least, it looks like some more re-work will be needed to
support that case.

Though, from what I understand, the confusion comes from a single line (and
reading it again, I do agree, because I got confused myself for a moment as
well, and I'm the one who wrote this, so that's bad bad bad):

        num_domains = soc->num_domains + soc->num_hwv_domains;

where, to avoid confusion, this should've been:

        num_domains = soc->num_domains ?
                      soc->num_domains : soc->num_hwv_domains;

Agreed.

(perhaps with a check `if (num_domains && num_hwv_domains) dev_err(only one
  supported)` but being this taken from hardcoded data, I'm not really for it)

Agreed, a print would be superfluous.

...and still, like the proposed fix does:

        scpsys->pd_data.num_domains = num_domains;


While I agree, again, in that the code could be improved (and it shall be),
how the logic behaves, right now, with the proposed fix, still looks good to me.

Okay, I'm queuing the $subject patch as a fix and by adding a stable/fixes-tag.

I can eventually just send a patch that clarifies the `num_domains` if you prefer.

Well, actually, I would suggest splitting the driver into two drivers
(each handling its own type of match data) and adding some shared
helper functions, that both drivers can use.

It's more work, but we should end up with more maintainable code, I think.

What do you think?

[...]

Kind regards
Uffe