All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maxime Ripard <maxime@cerno.tech>
To: Mike Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	linux-clk@vger.kernel.org
Cc: Naresh Kamboju <naresh.kamboju@linaro.org>,
	Alexander Stein <alexander.stein@ew.tq-group.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Tony Lindgren <tony@atomide.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Yassine Oudjana <y.oudjana@protonmail.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Maxime Ripard <maxime@cerno.tech>
Subject: [PATCH 01/22] clk: Drop the rate range on clk_put()
Date: Fri,  8 Apr 2022 11:10:16 +0200	[thread overview]
Message-ID: <20220408091037.2041955-2-maxime@cerno.tech> (raw)
In-Reply-To: <20220408091037.2041955-1-maxime@cerno.tech>

When clk_put() is called we don't make another clk_set_rate() call to
re-evaluate the rate boundaries. This is unlike clk_set_rate_range()
that evaluates the rate again each time it is called.

However, clk_put() is essentially equivalent to clk_set_rate_range()
since after clk_put() completes the consumer's boundaries shouldn't be
enforced anymore.

Let's add a call to clk_set_rate_range() in clk_put() to make sure those
rate boundaries are dropped and the clock provider drivers can react. In
order to be as non-intrusive as possible, we'll just make that call if
the clock had non-default boundaries.

Also add a few tests to make sure this case is covered.

Fixes: c80ac50cbb37 ("clk: Always set the rate on clk_set_range_rate")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/clk/clk.c      |  45 +++++++++++------
 drivers/clk/clk_test.c | 108 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 139 insertions(+), 14 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ed119182aa1b..4ccf52d31a21 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2332,19 +2332,15 @@ int clk_set_rate_exclusive(struct clk *clk, unsigned long rate)
 }
 EXPORT_SYMBOL_GPL(clk_set_rate_exclusive);
 
-/**
- * clk_set_rate_range - set a rate range for a clock source
- * @clk: clock source
- * @min: desired minimum clock rate in Hz, inclusive
- * @max: desired maximum clock rate in Hz, inclusive
- *
- * Returns success (0) or negative errno.
- */
-int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max)
+static int clk_set_rate_range_nolock(struct clk *clk,
+				     unsigned long min,
+				     unsigned long max)
 {
 	int ret = 0;
 	unsigned long old_min, old_max, rate;
 
+	lockdep_assert_held(&prepare_lock);
+
 	if (!clk)
 		return 0;
 
@@ -2357,8 +2353,6 @@ int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max)
 		return -EINVAL;
 	}
 
-	clk_prepare_lock();
-
 	if (clk->exclusive_count)
 		clk_core_rate_unprotect(clk->core);
 
@@ -2402,6 +2396,28 @@ int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max)
 	if (clk->exclusive_count)
 		clk_core_rate_protect(clk->core);
 
+	return ret;
+}
+
+/**
+ * clk_set_rate_range - set a rate range for a clock source
+ * @clk: clock source
+ * @min: desired minimum clock rate in Hz, inclusive
+ * @max: desired maximum clock rate in Hz, inclusive
+ *
+ * Return: 0 for success or negative errno on failure.
+ */
+int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max)
+{
+	int ret;
+
+	if (!clk)
+		return 0;
+
+	clk_prepare_lock();
+
+	ret = clk_set_rate_range_nolock(clk, min, max);
+
 	clk_prepare_unlock();
 
 	return ret;
@@ -4403,9 +4419,10 @@ void __clk_put(struct clk *clk)
 	}
 
 	hlist_del(&clk->clks_node);
-	if (clk->min_rate > clk->core->req_rate ||
-	    clk->max_rate < clk->core->req_rate)
-		clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
+
+	/* If we had any boundaries on that clock, let's drop them. */
+	if (clk->min_rate > 0 || clk->max_rate < ULONG_MAX)
+		clk_set_rate_range_nolock(clk, 0, ULONG_MAX);
 
 	owner = clk->core->owner;
 	kref_put(&clk->core->ref, __clk_release);
diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
index 6731a822f4e3..fd2339cc5898 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -760,9 +760,65 @@ static void clk_range_test_multiple_set_range_rate_maximized(struct kunit *test)
 	clk_put(user1);
 }
 
