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

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

[Frank Zhang]

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>
---
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;

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

[Ulf Hansson]

+ 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.

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.

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/

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

[Sebastian Reichel]

[-- 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 --]

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

[Ulf Hansson]

On Mon, 12 Jan 2026 at 02:11, Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> 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:

Fair enough!

I agree with the above, while perhaps the auxiliary bus is probably a
better choice instead of the fauxbus for this case.

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

Applied for fixes, thanks!

Kind regards
Uffe



>
> 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/
> >