RE: [PATCH 1/4] clk: renesas: rzv2h-cpg: Add support for coupled clock

[Stephen Boyd <sboyd@kernel.org>]

Quoting Biju Das (2025-03-21 07:21:24)
> > -----Original Message-----
> > From: Stephen Boyd <sboyd@kernel.org>
> > > > > >
> > > > > > The parent clock rate of spi and spix2 are different. If we use
> > > > > > an intermediate parent clk, What clk rate the parent will use??
> > > > >
> > > > > Alright, got it. Does the consumer care about the difference
> > > > > between the two clks for the gating
> > > > part?
> > > >
> > > > Although gating bit is same, for some reason their monitor bit is
> > > > different. So, to confirm clk on status we need to check respective
> > > > monitor bits. Parallelly, I will check with hardware team, does it need to monitor both these
> > bits??
> > >
> > > According to hardware team, the 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 to check the status after changing the clock ON/OFF register
> > > setting, please check the two monitor register bits together
> > >
> > 
> > That answers the hardware side of the question. Why does software need to care that they're two
> > different things vs. one clk?
> 
> From software point, Consumer driver bother only about spi_clk.
> 
> So, treating as one clk(spi_clk) should be OK and we should drop
> handling spi_x2 module clk in the clk driver instead treat this as an internal clock
> (".spi_clk_x2")??
> 
> Then we should update the binding to have only 3 module clks instead of 4 by dropping
> the spi_x2 module clk.

I don't see why the binding has to be updated. Can't we return a NULL
clk pointer when the driver calls clk_get() on the specifier for the
spi_x2 clk? Then nothing will happen for that clk. I guess we may need
to return the rate of the spi clk multiplied by 2 or something, but that
is far simpler to implement than arbitrating the hardware with custom
logic and meets the same result.

> 
> Geert, what is your opinion on this?
> 
> Example:
>         DEF_SDIV(".spi_clk_x2", CLK_PLLCM33_XSPI, CLK_SMUX2_XSPI_CLK1, CSDIV0_DIVCTL3,
>                  dtable_2_16),
>         DEF_FIXED(".pllcm33_xspi_div2", CLK_PLLCM33_XSPI_DIV2, CLK_PLLCM33_XSPI, 1, 2),
> 
> 
>         DEF_MOD("spi_clk_spi",                CLK_PLLCM33_XSPI_DIV2, 10, 1, 5, 1,
>                                                 BUS_MSTOP(4, BIT(5))),
> 
> Note:
> Currently I am facing an issue which is popped up using single clock,
> If I use spi clock for rpm, then flash write is failing. If it is turned
> on permanently then there is no issue.