Hi Geert,
> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 31 March 2025 19:24
> Subject: Re: [PATCH v3 1/9] dt-bindings: memory: Document RZ/G3E support
>
> 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/r
> > > > > > > +++ enes
> > > > > > > +++ 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?
>
> > > > > What's spix2 clk? Ah, re-adding dropped line:
> > > > >
> > > > > > > + clock-names = "ahb", "axi", "spi", "spix2";
> > > > >
> > > > > > Based on [1], the clk specifier cannot distinguish between spi
> > > > > > and
> > > > > > spix2 clk, as entries are same(gating bits). So, treating
> > > > > > spix2 as core clock to distinguish them.
> > > > > >
> > > > > > Please let me know if there are any issues in this approach?
> > > > >
> > > > > As you wrote in [2], you have to check the two monitor register
> > > > > bits together. How do you plan to handle that requirement?
> > > >
> > > > As per hardware team, spix2 clock is twice the frequency of the
> > > > spi clock, and the clock ON/OFF period displayed for each bit in
> > > > the monitor register varies slightly
> > > due to the difference in frequency.
> > > >
> > > > So, if I monitor the bit of slowest clock(spi) that will confirm both right?
> > >
> > > (perhaps naively) I would assume so, too.
> > >
> > > Bute then why did you (or the hardware team) write:
> > >
> > > "So to check the status after changing the clock ON/OFF register setting,
> > > please check the two monitor register bits together".
> >
> > Basically, It is feedback from hardware team.
> >
> > <snippet>
> > There is no use case in which each bit in the monitor register is used
> > independently, so as you pointed out, I think it would have been
> > better to bundle them into one bit, like the clock ON/OFF register.
> > Note that the spix2 clock is twice the frequency of the spi clock,
>
> OK, so one bit would have been fine...
>
> > and the clock ON/OFF period displayed for each bit in the monitor register varies slightly due to
> the difference in frequency.
> > To check the status after changing the clock ON/OFF register setting, please check the two monitor
> register bits together.
>
> ... except that this part says to check both.
Can you please share your thoughts to handle this?
1) Gate only spi clk
2) For monitoring use both clock
3) Clock specifier needs two distinct entries. So that consumer will get
proper rates for both clocks.
Cheers,
Biju
