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

[Maxime Ripard]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220225/9dccaf0e/attachment-0001.sig>