[PATCH 3/5] pmdomain: renesas: rcar-sysc: Don't keep unused PM domains powered-on

Ulf Hansson posted 5 patches 3 weeks, 2 days ago
[PATCH 3/5] pmdomain: renesas: rcar-sysc: Don't keep unused PM domains powered-on
Posted by Ulf Hansson 3 weeks, 2 days ago
The recent changes to genpd makes a genpd OF provider that is powered-on at
initialization to stay powered-on, until the ->sync_state() callback is
invoked for it.

This may not happen at all, if we wait for a consumer device to be probed,
leading to wasting energy. There are ways to enforce the ->sync_state()
callback to be invoked, through sysfs or via the probe-defer-timeout, but
none of them in its current form are a good fit for rcar-sysc PM domains.

Let's therefore opt-out from this behaviour of genpd for now, by using the
GENPD_FLAG_NO_STAY_ON.

Link: https://lore.kernel.org/all/20250701114733.636510-1-ulf.hansson@linaro.org/
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Fixes: 0e789b491ba0 ("pmdomain: core: Leave powered-on genpds on until sync_state")
Fixes: 13a4b7fb6260 ("pmdomain: core: Leave powered-on genpds on until late_initcall_sync")
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/renesas/rcar-sysc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pmdomain/renesas/rcar-sysc.c b/drivers/pmdomain/renesas/rcar-sysc.c
index 2d4161170c63..d8a8ffcde38d 100644
--- a/drivers/pmdomain/renesas/rcar-sysc.c
+++ b/drivers/pmdomain/renesas/rcar-sysc.c
@@ -241,6 +241,7 @@ static int __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd)
 		}
 	}
 
+	genpd->flags |= GENPD_FLAG_NO_STAY_ON;
 	genpd->power_off = rcar_sysc_pd_power_off;
 	genpd->power_on = rcar_sysc_pd_power_on;
 
-- 
2.43.0
Re: [PATCH 3/5] pmdomain: renesas: rcar-sysc: Don't keep unused PM domains powered-on
Posted by Geert Uytterhoeven 3 weeks ago
Hi Ulf,

On Tue, 9 Sept 2025 at 13:11, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> The recent changes to genpd makes a genpd OF provider that is powered-on at
> initialization to stay powered-on, until the ->sync_state() callback is
> invoked for it.
>
> This may not happen at all, if we wait for a consumer device to be probed,
> leading to wasting energy. There are ways to enforce the ->sync_state()
> callback to be invoked, through sysfs or via the probe-defer-timeout, but
> none of them in its current form are a good fit for rcar-sysc PM domains.
>
> Let's therefore opt-out from this behaviour of genpd for now, by using the
> GENPD_FLAG_NO_STAY_ON.
>
> Link: https://lore.kernel.org/all/20250701114733.636510-1-ulf.hansson@linaro.org/
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Fixes: 0e789b491ba0 ("pmdomain: core: Leave powered-on genpds on until sync_state")
> Fixes: 13a4b7fb6260 ("pmdomain: core: Leave powered-on genpds on until late_initcall_sync")
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

> --- a/drivers/pmdomain/renesas/rcar-sysc.c
> +++ b/drivers/pmdomain/renesas/rcar-sysc.c
> @@ -241,6 +241,7 @@ static int __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd)
>                 }
>         }
>
> +       genpd->flags |= GENPD_FLAG_NO_STAY_ON;

So this applies to all PM Domains.  While this doesn't hurt, perhaps it
should not be set for always-on domains, and thus moved up, to become
an "else" branch in the "if/else if/..."-logic handling always-on
domains at the top of the function?

This applies to rar-gen4-sysc.c, too.

>         genpd->power_off = rcar_sysc_pd_power_off;
>         genpd->power_on = rcar_sysc_pd_power_on;
>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH 3/5] pmdomain: renesas: rcar-sysc: Don't keep unused PM domains powered-on
Posted by Ulf Hansson 3 weeks ago
On Thu, 11 Sept 2025 at 11:02, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Ulf,
>
> On Tue, 9 Sept 2025 at 13:11, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > The recent changes to genpd makes a genpd OF provider that is powered-on at
> > initialization to stay powered-on, until the ->sync_state() callback is
> > invoked for it.
> >
> > This may not happen at all, if we wait for a consumer device to be probed,
> > leading to wasting energy. There are ways to enforce the ->sync_state()
> > callback to be invoked, through sysfs or via the probe-defer-timeout, but
> > none of them in its current form are a good fit for rcar-sysc PM domains.
> >
> > Let's therefore opt-out from this behaviour of genpd for now, by using the
> > GENPD_FLAG_NO_STAY_ON.
> >
> > Link: https://lore.kernel.org/all/20250701114733.636510-1-ulf.hansson@linaro.org/
> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > Fixes: 0e789b491ba0 ("pmdomain: core: Leave powered-on genpds on until sync_state")
> > Fixes: 13a4b7fb6260 ("pmdomain: core: Leave powered-on genpds on until late_initcall_sync")
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> > --- a/drivers/pmdomain/renesas/rcar-sysc.c
> > +++ b/drivers/pmdomain/renesas/rcar-sysc.c
> > @@ -241,6 +241,7 @@ static int __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd)
> >                 }
> >         }
> >
> > +       genpd->flags |= GENPD_FLAG_NO_STAY_ON;
>
> So this applies to all PM Domains.  While this doesn't hurt, perhaps it
> should not be set for always-on domains, and thus moved up, to become
> an "else" branch in the "if/else if/..."-logic handling always-on
> domains at the top of the function?
>
> This applies to rar-gen4-sysc.c, too.

You have a point, but currently this doesn't really matter. Genpd will
not power-off always-on-domains no matter whether
GENPD_FLAG_NO_STAY_ON is set or not.

The whole purpose from my side was to restore the behaviour we had
before, for the Reneas PM domains. I tend to think that it's better to
apply the $subject patch as is - and leave improvements to be made on
top.

Thanks a lot for testing and reviewing!

[...]

Kind regards
Uffe
Re: [PATCH 3/5] pmdomain: renesas: rcar-sysc: Don't keep unused PM domains powered-on
Posted by Geert Uytterhoeven 3 weeks ago
On Tue, 9 Sept 2025 at 13:11, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> The recent changes to genpd makes a genpd OF provider that is powered-on at
> initialization to stay powered-on, until the ->sync_state() callback is
> invoked for it.
>
> This may not happen at all, if we wait for a consumer device to be probed,
> leading to wasting energy. There are ways to enforce the ->sync_state()
> callback to be invoked, through sysfs or via the probe-defer-timeout, but
> none of them in its current form are a good fit for rcar-sysc PM domains.
>
> Let's therefore opt-out from this behaviour of genpd for now, by using the
> GENPD_FLAG_NO_STAY_ON.
>
> Link: https://lore.kernel.org/all/20250701114733.636510-1-ulf.hansson@linaro.org/
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Fixes: 0e789b491ba0 ("pmdomain: core: Leave powered-on genpds on until sync_state")
> Fixes: 13a4b7fb6260 ("pmdomain: core: Leave powered-on genpds on until late_initcall_sync")
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/pmdomain/renesas/rcar-sysc.c | 1 +

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
on R-Car M2-W (koelsch) and R-Car H3 ES2.0 (salvator-xs).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds