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 linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to