Re: [PATCH 3/4] soc: rockchip: power-domain: Add regulator support

[Robin Murphy <robin.murphy@arm.com>]

On 2021-12-20 09:44, Sascha Hauer wrote:
> On Fri, Dec 17, 2021 at 02:09:18PM +0100, Sascha Hauer wrote:
>> This patch allows to let a domain be supplied by a regulator which
>> is needed for the GPU on the rk3568-EVB board.
>>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> ---
>>   drivers/soc/rockchip/pm_domains.c | 24 +++++++++++++++++++++++-
>>   1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> +
>> +	regulator_disable(pd->regulator);
>> +
>> +	return 0;
>>   }
>>   
>>   static int rockchip_pd_attach_dev(struct generic_pm_domain *genpd,
>> @@ -500,6 +517,11 @@ static int rockchip_domain_probe(struct platform_device *pdev)
>>   	pd->info = pd_info;
>>   	pd->pmu = pd_info->pmu;
>>   
>> +	pd->regulator = devm_regulator_get(&pdev->dev, "power");
> 
> I was told that I should use this function instead of
> devm_regulator_get_optional() when the regulator is not optional and
> also I can drop the if (pd->regulator) dance when enabling the regulator
> because I get a dummy regulator here when using devm_regulator_get().
> 
> Well, all true and on one specific board the regulator is indeed not
> optional. However, on all other power domains that don't need a
> regulator and all other boards and all other SoCs this driver is used we
> now get:
> 
> [    0.185588] rk-power-domain rk-power-domain.8: supply power not found, using dummy regulator
> [    0.186036] rk-power-domain rk-power-domain.9: supply power not found, using dummy regulator
> [    0.186459] rk-power-domain rk-power-domain.10: supply power not found, using dummy regulator
> [    0.187039] rk-power-domain rk-power-domain.11: supply power not found, using dummy regulator
> [    0.187333] rk-power-domain rk-power-domain.13: supply power not found, using dummy regulator
> [    0.187644] rk-power-domain rk-power-domain.14: supply power not found, using dummy regulator
> [    0.188042] rk-power-domain rk-power-domain.15: supply power not found, using dummy regulator
> 
> I wonder if devm_regulator_get() is really the right function here. Or
> should the message be dropped?

Since the power domains are hierarchical and none of them really 
represent the physical owner of a supply, I'm not sure there's really a 
correct answer to the regulator question either way in this context. 
FWIW I reckon it would make sense to model things properly and teach the 
driver about the voltage domains that actually own the input supplies 
(maybe with a binding more like I/O domains where we just have 
explicitly-named supply properties for each one on the power 
controller?) - it's a little more work up-front, but seems like it 
should be relatively straightforward to fit into the genpd hierarchy, 
and be more robust in the long term.

Robin.

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