Re: [PATCH 3/3] cpufreq: Add a generic cpufreq-cpu0 driver

[Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>]

On Thu, Jul 19, 2012 at 11:54:41PM +0800, Shawn Guo wrote:

> +Optional properties:
> +- transition-latency: Specify the possible maximum transition latency,
> +  in unit of nanoseconds.

This should make it clear that the transition latency being documented
here is just that for the core clock change itself, there may be other
sources of latency like the regulator ramp time or reprogramming PLLs.

> +	if (cpu_reg) {
> +		opp = opp_find_freq_ceil(cpu_dev, &freq_Hz);
> +		if (IS_ERR(opp)) {
> +			pr_err("failed to find OPP for %ld\n", freq_Hz);
> +			return PTR_ERR(opp);
> +		}
> +		volt = opp_get_voltage(opp);
> +		tol = volt * voltage_tolerance / 100;
> +		volt_old = regulator_get_voltage(cpu_reg);

regulator_get_voltage() might be a bit expensive, many regulators will
query the hardware every time.  Since it's just for diagnostic purposes
it'd be better to not bother and let people read the historical
information from the logs.

> +	/* scaling up?  scale voltage before frequency */
> +	if (cpu_reg && freqs.new > freqs.old) {
> +		if (regulator_set_voltage(cpu_reg, volt - tol, volt + tol)) {

This is a totally sane and sensible use case for specifying a voltage
range, we should move it into the regulator core for other users so you
can just specify the voltage and the tolerance directly.

It is a little sad here as it means that we loose the ability to do
frequency only scaling if there's no regulator which is nice.