Hi,
On Thu, Feb 24, 2022 at 02:39:20PM -0800, Stephen Boyd wrote:
> 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.
You're right, that has been addressed in the last version I sent already
Maxime
Attachment:
signature.asc
Description: PGP signature
