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

[Peng Fan]

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

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'