linux-kernel - Re: [PATCH v7 2/6] clk: renesas: rzv2h-cpg: Add support for DSI clocks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdWj=W17j9WHhTR1gH9MZ_fcxve_dOi6eMiL62xphXG+GQ@mail.gmail.com>
Date: Tue, 19 Aug 2025 15:14:07 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Prabhakar <prabhakar.csengg@...il.com>
Cc: Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong <neil.armstrong@...aro.org>, 
	Robert Foss <rfoss@...nel.org>, Laurent Pinchart <Laurent.pinchart@...asonboard.com>, 
	Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, 
	Biju Das <biju.das.jz@...renesas.com>, Magnus Damm <magnus.damm@...il.com>, 
	dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-renesas-soc@...r.kernel.org, 
	linux-clk@...r.kernel.org, Fabrizio Castro <fabrizio.castro.jz@...esas.com>, 
	Tommaso Merciai <tommaso.merciai.xr@...renesas.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v7 2/6] clk: renesas: rzv2h-cpg: Add support for DSI clocks

Hi Prabhakar,

On Mon, 28 Jul 2025 at 22:14, Prabhakar <prabhakar.csengg@...il.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
>
> 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@...esas.com>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@...esas.com>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> ---
> 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@...der.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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