Hello Stephen, Thanks a lot for your feedback. On 02/11/2015 07:54 PM, Stephen Boyd wrote: > On 02/11, Javier Martinez Canillas wrote: >> --- a/drivers/clk/clk.c >> +++ b/drivers/clk/clk.c >> @@ -799,7 +799,7 @@ clk_mux_determine_rate_flags(struct clk_hw *hw, unsigned long rate, >> /* if NO_REPARENT flag set, pass through to current parent */ >> if (core->flags & CLK_SET_RATE_NO_REPARENT) { >> parent = core->parent; >> - if (core->flags & CLK_SET_RATE_PARENT) >> + if (core->flags & CLK_SET_RATE_PARENT && parent) >> best = __clk_determine_rate(parent->hw, rate, >> min_rate, max_rate); >> else if (parent) > > Sorry this doesn't look right. Before all the recent changes to > this file we would call __clk_round_rate() which would return 0 > if the first argument was NULL. Now we're going to take the else > if path and do something different. So we need a parent ? > parent->hw : NULL here. > Right, I'm not that familiar with the common clock framework so I didn't realize I was changing the behavior, sorry about that... > Of course, I wonder why a clock has the CLK_SET_RATE_PARENT flag > set if it doesn't actually have a parent. That also seems wrong. > Yes, I did not face this issue and only patch #2 was enough to fix my problem but the theoretical NULL pointer dereference was found when reading the code. I agree that a clock with that flag set should have at least one parent but afaict there is no sanity check on clock registration. And even if that was the case, I believe that the core should be robust enough to check for NULL before trying to dereference it. I'll post a v2 passing NULL as an argument and parent->hw if parent is not NULL as you suggested. Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html