On 01/21, Tomeu Vizoso wrote:
> @@ -2075,10 +2210,12 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw)
> }
> }
>
> - ret = __clk_init(dev, clk);
> + hw->clk = __clk_create_clk(hw, NULL, NULL);
> + ret = __clk_init(dev, hw->clk);
> if (!ret)
> - return clk;
> + return hw->clk;
>
> + kfree(hw->clk);
> fail_parent_names_copy:
> while (--i >= 0)
> kfree(clk->parent_names[i]);
Sigh, this patch is so huge I keep finding more things. Sorry. It
looks like __clk_create_clk() can return an error pointer, which
we then send directly to __clk_init. First off, we shouldn't
kfree() that pointer if it's an error pointer. Second, we
shouldn't crash in __clk_init() in such a situation so there
needs to be some sort of check somewhere.
BTW, please try and fixup checkpatch warnings.
> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index da4bda8..fac3244 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -69,20 +70,22 @@ struct clk *of_clk_get(struct device_node *np, int index)
[...]
> -struct clk *of_clk_get_by_name(struct device_node *np, const char *name)
> +static struct clk *__of_clk_get_by_name(struct device_node *np, const char *name)
It would be nice if this returned an already __clk_create_clk()ed
pointer.
> {
> struct clk *clk = ERR_PTR(-ENOENT);
>
> @@ -119,7 +122,33 @@ struct clk *of_clk_get_by_name(struct device_node *np, const char *name)
[...]
> +struct clk *of_clk_get_by_name(struct device_node *np, const char *name)
> +{
> + struct clk *clk = __of_clk_get_by_name(np, name);
> +
> + if (!IS_ERR(clk))
> + clk = __clk_create_clk(__clk_get_hw(clk), np->full_name, name);
Because we do it here where we know we're CONFIG_COMMON_CLK=y.
> +
> + return clk;
> +}
> EXPORT_SYMBOL(of_clk_get_by_name);
> +
> +#else /* defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK) */
> +
> +static struct clk *__of_clk_get_by_name(struct device_node *np, const char *name)
> +{
> + return ERR_PTR(-ENOENT);
> +}
> #endif
>
> /*
> @@ -185,9 +229,13 @@ struct clk *clk_get(struct device *dev, const char *con_id)
> struct clk *clk;
>
> if (dev) {
> - clk = of_clk_get_by_name(dev->of_node, con_id);
> - if (!IS_ERR(clk))
> + clk = __of_clk_get_by_name(dev->of_node, con_id);
> + if (!IS_ERR(clk)) {
> +#if defined(CONFIG_COMMON_CLK)
> + clk = __clk_create_clk(__clk_get_hw(clk), dev_id, con_id);
> +#endif
And we do it here where we could remove the #ifdef.
> return clk;
> + }
> if (PTR_ERR(clk) == -EPROBE_DEFER)
> return clk;
> }
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
