Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] clk: renesas: Set GENPD_FLAG_ALWAYS_ON for clock domain
@ 2019-08-16 12:52 Geert Uytterhoeven
  2019-08-16 12:52 ` [PATCH 1/3] clk: renesas: mstp: " Geert Uytterhoeven
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-08-16 12:52 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Rafael J . Wysocki, Kevin Hilman, Ulf Hansson, linux-renesas-soc,
	linux-clk, linux-pm, Geert Uytterhoeven

	Hi Mike, Stephen,

The Renesas Clock Domain drivers do not implement the
generic_pm_domain.power_{on,off}() callbacks, as the domains themselves
cannot be powered down.  Hence the domains should be marked as always-on
by setting the GENPD_FLAG_ALWAYS_ON flag.

This patch series that issue for R-Car M1A, RZ/A1, RZ/A2, and
RZ/N1 SoCs.
SH/R-Mobile SoCs are fixed in "[PATCH] soc: renesas: rmobile-sysc: Set
GENPD_FLAG_ALWAYS_ON for always-on domain"
(https://lore.kernel.org/linux-renesas-soc/20190816124106.15383-1-geert+renesas@glider.be/T/#u).
R-Car H1, Gen2, and Gen3 SoCs do not need a fix, as these SoCS use the
R-Car SYSC driver for Clock Domain creation, which already sets the
flag.

To be queued in clk-renesas for v5.4.

Thanks!

Geert Uytterhoeven (3):
  clk: renesas: mstp: Set GENPD_FLAG_ALWAYS_ON for clock domain
  clk: renesas: r9a06g032: Set GENPD_FLAG_ALWAYS_ON for clock domain
  clk: renesas: cpg-mssr: Set GENPD_FLAG_ALWAYS_ON for clock domain

 drivers/clk/renesas/clk-mstp.c         | 3 ++-
 drivers/clk/renesas/r9a06g032-clocks.c | 3 ++-
 drivers/clk/renesas/renesas-cpg-mssr.c | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

-- 
2.17.1

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/3] clk: renesas: mstp: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-16 12:52 [PATCH 0/3] clk: renesas: Set GENPD_FLAG_ALWAYS_ON for clock domain Geert Uytterhoeven
@ 2019-08-16 12:52 ` " Geert Uytterhoeven
  2019-08-16 15:38   ` Simon Horman
  2019-08-16 18:01   ` Stephen Boyd
  2019-08-16 12:52 ` [PATCH 2/3] clk: renesas: r9a06g032: " Geert Uytterhoeven
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-08-16 12:52 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Rafael J . Wysocki, Kevin Hilman, Ulf Hansson, linux-renesas-soc,
	linux-clk, linux-pm, Geert Uytterhoeven

The CPG/MSTP Clock Domain driver does not implement the
generic_pm_domain.power_{on,off}() callbacks, as the domain itself
cannot be powered down.  Hence the domain should be marked as always-on
by setting the GENPD_FLAG_ALWAYS_ON flag.

This gets rid of the following boot warning on RZ/A1:

    sh_mtu2 fcff0000.timer: PM domain cpg_clocks will not be powered off

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/clk/renesas/clk-mstp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c
index dfbef37eed4a82e8..fcf753a6249a0e53 100644
--- a/drivers/clk/renesas/clk-mstp.c
+++ b/drivers/clk/renesas/clk-mstp.c
@@ -335,7 +335,8 @@ void __init cpg_mstp_add_clk_domain(struct device_node *np)
 		return;
 
 	pd->name = np->name;
-	pd->flags = GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
+	pd->flags = GENPD_FLAG_PM_CLK | GENPD_FLAG_ALWAYS_ON |
+		    GENPD_FLAG_ACTIVE_WAKEUP;
 	pd->attach_dev = cpg_mstp_attach_dev;
 	pd->detach_dev = cpg_mstp_detach_dev;
 	pm_genpd_init(pd, &pm_domain_always_on_gov, false);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] clk: renesas: r9a06g032: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-16 12:52 [PATCH 0/3] clk: renesas: Set GENPD_FLAG_ALWAYS_ON for clock domain Geert Uytterhoeven
  2019-08-16 12:52 ` [PATCH 1/3] clk: renesas: mstp: " Geert Uytterhoeven
