Re: [PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready

[rmxpzlb <rmxpzlb@gmail.com>]

>Hi,
>> On Fri, Dec 05, 2025 at 02:47:39PM +0800, Frank Zhang wrote:
>> RK3588_PD_NPU initialize as GENPD_STATE_ON before regulator ready.
>> rknn_iommu initlized success and suspend RK3588_PD_NPU. When rocket
>> driver register, it will resume rknn_iommu.
>> 
>> If regulator is still not ready at this point, rknn_iommu resume fail,
>> pm runtime status will be error: -EPROBE_DEFER.
>> 
>> This patch check regulator when pmdomain init, if regulator is not ready
>> or not enabled, power off pmdomain. Consumer device can power on it's
>> pmdomain after regulator ready
>> 
>> Signed-off-by: Frank Zhang <rmxpzlb@gmail.com>
>> ---
>>  drivers/pmdomain/rockchip/pm-domains.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>> 
>> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
>> index 1955c6d453e4..bc69f5d840e6 100644
>> --- a/drivers/pmdomain/rockchip/pm-domains.c
>> +++ b/drivers/pmdomain/rockchip/pm-domains.c
>> @@ -659,6 +659,11 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
>>  	return ret;
>>  }
>>  
>> +static bool rockchip_pd_regulator_is_enabled(struct rockchip_pm_domain *pd)
>> +{
>> +	return IS_ERR_OR_NULL(pd->supply) ? false : regulator_is_enabled(pd->supply);
>> +}
>> +
>>  static int rockchip_pd_regulator_disable(struct rockchip_pm_domain *pd)
>>  {
>>  	return IS_ERR_OR_NULL(pd->supply) ? 0 : regulator_disable(pd->supply);
>> @@ -861,6 +866,15 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>>  		pd->genpd.name = pd->info->name;
>>  	else
>>  		pd->genpd.name = kbasename(node->full_name);
>> +
>> +	if (pd->info->need_regulator) {
>> +		if (IS_ERR_OR_NULL(pd->supply))
>> +			pd->supply = devm_of_regulator_get(pmu->dev, pd->node, "domain");
>> +
>> +		if (!rockchip_pd_regulator_is_enabled(pd))
>> +			rockchip_pd_power(pd, false);
>> +	}
>
>It is extremly unlikely, that you will be able to get the regulator
>at driver probe time. The typical regulator for NPU or GPU is
>connected via SPI or I2C, which will only be available after the
>power domain driver has been probed. So I suppose this could be
>simplified as:
>
>--------------------------------------------
>/*
> * power domain's needing a regulator should default to off, since
> * the regulator state is unknown at probe time. Also the regulator
> * state cannot be checked, since that usually requires IP needing
> * (a different) power domain.
> */
>if (pd->info->need_regulator)
>    rockchip_pd_power(pd, false);
>--------------------------------------------
>
>I think the proper fix would be to add support for registering the
>regulator needing power-domain's delayed and then enforce requesting
>the regulator at probe time. That's not trivial to implement, though.
>
>Greetings,
>
>-- Sebastian
>
>> +
>>  	pd->genpd.power_off = rockchip_pd_power_off;
>>  	pd->genpd.power_on = rockchip_pd_power_on;
>>  	pd->genpd.attach_dev = rockchip_pd_attach_dev;

Thanks for your comment. This simplification is OK for me.
I will test and send new patch.

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip