[PATCHv9 06/43] CLK: ti: add init support for clock IP blocks
Tero Kristo
t-kristo at ti.com
Fri Nov 1 05:12:53 EDT 2013
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 at ti.com>
>> ---
>> 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;
>>
>
>
More information about the linux-arm-kernel
mailing list