Hi Prabhakar,
> -----Original Message-----
> From: Lad, Prabhakar <prabhakar.csengg@xxxxxxxxx>
> Sent: 17 June 2025 22:05
> Subject: Re: [PATCH v3 1/9] dt-bindings: memory: Document RZ/G3E support
>
> Hi Geert,
>
> On Mon, Mar 31, 2025 at 7:25 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> >
> > Hi Biju,
> >
> > On Mon, 31 Mar 2025 at 17:33, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> On Mon, 31 Mar
> > > > 2025 at 16:34, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > > > > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> On Mon, 31 Mar
> > > > > > 2025 at 15:54, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > > > > > > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> Document
> > > > > > > > support for the Expanded Serial Peripheral Interface
> > > > > > > > (xSPI) Controller in the Renesas RZ/G3E
> > > > > > > > (R9A09G047) SoC.
> > > > > > > >
> > > > > > > > Reviewed-by: Rob Herring (Arm) <robh@xxxxxxxxxx>
> > > > > > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > > >
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/Documentation/devicetree/bindings/memory-controllers
> > > > > > > > +++ /renes
> > > > > > > > +++ as,r
> > > > > > > > +++ zg3e
> > > > > > > > +++ -xspi.yaml
> > > > > >
> > > > > > > > + spi@11030000 {
> > > > > > > > + compatible = "renesas,r9a09g047-xspi";
> > > > > > > > + reg = <0x11030000 0x10000>, <0x20000000 0x10000000>;
> > > > > > > > + reg-names = "regs", "dirmap";
> > > > > > > > + interrupts = <GIC_SPI 228 IRQ_TYPE_EDGE_RISING>,
> > > > > > > > + <GIC_SPI 229 IRQ_TYPE_EDGE_RISING>;
> > > > > > > > + interrupt-names = "pulse", "err_pulse";
> > > > > > > > + clocks = <&cpg CPG_MOD 0x9f>, <&cpg CPG_MOD 0xa0>,
> > > > > > > > + <&cpg CPG_MOD 0xa1>, <&cpg CPG_MOD
> > > > > > > > + 0xa1>;
> > > > > > >
> > > > > > > On the next version I am going to update spix2 clk as <&cpg
> > > > > > > CPG_CORE R9A09G047_SPI_CLK_SPIX2>
> >
> > According to the RZ/G3E clock system diagram, (the parent of) clk_spi
> > is derived from (the parent of) clk_spix2, not the other way around?
> > So you can model clk_spi as a fixed divider clock with parent
> > clk_spix2 and factor two. I.e. provide a new core clock
> > R9A09G047_SPI_CLK_SPI instead of your proposed R9A09G047_SPI_CLK_SPIX2?
> >
> With this approach when R9A09G047_SPI_CLK_SPI is used as a core clock and XSPI node is disabled the
> clk_summary reports the core clock is ON (while it's actually OFF).
>
> root@rzv2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep spi
> .smux2_xspi_clk1 0 0 0
> 320000000 0 0 50000 Y deviceless
> no_connection_id
> .pllcm33_xspi 0 0 0
> 40000000 0 0 50000 Y deviceless
> no_connection_id
> spi_clk_spix2 0 0 0
> 40000000 0 0 50000 N deviceless
> no_connection_id
> spi_clk_spi 0 0 0
> 20000000 0 0 50000 Y deviceless
> no_connection_id
> spi_aclk 0 0 0
> 200000000 0 0 50000 N deviceless
> no_connection_id
> spi_hclk 0 0 0
> 200000000 0 0 50000 N deviceless
> no_connection_id
> .smux2_xspi_clk0 0 0 0
> 533333333 0 0 50000 Y deviceless
> no_connection_id
>
> Can we maybe use a unused ON index and ON bit for example 25, 0 (ie
> 0x190) and represent this is a module clock for example for the
> spi_clk_spix2 clock and use this in the DT and let the CPG core code handle such turning ON/OF the
> module clocks based on the enable count which will be handled internally in the driver?
>
> I have some POC code for the paired clocks which will handle the enable count of the paired module
> clocks, below is the diff. Please share your thoughts.
>
> diff --git a/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
> b/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
> index 3c5984ee27ca..8a8f59ffdb4c 100644
> --- a/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
> @@ -290,10 +290,10 @@ xspi: spi@11030000 {
> interrupt-names = "pulse", "err_pulse";
> clocks = <&cpg CPG_MOD 0x9f>,
> <&cpg CPG_MOD 0xa0>,
> - <&cpg CPG_CORE R9A09G057_SPI_CLK_SPI>,
> - <&cpg CPG_MOD 0xa1>;
> + <&cpg CPG_MOD 0xa1>,
> + <&cpg CPG_MOD 0x190>;
Does this dummy index need to be documented to be in bindings?
> clock-names = "ahb", "axi", "spi", "spix2";
> - assigned-clocks = <&cpg CPG_CORE R9A09G057_SPI_CLK_SPI>;
> + assigned-clocks = <&cpg CPG_MOD 0xa1>;
> assigned-clock-rates = <133333334>;
> resets = <&cpg 0xa3>, <&cpg 0xa4>;
> reset-names = "hresetn", "aresetn"; diff --git
> a/drivers/clk/renesas/r9a09g057-cpg.c
> b/drivers/clk/renesas/r9a09g057-cpg.c
> index 9952474bcf48..d5eef17ad5fc 100644
> --- a/drivers/clk/renesas/r9a09g057-cpg.c
> +++ b/drivers/clk/renesas/r9a09g057-cpg.c
> @@ -43,6 +43,7 @@ enum clk_ids {
> CLK_SMUX2_XSPI_CLK0,
> CLK_SMUX2_XSPI_CLK1,
> CLK_PLLCM33_XSPI,
> + CLK_PLLCM33_XSPIX2,
> CLK_PLLCLN_DIV2,
> CLK_PLLCLN_DIV8,
> CLK_PLLCLN_DIV16,
> @@ -176,6 +177,7 @@ static const struct cpg_core_clk r9a09g057_core_clks[] __initconst = {
> DEF_SMUX(".smux2_xspi_clk1", CLK_SMUX2_XSPI_CLK1, SSEL1_SELCTL3, smux2_xspi_clk1),
> DEF_CSDIV(".pllcm33_xspi", CLK_PLLCM33_XSPI, CLK_SMUX2_XSPI_CLK1, CSDIV0_DIVCTL3,
> dtable_2_16),
> + DEF_FIXED(".pllcm33_xspix2", CLK_PLLCM33_XSPIX2,
> CLK_PLLCM33_XSPI, 2, 1),
Will this internal core clk shows same status like core clk? For eg: XSPI node is disabled the
clk_summary reports this internal core clock is ON (while it's actually OFF)?
>
> DEF_FIXED(".pllcln_div2", CLK_PLLCLN_DIV2, CLK_PLLCLN, 1, 2),
> DEF_FIXED(".pllcln_div8", CLK_PLLCLN_DIV8, CLK_PLLCLN, 1, 8), @@ -231,7 +233,6 @@ static const
> struct cpg_core_clk r9a09g057_core_clks[] __initconst = {
> CLK_PLLETH_DIV_125_FIX, 1, 1),
> DEF_FIXED("gbeth_1_clk_ptp_ref_i", R9A09G057_GBETH_1_CLK_PTP_REF_I,
> CLK_PLLETH_DIV_125_FIX, 1, 1),
> - DEF_FIXED("spi_clk_spi", R9A09G057_SPI_CLK_SPI, CLK_PLLCM33_XSPI, 1, 2),
> };
>
> static const struct rzv2h_mod_clk r9a09g057_mod_clks[] __initconst = { @@ -311,8 +312,10 @@ static
> const struct rzv2h_mod_clk r9a09g057_mod_clks[] __initconst = {
> BUS_MSTOP(4, BIT(5))),
> DEF_MOD("spi_aclk", CLK_PLLCM33_GEAR, 10, 0, 5, 0,
> BUS_MSTOP(4, BIT(5))),
> - DEF_MOD_NO_PM("spi_clk_spix2", CLK_PLLCM33_XSPI, 10, 1, 5, 2,
> + DEF_MOD_PAIRED("spi_clk_spi", CLK_PLLCM33_XSPI, 10, 1, 5, 1,
> BUS_MSTOP(4, BIT(5))),
> + DEF_MOD_PAIRED_ALIAS("spi_clk_spix2", CLK_PLLCM33_XSPIX2, 25, 0, 5, 2,
> + BUS_MSTOP(4, BIT(5)),
> + 10, 1),
> DEF_MOD("sdhi_0_imclk", CLK_PLLCLN_DIV8, 10, 3, 5, 3,
> BUS_MSTOP(8, BIT(2))),
> DEF_MOD("sdhi_0_imclk2", CLK_PLLCLN_DIV8, 10, 4, 5, 4,
> @@ -508,7 +511,7 @@ const struct rzv2h_cpg_info r9a09g057_cpg_info __initconst = {
> /* Module Clocks */
> .mod_clks = r9a09g057_mod_clks,
> .num_mod_clks = ARRAY_SIZE(r9a09g057_mod_clks),
> - .num_hw_mod_clks = 25 * 16,
> + .num_hw_mod_clks = 26 * 16,
.num_hw_mod_clks = 25 * 16 + 1,??
>
> /* Resets */
> .resets = r9a09g057_resets,
> diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c index
> 97bcd252fcbf..847ea71df865 100644
> --- a/drivers/clk/renesas/rzv2h-cpg.c
> +++ b/drivers/clk/renesas/rzv2h-cpg.c
> @@ -71,6 +71,12 @@
>
> #define CPG_CLKSTATUS0 (0x700)
>
> +struct rzv2h_paired_clock {
> + u8 on_index;
> + u8 on_bit;
> + unsigned int enabled_count;
refcount_t ??
Cheers,
Biju
