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. Thanks, Gregory > > Signed-off-by: Gregory CLEMENT <[email protected]> > Signed-off-by: Boris BREZILLON <[email protected]> > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
