Hi Biju,
On Wed, Jun 18, 2025 at 7:21 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
>
> 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
<snip>
> > 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?
>
Yes, indeed we will have to.
> > 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)?
>
The internal core clock state will show as always ON.
>
> >
> > 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,??
Agreed.
>
> >
> > /* 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 ??
>
Agreed.
Cheers,
Prabhakar