@ 2019-08-16 12:52 ` " Geert Uytterhoeven
  2019-08-16 15:39   ` Simon Horman
  2019-08-16 12:52 ` [PATCH 3/3] clk: renesas: cpg-mssr: " Geert Uytterhoeven
  2019-08-22 14:08 ` [PATCH 0/3] clk: renesas: " Ulf Hansson
  3 siblings, 1 reply; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-08-16 12:52 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Rafael J . Wysocki, Kevin Hilman, Ulf Hansson, linux-renesas-soc,
	linux-clk, linux-pm, Geert Uytterhoeven

The RZ/N1 Clock Domain driver does not implement the
generic_pm_domain.power_{on,off}() callbacks, as the domain itself
cannot be powered down.  Hence the domain should be marked as always-on
by setting the GENPD_FLAG_ALWAYS_ON flag.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Untested due to lack of hardware, but similar in spirit to the other
Clock Domain drivers.
---
 drivers/clk/renesas/r9a06g032-clocks.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/r9a06g032-clocks.c b/drivers/clk/renesas/r9a06g032-clocks.c
index b33e1383efe3abcd..1907ee195a08cf77 100644
--- a/drivers/clk/renesas/r9a06g032-clocks.c
+++ b/drivers/clk/renesas/r9a06g032-clocks.c
@@ -421,7 +421,8 @@ static int r9a06g032_add_clk_domain(struct device *dev)
 		return -ENOMEM;
 
 	pd->name = np->name;
-	pd->flags = GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
+	pd->flags = GENPD_FLAG_PM_CLK | GENPD_FLAG_ALWAYS_ON |
+		    GENPD_FLAG_ACTIVE_WAKEUP;
 	pd->attach_dev = r9a06g032_attach_dev;
 	pd->detach_dev = r9a06g032_detach_dev;
 	pm_genpd_init(pd, &pm_domain_always_on_gov, false);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/3] clk: renesas: cpg-mssr: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-16 12:52 [PATCH 0/3] clk: renesas: Set GENPD_FLAG_ALWAYS_ON for clock domain Geert Uytterhoeven
  2019-08-16 12:52 ` [PATCH 1/3] clk: renesas: mstp: " Geert Uytterhoeven
  2019-08-16 12:52 ` [PATCH 2/3] clk: renesas: r9a06g032: " Geert Uytterhoeven
@ 2019-08-16 12:52 ` " Geert Uytterhoeven
  2019-08-16 15:39   ` Simon Horman
  2019-08-22 14:08 ` [PATCH 0/3] clk: renesas: " Ulf Hansson
  3 siblings, 1 reply; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-08-16 12:52 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Rafael J . Wysocki, Kevin Hilman, Ulf Hansson, linux-renesas-soc,
	linux-clk, linux-pm, Geert Uytterhoeven

The CPG/MSSR Clock Domain driver does not implement the
generic_pm_domain.power_{on,off}() callbacks, as the domain itself
cannot be powered down.  Hence the domain should be marked as always-on
by setting the GENPD_FLAG_ALWAYS_ON flag.

Note that this only affects RZ/A2 SoCs.  On R-Car Gen2 and Gen3 SoCs,
the R-Car SYSC driver handles Clock Domain creation, and offloads only
device attachment/detachment to the CPG/MSSR driver.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/clk/renesas/renesas-cpg-mssr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
index d4075b13067429cd..132cc96895e3a978 100644
--- a/drivers/clk/renesas/renesas-cpg-mssr.c
+++ b/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -551,7 +551,8 @@ static int __init cpg_mssr_add_clk_domain(struct device *dev,
 
 	genpd = &pd->genpd;
 	genpd->name = np->name;
-	genpd->flags = GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
+	genpd->flags = GENPD_FLAG_PM_CLK | GENPD_FLAG_ALWAYS_ON |
+		       GENPD_FLAG_ACTIVE_WAKEUP;
 	genpd->attach_dev = cpg_mssr_attach_dev;
 	genpd->detach_dev = cpg_mssr_detach_dev;
 	pm_genpd_init(genpd, &pm_domain_always_on_gov, false);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] clk: renesas: mstp: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-16 12:52 ` [PATCH 1/3] clk: renesas: mstp: " Geert Uytterhoeven
