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.