Messages in this thread | | | Date | Wed, 21 May 2014 09:51:30 +0530 | Subject | Re: [PATCH V3 4/4] cpufreq: Tegra: implement intermediate frequency callbacks | From | Viresh Kumar <> |
| |
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.
| |