Skip to content

Commit

Permalink
Revert "clk: Always set the rate on clk_set_range_rate"
Browse files Browse the repository at this point in the history
This reverts commit c80ac50.
  • Loading branch information
digetx committed Mar 26, 2022
1 parent f52bb34 commit fd47ac9
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 47 deletions.
45 changes: 22 additions & 23 deletions drivers/clk/clk.c
Original file line number Diff line number Diff line change
Expand Up @@ -2373,29 +2373,28 @@ int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max)
goto out;
}

/*
* Since the boundaries have been changed, let's give the
* opportunity to the provider to adjust the clock rate based on
* the new boundaries.
*
* We also need to handle the case where the clock is currently
* outside of the boundaries. Clamping the last requested rate
* to the current minimum and maximum will also handle this.
*
* FIXME:
* There is a catch. It may fail for the usual reason (clock
* broken, clock protected, etc) but also because:
* - round_rate() was not favorable and fell on the wrong
* side of the boundary
* - the determine_rate() callback does not really check for
* this corner case when determining the rate
*/
rate = clamp(clk->core->req_rate, min, max);
ret = clk_core_set_rate_nolock(clk->core, rate);
if (ret) {
/* rollback the changes */
clk->min_rate = old_min;
clk->max_rate = old_max;
rate = clk_core_get_rate_nolock(clk->core);
if (rate < min || rate > max) {
/*
* FIXME:
* We are in bit of trouble here, current rate is outside the
* the requested range. We are going try to request appropriate
* range boundary but there is a catch. It may fail for the
* usual reason (clock broken, clock protected, etc) but also
* because:
* - round_rate() was not favorable and fell on the wrong
* side of the boundary
* - the determine_rate() callback does not really check for
* this corner case when determining the rate
*/

rate = clamp(clk->core->req_rate, min, max);
ret = clk_core_set_rate_nolock(clk->core, rate);
if (ret) {
/* rollback the changes */
clk->min_rate = old_min;
clk->max_rate = old_max;
}
}

out:
Expand Down
54 changes: 30 additions & 24 deletions drivers/clk/clk_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -549,12 +549,13 @@ static struct kunit_suite clk_range_test_suite = {
};

/*
* Test that if we have several subsequent calls to
* clk_set_rate_range(), the core will reevaluate whether a new rate is
* needed each and every time.
* Test that if:
* - we have several subsequent calls to clk_set_rate_range();
* - and we have a round_rate ops that always return the maximum
* frequency allowed;
*
* With clk_dummy_maximize_rate_ops, this means that the rate will
* trail along the maximum as it evolves.
* The clock will run at the minimum of all maximum boundaries
* requested, even if those boundaries aren't there anymore.
*/
static void clk_range_test_set_range_rate_maximized(struct kunit *test)
{
Expand Down Expand Up @@ -595,16 +596,18 @@ static void clk_range_test_set_range_rate_maximized(struct kunit *test)

rate = clk_get_rate(clk);
KUNIT_ASSERT_GT(test, rate, 0);
KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_2 - 1000);
}

/*
* Test that if we have several subsequent calls to
* clk_set_rate_range(), across multiple users, the core will reevaluate
* whether a new rate is needed each and every time.
* Test that if:
* - we have several subsequent calls to clk_set_rate_range(), across
* multiple users;
* - and we have a round_rate ops that always return the maximum
* frequency allowed;
*
* With clk_dummy_maximize_rate_ops, this means that the rate will
* trail along the maximum as it evolves.
* The clock will run at the minimum of all maximum boundaries
* requested, even if those boundaries aren't there anymore.
*/
static void clk_range_test_multiple_set_range_rate_maximized(struct kunit *test)
{
Expand Down Expand Up @@ -650,7 +653,7 @@ static void clk_range_test_multiple_set_range_rate_maximized(struct kunit *test)

rate = clk_get_rate(clk);
KUNIT_ASSERT_GT(test, rate, 0);
KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_1);

clk_put(user2);
clk_put(user1);
Expand All @@ -670,12 +673,13 @@ static struct kunit_suite clk_range_maximize_test_suite = {
};

/*
* Test that if we have several subsequent calls to
* clk_set_rate_range(), the core will reevaluate whether a new rate is
* needed each and every time.
* Test that if:
* - we have several subsequent calls to clk_set_rate_range()
* - and we have a round_rate ops that always return the minimum
* frequency allowed;
*
* With clk_dummy_minimize_rate_ops, this means that the rate will
* trail along the minimum as it evolves.
* The clock will run at the maximum of all minimum boundaries
* requested, even if those boundaries aren't there anymore.
*/
static void clk_range_test_set_range_rate_minimized(struct kunit *test)
{
Expand Down Expand Up @@ -716,16 +720,18 @@ static void clk_range_test_set_range_rate_minimized(struct kunit *test)

rate = clk_get_rate(clk);
KUNIT_ASSERT_GT(test, rate, 0);
KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_1 + 1000);
}

/*
* Test that if we have several subsequent calls to
* clk_set_rate_range(), across multiple users, the core will reevaluate
* whether a new rate is needed each and every time.
* Test that if:
* - we have several subsequent calls to clk_set_rate_range(), across
* multiple users;
* - and we have a round_rate ops that always return the minimum
* frequency allowed;
*
* With clk_dummy_minimize_rate_ops, this means that the rate will
* trail along the minimum as it evolves.
* The clock will run at the maximum of all minimum boundaries
* requested, even if those boundaries aren't there anymore.
*/
static void clk_range_test_multiple_set_range_rate_minimized(struct kunit *test)
{
Expand Down Expand Up @@ -767,7 +773,7 @@ static void clk_range_test_multiple_set_range_rate_minimized(struct kunit *test)

rate = clk_get_rate(clk);
KUNIT_ASSERT_GT(test, rate, 0);
KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_2);

clk_put(user2);
clk_put(user1);
Expand Down

0 comments on commit fd47ac9

Please sign in to comment.