Re: [PATCH v4 02/10] clk: Always clamp the rounded rate

[Stephen Boyd <sboyd@kernel.org>]

Quoting Maxime Ripard (2022-02-21 08:43:23)
> Hi again,
> 
> On Mon, Feb 21, 2022 at 05:18:21PM +0100, Maxime Ripard wrote:
> > On Fri, Feb 18, 2022 at 03:15:06PM -0800, Stephen Boyd wrote:
> > > Quoting Maxime Ripard (2022-01-25 06:15:41)
> > > > +/*
> > > > + * Test that if our clock has some boundaries and we try to round a rate
> > > > + * lower than the minimum, the returned rate will be within range.
> > > > + */
> > > > +static void clk_range_test_set_range_round_rate_lower(struct kunit *test)
> > > > +{
> > > > +       struct clk_dummy_context *ctx = test->priv;
> > > > +       struct clk_hw *hw = &ctx->hw;
> > > > +       struct clk *clk = hw->clk;
> > > > +       long rate;
> > > > +
> > > > +       KUNIT_ASSERT_EQ(test,
> > > > +                       clk_set_rate_range(clk,
> > > > +                                          DUMMY_CLOCK_RATE_1,
> > > > +                                          DUMMY_CLOCK_RATE_2),
> > > > +                       0);
> > > > +
> > > > +       rate = clk_round_rate(clk, DUMMY_CLOCK_RATE_1 - 1000);
> > > > +       KUNIT_ASSERT_GT(test, rate, 0);
> > > > +       KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
> > > 
> > > The comment says within range but this test says exactly the minimum
> > > rate. Please change it to test that the rate is within rate 1 and rate
> > > 2. Also, we should call clk_get_rate() here to make sure the rate is
> > > within the boundaries and matches what clk_round_rate() returned.
> > 
> > Ok
> 
> Actually, that doesn't work. Calling clk_round_rate() won't affect the
> clock rate, so the rate returned by clk_get_rate() won't match what
> clk_round_rate() will return.

Huh? This is asking "what rate will I get if I call clk_set_rate() with
DUMMY_CLOCK_RATE_1 - 1000 after setting the range to be rate 1 and rate
2. It should round that up to some value (and we should enforce that it
is inclusive or exclusive). I think I missed that this is
clk_round_rate().

Either way, the clk provider implementation could say that if you call
clk_set_rate() with a frequency below the minimum that it lies somewhere
between the rate 1 and rate 2. The expectation should only check that it
is within the range and not exactly the minimum because we're not
testing the clk provider implementation of the rounding here, just that
the constraints are satisfied and the rate is within range. That's my
understanding of the comment above the function and the function name.