[PATCHv10 03/41] CLK: ti: add init support for clock IP blocks

[Paul Walmsley]

On Tue, 26 Nov 2013, 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.
> 
> This patch also introduces a helper function for the TI clock drivers
> to get register info from DT and append the master IP info to this.
> 
> Signed-off-by: Tero Kristo <t-kristo at ti.com>

...

> diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
> index ef1a7cd..63f85e9 100644
> --- a/drivers/clk/ti/clk.c
> +++ b/drivers/clk/ti/clk.c
> @@ -19,10 +19,15 @@
>  #include <linux/clkdev.h>
>  #include <linux/clk/ti.h>
>  #include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/list.h>
>  
>  #undef pr_fmt
>  #define pr_fmt(fmt) "%s: " fmt, __func__
>  
> +extern struct of_device_id __clk_of_table[];

This results in a checkpatch.pl warning:

WARNING: externs should be avoided in .c files
#33: FILE: drivers/clk/ti/clk.c:28:
+extern struct of_device_id __clk_of_table[];

Please make sure your patches are checkpatch.pl-clean, with the exception 
of the 80-column warnings and any const-related warnings related to the 
clock framework.

> +static int ti_dt_clk_regmap_index;
> +
>  /**
>   * ti_dt_clocks_register - register DT alias clocks during boot
>   * @oclks: list of clocks to register
> @@ -53,3 +58,96 @@ void __init ti_dt_clocks_register(struct ti_dt_clk oclks[])
>  		}
>  	}
>  }
> +
> +typedef int (*ti_of_clk_init_cb_t)(struct device_node *);

Normally typedefs should be a red flag to reviewers due to 
Documentation/CodingStyle chapter 5.  Still it seems this patch is just 
duplicating the way that the CCF does it, so I'm not too worried about it.

> +
> +struct clk_init_item {
> +	int index;
> +	struct device_node *np;
> +	ti_of_clk_init_cb_t init_cb;
> +	struct list_head node;
> +};
> +
> +static LIST_HEAD(retry_list);
> +
> +/**
> + * ti_clk_get_reg_addr - get register address for a clock register
> + * @node: device node for the clock
> + * @index: register index from the clock node
> + *
> + * Builds clock register address from device tree information. This
> + * is a struct of type clk_omap_reg.
> + */
> +void __iomem *ti_clk_get_reg_addr(struct device_node *node, int index)
> +{
> +	struct clk_omap_reg *reg;
> +	u32 val;
> +	u32 tmp;
> +
> +	reg = (struct clk_omap_reg *)&tmp;
> +	reg->index = ti_dt_clk_regmap_index;
> +
> +	if (of_property_read_u32_index(node, "reg", index, &val)) {
> +		pr_err("%s must have reg[%d]!\n", node->name, index);
> +		return NULL;
> +	}
> +
> +	reg->offset = val;
> +
> +	return (void __iomem *)tmp;
> +}
> +
> +/**
> + * ti_dt_clk_init_provider - init master clock provider
> + * @parent: master node
> + * @index: internal index for clk_reg_ops
> + *
> + * Initializes a master clock IP block and its child clock nodes.
> + * Regmap is provided for accessing the register space for the
> + * IP block and all the clocks under it.
> + */
> +void ti_dt_clk_init_provider(struct device_node *parent, int index)
> +{
> +	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;
> +
> +	ti_dt_clk_regmap_index = index;
> +
> +	for_each_child_of_node(parent, np) {
> +		match = of_match_node(__clk_of_table, np);
> +		if (!match)
> +			continue;
> +		clk_init_cb = (ti_of_clk_init_cb_t)match->data;
> +		pr_debug("%s: initializing: %s\n", __func__, np->name);
> +		ret = clk_init_cb(np);
> +		if (ret == -EAGAIN) {
> +			pr_debug("%s: adding to again list...\n", np->name);
> +			retry = kzalloc(sizeof(*retry), GFP_KERNEL);
> +			retry->np = np;
> +			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);
> +		ti_dt_clk_regmap_index = retry->index;
> +		ret = retry->init_cb(retry->np);
> +		if (ret == -EAGAIN) {
> +			pr_debug("%s failed again?\n", retry->np->name);

This is presumably a serious error condition and should be a pr_warn() or 
pr_err(), right?

If retry_list won't be walked again, then it seems best to delete and free 
the list_entry no matter what the return value is from retry->init_cb(), 
since it's not like it will be retried.  Otherwise the code will leak 
memory.

> +		} 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 df94c24..d6bf530 100644
> --- a/include/linux/clk/ti.h
> +++ b/include/linux/clk/ti.h
> @@ -36,7 +36,21 @@ struct ti_dt_clk {
>  		.node_name = name,	\
>  	}
>  
> +/* Maximum number of clock regmaps */
> +#define CLK_MAX_REGMAPS			4
>  
> +/**
> + * struct clk_omap_reg - OMAP register declaration
> + * @offset: offset from the master IP module base address
> + * @index: index of the master IP module
> + */
> +struct clk_omap_reg {
> +	u16 offset;
> +	u16 index;
> +};
> +
> +void __iomem *ti_clk_get_reg_addr(struct device_node *node, int index);
>  void ti_dt_clocks_register(struct ti_dt_clk *oclks);
> +void ti_dt_clk_init_provider(struct device_node *np, int index);
>  
>  #endif
> -- 
> 1.7.9.5
> 


- Paul