[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<TY3PR01MB1134671558883AAE2C3E0A2C78672A@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Date: Wed, 18 Jun 2025 13:25:54 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
CC: Geert Uytterhoeven <geert+renesas@...der.be>, 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>, Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, 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>, Magnus
Damm <magnus.damm@...il.com>, "dri-devel@...ts.freedesktop.org"
<dri-devel@...ts.freedesktop.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "linux-renesas-soc@...r.kernel.org"
<linux-renesas-soc@...r.kernel.org>, "linux-clk@...r.kernel.org"
<linux-clk@...r.kernel.org>, Fabrizio Castro
<fabrizio.castro.jz@...esas.com>, Prabhakar Mahadev Lad
<prabhakar.mahadev-lad.rj@...renesas.com>
Subject: RE: [PATCH v6 1/4] clk: renesas: rzv2h-cpg: Add support for DSI
clocks
Hi Prabhakar,
> -----Original Message-----
> From: dri-devel <dri-devel-bounces@...ts.freedesktop.org> On Behalf Of Lad, Prabhakar
> Sent: 16 June 2025 11:45
> Subject: Re: [PATCH v6 1/4] clk: renesas: rzv2h-cpg: Add support for DSI clocks
>
> Hi Biju,
>
> Thank you for the review.
>
> On Fri, Jun 13, 2025 at 6:57 AM Biju Das <biju.das.jz@...renesas.com> wrote:
> >
> > Hi Prabhakar,
> >
> > > -----Original Message-----
> > > From: Prabhakar <prabhakar.csengg@...il.com>
> > > Sent: 30 May 2025 18:19
> > .castro.jz@...esas.com>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> > > lad.rj@...renesas.com>
> > > Subject: [PATCH v6 1/4] clk: renesas: rzv2h-cpg: Add support for DSI
> > > clocks
> > >
> > > 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>
> > > ---
> > > v5->v6:
> > > - Renamed CPG_PLL_STBY_SSCGEN_WEN to CPG_PLL_STBY_SSC_EN_WEN
> > > - Updated CPG_PLL_CLK1_DIV_K, CPG_PLL_CLK1_DIV_M, and
> > > CPG_PLL_CLK1_DIV_P macros to use GENMASK
> > > - Updated req->rate in rzv2h_cpg_plldsi_div_determine_rate()
> > > - Dropped the cast in rzv2h_cpg_plldsi_div_set_rate()
> > > - Dropped rzv2h_cpg_plldsi_round_rate() and implemented
> > > rzv2h_cpg_plldsi_determine_rate() instead
> > > - Made use of FIELD_PREP()
> > > - Moved CPG_CSDIV1 macro in patch 2/4
> > > - Dropped two_pow_s in rzv2h_dsi_get_pll_parameters_values()
> > > - Used mul_u32_u32() while calculating output_m and output_k_range
> > > - Used div_s64() instead of div64_s64() while calculating
> > > pll_k
> > > - Used mul_u32_u32() while calculating fvco and fvco checks
> > > - Rounded the final output using DIV_U64_ROUND_CLOSEST()
> > >
> > > v4->v5:
> > > - No changes
> > >
> > > v3->v4:
> > > - Corrected parameter name in rzv2h_dsi_get_pll_parameters_values()
> > > description freq_millihz
> > >
> > > v2->v3:
> > > - Update the commit message to clarify the purpose of `renesas-rzv2h-dsi.h`
> > > header
> > > - Used mul_u32_u32() in rzv2h_cpg_plldsi_div_determine_rate()
> > > - Replaced *_mhz to *_millihz for clarity
> > > - Updated u64->u32 for fvco limits
> > > - Initialized the members in declaration order for
> > > RZV2H_CPG_PLL_DSI_LIMITS() macro
> > > - Used clk_div_mask() in rzv2h_cpg_plldsi_div_recalc_rate()
> > > - Replaced `unsigned long long` with u64
> > > - Dropped rzv2h_cpg_plldsi_clk_recalc_rate() and reused
> > > rzv2h_cpg_pll_clk_recalc_rate() instead
> > > - In rzv2h_cpg_plldsi_div_set_rate() followed the same style
> > > of RMW-operation as done in the other functions
> > > - Renamed rzv2h_cpg_plldsi_set_rate() to rzv2h_cpg_pll_set_rate()
> > > - Dropped rzv2h_cpg_plldsi_clk_register() and reused
> > > rzv2h_cpg_pll_clk_register() instead
> > > - Added a gaurd in renesas-rzv2h-dsi.h header
> > >
> > > v1->v2:
> > > - No changes
> > > ---
> > > drivers/clk/renesas/rzv2h-cpg.c | 278 +++++++++++++++++++++++++-
> > > drivers/clk/renesas/rzv2h-cpg.h | 13 ++
> > > include/linux/clk/renesas-rzv2h-dsi.h | 210 +++++++++++++++++++
> > > 3 files changed, 492 insertions(+), 9 deletions(-) create mode
> > > 100644 include/linux/clk/renesas- rzv2h-dsi.h
> > >
> > > diff --git a/drivers/clk/renesas/rzv2h-cpg.c
> > > b/drivers/clk/renesas/rzv2h-cpg.c index
> > > 761da3bf77ce..d590f9f47371 100644
> > > --- a/drivers/clk/renesas/rzv2h-cpg.c
> > > +++ b/drivers/clk/renesas/rzv2h-cpg.c
> > > @@ -14,9 +14,13 @@
> > > #include <linux/bitfield.h>
> > > #include <linux/clk.h>
> > > #include <linux/clk-provider.h>
> > > +#include <linux/clk/renesas-rzv2h-dsi.h>
> > > #include <linux/delay.h>
> > > #include <linux/init.h>
> > > #include <linux/iopoll.h>
> > > +#include <linux/math.h>
> >
> >
> >
> > > + req->rate = DIV_ROUND_CLOSEST_ULL(dsi_dividers->freq_millihz,
> > > + MILLI);
> > > +
> > > + return 0;
> > > +};
> > > +
> > > +static int rzv2h_cpg_plldsi_div_set_rate(struct clk_hw *hw,
> > > + unsigned long rate,
> > > + unsigned long parent_rate) {
> > > + 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;
> > > + struct ddiv ddiv = dsi_div->ddiv;
> > > + const struct clk_div_table *clkt;
> > > + bool div_found = false;
> > > + u32 val, shift, div;
> > > +
> > > + div = dsi_dividers->csdiv;
> > > + for (clkt = dsi_div->dtable; clkt->div; clkt++) {
> > > + if (clkt->div == div) {
> > > + div_found = true;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + if (!div_found)
> > > + return -EINVAL;
> >
> > This check can be done in determine rate and cache the divider??
> >
> Ok, I'll drop this check as the divider is already cached. The for loop above is to determine the val
> which is used below to program the registers.
If you are caching actual divider value, then the check is not required here.
Otherwise the above code is fine.
Assume the csdiv you found, have no corresponding match in the table.
Cheers,
Biju
Powered by blists - more mailing lists