[PATCH 1/2] clk: renesas: rzg2l: Remove DSI clock rate restrictions
Chris Brandt
Chris.Brandt at renesas.com
Thu Jul 10 16:53:02 UTC 2025
Hi Biju,
Thank you for your review!
> > +/* Required division ratio for the MIPI clock */
> > +int dsi_div_ab;
>
> static int dsi_div_ab;
Good catch.
> for the DPI, DIV_DSI_B = 1 and DIV_DSI_A ={2, 4, 8}
>
> So, you need to adjust the below calculation for DPI as well??
You bring up a good point.
And looking at the hardware manual again, there are other restrictions when using FOUTPOSTDIV (straight PLL) compared to FOUT1PH0 (PLL/2).
>From a chip design standpoint, they just expect to have 'one big driver that configures everything at once'.
> Not sure do we need DSI driver registering a callback with CPG driver and CPG driver uses the callback to get DSI divider value and this callback can be used to distinguish DPI from DSI??
Ya, you can't just tell the CPG driver to 'give me this rate'. There is so much other information that it needs to have before it can set up the registers.
Hmm....
> > +found_clk:
> > + if (!found) {
>
> Can we add a dev_dbg statement here for !found clock?
Yes, good idea.
That was in the original driver before I started pulling out all the printk statements.
> > + /* If foutvco is above 1.5GHz, change parent and recalculate */
> > + if (priv->mux_dsi_div_params.clksrc && (foutvco_rate > 1500000000))
> > +{
>
> Check patch is complaining:
>
> CHECK: Unnecessary parentheses around 'foutvco_rate > 1500000000'
> #146: FILE: drivers/clk/renesas/rzg2l-cpg.c:648:
> + if (priv->mux_dsi_div_params.clksrc && (foutvco_rate > 1500000000)) {
I saw that...but I thought the ( ) makes it a little easier to read.
But, what's the general rule here? Make checkpatch come out perfect?
What's your thoughts?
Chris
-----Original Message-----
From: Biju Das <biju.das.jz at bp.renesas.com>
Sent: Thursday, July 10, 2025 6:00 AM
To: Chris Brandt <Chris.Brandt at renesas.com>; Geert Uytterhoeven <geert+renesas at glider.be>; Michael Turquette <mturquette at baylibre.com>; Stephen Boyd <sboyd at kernel.org>; Maarten Lankhorst <maarten.lankhorst at linux.intel.com>; Maxime Ripard <mripard at kernel.org>; Thomas Zimmermann <tzimmermann at suse.de>; David Airlie <airlied at gmail.com>; Simona Vetter <simona at ffwll.ch>; Hien Huynh <hien.huynh.px at renesas.com>; Nghia Vo <nghia.vo.zn at renesas.com>; Hugo Villeneuve <hugo at hugovil.com>
Cc: linux-renesas-soc at vger.kernel.org; linux-clk at vger.kernel.org; dri-devel at lists.freedesktop.org; Chris Brandt <Chris.Brandt at renesas.com>
Subject: RE: [PATCH 1/2] clk: renesas: rzg2l: Remove DSI clock rate restrictions
Hi Chris Brandt,
Thanks for the patch.
> -----Original Message-----
> From: Chris Brandt <chris.brandt at renesas.com>
> Sent: 09 July 2025 21:56
> Subject: [PATCH 1/2] clk: renesas: rzg2l: Remove DSI clock rate
> restrictions
>
> Convert the limited MIPI clock calculations to a full range of
> settings based on math including H/W limitation validation.
> Since the required DSI division setting must be specified from
> external sources before calculations, expose a new API to set it.
>
> Signed-off-by: Chris Brandt <chris.brandt at renesas.com>
> Signed-off-by: hienhuynh <hien.huynh.px at renesas.com>
> Signed-off-by: Nghia Vo <nghia.vo.zn at renesas.com>
> ---
> drivers/clk/renesas/rzg2l-cpg.c | 113 +++++++++++++++++++++++++++++---
> include/linux/clk/renesas.h | 4 ++
> 2 files changed, 107 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/clk/renesas/rzg2l-cpg.c
> b/drivers/clk/renesas/rzg2l-cpg.c index
> a8628f64a03b..317e50f5b967 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -68,6 +68,19 @@
>
> #define MAX_VCLK_FREQ (148500000)
>
> +#define PLL5_FOUTVCO_MIN 800000000
> +#define PLL5_FOUTVCO_MAX 3000000000
> +#define PLL5_POSTDIV_MIN 1
> +#define PLL5_POSTDIV_MAX 7
> +#define PLL5_POSTDIV_DEF 1
> +#define PLL5_REFDIV_MIN 1
> +#define PLL5_REFDIV_MAX 2
> +#define PLL5_REFDIV_DEF 1
> +#define PLL5_INTIN_MIN 20
> +#define PLL5_INTIN_MAX 320
> +#define PLL5_INTIN_DEF 125
> +#define PLL5_FRACIN_DEF 0
> +
> /**
> * struct clk_hw_data - clock hardware data
> * @hw: clock hw
> @@ -123,6 +136,9 @@ struct rzg2l_pll5_param {
> u8 pl5_spread;
> };
>
> +/* Required division ratio for the MIPI clock */
> +int dsi_div_ab;
static int dsi_div_ab;
> struct rzg2l_pll5_mux_dsi_div_param {
> u8 clksrc;
> u8 dsi_div_a;
> @@ -548,24 +564,93 @@ rzg2l_cpg_sd_mux_clk_register(const struct
> cpg_core_clk *core, }
>
> static unsigned long
> -rzg2l_cpg_get_foutpostdiv_rate(struct rzg2l_pll5_param *params,
> +rzg2l_cpg_get_foutpostdiv_rate(struct rzg2l_cpg_priv *priv,
> + struct rzg2l_pll5_param *params,
> unsigned long rate)
> {
> unsigned long foutpostdiv_rate, foutvco_rate;
> + u8 div = 1;
> + bool found = 0;
> + int a, b;
> +
> + if (priv->mux_dsi_div_params.clksrc)
> + div = 2;
> +
for the DPI, DIV_DSI_B = 1 and DIV_DSI_A ={2, 4, 8}
So, you need to adjust the below calculation for DPI as well??
Not sure do we need DSI driver registering a callback with CPG driver and CPG driver uses the callback to get DSI divider value and this callback can be used to distinguish DPI from DSI??
Maybe Geert can provide more input on this?
> + /* Calculate the DIV_DSI_A and DIV_DSI_B based on the final DIV DSI */
> + for (a = 0; a < 4; a++) {
> + for (b = 0; b < 16; b++) {
> + if (((1 << a) * (b + 1)) == dsi_div_ab) {
> + priv->mux_dsi_div_params.dsi_div_a = a;
> + priv->mux_dsi_div_params.dsi_div_b = b;
> +
> + goto found_dsi_div;
> + }
> + }
> + }
> +
> +found_dsi_div:
> + /*
> + * Below conditions must be set for PLL5 parameters:
> + * - REFDIV must be between 1 and 2.
> + * - POSTDIV1/2 must be between 1 and 7.
> + * - INTIN must be between 20 and 320.
> + * - FOUTVCO must be between 800MHz and 3000MHz.
> + */
> + for (params->pl5_postdiv1 = PLL5_POSTDIV_MIN;
> + params->pl5_postdiv1 < PLL5_POSTDIV_MAX + 1;
> + params->pl5_postdiv1++) {
> + for (params->pl5_postdiv2 = PLL5_POSTDIV_MIN;
> + params->pl5_postdiv2 < PLL5_POSTDIV_MAX + 1;
> + params->pl5_postdiv2++) {
> + foutvco_rate = rate * ((1 << priv->mux_dsi_div_params.dsi_div_a) *
> + (priv->mux_dsi_div_params.dsi_div_b + 1)) *
> + div * params->pl5_postdiv1 * params->pl5_postdiv2;
> + if (foutvco_rate < PLL5_FOUTVCO_MIN + 1 ||
> + foutvco_rate > PLL5_FOUTVCO_MAX - 1)
> + continue;
> +
> + for (params->pl5_refdiv = PLL5_REFDIV_MIN;
> + params->pl5_refdiv < PLL5_REFDIV_MAX + 1;
> + params->pl5_refdiv++) {
> + params->pl5_intin = (foutvco_rate * params->pl5_refdiv) /
> + (EXTAL_FREQ_IN_MEGA_HZ * MEGA);
> + if (params->pl5_intin < PLL5_INTIN_MIN + 1 ||
> + params->pl5_intin > PLL5_INTIN_MAX - 1)
> + continue;
> + params->pl5_fracin = div_u64(((u64)
> + (foutvco_rate * params->pl5_refdiv) %
> + (EXTAL_FREQ_IN_MEGA_HZ * MEGA)) << 24,
> + EXTAL_FREQ_IN_MEGA_HZ * MEGA);
> + found = 1;
> + goto found_clk;
> + }
> + }
> + }
> +
> +found_clk:
> + if (!found) {
Can we add a dev_dbg statement here for !found clock?
> + params->pl5_intin = PLL5_INTIN_DEF;
> + params->pl5_fracin = PLL5_FRACIN_DEF;
> + params->pl5_refdiv = PLL5_REFDIV_DEF;
> + params->pl5_postdiv1 = PLL5_POSTDIV_DEF;
> + params->pl5_postdiv2 = PLL5_POSTDIV_DEF;
> + }
>
> - params->pl5_intin = rate / MEGA;
> - params->pl5_fracin = div_u64(((u64)rate % MEGA) << 24, MEGA);
> - params->pl5_refdiv = 2;
> - params->pl5_postdiv1 = 1;
> - params->pl5_postdiv2 = 1;
> params->pl5_spread = 0x16;
>
> foutvco_rate = div_u64(mul_u32_u32(EXTAL_FREQ_IN_MEGA_HZ * MEGA,
> - (params->pl5_intin << 24) + params->pl5_fracin),
> - params->pl5_refdiv) >> 24;
> + (params->pl5_intin << 24) + params->pl5_fracin),
> + params->pl5_refdiv) >> 24;
> foutpostdiv_rate = DIV_ROUND_CLOSEST_ULL(foutvco_rate,
> params->pl5_postdiv1 * params->pl5_postdiv2);
>
> + /* If foutvco is above 1.5GHz, change parent and recalculate */
> + if (priv->mux_dsi_div_params.clksrc && (foutvco_rate > 1500000000))
> +{
Check patch is complaining:
CHECK: Unnecessary parentheses around 'foutvco_rate > 1500000000'
#146: FILE: drivers/clk/renesas/rzg2l-cpg.c:648:
+ if (priv->mux_dsi_div_params.clksrc && (foutvco_rate > 1500000000)) {
total: 0 errors, 0 warnings, 1 checks, 172 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
0002-clk-renesas-rzg2l-Remove-DSI-clock-rate-restrictions.patch has style problems, please review.
Cheers,
Biju
More information about the dri-devel
mailing list