lkml.org 
[lkml]   [2014]   [May]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH V3 4/4] cpufreq: Tegra: implement intermediate frequency callbacks
From
On 20 May 2014 22:19, Doug Anderson <dianders@chromium.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.


\
 
 \ /
  Last update: 2014-05-21 07:06    [W:0.043 / U:1.196 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site