On 20 May 2014 22:19, Doug Anderson <diand...@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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
