linux-kernel - RE: [PATCH v2 2/3] clocksource: update "fn" at CLOCKSOURCE_OF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 31 Oct 2016 15:19:15 +0000
From:   Noam Camus <noamca@...lanox.com>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>
CC:     "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 2/3] clocksource: update "fn" at
 CLOCKSOURCE_OF_DECLARE() of nps400 timer

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

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