On 10/31/2013 05:42 PM, Nishanth Menon wrote:
On 10/25/2013 10:57 AM, Tero Kristo wrote:ti_dt_clk_init_provider() can now be used to initialize the contents of a single clock IP block. This parses all the clocks under the IP block and calls the corresponding init function for them. Signed-off-by: Tero Kristo <t-kristo@xxxxxx> --- drivers/clk/ti/clk.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/clk/ti.h | 1 + 2 files changed, 60 insertions(+) diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c index ad58b01..7f030d7 100644 --- a/drivers/clk/ti/clk.c +++ b/drivers/clk/ti/clk.c @@ -19,6 +19,9 @@ #include <linux/clkdev.h> #include <linux/clk/ti.h> #include <linux/of.h> +#include <linux/list.h> + +extern struct of_device_id __clk_of_table[]; /** * ti_dt_clocks_register - register DT duplicate clocks during boot @@ -50,3 +53,59 @@ void __init ti_dt_clocks_register(struct ti_dt_clk oclks[]) } } } + +typedef int (*ti_of_clk_init_cb_t)(struct device_node *, struct regmap *); + +struct clk_init_item { + struct regmap *regmap; + struct device_node *np; + ti_of_clk_init_cb_t init_cb; + struct list_head node; +}; + +static LIST_HEAD(retry_list); + +void __init ti_dt_clk_init_provider(struct device_node *parent, + struct regmap *regmap) +{ + const struct of_device_id *match; + struct device_node *np; + ti_of_clk_init_cb_t clk_init_cb; + struct clk_init_item *retry; + struct clk_init_item *tmp; + int ret; + + for_each_child_of_node(parent, np) { + match = of_match_node(__clk_of_table, np); + if (!match) + continue; + clk_init_cb = match->data;I must admit I am confused here.
Yea this patch is something I am not quite comfortable myself yet and would like improvement ideas....
a) of_clk_init (in the generic clk.c) uses of_clk_init_cb_t as match data. The prototype of the generic of_clk_init_cb_t is typedef void (*of_clk_init_cb_t)(struct device_node *); b) both of_clk_init and ti_dt_clk_init_provider looks up clock nodes from __clk_of_table
__clk_of_table contains the function pointers for the clock init functions, not clock nodes.
I assume we need ti_dt_clk_init_provider to be always called with clock list, as a result of_clk_init(NULL); will never be invoked. This is counter productive as you have have generic non SoC clock providers as well who would have been invoked with of_clk_init(NULL);
Can't call of_clk_init(NULL) anymore, as Paul wants to map the clock nodes under respective IP blocks. Two reasons here:
a) This requires me to pass some info from the IP block (CM1/CM2/PRM) to the clock init functions, basically the pointer to the memory map region (regmap.) b) of_clk_init(NULL) will initialize all the clock nodes in the system, irrespective of the hierarchy considerations.
Only thing that can be done, is to make the API introduced in this patch a generic API and call it something like of_clk_init_children().
I do strongly encourage using of_clk_init(NULL) and not having to switch the clk_init call back with SoC specific one as it forces un-intended dependency.
Can't do this as mentioned above.
Further such as SoC specific callback should have been in a header.+ pr_debug("%s: initializing: %s\n", __func__, np->name);one further comment - using pr_fmt will save us from using __func__ for every pr_* message :(
Hmm good comment, will fix that for next rev.
+ ret = clk_init_cb(np, regmap); + if (ret == -EAGAIN) { + pr_debug("%s: adding to again list...\n", np->name); + retry = kzalloc(sizeof(*retry), GFP_KERNEL); + retry->np = np; + retry->regmap = regmap; + retry->init_cb = clk_init_cb; + list_add(&retry->node, &retry_list); + } else if (ret) { + pr_err("%s: clock init failed for %s (%d)!\n", __func__, + np->name, ret); + } + } + + list_for_each_entry_safe(retry, tmp, &retry_list, node) { + pr_debug("%s: retry-init: %s\n", __func__, retry->np->name); + ret = retry->init_cb(retry->np, retry->regmap); + if (ret == -EAGAIN) { + pr_debug("%s failed again?\n", retry->np->name); + } else { + if (ret) + pr_err("%s: clock init failed for %s (%d)!\n", + __func__, retry->np->name, ret); + list_del(&retry->node); + kfree(retry); + } + } +} diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h index 9786752..7ab02fa 100644 --- a/include/linux/clk/ti.h +++ b/include/linux/clk/ti.h @@ -177,6 +177,7 @@ int omap3_dpll4_set_rate(struct clk_hw *clk, unsigned long rate, unsigned long parent_rate); void ti_dt_clocks_register(struct ti_dt_clk *oclks); +void ti_dt_clk_init_provider(struct device_node *np, struct regmap *regmap); extern const struct clk_hw_omap_ops clkhwops_omap3_dpll; extern const struct clk_hw_omap_ops clkhwops_omap4_dpllmx;
-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
