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-clk,org.kernel.vger.linux-kernel,org.kernel.vger.linux-renesas-soc |
|---|---|
| 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