@ 2019-08-16 15:38   ` Simon Horman
  2019-08-16 18:01   ` Stephen Boyd
  1 sibling, 0 replies; 15+ messages in thread
From: Simon Horman @ 2019-08-16 15:38 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, Rafael J . Wysocki,
	Kevin Hilman, Ulf Hansson, linux-renesas-soc, linux-clk,
	linux-pm

On Fri, Aug 16, 2019 at 02:52:23PM +0200, Geert Uytterhoeven wrote:
> The CPG/MSTP Clock Domain driver does not implement the
> generic_pm_domain.power_{on,off}() callbacks, as the domain itself
> cannot be powered down.  Hence the domain should be marked as always-on
> by setting the GENPD_FLAG_ALWAYS_ON flag.
> 
> This gets rid of the following boot warning on RZ/A1:
> 
>     sh_mtu2 fcff0000.timer: PM domain cpg_clocks will not be powered off
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] clk: renesas: cpg-mssr: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-16 12:52 ` [PATCH 3/3] clk: renesas: cpg-mssr: " Geert Uytterhoeven
@ 2019-08-16 15:39   ` Simon Horman
  0 siblings, 0 replies; 15+ messages in thread
From: Simon Horman @ 2019-08-16 15:39 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, Rafael J . Wysocki,
	Kevin Hilman, Ulf Hansson, linux-renesas-soc, linux-clk,
	linux-pm

On Fri, Aug 16, 2019 at 02:52:25PM +0200, Geert Uytterhoeven wrote:
> The CPG/MSSR Clock Domain driver does not implement the
> generic_pm_domain.power_{on,off}() callbacks, as the domain itself
> cannot be powered down.  Hence the domain should be marked as always-on
> by setting the GENPD_FLAG_ALWAYS_ON flag.
> 
> Note that this only affects RZ/A2 SoCs.  On R-Car Gen2 and Gen3 SoCs,
> the R-Car SYSC driver handles Clock Domain creation, and offloads only
> device attachment/detachment to the CPG/MSSR driver.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] clk: renesas: r9a06g032: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-16 12:52 ` [PATCH 2/3] clk: renesas: r9a06g032: " Geert Uytterhoeven
@ 2019-08-16 15:39   ` Simon Horman
  0 siblings, 0 replies; 15+ messages in thread
From: Simon Horman @ 2019-08-16 15:39 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, Rafael J . Wysocki,
	Kevin Hilman, Ulf Hansson, linux-renesas-soc, linux-clk,
	linux-pm

On Fri, Aug 16, 2019 at 02:52:24PM +0200, Geert Uytterhoeven wrote:
> The RZ/N1 Clock Domain driver does not implement the
> generic_pm_domain.power_{on,off}() callbacks, as the domain itself
> cannot be powered down.  Hence the domain should be marked as always-on
> by setting the GENPD_FLAG_ALWAYS_ON flag.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Untested due to lack of hardware, but similar in spirit to the other
> Clock Domain drivers.

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] clk: renesas: mstp: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-16 12:52 ` [PATCH 1/3] clk: renesas: mstp: " Geert Uytterhoeven
  2019-08-16 15:38   ` Simon Horman
@ 2019-08-16 18:01   ` Stephen Boyd
  2019-08-16 19:59     ` Geert Uytterhoeven
  1 sibling, 1 reply; 15+ messages in thread
From: Stephen Boyd @ 2019-08-16 18:01 UTC (permalink / raw)
  To: Geert Uytterhoeven, Michael Turquette
  Cc: Rafael J . Wysocki, Kevin Hilman, Ulf Hansson, linux-renesas-soc,
	linux-clk, linux-pm, Geert Uytterhoeven

Quoting Geert Uytterhoeven (2019-08-16 05:52:23)
> The CPG/MSTP Clock Domain driver does not implement the
> generic_pm_domain.power_{on,off}() callbacks, as the domain itself
> cannot be powered down.  Hence the domain should be marked as always-on
> by setting the GENPD_FLAG_ALWAYS_ON flag.
> 
> This gets rid of the following boot warning on RZ/A1:
> 
>     sh_mtu2 fcff0000.timer: PM domain cpg_clocks will not be powered off
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---

