Thread (28 messages) 28 messages, 3 authors, 2024-08-10

RE: [PATCH v6 01/20] clk: bump stdout clock usage for earlycon

From: Peng Fan <peng.fan@nxp.com>
Date: 2024-08-09 07:16:13
Also in: imx, linux-clk, linux-samsung-soc, lkml

quoted hunk ↗ jump to hunk
Subject: [PATCH v6 01/20] clk: bump stdout clock usage for earlycon

On some platforms, earlycon depends on the bootloader setup stdout
clocks being retained. In some cases stdout UART clocks (or their
parents) can get disabled during loading of other drivers (e.g. i2c)
causing earlycon to stop to work sometime into the boot, halting the
whole system.

Since there are at least two platforms where that is the case, i.MX and
the Exynos-derivative gs101, this patch adds some logic to the clk core
to detect these clocks if earlycon is enabled, to bump their usage count
as part of of_clk_add_hw_provider() and of_clk_add_provider(), and to
release them again at the end of init.

This way code duplication in affected platforms can be avoided.

The general idea is based on similar code in the i.MX clock driver, but
this here is a bit more generic as in general (e.g. on gs101) clocks can
come from various different clock units (driver instances) and therefore
it can be necessary to run this code multiple times until all required
stdout clocks have probed.

Signed-off-by: André Draszik <andre.draszik@linaro.org>

---
v6:
* drop a stray #include from drivers/clk/samsung/clk-gs101.c
---
 drivers/clk/clk.c | 129
++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index
7264cf6165ce..03c5d80e833c 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -4923,6 +4923,131 @@ static void
clk_core_reparent_orphans(void)
 	clk_prepare_unlock();
 }

+/**
+ * struct of_clk_stdout_clks - holds data that is required for handling
+extra
+ * references to stdout clocks during early boot.
+ *
+ * On some platforms, earlycon depends on the bootloader setup
stdout
+clocks
+ * being retained. In some cases stdout UART clocks (or their parents)
+can get
+ * disabled during loading of other drivers (e.g. i2c) causing earlycon
+to stop
+ * to work sometime into the boot, halting the system.
+ *
+ * Having logic to detect these clocks if earlycon is enabled helps
+with those
+ * cases by bumping their usage count during init. The extra usage
+count is
+ * later dropped at the end of init.
+ *
+ * @bump_refs: whether or not to add the extra stdout clock
references
+ * @lock: mutex protecting access
+ * @have_all: whether or not we have acquired all clocks, to handle
cases of
+ *            clocks coming from different drivers / instances
+ * @clks: clocks associated with stdout
+ * @n_clks: number of clocks associated with stdout  */ static struct
+of_clk_stdout_clks {
+	bool bump_refs;
+
+	struct mutex lock;
+	bool have_all;
+	struct clk **clks;
+	size_t n_clks;
+} of_clk_stdout_clks = {
+	.lock = __MUTEX_INITIALIZER(of_clk_stdout_clks.lock),
+};
+
+static int __init of_clk_bump_stdout_clocks_param(char *str) {
+	of_clk_stdout_clks.bump_refs = true;
+	return 0;
+}
+__setup("earlycon", of_clk_bump_stdout_clocks_param);
+__setup_param("earlyprintk", of_clk_keep_stdout_clocks_earlyprintk,
+	      of_clk_bump_stdout_clocks_param, 0);
+
+static void of_clk_bump_stdout_clocks(void) {
+	size_t n_clks;
+
+	/*
+	 * We only need to run this code if required to do so and only
ever
+	 * before late initcalls have run. Otherwise it'd be impossible to
know
+	 * when to drop the extra clock references again.
+	 *
+	 * This generally means that this only works if on affected
platforms
+	 * the clock drivers have been built-in (as opposed to being
modules).
+	 */
+	if (!of_clk_stdout_clks.bump_refs)
+		return;
+
+	n_clks = of_clk_get_parent_count(of_stdout);
+	if (!n_clks || !of_stdout)
+		return;
+
+	mutex_lock(&of_clk_stdout_clks.lock);
+
+	/*
+	 * We only need to keep trying if we have not succeeded
previously,
+	 * i.e. if not all required clocks were ready during previous
attempts.
+	 */
+	if (of_clk_stdout_clks.have_all)
+		goto out_unlock;
+
+	if (!of_clk_stdout_clks.clks) {
+		of_clk_stdout_clks.n_clks = n_clks;
+
+		of_clk_stdout_clks.clks =
kcalloc(of_clk_stdout_clks.n_clks,
+
sizeof(*of_clk_stdout_clks.clks),
+					      GFP_KERNEL);
+		if (!of_clk_stdout_clks.clks)
+			goto out_unlock;
+	}
+
+	/* assume that this time we'll be able to grab all required
clocks */
+	of_clk_stdout_clks.have_all = true;
+	for (size_t i = 0; i < n_clks; ++i) {
+		struct clk *clk;
+
+		/* we might have grabbed this clock in a previous
attempt */
+		if (of_clk_stdout_clks.clks[i])
+			continue;
+
+		clk = of_clk_get(of_stdout, i);
+		if (IS_ERR(clk)) {
+			/* retry next time if clock has not probed yet
*/
+			of_clk_stdout_clks.have_all = false;
+			continue;
+		}
+
+		if (clk_prepare_enable(clk)) {
+			clk_put(clk);
+			continue;
+		}
+		of_clk_stdout_clks.clks[i] = clk;
+	}
+
+out_unlock:
+	mutex_unlock(&of_clk_stdout_clks.lock);
+}
+
+static int __init of_clk_drop_stdout_clocks(void) {
+	for (size_t i = 0; i < of_clk_stdout_clks.n_clks; ++i) {
+		clk_disable_unprepare(of_clk_stdout_clks.clks[i]);
+		clk_put(of_clk_stdout_clks.clks[i]);
+	}
+
+	kfree(of_clk_stdout_clks.clks);
+
+	/*
+	 * Do not try to acquire stdout clocks after late initcalls, e.g.
+	 * during further module loading, as we then wouldn't have a
way to
+	 * drop the references (and associated allocations) ever again.
+	 */
+	of_clk_stdout_clks.bump_refs = false;
+
+	return 0;
+}
+late_initcall_sync(of_clk_drop_stdout_clocks);
If the uart driver is built as module, this might break earlycon.
Before uart driver loaded, clk disabled per my understanding.
quoted hunk ↗ jump to hunk
+
 /**
  * struct of_clk_provider - Clock provider registration structure
  * @link: Entry in global list of clock providers @@ -5031,6 +5156,8
@@ int of_clk_add_provider(struct device_node *np,

 	fwnode_dev_initialized(&np->fwnode, true);

+	of_clk_bump_stdout_clocks();
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(of_clk_add_provider);
@@ -5073,6 +5200,8 @@ int of_clk_add_hw_provider(struct
device_node *np,

 	fwnode_dev_initialized(&np->fwnode, true);

+	of_clk_bump_stdout_clocks();
If clock driver is built as module,  the will make the
clocks will be always enabled, if my understanding is correct.


Regards,
Peng.
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(of_clk_add_hw_provider);

--
2.46.0.rc2.264.g509ed76dc8-goog
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help