Re: [PATCHv9 06/43] CLK: ti: add init support for clock IP blocks
From: Nishanth Menon <nm@ti.com>
Date: 2013-10-31 15:42:16
Also in:
linux-arm-kernel, linux-omap
On 10/25/2013 10:57 AM, Tero Kristo wrote:
quoted hunk ↗ jump to hunk
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 <redacted> --- 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. 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 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); 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. 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 :(
quoted hunk ↗ jump to hunk
+ 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;
-- Regards, Nishanth Menon