Are you going to add a Fixes tag?


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] clk: renesas: mstp: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-16 18:01   ` Stephen Boyd
@ 2019-08-16 19:59     ` Geert Uytterhoeven
  2019-08-17  3:48       ` Stephen Boyd
  0 siblings, 1 reply; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-08-16 19:59 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Geert Uytterhoeven, Michael Turquette, Rafael J . Wysocki,
	Kevin Hilman, Ulf Hansson, Linux-Renesas, linux-clk,
	Linux PM list

Hi Stephen,

On Fri, Aug 16, 2019 at 8:01 PM Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Geert Uytterhoeven (2019-08-16 05:52:23)
> > The CPG/MSTP Clock Domain driver does not implement the
> > generic_pm_domain.power_{on,off}() callbacks, as the domain itself
> > cannot be powered down.  Hence the domain should be marked as always-on
> > by setting the GENPD_FLAG_ALWAYS_ON flag.
> >
> > This gets rid of the following boot warning on RZ/A1:
> >
> >     sh_mtu2 fcff0000.timer: PM domain cpg_clocks will not be powered off
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
>
> Are you going to add a Fixes tag?

I didn't add a Fixes tag, as there's no clear point in history where the
problem appeared: the Clock Domain code in this driver predates the
introduction of the GENPD_FLAG_ALWAYS_ON flag by ca. 18 months.

Candidates are:
d716f4798ff8c65a ("PM / Domains: Support IRQ safe PM domains")
ffaa42e8a40b7f10 ("PM / Domains: Enable users of genpd to specify
always on PM domains")
075c37d59ecd4a8b ("PM / Domains: Don't warn about IRQ safe device for
an always on PM domain")

Do you think it's worth adding one or more of the above?
Thanks!

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] clk: renesas: mstp: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-16 19:59     ` Geert Uytterhoeven
@ 2019-08-17  3:48       ` Stephen Boyd
  2019-08-18  8:27         ` Geert Uytterhoeven
  0 siblings, 1 reply; 15+ messages in thread
From: Stephen Boyd @ 2019-08-17  3:48 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Geert Uytterhoeven, Michael Turquette, Rafael J . Wysocki,
	Kevin Hilman, Ulf Hansson, Linux-Renesas, linux-clk,
	Linux PM list

