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

[Sebastian Reichel <sebastian.reichel@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 3448 bytes --]

Hi,

On Tue, Dec 30, 2025 at 03:07:37PM +0100, Ulf Hansson wrote:
> + Nicolas
> 
> On Tue, 16 Dec 2025 at 06:53, Frank Zhang <rmxpzlb@gmail.com> 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 set pmdomain to off if it need regulator during probe,
> > consumer device can power on pmdomain after regulator ready.
> >
> > Signed-off-by: Frank Zhang <rmxpzlb@gmail.com>
> > Tested-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > Tested-by: Quentin Schulz <quentin.schulz@cherry.de>
> 
> The problem with the child-domain using a regulator has been discussed
> before [1] between Nicolas, Heiko and me. That said, I have looped in
> Nicolas to allow him to share his opinion about this too.
> fauxbus
> My view on is that I would prefer that we try to address/fix the root
> cause, rather than trying to paper over the problem as what seems to
> be suggested in the $subject patch. Or at least I need Nicolas/Heiko
> to confirm that they are fine with the $subject patch, before I pick
> it up.

FWIW my thoughts on this:

I believe the proper solution would be to acquire the regulator at
probe time how it is usually being done in other drivers. I think
this requires restructuring the driver, so that the sub-domains are
registered as sub-devices (e.g. via fauxbus) to avoid the
chicken-and-egg problem of the regulator for pmdomain1 needing
pmdomain2. As this modification is most likely too big to be
backported I think this patch should be merged for now:

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Cc: stable@vger.kernel.org
Fixes: db6df2e3fc16 ("pmdomain: rockchip: add regulator support")

Greetings,

-- Sebastian

> 
> Kind regards
> Uffe
> 
> > ---
> > Changes in v2:
> > - Simplified the regulator check logic, trun off pmdomain if need
> >   regulator.
> > ---
> >  drivers/pmdomain/rockchip/pm-domains.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> > index 4f1336a0f49a..997e93c12951 100644
> > --- a/drivers/pmdomain/rockchip/pm-domains.c
> > +++ b/drivers/pmdomain/rockchip/pm-domains.c
> > @@ -879,6 +879,16 @@ 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);
> > +
> > +       /*
> > +        * 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);
> > +
> >         pd->genpd.power_off = rockchip_pd_power_off;
> >         pd->genpd.power_on = rockchip_pd_power_on;
> >         pd->genpd.attach_dev = rockchip_pd_attach_dev;
> 
> [1]
> https://lore.kernel.org/all/CAPDyKFr=GwJ+cO3cW4Ed_LsS=q_JtuuQPDweDpLgDO4hBLFXUA@mail.gmail.com/
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]