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

On Fri, 2024-08-09 at 10:02 +0100, André Draszik wrote:

quoted

Hi Peng,

On Fri, 2024-08-09 at 07:16 +0000, Peng Fan wrote:

quoted

quoted

+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.

You're right.

With this in mind, I'm not sure then if a generic solution is possible...

I guess it has to be duplicated into the platforms after all and
platforms can enable this if they opt to disallow uart as module?

Any other suggestions?

quoted

quoted

+
 /**
  * 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.

until late_initcall_sync(), at which point it'll be disabled before
the uart driver has probed, yes :-(

Sorry, ignore that. If clock driver is built as module, the code to bump
the clocks is disabled by the time this code runs (due to setting the flag
as part of late_initcall_sync(of_clk_drop_stdout_clocks)), in other
words it will not bump the clocks at all in that case and behaviour is as
before.

Just checked again, you are right.
If the clock driver is built at module and "earlycon" is set in bootargs,
of_clk_stdout_clks.bump_refs will be true. late_initcall_sync will
set it back to false.

Regards,
Peng.

Did I miss something?

Cheers,
Andre'