Quoting Geert Uytterhoeven (2019-08-16 12:59:32)
> Hi Stephen,
> 
> On Fri, Aug 16, 2019 at 8:01 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > Quoting Geert Uytterhoeven (2019-08-16 05:52:23)
> > > The CPG/MSTP Clock Domain driver does not implement the
> > > generic_pm_domain.power_{on,off}() callbacks, as the domain itself
> > > cannot be powered down.  Hence the domain should be marked as always-on
> > > by setting the GENPD_FLAG_ALWAYS_ON flag.
> > >
> > > This gets rid of the following boot warning on RZ/A1:
> > >
> > >     sh_mtu2 fcff0000.timer: PM domain cpg_clocks will not be powered off
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> >
> > Are you going to add a Fixes tag?
> 
> I didn't add a Fixes tag, as there's no clear point in history where the
> problem appeared: the Clock Domain code in this driver predates the
> introduction of the GENPD_FLAG_ALWAYS_ON flag by ca. 18 months.
> 
> Candidates are:
> d716f4798ff8c65a ("PM / Domains: Support IRQ safe PM domains")
> ffaa42e8a40b7f10 ("PM / Domains: Enable users of genpd to specify
> always on PM domains")
> 075c37d59ecd4a8b ("PM / Domains: Don't warn about IRQ safe device for
> an always on PM domain")
> 
> Do you think it's worth adding one or more of the above?
> Thanks!
> 

Well is it actually a problem to not specify the flag? I guess it's just
a potential problem if the genpd is ever powered off, but given that the
governor decides to leave it always enabled it doesn't actually matter?
So it's not really fixing anything besides silencing a harmless warning?


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] clk: renesas: mstp: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-17  3:48       ` Stephen Boyd
@ 2019-08-18  8:27         ` Geert Uytterhoeven
  2019-08-22 14:06           ` Ulf Hansson
  0 siblings, 1 reply; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-08-18  8:27 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Geert Uytterhoeven, Michael Turquette, Rafael J . Wysocki,
	Kevin Hilman, Ulf Hansson, Linux-Renesas, linux-clk,
	Linux PM list

Hi Stephen,

On Sat, Aug 17, 2019 at 5:48 AM Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Geert Uytterhoeven (2019-08-16 12:59:32)
> > On Fri, Aug 16, 2019 at 8:01 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > > Quoting Geert Uytterhoeven (2019-08-16 05:52:23)
> > > > The CPG/MSTP Clock Domain driver does not implement the
> > > > generic_pm_domain.power_{on,off}() callbacks, as the domain itself
> > > > cannot be powered down.  Hence the domain should be marked as always-on
> > > > by setting the GENPD_FLAG_ALWAYS_ON flag.
> > > >
> > > > This gets rid of the following boot warning on RZ/A1:
> > > >
> > > >     sh_mtu2 fcff0000.timer: PM domain cpg_clocks will not be powered off
> > > >
> > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > ---
> > >
> > > Are you going to add a Fixes tag?
> >
> > I didn't add a Fixes tag, as there's no clear point in history where the
> > problem appeared: the Clock Domain code in this driver predates the
> > introduction of the GENPD_FLAG_ALWAYS_ON flag by ca. 18 months.
> >
> > Candidates are:
> > d716f4798ff8c65a ("PM / Domains: Support IRQ safe PM domains")
> > ffaa42e8a40b7f10 ("PM / Domains: Enable users of genpd to specify
> > always on PM domains")
> > 075c37d59ecd4a8b ("PM / Domains: Don't warn about IRQ safe device for
> > an always on PM domain")
> >
> > Do you think it's worth adding one or more of the above?
>
> Well is it actually a problem to not specify the flag? I guess it's just
> a potential problem if the genpd is ever powered off, but given that the
> governor decides to leave it always enabled it doesn't actually matter?
> So it's not really fixing anything besides silencing a harmless warning?

The warning is indeed harmless.

The "interesting" case is the case where no warning is printed, as no
IRQ-safe device is present.  In that case, the absence of the
GENPD_FLAG_ALWAYS_ON flag means that the core PM Domain code will
consider the domain for power-off, and will loop over all devices part
of it, which is suboptimal.  Setting the flag avoids that.

Thanks for your continued questions, it made me realize I need to add more
meat to the description to these "simple" patches!

For the PM people: would it make sense to add a
WARN(!genpd->power_off && !genpd_is_always_on(genpd), "...") check to
pm_genpd_init()?
Or set GENPD_FLAG_ALWAYS_ON automatically if !genpd->power_off?

Thanks!

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] clk: renesas: mstp: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-18  8:27         ` Geert Uytterhoeven
@ 2019-08-22 14:06           ` Ulf Hansson
  2019-08-23  8:59             ` Geert Uytterhoeven
  0 siblings, 1 reply; 15+ messages in thread
From: Ulf Hansson @ 2019-08-22 14:06 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Stephen Boyd, Geert Uytterhoeven, Michael Turquette,
	Rafael J . Wysocki, Kevin Hilman, Linux-Renesas, linux-clk,
	Linux PM list

[...]

> >
> > Well is it actually a problem to not specify the flag? I guess it's just
> > a potential problem if the genpd is ever powered off, but given that the
> > governor decides to leave it always enabled it doesn't actually matter?
> > So it's not really fixing anything besides silencing a harmless warning?
>
> The warning is indeed harmless.
>
> The "interesting" case is the case where no warning is printed, as no
> IRQ-safe device is present.  In that case, the absence of the
> GENPD_FLAG_ALWAYS_ON flag means that the core PM Domain code will
> consider the domain for power-off, and will loop over all devices part
> of it, which is suboptimal.  Setting the flag avoids that.
>
> Thanks for your continued questions, it made me realize I need to add more
> meat to the description to these "simple" patches!
>
> For the PM people: would it make sense to add a
> WARN(!genpd->power_off && !genpd_is_always_on(genpd), "...") check to
> pm_genpd_init()?
> Or set GENPD_FLAG_ALWAYS_ON automatically if !genpd->power_off?

Well, wouldn't it be possible that the power is provided through a
master domain, thus not having the ->power_off() callback assigned for
the subdomain is perfectly fine, even without having
GENPD_FLAG_ALWAYS_ON not set.

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] clk: renesas: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-16 12:52 [PATCH 0/3] clk: renesas: Set GENPD_FLAG_ALWAYS_ON for clock domain Geert Uytterhoeven
                   ` (2 preceding siblings ...)
  2019-08-16 12:52 ` [PATCH 3/3] clk: renesas: cpg-mssr: " Geert Uytterhoeven
