Re: [PATCH v2 1/9] clk: renesas: rzg2l: Add DSI divider clock support for RZ/G3L

[Geert Uytterhoeven]

Hi Biju,

On Tue, 11 Aug 2026 at 20:27, Biju <biju.das.au@xxxxxxxxx> wrote:
> From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
>
> Add a new DSI divider clock type (CLK_TYPE_G3L_PLLDSI_DIV) for the RZ/G3L
> SoC, which requires a different divider implementation than the existing
> RZ/G2L DSI divider clock.
>
> The RZ/G3L DSI divider uses two cascaded dividers, DIV_DSI_A and
> DIV_DSI_B, where the effective divider is:
>
>   rate = parent_rate / ((1 << div_a) * (div_b + 1))
>
> DIV_DSI_A is a power-of-two divider with values in the range [0, 5],
> and DIV_DSI_B is a linear divider with values in the range [1, 16].
>
> Introduce the g3l_dsi_div_hw_data structure, rzg3l_cpg_dsi_div_ops, and
> rzg3l_cpg_dsi_div_clk_register() to implement the new clock type, and add
> the DEF_G3L_PLLDSI_DIV() macro for use in clock table definitions.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> ---
> v1->v2:
>  * Introduced the macro DIV_DSI_{A,B}_SET and used inside
>    rzg3l_cpg_dsi_div_recalc_rate() and rzg3l_cpg_dsi_div_set_rate()
>  * Simplified the divider to "(div_b + 1) << div_a" in
>    rzg3l_cpg_dsi_div_recalc_rate().
>  * Replaced the data type u32->unsigned int for the variable divider in
>    rzg3l_cpg_dsi_div_determine_rate()
>  * Moved the declarations of the loop counters inside the for()-statements
>  * Updated the comment sections in rzg3l_cpg_dsi_div_determine_rate().
>  * Updated loop logic in rzg3l_cpg_dsi_div_determine_rate().

Thanks for the update!

> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c

> +static int rzg3l_cpg_dsi_div_determine_rate(struct clk_hw *hw,
> +                                           struct clk_rate_request *req)
> +{
> +       struct g3l_dsi_div_hw_data *dsi_div = to_g3l_dsi_div_hw_data(hw);
> +       struct rzg2l_cpg_priv *priv = dsi_div->priv;
> +       unsigned int divider = dsi_div_ab_desired;
> +       bool divider_found = false;
> +
> +       if (dsi_div_target) {
> +               /* Calculate DIV_DSI_A and DIV_DSI_B */
> +               for (unsigned int div_a = 0; div_a <= 5 && !divider_found; div_a++) {
> +                       unsigned int div_a_tmp = 5 - div_a;

So div_a_tmp is the actual value of div_a... That is confusing, so I'll do
s/div_a/i/ and s/div_a_tmp/div_a/ while applying.

> +
> +                       for (unsigned int div_b = 0; div_b < 16; div_b++) {
> +                               divider = (div_b + 1) << div_a_tmp;
> +                               if (divider == dsi_div_ab_desired) {
> +                                       dsi_div->div_a = div_a_tmp;
> +                                       dsi_div->div_b = div_b;
> +                                       divider_found = true;
> +                                       break;
> +                               }
> +                       }
> +               }
> +       } else {
> +               dsi_div->div_b = 0;
> +               /* Calculate DIV_DSI_A */
> +               for (unsigned int div_a = 0; div_a <= 5 && !divider_found; div_a++) {
> +                       unsigned int div_a_tmp = 5 - div_a;
> +
> +                       divider = 1 << div_a_tmp;
> +                       if (divider == dsi_div_ab_desired) {
> +                               dsi_div->div_a = div_a_tmp;
> +                               divider_found = true;
> +                               break;
> +                       }
> +               }

This is basically the same code as in the other branch, just looping
over a single value of div_b:

    unsigned int div_b_max = dsi_div_target ? 15 : 0;

    for (unsigned int div_b = 0; div_b <= div_b_max; div_b++)

> +       }

Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
i.e. will queue in renesas-clk for v7.4, with the above fixed.

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