Re: [PATCH 1/4] clk: renesas: rzv2h-cpg: Use per-SoC PLL reference frequency for calculations

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Fri, Jun 5, 2026 at 2:37 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> On Mon, 11 May 2026 at 21:19, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > Use a per-SoC PLL reference input frequency for PLL parameter
> > calculations instead of relying on the hardcoded 24MHz constant.
> >
> > Add an input_fref field to struct rzv2h_pll_limits and derive the PLL
> > reference frequency from it in rzv2h_get_pll_pars(). Fall back to the
> > existing 24MHz value when no SoC-specific input is provided.
> >
> > This allows the existing PLL divider calculation logic to be reused
> > unchanged on SoCs such as RZ/T2H, which use a 48MHz PLL reference
> > input instead of the 24MHz reference used on RZ/V2H(P), while keeping
> > current RZ/V2H(P) behaviour intact.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/drivers/clk/renesas/rzv2h-cpg.c
> > +++ b/drivers/clk/renesas/rzv2h-cpg.c
> > @@ -242,6 +242,7 @@ struct rzv2h_plldsi_div_clk {
> >  bool rzv2h_get_pll_pars(const struct rzv2h_pll_limits *limits,
> >                         struct rzv2h_pll_pars *pars, u64 freq_millihz)
> >  {
> > +       unsigned long input_fref = limits->input_fref ?: RZ_V2H_OSC_CLK_IN_MEGA;
> >         u64 fout_min_millihz = mul_u32_u32(limits->fout.min, MILLI);
> >         u64 fout_max_millihz = mul_u32_u32(limits->fout.max, MILLI);
> >         struct rzv2h_pll_pars p, best;
>
> > --- a/include/linux/clk/renesas.h
> > +++ b/include/linux/clk/renesas.h
> > @@ -53,6 +53,8 @@ static inline void rzg2l_cpg_dsi_div_set_divider(u8 divider, int target) { }
> >   * various parameters used to configure a PLL. These limits ensure
> >   * the PLL operates within valid and stable ranges.
> >   *
> > + * @input_fref: Reference input frequency to the PLL (in MHz)
>
> Iff there is a default, it should be documented here?
>
Agreed.

> > + *
> >   * @fout: Output frequency range (in MHz)
> >   * @fout.min: Minimum allowed output frequency
> >   * @fout.max: Maximum allowed output frequency
> > @@ -78,6 +80,8 @@ static inline void rzg2l_cpg_dsi_div_set_divider(u8 divider, int target) { }
> >   * @k.max: Maximum delta-sigma value
> >   */
> >  struct rzv2h_pll_limits {
> > +       u32 input_fref;
> > +
> >         struct {
> >                 u32 min;
> >                 u32 max;
> > @@ -156,6 +160,7 @@ struct rzv2h_pll_div_pars {
> >
> >  #define RZV2H_CPG_PLL_DSI_LIMITS(name)                                 \
> >         static const struct rzv2h_pll_limits (name) = {                 \
> > +               .input_fref = 24 * MEGA,                                \
>
> Why add this if 24 * MEGA is the default value anyway?
> Why not do the same for the two similar RZ/G3E macros?
> Perhaps the default handling and the RZ_V2H_OSC_CLK_IN_MEGA macro
> should just be dropped?
>
Agreed. I will drop this change and get rid of RZ_V2H_OSC_CLK_IN_MEGA mcro.

Cheers,
Prabhakar

> >                 .fout = { .min = 25 * MEGA, .max = 375 * MEGA },        \
> >                 .fvco = { .min = 1600 * MEGA, .max = 3200 * MEGA },     \
> >                 .m = { .min = 64, .max = 533 },                         \
>
> 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