From mboxrd@z Thu Jan 1 00:00:00 1970 From: mturquette@linaro.org (Mike Turquette) Date: Thu, 20 Mar 2014 20:52:33 -0700 Subject: [PATCH v3] clk: respect the clock dependencies in of_clk_init In-Reply-To: <531068F4.3060903@free-electrons.com> References: <1393265413-16641-1-git-send-email-gregory.clement@free-electrons.com> <531068F4.3060903@free-electrons.com> Message-ID: <20140321035233.32624.98694@quantum> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Quoting Gregory CLEMENT (2014-02-28 02:46:12) > Hi Mike, > > On 24/02/2014 19:10, Gregory CLEMENT wrote: > > Until now the clock providers were initialized in the order found in > > the device tree. This led to have the dependencies between the clocks > > not respected: children clocks could be initialized before their > > parent clocks. > > > > Instead of forcing each platform to manage its own initialization order, > > this patch adds this work inside the framework itself. > > > > Using the data of the device tree the of_clk_init function now delayed > > the initialization of a clock provider if its parent provider was not > > ready yet. > > > > The strict dependency check (all parents of a given clk must be > > initialized) was added by Boris BREZILLON > > Are you ok with this version? > Will you take it for 3.15? > Or maybe you expected that it will be part of a pull request? > > However as it is modifying the core of the framework I thought that you > would take it and apply yourself. Hi Gregory, I have taken this into clk-next. If no regressions pop up over the next few days then it should go into 3.15. Thanks, Mike > > Thanks, > > Gregory > > > > > Signed-off-by: Gregory CLEMENT > > Signed-off-by: Boris BREZILLON > > --- > > Mike, > > > > This patch depend on the patch "clk: return probe defer when DT clock > > not yet ready": http://article.gmane.org/gmane.linux.kernel/1643466 > > > > If for any reason you don't want to take it, then I will write a new > > version closer to the v2 for the parent_ready() function. > > > > I have taken into account all the remarks from Tomasz. > > > > Thanks, > > > > Changelog: > > v2->v3: > > > > - added the SoB flag of Boris > > - used of_clk_get(np, i) in the parent_ready() function. > > - simplified the loops to manage the order dependencies in of_clk_init() > > > > v1 -> v2: > > Merged the strict dependency check from Boris. > > > > > > drivers/clk/clk.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 79 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > index 32d84e921b23..3a07f8ac2e11 100644 > > --- a/drivers/clk/clk.c > > +++ b/drivers/clk/clk.c > > @@ -2526,24 +2526,99 @@ const char *of_clk_get_parent_name(struct device_node *np, int index) > > } > > EXPORT_SYMBOL_GPL(of_clk_get_parent_name); > > > > +struct clock_provider { > > + of_clk_init_cb_t clk_init_cb; > > + struct device_node *np; > > + struct list_head node; > > +}; > > + > > +static LIST_HEAD(clk_provider_list); > > + > > +/* > > + * This function looks for a parent clock. If there is one, then it > > + * checks that the provider for this parent clock was initialized, in > > + * this case the parent clock will be ready. > > + */ > > +static int parent_ready(struct device_node *np) > > +{ > > + int i = 0; > > + > > + while (true) { > > + struct clk *clk = of_clk_get(np, i); > > + > > + /* this parent is ready we can check the next one */ > > + if (!IS_ERR(clk)) { > > + clk_put(clk); > > + i++; > > + continue; > > + } > > + > > + /* at least one parent is not ready, we exit now */ > > + if (PTR_ERR(clk) == -EPROBE_DEFER) > > + return 0; > > + > > + /* > > + * Here we make assumption that the device tree is > > + * written correctly. So an error means that there is > > + * no more parent. As we didn't exit yet, then the > > + * previous parent are ready. If there is no clock > > + * parent, no need to wait for them, then we can > > + * consider their absence as being ready > > + */ > > + return 1; > > + } > > +} > > + > > /** > > * of_clk_init() - Scan and init clock providers from the DT > > * @matches: array of compatible values and init functions for providers. > > * > > - * This function scans the device tree for matching clock providers and > > - * calls their initialization functions > > + * This function scans the device tree for matching clock providers > > + * and calls their initialization functions. It also do it by trying > > + * to follow the dependencies. > > */ > > void __init of_clk_init(const struct of_device_id *matches) > > { > > const struct of_device_id *match; > > struct device_node *np; > > + struct clock_provider *clk_provider, *next; > > + bool is_init_done; > > + bool force = false; > > > > if (!matches) > > matches = &__clk_of_table; > > > > + /* First prepare the list of the clocks providers */ > > for_each_matching_node_and_match(np, matches, &match) { > > - of_clk_init_cb_t clk_init_cb = match->data; > > - clk_init_cb(np); > > + struct clock_provider *parent = > > + kzalloc(sizeof(struct clock_provider), GFP_KERNEL); > > + > > + parent->clk_init_cb = match->data; > > + parent->np = np; > > + list_add(&parent->node, &clk_provider_list); > > + } > > + > > + while (!list_empty(&clk_provider_list)) { > > + is_init_done = false; > > + list_for_each_entry_safe(clk_provider, next, > > + &clk_provider_list, node) { > > + if (force || parent_ready(clk_provider->np)) { > > + clk_provider->clk_init_cb(clk_provider->np); > > + list_del(&clk_provider->node); > > + kfree(clk_provider); > > + is_init_done = true; > > + } > > + } > > + > > + /* > > + * We didn't managed to initialize any of the > > + * remaining providers during the last loop, so now we > > + * initialize all the remaining ones unconditionally > > + * in case the clock parent was not mandatory > > + */ > > + if (!is_init_done) > > + force = true; > > + > > } > > } > > #endif > > > > > -- > Gregory Clement, Free Electrons > Kernel, drivers, real-time and embedded Linux > development, consulting, training and support. > http://free-electrons.com