Re: [PATCH v2 2/2] firmware: arm_scmi: round rate bisecting in discrete rates

[Etienne CARRIERE - foss <etienne.carriere@foss.st.com>]

Hello Dan,

Thanks for looking at this change.

On Friday, December 6, 2024, Dan Carpenter wrote:
> 
> On Tue, Dec 03, 2024 at 06:39:08PM +0100, Etienne Carriere wrote:
> > diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
> > index 09ccd6cea7f2..7bbb2ee55f4f 100644
> > --- a/drivers/clk/clk-scmi.c
> > +++ b/drivers/clk/clk-scmi.c
> > @@ -61,13 +61,20 @@ static long scmi_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> >       struct scmi_clk *clk = to_scmi_clk(hw);
> >
> >       /*
> > -      * We can't figure out what rate it will be, so just return the
> > +      * In case we can't figure out what rate it will be when the clock
> > +      * describes a list of discrete rates, then just return the
> >        * rate back to the caller. scmi_clk_recalc_rate() will be called
> >        * after the rate is set and we'll know what rate the clock is
> >        * running at then.
> >        */
> > -     if (clk->info->rate_discrete)
> > +     if (clk->info->rate_discrete) {
> > +             ftmp = rate;
> 
> No need for this assignment.

It is needed. The round_rate handler in scmi clock protocol driver 
I added in drivers/firmware/arm_scmi/clock.c expects the argument
to carry the target rate as input value and provide the closest reachable
rate as output value, hence initializing @ftmp above. I needed to preserve
the value @rate since used if this round_rate handle is not available
or returns with an error.

Note that I could have changed scmi_proto_clk_ops->round_rate() 
function ABI to split input target rate and output rounded rate into
2 separated explicit arguments.

Regards,
Etienne

> 
> > +             if (scmi_proto_clk_ops->round_rate &&
> > +                 !scmi_proto_clk_ops->round_rate(clk->ph, clk->id, &ftmp))
> > +                     return ftmp;
> > +
> >               return rate;
> > +     }
> >
> >       fmin = clk->info->range.min_rate;
> >       fmax = clk->info->range.max_rate;
> 
> regards,
> dan carpenter
>