Re: [PATCH v3 4/5] clk: renesas: rzv2h-cpg: Extract PLL calculation math into a library

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Wed, Jun 17, 2026 at 11:05 AM Geert Uytterhoeven
<geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> On Mon, 15 Jun 2026 at 12:48, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > Move the common PLL and divider parameter calculation logic from the
> > core rzv2h-cpg driver into a standalone library file.
> >
> > Introduce the CLK_RZV2H_CPG_LIB Kconfig configuration symbol and create
> > rzv2h-cpg-lib.c to house rzv2h_cpg_get_pll_pars() and
> > rzv2h_cpg_get_pll_divs_pars().
> >
> > Keep rzv2h_get_pll_pars() and rzv2h_get_pll_divs_pars() in the original
> > driver as wrappers that call into the new library helper endpoints.
> > These wrappers are maintained for this cycle because they are actively
> > referenced by the DSI driver; they will be safely removed in a subsequent
> > cycle once the DSI driver is updated to use the new APIs from the library,
> > preventing cross-subsystem build breakages.
> >
> > This restructuring allows other Renesas SoC clock drivers, such as the
> > upcoming RZ/T2H and RZ/N2H platforms that utilize similar LCDC clock
> > divider mathematical logic, to share the iterative calculation helper
> > infrastructure without duplication.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/include/linux/clk/renesas.h
> > +++ b/include/linux/clk/renesas.h
> > @@ -213,4 +213,27 @@ static inline bool rzv2h_get_pll_divs_pars(const struct rzv2h_pll_limits *limits
> >  }
> >  #endif
> >
> > +#ifdef CONFIG_CLK_RZV2H_CPG_LIB
> > +bool rzv2h_cpg_get_pll_pars(const struct rzv2h_pll_limits *limits,
> > +                           struct rzv2h_pll_pars *pars, u64 freq_millihz);
> > +
> > +bool rzv2h_cpg_get_pll_divs_pars(const struct rzv2h_pll_limits *limits,
> > +                                struct rzv2h_pll_div_pars *pars,
> > +                                const u8 *table, u8 table_size, u64 freq_millihz);
> > +#else
> > +static inline bool rzv2h_cpg_get_pll_pars(const struct rzv2h_pll_limits *limits,
> > +                                         struct rzv2h_pll_pars *pars,
> > +                                         u64 freq_millihz)
> > +{
> > +       return false;
> > +}
> > +
> > +static inline bool rzv2h_cpg_get_pll_divs_pars(const struct rzv2h_pll_limits *limits,
> > +                                              struct rzv2h_pll_div_pars *pars,
> > +                                              const u8 *table, u8 table_size,
> > +                                              u64 freq_millihz)
> > +{
> > +       return false;
> > +}
> > +#endif
> >  #endif
>
> What about just dropping the old functions, and adding two simple
> compatibility defines in the header file:
>
>     #define rzv2h_get_pll_pars rzv2h_cpg_get_pll_pars
>     #define rzv2h_get_pll_divs_pars rzv2h_cpg_get_pll_divs_pars
>
Thats neat!

> That way there is less code to change in the next phase.
>
Agreed.

Cheers,
Prabhakar

> 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
>