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?
> > > > 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.
> </snippet>
Oh well...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
