Messages in this thread | | | From | Noam Camus <> | Subject | RE: [PATCH v2 2/3] clocksource: update "fn" at CLOCKSOURCE_OF_DECLARE() of nps400 timer | Date | Mon, 31 Oct 2016 15:19:15 +0000 |
| |
> From: Daniel Lezcano [mailto:daniel.lezcano@linaro.org] > Sent: Monday, October 31, 2016 12:28 PM
>> From: Noam Camus <noamca@mellanox.com> >> >> 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
| |