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: Dmitry Osipenko <dmitry.osipenko@collabora.com>,
	Maxime Ripard <maxime@cerno.tech>
Subject: [PATCH v2 3/3] clk: Drop the rate range on clk_put
Date: Fri, 25 Mar 2022 17:11:44 +0100	[thread overview]
Message-ID: <20220325161144.1901695-4-maxime@cerno.tech> (raw)
In-Reply-To: <20220325161144.1901695-1-maxime@cerno.tech>

While the current code will trigger a new clk_set_rate call whenever the
rate boundaries are changed through clk_set_rate_range, this doesn't
occur when clk_put() is called.

However, this is essentially equivalent since, after clk_put()
completes, those boundaries won'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 drivers can react.

Let's 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      |  42 ++++++++++------
 drivers/clk/clk_test.c | 108 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+), 14 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 915a2fa363b1..91f863b7a824 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
+ *
+ * Returns success (0) or negative errno.
+ */
+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,7 @@ 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);
+	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 146b1759798e..b205c329cf32 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 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 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


  parent reply	other threads:[~2022-03-25 16:11 UTC|newest]

Thread overview: 93+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-25 16:11 [PATCH v2 0/3] clk: Some Clock Range Fixes Maxime Ripard
2022-03-25 16:11 ` [PATCH v2 1/3] clk: Initialize orphan req_rate Maxime Ripard
2022-03-29 18:36   ` Stephen Boyd
2022-03-25 16:11 ` [PATCH v2 2/3] clk: test: Test clk_set_rate_range on orphan mux Maxime Ripard
2022-03-29 18:36   ` Stephen Boyd
2022-03-25 16:11 ` Maxime Ripard [this message]
2022-03-29 18:36   ` [PATCH v2 3/3] clk: Drop the rate range on clk_put Stephen Boyd
     [not found]   ` <CGME20220330080612eucas1p195caaf35d900412de762a27ae02b7b9e@eucas1p1.samsung.com>
2022-03-30  8:06     ` Marek Szyprowski
2022-03-30  8:06       ` Marek Szyprowski
2022-03-30  8:47       ` Maxime Ripard
2022-03-30  8:47         ` Maxime Ripard
2022-03-31  9:42         ` Tony Lindgren
2022-03-31  9:42           ` Tony Lindgren
2022-03-31  9:42           ` Tony Lindgren
2022-03-31  9:54           ` Maxime Ripard
2022-03-31  9:54             ` Maxime Ripard
2022-03-31  9:54             ` Maxime Ripard
2022-03-31 15:00             ` Tony Lindgren
2022-03-31 15:00               ` Tony Lindgren
2022-03-31 15:00               ` Tony Lindgren
2022-03-31 15:31               ` Maxime Ripard
2022-03-31 15:31                 ` Maxime Ripard
2022-03-31 15:31                 ` Maxime Ripard
2022-03-31 17:00                 ` Tony Lindgren
2022-03-31 17:00                   ` Tony Lindgren
2022-03-31 17:00                   ` Tony Lindgren
2022-03-31 21:58                   ` Stephen Boyd
2022-03-31 21:58                     ` Stephen Boyd
2022-03-31 21:58                     ` Stephen Boyd
2022-04-01 12:28                     ` Maxime Ripard
2022-04-01 12:28                       ` Maxime Ripard
2022-04-01 12:28                       ` Maxime Ripard
2022-04-03  2:14                       ` Stephen Boyd
2022-04-03  2:14                         ` Stephen Boyd
2022-04-03  2:14                         ` Stephen Boyd
2022-04-01 11:55                 ` (EXT) " Alexander Stein
2022-04-01 11:55                   ` Alexander Stein
2022-04-01 11:55                   ` Alexander Stein
2022-04-01 12:27                   ` Maxime Ripard
2022-04-01 12:27                     ` Maxime Ripard
2022-04-01 12:27                     ` Maxime Ripard
2022-04-01 12:59                     ` (EXT) " Alexander Stein
2022-04-01 12:59                       ` Alexander Stein
2022-04-01 12:59                       ` Alexander Stein
2022-04-01 13:04                       ` Maxime Ripard
2022-04-01 13:04                         ` Maxime Ripard
2022-04-01 13:04                         ` Maxime Ripard
2022-04-01 13:07                         ` (EXT) " Alexander Stein
2022-04-01 13:07                           ` Alexander Stein
2022-04-01 13:07                           ` Alexander Stein
2022-04-01 13:34                           ` Maxime Ripard
2022-04-01 13:34                             ` Maxime Ripard
2022-04-01 13:34                             ` Maxime Ripard
2022-04-01 13:49                             ` (EXT) " Alexander Stein
2022-04-01 13:49                               ` Alexander Stein
2022-04-01 13:49                               ` Alexander Stein
2022-04-01 14:55                               ` Maxime Ripard
2022-04-01 14:55                                 ` Maxime Ripard
2022-04-01 14:55                                 ` Maxime Ripard
2022-04-04  7:06                                 ` (EXT) " Alexander Stein
2022-04-04  7:06                                   ` Alexander Stein
2022-04-04  7:06                                   ` Alexander Stein
2022-04-04  7:27                                   ` Maxime Ripard
2022-04-04  7:27                                     ` Maxime Ripard
2022-04-04  7:27                                     ` Maxime Ripard
2022-04-04 10:54                                     ` (EXT) " Alexander Stein
2022-04-04 10:54                                       ` Alexander Stein
2022-04-04 10:54                                       ` Alexander Stein
2022-04-07  8:09                                       ` Maxime Ripard
2022-04-07  8:09                                         ` Maxime Ripard
2022-04-07  8:09                                         ` Maxime Ripard
2022-04-02 17:01                     ` Maxime Ripard
2022-04-02 17:01                       ` Maxime Ripard
2022-04-02 17:01                       ` Maxime Ripard
2022-04-07  7:53           ` Maxime Ripard
2022-04-07  7:53             ` Maxime Ripard
2022-04-07  7:53             ` Maxime Ripard
2022-04-07  8:03             ` Tony Lindgren
2022-04-07  8:03               ` Tony Lindgren
2022-04-07  8:03               ` Tony Lindgren
2022-04-07 11:08               ` Tony Lindgren
2022-04-07 11:08                 ` Tony Lindgren
2022-04-07 11:08                 ` Tony Lindgren
2022-04-07 13:45                 ` Maxime Ripard
2022-04-07 13:45                   ` Maxime Ripard
2022-04-07 13:45                   ` Maxime Ripard
2022-04-08  5:03                   ` Tony Lindgren
2022-04-08  5:03                     ` Tony Lindgren
2022-04-08  5:03                     ` Tony Lindgren
2022-03-31  9:56         ` Marek Szyprowski
2022-03-31  9:56           ` Marek Szyprowski
2022-03-31 10:19           ` Maxime Ripard
2022-03-31 10:19             ` 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=20220325161144.1901695-4-maxime@cerno.tech \
    --to=maxime@cerno.tech \
    --cc=dmitry.osipenko@collabora.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.org \
    /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.