On 01/29/15 05:31, Geert Uytterhoeven wrote: > Hi Tomeu, Mike, > > On Fri, Jan 23, 2015 at 12:03 PM, Tomeu Vizoso > <tomeu.vizoso@xxxxxxxxxxxxx> wrote: >> --- a/drivers/clk/clk.c >> +++ b/drivers/clk/clk.c >> @@ -2391,25 +2543,24 @@ int __clk_get(struct clk *clk) >> return 1; >> } >> >> -static void clk_core_put(struct clk_core *core) >> +void __clk_put(struct clk *clk) >> { >> struct module *owner; >> >> - owner = core->owner; >> + if (!clk || WARN_ON_ONCE(IS_ERR(clk))) >> + return; >> >> clk_prepare_lock(); >> - kref_put(&core->ref, __clk_release); >> + >> + hlist_del(&clk->child_node); >> + clk_core_set_rate_nolock(clk->core, clk->core->req_rate); > At this point, clk->core->req_rate is still zero, causing > cpg_div6_clock_round_rate() to be called with a zero "rate" parameter, > e.g. on r8a7791: Hmm.. I wonder if we should assign core->req_rate to be the same as core->rate during __clk_init()? That would make this call to clk_core_set_rate_nolock() a nop in this case. > > cpg_div6_clock_round_rate: clock sd2 rate 0 parent_rate 780000000 div 1 > cpg_div6_clock_round_rate: clock sd1 rate 0 parent_rate 780000000 div 1 > cpg_div6_clock_round_rate: clock mmc0 rate 0 parent_rate 780000000 div 1 > cpg_div6_clock_round_rate: clock sd1 rate 0 parent_rate 780000000 div 1 > cpg_div6_clock_round_rate: clock sd1 rate 0 parent_rate 780000000 div 1 > cpg_div6_clock_round_rate: clock sd2 rate 0 parent_rate 780000000 div 1 > cpg_div6_clock_round_rate: clock sd2 rate 0 parent_rate 780000000 div 1 > > and cpg_div6_clock_calc_div() is called to calculate > > div = DIV_ROUND_CLOSEST(parent_rate, rate); > > Why was this call to clk_core_set_rate_nolock() in __clk_put() added? > Before, there was no rate setting done at this point, and > cpg_div6_clock_round_rate() was not called. We need to call clk_core_set_rate_nolock() here to drop any min/max rate request that this consumer has. > > Have the semantics changed? Should .round_rate() be ready to > accept a "zero" rate, and use e.g. the current rate instead? It seems like we've also exposed a bug in cpg_div6_clock_calc_div(). Technically any driver could have called clk_round_rate() with a zero rate before this change and that would have caused the same division by zero. -- 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