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

Geert Uytterhoeven <[email protected]>
Newsgroups org.kernel.vger.linux-renesas-soc,org.kernel.vger.linux-clk,org.kernel.vger.linux-kernel
Message-ID <CAMuHMdViJB2qvzbOkX7exqpq7gF4tbBcNYMHmCmEXHSh6AYfew@mail.gmail.com>
Hi Biju,

On Tue, 11 Aug 2026 at 20:27, Biju <[email protected]> wrote:
> From: Biju Das <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
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 -- [email protected]

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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.