[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

On Fri, Jul 16, 2010 at 02:37:04PM +0900, MyungJoo Ham wrote:

On Thu, Jul 15, 2010 at 8:59 PM, Mark Brown

quoted

Doing this through the regulator API avoids hard coding details of an
individual regulator in your driver and ensures that all users of this
regulator can handle the ramp rate.

Making regulator API wait for the ramp up seems to be something like
"synchronous I/O", which in turn, forces CPU to do nothing while CPU
may execute some instructions while waiting for the ramp up. However,

Yeah, I know, but even if the actual delay is implemented in the
consumer driver the ramp time should still be being queried via the
regulator API so that the details of the specific device aren't hard
coded into the consumer.  In any case as you say...

we just do udelay() anyway; thus, it doesn't matter for us as well.
I'll try to put those udelay into MAX8998 PMIC drivers, which is for
S5PV210. I'll remove these udelay and PMIC_RAMP)UP from cpufreq driver
with the next version.

...the ramp times are usually vanishingly small so the overwhelming
majority of consumers probably don't care anyway.  If it's a problem we
can add an interface which doesn't do the delay automatically but for
most users it's going to be simpler to just deal with it transparently.

quoted

quoted

+ ? ? ? ? ? ? ? ? ? ? ? regulator_set_voltage(arm_regulator,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? arm_volt, arm_volt);

quoted

You shouldn't be specifying an exact voltage here - there's no guarantee
that the exact voltage you request will be delivered by the regulator,
and some boards may wish to use constraints to eliminate some voltages
(eg, to restrict the maximum operating point for the CPU).

quoted

What you'd normally have here is that the minimum voltage would be the
desired operating point for the CPU and the maximum voltage would be
fixed at the maximum rated supply for the part. ?This will allow the
regulator API to select the lowest voltage it is able to deliver.

Currently, there is a bug in MAX8998 PMIC driver that sets VDDARM to
minimum possible voltage that is LARGER THAN or EQUAL TO max_vol, not
min_vol. I'll correct set_voltage usage of CPUFREQ assuming that
bugfix for MAX8998 PMIC driver is sent. (Joonyoung Shim, who's just
started submitting MAX8998 PMIC driver, know about this issue and will
probably send a patch soon.)

Oh, fail.  Like you say that's a bug in the regulator driver which will
hopefully be fixed before this gets merged.

Besides, the whole section enclosed by this CONFIG_REGULATOR will be removed.

A while ago, WM8994 had been using CLK_OUT as the clock source with
APLL dependent setting; thus "target" function accessed CLK_OUT when
APLL changed. However, it no longer uses APLL dependent clock source
and we no longer access CLK_OUT at the "target" function; thus, this
code in "init" function can be removed.

Ah, OK.  If other people use that sort of clocking scheme we will need
to come up with a reasonable way of coping with it, but let's save that
for when the problem arises.