* [PATCH v2 0/5] Add XSPI clocks and improve divider clock handling for R9A09G077 SoC
@ 2025-10-28 16:51 Prabhakar
2025-10-28 16:51 ` [PATCH v2 1/5] clk: renesas: r9a09g077: Propagate rate changes to parent clocks Prabhakar
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Prabhakar @ 2025-10-28 16:51 UTC (permalink / raw)
To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm
Cc: linux-renesas-soc, linux-clk, linux-kernel, devicetree, Prabhakar,
Biju Das, Fabrizio Castro, Lad Prabhakar
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Hi All,
This patch series introduces support for the XSPI core and module clocks
in the Renesas R9A09G077 SoC. It also enhances the existing divider clock
registration process by utilizing device-managed helper functions and
ensuring proper propagation of rate changes to parent clocks.
Additionally, it adds necessary clock definitions for XSPI0/1 to the
device tree bindings for both R9A09G077 and R9A09G087 SoCs.
v1->v2 changes:
- In the fifth patch, added a custom divider clock type for XSPI clocks
to enforce hardware constraints on supported operating rates.
- In the fourth patch, added Acked-by and Reviewed-by tags.
- Added three new patches 1-3.
Cheers,
Prabhakar
Lad Prabhakar (5):
clk: renesas: r9a09g077: Propagate rate changes to parent clocks
clk: renesas: r9a09g077: Remove stray blank line
clk: renesas: r9a09g077: Use devm_ helpers for divider clock
registration
dt-bindings: clock: renesas,r9a09g077/87: Add XSPI0/1 IDs
clk: renesas: r9a09g077: Add xSPI core and module clocks
drivers/clk/renesas/r9a09g077-cpg.c | 186 ++++++++++++++++--
.../clock/renesas,r9a09g077-cpg-mssr.h | 2 +
.../clock/renesas,r9a09g087-cpg-mssr.h | 2 +
3 files changed, 174 insertions(+), 16 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 1/5] clk: renesas: r9a09g077: Propagate rate changes to parent clocks 2025-10-28 16:51 [PATCH v2 0/5] Add XSPI clocks and improve divider clock handling for R9A09G077 SoC Prabhakar @ 2025-10-28 16:51 ` Prabhakar 2025-11-10 13:18 ` Geert Uytterhoeven 2025-10-28 16:51 ` [PATCH v2 2/5] clk: renesas: r9a09g077: Remove stray blank line Prabhakar ` (3 subsequent siblings) 4 siblings, 1 reply; 14+ messages in thread From: Prabhakar @ 2025-10-28 16:51 UTC (permalink / raw) To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm Cc: linux-renesas-soc, linux-clk, linux-kernel, devicetree, Prabhakar, Biju Das, Fabrizio Castro, Lad Prabhakar From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Add the CLK_SET_RATE_PARENT flag to divider clock registration so that rate changes can propagate to parent clocks when needed. This allows the CPG divider clocks to request rate adjustments from their parent, ensuring correct frequency scaling and improved flexibility in clock rate selection. Fixes: 065fe720eec6e ("clk: renesas: Add support for R9A09G077 SoC") Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- v1-v2: - New patch --- drivers/clk/renesas/r9a09g077-cpg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/clk/renesas/r9a09g077-cpg.c b/drivers/clk/renesas/r9a09g077-cpg.c index 8b7e84a4c307..1cb33c12234e 100644 --- a/drivers/clk/renesas/r9a09g077-cpg.c +++ b/drivers/clk/renesas/r9a09g077-cpg.c @@ -221,7 +221,7 @@ r9a09g077_cpg_div_clk_register(struct device *dev, if (core->dtable) clk_hw = clk_hw_register_divider_table(dev, core->name, - parent_name, 0, + parent_name, CLK_SET_RATE_PARENT, addr, GET_SHIFT(core->conf), GET_WIDTH(core->conf), @@ -230,7 +230,7 @@ r9a09g077_cpg_div_clk_register(struct device *dev, &pub->rmw_lock); else clk_hw = clk_hw_register_divider(dev, core->name, - parent_name, 0, + parent_name, CLK_SET_RATE_PARENT, addr, GET_SHIFT(core->conf), GET_WIDTH(core->conf), -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/5] clk: renesas: r9a09g077: Propagate rate changes to parent clocks 2025-10-28 16:51 ` [PATCH v2 1/5] clk: renesas: r9a09g077: Propagate rate changes to parent clocks Prabhakar @ 2025-11-10 13:18 ` Geert Uytterhoeven 0 siblings, 0 replies; 14+ messages in thread From: Geert Uytterhoeven @ 2025-11-10 13:18 UTC (permalink / raw) To: Prabhakar Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-renesas-soc, linux-clk, linux-kernel, devicetree, Biju Das, Fabrizio Castro, Lad Prabhakar On Tue, 28 Oct 2025 at 17:51, Prabhakar <prabhakar.csengg@gmail.com> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Add the CLK_SET_RATE_PARENT flag to divider clock registration so that rate > changes can propagate to parent clocks when needed. This allows the CPG > divider clocks to request rate adjustments from their parent, ensuring > correct frequency scaling and improved flexibility in clock rate selection. > > Fixes: 065fe720eec6e ("clk: renesas: Add support for R9A09G077 SoC") > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> i.e. will queue in renesas-clk for v6.19. 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] 14+ messages in thread
* [PATCH v2 2/5] clk: renesas: r9a09g077: Remove stray blank line 2025-10-28 16:51 [PATCH v2 0/5] Add XSPI clocks and improve divider clock handling for R9A09G077 SoC Prabhakar 2025-10-28 16:51 ` [PATCH v2 1/5] clk: renesas: r9a09g077: Propagate rate changes to parent clocks Prabhakar @ 2025-10-28 16:51 ` Prabhakar 2025-11-10 13:18 ` Geert Uytterhoeven 2025-10-28 16:51 ` [PATCH v2 3/5] clk: renesas: r9a09g077: Use devm_ helpers for divider clock registration Prabhakar ` (2 subsequent siblings) 4 siblings, 1 reply; 14+ messages in thread From: Prabhakar @ 2025-10-28 16:51 UTC (permalink / raw) To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm Cc: linux-renesas-soc, linux-clk, linux-kernel, devicetree, Prabhakar, Biju Das, Fabrizio Castro, Lad Prabhakar From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Remove an unnecessary blank line at the end of r9a09g077_cpg_div_clk_register() to tidy up the code. Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- v1-v2: - New patch --- drivers/clk/renesas/r9a09g077-cpg.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/clk/renesas/r9a09g077-cpg.c b/drivers/clk/renesas/r9a09g077-cpg.c index 1cb33c12234e..666fc16b9a81 100644 --- a/drivers/clk/renesas/r9a09g077-cpg.c +++ b/drivers/clk/renesas/r9a09g077-cpg.c @@ -240,7 +240,6 @@ r9a09g077_cpg_div_clk_register(struct device *dev, return ERR_CAST(clk_hw); return clk_hw->clk; - } static struct clk * __init -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/5] clk: renesas: r9a09g077: Remove stray blank line 2025-10-28 16:51 ` [PATCH v2 2/5] clk: renesas: r9a09g077: Remove stray blank line Prabhakar @ 2025-11-10 13:18 ` Geert Uytterhoeven 0 siblings, 0 replies; 14+ messages in thread From: Geert Uytterhoeven @ 2025-11-10 13:18 UTC (permalink / raw) To: Prabhakar Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-renesas-soc, linux-clk, linux-kernel, devicetree, Biju Das, Fabrizio Castro, Lad Prabhakar On Tue, 28 Oct 2025 at 17:52, Prabhakar <prabhakar.csengg@gmail.com> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Remove an unnecessary blank line at the end of > r9a09g077_cpg_div_clk_register() to tidy up the code. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> i.e. will queue in renesas-clk for v6.19. 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] 14+ messages in thread
* [PATCH v2 3/5] clk: renesas: r9a09g077: Use devm_ helpers for divider clock registration 2025-10-28 16:51 [PATCH v2 0/5] Add XSPI clocks and improve divider clock handling for R9A09G077 SoC Prabhakar 2025-10-28 16:51 ` [PATCH v2 1/5] clk: renesas: r9a09g077: Propagate rate changes to parent clocks Prabhakar 2025-10-28 16:51 ` [PATCH v2 2/5] clk: renesas: r9a09g077: Remove stray blank line Prabhakar @ 2025-10-28 16:51 ` Prabhakar 2025-11-10 13:19 ` Geert Uytterhoeven 2025-10-28 16:51 ` [PATCH v2 4/5] dt-bindings: clock: renesas,r9a09g077/87: Add XSPI0/1 IDs Prabhakar 2025-10-28 16:51 ` [PATCH v2 5/5] clk: renesas: r9a09g077: Add xSPI core and module clocks Prabhakar 4 siblings, 1 reply; 14+ messages in thread From: Prabhakar @ 2025-10-28 16:51 UTC (permalink / raw) To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm Cc: linux-renesas-soc, linux-clk, linux-kernel, devicetree, Prabhakar, Biju Das, Fabrizio Castro, Lad Prabhakar From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Convert the divider clock registration in the R9A09G077 CPG driver to use device-managed (devm_) helper functions. Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- v1-v2: - New patch --- drivers/clk/renesas/r9a09g077-cpg.c | 30 +++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/drivers/clk/renesas/r9a09g077-cpg.c b/drivers/clk/renesas/r9a09g077-cpg.c index 666fc16b9a81..b46167d42084 100644 --- a/drivers/clk/renesas/r9a09g077-cpg.c +++ b/drivers/clk/renesas/r9a09g077-cpg.c @@ -220,21 +220,23 @@ r9a09g077_cpg_div_clk_register(struct device *dev, parent_name = __clk_get_name(parent); if (core->dtable) - clk_hw = clk_hw_register_divider_table(dev, core->name, - parent_name, CLK_SET_RATE_PARENT, - addr, - GET_SHIFT(core->conf), - GET_WIDTH(core->conf), - core->flag, - core->dtable, - &pub->rmw_lock); + clk_hw = devm_clk_hw_register_divider_table(dev, core->name, + parent_name, + CLK_SET_RATE_PARENT, + addr, + GET_SHIFT(core->conf), + GET_WIDTH(core->conf), + core->flag, + core->dtable, + &pub->rmw_lock); else - clk_hw = clk_hw_register_divider(dev, core->name, - parent_name, CLK_SET_RATE_PARENT, - addr, - GET_SHIFT(core->conf), - GET_WIDTH(core->conf), - core->flag, &pub->rmw_lock); + clk_hw = devm_clk_hw_register_divider(dev, core->name, + parent_name, + CLK_SET_RATE_PARENT, + addr, + GET_SHIFT(core->conf), + GET_WIDTH(core->conf), + core->flag, &pub->rmw_lock); if (IS_ERR(clk_hw)) return ERR_CAST(clk_hw); -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/5] clk: renesas: r9a09g077: Use devm_ helpers for divider clock registration 2025-10-28 16:51 ` [PATCH v2 3/5] clk: renesas: r9a09g077: Use devm_ helpers for divider clock registration Prabhakar @ 2025-11-10 13:19 ` Geert Uytterhoeven 0 siblings, 0 replies; 14+ messages in thread From: Geert Uytterhoeven @ 2025-11-10 13:19 UTC (permalink / raw) To: Prabhakar Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-renesas-soc, linux-clk, linux-kernel, devicetree, Biju Das, Fabrizio Castro, Lad Prabhakar On Tue, 28 Oct 2025 at 17:52, Prabhakar <prabhakar.csengg@gmail.com> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Convert the divider clock registration in the R9A09G077 CPG driver to use > device-managed (devm_) helper functions. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> i.e. will queue in renesas-clk for v6.19. 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] 14+ messages in thread
* [PATCH v2 4/5] dt-bindings: clock: renesas,r9a09g077/87: Add XSPI0/1 IDs 2025-10-28 16:51 [PATCH v2 0/5] Add XSPI clocks and improve divider clock handling for R9A09G077 SoC Prabhakar ` (2 preceding siblings ...) 2025-10-28 16:51 ` [PATCH v2 3/5] clk: renesas: r9a09g077: Use devm_ helpers for divider clock registration Prabhakar @ 2025-10-28 16:51 ` Prabhakar 2025-10-28 16:51 ` [PATCH v2 5/5] clk: renesas: r9a09g077: Add xSPI core and module clocks Prabhakar 4 siblings, 0 replies; 14+ messages in thread From: Prabhakar @ 2025-10-28 16:51 UTC (permalink / raw) To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm Cc: linux-renesas-soc, linux-clk, linux-kernel, devicetree, Prabhakar, Biju Das, Fabrizio Castro, Lad Prabhakar, Conor Dooley From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Add clock definitions for XSPI0/1 to both R9A09G077 and R9A09G087 SoCs. These definitions are required for describing XSPI devices in DT Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Acked-by: Conor Dooley <conor.dooley@microchip.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> --- v1-v2: - Added Acked-by and Reviewed-by tags --- include/dt-bindings/clock/renesas,r9a09g077-cpg-mssr.h | 2 ++ include/dt-bindings/clock/renesas,r9a09g087-cpg-mssr.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/include/dt-bindings/clock/renesas,r9a09g077-cpg-mssr.h b/include/dt-bindings/clock/renesas,r9a09g077-cpg-mssr.h index 2a805e06487b..9eaedca6a616 100644 --- a/include/dt-bindings/clock/renesas,r9a09g077-cpg-mssr.h +++ b/include/dt-bindings/clock/renesas,r9a09g077-cpg-mssr.h @@ -31,5 +31,7 @@ #define R9A09G077_ETCLKC 19 #define R9A09G077_ETCLKD 20 #define R9A09G077_ETCLKE 21 +#define R9A09G077_XSPI_CLK0 22 +#define R9A09G077_XSPI_CLK1 23 #endif /* __DT_BINDINGS_CLOCK_RENESAS_R9A09G077_CPG_H__ */ diff --git a/include/dt-bindings/clock/renesas,r9a09g087-cpg-mssr.h b/include/dt-bindings/clock/renesas,r9a09g087-cpg-mssr.h index 09da0ad33be6..606468ac49a4 100644 --- a/include/dt-bindings/clock/renesas,r9a09g087-cpg-mssr.h +++ b/include/dt-bindings/clock/renesas,r9a09g087-cpg-mssr.h @@ -31,5 +31,7 @@ #define R9A09G087_ETCLKC 19 #define R9A09G087_ETCLKD 20 #define R9A09G087_ETCLKE 21 +#define R9A09G087_XSPI_CLK0 22 +#define R9A09G087_XSPI_CLK1 23 #endif /* __DT_BINDINGS_CLOCK_RENESAS_R9A09G087_CPG_H__ */ -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 5/5] clk: renesas: r9a09g077: Add xSPI core and module clocks 2025-10-28 16:51 [PATCH v2 0/5] Add XSPI clocks and improve divider clock handling for R9A09G077 SoC Prabhakar ` (3 preceding siblings ...) 2025-10-28 16:51 ` [PATCH v2 4/5] dt-bindings: clock: renesas,r9a09g077/87: Add XSPI0/1 IDs Prabhakar @ 2025-10-28 16:51 ` Prabhakar 2025-11-10 13:48 ` Geert Uytterhoeven 4 siblings, 1 reply; 14+ messages in thread From: Prabhakar @ 2025-10-28 16:51 UTC (permalink / raw) To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm Cc: linux-renesas-soc, linux-clk, linux-kernel, devicetree, Prabhakar, Biju Das, Fabrizio Castro, Lad Prabhakar From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Add core clocks and module clock definitions required by the xSPI (Expanded SPI) IP on the R9A09G077 SoC. Define the new SCKCR fields FSELXSPI0/FSELXSPI1 and DIVSEL_XSPI0/1 and add two new core clocks XSPI_CLK0 and XSPI_CLK1. The xSPI block uses PCLKH as its bus clock (use as module clock parent) while the operation clock (XSPI_CLKn) is derived from PLL4. To support this arrangement provide mux/div selectors and divider tables for the supported XSPI operating rates. Add CLK_TYPE_RZT2H_FSELXSPI to implement a custom divider/mux clock where the determine_rate() callback enforces the hardware constraint: when the parent output is 600MHz only dividers 8 and 16 are valid, whereas for 800MHz operation the full divider set (6,8,16,32,64) may be used. The custom determine_rate() picks the best parent/divider pair to match the requested rate and programs the appropriate SCKCR fields. Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- v1->v2: - Added custom divider clock type for XSPI clocks to enforce hardware constraints on supported operating rates. --- drivers/clk/renesas/r9a09g077-cpg.c | 155 +++++++++++++++++++++++++++- 1 file changed, 154 insertions(+), 1 deletion(-) diff --git a/drivers/clk/renesas/r9a09g077-cpg.c b/drivers/clk/renesas/r9a09g077-cpg.c index b46167d42084..678dc36461c0 100644 --- a/drivers/clk/renesas/r9a09g077-cpg.c +++ b/drivers/clk/renesas/r9a09g077-cpg.c @@ -11,6 +11,8 @@ #include <linux/device.h> #include <linux/init.h> #include <linux/kernel.h> +#include <linux/math.h> +#include <linux/types.h> #include <dt-bindings/clock/renesas,r9a09g077-cpg-mssr.h> #include <dt-bindings/clock/renesas,r9a09g087-cpg-mssr.h> @@ -54,12 +56,19 @@ #define DIVSCI3ASYNC CONF_PACK(SCKCR3, 12, 2) #define DIVSCI4ASYNC CONF_PACK(SCKCR3, 14, 2) +#define FSELXSPI0 CONF_PACK(SCKCR, 0, 3) +#define FSELXSPI1 CONF_PACK(SCKCR, 8, 3) +#define DIVSEL_XSPI0 CONF_PACK(SCKCR, 6, 1) +#define DIVSEL_XSPI1 CONF_PACK(SCKCR, 14, 1) #define SEL_PLL CONF_PACK(SCKCR, 22, 1) +#define DIVSELXSPI_RATE_600MHZ 600000000UL +#define DIVSELXSPI_RATE_800MHZ 800000000UL enum rzt2h_clk_types { CLK_TYPE_RZT2H_DIV = CLK_TYPE_CUSTOM, /* Clock with divider */ CLK_TYPE_RZT2H_MUX, /* Clock with clock source selector */ + CLK_TYPE_RZT2H_FSELXSPI, }; #define DEF_DIV(_name, _id, _parent, _conf, _dtable) \ @@ -69,10 +78,13 @@ enum rzt2h_clk_types { DEF_TYPE(_name, _id, CLK_TYPE_RZT2H_MUX, .conf = _conf, \ .parent_names = _parent_names, .num_parents = _num_parents, \ .flag = 0, .mux_flags = _mux_flags) +#define DEF_DIV_FSELXSPI(_name, _id, _parent, _conf, _dtable) \ + DEF_TYPE(_name, _id, CLK_TYPE_RZT2H_FSELXSPI, .conf = _conf, \ + .parent = _parent, .dtable = _dtable, .flag = 0) enum clk_ids { /* Core Clock Outputs exported to DT */ - LAST_DT_CORE_CLK = R9A09G077_ETCLKE, + LAST_DT_CORE_CLK = R9A09G077_XSPI_CLK1, /* External Input Clocks */ CLK_EXTAL, @@ -88,12 +100,16 @@ enum clk_ids { CLK_SEL_CLK_PLL2, CLK_SEL_CLK_PLL4, CLK_PLL4D1, + CLK_PLL4D1_DIV3, + CLK_PLL4D1_DIV4, CLK_SCI0ASYNC, CLK_SCI1ASYNC, CLK_SCI2ASYNC, CLK_SCI3ASYNC, CLK_SCI4ASYNC, CLK_SCI5ASYNC, + CLK_DIVSELXSPI0_SCKCR, + CLK_DIVSELXSPI1_SCKCR, /* Module Clocks */ MOD_CLK_BASE, @@ -105,6 +121,15 @@ static const struct clk_div_table dtable_1_2[] = { {0, 0}, }; +static const struct clk_div_table dtable_6_8_16_32_64[] = { + {6, 64}, + {5, 32}, + {4, 16}, + {3, 8}, + {2, 6}, + {0, 0}, +}; + static const struct clk_div_table dtable_24_25_30_32[] = { {0, 32}, {1, 30}, @@ -119,6 +144,7 @@ static const char * const sel_clk_pll0[] = { ".loco", ".pll0" }; static const char * const sel_clk_pll1[] = { ".loco", ".pll1" }; static const char * const sel_clk_pll2[] = { ".loco", ".pll2" }; static const char * const sel_clk_pll4[] = { ".loco", ".pll4" }; +static const char * const sel_clk_pll4d1_div3_div4[] = { ".pll4d1_div3", ".pll4d1_div4" }; static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = { /* External Clock Inputs */ @@ -154,6 +180,15 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = { DEF_DIV(".sci5async", CLK_SCI5ASYNC, CLK_PLL4D1, DIVSCI5ASYNC, dtable_24_25_30_32), + DEF_FIXED(".pll4d1_div3", CLK_PLL4D1_DIV3, CLK_PLL4D1, 3, 1), + DEF_FIXED(".pll4d1_div4", CLK_PLL4D1_DIV4, CLK_PLL4D1, 4, 1), + DEF_MUX(".divselxspi0", CLK_DIVSELXSPI0_SCKCR, DIVSEL_XSPI0, + sel_clk_pll4d1_div3_div4, + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0), + DEF_MUX(".divselxspi1", CLK_DIVSELXSPI1_SCKCR, DIVSEL_XSPI1, + sel_clk_pll4d1_div3_div4, + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0), + /* Core output clk */ DEF_DIV("CA55C0", R9A09G077_CLK_CA55C0, CLK_SEL_CLK_PLL0, DIVCA55C0, dtable_1_2), @@ -178,9 +213,15 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = { DEF_FIXED("ETCLKC", R9A09G077_ETCLKC, CLK_SEL_CLK_PLL1, 10, 1), DEF_FIXED("ETCLKD", R9A09G077_ETCLKD, CLK_SEL_CLK_PLL1, 20, 1), DEF_FIXED("ETCLKE", R9A09G077_ETCLKE, CLK_SEL_CLK_PLL1, 40, 1), + DEF_DIV_FSELXSPI("XSPI_CLK0", R9A09G077_XSPI_CLK0, CLK_DIVSELXSPI0_SCKCR, + FSELXSPI0, dtable_6_8_16_32_64), + DEF_DIV_FSELXSPI("XSPI_CLK1", R9A09G077_XSPI_CLK1, CLK_DIVSELXSPI1_SCKCR, + FSELXSPI1, dtable_6_8_16_32_64), }; static const struct mssr_mod_clk r9a09g077_mod_clks[] __initconst = { + DEF_MOD("xspi0", 4, R9A09G077_CLK_PCLKH), + DEF_MOD("xspi1", 5, R9A09G077_CLK_PCLKH), DEF_MOD("sci0fck", 8, CLK_SCI0ASYNC), DEF_MOD("sci1fck", 9, CLK_SCI1ASYNC), DEF_MOD("sci2fck", 10, CLK_SCI2ASYNC), @@ -264,6 +305,116 @@ r9a09g077_cpg_mux_clk_register(struct device *dev, return clk_hw->clk; } +static int r9a09g077_cpg_fselxspi_determine_rate(struct clk_hw *hw, + struct clk_rate_request *req) +{ + struct clk_divider *divider = to_clk_divider(hw); + unsigned long parent_rate, best = 0, now; + const struct clk_div_table *clkt; + unsigned long rate = req->rate; + int div = 0; + + if (!rate) + rate = 1; + + for (clkt = divider->table; clkt->div; clkt++) { + parent_rate = clk_hw_round_rate(req->best_parent_hw, rate * clkt->div); + /* + * DIVSELXSPIx supports 800MHz and 600MHz operation. + * When the parent_rate is 600MHz, only dividers of 8 and 16 + * are supported otherwise dividers of 6, 8, 16, 32, 64 are supported. + * This check ensures that FSELXSPIx is set correctly. + */ + if (parent_rate == DIVSELXSPI_RATE_600MHZ && + (clkt->div != 8 && clkt->div != 16)) + continue; + now = DIV_ROUND_UP_ULL((u64)parent_rate, clkt->div); + if (abs(rate - now) < abs(rate - best)) { + div = clkt->div; + best = now; + req->best_parent_rate = parent_rate; + } + } + + if (!div) { + u8 maxdiv = 0; + + req->best_parent_rate = clk_hw_round_rate(req->best_parent_hw, 1); + /* + * If DIVSELXSPIx is set to 800MHz set the maximum divider + * or else fall back to divider of 16 which is a maximum + * supported divider for 600MHz operation. + */ + if (req->best_parent_rate == DIVSELXSPI_RATE_800MHZ) { + for (clkt = divider->table; clkt->div; clkt++) { + if (clkt->div > maxdiv) + maxdiv = clkt->div; + } + div = maxdiv; + } else { + div = 16; + } + } + + req->rate = DIV_ROUND_UP_ULL((u64)req->best_parent_rate, div); + + return 0; +} + +static struct clk * __init +r9a09g077_cpg_fselxspi_div_clk_register(struct device *dev, + const struct cpg_core_clk *core, + void __iomem *addr, + struct cpg_mssr_pub *pub) +{ + static struct clk_ops *xspi_div_ops; + struct clk_init_data init = {}; + const struct clk *parent; + const char *parent_name; + struct clk_divider *div; + struct clk_hw *hw; + int ret; + + parent = pub->clks[core->parent]; + if (IS_ERR(parent)) + return ERR_CAST(parent); + + div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL); + if (!div) + return ERR_PTR(-ENOMEM); + + if (!xspi_div_ops) { + xspi_div_ops = devm_kzalloc(dev, sizeof(*xspi_div_ops), GFP_KERNEL); + if (!xspi_div_ops) + return ERR_PTR(-ENOMEM); + memcpy(xspi_div_ops, &clk_divider_ops, + sizeof(const struct clk_ops)); + xspi_div_ops->determine_rate = r9a09g077_cpg_fselxspi_determine_rate; + } + + parent_name = __clk_get_name(parent); + init.name = core->name; + init.ops = xspi_div_ops; + init.flags = CLK_SET_RATE_PARENT; + init.parent_names = &parent_name; + init.num_parents = 1; + + div->reg = addr; + div->shift = GET_SHIFT(core->conf); + div->width = GET_WIDTH(core->conf); + div->flags = core->flag; + div->lock = &pub->rmw_lock; + div->hw.init = &init; + div->table = core->dtable; + + hw = &div->hw; + ret = devm_clk_hw_register(dev, hw); + if (ret) + return ERR_PTR(ret); + + return hw->clk; +} + static struct clk * __init r9a09g077_cpg_clk_register(struct device *dev, const struct cpg_core_clk *core, const struct cpg_mssr_info *info, @@ -278,6 +429,8 @@ r9a09g077_cpg_clk_register(struct device *dev, const struct cpg_core_clk *core, return r9a09g077_cpg_div_clk_register(dev, core, addr, pub); case CLK_TYPE_RZT2H_MUX: return r9a09g077_cpg_mux_clk_register(dev, core, addr, pub); + case CLK_TYPE_RZT2H_FSELXSPI: + return r9a09g077_cpg_fselxspi_div_clk_register(dev, core, addr, pub); default: return ERR_PTR(-EINVAL); } -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/5] clk: renesas: r9a09g077: Add xSPI core and module clocks 2025-10-28 16:51 ` [PATCH v2 5/5] clk: renesas: r9a09g077: Add xSPI core and module clocks Prabhakar @ 2025-11-10 13:48 ` Geert Uytterhoeven 2025-11-10 21:38 ` Lad, Prabhakar 0 siblings, 1 reply; 14+ messages in thread From: Geert Uytterhoeven @ 2025-11-10 13:48 UTC (permalink / raw) To: Prabhakar Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-renesas-soc, linux-clk, linux-kernel, devicetree, Biju Das, Fabrizio Castro, Lad Prabhakar Hi Prabhakar, On Tue, 28 Oct 2025 at 17:52, Prabhakar <prabhakar.csengg@gmail.com> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Add core clocks and module clock definitions required by the xSPI > (Expanded SPI) IP on the R9A09G077 SoC. > > Define the new SCKCR fields FSELXSPI0/FSELXSPI1 and DIVSEL_XSPI0/1 and > add two new core clocks XSPI_CLK0 and XSPI_CLK1. The xSPI block uses > PCLKH as its bus clock (use as module clock parent) while the operation > clock (XSPI_CLKn) is derived from PLL4. To support this arrangement > provide mux/div selectors and divider tables for the supported > XSPI operating rates. > > Add CLK_TYPE_RZT2H_FSELXSPI to implement a custom divider/mux clock > where the determine_rate() callback enforces the hardware constraint: > when the parent output is 600MHz only dividers 8 and 16 are valid, > whereas for 800MHz operation the full divider set (6,8,16,32,64) may > be used. The custom determine_rate() picks the best parent/divider pair > to match the requested rate and programs the appropriate SCKCR fields. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > v1->v2: > - Added custom divider clock type for XSPI clocks to enforce hardware > constraints on supported operating rates. Thanks for the update! > --- a/drivers/clk/renesas/r9a09g077-cpg.c > +++ b/drivers/clk/renesas/r9a09g077-cpg.c > @@ -54,12 +56,19 @@ > #define DIVSCI3ASYNC CONF_PACK(SCKCR3, 12, 2) > #define DIVSCI4ASYNC CONF_PACK(SCKCR3, 14, 2) > > +#define FSELXSPI0 CONF_PACK(SCKCR, 0, 3) > +#define FSELXSPI1 CONF_PACK(SCKCR, 8, 3) > +#define DIVSEL_XSPI0 CONF_PACK(SCKCR, 6, 1) > +#define DIVSEL_XSPI1 CONF_PACK(SCKCR, 14, 1) > #define SEL_PLL CONF_PACK(SCKCR, 22, 1) > > +#define DIVSELXSPI_RATE_600MHZ 600000000UL > +#define DIVSELXSPI_RATE_800MHZ 800000000UL I find it a bit weird that the name of the define includes its value. Perhaps just use "600 * MEGA" resp. "800 * MEGA" in the code instead? But see below... > @@ -154,6 +180,15 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = { > DEF_DIV(".sci5async", CLK_SCI5ASYNC, CLK_PLL4D1, DIVSCI5ASYNC, > dtable_24_25_30_32), > > + DEF_FIXED(".pll4d1_div3", CLK_PLL4D1_DIV3, CLK_PLL4D1, 3, 1), > + DEF_FIXED(".pll4d1_div4", CLK_PLL4D1_DIV4, CLK_PLL4D1, 4, 1), Please move these two just below the existing entry for ".pll4d1". > + DEF_MUX(".divselxspi0", CLK_DIVSELXSPI0_SCKCR, DIVSEL_XSPI0, > + sel_clk_pll4d1_div3_div4, > + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0), > + DEF_MUX(".divselxspi1", CLK_DIVSELXSPI1_SCKCR, DIVSEL_XSPI1, > + sel_clk_pll4d1_div3_div4, > + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0), > + > /* Core output clk */ > DEF_DIV("CA55C0", R9A09G077_CLK_CA55C0, CLK_SEL_CLK_PLL0, DIVCA55C0, > dtable_1_2), > @@ -264,6 +305,116 @@ r9a09g077_cpg_mux_clk_register(struct device *dev, > return clk_hw->clk; > } > > +static int r9a09g077_cpg_fselxspi_determine_rate(struct clk_hw *hw, > + struct clk_rate_request *req) > +{ > + struct clk_divider *divider = to_clk_divider(hw); > + unsigned long parent_rate, best = 0, now; > + const struct clk_div_table *clkt; > + unsigned long rate = req->rate; > + int div = 0; unsigned int > + > + if (!rate) > + rate = 1; > + > + for (clkt = divider->table; clkt->div; clkt++) { > + parent_rate = clk_hw_round_rate(req->best_parent_hw, rate * clkt->div); I had expected the use of some *_determinate_rate_*() helper, as the parent can be changed to find a better clock rate? Perhaps you should use a composite clock for that? > + /* > + * DIVSELXSPIx supports 800MHz and 600MHz operation. > + * When the parent_rate is 600MHz, only dividers of 8 and 16 > + * are supported otherwise dividers of 6, 8, 16, 32, 64 are supported. > + * This check ensures that FSELXSPIx is set correctly. > + */ > + if (parent_rate == DIVSELXSPI_RATE_600MHZ && Does this actually work as expected? I doubt parent_rate is guaranteed to be exactly 600 or 800 MHz, and expect it can differ slightly due to rounding. Hence I would look at clk_fixed_factor.div instead. > + (clkt->div != 8 && clkt->div != 16)) > + continue; > + now = DIV_ROUND_UP_ULL((u64)parent_rate, clkt->div); No need to cast to u64 (DIV_ROUND_*_ULL() handle this internally). > + if (abs(rate - now) < abs(rate - best)) { > + div = clkt->div; > + best = now; > + req->best_parent_rate = parent_rate; > + } > + } > + > + if (!div) { > + u8 maxdiv = 0; > + > + req->best_parent_rate = clk_hw_round_rate(req->best_parent_hw, 1); > + /* > + * If DIVSELXSPIx is set to 800MHz set the maximum divider > + * or else fall back to divider of 16 which is a maximum > + * supported divider for 600MHz operation. > + */ > + if (req->best_parent_rate == DIVSELXSPI_RATE_800MHZ) { > + for (clkt = divider->table; clkt->div; clkt++) { > + if (clkt->div > maxdiv) > + maxdiv = clkt->div; > + } > + div = maxdiv; Why not hardcode the divider, like in the else branch? > + } else { > + div = 16; > + } > + } > + > + req->rate = DIV_ROUND_UP_ULL((u64)req->best_parent_rate, div); No need to cast to u64. > + > + return 0; > +} 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] 14+ messages in thread
* Re: [PATCH v2 5/5] clk: renesas: r9a09g077: Add xSPI core and module clocks 2025-11-10 13:48 ` Geert Uytterhoeven @ 2025-11-10 21:38 ` Lad, Prabhakar 2025-11-17 10:52 ` Lad, Prabhakar 2025-11-17 13:28 ` Geert Uytterhoeven 0 siblings, 2 replies; 14+ messages in thread From: Lad, Prabhakar @ 2025-11-10 21:38 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-renesas-soc, linux-clk, linux-kernel, devicetree, Biju Das, Fabrizio Castro, Lad Prabhakar Hi Geert, Thank you for the review. On Mon, Nov 10, 2025 at 1:48 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Tue, 28 Oct 2025 at 17:52, Prabhakar <prabhakar.csengg@gmail.com> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Add core clocks and module clock definitions required by the xSPI > > (Expanded SPI) IP on the R9A09G077 SoC. > > > > Define the new SCKCR fields FSELXSPI0/FSELXSPI1 and DIVSEL_XSPI0/1 and > > add two new core clocks XSPI_CLK0 and XSPI_CLK1. The xSPI block uses > > PCLKH as its bus clock (use as module clock parent) while the operation > > clock (XSPI_CLKn) is derived from PLL4. To support this arrangement > > provide mux/div selectors and divider tables for the supported > > XSPI operating rates. > > > > Add CLK_TYPE_RZT2H_FSELXSPI to implement a custom divider/mux clock > > where the determine_rate() callback enforces the hardware constraint: > > when the parent output is 600MHz only dividers 8 and 16 are valid, > > whereas for 800MHz operation the full divider set (6,8,16,32,64) may > > be used. The custom determine_rate() picks the best parent/divider pair > > to match the requested rate and programs the appropriate SCKCR fields. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > v1->v2: > > - Added custom divider clock type for XSPI clocks to enforce hardware > > constraints on supported operating rates. > > Thanks for the update! > > > --- a/drivers/clk/renesas/r9a09g077-cpg.c > > +++ b/drivers/clk/renesas/r9a09g077-cpg.c > > > @@ -54,12 +56,19 @@ > > #define DIVSCI3ASYNC CONF_PACK(SCKCR3, 12, 2) > > #define DIVSCI4ASYNC CONF_PACK(SCKCR3, 14, 2) > > > > +#define FSELXSPI0 CONF_PACK(SCKCR, 0, 3) > > +#define FSELXSPI1 CONF_PACK(SCKCR, 8, 3) > > +#define DIVSEL_XSPI0 CONF_PACK(SCKCR, 6, 1) > > +#define DIVSEL_XSPI1 CONF_PACK(SCKCR, 14, 1) > > #define SEL_PLL CONF_PACK(SCKCR, 22, 1) > > > > +#define DIVSELXSPI_RATE_600MHZ 600000000UL > > +#define DIVSELXSPI_RATE_800MHZ 800000000UL > > I find it a bit weird that the name of the define includes its value. > Perhaps just use "600 * MEGA" resp. "800 * MEGA" in the code instead? OK. > But see below... > > > @@ -154,6 +180,15 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = { > > DEF_DIV(".sci5async", CLK_SCI5ASYNC, CLK_PLL4D1, DIVSCI5ASYNC, > > dtable_24_25_30_32), > > > > + DEF_FIXED(".pll4d1_div3", CLK_PLL4D1_DIV3, CLK_PLL4D1, 3, 1), > > + DEF_FIXED(".pll4d1_div4", CLK_PLL4D1_DIV4, CLK_PLL4D1, 4, 1), > > Please move these two just below the existing entry for ".pll4d1". > Ok, I will move it below .pll4d1 > > + DEF_MUX(".divselxspi0", CLK_DIVSELXSPI0_SCKCR, DIVSEL_XSPI0, > > + sel_clk_pll4d1_div3_div4, > > + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0), > > + DEF_MUX(".divselxspi1", CLK_DIVSELXSPI1_SCKCR, DIVSEL_XSPI1, > > + sel_clk_pll4d1_div3_div4, > > + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0), > > + > > /* Core output clk */ > > DEF_DIV("CA55C0", R9A09G077_CLK_CA55C0, CLK_SEL_CLK_PLL0, DIVCA55C0, > > dtable_1_2), > > > @@ -264,6 +305,116 @@ r9a09g077_cpg_mux_clk_register(struct device *dev, > > return clk_hw->clk; > > } > > > > +static int r9a09g077_cpg_fselxspi_determine_rate(struct clk_hw *hw, > > + struct clk_rate_request *req) > > +{ > > + struct clk_divider *divider = to_clk_divider(hw); > > + unsigned long parent_rate, best = 0, now; > > + const struct clk_div_table *clkt; > > + unsigned long rate = req->rate; > > + int div = 0; > > unsigned int > Ok. > > + > > + if (!rate) > > + rate = 1; > > + > > + for (clkt = divider->table; clkt->div; clkt++) { > > + parent_rate = clk_hw_round_rate(req->best_parent_hw, rate * clkt->div); > > I had expected the use of some *_determinate_rate_*() helper, as the > parent can be changed to find a better clock rate? > Perhaps you should use a composite clock for that? > > > + /* > > + * DIVSELXSPIx supports 800MHz and 600MHz operation. > > + * When the parent_rate is 600MHz, only dividers of 8 and 16 > > + * are supported otherwise dividers of 6, 8, 16, 32, 64 are supported. > > + * This check ensures that FSELXSPIx is set correctly. > > + */ > > + if (parent_rate == DIVSELXSPI_RATE_600MHZ && > > Does this actually work as expected? I doubt parent_rate is guaranteed > to be exactly 600 or 800 MHz, and expect it can differ slightly due > to rounding. Hence I would look at clk_fixed_factor.div instead. > With below diff, Ive got the below results for the various freqs requested where appropriate parent and divider clocks are picked. @@ -317,6 +317,7 @@ static int r9a09g077_cpg_fselxspi_determine_rate(struct clk_hw *hw, for (clkt = divider->table; clkt->div; clkt++) { parent_rate = clk_hw_round_rate(req->best_parent_hw, rate * clkt->div); + pr_err("parent_rate=%lu, req-rate=%lu div=%u\n", parent_rate, rate, clkt->div); /* * DIVSELXSPIx supports 800MHz and 600MHz operation. * When the parent_rate is 600MHz, only dividers of 8 and 16 Logs: --------- Case 0# assigned-clock-rates = <133333334>; [ 15.419300] parent_rate=800000000, req-rate=133333334 div=64 [ 15.437698] parent_rate=800000000, req-rate=133333334 div=32 [ 15.455224] parent_rate=800000000, req-rate=133333334 div=16 [ 15.501291] parent_rate=800000000, req-rate=133333334 div=8 [ 15.507801] parent_rate=800000000, req-rate=133333334 div=6 [ 15.519221] parent_rate=800000000, req-rate=133333334 div=64 [ 15.525789] parent_rate=800000000, req-rate=133333334 div=32 [ 15.549625] parent_rate=800000000, req-rate=133333334 div=16 [ 15.556120] parent_rate=800000000, req-rate=133333334 div=8 [ 15.564110] parent_rate=800000000, req-rate=133333334 div=6 root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e "xspi0" -e "XSPI_CLK0" -e "divselxspi0" .divselxspi0 1 1 0 800000000 0 0 50000 Y deviceless no_connection_id XSPI_CLK0 1 1 0 133333334 0 0 50000 Y 801c0000.spi spi xspi0 0 1 0 250000000 0 0 50000 N deviceless of_clk_get_from_provider root@rzt2h-evk:~# Case 1# assigned-clock-rates = <100000000>; [ 15.496291] parent_rate=800000000, req-rate=100000000 div=64 [ 15.510068] parent_rate=800000000, req-rate=100000000 div=32 [ 15.517142] parent_rate=800000000, req-rate=100000000 div=16 [ 15.524047] parent_rate=800000000, req-rate=100000000 div=8 [ 15.533174] parent_rate=600000000, req-rate=100000000 div=6 [ 15.540096] parent_rate=800000000, req-rate=100000000 div=64 [ 15.548135] parent_rate=800000000, req-rate=100000000 div=32 [ 15.555119] parent_rate=800000000, req-rate=100000000 div=16 [ 15.562395] parent_rate=800000000, req-rate=100000000 div=8 [ 15.573521] parent_rate=600000000, req-rate=100000000 div=6 root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e "xspi0" -e "XSPI_CLK0" -e "divselxspi0" .divselxspi0 1 1 0 800000000 0 0 50000 Y deviceless no_connection_id XSPI_CLK0 1 1 0 100000000 0 0 50000 Y 801c0000.spi spi xspi0 0 1 0 250000000 0 0 50000 N deviceless of_clk_get_from_provider root@rzt2h-evk:~# Case 2# assigned-clock-rates = <75000000>; [ 12.288507] parent_rate=800000000, req-rate=75000000 div=64 [ 12.310528] parent_rate=800000000, req-rate=75000000 div=32 [ 12.318426] parent_rate=800000000, req-rate=75000000 div=16 [ 12.326361] parent_rate=600000000, req-rate=75000000 div=8 [ 12.341540] parent_rate=0, req-rate=75000000 div=6 [ 12.347546] parent_rate=800000000, req-rate=75000000 div=64 [ 12.357593] parent_rate=800000000, req-rate=75000000 div=32 [ 12.367148] parent_rate=800000000, req-rate=75000000 div=16 [ 12.418871] parent_rate=600000000, req-rate=75000000 div=8 [ 12.433560] parent_rate=0, req-rate=75000000 div=6 root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e "xspi0" -e "XSPI_CLK0" -e "divselxspi0" .divselxspi0 1 1 0 600000000 0 0 50000 Y deviceless no_connection_id XSPI_CLK0 1 1 0 75000000 0 0 50000 Y 801c0000.spi spi xspi0 0 1 0 250000000 0 0 50000 N deviceless of_clk_get_from_provider root@rzt2h-evk:~# Case 3# assigned-clock-rates = <50000000>; [ 15.240214] parent_rate=800000000, req-rate=50000000 div=64 [ 15.253498] parent_rate=800000000, req-rate=50000000 div=32 [ 15.261521] parent_rate=800000000, req-rate=50000000 div=16 [ 15.272941] parent_rate=0, req-rate=50000000 div=8 [ 15.280532] parent_rate=0, req-rate=50000000 div=6 [ 15.289979] parent_rate=800000000, req-rate=50000000 div=64 [ 15.298745] parent_rate=800000000, req-rate=50000000 div=32 [ 15.309879] parent_rate=800000000, req-rate=50000000 div=16 [ 15.319881] parent_rate=0, req-rate=50000000 div=8 [ 15.327977] parent_rate=0, req-rate=50000000 div=6 root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e "xspi0" -e "XSPI_CLK0" -e "divselxspi0" .divselxspi0 1 1 0 800000000 0 0 50000 Y deviceless no_connection_id XSPI_CLK0 1 1 0 50000000 0 0 50000 Y 801c0000.spi spi xspi0 0 1 0 250000000 0 0 50000 N deviceless of_clk_get_from_provider root@rzt2h-evk:~# Case 4# assigned-clock-rates = <37500000>; [ 71.710064] parent_rate=800000000, req-rate=37500000 div=64 [ 71.718567] parent_rate=800000000, req-rate=37500000 div=32 [ 71.725137] parent_rate=600000000, req-rate=37500000 div=16 [ 71.731550] parent_rate=0, req-rate=37500000 div=8 [ 71.740622] parent_rate=0, req-rate=37500000 div=6 [ 71.746376] parent_rate=800000000, req-rate=37500000 div=64 [ 71.752887] parent_rate=800000000, req-rate=37500000 div=32 [ 71.767422] parent_rate=600000000, req-rate=37500000 div=16 [ 71.778671] parent_rate=0, req-rate=37500000 div=8 [ 71.790895] parent_rate=0, req-rate=37500000 div=6 root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e "xspi0" -e "XSPI_CLK0" -e "divselxspi0" .divselxspi0 1 1 0 600000000 0 0 50000 Y deviceless no_connection_id XSPI_CLK0 1 1 0 37500000 0 0 50000 Y 801c0000.spi spi xspi0 0 1 0 250000000 0 0 50000 N deviceless of_clk_get_from_provider root@rzt2h-evk:~# Case 5# assigned-clock-rates = <25000000>; [ 12.411660] parent_rate=800000000, req-rate=25000000 div=64 [ 12.429285] parent_rate=800000000, req-rate=25000000 div=32 [ 12.436144] parent_rate=0, req-rate=25000000 div=16 [ 12.448110] parent_rate=0, req-rate=25000000 div=8 [ 12.458785] parent_rate=0, req-rate=25000000 div=6 [ 12.465401] parent_rate=800000000, req-rate=25000000 div=64 [ 12.482547] parent_rate=800000000, req-rate=25000000 div=32 [ 12.497126] parent_rate=0, req-rate=25000000 div=16 [ 12.509619] parent_rate=0, req-rate=25000000 div=8 [ 12.518212] parent_rate=0, req-rate=25000000 div=6 root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e "xspi0" -e "XSPI_CLK0" -e "divselxspi0" .divselxspi0 1 1 0 800000000 0 0 50000 Y deviceless no_connection_id XSPI_CLK0 1 1 0 25000000 0 0 50000 Y 801c0000.spi spi xspi0 0 1 0 250000000 0 0 50000 N deviceless of_clk_get_from_provider root@rzt2h-evk:~# Case 6# assigned-clock-rates = <12500000>; [ 87.409877] parent_rate=800000000, req-rate=12500000 div=64 [ 87.470663] parent_rate=0, req-rate=12500000 div=32 [ 87.485940] parent_rate=0, req-rate=12500000 div=16 [ 87.492760] parent_rate=0, req-rate=12500000 div=8 [ 87.498313] parent_rate=0, req-rate=12500000 div=6 root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e "xspi0" -e "XSPI_CLK0" -e "divselxspi0" .divselxspi0 1 1 0 800000000 0 0 50000 Y deviceless no_connection_id XSPI_CLK0 1 1 0 12500000 0 0 50000 Y 801c0000.spi spi xspi0 0 1 0 250000000 0 0 50000 N deviceless of_clk_get_from_provider root@rzt2h-evk:~# Looking at the logs I think I could optimize the code to continue when parent_rate == 0 Based on the above logs, would you prefer me to represent it as a composite clock? > > + (clkt->div != 8 && clkt->div != 16)) > > + continue; > > + now = DIV_ROUND_UP_ULL((u64)parent_rate, clkt->div); > > No need to cast to u64 (DIV_ROUND_*_ULL() handle this internally). > > > + if (abs(rate - now) < abs(rate - best)) { > > + div = clkt->div; > > + best = now; > > + req->best_parent_rate = parent_rate; > > + } > > + } > > + > > + if (!div) { > > + u8 maxdiv = 0; > > + > > + req->best_parent_rate = clk_hw_round_rate(req->best_parent_hw, 1); > > + /* > > + * If DIVSELXSPIx is set to 800MHz set the maximum divider > > + * or else fall back to divider of 16 which is a maximum > > + * supported divider for 600MHz operation. > > + */ > > + if (req->best_parent_rate == DIVSELXSPI_RATE_800MHZ) { > > + for (clkt = divider->table; clkt->div; clkt++) { > > + if (clkt->div > maxdiv) > > + maxdiv = clkt->div; > > + } > > + div = maxdiv; > > Why not hardcode the divider, like in the else branch? > Agreed. > > + } else { > > + div = 16; > > + } > > + } > > + > > + req->rate = DIV_ROUND_UP_ULL((u64)req->best_parent_rate, div); > > No need to cast to u64. > Ok. Cheers, Prabhakar ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/5] clk: renesas: r9a09g077: Add xSPI core and module clocks 2025-11-10 21:38 ` Lad, Prabhakar @ 2025-11-17 10:52 ` Lad, Prabhakar 2025-11-17 13:28 ` Geert Uytterhoeven 1 sibling, 0 replies; 14+ messages in thread From: Lad, Prabhakar @ 2025-11-17 10:52 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-renesas-soc, linux-clk, linux-kernel, devicetree, Biju Das, Fabrizio Castro, Lad Prabhakar Hi Geert, On Mon, Nov 10, 2025 at 9:38 PM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > > Hi Geert, > > Thank you for the review. > > On Mon, Nov 10, 2025 at 1:48 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > Hi Prabhakar, > > > > On Tue, 28 Oct 2025 at 17:52, Prabhakar <prabhakar.csengg@gmail.com> wrote: > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > Add core clocks and module clock definitions required by the xSPI > > > (Expanded SPI) IP on the R9A09G077 SoC. > > > > > > Define the new SCKCR fields FSELXSPI0/FSELXSPI1 and DIVSEL_XSPI0/1 and > > > add two new core clocks XSPI_CLK0 and XSPI_CLK1. The xSPI block uses > > > PCLKH as its bus clock (use as module clock parent) while the operation > > > clock (XSPI_CLKn) is derived from PLL4. To support this arrangement > > > provide mux/div selectors and divider tables for the supported > > > XSPI operating rates. > > > > > > Add CLK_TYPE_RZT2H_FSELXSPI to implement a custom divider/mux clock > > > where the determine_rate() callback enforces the hardware constraint: > > > when the parent output is 600MHz only dividers 8 and 16 are valid, > > > whereas for 800MHz operation the full divider set (6,8,16,32,64) may > > > be used. The custom determine_rate() picks the best parent/divider pair > > > to match the requested rate and programs the appropriate SCKCR fields. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > --- > > > v1->v2: > > > - Added custom divider clock type for XSPI clocks to enforce hardware > > > constraints on supported operating rates. > > > > Thanks for the update! > > > > > --- a/drivers/clk/renesas/r9a09g077-cpg.c > > > +++ b/drivers/clk/renesas/r9a09g077-cpg.c > > > > > @@ -54,12 +56,19 @@ > > > #define DIVSCI3ASYNC CONF_PACK(SCKCR3, 12, 2) > > > #define DIVSCI4ASYNC CONF_PACK(SCKCR3, 14, 2) > > > > > > +#define FSELXSPI0 CONF_PACK(SCKCR, 0, 3) > > > +#define FSELXSPI1 CONF_PACK(SCKCR, 8, 3) > > > +#define DIVSEL_XSPI0 CONF_PACK(SCKCR, 6, 1) > > > +#define DIVSEL_XSPI1 CONF_PACK(SCKCR, 14, 1) > > > #define SEL_PLL CONF_PACK(SCKCR, 22, 1) > > > > > > +#define DIVSELXSPI_RATE_600MHZ 600000000UL > > > +#define DIVSELXSPI_RATE_800MHZ 800000000UL > > > > I find it a bit weird that the name of the define includes its value. > > Perhaps just use "600 * MEGA" resp. "800 * MEGA" in the code instead? > OK. > > > But see below... > > > > > @@ -154,6 +180,15 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = { > > > DEF_DIV(".sci5async", CLK_SCI5ASYNC, CLK_PLL4D1, DIVSCI5ASYNC, > > > dtable_24_25_30_32), > > > > > > + DEF_FIXED(".pll4d1_div3", CLK_PLL4D1_DIV3, CLK_PLL4D1, 3, 1), > > > + DEF_FIXED(".pll4d1_div4", CLK_PLL4D1_DIV4, CLK_PLL4D1, 4, 1), > > > > Please move these two just below the existing entry for ".pll4d1". > > > Ok, I will move it below .pll4d1 > > > > + DEF_MUX(".divselxspi0", CLK_DIVSELXSPI0_SCKCR, DIVSEL_XSPI0, > > > + sel_clk_pll4d1_div3_div4, > > > + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0), > > > + DEF_MUX(".divselxspi1", CLK_DIVSELXSPI1_SCKCR, DIVSEL_XSPI1, > > > + sel_clk_pll4d1_div3_div4, > > > + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0), > > > + > > > /* Core output clk */ > > > DEF_DIV("CA55C0", R9A09G077_CLK_CA55C0, CLK_SEL_CLK_PLL0, DIVCA55C0, > > > dtable_1_2), > > > > > @@ -264,6 +305,116 @@ r9a09g077_cpg_mux_clk_register(struct device *dev, > > > return clk_hw->clk; > > > } > > > > > > +static int r9a09g077_cpg_fselxspi_determine_rate(struct clk_hw *hw, > > > + struct clk_rate_request *req) > > > +{ > > > + struct clk_divider *divider = to_clk_divider(hw); > > > + unsigned long parent_rate, best = 0, now; > > > + const struct clk_div_table *clkt; > > > + unsigned long rate = req->rate; > > > + int div = 0; > > > > unsigned int > > > Ok. > > > > + > > > + if (!rate) > > > + rate = 1; > > > + > > > + for (clkt = divider->table; clkt->div; clkt++) { > > > + parent_rate = clk_hw_round_rate(req->best_parent_hw, rate * clkt->div); > > > > I had expected the use of some *_determinate_rate_*() helper, as the > > parent can be changed to find a better clock rate? > > Perhaps you should use a composite clock for that? > > > > > + /* > > > + * DIVSELXSPIx supports 800MHz and 600MHz operation. > > > + * When the parent_rate is 600MHz, only dividers of 8 and 16 > > > + * are supported otherwise dividers of 6, 8, 16, 32, 64 are supported. > > > + * This check ensures that FSELXSPIx is set correctly. > > > + */ > > > + if (parent_rate == DIVSELXSPI_RATE_600MHZ && > > > > Does this actually work as expected? I doubt parent_rate is guaranteed > > to be exactly 600 or 800 MHz, and expect it can differ slightly due > > to rounding. Hence I would look at clk_fixed_factor.div instead. > > > With below diff, Ive got the below results for the various freqs > requested where appropriate parent and divider clocks are picked. > > @@ -317,6 +317,7 @@ static int > r9a09g077_cpg_fselxspi_determine_rate(struct clk_hw *hw, > > for (clkt = divider->table; clkt->div; clkt++) { > parent_rate = clk_hw_round_rate(req->best_parent_hw, > rate * clkt->div); > + pr_err("parent_rate=%lu, req-rate=%lu div=%u\n", > parent_rate, rate, clkt->div); > /* > * DIVSELXSPIx supports 800MHz and 600MHz operation. > * When the parent_rate is 600MHz, only dividers of 8 and 16 > > Logs: > --------- > > Case 0# assigned-clock-rates = <133333334>; > [ 15.419300] parent_rate=800000000, req-rate=133333334 div=64 > [ 15.437698] parent_rate=800000000, req-rate=133333334 div=32 > [ 15.455224] parent_rate=800000000, req-rate=133333334 div=16 > [ 15.501291] parent_rate=800000000, req-rate=133333334 div=8 > [ 15.507801] parent_rate=800000000, req-rate=133333334 div=6 > [ 15.519221] parent_rate=800000000, req-rate=133333334 div=64 > [ 15.525789] parent_rate=800000000, req-rate=133333334 div=32 > [ 15.549625] parent_rate=800000000, req-rate=133333334 div=16 > [ 15.556120] parent_rate=800000000, req-rate=133333334 div=8 > [ 15.564110] parent_rate=800000000, req-rate=133333334 div=6 > > root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e > "xspi0" -e "XSPI_CLK0" -e "divselxspi0" > .divselxspi0 1 1 0 > 800000000 0 0 50000 Y deviceless > no_connection_id > XSPI_CLK0 1 1 0 > 133333334 0 0 50000 Y > 801c0000.spi spi > xspi0 0 1 0 > 250000000 0 0 50000 N deviceless > of_clk_get_from_provider > root@rzt2h-evk:~# > > Case 1# assigned-clock-rates = <100000000>; > [ 15.496291] parent_rate=800000000, req-rate=100000000 div=64 > [ 15.510068] parent_rate=800000000, req-rate=100000000 div=32 > [ 15.517142] parent_rate=800000000, req-rate=100000000 div=16 > [ 15.524047] parent_rate=800000000, req-rate=100000000 div=8 > [ 15.533174] parent_rate=600000000, req-rate=100000000 div=6 > [ 15.540096] parent_rate=800000000, req-rate=100000000 div=64 > [ 15.548135] parent_rate=800000000, req-rate=100000000 div=32 > [ 15.555119] parent_rate=800000000, req-rate=100000000 div=16 > [ 15.562395] parent_rate=800000000, req-rate=100000000 div=8 > [ 15.573521] parent_rate=600000000, req-rate=100000000 div=6 > > root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e > "xspi0" -e "XSPI_CLK0" -e "divselxspi0" > .divselxspi0 1 1 0 > 800000000 0 0 50000 Y deviceless > no_connection_id > XSPI_CLK0 1 1 0 > 100000000 0 0 50000 Y > 801c0000.spi spi > xspi0 0 1 0 > 250000000 0 0 50000 N deviceless > of_clk_get_from_provider > root@rzt2h-evk:~# > > > Case 2# assigned-clock-rates = <75000000>; > [ 12.288507] parent_rate=800000000, req-rate=75000000 div=64 > [ 12.310528] parent_rate=800000000, req-rate=75000000 div=32 > [ 12.318426] parent_rate=800000000, req-rate=75000000 div=16 > [ 12.326361] parent_rate=600000000, req-rate=75000000 div=8 > [ 12.341540] parent_rate=0, req-rate=75000000 div=6 > [ 12.347546] parent_rate=800000000, req-rate=75000000 div=64 > [ 12.357593] parent_rate=800000000, req-rate=75000000 div=32 > [ 12.367148] parent_rate=800000000, req-rate=75000000 div=16 > [ 12.418871] parent_rate=600000000, req-rate=75000000 div=8 > [ 12.433560] parent_rate=0, req-rate=75000000 div=6 > > root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e > "xspi0" -e "XSPI_CLK0" -e "divselxspi0" > .divselxspi0 1 1 0 > 600000000 0 0 50000 Y deviceless > no_connection_id > XSPI_CLK0 1 1 0 > 75000000 0 0 50000 Y > 801c0000.spi spi > xspi0 0 1 0 > 250000000 0 0 50000 N deviceless > of_clk_get_from_provider > root@rzt2h-evk:~# > > Case 3# assigned-clock-rates = <50000000>; > [ 15.240214] parent_rate=800000000, req-rate=50000000 div=64 > [ 15.253498] parent_rate=800000000, req-rate=50000000 div=32 > [ 15.261521] parent_rate=800000000, req-rate=50000000 div=16 > [ 15.272941] parent_rate=0, req-rate=50000000 div=8 > [ 15.280532] parent_rate=0, req-rate=50000000 div=6 > [ 15.289979] parent_rate=800000000, req-rate=50000000 div=64 > [ 15.298745] parent_rate=800000000, req-rate=50000000 div=32 > [ 15.309879] parent_rate=800000000, req-rate=50000000 div=16 > [ 15.319881] parent_rate=0, req-rate=50000000 div=8 > [ 15.327977] parent_rate=0, req-rate=50000000 div=6 > > root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e > "xspi0" -e "XSPI_CLK0" -e "divselxspi0" > .divselxspi0 1 1 0 > 800000000 0 0 50000 Y deviceless > no_connection_id > XSPI_CLK0 1 1 0 > 50000000 0 0 50000 Y > 801c0000.spi spi > xspi0 0 1 0 > 250000000 0 0 50000 N deviceless > of_clk_get_from_provider > root@rzt2h-evk:~# > > > Case 4# assigned-clock-rates = <37500000>; > [ 71.710064] parent_rate=800000000, req-rate=37500000 div=64 > [ 71.718567] parent_rate=800000000, req-rate=37500000 div=32 > [ 71.725137] parent_rate=600000000, req-rate=37500000 div=16 > [ 71.731550] parent_rate=0, req-rate=37500000 div=8 > [ 71.740622] parent_rate=0, req-rate=37500000 div=6 > [ 71.746376] parent_rate=800000000, req-rate=37500000 div=64 > [ 71.752887] parent_rate=800000000, req-rate=37500000 div=32 > [ 71.767422] parent_rate=600000000, req-rate=37500000 div=16 > [ 71.778671] parent_rate=0, req-rate=37500000 div=8 > [ 71.790895] parent_rate=0, req-rate=37500000 div=6 > > root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e > "xspi0" -e "XSPI_CLK0" -e "divselxspi0" > .divselxspi0 1 1 0 > 600000000 0 0 50000 Y deviceless > no_connection_id > XSPI_CLK0 1 1 0 > 37500000 0 0 50000 Y > 801c0000.spi spi > xspi0 0 1 0 > 250000000 0 0 50000 N deviceless > of_clk_get_from_provider > root@rzt2h-evk:~# > > > Case 5# assigned-clock-rates = <25000000>; > [ 12.411660] parent_rate=800000000, req-rate=25000000 div=64 > [ 12.429285] parent_rate=800000000, req-rate=25000000 div=32 > [ 12.436144] parent_rate=0, req-rate=25000000 div=16 > [ 12.448110] parent_rate=0, req-rate=25000000 div=8 > [ 12.458785] parent_rate=0, req-rate=25000000 div=6 > [ 12.465401] parent_rate=800000000, req-rate=25000000 div=64 > [ 12.482547] parent_rate=800000000, req-rate=25000000 div=32 > [ 12.497126] parent_rate=0, req-rate=25000000 div=16 > [ 12.509619] parent_rate=0, req-rate=25000000 div=8 > [ 12.518212] parent_rate=0, req-rate=25000000 div=6 > > root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e > "xspi0" -e "XSPI_CLK0" -e "divselxspi0" > .divselxspi0 1 1 0 > 800000000 0 0 50000 Y deviceless > no_connection_id > XSPI_CLK0 1 1 0 > 25000000 0 0 50000 Y > 801c0000.spi spi > xspi0 0 1 0 > 250000000 0 0 50000 N deviceless > of_clk_get_from_provider > root@rzt2h-evk:~# > > Case 6# assigned-clock-rates = <12500000>; > [ 87.409877] parent_rate=800000000, req-rate=12500000 div=64 > [ 87.470663] parent_rate=0, req-rate=12500000 div=32 > [ 87.485940] parent_rate=0, req-rate=12500000 div=16 > [ 87.492760] parent_rate=0, req-rate=12500000 div=8 > [ 87.498313] parent_rate=0, req-rate=12500000 div=6 > > > root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e > "xspi0" -e "XSPI_CLK0" -e "divselxspi0" > .divselxspi0 1 1 0 > 800000000 0 0 50000 Y deviceless > no_connection_id > XSPI_CLK0 1 1 0 > 12500000 0 0 50000 Y > 801c0000.spi spi > xspi0 0 1 0 > 250000000 0 0 50000 N deviceless > of_clk_get_from_provider > root@rzt2h-evk:~# > > Looking at the logs I think I could optimize the code to continue when > parent_rate == 0 > > Based on the above logs, would you prefer me to represent it as a > composite clock? > Gentle ping. Cheers, Prabhakar > > > + (clkt->div != 8 && clkt->div != 16)) > > > + continue; > > > + now = DIV_ROUND_UP_ULL((u64)parent_rate, clkt->div); > > > > No need to cast to u64 (DIV_ROUND_*_ULL() handle this internally). > > > > > + if (abs(rate - now) < abs(rate - best)) { > > > + div = clkt->div; > > > + best = now; > > > + req->best_parent_rate = parent_rate; > > > + } > > > + } > > > + > > > + if (!div) { > > > + u8 maxdiv = 0; > > > + > > > + req->best_parent_rate = clk_hw_round_rate(req->best_parent_hw, 1); > > > + /* > > > + * If DIVSELXSPIx is set to 800MHz set the maximum divider > > > + * or else fall back to divider of 16 which is a maximum > > > + * supported divider for 600MHz operation. > > > + */ > > > + if (req->best_parent_rate == DIVSELXSPI_RATE_800MHZ) { > > > + for (clkt = divider->table; clkt->div; clkt++) { > > > + if (clkt->div > maxdiv) > > > + maxdiv = clkt->div; > > > + } > > > + div = maxdiv; > > > > Why not hardcode the divider, like in the else branch? > > > Agreed. > > > > + } else { > > > + div = 16; > > > + } > > > + } > > > + > > > + req->rate = DIV_ROUND_UP_ULL((u64)req->best_parent_rate, div); > > > > No need to cast to u64. > > > Ok. > > Cheers, > Prabhakar ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/5] clk: renesas: r9a09g077: Add xSPI core and module clocks 2025-11-10 21:38 ` Lad, Prabhakar 2025-11-17 10:52 ` Lad, Prabhakar @ 2025-11-17 13:28 ` Geert Uytterhoeven 2025-11-17 20:25 ` Lad, Prabhakar 1 sibling, 1 reply; 14+ messages in thread From: Geert Uytterhoeven @ 2025-11-17 13:28 UTC (permalink / raw) To: Lad, Prabhakar Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-renesas-soc, linux-clk, linux-kernel, devicetree, Biju Das, Fabrizio Castro, Lad Prabhakar Hi Prabhakar, On Mon, 10 Nov 2025 at 22:38, Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Mon, Nov 10, 2025 at 1:48 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Tue, 28 Oct 2025 at 17:52, Prabhakar <prabhakar.csengg@gmail.com> wrote: > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > Add core clocks and module clock definitions required by the xSPI > > > (Expanded SPI) IP on the R9A09G077 SoC. > > > > > > Define the new SCKCR fields FSELXSPI0/FSELXSPI1 and DIVSEL_XSPI0/1 and > > > add two new core clocks XSPI_CLK0 and XSPI_CLK1. The xSPI block uses > > > PCLKH as its bus clock (use as module clock parent) while the operation > > > clock (XSPI_CLKn) is derived from PLL4. To support this arrangement > > > provide mux/div selectors and divider tables for the supported > > > XSPI operating rates. > > > > > > Add CLK_TYPE_RZT2H_FSELXSPI to implement a custom divider/mux clock > > > where the determine_rate() callback enforces the hardware constraint: > > > when the parent output is 600MHz only dividers 8 and 16 are valid, > > > whereas for 800MHz operation the full divider set (6,8,16,32,64) may > > > be used. The custom determine_rate() picks the best parent/divider pair > > > to match the requested rate and programs the appropriate SCKCR fields. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > --- > > > v1->v2: > > > - Added custom divider clock type for XSPI clocks to enforce hardware > > > constraints on supported operating rates. > > > --- a/drivers/clk/renesas/r9a09g077-cpg.c > > > +++ b/drivers/clk/renesas/r9a09g077-cpg.c > > > @@ -264,6 +305,116 @@ r9a09g077_cpg_mux_clk_register(struct device *dev, > > > return clk_hw->clk; > > > } > > > > > > +static int r9a09g077_cpg_fselxspi_determine_rate(struct clk_hw *hw, > > > + struct clk_rate_request *req) > > > +{ > > > + struct clk_divider *divider = to_clk_divider(hw); > > > + unsigned long parent_rate, best = 0, now; > > > + const struct clk_div_table *clkt; > > > + unsigned long rate = req->rate; > > > + int div = 0; > > > + > > > + if (!rate) > > > + rate = 1; > > > + > > > + for (clkt = divider->table; clkt->div; clkt++) { > > > + parent_rate = clk_hw_round_rate(req->best_parent_hw, rate * clkt->div); > > > > I had expected the use of some *_determinate_rate_*() helper, as the > > parent can be changed to find a better clock rate? > > Perhaps you should use a composite clock for that? OK, so per your test results, the core clock code does try various parents. > > > > > + /* > > > + * DIVSELXSPIx supports 800MHz and 600MHz operation. > > > + * When the parent_rate is 600MHz, only dividers of 8 and 16 > > > + * are supported otherwise dividers of 6, 8, 16, 32, 64 are supported. > > > + * This check ensures that FSELXSPIx is set correctly. > > > + */ > > > + if (parent_rate == DIVSELXSPI_RATE_600MHZ && > > > > Does this actually work as expected? I doubt parent_rate is guaranteed > > to be exactly 600 or 800 MHz, and expect it can differ slightly due > > to rounding. Hence I would look at clk_fixed_factor.div instead. > > > With below diff, Ive got the below results for the various freqs > requested where appropriate parent and divider clocks are picked. > > @@ -317,6 +317,7 @@ static int > r9a09g077_cpg_fselxspi_determine_rate(struct clk_hw *hw, > > for (clkt = divider->table; clkt->div; clkt++) { > parent_rate = clk_hw_round_rate(req->best_parent_hw, > rate * clkt->div); > + pr_err("parent_rate=%lu, req-rate=%lu div=%u\n", > parent_rate, rate, clkt->div); > /* > * DIVSELXSPIx supports 800MHz and 600MHz operation. > * When the parent_rate is 600MHz, only dividers of 8 and 16 > Case 2# assigned-clock-rates = <75000000>; > [ 12.288507] parent_rate=800000000, req-rate=75000000 div=64 > [ 12.310528] parent_rate=800000000, req-rate=75000000 div=32 > [ 12.318426] parent_rate=800000000, req-rate=75000000 div=16 > [ 12.326361] parent_rate=600000000, req-rate=75000000 div=8 > [ 12.341540] parent_rate=0, req-rate=75000000 div=6 > [ 12.347546] parent_rate=800000000, req-rate=75000000 div=64 > [ 12.357593] parent_rate=800000000, req-rate=75000000 div=32 > [ 12.367148] parent_rate=800000000, req-rate=75000000 div=16 > [ 12.418871] parent_rate=600000000, req-rate=75000000 div=8 > [ 12.433560] parent_rate=0, req-rate=75000000 div=6 [...] Thanks for checking! > Looking at the logs I think I could optimize the code to continue when > parent_rate == 0 Do you know why it gets called with parent_rate == 0? > Based on the above logs, would you prefer me to represent it as a > composite clock? Given the core code does try the various parent clocks, there is no need to model it as a composite clock. However, I still think you should look at the parent's divider value (clk_fixed_factor.div) instead of at the actual clock rate, as that may not be 600 or 800 MHz exactly (e.g. when underclocking the SoC on a custom board using a 24 instead of a 25 MHz crystal). 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] 14+ messages in thread
* Re: [PATCH v2 5/5] clk: renesas: r9a09g077: Add xSPI core and module clocks 2025-11-17 13:28 ` Geert Uytterhoeven @ 2025-11-17 20:25 ` Lad, Prabhakar 0 siblings, 0 replies; 14+ messages in thread From: Lad, Prabhakar @ 2025-11-17 20:25 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-renesas-soc, linux-clk, linux-kernel, devicetree, Biju Das, Fabrizio Castro, Lad Prabhakar Hi Geert, On Mon, Nov 17, 2025 at 1:32 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Mon, 10 Nov 2025 at 22:38, Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > > On Mon, Nov 10, 2025 at 1:48 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Tue, 28 Oct 2025 at 17:52, Prabhakar <prabhakar.csengg@gmail.com> wrote: > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > > > Add core clocks and module clock definitions required by the xSPI > > > > (Expanded SPI) IP on the R9A09G077 SoC. > > > > > > > > Define the new SCKCR fields FSELXSPI0/FSELXSPI1 and DIVSEL_XSPI0/1 and > > > > add two new core clocks XSPI_CLK0 and XSPI_CLK1. The xSPI block uses > > > > PCLKH as its bus clock (use as module clock parent) while the operation > > > > clock (XSPI_CLKn) is derived from PLL4. To support this arrangement > > > > provide mux/div selectors and divider tables for the supported > > > > XSPI operating rates. > > > > > > > > Add CLK_TYPE_RZT2H_FSELXSPI to implement a custom divider/mux clock > > > > where the determine_rate() callback enforces the hardware constraint: > > > > when the parent output is 600MHz only dividers 8 and 16 are valid, > > > > whereas for 800MHz operation the full divider set (6,8,16,32,64) may > > > > be used. The custom determine_rate() picks the best parent/divider pair > > > > to match the requested rate and programs the appropriate SCKCR fields. > > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > --- > > > > v1->v2: > > > > - Added custom divider clock type for XSPI clocks to enforce hardware > > > > constraints on supported operating rates. > > > > > --- a/drivers/clk/renesas/r9a09g077-cpg.c > > > > +++ b/drivers/clk/renesas/r9a09g077-cpg.c > > > > > @@ -264,6 +305,116 @@ r9a09g077_cpg_mux_clk_register(struct device *dev, > > > > return clk_hw->clk; > > > > } > > > > > > > > +static int r9a09g077_cpg_fselxspi_determine_rate(struct clk_hw *hw, > > > > + struct clk_rate_request *req) > > > > +{ > > > > + struct clk_divider *divider = to_clk_divider(hw); > > > > + unsigned long parent_rate, best = 0, now; > > > > + const struct clk_div_table *clkt; > > > > + unsigned long rate = req->rate; > > > > + int div = 0; > > > > + > > > > + if (!rate) > > > > + rate = 1; > > > > + > > > > + for (clkt = divider->table; clkt->div; clkt++) { > > > > + parent_rate = clk_hw_round_rate(req->best_parent_hw, rate * clkt->div); > > > > > > I had expected the use of some *_determinate_rate_*() helper, as the > > > parent can be changed to find a better clock rate? > > > Perhaps you should use a composite clock for that? > > OK, so per your test results, the core clock code does try > various parents. > > > > > > > > + /* > > > > + * DIVSELXSPIx supports 800MHz and 600MHz operation. > > > > + * When the parent_rate is 600MHz, only dividers of 8 and 16 > > > > + * are supported otherwise dividers of 6, 8, 16, 32, 64 are supported. > > > > + * This check ensures that FSELXSPIx is set correctly. > > > > + */ > > > > + if (parent_rate == DIVSELXSPI_RATE_600MHZ && > > > > > > Does this actually work as expected? I doubt parent_rate is guaranteed > > > to be exactly 600 or 800 MHz, and expect it can differ slightly due > > > to rounding. Hence I would look at clk_fixed_factor.div instead. > > > > > With below diff, Ive got the below results for the various freqs > > requested where appropriate parent and divider clocks are picked. > > > > @@ -317,6 +317,7 @@ static int > > r9a09g077_cpg_fselxspi_determine_rate(struct clk_hw *hw, > > > > for (clkt = divider->table; clkt->div; clkt++) { > > parent_rate = clk_hw_round_rate(req->best_parent_hw, > > rate * clkt->div); > > + pr_err("parent_rate=%lu, req-rate=%lu div=%u\n", > > parent_rate, rate, clkt->div); > > /* > > * DIVSELXSPIx supports 800MHz and 600MHz operation. > > * When the parent_rate is 600MHz, only dividers of 8 and 16 > > > Case 2# assigned-clock-rates = <75000000>; > > [ 12.288507] parent_rate=800000000, req-rate=75000000 div=64 > > [ 12.310528] parent_rate=800000000, req-rate=75000000 div=32 > > [ 12.318426] parent_rate=800000000, req-rate=75000000 div=16 > > [ 12.326361] parent_rate=600000000, req-rate=75000000 div=8 > > [ 12.341540] parent_rate=0, req-rate=75000000 div=6 > > [ 12.347546] parent_rate=800000000, req-rate=75000000 div=64 > > [ 12.357593] parent_rate=800000000, req-rate=75000000 div=32 > > [ 12.367148] parent_rate=800000000, req-rate=75000000 div=16 > > [ 12.418871] parent_rate=600000000, req-rate=75000000 div=8 > > [ 12.433560] parent_rate=0, req-rate=75000000 div=6 > [...] > > Thanks for checking! > > > Looking at the logs I think I could optimize the code to continue when > > parent_rate == 0 > > Do you know why it gets called with parent_rate == 0? > When it doesnt find the best parent, parent_rate == 0. > > Based on the above logs, would you prefer me to represent it as a > > composite clock? > > Given the core code does try the various parent clocks, there is > no need to model it as a composite clock. > Ok. > However, I still think you should look at the parent's divider value > (clk_fixed_factor.div) instead of at the actual clock rate, as that > may not be 600 or 800 MHz exactly (e.g. when underclocking the SoC > on a custom board using a 24 instead of a 25 MHz crystal). > Ok, I will have to iterate over the parents to determine the divider value. Cheers, Prabhakar ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-11-17 20:25 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-28 16:51 [PATCH v2 0/5] Add XSPI clocks and improve divider clock handling for R9A09G077 SoC Prabhakar 2025-10-28 16:51 ` [PATCH v2 1/5] clk: renesas: r9a09g077: Propagate rate changes to parent clocks Prabhakar 2025-11-10 13:18 ` Geert Uytterhoeven 2025-10-28 16:51 ` [PATCH v2 2/5] clk: renesas: r9a09g077: Remove stray blank line Prabhakar 2025-11-10 13:18 ` Geert Uytterhoeven 2025-10-28 16:51 ` [PATCH v2 3/5] clk: renesas: r9a09g077: Use devm_ helpers for divider clock registration Prabhakar 2025-11-10 13:19 ` Geert Uytterhoeven 2025-10-28 16:51 ` [PATCH v2 4/5] dt-bindings: clock: renesas,r9a09g077/87: Add XSPI0/1 IDs Prabhakar 2025-10-28 16:51 ` [PATCH v2 5/5] clk: renesas: r9a09g077: Add xSPI core and module clocks Prabhakar 2025-11-10 13:48 ` Geert Uytterhoeven 2025-11-10 21:38 ` Lad, Prabhakar 2025-11-17 10:52 ` Lad, Prabhakar 2025-11-17 13:28 ` Geert Uytterhoeven 2025-11-17 20:25 ` Lad, Prabhakar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).