linux-kernel - Re: [PATCH V3 4/4] cpufreq: Tegra: implement intermediate frequency callbacks

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] [day] [month] [year] [list]

Date:	Wed, 21 May 2014 09:51:30 +0530
From:	Viresh Kumar <viresh.kumar@...aro.org>
To:	Doug Anderson <dianders@...omium.org>
Cc:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Lists linaro-kernel <linaro-kernel@...ts.linaro.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Arvind Chauhan <arvind.chauhan@....com>,
	Stephen Warren <swarren@...dia.com>,
	Nicolas Pitre <nicolas.pitre@...aro.org>,
	Stephen Warren <swarren@...dotorg.org>,
	Russell King <linux@....linux.org.uk>,
	Thomas Abraham <thomas.abraham@...aro.org>,
	Peter De Schrijver <pdeschrijver@...dia.com>
Subject: Re: [PATCH V3 4/4] cpufreq: Tegra: implement intermediate frequency callbacks

On 20 May 2014 22:19, Doug Anderson <dianders@...omium.org> wrote:
> Note that in the old code you used to set the "emc" clock before the
> transition to the intermediate clock.  Now you don't.  Are you sure
> it's OK to change this order?

Yeah, I have seen that and as Stephen didn't had any objection to the
change I thought it is probably fine.

>> @@ -98,10 +88,21 @@ static int tegra_target(struct cpufreq_policy *policy, unsigned int index)
>>         else
>>                 clk_set_rate(emc_clk, 100000000);  /* emc 50Mhz */
>>
>> -       ret = tegra_cpu_clk_set_rate(rate * 1000);
>> +       if (rate == clk_get_rate(pll_p_clk))
>
> Shouldn't this be "rate * 1000 =="?

Yes.

>> +               goto out;
>> +
>> +       ret = clk_set_rate(pll_x_clk, rate * 1000);
>> +       if (ret) {
>> +               pr_err("Failed to change pll_x to %lu\n", rate);
>> +               goto out;
>> +       }
>
> I feel like this should be in tegra_target_intermediate(), since it
> could fail (right?).  Essentially we want to be as sure as possible
> that tegra_target() doesn't return an error code.
>
>
>> +
>> +       ret = clk_set_parent(cpu_clk, pll_x_clk);
>
> Presumably this won't actually ever fail, since that violates the
> rules that target() shouldn't fail if you used an intermediate
> frequency.

Don't know, it looks this rule isn't going to last long.. Let me see
if I can improve it a bit.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/