[PATCH v5 3/4] mach-snapdragon: fixup power-domains

[Neil Armstrong]

On 11/3/25 13:31, Casey Connolly wrote:
> 
> 
> On 10/31/25 11:12, Aswin Murugan wrote:
>>
>> On 10/28/2025 6:32 PM, neil.armstrong at linaro.org wrote:
>>> On 10/27/25 12:33, Aswin Murugan wrote:
>>>> From: Aswin Murugan <aswin.murugan at oss.qualcomm.com>
>>>>
>>>> RPMH power domain properties were initially removed from the
>>>> device tree due to the absence of driver support. Since RPMH
>>>> power domain driver is added now, the fixups are removed.
>>>>
>>>> Signed-off-by: Aswin Murugan <aswin.murugan at oss.qualcomm.com>
>>>> ---
>>>> v5:
>>>> - No changes to this patch in v5
>>>>
>>>> v4:
>>>> - Reverted the inclusion of qcom-rpmpd.h header in pervious versions.
>>>>
>>>> v3:
>>>> - Removed OF_LIVE_SA8775P config, since there is no SA8775P specific fixups are done
>>>>
>>>> v2:
>>>> - Removed the power domain fixups, since the power domains are handled in qcom-rpmhpd driver
>>>> ---
>>>>   arch/arm/mach-snapdragon/of_fixup.c | 31 -----------------------------
>>>>   1 file changed, 31 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-snapdragon/of_fixup.c b/arch/arm/mach- snapdragon/of_fixup.c
>>>> index eec2c0c757e..93c16821e54 100644
>>>> --- a/arch/arm/mach-snapdragon/of_fixup.c
>>>> +++ b/arch/arm/mach-snapdragon/of_fixup.c
>>>> @@ -116,36 +116,6 @@ static void fixup_usb_nodes(struct device_node *root)
>>>>       }
>>>>   }
>>>>   -/* Remove all references to the rpmhpd device */
>>>> -static void fixup_power_domains(struct device_node *root)
>>>> -{
>>>> -    struct device_node *pd = NULL, *np = NULL;
>>>> -    struct property *prop;
>>>> -    const __be32 *val;
>>>> -
>>>> -    /* All Qualcomm platforms name the rpm(h)pd "power-controller" */
>>>> -    for_each_of_allnodes_from(root, pd) {
>>>> -        if (pd->name && !strcmp("power-controller", pd->name))
>>>> -            break;
>>>> -    }
>>>> -
>>>> -    /* Sanity check that this is indeed a power domain controller */
>>>> -    if (!of_find_property(pd, "#power-domain-cells", NULL)) {
>>>> -        log_err("Found power-controller but it doesn't have #power- domain-cells\n");
>>>> -        return;
>>>> -    }
>>>> -
>>>> -    /* Remove all references to the power domain controller */
>>>> -    for_each_of_allnodes_from(root, np) {
>>>> -        if (!(prop = of_find_property(np, "power-domains", NULL)))
>>>> -            continue;
>>>> -
>>>> -        val = prop->value;
>>>> -        if (val[0] == cpu_to_fdt32(pd->phandle))
>>>> -            of_remove_property(np, prop);
>>>> -    }
>>>> -}
>>>> -
>>>>   #define time_call(func, ...) \
>>>>       do { \
>>>>           u64 start = timer_get_us(); \
>>>> @@ -158,7 +128,6 @@ static int qcom_of_fixup_nodes(void * __maybe_unused ctx, struct event *event)
>>>>       struct device_node *root = event->data.of_live_built.root;
>>>>         time_call(fixup_usb_nodes, root);
>>>> -    time_call(fixup_power_domains, root);
>>>
>>> I think the fixup should be removed only if QCOM_POWER_DOMAIN is enabled, no ?
>>>
>>> Neil
> 
> Hi Aswin,
> 
>>>
>>> In line with the previous review comment in thread Re: [PATCH v1 0/4] Added QCOM RPMH Power Domain Driver - Casey Connolly <https:// lore.kernel.org/u-boot/5afbdafa-a0f9-4146- b64e-8869f68bc7f8 at linaro.org/#t>, which recommended the complete removal of the fixup code, we have removed it accordingly, also the QCOM_POWER_DOMAIN config is enabled by default in qcom_defconfig
>>> Please let us know if you would like any further changes or adjustments.
> 
> Yes ideally the fixup code would be removed, but we support platforms like msm8916 (apq8064) which use rpm (no H) which are not supported by your series.
> 
> So I think the fixup code needs to stay, and we likely need some kind of runtime check (best I can think of right now is checking if the compatible string ends in "rpmpd" or "pmhpd" and only doing the fixups for non-rpmh platforms).

I think the proper fix would be to just bail out when no power-domain device is found in dev_power_domain_ctrl(),

=======================><=============================
diff --git a/drivers/power/domain/power-domain-uclass.c b/drivers/power/domain/power-domain-uclass.c
index d9fa8ad4bd2..61f693c41bc 100644
--- a/drivers/power/domain/power-domain-uclass.c
+++ b/drivers/power/domain/power-domain-uclass.c
@@ -184,6 +184,8 @@ static int dev_power_domain_ctrl(struct udevice *dev, bool on)
                                             "#power-domain-cells", 0);
         for (i = 0; i < count; i++) {
                 ret = power_domain_get_by_index(dev, &pd, i);
+               if (ret == -ENODEV)
+                       return 0;
                 if (ret)
                         return ret;
                 if (on)
========================><================================

Probably a warning could help aswell for debugging.

Neil



> 
> Sorry if this wasn't communicated very clearly, hopefully it makes sense now(?).
> 
> Ideally we'd be able to have ARCH_SNAPDRAGON select the rpmh pd driver, but since it isn't needed on some platforms it doesn't make much sense to do that I guess.
> 
> Kind regards,
> Casey (she/they)
> 
>>>
>>> Thanks,
>>> Aswin
>>>
>>>>         return 0;
>>>>   }
>>>
>