linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Revert "clk: Drop the rate range on clk_put()"
@ 2022-04-03  2:28 Stephen Boyd
  2022-04-04  7:29 ` Maxime Ripard
  0 siblings, 1 reply; 3+ messages in thread
From: Stephen Boyd @ 2022-04-03  2:28 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Marek Szyprowski, Tony Lindgren,
	Alexander Stein, Naresh Kamboju, Maxime Ripard

This reverts commit 7dabfa2bc4803eed83d6f22bd6f045495f40636b. There are
multiple reports that this breaks boot on various systems. The common
theme is that orphan clks are having rates set on them when that isn't
expected. Let's revert it out for now so that -rc1 boots.

Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reported-by: Tony Lindgren <tony@atomide.com>
Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Link: https://lore.kernel.org/r/366a0232-bb4a-c357-6aa8-636e398e05eb@samsung.com
Cc: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clk.c      |  42 ++++++----------
 drivers/clk/clk_test.c | 108 -----------------------------------------
 2 files changed, 14 insertions(+), 136 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 07a27b65b773..ed119182aa1b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2332,15 +2332,19 @@ int clk_set_rate_exclusive(struct clk *clk, unsigned long rate)
 }
 EXPORT_SYMBOL_GPL(clk_set_rate_exclusive);
 
-static int clk_set_rate_range_nolock(struct clk *clk,
-				     unsigned long min,
-				     unsigned long max)
+/**
+ * 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 = 0;
 	unsigned long old_min, old_max, rate;
 
-	lockdep_assert_held(&prepare_lock);
-
 	if (!clk)
 		return 0;
 
@@ -2353,6 +2357,8 @@ static int clk_set_rate_range_nolock(struct clk *clk,
 		return -EINVAL;
 	}
 
+	clk_prepare_lock();
+
 	if (clk->exclusive_count)
 		clk_core_rate_unprotect(clk->core);
 
@@ -2396,28 +2402,6 @@ static int clk_set_rate_range_nolock(struct clk *clk,
 	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;
@@ -4419,7 +4403,9 @@ void __clk_put(struct clk *clk)
 	}
 
 	hlist_del(&clk->clks_node);
-	clk_set_rate_range_nolock(clk, 0, ULONG_MAX);
+	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);
 
 	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 fd2339cc5898..6731a822f4e3 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -760,65 +760,9 @@ 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),
 	{}
 };
 
@@ -933,61 +877,9 @@ 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),
 	{}
 };
 
-- 
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Revert "clk: Drop the rate range on clk_put()"
  2022-04-03  2:28 [PATCH] Revert "clk: Drop the rate range on clk_put()" Stephen Boyd
@ 2022-04-04  7:29 ` Maxime Ripard
  2022-04-04 20:05   ` Stephen Boyd
  0 siblings, 1 reply; 3+ messages in thread
From: Maxime Ripard @ 2022-04-04  7:29 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, linux-kernel, linux-clk, Marek Szyprowski,
	Tony Lindgren, Alexander Stein, Naresh Kamboju

On Sat, Apr 02, 2022 at 07:28:18PM -0700, Stephen Boyd wrote:
> This reverts commit 7dabfa2bc4803eed83d6f22bd6f045495f40636b. There are
> multiple reports that this breaks boot on various systems. The common
> theme is that orphan clks are having rates set on them when that isn't
> expected. Let's revert it out for now so that -rc1 boots.
> 
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Reported-by: Tony Lindgren <tony@atomide.com>
> Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Link: https://lore.kernel.org/r/366a0232-bb4a-c357-6aa8-636e398e05eb@samsung.com
> Cc: Maxime Ripard <maxime@cerno.tech>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>

I really like the attention it's getting now that it's broken, we can
fix a lot of things :)

It doesn't seem to be restricted to orphan clocks though :/

But obviously,
Acked-by: Maxime Ripard <maxime@cerno.tech>

Thanks!
Maxime

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Revert "clk: Drop the rate range on clk_put()"
  2022-04-04  7:29 ` Maxime Ripard
@ 2022-04-04 20:05   ` Stephen Boyd
  0 siblings, 0 replies; 3+ messages in thread
From: Stephen Boyd @ 2022-04-04 20:05 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Michael Turquette, linux-kernel, linux-clk, Marek Szyprowski,
	Tony Lindgren, Alexander Stein, Naresh Kamboju

Quoting Maxime Ripard (2022-04-04 00:29:00)
> On Sat, Apr 02, 2022 at 07:28:18PM -0700, Stephen Boyd wrote:
> > This reverts commit 7dabfa2bc4803eed83d6f22bd6f045495f40636b. There are
> > multiple reports that this breaks boot on various systems. The common
> > theme is that orphan clks are having rates set on them when that isn't
> > expected. Let's revert it out for now so that -rc1 boots.
> > 
> > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Reported-by: Tony Lindgren <tony@atomide.com>
> > Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> > Link: https://lore.kernel.org/r/366a0232-bb4a-c357-6aa8-636e398e05eb@samsung.com
> > Cc: Maxime Ripard <maxime@cerno.tech>
> > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> 
> I really like the attention it's getting now that it's broken, we can
> fix a lot of things :)

Sure! Except that can quickly turn into other attention.

> 
> It doesn't seem to be restricted to orphan clocks though :/

Oof ok. I was busy last week so couldn't pay much attention.

> 
> But obviously,
> Acked-by: Maxime Ripard <maxime@cerno.tech>

Thanks. Looks like it just made -rc1 so we can work through the fix at a
more leisurely pace now.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-04-04 21:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-03  2:28 [PATCH] Revert "clk: Drop the rate range on clk_put()" Stephen Boyd
2022-04-04  7:29 ` Maxime Ripard
2022-04-04 20:05   ` Stephen Boyd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).