On 10/25/2013 10:57 AM, Tero Kristo wrote:
[...]
> diff --git a/drivers/clk/ti/mux.c b/drivers/clk/ti/mux.c
> new file mode 100644
> index 0000000..9c5259a
> --- /dev/null
> +++ b/drivers/clk/ti/mux.c
[...]
> +/**
> + * of_mux_clk_setup() - Setup function for simple mux rate clock
> + */
> +static int of_mux_clk_setup(struct device_node *node, struct regmap *regmap)
$ ./scripts/kernel-doc drivers/clk/ti/mux.c >/dev/null
Warning(drivers/clk/ti/mux.c:29): No description found for parameter
'node'
Warning(drivers/clk/ti/mux.c:29): No description found for parameter
'regmap'
I suggest in the next rev we do a verification if we have kernel doc
errors as well..
> +{
> + struct clk *clk;
> + const char *clk_name = node->name;
> + void __iomem *reg;
> + int num_parents;
> + const char **parent_names;
> + int i;
> + u8 clk_mux_flags = 0;
> + u32 mask = 0;
> + u32 shift = 0;
> + u32 flags = 0;
> + u32 val;
> +
> + num_parents = of_clk_get_parent_count(node);
> + if (num_parents < 1) {
> + pr_err("%s: mux-clock %s must have parent(s)\n",
> + __func__, node->name);
> + return -EINVAL;
> + }
> + parent_names = kzalloc((sizeof(char *) * num_parents), GFP_KERNEL);
> + if (!parent_names) {
> + pr_err("%s: memory alloc failed\n", __func__);
as discussed, could be dropped.
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < num_parents; i++)
> + parent_names[i] = of_clk_get_parent_name(node, i);
> +
> + of_property_read_u32(node, "reg", &val);
is'nt this mandatory? error check?
> + reg = (void *)val;
> +
> + if (of_property_read_u32(node, "ti,bit-shift", &shift)) {
> + pr_debug("%s: bit-shift property defaults to 0x%x for %s\n",
> + __func__, shift, node->name);
why a debug if this is optional?
> + }
> +
> + if (of_property_read_bool(node, "ti,index-starts-at-one"))
> + clk_mux_flags |= CLK_MUX_INDEX_ONE;
> +
> + if (of_property_read_bool(node, "ti,set-rate-parent"))
> + flags |= CLK_SET_RATE_PARENT;
> +
> + /* Generate bit-mask based on parent info */
> + mask = num_parents;
> + if (!(clk_mux_flags & CLK_MUX_INDEX_ONE))
> + mask--;
we are assuming there wont be holes in the map (like reserved mux option?)
> +
> + mask = (1 << fls(mask)) - 1;
> +
> + clk = clk_register_mux_table_regmap(NULL, clk_name, parent_names,
> + num_parents, flags, reg, regmap,
> + shift, mask, clk_mux_flags, NULL,
> + NULL);
> +
> + if (!IS_ERR(clk)) {
> + of_clk_add_provider(node, of_clk_src_simple_get, clk);
> + return 0;
> + }
> +
kfree(parent_names)?
> + return PTR_ERR(clk);
> +}
> +CLK_OF_DECLARE(mux_clk, "ti,mux-clock", of_mux_clk_setup);
> +
> +static int __init of_ti_composite_mux_clk_setup(struct device_node *node,
> + struct regmap *regmap)
> +{
> + struct clk_mux *mux;
> + int num_parents;
> + int ret;
> + u32 val;
> +
> + mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> + if (!mux)
> + return -ENOMEM;
> +
> + of_property_read_u32(node, "reg", &val);
is'nt this mandatory? error check?
> +
> + mux->reg = (void *)val;
> + mux->regmap = regmap;
> +
> + if (of_property_read_u32(node, "ti,bit-shift", &val)) {
> + pr_debug("%s: no bit-shift for %s, default=0\n",
> + __func__, node->name);
> + val = 0;
> + }
> + mux->shift = val;
> +
> + num_parents = of_clk_get_parent_count(node);
mandatory parameter without check?
ti,index-starts-at-one, ti,set-rate-parent
these seem not supported here even though the bindings dont tell us that.
> +
> + mux->mask = num_parents - 1;
> + mux->mask = (1 << fls(mux->mask)) - 1;
> +
> + ret = ti_clk_add_component(node, &mux->hw, CLK_COMPONENT_TYPE_MUX);
> + if (!ret)
> + return 0;
> +
> + kfree(mux);
> + return -ret;
> +}
> +CLK_OF_DECLARE(ti_composite_mux_clk_setup, "ti,composite-mux-clock",
> + of_ti_composite_mux_clk_setup);
>
--
Regards,
Nishanth Menon
--
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
