RE: [PATCH v14 09/11] clk: realtek: Add RTD1625-CRT clock controller driver

[Yu-Chun Lin [林祐君]]

> > Hi Jerome,
> >
> >> > +
> >> > +static const char * const clk_gpu_parents[] = {"pll_gpu",
> >> > +"clk_sys"}; static RTK_CLK_REGMAP_MUX(clk_gpu, clk_gpu_parents,
> >> CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT,
> >> > +                       0x28, 12, 0x1); static const char * const
> >> > +clk_ve_parents[] = {"pll_vo", "clk_sysh", "pll_ve1", "pll_ve2"};
> >> > +static RTK_CLK_REGMAP_MUX(clk_ve1, clk_ve_parents,
> >> CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT,
> >> > +                       0x4c, 0, 0x3); static
> >> > +RTK_CLK_REGMAP_MUX(clk_ve2, clk_ve_parents,
> CLK_SET_RATE_PARENT |
> >> CLK_SET_RATE_NO_REPARENT,
> >> > +                       0x4c, 3, 0x3); static
> >> > +RTK_CLK_REGMAP_MUX(clk_ve4, clk_ve_parents,
> CLK_SET_RATE_PARENT |
> >> CLK_SET_RATE_NO_REPARENT,
> >> > +                       0x4c, 6, 0x3); static
> >> > +RTK_CLK_REGMAP_GATE_NO_PARENT(clk_en_misc, CLK_IS_CRITICAL,
> 0x50,
> >> 0,
> >> > +1); clk_en_pcie0, 0, 0x50, 2,
> >> > +1); clk_en_gspi, 0, 0x50, 6, 1);
> >> > +clk_en_iso_misc, 0, 0x50, 10,
> >> > +1); clk_en_sds, 0, 0x50, 12, 1);
> >> > +clk_en_hdmi, 0, 0x50, 14, 1);
> >>
> >> This is a lot of clock with no parents which is a bit suspicious
> >> especially for gates.
> >> What is really feeding those ?
> >>
> >
> > In v15, we will do our best to reduce the number of gate clocks without a
> parent.
> >
> > However, after discussing with our colleague, we confirmed that for
> > some of these NO_PARENT gate clocks, their actual upstream clocks
> > (like PLLs and
> > Muxes) are located in separate, independent hardware subsystems. The
> > registers controlling those upstream clocks are physically
> > inaccessible from the Linux CPU domain.
> 
> Control and input are different things. It feels like your DT (binding) description
> is not complete as it stands which will eventually be a problem.
> 
> A clock with no input is something that does generate the clock by itself, like an
> osciallator. Pretending it has no parent because you don't know how describe it
> yet is wrong.
> 

Thanks for your explanation.

In v15, there will be no more NO_PARENT for gates. For example,
mipi_csi -> clk_sysh, and all other gates -> clk_sys.

Additionally, we will properly model the firmware-managed PLLs backed by
real hardware registers. We will replace the fixed 351 MHz pll_vodma with
a read-only MNO PLL.

This allows the CCF to correctly derive the frequency from the register
(e.g., when acting as a parent for muxes), while guaranteeing the kernel
will never reprogram or disable the boot code's settings.

Best regards,
Yu-Chun