[PATCH v5] cpufreq: add imx6q-cpufreq driver

[Shawn Guo]

On Mon, Feb 04, 2013 at 11:06:22AM -0500, Anson Huang wrote:
> > +	/*
> > +	 * The setpoints are selected per PLL/PDF frequencies, so we need to
> > +	 * reprogram PLL for frequency scaling.  The procedure of reprogramming
> > +	 * PLL1 is as below.
> > +	 *
> > +	 *  - Enable pll2_pfd2_396m_clk and reparent pll1_sw_clk to it
> > +	 *  - Disable pll1_sys_clk and reprogram it
> > +	 *  - Enable pll1_sys_clk and reparent pll1_sw_clk back to it
> > +	 *  - Disable pll2_pfd2_396m_clk
> > +	 */
> > +	clk_prepare_enable(pll2_pfd2_396m_clk);
> > +	clk_set_parent(step_clk, pll2_pfd2_396m_clk);
> > +	clk_set_parent(pll1_sw_clk, step_clk);
> > +	clk_prepare_enable(pll1_sys_clk);
> > +	if (freq_hz > clk_get_rate(pll2_pfd2_396m_clk)) {
> > +		clk_disable_unprepare(pll1_sys_clk);
> > +		clk_set_rate(pll1_sys_clk, freqs.new * 1000);
> > +		clk_prepare_enable(pll1_sys_clk);
> > +		clk_set_parent(pll1_sw_clk, pll1_sys_clk);
> > +		clk_disable_unprepare(pll2_pfd2_396m_clk);
> > +	} else {
> > +		/*
> > +		 * Disable pll1_sys_clk if pll2_pfd2_396m_clk is sufficient
> > +		 * to provide the frequency.
> > +		 */
> > +		clk_disable_unprepare(pll1_sys_clk);
> > +	}
> Seems like we will get pll2_pfd2_396m_clk's use count mismatch? As every time cpu freq is changed, pll2_pfd2_396m_clk will be added at the beginning of this code piece, but only decreased when cpu freq > 396M, so everytime cpu freq changed to 396M, this pll2_pfd2_396m_clk will in increased?

Ah, good catch.  And pll1_sys_clk has the same problem.  Since it's
been verified by FSL kernel that we do not necessarily need to disable
pll1_sys_clk before reprogramming it, I would choose to rewrite the
code as below to make it cleaner and correct on clock usage.

        /*
         * The setpoints are selected per PLL/PDF frequencies, so we need to
         * reprogram PLL for frequency scaling.  The procedure of reprogramming
         * PLL1 is as below.
         *
         *  - Enable pll2_pfd2_396m_clk and reparent pll1_sw_clk to it
         *  - Reprogram pll1_sys_clk and reparent pll1_sw_clk back to it
         *  - Disable pll2_pfd2_396m_clk
         */
        clk_prepare_enable(pll2_pfd2_396m_clk);
        clk_set_parent(step_clk, pll2_pfd2_396m_clk);
        clk_set_parent(pll1_sw_clk, step_clk);
        if (freq_hz > clk_get_rate(pll2_pfd2_396m_clk)) {
                clk_set_rate(pll1_sys_clk, freqs.new * 1000);
                /*
                 * If we are leaving 396 MHz set-point, we need to enable
                 * pll1_sys_clk and disable pll2_pfd2_396m_clk to keep
                 * their use count correct.
                 */
                if (freqs.old * 1000 <= clk_get_rate(pll2_pfd2_396m_clk)) {
                        clk_prepare_enable(pll1_sys_clk);
                        clk_disable_unprepare(pll2_pfd2_396m_clk);
                }
                clk_set_parent(pll1_sw_clk, pll1_sys_clk);
                clk_disable_unprepare(pll2_pfd2_396m_clk);
        } else {
                /*
                 * Disable pll1_sys_clk if pll2_pfd2_396m_clk is sufficient
                 * to provide the frequency.
                 */
                clk_disable_unprepare(pll1_sys_clk);
        }

> > +
> > +	/* Ensure the arm clock divider is what we expect */
> > +	ret = clk_set_rate(arm_clk, freqs.new * 1000);
> > +	if (ret) {
> > +		dev_err(cpu_dev, "failed to set clock rate: %d\n", ret);
> > +		regulator_set_voltage_tol(arm_reg, volt_old, 0);
> > +		return ret;
> > +	}
> > +
> > +	/* scaling down?  scale voltage after frequency */
> > +	if (freqs.new < freqs.old) {
> > +		ret = regulator_set_voltage_tol(arm_reg, volt, 0);
> > +		if (ret)
> > +			dev_warn(cpu_dev,
> > +				 "failed to scale vddarm down: %d\n", ret);
> > +
> > +		if (freqs.old == FREQ_1P2_GHZ / 1000) {
> > +			regulator_set_voltage_tol(pu_reg,
> > +					PU_SOC_VOLTAGE_NORMAL, 0);
> > +			regulator_set_voltage_tol(soc_reg,
> > +					PU_SOC_VOLTAGE_NORMAL, 0);
> > +		}
> > +	}
> > +
> > +	for_each_online_cpu(cpu) {
> > +		freqs.cpu = cpu;
> > +		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> > +	}
> > +
> Should we need to update the percpu loops_per_jiffy variable and global loops_per_jiffy? As the udelay and mdelay will rely on this global loops_per_jiffy? Or the latest kernel has handle it in other place such as cpufreq common driver? I remembered that common cpufreq driver only handle the noSMP case.

No, it's not needed since commit ec971ea (ARM: add cpufreq transiton
notifier to adjust loops_per_jiffy for smp) is in place.

Shawn