Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH 1/9] clk: renesas: rzg2l: Add FOUTPOSTDIV clk support
>
> Hi Biju,
>
> On Fri, Mar 18, 2022 at 6:51 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > PLL5 generates FOUTPOSTDIV clk and is sourced by LCDC/DSI modules.
> > The FOUTPOSTDIV is connected to PLL5_4 MUX. Video clock is sourced
> > from DSI divider which is connected to PLL5_4 MUX.
> >
> > This patch adds support for generating FOUTPOSTDIV clk.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > RFC->V1:
> > * Removed LUT.
> > * Replaced magic numbers with macros.
>
> Thanks for the update!
>
> > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > +++ b/drivers/clk/renesas/rzg2l-cpg.c
>
> > @@ -266,6 +278,131 @@ rzg2l_cpg_sd_mux_clk_register(const struct
> cpg_core_clk *core,
> > return clk_hw->clk;
> > }
> >
> > +struct sipll5 {
> > + struct clk_hw hw;
> > + u32 conf;
> > + struct rzg2l_cpg_priv *priv;
> > +};
> > +
> > +#define to_sipll5(_hw) container_of(_hw, struct sipll5, hw)
> > +
> > +static unsigned long rzg2l_cpg_sipll5_recalc_rate(struct clk_hw *hw,
> > + unsigned long
> > +parent_rate) {
> > + struct sipll5 *sipll5 = to_sipll5(hw);
> > + struct rzg2l_cpg_priv *priv = sipll5->priv;
> > +
> > + return priv->pll5_params.frequency; }
> > +
> > +static long rzg2l_cpg_sipll5_round_rate(struct clk_hw *hw,
> > + unsigned long rate,
> > + unsigned long *parent_rate) {
> > + struct sipll5 *sipll5 = to_sipll5(hw);
> > + struct rzg2l_cpg_priv *priv = sipll5->priv;
> > +
> > + return priv->pll5_params.frequency; }
> > +
> > +static int rzg2l_cpg_sipll5_set_rate(struct clk_hw *hw,
> > + unsigned long rate,
> > + unsigned long parent_rate) {
>
> The above 3 functions all ignore their input rates and parent_rates, as you
> rely on setting up pll5_params in the DSI divider (patch 3), and the clock
> core propagating up to all parents (PLL5_4 in patch 2, and FOUTPOSDIV
> here), right?
Yes, that is correct, PLL5 parameters are calculated based on video clock in DSI divider(patch3)
And propagating up to here and setting parent's clkrates based on that.
>
> I think it would help to document that somewhere.
OK Will Document this
Cheers,
Biju
>
> The rest LGTM, so
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
>
> 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
