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