@ 2019-08-22 14:08 ` " Ulf Hansson
  2019-08-23  9:08   ` Geert Uytterhoeven
  3 siblings, 1 reply; 15+ messages in thread
From: Ulf Hansson @ 2019-08-22 14:08 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, Rafael J . Wysocki,
	Kevin Hilman, Linux-Renesas, linux-clk, Linux PM

On Fri, 16 Aug 2019 at 14:52, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
>         Hi Mike, Stephen,
>
> The Renesas Clock Domain drivers do not implement the
> generic_pm_domain.power_{on,off}() callbacks, as the domains themselves
> cannot be powered down.  Hence the domains should be marked as always-on
> by setting the GENPD_FLAG_ALWAYS_ON flag.
>
> This patch series that issue for R-Car M1A, RZ/A1, RZ/A2, and
> RZ/N1 SoCs.
> SH/R-Mobile SoCs are fixed in "[PATCH] soc: renesas: rmobile-sysc: Set
> GENPD_FLAG_ALWAYS_ON for always-on domain"
> (https://lore.kernel.org/linux-renesas-soc/20190816124106.15383-1-geert+renesas@glider.be/T/#u).
> R-Car H1, Gen2, and Gen3 SoCs do not need a fix, as these SoCS use the
> R-Car SYSC driver for Clock Domain creation, which already sets the
> flag.
>
> To be queued in clk-renesas for v5.4.
>
> Thanks!
>
> Geert Uytterhoeven (3):
>   clk: renesas: mstp: Set GENPD_FLAG_ALWAYS_ON for clock domain
>   clk: renesas: r9a06g032: Set GENPD_FLAG_ALWAYS_ON for clock domain
>   clk: renesas: cpg-mssr: Set GENPD_FLAG_ALWAYS_ON for clock domain
>
>  drivers/clk/renesas/clk-mstp.c         | 3 ++-
>  drivers/clk/renesas/r9a06g032-clocks.c | 3 ++-
>  drivers/clk/renesas/renesas-cpg-mssr.c | 3 ++-
>  3 files changed, 6 insertions(+), 3 deletions(-)
>

Feel free to add:

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] clk: renesas: mstp: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-22 14:06           ` Ulf Hansson
@ 2019-08-23  8:59             ` Geert Uytterhoeven
  0 siblings, 0 replies; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-08-23  8:59 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Stephen Boyd, Geert Uytterhoeven, Michael Turquette,
	Rafael J . Wysocki, Kevin Hilman, Linux-Renesas, linux-clk,
	Linux PM list

Hi Ulf,

On Thu, Aug 22, 2019 at 4:06 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > Well is it actually a problem to not specify the flag? I guess it's just
> > > a potential problem if the genpd is ever powered off, but given that the
> > > governor decides to leave it always enabled it doesn't actually matter?
> > > So it's not really fixing anything besides silencing a harmless warning?
> >
> > The warning is indeed harmless.
> >
> > The "interesting" case is the case where no warning is printed, as no
> > IRQ-safe device is present.  In that case, the absence of the
> > GENPD_FLAG_ALWAYS_ON flag means that the core PM Domain code will
> > consider the domain for power-off, and will loop over all devices part
> > of it, which is suboptimal.  Setting the flag avoids that.
> >
> > Thanks for your continued questions, it made me realize I need to add more
> > meat to the description to these "simple" patches!
> >
> > For the PM people: would it make sense to add a
> > WARN(!genpd->power_off && !genpd_is_always_on(genpd), "...") check to
> > pm_genpd_init()?
> > Or set GENPD_FLAG_ALWAYS_ON automatically if !genpd->power_off?
>
> Well, wouldn't it be possible that the power is provided through a
> master domain, thus not having the ->power_off() callback assigned for
> the subdomain is perfectly fine, even without having
> GENPD_FLAG_ALWAYS_ON not set.

