Re: [PATCH v4 03/10] clk: Use clamp instead of open-coding our own

[Stephen Boyd <sboyd@kernel.org>]

Quoting Maxime Ripard (2022-02-21 08:30:01)
> On Fri, Feb 18, 2022 at 02:34:20PM -0800, Stephen Boyd wrote:
> > Quoting Maxime Ripard (2022-01-25 06:15:42)
> > > The code in clk_set_rate_range() will, if the current rate is outside of
> > > the new range, will force it to the minimum or maximum. This is
> > > equivalent to using clamp, while being less readable. Let's switch to
> > > using clamp instead.
> > > 
> > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > > ---
> > >  drivers/clk/clk.c | 6 +-----
> > >  1 file changed, 1 insertion(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > > index 7bb5ae0fb688..150d1bc0985b 100644
> > > --- a/drivers/clk/clk.c
> > > +++ b/drivers/clk/clk.c
> > > @@ -2365,11 +2365,7 @@ int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max)
> > >                  *   this corner case when determining the rate
> > >                  */
> > >  
> > > -               if (rate < min)
> > > -                       rate = min;
> > > -               else
> > > -                       rate = max;
> > > -
> > > +               rate = clamp(clk->core->req_rate, min, max);
> > 
> > This isn't equivalent. The else arm is taken if rate >= min and rate is
> > set to max, whereas clamp() will leave the rate unchanged if rate >= min
> > && rate < max.
> 
> This can't happen, since we're in an if block that is (rate < min ||
> rate > max), so at this point if rate is not less than min, it is
> greater than rate. Thus, it's equivalent to clamp.
> 
> Still, the commit message could be better, I'll rephrase it.

Perfect! Should probably add a comment above the clamp as well just in
case someone decides to move it out of that if block.