Re: [PATCH v2 3/9] clk: renesas: rzg2l: Add DSI divider clk support

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Biju,

On Wed, Apr 27, 2022 at 11:48 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> M3 clock is sourced from DSI Divider (DSIDIVA * DSIDIVB)
>
> This patch add support for DSI divider clk by combining
> DSIDIVA and DSIDIVB .
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> ---
> V1->V2:
>  * Replaced round_rate with determine_rate
>  * Update rate variable during set_rate
>  * Added get_vclk_parent_rate helper function
>  * Replaced pll5_params with mux_dsi_div_params for dsi div values.

Thanks for the update!

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

> +static unsigned long rzg2l_cpg_get_vclk_parent_rate(struct clk_hw *hw,
> +                                                   unsigned long rate)
> +{
> +       unsigned long parent_rate;

Please drop the blank line.

> +
> +       struct dsi_div_hw_data *dsi_div = to_dsi_div_hw_data(hw);
> +       struct rzg2l_cpg_priv *priv = dsi_div->priv;
> +       struct rzg2l_pll5_param params;

Reverse Xmas tree order?

> +
> +       parent_rate = rzg2l_cpg_get_foutpostdiv_rate(&params, rate);
> +
> +       if (priv->mux_dsi_div_params.clksrc)
> +               parent_rate /= 2;
> +
> +       return parent_rate;
> +}
> +
> +static int rzg2l_cpg_dsi_div_determine_rate(struct clk_hw *hw,
> +                                           struct clk_rate_request *req)
> +{
> +       req->best_parent_rate = rzg2l_cpg_get_vclk_parent_rate(hw, req->rate);
> +
> +       return 0;

So any value of req->rate passed is supported, and req->rate never
needs to be updated?

> +}
> +
> +static int rzg2l_cpg_dsi_div_set_rate(struct clk_hw *hw,
> +                                     unsigned long rate,
> +                                     unsigned long parent_rate)
> +{
> +       struct dsi_div_hw_data *dsi_div = to_dsi_div_hw_data(hw);
> +       struct rzg2l_cpg_priv *priv = dsi_div->priv;
> +
> +       /*
> +        * MUX -->DIV_DSI_{A,B} -->M3 -->VCLK
> +        *
> +        * Based on the dot clock, the DSI divider clock sets the divider value,
> +        * calculates the pll parameters for generating FOUTPOSTDIV and the clk
> +        * source for the MUX and propagates that info to the parents.
> +        */
> +
> +       if (!rate)
> +               return -EINVAL;
> +
> +       dsi_div->rate = rate;

So any non-zero value of rate is supported?

> +       writel(CPG_PL5_SDIV_DIV_DSI_A_WEN | CPG_PL5_SDIV_DIV_DSI_B_WEN |
> +              (priv->mux_dsi_div_params.dsi_div_a << 0) |
> +              (priv->mux_dsi_div_params.dsi_div_b << 8),
> +              priv->base + CPG_PL5_SDIV);
> +
> +       return 0;
> +}

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