+/*
+ * 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, including when a user drop its clock.
+ *
+ * With clk_dummy_maximize_rate_ops, this means that the rate will
+ * trail along the maximum as it evolves.
+ */
+static void clk_range_test_multiple_set_range_rate_put_maximized(struct kunit *test)
+{
+	struct clk_dummy_context *ctx = test->priv;
+	struct clk_hw *hw = &ctx->hw;
+	struct clk *clk = hw->clk;
+	struct clk *user1, *user2;
+	unsigned long rate;
+
+	user1 = clk_hw_get_clk(hw, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, user1);
+
+	user2 = clk_hw_get_clk(hw, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, user2);
+
+	KUNIT_ASSERT_EQ(test,
+			clk_set_rate(clk, DUMMY_CLOCK_RATE_2 + 1000),
+			0);
+
+	KUNIT_ASSERT_EQ(test,
+			clk_set_rate_range(user1,
+					   0,
+					   DUMMY_CLOCK_RATE_2),
+			0);
+
+	rate = clk_get_rate(clk);
+	KUNIT_ASSERT_GT(test, rate, 0);
+	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+
+	KUNIT_ASSERT_EQ(test,
+			clk_set_rate_range(user2,
+					   0,
+					   DUMMY_CLOCK_RATE_1),
+			0);
+
+	rate = clk_get_rate(clk);
+	KUNIT_ASSERT_GT(test, rate, 0);
+	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+
+	clk_put(user2);
+
+	rate = clk_get_rate(clk);
+	KUNIT_ASSERT_GT(test, rate, 0);
+	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+
+	clk_put(user1);
+}
+
 static struct kunit_case clk_range_maximize_test_cases[] = {
 	KUNIT_CASE(clk_range_test_set_range_rate_maximized),
 	KUNIT_CASE(clk_range_test_multiple_set_range_rate_maximized),
+	KUNIT_CASE(clk_range_test_multiple_set_range_rate_put_maximized),
 	{}
 };
 
@@ -877,9 +933,61 @@ static void clk_range_test_multiple_set_range_rate_minimized(struct kunit *test)
 	clk_put(user1);
 }
 
+/*
+ * 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, including when a user drop its clock.
+ *
+ * With clk_dummy_minimize_rate_ops, this means that the rate will
+ * trail along the minimum as it evolves.
+ */
+static void clk_range_test_multiple_set_range_rate_put_minimized(struct kunit *test)
+{
+	struct clk_dummy_context *ctx = test->priv;
+	struct clk_hw *hw = &ctx->hw;
+	struct clk *clk = hw->clk;
+	struct clk *user1, *user2;
+	unsigned long rate;
+
+	user1 = clk_hw_get_clk(hw, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, user1);
+
+	user2 = clk_hw_get_clk(hw, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, user2);
+
+	KUNIT_ASSERT_EQ(test,
+			clk_set_rate_range(user1,
+					   DUMMY_CLOCK_RATE_1,
+					   ULONG_MAX),
+			0);
+
+	rate = clk_get_rate(clk);
+	KUNIT_ASSERT_GT(test, rate, 0);
+	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+
+	KUNIT_ASSERT_EQ(test,
+			clk_set_rate_range(user2,
+					   DUMMY_CLOCK_RATE_2,
+					   ULONG_MAX),
+			0);
+
+	rate = clk_get_rate(clk);
+	KUNIT_ASSERT_GT(test, rate, 0);
+	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+
+	clk_put(user2);
+
+	rate = clk_get_rate(clk);
+	KUNIT_ASSERT_GT(test, rate, 0);
+	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+
+	clk_put(user1);
+}
+
 static struct kunit_case clk_range_minimize_test_cases[] = {
 	KUNIT_CASE(clk_range_test_set_range_rate_minimized),
 	KUNIT_CASE(clk_range_test_multiple_set_range_rate_minimized),
+	KUNIT_CASE(clk_range_test_multiple_set_range_rate_put_minimized),
 	{}
 };
 
