mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] clk: rockchip rk3588: configure CPLL in driver
@ 2025-10-27 14:15 Sascha Hauer
  2025-10-27 14:27 ` Alexander Shiyan
  0 siblings, 1 reply; 3+ messages in thread
From: Sascha Hauer @ 2025-10-27 14:15 UTC (permalink / raw)
  To: Barebox List; +Cc: Michael Tretter, Alexander Shiyan

The rk3588 CPLL should be configured to 1.5GHz and 09c87c85e0 ("ARM:
dts: rockchip: Set CPLL frequency for RK3588") does this. It does it
however after the assigned-clocks/assigned-clock-rates properties of the
"rockchip,rk3588-cru" node have been evaluated which contain a setting
of CLK_150M_SRC which is a child clock of the CPLL.  Configuring the
CPLL after CLK_150M_SRC alters the setting of the just configured 150M
clock again.

We must make sure to configure the CPLL before its child clocks. For
this we could overwrite the assigned-* properties in the
"rockchip,rk3588-cru" node, but with that we would miss future updates
to this property, so configure the CPLL in the driver code instead right
before we call into of_clk_add_provider().

Fixes: 09c87c85e0 ("ARM: dts: rockchip: Set CPLL frequency for RK3588")
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/dts/rk3588.dtsi          | 3 ---
 drivers/clk/rockchip/clk-rk3588.c | 7 +++++++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm/dts/rk3588.dtsi b/arch/arm/dts/rk3588.dtsi
index 42d692a9bd..416700cf0e 100644
--- a/arch/arm/dts/rk3588.dtsi
+++ b/arch/arm/dts/rk3588.dtsi
@@ -1,9 +1,6 @@
 // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
 
 / {
-	assigned-clocks = <&cru PLL_CPLL>;
-	assigned-clock-rates = <1500000000>;
-
 	dmc: memory-controller {
 		compatible = "rockchip,rk3588-dmc";
 		rockchip,pmu = <&pmu1grf>;
diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
index 5aecfb3b1b..fcf95131df 100644
--- a/drivers/clk/rockchip/clk-rk3588.c
+++ b/drivers/clk/rockchip/clk-rk3588.c
@@ -2500,6 +2500,13 @@ static void __init rk3588_clk_init(struct device_node *np)
 
 	rockchip_register_restart_notifier(ctx, RK3588_GLB_SRST_FST);
 
+	/*
+	 * CPLL must run at 1.5GHz. Do this here instead via assigned-clocks
+	 * in the device tree so that we do not have to overwrite the properties
+	 * in the upstream device tree.
+	 */
+	clk_set_rate(ctx->clk_data.clks[PLL_PPLL], 1500000000);
+
 	rockchip_clk_of_add_provider(np, ctx);
 }
 
-- 
2.47.3




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

* Re: [PATCH] clk: rockchip rk3588: configure CPLL in driver
  2025-10-27 14:15 [PATCH] clk: rockchip rk3588: configure CPLL in driver Sascha Hauer
@ 2025-10-27 14:27 ` Alexander Shiyan
  2025-10-28  7:03   ` Sascha Hauer
  0 siblings, 1 reply; 3+ messages in thread
From: Alexander Shiyan @ 2025-10-27 14:27 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List, Michael Tretter

Hello.

PLL_PPLL -> PLL_CPLL ?

пн, 27 окт. 2025 г. в 17:16, Sascha Hauer <s.hauer@pengutronix.de>:
>
> The rk3588 CPLL should be configured to 1.5GHz and 09c87c85e0 ("ARM:
> dts: rockchip: Set CPLL frequency for RK3588") does this. It does it
> however after the assigned-clocks/assigned-clock-rates properties of the
> "rockchip,rk3588-cru" node have been evaluated which contain a setting
> of CLK_150M_SRC which is a child clock of the CPLL.  Configuring the
> CPLL after CLK_150M_SRC alters the setting of the just configured 150M
> clock again.
>
> We must make sure to configure the CPLL before its child clocks. For
> this we could overwrite the assigned-* properties in the
> "rockchip,rk3588-cru" node, but with that we would miss future updates
> to this property, so configure the CPLL in the driver code instead right
> before we call into of_clk_add_provider().
>
> Fixes: 09c87c85e0 ("ARM: dts: rockchip: Set CPLL frequency for RK3588")
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  arch/arm/dts/rk3588.dtsi          | 3 ---
>  drivers/clk/rockchip/clk-rk3588.c | 7 +++++++
>  2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/dts/rk3588.dtsi b/arch/arm/dts/rk3588.dtsi
> index 42d692a9bd..416700cf0e 100644
> --- a/arch/arm/dts/rk3588.dtsi
> +++ b/arch/arm/dts/rk3588.dtsi
> @@ -1,9 +1,6 @@
>  // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>
>  / {
> -       assigned-clocks = <&cru PLL_CPLL>;
> -       assigned-clock-rates = <1500000000>;
> -
>         dmc: memory-controller {
>                 compatible = "rockchip,rk3588-dmc";
>                 rockchip,pmu = <&pmu1grf>;
> diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
> index 5aecfb3b1b..fcf95131df 100644
> --- a/drivers/clk/rockchip/clk-rk3588.c
> +++ b/drivers/clk/rockchip/clk-rk3588.c
> @@ -2500,6 +2500,13 @@ static void __init rk3588_clk_init(struct device_node *np)
>
>         rockchip_register_restart_notifier(ctx, RK3588_GLB_SRST_FST);
>
> +       /*
> +        * CPLL must run at 1.5GHz. Do this here instead via assigned-clocks
> +        * in the device tree so that we do not have to overwrite the properties
> +        * in the upstream device tree.
> +        */
> +       clk_set_rate(ctx->clk_data.clks[PLL_PPLL], 1500000000);
> +
>         rockchip_clk_of_add_provider(np, ctx);
>  }
>
> --
> 2.47.3
>



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

* Re: [PATCH] clk: rockchip rk3588: configure CPLL in driver
  2025-10-27 14:27 ` Alexander Shiyan
