> From: Daniel Lezcano [mailto:[email protected]] > Sent: Monday, October 31, 2016 12:28 PM
>> From: Noam Camus <[email protected]> >> >> nps_setup_clocksource() should take node as only argument i.e.: >> replace >> int __init nps_setup_clocksource(struct device_node *node, struct clk >> *clk) with int __init nps_setup_clocksource(struct device_node *node) >> >> This is also serve as preperation for next patch which adds support > >s/preperation/preparation/ Thanks, will fix in V4 of this patch set ... >> +static int nps_get_timer_clk(struct device_node *node, >> + unsigned long *timer_freq, >> + struct clk *clk) > >This function prototype does not make sense. A pointer to a clock is passed >for nothing here. Thanks, I passed *clk in order for one to do rollback on error (pass clk to clk_disable_unprepare). I will change prototype to **clk. >> +{ >> + int ret; >> + >> + clk = of_clk_get(node, 0); >> + if (IS_ERR(clk)) { >> + pr_err("timer missing clk"); >> + return PTR_ERR(clk); >> + } >> + >> + ret = clk_prepare_enable(clk); >> + if (ret) { >> + pr_err("Couldn't enable parent clk\n"); >> + return ret; >> + } >> + >> + *timer_freq = clk_get_rate(clk); >> + > > timer_freq check. > > rollback on error. Thanks, will fix at V4 ... >> - if (ret) { >> - pr_err("Couldn't enable parent clock\n"); >> - return ret; >> - } >> - >> - nps_timer_rate = clk_get_rate(clk); >> + nps_get_timer_clk(node, &nps_timer_rate, clk); >Return code check ? Thanks, will fix at V4 -Noam