-- 
2.35.1


  reply	other threads:[~2022-04-08  9:13 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-08  9:10 [PATCH 00/22] clk: More clock rate fixes and tests Maxime Ripard
2022-04-08  9:10 ` Maxime Ripard [this message]
2022-04-08  9:10 ` [PATCH 02/22] clk: tests: Add test suites description Maxime Ripard
2022-04-23  4:06   ` Stephen Boyd
2022-04-08  9:10 ` [PATCH 03/22] clk: tests: Add reference to the orphan mux bug report Maxime Ripard
2022-04-08  9:10 ` [PATCH 04/22] clk: tests: Add tests for uncached clock Maxime Ripard
2022-04-08  9:10 ` [PATCH 05/22] clk: tests: Add tests for single parent mux Maxime Ripard
2022-04-08  9:10 ` [PATCH 06/22] clk: tests: Add tests for mux with multiple parents Maxime Ripard
2022-04-08  9:10 ` [PATCH 07/22] clk: tests: Add some tests for orphan " Maxime Ripard
2022-04-08  9:10 ` [PATCH 08/22] clk: Take into account uncached clocks in clk_set_rate_range() Maxime Ripard
2022-04-08  9:10 ` [PATCH 09/22] clk: Fix clk_get_parent() documentation Maxime Ripard
2022-04-08  9:10 ` [PATCH 10/22] clk: Set req_rate on reparenting Maxime Ripard
2022-04-08  9:10 ` [PATCH 11/22] clk: Skip set_rate_range if our clock is orphan Maxime Ripard
2022-04-08  9:10 ` [PATCH 12/22] clk: Add our request boundaries in clk_core_init_rate_req Maxime Ripard
2022-04-08  9:10 ` [PATCH 13/22] clk: Change clk_core_init_rate_req prototype Maxime Ripard
2022-04-08  9:10 ` [PATCH 14/22] clk: Introduce clk_hw_init_rate_request() Maxime Ripard
2022-04-23  3:46   ` Stephen Boyd
2022-04-23  7:17     ` Maxime Ripard
2022-04-08  9:10 ` [PATCH 15/22] clk: Add missing clk_core_init_rate_req calls Maxime Ripard
2022-04-23  3:51   ` Stephen Boyd
2022-04-23  7:32     ` Maxime Ripard
2022-04-08  9:10 ` [PATCH 16/22] clk: Remove redundant clk_core_init_rate_req() call Maxime Ripard
2022-04-23  4:02   ` Stephen Boyd
2022-04-23  7:44     ` Maxime Ripard
2022-04-08  9:10 ` [PATCH 17/22] clk: Switch from __clk_determine_rate to clk_core_round_rate_nolock Maxime Ripard
2022-04-08  9:10 ` [PATCH 18/22] clk: Introduce clk_core_has_parent() Maxime Ripard
2022-04-08  9:10 ` [PATCH 19/22] clk: Stop forwarding clk_rate_requests to the parent Maxime Ripard
2022-04-08  9:10 ` [PATCH 20/22] clk: Zero the clk_rate_request structure Maxime Ripard
2022-04-08  9:10 ` [PATCH 21/22] clk: Test the clock pointer in clk_hw_get_name() Maxime Ripard
2022-04-08  9:10 ` [PATCH 22/22] clk: Prevent a clock without a rate to register Maxime Ripard
2022-04-08  9:18   ` Jerome Brunet
2022-04-08 10:41     ` Maxime Ripard
2022-04-08 11:24       ` Jerome Brunet
2022-04-08 12:55         ` Maxime Ripard
2022-04-08 14:48           ` Jerome Brunet
2022-04-08 15:36             ` Maxime Ripard
2022-04-11  7:40               ` Neil Armstrong
2022-04-12 12:56                 ` Maxime Ripard
2022-04-11  8:20               ` Jerome Brunet
2022-04-23  4:42       ` Stephen Boyd
2022-04-23  9:17         ` Maxime Ripard
2022-04-29  2:08           ` Stephen Boyd
2022-04-29 15:45             ` Maxime Ripard
2022-04-08 12:17     ` Marek Szyprowski
2022-04-08 12:25       ` Maxime Ripard
2022-04-08 13:46         ` Marek Szyprowski
2022-04-23  4:12   ` Stephen Boyd
2022-04-23  7:49     ` Maxime Ripard
2022-04-10 12:06 ` [PATCH 00/22] clk: More clock rate fixes and tests Yassine Oudjana
2022-04-11 11:39   ` Maxime Ripard
2022-04-11  6:25 ` (EXT) " Alexander Stein
2022-04-11  7:24   ` Alexander Stein
2022-04-11 11:54     ` Maxime Ripard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220408091037.2041955-2-maxime@cerno.tech \
    --to=maxime@cerno.tech \
    --cc=alexander.stein@ew.tq-group.com \
    --cc=jbrunet@baylibre.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=mturquette@baylibre.com \
    --cc=naresh.kamboju@linaro.org \
    --cc=narmstrong@baylibre.com \
    --cc=sboyd@kernel.org \
    --cc=tony@atomide.com \
    --cc=y.oudjana@protonmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.