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

[Mark Brown]

On Fri, Jul 16, 2010 at 02:37:04PM +0900, MyungJoo Ham wrote:
> On Thu, Jul 15, 2010 at 8:59 PM, Mark Brown

> > 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.

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

> > 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).

> > 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.