Hi Prabhakar,
On Mon, 28 Jul 2025 at 22:14, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> Add support for PLLDSI and PLLDSI divider clocks.
>
> Introduce the `renesas-rzv2h-cpg-pll.h` header to centralize and share
> PLLDSI related data structures, limits, and algorithms between the
> RZ/V2H(P) CPG and DSI drivers.
>
> The DSI PLL is functionally similar to the CPG's PLLDSI, but has slightly
> different parameter limits and omits the programmable divider present in
> CPG. To ensure precise frequency calculations, especially for milliHz-level
> accuracy needed by the DSI driver, the shared algorithm allows both drivers
> to compute PLL parameters consistently using the same logic and input
> clock.
>
> Co-developed-by: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> ---
> v6->v7:
> - Made struct rzv2h_pll_limits more modular also added Ffout limits
> - Made the alogirithm modular and also added apis based on the
> needs for lvds and dpi
Thanks for the update!
> --- a/drivers/clk/renesas/rzv2h-cpg.c
> +++ b/drivers/clk/renesas/rzv2h-cpg.c
> +static struct rzv2h_pll_dsi_info *rzv2h_get_pll_dsi_info(struct clk_hw *pll_dsi,
> + struct rzv2h_cpg_priv *priv)
> +{
> + struct pll_clk *pll_clk = to_pll(pll_dsi);
> +
> + return &priv->pll_dsi_info[pll_clk->pll.instance];
> +}
This (very simple helper) is used twice, while there are two (almost
three) other locations where it is open-coded. Perhaps just open-code
it everywhere?
> @@ -246,7 +526,8 @@ static const struct clk_ops rzv2h_cpg_pll_ops = {
> static struct clk * __init
> rzv2h_cpg_pll_clk_register(const struct cpg_core_clk *core,
> struct rzv2h_cpg_priv *priv,
> - const struct clk_ops *ops)
> + const struct clk_ops *ops,
> + bool is_plldsi)
No need for this parameter...
> {
> struct device *dev = priv->dev;
> struct clk_init_data init;
> @@ -263,6 +544,10 @@ rzv2h_cpg_pll_clk_register(const struct cpg_core_clk *core,
> if (!pll_clk)
> return ERR_PTR(-ENOMEM);
>
> + if (is_plldsi)
... as you can just test "core->type == CLK_TYPE_PLLDSI" here.
> + priv->pll_dsi_info[core->cfg.pll.instance].pll_dsi_limits =
> + core->cfg.pll.limits;
> +
> parent_name = __clk_get_name(parent);
> init.name = core->name;
> init.ops = ops;
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@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
