[PATCH] clk: renesas: mstp: Add genpd OF provider at postcore_initcall()

[PATCH] clk: renesas: mstp: Add genpd OF provider at postcore_initcall()

[Geert Uytterhoeven]

Genpd OF providers must now be registered after genpd bus registration.
However, cpg_mstp_add_clk_domain() is only called from CLK_OF_DECLARE(),
which is too early.  Hence on R-Car M1A, R-Car H1, and RZ/A1, the
CPG/MSTP Clock Domain fails to register, and any devices residing in
that clock domain fail to probe.

Fix this by splitting initialization into two steps:
  - The first part keeps on registering the PM domain with genpd at
    CLK_OF_DECLARE(),
  - The second and new part moves the registration of the genpd OF
    provider to a postcore_initcall().

See also commit c5ae5a0c61120d0c ("pmdomain: renesas: rcar-sysc: Add
genpd OF provider at postcore_initcall").

Fixes: 18a3a510ecfd0e50 ("pmdomain: core: Add the genpd->dev to the genpd provider bus")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
To be queued as a fix in renesas-clk-for-v6.17.

drivers/clk/mmp/clk-of-mmp2.c:mmp2_pm_domain_init() has the same issue.

Note that R-Car H1 still booted fine, as the CPG/MSTP Clock Domain is no
longer used directly on that SoC: all devices were moved to the R-Car
SYSC PM Domain in commits 751e29bbb64ad091 ("ARM: dts: r8a7779: Use SYSC
"always-on" PM Domain") and commit a03fa77d85a736d3 ("ARM: dts: r8a7779:
Use SYSC "always-on" PM Domain for HSCIF"), and use the clock domain
only indirectly from rcar-sysc through cpg_mstp_{at,de}tach_dev()).
---
 drivers/clk/renesas/clk-mstp.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c
index 6b47bb5eee45f75b..5fcc81b92a973771 100644
--- a/drivers/clk/renesas/clk-mstp.c
+++ b/drivers/clk/renesas/clk-mstp.c
@@ -305,6 +305,9 @@ void cpg_mstp_detach_dev(struct generic_pm_domain *unused, struct device *dev)
 		pm_clk_destroy(dev);
 }
 
+static struct device_node *cpg_mstp_pd_np __initdata = NULL;
+static struct generic_pm_domain *cpg_mstp_pd_genpd __initdata = NULL;
+
 void __init cpg_mstp_add_clk_domain(struct device_node *np)
 {
 	struct generic_pm_domain *pd;
@@ -326,5 +329,20 @@ void __init cpg_mstp_add_clk_domain(struct device_node *np)
 	pd->detach_dev = cpg_mstp_detach_dev;
 	pm_genpd_init(pd, &pm_domain_always_on_gov, false);
 
-	of_genpd_add_provider_simple(np, pd);
+	cpg_mstp_pd_np = of_node_get(np);
+	cpg_mstp_pd_genpd = pd;
+}
+
+static int __init cpg_mstp_pd_init_provider(void)
+{
+	int error;
+
+	if (!cpg_mstp_pd_np)
+		return -ENODEV;
+
+	error = of_genpd_add_provider_simple(cpg_mstp_pd_np, cpg_mstp_pd_genpd);
+
+	of_node_put(cpg_mstp_pd_np);
+	return error;
 }
+postcore_initcall(cpg_mstp_pd_init_provider);
-- 
2.43.0

Re: [PATCH] clk: renesas: mstp: Add genpd OF provider at postcore_initcall()

[Ulf Hansson]

