linux-kernel - Re: [PATCH v5 1/4] 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAMuHMdXb5ZCX=U_BR0=AkGtdGkVosty0cGsbKQryTy11Au8H-A@mail.gmail.com>
Date: Wed, 28 May 2025 09:09:12 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
Cc: Fabrizio Castro <fabrizio.castro.jz@...esas.com>, 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, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v5 1/4] clk: renesas: rzv2h-cpg: Add support for DSI clocks

Hi Prabhakar,

On Tue, 27 May 2025 at 23:51, Lad, Prabhakar <prabhakar.csengg@...il.com> wrote:
> On Fri, May 23, 2025 at 3:45 PM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> > On Mon, 12 May 2025 at 20:43, 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-dsi.h` header to centralize and share
> > > PLLDSI-related data structures, limits, and algorithms between the RZ/V2H
> > > 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>

> > > +static int rzv2h_cpg_plldsi_div_determine_rate(struct clk_hw *hw,
> > > +                                              struct clk_rate_request *req)
> > > +{
> > > +       struct rzv2h_plldsi_div_clk *dsi_div = to_plldsi_div_clk(hw);
> > > +       struct rzv2h_cpg_priv *priv = dsi_div->priv;
> > > +       struct rzv2h_plldsi_parameters *dsi_dividers = &priv->plldsi_div_parameters;
> > > +       u64 rate_millihz;
> > > +
> > > +       /*
> > > +        * Adjust the requested clock rate (`req->rate`) to ensure it falls within
> > > +        * the supported range of 5.44 MHz to 187.5 MHz.
> > > +        */
> > > +       req->rate = clamp(req->rate, 5440000UL, 187500000UL);
> > > +
> > > +       rate_millihz = mul_u32_u32(req->rate, MILLI);
> > > +       if (rate_millihz == dsi_dividers->error_millihz + dsi_dividers->freq_millihz)
> > > +               goto exit_determine_rate;
> > > +
> > > +       if (!rzv2h_dsi_get_pll_parameters_values(priv->dsi_limits,
> > > +                                                dsi_dividers, rate_millihz)) {
> > > +               dev_err(priv->dev,
> > > +                       "failed to determine rate for req->rate: %lu\n",
> > > +                       req->rate);
> > > +               return -EINVAL;
> > > +       }
> > > +
> > > +exit_determine_rate:
> > > +       req->best_parent_rate = req->rate * dsi_dividers->csdiv;
> >
> > Shouldn't this also update req->rate with the actual rate?
> >
> >     req->rate = DIV_ROUND_CLOSEST_ULL(dsi_dividers->freq_millihz, MILLI);
> >
> Agreed, I will update it.

I think not updating req->rate may cause clk_get_rate() to return
an incorrect value (can error_millihz > 1000?).  Any chance this fix
can simplify the clock handling in the DSI driver?

> > Would it help the DSI driver if this clock would provide a
> > .recalc_accuracy() callback that takes into account the difference
> > between req->rate and dsi_dividers->freq_millihz?
> > Or would that be considered abuse of the accuracy concept?
> >
> Our understanding is that this describes how precisely a clock keeps
> time. A clock with 1 ppb accuracy will gain or lose one second in
> approximately 31.5 million seconds (1 year). In our case the meaning
> is completely different.

Yeah, I know...

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