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

[Biju Das <biju.das.jz@xxxxxxxxxxxxxx>]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v2 3/9] clk: renesas: rzg2l: Add DSI divider clk
> support
> 
> Hi Biju,
> 
> On Fri, Apr 29, 2022 at 11:50 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > > Subject: Re: [PATCH v2 3/9] clk: renesas: rzg2l: Add DSI divider clk
> > > support
> 
> > > 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>
> 
> > > > +
> > > > +       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?
> >
> > Yes, We can add a check for freq greater than 148.5MHz and if it is
> > greater, Assign req->rate to 148.5MHz.
> >
> > >
> > > > +}
> > > > +
> > > > +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?
> >
> > Currently yes, But If we add a check for > 148.5Mhz in determine_rate,
> it will allow only upto 148.5Mhz.
> 
> You still need that check here, too, as there is no guarantee users will
> call .set_rate() with the rounded parameters obtained from
> .determine_rate().

OK, will add a check and return -EINVAL for freq > 148.5 here.

Cheers,
Biju