On Wed, 13 Aug 2025 at 10:20, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> Genpd OF providers must now be registered after genpd bus registration.
> However, cpg_mstp_add_clk_domain() is only called from CLK_OF_DECLARE(),
> which is too early.  Hence on R-Car M1A, R-Car H1, and RZ/A1, the
> CPG/MSTP Clock Domain fails to register, and any devices residing in
> that clock domain fail to probe.
>
> Fix this by splitting initialization into two steps:
>   - The first part keeps on registering the PM domain with genpd at
>     CLK_OF_DECLARE(),
>   - The second and new part moves the registration of the genpd OF
>     provider to a postcore_initcall().
>
> See also commit c5ae5a0c61120d0c ("pmdomain: renesas: rcar-sysc: Add
> genpd OF provider at postcore_initcall").
>
> Fixes: 18a3a510ecfd0e50 ("pmdomain: core: Add the genpd->dev to the genpd provider bus")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

I assume there is a good reason to have one early part and one later
part for the OF provider registration, otherwise we might as well do
all the genpd registration at postcore_initcall, right?

In any case, please add:
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
> To be queued as a fix in renesas-clk-for-v6.17.
>
> drivers/clk/mmp/clk-of-mmp2.c:mmp2_pm_domain_init() has the same issue.
>
> Note that R-Car H1 still booted fine, as the CPG/MSTP Clock Domain is no
> longer used directly on that SoC: all devices were moved to the R-Car
> SYSC PM Domain in commits 751e29bbb64ad091 ("ARM: dts: r8a7779: Use SYSC
> "always-on" PM Domain") and commit a03fa77d85a736d3 ("ARM: dts: r8a7779:
> Use SYSC "always-on" PM Domain for HSCIF"), and use the clock domain
> only indirectly from rcar-sysc through cpg_mstp_{at,de}tach_dev()).
> ---
>  drivers/clk/renesas/clk-mstp.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c
> index 6b47bb5eee45f75b..5fcc81b92a973771 100644
> --- a/drivers/clk/renesas/clk-mstp.c
> +++ b/drivers/clk/renesas/clk-mstp.c
> @@ -305,6 +305,9 @@ void cpg_mstp_detach_dev(struct generic_pm_domain *unused, struct device *dev)
>                 pm_clk_destroy(dev);
>  }
>
> +static struct device_node *cpg_mstp_pd_np __initdata = NULL;
> +static struct generic_pm_domain *cpg_mstp_pd_genpd __initdata = NULL;
> +
>  void __init cpg_mstp_add_clk_domain(struct device_node *np)
>  {
>         struct generic_pm_domain *pd;
> @@ -326,5 +329,20 @@ void __init cpg_mstp_add_clk_domain(struct device_node *np)
>         pd->detach_dev = cpg_mstp_detach_dev;
>         pm_genpd_init(pd, &pm_domain_always_on_gov, false);
>
> -       of_genpd_add_provider_simple(np, pd);
> +       cpg_mstp_pd_np = of_node_get(np);
> +       cpg_mstp_pd_genpd = pd;
> +}
> +
> +static int __init cpg_mstp_pd_init_provider(void)
> +{
> +       int error;
> +
> +       if (!cpg_mstp_pd_np)
> +               return -ENODEV;
> +
> +       error = of_genpd_add_provider_simple(cpg_mstp_pd_np, cpg_mstp_pd_genpd);
> +
> +       of_node_put(cpg_mstp_pd_np);
> +       return error;
>  }
> +postcore_initcall(cpg_mstp_pd_init_provider);
> --
> 2.43.0
>

Re: [PATCH] clk: renesas: mstp: Add genpd OF provider at postcore_initcall()

[Geert Uytterhoeven]

Hi Ulf,

On Wed, 13 Aug 2025 at 13:18, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On Wed, 13 Aug 2025 at 10:20, Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> > Genpd OF providers must now be registered after genpd bus registration.
> > However, cpg_mstp_add_clk_domain() is only called from CLK_OF_DECLARE(),
> > which is too early.  Hence on R-Car M1A, R-Car H1, and RZ/A1, the
> > CPG/MSTP Clock Domain fails to register, and any devices residing in
> > that clock domain fail to probe.
> >
> > Fix this by splitting initialization into two steps:
> >   - The first part keeps on registering the PM domain with genpd at
> >     CLK_OF_DECLARE(),
> >   - The second and new part moves the registration of the genpd OF
> >     provider to a postcore_initcall().
> >
> > See also commit c5ae5a0c61120d0c ("pmdomain: renesas: rcar-sysc: Add
> > genpd OF provider at postcore_initcall").
> >
> > Fixes: 18a3a510ecfd0e50 ("pmdomain: core: Add the genpd->dev to the genpd provider bus")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> I assume there is a good reason to have one early part and one later
> part for the OF provider registration, otherwise we might as well do
> all the genpd registration at postcore_initcall, right?

Doing that requires migrating all clock drivers that provide a clock
domain from CLK_OF_DECLARE() to postcore_initcall() and/or platform
drivers.

> In any case, please add:
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

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