Thanks, I hadn't considered that the clock domain might be a subdomain
of a power domain.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] clk: renesas: Set GENPD_FLAG_ALWAYS_ON for clock domain
  2019-08-22 14:08 ` [PATCH 0/3] clk: renesas: " Ulf Hansson
@ 2019-08-23  9:08   ` Geert Uytterhoeven
  0 siblings, 0 replies; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-08-23  9:08 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Geert Uytterhoeven, Michael Turquette, Stephen Boyd,
	Rafael J . Wysocki, Kevin Hilman, Linux-Renesas, linux-clk,
	Linux PM

On Thu, Aug 22, 2019 at 4:09 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On Fri, 16 Aug 2019 at 14:52, Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> > The Renesas Clock Domain drivers do not implement the
> > generic_pm_domain.power_{on,off}() callbacks, as the domains themselves
> > cannot be powered down.  Hence the domains should be marked as always-on
> > by setting the GENPD_FLAG_ALWAYS_ON flag.
> >
> > This patch series that issue for R-Car M1A, RZ/A1, RZ/A2, and
> > RZ/N1 SoCs.
> > SH/R-Mobile SoCs are fixed in "[PATCH] soc: renesas: rmobile-sysc: Set
> > GENPD_FLAG_ALWAYS_ON for always-on domain"
> > (https://lore.kernel.org/linux-renesas-soc/20190816124106.15383-1-geert+renesas@glider.be/T/#u).
> > R-Car H1, Gen2, and Gen3 SoCs do not need a fix, as these SoCS use the
> > R-Car SYSC driver for Clock Domain creation, which already sets the
> > flag.
> >
> > To be queued in clk-renesas for v5.4.
> >
> > Thanks!
> >
> > Geert Uytterhoeven (3):
> >   clk: renesas: mstp: Set GENPD_FLAG_ALWAYS_ON for clock domain
> >   clk: renesas: r9a06g032: Set GENPD_FLAG_ALWAYS_ON for clock domain
> >   clk: renesas: cpg-mssr: Set GENPD_FLAG_ALWAYS_ON for clock domain
> >
> >  drivers/clk/renesas/clk-mstp.c         | 3 ++-
> >  drivers/clk/renesas/r9a06g032-clocks.c | 3 ++-
> >  drivers/clk/renesas/renesas-cpg-mssr.c | 3 ++-
> >  3 files changed, 6 insertions(+), 3 deletions(-)
> >
>
> Feel free to add:
>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Thanks, queuing in clk-renesas-for-v5.4, with enhanced commit description:

    ... to prevent the core PM Domain code from considering it for power-off,
    and doing unnessary processing.

for all three of them, and

    This also gets rid of a boot warning when the Clock Domain contains an
    IRQ-safe device, e.g. on RZ/A1:

for the first patch.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, back to index

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-16 12:52 [PATCH 0/3] clk: renesas: Set GENPD_FLAG_ALWAYS_ON for clock domain Geert Uytterhoeven
2019-08-16 12:52 ` [PATCH 1/3] clk: renesas: mstp: " Geert Uytterhoeven
2019-08-16 15:38   ` Simon Horman
2019-08-16 18:01   ` Stephen Boyd
2019-08-16 19:59     ` Geert Uytterhoeven
2019-08-17  3:48       ` Stephen Boyd
2019-08-18  8:27         ` Geert Uytterhoeven
2019-08-22 14:06           ` Ulf Hansson
2019-08-23  8:59             ` Geert Uytterhoeven
2019-08-16 12:52 ` [PATCH 2/3] clk: renesas: r9a06g032: " Geert Uytterhoeven
2019-08-16 15:39   ` Simon Horman
2019-08-16 12:52 ` [PATCH 3/3] clk: renesas: cpg-mssr: " Geert Uytterhoeven
2019-08-16 15:39   ` Simon Horman
2019-08-22 14:08 ` [PATCH 0/3] clk: renesas: " Ulf Hansson
2019-08-23  9:08   ` Geert Uytterhoeven

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pm linux-pm/ https://lore.kernel.org/linux-pm \
		linux-pm@vger.kernel.org linux-pm@archiver.kernel.org
	public-inbox-index linux-pm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox