Subject: [Cocci] [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances

From: [email-masked] (Stephen Boyd)
Date: Wed, 04 Feb 2015 15:26:19 -0800

On 02/03/15 08:04, Quentin Lambert wrote:
> Hello,
> Julia asked me to have a look and see if I can help.
>
> I have found these three cases using Coccinnelle in the mach-omap2
> directory.
>
>
>
> ./arch/arm/mach-omap2/clkt_clksel.c
> @@ -67,7 +67,6 @@ static const struct clksel *_get_clksel_
>                 return NULL;
>
>         for (clks = clk->clksel; clks->parent; clks++)
>                 if (clks->parent == src_clk)

This probably needs to compare hw pointers again given that it's in the
clk-provider code. It would be nice if we could use indices instead though.

>                         break; /* Found the requested parent */
>
>         if (!clks->parent) {
> ./arch/arm/mach-omap2/dpll3xxx.c
> @@ -551,7 +549,6 @@ int omap3_noncore_dpll_set_rate(struct c
>         if (!dd)
>                 return -EINVAL;
>
>         if (__clk_get_parent(hw->clk) != dd->clk_ref)

This one is what this thread has started on about. Comparing hw pointers
is ok for now...

>                 return -EINVAL;
>
>         if (dd->last_rounded_rate == 0)
> ./arch/arm/mach-omap2/timer.c
> @@ -298,7 +298,6 @@ static int __init omap_dm_timer_init_one
>         if (IS_ERR(src))
>                 return PTR_ERR(src);
>
>         if (clk_get_parent(timer->fclk) != src) {

This one looks like an optimization. We can just always try to set the
parent and if it's already the current parent then the core should bail
out early without an error. So the fix would be to just remove the if
condition.

>                 r = clk_set_parent(timer->fclk, src);
>                 if (r < 0) {
>                         pr_warn("%s: %s cannot set source\n", __func__,
>
>
> Are they instances of your issue?

Yes these all look like instances of the problem.

> If you want me to I can enlarge the search to other directories.

Yes please do. And if you could share the coccinelle patch that would be
great. Thanks.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project