@ 2025-10-28  7:03   ` Sascha Hauer
  0 siblings, 0 replies; 3+ messages in thread
From: Sascha Hauer @ 2025-10-28  7:03 UTC (permalink / raw)
  To: Alexander Shiyan; +Cc: Barebox List, Michael Tretter

On Mon, Oct 27, 2025 at 05:27:43PM +0300, Alexander Shiyan wrote:
> Hello.
> 
> PLL_PPLL -> PLL_CPLL ?

Ups, indeed, yes. It seems I only checked CLK_150M_SRC to be at 150MHz,
but not that CPLL runs at 1.5GHz.

I am happy that with CPLL instead of PPLL the patch works as I had
intended :)

Sascha

> 
> пн, 27 окт. 2025 г. в 17:16, Sascha Hauer <s.hauer@pengutronix.de>:
> >
> > The rk3588 CPLL should be configured to 1.5GHz and 09c87c85e0 ("ARM:
> > dts: rockchip: Set CPLL frequency for RK3588") does this. It does it
> > however after the assigned-clocks/assigned-clock-rates properties of the
> > "rockchip,rk3588-cru" node have been evaluated which contain a setting
> > of CLK_150M_SRC which is a child clock of the CPLL.  Configuring the
> > CPLL after CLK_150M_SRC alters the setting of the just configured 150M
> > clock again.
> >
> > We must make sure to configure the CPLL before its child clocks. For
> > this we could overwrite the assigned-* properties in the
> > "rockchip,rk3588-cru" node, but with that we would miss future updates
> > to this property, so configure the CPLL in the driver code instead right
> > before we call into of_clk_add_provider().
> >
> > Fixes: 09c87c85e0 ("ARM: dts: rockchip: Set CPLL frequency for RK3588")
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  arch/arm/dts/rk3588.dtsi          | 3 ---
> >  drivers/clk/rockchip/clk-rk3588.c | 7 +++++++
> >  2 files changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/dts/rk3588.dtsi b/arch/arm/dts/rk3588.dtsi
> > index 42d692a9bd..416700cf0e 100644
> > --- a/arch/arm/dts/rk3588.dtsi
> > +++ b/arch/arm/dts/rk3588.dtsi
> > @@ -1,9 +1,6 @@
> >  // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >
> >  / {
> > -       assigned-clocks = <&cru PLL_CPLL>;
> > -       assigned-clock-rates = <1500000000>;
> > -
> >         dmc: memory-controller {
> >                 compatible = "rockchip,rk3588-dmc";
> >                 rockchip,pmu = <&pmu1grf>;
> > diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
> > index 5aecfb3b1b..fcf95131df 100644
> > --- a/drivers/clk/rockchip/clk-rk3588.c
> > +++ b/drivers/clk/rockchip/clk-rk3588.c
> > @@ -2500,6 +2500,13 @@ static void __init rk3588_clk_init(struct device_node *np)
> >
> >         rockchip_register_restart_notifier(ctx, RK3588_GLB_SRST_FST);
> >
> > +       /*
> > +        * CPLL must run at 1.5GHz. Do this here instead via assigned-clocks
> > +        * in the device tree so that we do not have to overwrite the properties
> > +        * in the upstream device tree.
> > +        */
> > +       clk_set_rate(ctx->clk_data.clks[PLL_PPLL], 1500000000);
> > +
> >         rockchip_clk_of_add_provider(np, ctx);
> >  }
> >
> > --
> > 2.47.3
> >
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



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

end of thread, other threads:[~2025-10-28  7:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-27 14:15 [PATCH] clk: rockchip rk3588: configure CPLL in driver Sascha Hauer
2025-10-27 14:27 ` Alexander Shiyan
2025-10-28  7:03   ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox