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

[Dan Carpenter <dan.carpenter@linaro.org>]

On Mon, Dec 09, 2024 at 08:16:24AM +0000, Etienne CARRIERE - foss wrote:
> 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.

No, no.  Leave it.  Sorry.  I wasn't paying attention.

regards,
dan carpenter