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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d9e2f63519f3fcbf4fe334975691d573e20c53c8.camel@linaro.org>
Date: Fri, 09 Aug 2024 10:07:12 +0100
From: André Draszik <andre.draszik@...aro.org>
To: Peng Fan <peng.fan@....com>, Michael Turquette
 <mturquette@...libre.com>,  Stephen Boyd <sboyd@...nel.org>, Peter Griffin
 <peter.griffin@...aro.org>, Krzysztof Kozlowski <krzk@...nel.org>,
 Sylwester Nawrocki <s.nawrocki@...sung.com>, Chanwoo Choi
 <cw00.choi@...sung.com>, Alim Akhtar <alim.akhtar@...sung.com>, Sam
 Protsenko <semen.protsenko@...aro.org>, Tudor Ambarus
 <tudor.ambarus@...aro.org>, Abel Vesa <abelvesa@...nel.org>, Shawn Guo
 <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix
 Kernel Team <kernel@...gutronix.de>,  Fabio Estevam <festevam@...il.com>
Cc: Will McVicker <willmcvicker@...gle.com>, "kernel-team@...roid.com"
 <kernel-team@...roid.com>, "linux-clk@...r.kernel.org"
 <linux-clk@...r.kernel.org>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>, "linux-samsung-soc@...r.kernel.org"
 <linux-samsung-soc@...r.kernel.org>, "imx@...ts.linux.dev"
 <imx@...ts.linux.dev>
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:
> Hi Peng,
> 
> On Fri, 2024-08-09 at 07:16 +0000, Peng Fan wrote:
> > > +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?
> 
> > > +
> > >  /**
> > >   * 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.

Did I miss something?

Cheers,
Andre'