[PATCH v3 15/15] usb: ohci-at91: fix possible hang chainloading barebox

Ahmad Fatoum a.fatoum at pengutronix.de
Thu Feb 15 08:30:09 PST 2024


barebox, like Linux, will consider disabling parents when disabling a
child clock. In the AT91 OHCI driver ported to barebox from Linux, this
leads to the USB clock shutdown during device shutdown to propagate up
to PLLB, which is also disabled.

On probe of the kernel driver, the USB clock rate is set to 48MHz, which
propagates up to the PLL, which is powered on again. In barebox, this clock
rate propagation does not happen and the PLL is only initially
configured in the first stage bootloader.

This has the effect that chainloading barebox from within barebox will
hang as the first barebox disables PLLB on shutdown and the second
barebox never power it on.

The proper solution would be to support propagation of clock rate change
requests, but till we have that, patch the driver, so only the immediate
clock is disabled and not its parents.

Signed-off-by: Ahmad Fatoum <a.fatoum at pengutronix.de>
---
v3:
  - new patch
---
 drivers/clk/clk.c            | 12 +++++++++---
 drivers/usb/host/ohci-at91.c | 13 +++++++++++--
 include/linux/clk.h          | 20 ++++++++++++++++++--
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index d3f5d5e83880..03533b61df0a 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -66,7 +66,7 @@ int clk_enable(struct clk *clk)
 	return 0;
 }
 
-void clk_disable(struct clk *clk)
+void clk_disable_one(struct clk *clk)
 {
 	struct clk_hw *hw;
 
@@ -91,11 +91,17 @@ void clk_disable(struct clk *clk)
 	if (!clk->enable_count) {
 		if (clk->ops->disable)
 			clk->ops->disable(hw);
-
-		clk_parent_disable(clk);
 	}
 }
 
+void clk_disable(struct clk *clk)
+{
+	clk_disable_one(clk);
+
+	if (!IS_ERR_OR_NULL(clk) && !clk->enable_count)
+		clk_parent_disable(clk);
+}
+
 unsigned long clk_get_rate(struct clk *clk)
 {
 	struct clk_hw *hw;
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 867c0977be78..447d928ad4ce 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -50,8 +50,17 @@ static int at91_start_clock(struct ohci_at91_priv *ohci_at91)
 
 static void at91_stop_clock(struct ohci_at91_priv *ohci_at91)
 {
-	clk_disable(ohci_at91->fclk);
-	clk_disable(ohci_at91->iclk);
+	/*
+	 * We don't want to use clk_disable() here as that would
+	 * propagate up until PLLB is disabled breaking chainloadig
+	 * barebox from barebox. The proper solution would be to
+	 * set rate to 48MHz in at91_start_clock() and teach the CCF
+	 * to propagate up rate requests like Linux does, but till we
+	 * have that, we take the easy way out and ensure PLLB remains
+	 * enabled with the parameters that the first stage configured.
+	 */
+	clk_disable_one(ohci_at91->fclk);
+	clk_disable_one(ohci_at91->iclk);
 }
 
 static int at91_ohci_probe_dt(struct device *dev)
diff --git a/include/linux/clk.h b/include/linux/clk.h
index fe0b1ce3e36c..6be6e91e9eee 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -83,11 +83,27 @@ int clk_enable(struct clk *clk);
  *
  * Implementation detail: if the clock source is shared between
  * multiple drivers, clk_enable() calls must be balanced by the
- * same number of clk_disable() calls for the clock source to be
- * disabled.
+ * same number of clk_disable() or clk_disable_one() calls for
+ * the clock source to be disabled.
  */
 void clk_disable(struct clk *clk);
 
+/**
+ * clk_disable_one - inform the system when a specific clock is no longer required.
+ * @clk: clock source
+ *
+ * Inform the system that a clock source is no longer required by
+ * a driver and may be shut down. Unlike clk_disable(), this only
+ * affects the specified @clk and can't result in disabling any
+ * parents.
+ *
+ * Implementation detail: if the clock source is shared between
+ * multiple drivers, clk_enable() calls must be balanced by the
+ * same number of clk_disable() or clk_disable_one() calls for
+ * the clock source to be disabled.
+ */
+void clk_disable_one(struct clk *clk);
+
 /**
  * clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
  *		  This is only valid once the clock source has been enabled.
-- 
2.39.2




More information about the barebox mailing list