RE: [PATCH v2 2/3] clocksource: update "fn" at CLOCKSOURCE_OF_DECLARE() of nps400 timer

[Noam Camus]

> From: Daniel Lezcano [mailto:daniel.lezcano@xxxxxxxxxx] 
> Sent: Monday, October 31, 2016 12:28 PM

>> From: Noam Camus <noamca@xxxxxxxxxxxx>
>> 
>> 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