community PM requirements/issues and PowerOP [Was: Re: So, what's the status on the recent patches here?]

Pavel,

[snip]

the patch set i sent out has cpufreq used for changing cpu frequency, hasn't it?

the patch set I sent out allows to extend cpufreq interface to control memory clock, etc by building all necessary additional pieces on top of PowerOP Core layer which in conjunction with PM Core and clock/voltage framework layers provides PM stack with well defined loosely coupled interfaces between layers and PM functionality logically spread across the layers instead if having everything mixed in one piece as cpufreq does.

PowerOP patch set for now is presenting just main basic block but integration with devices suspend/resume, constraints, etc is next steps. For now exactly existing interfaces for turning devices on/off may be used in conjunction with poweop patch set although the interfaces may be improved in the future if necessary

PowerOP is interface which allows wide range of interfaces to be built on top of it including the interface you mentioned here. PowerOP/cpufreq integration patch demonstrates this.

can we eventually start talking more close to the code rather than speculating without it?

Pavel, if you feel that comments I put here don't correspond to what my code is doing please show this by commenting the patch set code.

Thanks,
Eugeny

Hi!

I was talking about kernel<->user interface.

You did echo low > something to change CPU frequency, IIRC.

Lets get kernel<->user interface right, first. You'll need to create Documentation/ entries for your interfaces, eventually, so lets do that, first, and then talk about code. Oh and it would be nice to cc lkml on that document, too. New kernel<->user interface is not decision taken lightly.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

me too. PowerOP is inkernel interface but which _allows_ to build various different kernel<->user interfaces on top of it. This PowerOP _advantage_ allows community to experiment with various kernel<->user interfaces on top and eventually end up with the best solution. The solution can be either one universal, agreed by community kernel<-> user interface on top or I can imaging the approach when kernel<-> user interfaces on top are configurable feature and system designer choses kernel<->user interface which fits best the systems he/she builds.

My patch set presents two different interfaces built on top of PowerOP - cpufreq and sysfs interfaces. So _no_, PowerOP is not all about 'echo low > something'. Such kernel<-> user interface is an _option_. PowerOP supports full _legacy_ cpufreq interface as another option - please check out PowerOP/cpufreq integration patch set. The goal is either to end up with one kernel<->user interface which addresses both pc world and embedded world requirements to user<->kernel interface _or_ - as an alternative I'd prefer - have these kernel<->user interfaces configurable: if you build laptop system - chose cpufreq interface on top of PowerOP; if you build embedded - chose sysfs interface. But with the approach when everything is built on top PowerOP[PM Core, Clock framework] you just eliminate all unnecessary duplication if PM functionality in PM stack.

Just in case you are confused by the fact I didn;t send out PowerOP/cpufreq integration patch along with the last PowerOP Core take: PowereOP Core patch I sent out in the last take is a standalone functional piece. PowerOP/cpufreq integration patch is just a further extension and applies to the most recent POwerOP Core without any changes so there was no reason to resend PowerOP/cpufreq integration.

Documentation/ piece is fine, I will add it. But I feel I put quite detailed comments along with the patches at least to explain interfaces.

PowerOP Core and PowerOP/cpufreq integration patch sets present two clear and configurable kernel<->user interfaces. I personally feel that interfaces configuration feature allows graceful interface discussion and possibility to get a decision smoothly instead of a flame on a list. If you argue against configuration feature please put comments on exactly configuration feature. Otherwise please explicitly list the requirements for kernel<->user interface you have which are not addressed by these two interfaces assuming you can chose either interface having PowerOP underneath.

Thanks,
Eugeny

Hi!

Kernel interface is not something to be experimented with.

...and configurable kernel interfaces are very bad idea.

Okay, drop sysfs interface, and we may have something that can be reviewed.

Actually that's good idea. Submit powerop without doing _any_ kernel interface changes, so we can see that it makes sense...

...bonus points if your submission actually deletes more code than it adds.

Your code should be good enough to survive some flaming. I do not think it is, so yes, I think submitting it to lkml is good idea. You'll have to do that at some point, anyway.

(And note lkml only flames you when you are doing something wrong.)

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Sysfs is a separate patch that can rejected. Nothing is stopping people from reviewing.

Just to be clear this is the approach we did and are doing.

Isn't this (linux-pm) the place to discuss power management patches and interfaces? I thought we would get ACK's from the power management maintainers and then the patches go into -mm. Who on lkml needs to be in this discussion that is not on this list?

btw, if people on this list are not ready to ACK PowerOP, I would like
to hear why before we go elsewhere. It looks like all major issues
have been addressed by our approach and implementation.

If you submit patch series with one bad patch, you are very unlikely to get feedback for the good patches.

That's not what I remember. Please resubmit, then. And cc lkml this time.

No, I'm not ready to ACK. Actually I'd describe it as "broken piece of code noone needs". And IIRC Greg's last question was "what is it good for?". Dave Jones was not too pleased with cpufreq/powerop integration. Intel people explained you broke centrino speedstep. Shall I continue?

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Oh and I am pretty tired of teaching you 'how to submit a patch', so if I'm quiet, do not take it as an "ACK".
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Tired of teaching me? We are following the same process documented and that everyone else is doing. I just don't understand your reasons for not ACKing and requesting we move this discussion to lkml before people on this list are ready to ACK. That has nothing to do with the process for submitting a patch. No teaching involved or necessary.

What does your going quiet mean? You have had some good feedback so I much prefer we reach some sort of understanding. If your final statement is that PowerOP is not needed and you are never going to like it or ACK It, let us know. We can agree to disagree.

...

You got the interfaces wrong. While I believe that something like powerop can indeed be useful for system-on-chip platforms, I do not think it should be exposed outside of kernel.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Ok. I don't think its wrong because its designed from understanding
the requirements of pm software for embedded mobile devices. I think
the embedded folk all agree that the type of interface submitted is required. I don't understand why you think its wrong. Just to be clear, your previous email made it very clear you don't like the userspace interface but this email says interfaces generically. I am assuming your only objection at this point is the userspace interface.
We are more than willing to work this out. The current sysfs
interface is surrounded by ifdefs and is optional. If there is no exposure to userspace, then testing/debuging will be more difficult.

Greg, Pavel, Dominik, Dave J and Dave B,

I would like to get a plan in place for acceptance with the power management guys before we move this to lkml. I propose that we submit the current set of PowerOP patches plus final few changes (from Greg's comments and a Documentation/ file). The patches do not affect anyone else. The sysfs interface is optional. If necessary Eugeny and I will maintain userspace interface patches outside the mainline for now. Will any of the power management maintainers ACK this plan and then ACK the patches? If no one here is willing to ACK, then I don't see what will change by submitting to lkml.

So you'll have to do lots of explaining. On lkml.

I meant user<->kernel interface.

Having interface surrounded by #ifdefs is evil. You can test it separately, then just remove that code before submission. Or maybe move it to debugfs.

Documentation/SubmittingPatches says:
#5) Select your CC (e-mail carbon copy) list.
#
#Unless you have a reason NOT to do so, CC
#[email protected].

and sorry, but I insist on using proper procedure here. And no, "we are afraid of being flamed" is not good enough reason not to.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Sure, let's see something here that we all agree on. You have yet to achieve that, so you still have work to do.

Nothing is keeping you from sending these to the list now. Please do so.

If so, it will be interesting to see why the code is even needed, I await the patches :)

Why? What good would the in-kernel patches be then if it can't be used except for some external patches? That's not acceptable. And the user interface has been tied to the other kernel code, so I think you need them both, but am willing to be convinced otherwise.

Let's see the code please.

Let's get this agreed on first, I feel that you still have some way to go here.

Sending stuff to lkml is fine too, you should be doing that for such a core change anyway. I don't see why you can't do that at the same time, it's just an extra email on the CC: line...

thanks,

greg k-h

The only thing stopping us is time:) It will happen soon.

PowerOP patches have been submitted to this list several times for review. You even reviewed a version or two. The main comments we are addressing are small issues such as add a file in Documentation and module reference counting. Not much else will change so you have the code. The cpufreq<->PowerOP integration patches have also been submitted but no one has responded to those.

Just read the other emails. I will stop here. It's time to reset the discussion again.

I think so too. This offer is in response to Pavel's comment that PowerOP is ok for in-kernel usage but not userspace.

Hi Matt,

The Linux-PM list booted me off itself for a while, and I just got back on and haven't gotten through the backlog yet. Right now the distinctions and relationships between any of the "recent patches" and system-wide mechanisms like

- lowpower idle tasks;

- multiple run states ... cpufreq doing an inadequte subset, even if you're only interested in CPUs;

- multiple sleep states ... /sys/power/state listing "standby" and/or "mem" (kind of weak for embedded systems) and "disk" (which is checkpoint/resume, not really sleep);

are not fully evident to me, so acking anything seems premature. Plus there's an "ACPI way" to do so much of that, which doesn't much help most non-x86 systems.

I thought I saw patches generalizing both sleep and run states (separate patch sets!) ... plus a good comment from Greg that a short (!) "elevator pitch" seemed to be lacking.

- Dave

But people on this list from TI, Nokia, and Motorola have explained why its needed. Also, we explained why in the f2f linux-pm summit. Can you give some concrete reasons why its not needed that have not been addressed already?

That was a long time ago and again many people from the embedded community have provided lots of answers.

Really?! I don't think he has reviewed our latest integration patches.
Our first submission was a result of discussion with Dominik at the
linux-pm summit. The latest submission is a result of Dave Jones comments. I think we have addressed all expressed concerns. I can't force anybody to review this stuff and I'm sure Dave is still catching up from his vacation.

I didn't see that. Was it off-list? We did not break centrino. In fact we use both the acpi and hardcoded tables just as cpufreq does. If its broken, then it was already broken.

Please. Again, I still say we have addressed all expressed concerns/questions/issues. If you don't feel we have, please give some concrete statements that you are concerned about or don't understand. As you can see, many people are happy to help explain.

PowerOP is necessary to build the power management software required for embedded mobile devices, so we need to reach an understanding. If the power management guys on this list don't understand PowerOP, then moving this discussion to lkml isn't going to help. I much prefer to work together to figure out something than throw it over the wall.

Hi!

YOU GOT THE KERNEL<->USER INTERFACE WRONG.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

I'm still not convinced. You can't break the current cpufreq interface, and to claim that it is irrelevant is just ignoring 95% of the Linux market right now :)

So, you all better work with the cpufreq people, otherwise this code is going to go nowhere.

I'm with Pavel here, in my opinion, he is correct.

thanks,

greg k-h

Those where directed to the other patch that David Singleton posted.

Right.

Pavel and others, I know its confusing having the two. Let's focus
on just PowerOP. The PowerOP patches are just powerop and do not affect other subsystems or existing PM code. The other stuff (cpufreq
integration, suspend/resume integration) can come later. Nothing in
PowerOP prevents us from adding more later.

it is since current interface (cpufreq in conjunction with /sys/power/state, etc) does not address all requirements to the interface. PowerOP approach helps to figure out best interface gracefully since there is no common opinion in the community yet.

it's not an argument. the ideal goal to end up with one universal interface. but until you personally stuck with cpufreq interface this is the only way to proceed towards universal interface design. let people to investigate advantages and disadvantages of this and that interfaces on a working system since we can't convince each other by this flame.

previous take of PowerOP did not contain sysfs part but I have not received any you comment on this. Sysfs is completely optional feature after all.

It was submitted this way several times.

Eugeny

Removing kernel interface takes 2+ years. So no, kernel interface is not something to be experimented with, at least not on mainline.

So try again, and cc lkml this time.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Disagree. How else to get it right??
Try, learn, improve. That's experimentation.
Linux has no "stable API nonsense" ...

The point of "cathedral vs bazaar" was that experimentation and evolution are more successful processes than "get it right the first time".

Or sometimes, when you post on a day whose name (in English) ends with a "y"... depends on the topic! ;)

- Dave

---

I think Pavel's point was that the userspace interface to the kernel (at least the syscall interface) is stable (open to Extension by addition of syscalls, but closed to modification or deletion of existing and that any experimentation needs to happen before functionality goes into the mainstream.

That said, I do think we probably need to add a new interface to cover operating points, because some of believe that policy needs to be in user space and the whole point of the OP abstraction is that operating points are atomic. You can't just add knobs to control more factors, because all those knobs need to be turned at the same time. Setting an operating point is inherently an atomic operation, not n separate operations that can be taken serially.

scott

Hi!

Yes, that was my point. (And I'd add "plus you can't get around that by introducing interface hidden by #ifdef").

Ok, "needs to be atomic" is first real argument for OP abstraction. I wonder if we can get around that by delaying the transaction until userspace tells us it made all the changes... otoh that is going to be messy.

But do you have some reasonable way to integrate OP with cpufreq?

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Hi,

Do we really need to integrate OP with cpufreq at this point?

Vitaly

Yes, it was presented in the PowerOP/cpufreq integration patches. The cpufreq_driver layer is the natural integration point.

Exactly.

Separate or one universal user space<->kernel interface is another story. Universal is preferred of course and in two words to achieve universal interface current cpufreq interface needs to be improved - but remains unchanged for user space !!!! - in the way to handle "chose closet predefined frequency to an arbitrary freq value echo'ed into /sys/cpufreq/cpuN/freq" functionality in user space instead of in the kernel. Assuming that frequency attribute is exported for all available operating points it is possible to implement the "cpufreq frequency selection logic" in user space and having such functionality in the kernel just violates the main rule of having everything possible outside of the kernel.

More details here:
http://lists.osdl.org/pipermail/linux-pm/2006-September/003660.html
and here
http://lists.osdl.org/pipermail/linux-pm/2006-September/003671.html

Paval, plz NOTE, that you don't have lkml in CC on this thread and I personally feel that you've brought a really terrible confusion to everyone with your lkml step. I'm wondering whether you are braking "no cross postings" rule as well.....

Eugeny

Hi!

You got the rules wrong. "Keep the code out of kernel" is important rule, but probably not the main one.

Cc-ing lkml is considered okay.

Anyway, please do _proper_ submission, cc-ing lkml, explaining why it is needed so that me and lkml actually know what is going on. Include those "elevator pitches".
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

funny. not to mention that it was not the only argument I presented but please tell us explicitly what's your reason to blow out kernel footprint by the code which can be handled outside the kernel. I'd prefer to see technical reasons a kind of latencies, etc but not the constant refrain "don't touch cpufreq interface". Especially considering that proposed improvements _do_ _not_ _change_ the interface.

And just FYI kernel footprint was stated as one of main current issues at least on the last OLS.

I already did _proper_ submissions several time on IMO the _proper_ list.

will do

Eugeny

Pavel,

from aside, it looks like you're choosing 'rules' and assign then 'priorities' in a too arbitrary way which is by a strange accident fits your point of view best of all.
May I remind you that Linux world is not only laptops and Sharp Zaurus? ;)

Thanks,
Vitaly

Actually, laptops and zauruses seem to be the only "interesting" machines from pm perspective. Then there are Motorola cellphones, but Motorola tried hard not to enable users changing kernel... so they are irrelevant.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Make that "mostly irrelevant"... as community will find a way to flash them, eventually. Still 2.6-based machine with more advanced power management would be nice. AFAICT zaurus are best machines today...
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Pavel,

Well, I never said Zaurus was no good :)
Please don't forget about Nokia 770 and its possible follow-ups. Also, please keep in mind that if we come to a joint PM solution that is in mainline, Motorola will use it... and will be contributing changes back to the community. If there was a PM solution adopted in mainline which fit the needs of embedded devices, more vendors would turn to using Linux.
I. e., what prevents Sharp from making a open-code Lnux-based cell phone? I do think that the absense of PM framework suitable for embedded is one of the main engineering reasons... So let us broaden our views and work together :)

Vitaly

Hi!

Well, for now, Nokia 770 is zaurus with wifi but without keyboard (mostly). It seems to be very similar to zaurus pm-wise.

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Yea, but AFAIK from FOSDEM/OLS Nokia folks are not satisfied with that and would like to have a more comprehensive PM framework... like PowerOP, for instance :)
That's my impression.

Vitaly

Not hardly. I suggest you _start_ by looking at the clock tree specs for the OMAP5912 (very similar to the OMAP1710 in N770) ...

You have conveniently chosen to forget that Nokia 770 does not support suspend-to-disk - it is a S-T-R device. In fact, personally I agree with Linus' comments elsewhere about S-T-D being irrelevant.

Regards,
Amit

Amit Kucheria <[email protected]>
Nokia

Hi!

Now I do not understand... this was power savings debate, no? Zaurus basically does not support S-T-D either (or at least I do not use it there), and I agree that S-T-D is irrelevant in the long term. And it is hardly relevant for non-PC machines _now_.

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

I was referring to the default method of power save on Zaurus on the Sharp ROMs. It was S-T-D (Flash), IIRC. I do not recall drivers doing runtime power management on Zaurus; that is a major difference from Nokia 770.

Regards,
Amit

Amit Kucheria <[email protected]>
Nokia

I'm pretty sure Zauruses do s-t-ram. Flash is read only during normal operation.

I'm very sure Sharp zaurus sl-5500 does s-t-ram. After all, it has 16MB of flash and 64MB of ram. Suspend-to-flash is not an option.

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

(Modified recipient list)

If that is the case, I stand corrected. Did the drivers support dynamic idling?

Although something about the time it took to resume still bothers me.... I will try to find time to flash it and check.

Amit Kucheria <[email protected]>
Nokia

...

Google provided a reference to what appears to be a student paper on power management in the Zaurus, at:
<http://personals.ac.upc.edu/mpericas/zaurus_project-final.pdf#search=%2
2%22power%20management%22%20zaurus%20linux%22>.

scaling and a sleep mode that is basically suspend-to-RAM. This was apparently done via backporting stuff to 2.4.

Scott

Not sure about that one.

Well, in 2.6.X suspend-to-RAM means console switch and process freezing. (Yes, it would be nice to fix that).
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Get yourself some OMAP hardware then; your blinders are way too effective!

OMAP1 hardware is easily had; an OSK, or a Nokia N770. The latter is battery powered already, without the hardware mods...

- Dave

Hi!

Well, I already have zaurus, and nokia 770 is inferior AFAICT. So the next one will be linux cellphone...

(And IIRC zaurus lasts longer on batteries than 770 does... or at least comparable. Maybe OMAP is more complex, but is it actually good thing?)
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Well, it might as well depend on the batteries' volume :) Also, WiFi needs more power.

Vitaly

Wifi + Significantly higher resolution display tend to play a 'small' role in this.

Amit Kucheria <[email protected]>
Nokia

640x480 display vs. 800x480 display does not sound like _that_ big deal to me. You are right with wifi, that beasts eat 1W-or-so. (and it would be great if someone implemented wifi power saving...)

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

wifi power saving does exist and you have it already in 770 and also color depth probably affects performance: do they have the same number of bits per pixel?

Cheers,
           Igor

Igor Stoppa (Nokia M - OSSO / Tampere)

Not sure, but I believe it has 65000 colors.

What wireless chip is included in 770? Are you using devicespace stack? Is wifi driver opensource?
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

It is similar to other 'open-sourced' drivers - binary firmware, open source driver.

Anyways... this is getting off-topic. Sharp Zaurus was great hardware, but its not shipped in USA/Europe anymore.

And if the only criteria for hardware-that-matters is hackability, then Nokia 770 can be flashed with your own kernel and rootfs. Check www.maemo.org

Regards,
Amit

Amit Kucheria <[email protected]>
Nokia

Hi!

Ok.

Agreed, this is getting off-topic.

I agree that Nokia is doing the right thing (tm) here (and THANKS!). (Excluding charging circuity, but someone is going to fix that). Unfortunately Motorola is not doing the right thing...

Could you perhaps provide list of operating points for 770? It would help understanding a bit, I'd say.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

If there was a followup here, I missed it ...

ISTR that it doesn't use (formal) operating points, and that a lot of the basic SOC power savings (vs powering off the display or wifi) come from a different kind of mechanism entirely. Namely, a combination of dynamic tick with a modified system idle task, which enters one of the OMAP low power modes during those long periods between clock ticks or other irqs.

That's one of the standard power saving schemes used on OMAP1 platforms with Linux.

- Dave

sample of reasonable operating points for OMAP1 platforms along with appropriate patches may be found at http://dynamicpower.sourceforge.net/dpm-omap.html

Eugeny

Okay... so you have system with 4 different cpu speeds. That should be simple enough to get running using cpufreq, no need for complex operating points patches.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

That's the old DPM stuff not any of the newer proposals right?

I thought the question was specifically what N770 used.

And OMAP1 does have cpufreq support. I believe that if I checked it out I'd find that it's used on N770, as a good way to kick in low voltage operation modes.

... that's in addition to the reduced power idle, which didn't seem to fit into the DPM framework in any obvious way.

- Dave

Example operating points were requested. Using the operating points from DPM is fine for examples.

Answered this in my other email.

Except that cpufreq doesn't really support voltage changes and N770 doesn't have the h/w to change voltage.

Actually, automatically reducing power at idle is what DPM is all about. In DPM an operating point is mapped to the idle state so when the idle task is entered an operating point change is triggered. The system can choose the best point for the idle task based on latency requirements, length of idle when using dynamic ticks, etc.

The majority of cpufreq drivers in the tree right now support voltage scaling just fine.

Dave

http://www.codemonkey.org.uk

Ok, let's reword my sentence. The cpufreq interface does not support voltage as a parameter which is needed on platforms where you need to do more than infer the correct voltage from the requested cpu frequency. Or something like that. I wasn't challenging the existing x86 cpufreq drivers, but the omap cpufreq driver does not support voltage scaling.

Please explain why this is useful at all.
Under what circumstance is "Set the CPU voltage to 1.2V" a useful thing for a user, sysadmin, or application to do ?

The voltage something runs at is utterly irrelevant to all the above. Given it's inherently tied to the speed a chip runs at, which makes a lot more sense for all the above scenarios to change, we export that instead.

AFAICS, exposing voltages to userspace would be completely pointless.

Dave

http://www.codemonkey.org.uk

Voltage scaling is a well known technique to reduce power consumption, try google search "Dynamic voltage scaling" if you are intrested in it.

I agree, in most cases we don't need to scale CPU voltage and frequency independently since CPU frequency usually coupled with appropriate voltage to set. But some CPU's permit to choose frequency value for
a given voltage or vice versa, and it makes sense to get a mechanism to control both voltage and frequency in this case. Some platforms, Intel Mainstone for example, have external s/w-controlled voltage requlators,
that allow to change CPU core voltage in pretty wide range.

Also, CPU is not the only power consumer in the system, you may need to scale
voltage for other devices to get a best perfomance/power trade off. And here is an obvious limitation of cpufreq since it takes care only about CPU.

Regards,
Dmitry

You've missed the point. I'm well aware of dynamic voltage scaling, and some of the cpufreq drivers I've written do this.

A CPU frequency _always_ has a specific voltage associated with it.

But what sense does it make for a user,sysadmin or application to change voltage? If any of these want to save power, they do so by a) doing less work, and then b) lowering the clock speed as this is inherently tied to voltage.

And if you lower voltage, you *MUST* reduce the frequency anyway. Running CPUs at higher speeds at lower voltages than those frequencies are rated for is a fastpath to a crash.

This is one fundamental reason I'm opposed to exporting voltages to userspace. Because people _will_ get this wrong, and we will see all manner of strange bugs and crashes reported from people running CPUs at higher speeds than the voltage is rated for.

By putting this into the hands of userspace, we're setting ourselves up to fail. How does userspace/users know what voltage is safe for what frequency? Without being intimate with the CPU datasheets, they're almost guaranteed to fail. The _driver_ needs to know the pairings of voltage/frequency and export the only thing that makes sense.

You see it as a limitation, I see it behaving 'as designed'. If those devices have nothing to do with the CPU, why are they even relevant to this discussion?

The argument seems to be "cpufreq only cares about CPU frequency, therefore we need something different". But, why? I see no reason why we need to change cpufreq at all, just add additional interfaces to cover those
other devices, as they're unrelated anyway. And if there *are* any
devices that are related, they can be tied into the existing cpufreq drivers just as for eg, the sa1100 drivers scale DRAM timings with CPU speed.

Dave

http://www.codemonkey.org.uk

It makes sense to perform changing an operating point dynamically in userland.
That said, voltage should be accessible from userland.

Vitaly

An operating point does not have to revolve around the voltage, nor should it. Fill out the following.

* A sysadmin wants to set the CPU voltage to 1.5V because _________________

* A user wants to set the CPU voltage to 1.5V because _________________

* An application wants to dynamically set the voltage rather than the frequency because ______________________

Try again. It's completely the wrong knob to be turning.

Dave

http://www.codemonkey.org.uk

In very special cases, yes.

Why?

If few states differ in voltage but not frequency, the only user-visible thing is different latency of enter/exit.

If you only have two voltages per frequency (as it was shown in examples here)... just add 1Hz to make difference between the two.

...but I guess you should teach cpufreq's ondemand governor to deal with different latencies for different voltages. Before that is done and shown to have significant power-saving impact, userspace interface debate is useless.
Pavel

Thanks for all the (sleeping) penguins.

I'm sorry, I didn't want to offend you, just to be sure we are talking about
the same things.

I have to correct you: "CPU frequency of x86 CPU's always has a specific voltage", it is because of CPU designers have defined this voltage as the best (or optimal maybe) voltage for this given frequency, it doesn't mean CPU won't work on some different voltage, you just can say the CPU with probability of say 99.9% will work stable on this voltage, but if you want to rise performance you could do it via rising of voltage for example.

But, nobody says "let's give desktop user's the ability to control whatever they may control, of course, it is dangerous to let an average desktop user to do such things. But, think about designer who build embedded mobile system that powered by Linux. Any mobile system always needs for fine grained power management but you won't be able to tune an optimal power consumption if you operate by only one parameter (cpufreq).

You always think about this triage user,sysadmin and application of PC's, think about embedded devices please, there we don't have stupid users that want to on/off everthing. Why don't let a qualified engineer to tune a number of parameters to get an optimal performance/power tradeoff?

I don't think it's always true, simply CMOS circuity won't work on voltage levels below some limit.

Here I encourage to think about other users (system designers I mean) who definetely have to read datasheets and other related docs.

Because the discussion is related to PowerOP, and PowerOP in turn is a set of different power parameters (or maybe not only power, it's not a limitation).

Because a number of platforms permit to tune more parameters than CPU freq, that in turn lets people to design more powersave systems, but cpufreq interface
restricts this ability.

But, I didn't ever say we need to remove cpufreq, I say we need to think not only about PC's but also about embedded (mobile) systems that requre some different
solution.

Regards,
Dmitry

I read the query to be specifically about operating points used in that product ... as giving better insight into the real PM requirements for Linux-based products.

Examples of how one solution _could_ be used address another purpose than highlighting requirements.

Hmm, that's something that somehow never came out in previous discussions. At least, not to me. It's possible that the point did come out, but got buried in all the noise.

So it would appear that an operating point (in DPM, anyway) is a combination of clock, voltage, and probably other settings, coupled with an implementation of the idle task and a policy implemented by idle task ... yes?

Given that the clock API manages clock settings, and cpufreq (for better or worse) handles CPU-specific clock and voltage, you can still color me confused about how an "operating point" would be a programming construct rather than a side effect of those other APIs on an implementation. (If anyone wants to enlighten me, that should go into a new thread!)

- Dave

It fits. Omitting some details DPM allows to map/stick operating point to tasks. Mapping one of OMAP low power mode operating point to idle task does the job.

Eugeny

That's what I call a hack.

Take a look at acpi C1..C3 state on i386. They provide same functionality ("reduced power idle"), but they are implemented in reasonable way and already merged.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

(Of course the 17xx OMAPs already use very low voltage, so systems using them might not win much by runtime switching of voltage ...)

The "operating" points on the web page you referenced did not include the "Big Sleep" state ... the CPU does not "operate" there, so that's understandable.

It would be good to have one of the Nokia folk who worked on PM for the 770 provide a bit more detail. I'm all but positive it doesn't use any incarnation of DPM. I'm sure that if any numbers (even rough ones) were available, that'd be very interesting.

Pavel's comment about C2/C3/etc on PCs is relevant in some ways, but it's also important to remember that many systems don't have states compatible (in detail, affecting infrastructure) with Intel's Cx model. In particular, CPU caches usually won't snoop, so the constraints associated with cache and DMA interactions won't exist; but different constraints, e.g clock usage, could easily apply.

- Dave

I think there was some sort of followup but I don't think it explictly listed the operating point values.

Well sure but I think this discussion was about using the 770 as a specific example of operating point values. Here are the operating points used on omap1. The board specific part is just whether or not the board has a scalable voltage regulator. Omap1 has a fairly simple operating point definition and is, of course, not the latest h/w.

This table comes from the DPM implementation on OMAP1 and is expressed as dividers.

Name Voltage, DPLL multiplier, DPLL divider, cpu divider, tc divider
(mem bus).

  192        	1300 16 1 1  2
  168        	1300 14 1 1  2
  84			1300 14 1 2  2
  84-1.1v   	1050 14 1 2  2
  60         		1300  5 1 1  1
  60-1.1v    	1050  5 1 1  1
  sleep-168  	1300 14 1 0 -1
  sleep-60   	1300  1 2 0  2

Several other parameters could be added such as dsp and dsp mmu if one wanted to do more aggressive power management. btw, the parameter names I list above are from memory so I might have the order mixed up but you get the idea.

The dpm utilities on sf.net also include 8 example operating points for pxa272. The operating point definition on pxa has ~5 parameters including voltage, cpu frequency and other clocks.

Well, and future parts and machines that are in the works.

That might be a bit unfair. Those engineers at Motorola who put linux on the cellphones and mobile devices do need to have the PM support. They are NOT irrelevant.

jdl

Of course I am unfair. It is nice that motorola ships linux-based cellphones, but they made really sure that users can't hack them... which is considered anti-social by the community.

So it would be nice if kernel matched requirements by Motorola, but it is *way* more important that it matches requirements by Sharp and notebook requirements... because Sharp & notebook manufacturers actually play fair.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

---

Depends, of course, on your definition of "fair". The CE manufacturers are investing a lot of work in Linux, either directly or through hiring developers and working through distributors. And they're giving their enhancements back to the community, which is what was generally considered "fair" for open-source development. The power management stuff may be too narrow to be interesting to laptop-based developers,but things like reduced boot time ought to be Of more general interest.

I expect modifiable kernels in the future, but that requires a lot of extra engineering (beyond just making the device work) to harden the non-open elements so that malicious kernel changes couldn't compromise them. None of the manufacturers is going to build a cellphone or DVR that service providers won't allow on their networks...

scott

Hi!

Well, CE equipment usually uses obsolete, heavily modified kernels, and developed in-house (as opposed by community). Yes, it is nice that Motorola uses Linux and not WindowsCE...

Yep, boottime reductions are always nice... if they are not too specialized.

What is the issue here? I thought that GSM stack runs on separate CPU, anyway? And in practice, it is probably possible to flash your equipment... Like small shops offering "unlocking" do it all the time.

What is the issue? I can do whatever I want (including modifying phone-side firmware so that phone works without SIMcard, just with Ki) with siemens ME45 already. Of course Siemens will not support me doing that... That should not be too different with Motorola...?
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

---

There are perfectly sensible business reasons why device manufacturers need to freeze their development on old kernels (and other components) for long periods of time. Are you saying this is "unfair" to the developers? I understand that it makes the patches less useful, in many cases, but unfair?

I do hope that Motorola, and other CE manufacturers, will start working more closely with the community. The CE Linux Forum was set up in part to make that easier and more effective. I believe it would be better for both the manufacturers and the community.

---

---
[NOTE again that I do not speak for Motorola in this mailing list; these comments are not based on direct knowledge of policy, and IANAL!]

The issue is "due diligence" - a manufacturer could be held liable for damage done by a modified device if they have not made a good faith effort to make such modification either (a) safe or
(b) very difficult. Yes, this is non-value-add work, but there are people in the world who do malicious things, and manufacturers are very worried about courts finding that they were negligent in not making a greater effort to stop such malicious people.

There is also a liability issue with non-malicious changes, in that manufacturers can sometimes be held liable for things that users do to themselves. [If you believe that disclaimers of liability or warranty will actually protect you in court, you're probably being naïve.]

There are also, of course, contractual issues with carriers and content owners, who could also sue if a manufacturer didn't make a sufficient effort to protect the terms of the contract. The carriers and content owners are continually raising the bar on the level of protection they require.

---

---

While legal specifics may vary from country to country, for US Manufacturers, the same issues as above...

scott

Hi!

No, I call "you may not use your own kernel on your own hardware", even if manufacturer of that hardware is using your code and code is GPLed "unfair". Being locked onto old kernel is unfortunate, but fair.

Yep, I hope so, too.

Well, I believe Motorola is going a bit further than "due diligence". Making directories inaccessible on running system (not even read only!) was probably not mandatory, right? And even M$ smartphones support end-user updating of firmware.

Yep, I see it sucks being phone manufacturer. But you have separate CPU doing GSM stack in current cellphones, right? I believe you even use AT commands for communication... That should be about as hard as pcmcia card to "harden", and yes, people are routinelly selling pcmcia GSM cards.

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Eugeny,

Let's not be in a hurry, if possible.
I wonder why not to present PowerOP with a _separate_ *kernel* API at the moment.

Again, let's first make PowerOP accepted in mainline and then start talking about integration with cpufreq.
Looking again at the links you've provided, I'd guess BTW that you meant configfs not sysfs in some cases.

Vitaly

Vitaly,

it is exactly how PowerOP submission is positioned - it was submitted as a standalone patch without any relationship to cpufreq integration patch.

Cpufreq patch was submitted mostly for reference and discussion; to demonstrate that future integration with cpufreq is in mind and outline that PowerOP will not require cpufreq _interface_ to be changed.

that's exactly our understanding of an evolutionary development process.

configfs is in my TODO list.

Eugeny

Pavel, Greg,

I started CC'ing lkml as you guys requested (well forced really). Now we have half the discussion occurring on linux-pm and the other half on both. The guys on lkml are going to be lost and even more confused than the people on this list. If you guys want lkml to see the discussion, make sure you add them to the CC when replying to linux-pm only emails.

Perhaps we should keep the discussion on linux-pm until we are all straight and then send the results to lkml. I'm open to whatever you want as along as we are consistent.

Matt

Yep, try to cc lkml on new threads, and keep ccs intact when replying. That should be ok.

No, we want lkml included in the discussion.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

I don't understand why that would be ok. lkml will miss key discussion in threads that don't include lkml required to understand the threads that include do lkml. btw, isn't there some rule against cross posting like this?

Then you need to include lkml in the discussion not just random parts of the discussion.

Yes, it was presented in the PowerOP/cpufreq integration patches. The cpufreq_driver layer is the natural integration point.

I know its confusing having oppoint (from Dave Singleton) and powerop being discussed at the same time. However, I believe we (PowerOP) have already address most of the concerns/requirements from the community and the embedded folks. Here is a summary of PowerOP that will (hopefully) clear up our approach:

- PowerOP is only one layer (towards the bottom) in a power management solution.
- PowerOP does *not* replace cpufreq
- PowerOP can replace the cpufreq_driver layer of cpufreq but cpufreq integration not required.
- in kernel cpufreq governors are not directly affect by integrating cpufreq and powerop. (Can be extended and/or ported to more architectures and operating points)
- Again, cpufreq userspace and kernel interfaces can continue to be used as they are today.
- PowerOP defines the parameters you want to change on a per architecture/board basis.
- The PowerOP interface was discussed in detail on this list and we haven't heard any negative comments.
- We are not advocating the integration with sleep states. We want to get the PowerOP interface accepted and then we can build on it.
- PowerOP is *required* by the embedded folks.

We have a few more comments from Greg to take care of and we can add a Documentation/ file. Then I think its time to get the PowerOP patches
in the queue for acceptance. Any comments about this?

btw, PowerOP<->Cpufreq integration discussion can wait until Dominik and Dave Jones have time for more discussion.

Hi!

Yes, it is.

PowerOP provides userland interface for changing processor frequency. That's bad -- duplicate interface.

Eh? Was I on different list?

Good.

Well, you'll only get good interface review when you have Documentation/ , and it needs to go to lkml before it goes to any queues.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Basically the biggest problem with cpufreq interface is that cpufreq has "chose predefined closest to a given frequency" functionality implemented in the kernel while there is _no_ any reason to have this functionality implemented in the kernel if we have sysfs interface exported by PowerOP in place - you just _have_ to keep all possible functionality out of the kernel. CPufreq interface is just subset of sysfs interface provided by PowerOP and _must_ be implemented in userspace on top of sysfs interface - this is the proper way to scape duplication. Such issue with cpufreq<->kernel userspace interface is consequence of the fact that cpufreq implements incorrect design of PM stack layers and interfaces. PowerOP solves this issues as well.

PM stack is too complex and heavy part to go in such pieces thru lkml. i expect all linux pm experts to be on this list

Eugeny

No, there is reason to keep that in kernel -- so that cpufreq userspace interface can be kept, and so that resulting kernel<->user interface is not ugly.

Actually I believe that cpufreq implements correct design and PowerOP got it wrong.

"We are too lazy to make the code clean enough / well enough explained to survive lkml review".

Your interface impacts everyone, so everyone should have chance to comment on it.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

---

Just as a data point, "keeping the cpufreq interface" is irrelevant to a number of us, because we configure it out of the system. I'm not really arguing that we should get rid of an existing kernel interface, but I don't see any reason why we shouldn't be able to have a separately configurable interface if cpufreq doesn't meet our needs.

Note that Matthew is not arguing even that and expresses apparent contentment with cpufreq's interface.

Regards,
scott

Configurable interfaces are evil, and I do not think you can push such patch. You have developed your own little interface that suits your needs -- and that's fine -- but now you are trying to push it into mainline... and that is not, because those interfaces were not really designed to work together.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Are you saying that not having sysfs attribute nodes for entities which don't exist in a certain configuration is evil?

In x86 configuration you'd like to have just one attribute - frequency? It's just subset of common case when all power parameters available on a platform are exported. In x86 configuration you'd like to echo arbitrary frequency value into 'sys/something' and have underneath logic to chose "closest" predefine frequency? No any reason to handle this in the kernel once frequency attribute is exported.

once cpufreq userland interface functionality which does not belong to the kernel is moved out of the kernel cpufreq interface becomes a subset of PowerOP
sysfs interface. In other words this means that improvements of PM stack
layers/interfaces design will allow to design/develop an universal userspace interface. We'd prefer to move gracefully in this direction though.

Eugeny

Hi!

(cc-ed to lkml).

I'm saying that

#ifdef CONFIG_FOO
	provide user<->kernel interface

#endif

is evil.

<tongue-in-cheek warning>

Yes, once cpufreq userland interface is removed from kernel, merging powerop is reasonable thing to do. But please get at least Documentation/feature-removal-schedule.txt patch merged to mainline before attempting next powerop submission :-P.

<I'm trying to explain that removing cpufreq userland interface is about as probable as MS Linux, and only a bit less likely than hell freezing over.>
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

I disagree.

The PowerOP patch has nothing to do with the removal of cpufreq. You may be confusing this work with the david signleton patch.

--mgross

I am totally confused about who is working on what, and what provides what.

Can someone make quick cheat-sheet that shows the different developers/companies and projects here? The fact that it's impossible to keep this straight in the first place does not bode well...

Or, can each of the different projects send a _short_, 1 paragraph, 4 sentance maximum summary of the different things. Think of it as your "elevator pitch". The different multi-page summaries ware making my eyes glaze over...

thanks,

greg k-h

Sure, I think that is very necessary right now. In fact, everyone should probably hold off on commenting until this is well understood.

While I think its good to reset the discussion with a high level summary, a lot of discussion explaining why PowerOP is required for embedded devices has occurred over the last several months that can not be summarized in 4 sentences. People will definitely need to read the archives to catch up on the why.

The PowerOP summary will follow soon.

Me too.

And while we are summarizing, can we review the strategy for the large-scale improvements we are contemplating here? And then maybe even offer a list of ordered steps?

Like, do we all agree/disagree that we should do any/all of these?

- Generalize the cpufreq codebase
- Generalize the cpufreq code with generalized concept of Operating Point
- Unify handling of reduced operating states with actual sleep/off states
- Unify the PowerOP code with the Op Point code
- Unify the PowerOP, OpPoint, cpufreq and ACPI code
- Dispense with PM unification entirely and let specific implementations emerge for now
- Rewrite the lowest level of operating point handling
- Implement a user<->kernel interface for Power Management
- Allow only kernel control of PM
- Allow user-level control of PM
- Allow both/either User and Kernel code to decide on PM state transitions
- Provide distinct code bases at the lowest level for handling different components of an operating point and recognize cpufreq as one part of that, with other parts needing to be written first, before a generalized operating point handler can be written at the next layer

I think there has been some focus lost in this discussion, and I'd like to see a clear delineation of some requirements and direction just to make sure it is all headed in a common, right direction.

Thanks,
jdl

Power-OP: Matt, Eugeny, Igor, myself to a much lesser extent
Oppoint: David

Interested parties to getting operating point concepts into the main line kernel are CELF and MLI members.

CE and embedded Linux users have not been able to use cpufreq, mostly because those platforms don't have ACPI like FW for controlling all the power parameters NOT exposed on PC's and laptops, where the PM control possibilities are much more exposed to the OS requiring OS support to leverage.

The notion of operating point has become very popular in the CE /embedded space. Some variant of it is used in every CE device and there is a need to get some common way to tickle the platform PM for non PC hardware.

PowerOP is the derived from Operating Point interface portion of the DPM and is an attempt to provide a common way for this. It started life as a patch that came out about 1 year ago by Todd Poyner. This version of PowerOP has been a collaboration of a group of folks that got together at the PM-Summit last April.

Oppoint is also derived from the interface portion of DPM but goes farther to include more of DPM, causes confusion and controversy and was pushed at a bad time. It would be nice if the Oppoint patch be re-submitted as a patch to the PowerOP patch once it gets some traction in the MM tree.

hi!

Thanks.

?? zaurus uses cpufreq just fine, and you could do that on omap. You'd not have powersavings coming out of powering down devices, but otherwise it should be doable.

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Pavel,

I do think you'd better broaden your views here ;) cpufreq as it is is not well-suitable for complex power management cases where you need to switch between multiple oscillators/clock sources/PLLs etc to gain the most efficien power management. Zaurus is not that complicated in PM AFAIK and I'm sure it could be even better if it used PowerOP or similar solution. :)

Vitaly

Well, I do think you'd better broaden your views here ;) Solutions you propose are not well-suitable for big systems, like 8 cpu server box with 40 PCI devices.

What we need to do is something that works well from whole range, from 770 through thinkpad x60 to 8x Xeon server.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Cpuferq defines cpufreq_frequency_table structure in arch independent header while it's arch dependent data structure. A lot of code is built around this invalid basic brick and therefore is invalid: cpufreq tables, interface with cpu freq drivers, etc. Notion of transition latency as it defined by cpufreq is wrong because it's not a function of cpu type but function of current and next operating point. no runtime control on operating points set. it's always the same set of operating points for all system cpus in smp case and no way to define different sets or track any dependencies in case say multi core cpus. insufficient kernel<->user space interface to handle embedded requirements and no way to extend it within current design. Shall I continue? Why should then anyone want to keep cpufreq userspace interface just due to keep it?

PowerOP/cufreq integration patch set contains very details explanation of the cpufreq design issues and POwerOP solutions. Comment there is you feel it's wrong.

Eugeny

Hi!

Just for the record... this goes out to the lkml. This discussion was internal for way too long. (for interested lkml readers, I'm sure linux-pm mailing list has public archive somewhere).

Yes, please continue. I do not think we can just rip cpufreq interface out of kernel, and I do not think it is as broken as you claim it is. Ripping interface out of kernel takes years.

I'm sure cpufreq_frequency_table could be moved to asm/ header if you felt strongly about that.

I believe we need to fix cpufreq if it is broken for embedded cases.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

This was rude.

cpufreq is broken at the cpufreq_driver interface for embedded applications needing control over more than one control variable at a time.

That interface only supports setting target frequencies, and expanding it to set target frequencies and voltages is not possible without something like PowerOP. Adding the types of parameters to cpufreq would likely make cpufreq a mess. I think we would be better off with something that coexists with cpufreq, like the powerop patch from Eugeny.

God help you if you try to use cpufreq on a complex non-PC platform with multiple power and clock domains that need to be tweaked to squeeze out competitive battery life.

Because of the existing user base of cpufreq removing cpufreq will never happen. No one supporting the PowerOP patch has never recommended such a thing. However; holding back innovation because of an existing solution that doesn't support a large class of users seems dumb.

--mgross

But you can't break the existing stuff, and it seems that some of these proposals are doing just that. :(

thanks,

greg k-h

Greg,
here's a few paragraphs about the power management code I'm working on.
The OpPoint patch set is a fully functionaly power management solution, from kernel operating state support to userland power manager.

OpPoint constructs operating points for all supported frequency, voltage and suspend states for PC and SoC solutions running Linux. OpPoint does not break or replace cpufreq. It leverages cpufreq code to provide the same driver scaling functions when cpu frequency changes affect drivers.

(The ARM pxa27x patch uses the cpufreq scaling routines to scale the LCD when frequencies are changed and works well when playing mpeg movies on the LCD during frequency scaling operations).

The Operating Points in OpPoint are simply created at compile time, in the same manner cpufreq tables are, and registered in /sys/power/operating_states directory when the cpu is identified at boot time.

The states are ordered by name on their power consumption level from lowest to highest so the power manager can operate correctly regardless of what frequencies or voltages are associated with the lowest or highest operating points.

There is no kernel interface to parse and create all the parameters needed to create an operating point. Platform specific information is supplied to the oppoint structure through an ops vector of three routines and a void * pointer to supply the platform specific data, in the same manner drivers have a void * for their private data.

The ops vectors provide operating point specific functions to prepare to change to a new operating point, transition to the target operating point, and a finish transition routine to either notify driver that the clocks have scaled and operation of bus and DMA traffic may continue.

OpPoint draws the line about what's needed in the kernel a bit differently than Matt's PowerOp code. OpPoint only puts operating point support in the kernel. Polices for operting states and classes of operating states are left to the power manager, in userland. This simplifies the kernel code, no string parsers for operating point parameter construction, and makes it easier to customize a solution by customizing the power manager.

A power manager is supplied with the OpPoint patch set as well. I borrowed the cpuspeed deamon and made a simple patch that uses the new OpPoint sysfs interface. The daemon can be compiled as the original cpuspeed or oppointd deamon depending on the users choice. The daemon provides the same functions as the cpuspeed daemon.

OpPoint is a fully functional solution ready for testing and evaluation in Andrew's or your tree.

The kernel patches are available at:

       http://source.mvista.com/~dsingleton/2.6.1-rc6

the power manager source code and patch is available at:

http://source.mvista.com/~dsingleton/oppointd

David

...producing very hard-to-manage and long-to-search lists (did you consider using trees for that BTW?)
Also, you'll have to maintain different lists for differemt modifications of the same SoC. That will complicate the code prety much.

PXA is a hi-end stuff for embedded, so that might be not very illustrative.

So do you say there's no *kernel* interface to create OPs even? Assume you've got a device which needs scaling and which driver may be compiled as a module. Thus, you'll have to include OPs that reflect this driver's clock scaling at the kernel compile time. Which is nonsense in a way because the clock itself should be switched on during probe() operation of the device driver.

That sounds nice :)

Can you please list the SoCs which this solution has been tested on?

Thanks,
Vitaly

Thanks for the summary, but it was a bit longer than just "one paragraph" :)

So it works with cpufreq? That's a good thing, as it is a requirement. We can't just break current usages.

I get a 404 with that link :(

Care to resend your patches in the proper format, through email so that we can see them, and possibly get some testing in -mm if they look sane?

thanks,

greg k-h

Hi Greg,

I bet David meant http://source.mvista.com/~dsingleton/2.6.18-rc6/

Vitaly

Whoops, there are in the 2.6.18-rc6 directory. Here's the core patch inlined:

drivers/base/driver.c        |    1
drivers/base/power/Makefile  |    2
drivers/base/power/oppoint.c |   74 +++++++++
include/linux/pm.h           |   34 ++++
kernel/power/main.c          |  328 +++++++++++++++++++++++++++++++++++++------
kernel/power/power.h         |    2
6 files changed, 397 insertions(+), 44 deletions(-)

Index: linux-2.6.17/kernel/power/main.c
===================================================================
@@ -16,6 +16,7 @@

#include <linux/init.h>
#include <linux/pm.h>
#include <linux/console.h>
+#include <linux/module.h>

#include "power.h"

@@ -49,7 +50,7 @@ void pm_set_ops(struct pm_ops * ops)
* the platform can enter the requested state.
*/

-static int suspend_prepare(suspend_state_t state) +static int suspend_prepare(struct oppoint * state)

{
       int error = 0;
       unsigned int free_pages;
@@ -82,7 +83,7 @@ static int suspend_prepare(suspend_state
       }

       if (pm_ops->prepare) {
-               if ((error = pm_ops->prepare(state)))
+               if ((error = pm_ops->prepare(state->type)))
                       goto Thaw;
       }

@@ -94,7 +95,7 @@ static int suspend_prepare(suspend_state

       return 0;
 Finish:
       if (pm_ops->finish)
-               pm_ops->finish(state);
+               pm_ops->finish(state->type);
 Thaw:
       thaw_processes();

Enable_cpu:
@@ -104,7 +105,7 @@ static int suspend_prepare(suspend_state }

-int suspend_enter(suspend_state_t state)
+int suspend_enter(struct oppoint * state)

{
       int error = 0;
       unsigned long flags;
@@ -115,7 +116,7 @@ int suspend_enter(suspend_state_t state)
               printk(KERN_ERR "Some devices failed to power down\n");
               goto Done;
       }
-       error = pm_ops->enter(state);
+       error = pm_ops->enter(state->type);
       device_power_up();
 Done:
       local_irq_restore(flags);

@@ -131,36 +132,98 @@ int suspend_enter(suspend_state_t state)
* console that we've allocated. This is not called for suspend-to-disk.
*/

-static void suspend_finish(suspend_state_t state) +static void suspend_finish(struct oppoint * state)

{
       device_resume();
       resume_console();
       thaw_processes();
       enable_nonboot_cpus();
       if (pm_ops && pm_ops->finish)
-               pm_ops->finish(state);
+               pm_ops->finish(state->type);
       pm_restore_console();
}

+struct oppoint *current_state;
+struct oppoint pm_states = {
+ .name = "default",
+ .type = PM_SUSPEND_ON,
+};
+
+static struct oppoint standby = {
+ .name = "standby",
+ .type = PM_SUSPEND_STANDBY,
+};
+struct oppoint *oppoint_standby;

+static struct oppoint mem = {

+       .name = "mem",
+       .type = PM_SUSPEND_MEM,
+       .frequency = 0,
+       .voltage = 0,
+       .latency = 150,

+};
+struct oppoint *oppoint_mem;

-static const char * const pm_states[PM_SUSPEND_MAX] = {

-       [PM_SUSPEND_STANDBY]    = "standby",
-       [PM_SUSPEND_MEM]        = "mem",

#ifdef CONFIG_SOFTWARE_SUSPEND
- [PM_SUSPEND_DISK] = "disk",
-#endif
+struct oppoint disk = {
+ .name = "disk",
+ .type = PM_SUSPEND_DISK,
};
+#endif

-static inline int valid_state(suspend_state_t state) +/*
+ *
+ */
+static int pm_change_state(struct oppoint *state) +{
+ int error = 0;
+
+ printk("OpPoint: changing from %s to %s\n", current_state->name,
+ state->name);
+ /*

+        * compare to current operating point.
+        * if different change to new operating point.
+        */

+ if (current_state == state)
+ goto out;
+
+ /*

+        * prepare_transition does device constraint checking.  If
+        * a new operating point will put a device in an unsupported
+        * state, lcd clock too low, NIC bus too low, etc.  the new state
+        * cannot be entered (until the constrainded device is suspended).
+        * If prepare_transition fails we don't go to the new operating
+        * point.
+        */

+ if ((error = state->prepare_transition(current_state, state)))
+ goto out;
+
+ /*

+        * if the transition fails we call the finish transistion
+        * with the current state as the new state, causing
+        * the finish to return to the current_state.
+        */

+
+ if ((error = state->transition(current_state, state)))
+ state = current_state;
+
+ if ((state->finish_transition(current_state, state)) == 0)
+ current_state = state;
+
+out:
+ printk("OpPoint: State change returned %d\n", error);
+ return error;
+}
+
+static inline int valid_state(struct oppoint * state) {
/* Suspend-to-disk does not really need low-level support.
* It can work with reboot if needed. */
- if (state == PM_SUSPEND_DISK)
+ if (state->type == PM_SUSPEND_DISK)

               return 1;

- if (pm_ops && pm_ops->valid && !pm_ops->valid(state))
+ if (pm_ops && pm_ops->valid && !pm_ops->valid(state->type))

               return 0;
       return 1;
}

@@ -168,7 +231,7 @@ static inline int valid_state(suspend_st

/**
* enter_state - Do common work of entering low-power state.

- *     @state:         pm_state structure for state we're entering.
+ *     @state:         oppoint structure for state we're entering.

*
* Make sure we're the only ones trying to enter a sleep state. Fail
* if someone has beat us to it, since we don't want anything weird to
@@ -177,7 +240,7 @@ static inline int valid_state(suspend_st
* we've woken up).
*/

-static int enter_state(suspend_state_t state)
+static int enter_state(struct oppoint *state)

{
       int error;

@@ -186,16 +249,21 @@ static int enter_state(suspend_state_t s

       if (down_trylock(&pm_sem))
               return -EBUSY;

- if (state == PM_SUSPEND_DISK) {

+       if (state->type == PM_SUSPEND_DISK) {
               error = pm_suspend_disk();
               goto Unlock;
       }

- pr_debug("PM: Preparing system for %s sleep\n", pm_states[state]);
+ if (state->type == PM_FREQ_CHANGE || state->type == PM_VOLT_CHANGE) {
+ error = pm_change_state(state);
+ goto Unlock;
+ }
+
+ pr_debug("PM: Preparing system for %s sleep\n", state->name);
if ((error = suspend_prepare(state)))
goto Unlock;

- pr_debug("PM: Entering %s sleep\n", pm_states[state]);

+       pr_debug("PM: Entering %s sleep\n", state->name);
       error = suspend_enter(state);

       pr_debug("PM: Finishing wakeup.\n");

@@ -211,7 +279,15 @@ static int enter_state(suspend_state_t s
*/
int software_suspend(void)
{

-       return enter_state(PM_SUSPEND_DISK);
+       struct oppoint *this, *next;
+       struct list_head *head = &pm_states.list;
+       int error = 0;

+
+ list_for_each_entry_safe(this, next, head, list) {
+ if (this->type == PM_SUSPEND_DISK)
+ error= enter_state(this);
+ }
+ return error;
}

@@ -223,9 +299,9 @@ int software_suspend(void)
* structure, and enter (above).
*/

-int pm_suspend(suspend_state_t state)
+int pm_suspend(struct oppoint * state)
{
- if (state > PM_SUSPEND_ON && state <= PM_SUSPEND_MAX)
+ if (state->type > PM_SUSPEND_ON && state->type <= PM_SUSPEND_MAX)

               return enter_state(state);
       return -EINVAL;
}

@@ -248,36 +324,29 @@ decl_subsys(power,NULL,NULL);

static ssize_t state_show(struct subsystem * subsys, char * buf) {

-       int i;
       char * s = buf;

- for (i = 0; i < PM_SUSPEND_MAX; i++) {
- if (pm_states[i] && valid_state(i))
- s += sprintf(s,"%s ", pm_states[i]);

-       }
-       s += sprintf(s,"\n");
+       s += sprintf(s,"%s\n", current_state->name);
       return (s - buf);
}

static ssize_t state_store(struct subsystem * subsys, const char * buf, size_t n)
{

-       suspend_state_t state = PM_SUSPEND_STANDBY;
-       const char * const *s;
+       struct oppoint *this, *next;
+       struct list_head *head = &pm_states.list;
       char *p;
-       int error;
+       int error = -EINVAL;
       int len;

       p = memchr(buf, '\n', n);
       len = p ? p - buf : n;

-
- for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) {
- if (*s && !strncmp(buf, *s, len))
+ list_for_each_entry_safe(this, next, head, list) {
+ if ((strlen(this->name) == len) &&
+ (!strncmp(this->name, buf, len))) {

+                       error = enter_state(this);
                       break;
+               }
       }
-       if (state < PM_SUSPEND_MAX && *s)
-               error = enter_state(state);
-       else
-               error = -EINVAL;
       return error ? error : n;
}

@@ -292,12 +361,191 @@ static struct attribute_group attr_group

       .attrs = g,
};

+static struct kobject oppoint_kobj = {
+ .kset = &power_subsys.kset,
+};
+
+struct oppoint_attribute {

+        struct attribute        attr;
+        ssize_t (*show)(struct kobject * kobj, char * buf);
+        ssize_t (*store)(struct kobject * kobj, const char * buf,

size_t count);
+};
+
+#define to_oppoint(obj) container_of(obj,struct oppoint,kobj) +#define to_oppoint_attr(_attr) container_of(_attr,struct oppoint_attribute,attr)
+/*
+ * the frequency, voltage and latency files are readonly
+ */
+
+static ssize_t oppoint_voltage_show(struct kobject * kobj, char * buf) +{
+ ssize_t len;
+ struct oppoint *opt = to_oppoint(kobj);
+
+ len = sprintf(buf, "%8d\n", opt->voltage);
+
+ return len;
+}
+
+static ssize_t oppoint_voltage_store(struct kobject * kobj, const char * buf,
+ size_t n)
+{
+ return -EINVAL;
+
+}
+
+static ssize_t oppoint_frequency_show(struct kobject * kobj, char * buf) +{
+ ssize_t len;
+ struct oppoint *opt = to_oppoint(kobj);
+
+ len = sprintf(buf, "%8d\n", opt->frequency);
+
+ return len;
+}
+
+static ssize_t oppoint_frequency_store(struct kobject * kobj,
+ const char * buf, size_t n)
+{
+ return -EINVAL;
+
+}
+
+static ssize_t oppoint_latency_show(struct kobject * kobj, char * buf) +{
+ ssize_t len;
+ struct oppoint *opt = to_oppoint(kobj);
+
+ len = sprintf(buf, "%8d\n", opt->latency);
+
+ return len;
+}
+
+static ssize_t oppoint_latency_store(struct kobject * kobj,
+ const char * buf, size_t n)
+{
+ return -EINVAL;
+
+}
+
+static struct oppoint_attribute frequency_attr = {

+        .attr   = {
+                .name = "frequency",
+                .mode = 0400,
+        },
+        .show   = oppoint_frequency_show,
+        .store  = oppoint_frequency_store,
+};
+
+static struct oppoint_attribute voltage_attr = {
+        .attr   = {
+                .name = "voltage",
+                .mode = 0400,
+        },
+        .show   = oppoint_voltage_show,
+        .store  = oppoint_voltage_store,
+};
+
+static struct oppoint_attribute latency_attr = {
+        .attr   = {
+                .name = "latency",
+                .mode = 0400,
+        },
+        .show   = oppoint_latency_show,
+        .store  = oppoint_latency_store,

+};
+
+static ssize_t
+oppoint_attr_show(struct kobject * kobj, struct attribute * attr, char * buf) +{
+ struct oppoint_attribute * opt_attr = to_oppoint_attr(attr);
+ ssize_t ret = 0;
+
+ if (opt_attr->show)
+ ret = opt_attr->show(kobj,buf);
+ return ret;
+}
+
+static ssize_t
+oppoint_attr_store(struct kobject * kobj, struct attribute * attr,
+ const char * buf, size_t count)
+{
+ return -EINVAL;
+}
+
+static void oppoint_kobj_release(struct kobject *kobj)
+{
+ return;
+}
+
+static struct sysfs_ops oppoint_sysfs_ops = {

+       .show   = oppoint_attr_show,
+       .store  = oppoint_attr_store,
+};
+
+static struct attribute * oppoint_default_attrs[] = {
+       &frequency_attr.attr,
+       &voltage_attr.attr,
+       &latency_attr.attr,
+       NULL,
+};
+
+static struct kobj_type ktype_operating_point = {
+        .release        = oppoint_kobj_release,
+        .sysfs_ops      = &oppoint_sysfs_ops,
+        .default_attrs  = oppoint_default_attrs,

+};
+
+int unregister_operating_point(struct oppoint *opt) +{

+       down(&pm_sem);
+       list_del_init(&opt->list);
+       sysfs_remove_file(&opt->kobj, &frequency_attr.attr);
+       sysfs_remove_file(&opt->kobj, &voltage_attr.attr);
+       sysfs_remove_file(&opt->kobj, &latency_attr.attr);
+       up(&pm_sem);

+} +EXPORT_SYMBOL(unregister_operating_point); +
+int register_operating_point(struct oppoint *opt) +{

+       down(&pm_sem);
+       kobject_set_name(&opt->kobj, opt->name);
+       opt->kobj.kset = &power_subsys.kset;
+       opt->kobj.parent = &oppoint_kobj;
+       opt->kobj.ktype = &ktype_operating_point;
+       kobject_register(&opt->kobj);
+
+       sysfs_create_file(&opt->kobj, &frequency_attr.attr);
+       sysfs_create_file(&opt->kobj, &voltage_attr.attr);
+       sysfs_create_file(&opt->kobj, &latency_attr.attr);
+
+       list_add_tail(&opt->list, &pm_states.list);
+       up(&pm_sem);
+       return 0;

+}
+EXPORT_SYMBOL(register_operating_point);

static int __init pm_init(void)
{
+

       int error = subsystem_register(&power_subsys);
-       if (!error)
+       if (!error) {
               error = sysfs_create_group(&power_subsys.kset.kobj,&attr_group);

+ kobject_set_name(&oppoint_kobj, "operating_points");
+ kobject_register(&oppoint_kobj);
+ }
+
+
+ INIT_LIST_HEAD(&pm_states.list);
+
+#ifdef CONFIG_SOFTWARE_SUSPEND
+ register_operating_point(&disk);
+#endif

+       register_operating_point(&mem);
+       register_operating_point(&standby);
+       current_state = &pm_states;
+
       return error;
}

Index: linux-2.6.17/include/linux/pm.h
===================================================================
@@ -24,6 +24,7 @@

#ifdef __KERNEL__

#include <linux/list.h>
+#include <linux/kobject.h>
#include <asm/atomic.h>

/*
@@ -108,7 +109,36 @@ typedef int __bitwise suspend_state_t;
#define PM_SUSPEND_STANDBY ((__force suspend_state_t) 1)
#define PM_SUSPEND_MEM ((__force suspend_state_t) 3)
#define PM_SUSPEND_DISK ((__force suspend_state_t) 4)

-#define PM_SUSPEND_MAX         ((__force suspend_state_t) 5)
+#define PM_FREQ_CHANGE         ((__force suspend_state_t) 5)
+#define PM_VOLT_CHANGE         ((__force suspend_state_t) 6)
+#define PM_SUSPEND_MAX         ((__force suspend_state_t) 7)
+
+struct oppoint {
+       struct list_head list;
+       suspend_state_t type;
+       unsigned int flags;
+       char *name;
+       unsigned int frequency;         /* in KHz */
+       unsigned int voltage;           /* mV */
+       unsigned int latency;           /* transition latency in us */
+       int     (*prepare_transition)(struct oppoint *cur, struct oppoint *new);
+       int     (*transition)(struct oppoint *cur, struct oppoint *new);
+       int     (*finish_transition)(struct oppoint *cur, struct oppoint *new);

+
+ void *md_data; /* arch dependent data */
+ struct kobject kobj;
+};
+
+
+extern struct oppoint pm_states;
+extern struct oppoint *current_state;
+extern unsigned long oppoint_compute_lpj(unsigned long ref, u_int div, u_int mult);
+extern int register_operating_point(struct oppoint *opt); +extern int unregister_operating_point(struct oppoint *opt); +struct notifier_block;
+extern void oppoint_register_scale(struct notifier_block *nb, int level); +extern void oppoint_unregister_scale(struct notifier_block *nb, int level); +extern int oppoint_driver_scale(int level, struct oppoint *new);

typedef int __bitwise suspend_disk_method_t;

@@ -128,7 +158,7 @@ struct pm_ops {

extern void pm_set_ops(struct pm_ops *);
extern struct pm_ops *pm_ops;
-extern int pm_suspend(suspend_state_t state);
+extern int pm_suspend(struct oppoint *state);

/*

Index: linux-2.6.17/kernel/power/power.h
===================================================================
@@ -113,4 +113,4 @@ extern int swsusp_resume(void);

extern int swsusp_read(void);
extern int swsusp_write(void);
extern void swsusp_close(void);
-extern int suspend_enter(suspend_state_t state); +extern int suspend_enter(struct oppoint * state);

Index: linux-2.6.17/drivers/base/driver.c
===================================================================
@@ -12,6 +12,7 @@

#include <linux/module.h>
#include <linux/errno.h>
#include <linux/string.h>
+#include <linux/pm.h>
#include "base.h"

#define to_dev(node) container_of(node, struct device, driver_list)

Index: linux-2.6.17/drivers/base/power/Makefile
===================================================================
@@ -1,4 +1,4 @@
-obj-y                  := shutdown.o
+obj-y                  := shutdown.o oppoint.o

obj-$(CONFIG_PM) += main.o suspend.o resume.o runtime.o sysfs.o
obj-$(CONFIG_PM_TRACE) += trace.o

David

Greg,
here's the patch that leverages the cpufreq notifier lists for
driver PRE and POST
change functions. I'm also rebasing to 2.6.18-rc7 and making changes Pavel suggested about just having suspend states in /sys/power/state and moving the operating point control file down under /sys/power/operating_states directory.

 drivers/cpufreq/cpufreq.c |   36 ++++++++++++++++++++++++++++++++++++
 include/linux/cpufreq.h   |    2 ++
 2 files changed, 38 insertions(+)

Index: linux-2.6.17/drivers/cpufreq/cpufreq.c
===================================================================
@@ -226,6 +226,35 @@ static void adjust_jiffies(unsigned long
 static inline void adjust_jiffies(unsigned long val, struct

cpufreq_freqs *ci) { return; }
#endif

+int cpufreq_prepare_transition(struct oppoint *cur, struct oppoint *new) +{
+ struct cpufreq_freqs freqs;
+

+       freqs.old = cur->frequency;
+       freqs.new = new->frequency;
+       freqs.cpu = 0;
+       freqs.flags = new->flags;
+       blocking_notifier_call_chain(&cpufreq_transition_notifier_list,

+ CPUFREQ_PRECHANGE, &freqs);
+ adjust_jiffies(CPUFREQ_PRECHANGE, &freqs);
+ return 0;
+}
+EXPORT_SYMBOL(cpufreq_prepare_transition);
+
+int cpufreq_finish_transition(struct oppoint *cur, struct oppoint *new) +{
+ struct cpufreq_freqs freqs;
+

+       freqs.old = cur->frequency;
+       freqs.new = new->frequency;
+       freqs.cpu = 0;
+       freqs.flags = new->flags;
+       adjust_jiffies(CPUFREQ_POSTCHANGE, &freqs);
+       blocking_notifier_call_chain(&cpufreq_transition_notifier_list,

+ CPUFREQ_POSTCHANGE, &freqs);
+ return 0;
+}
+EXPORT_SYMBOL(cpufreq_finish_transition);

/**
* cpufreq_notify_transition - call notifier chain and adjust_jiffies
@@ -920,6 +949,12 @@ static void cpufreq_out_of_sync(unsigned
}

+#ifdef CONFIG_PM
+unsigned int cpufreq_quick_get(unsigned int cpu)
+{
+ return (current_state->frequency);
+}
+#else
/**
* cpufreq_quick_get - get the CPU frequency (in kHz) frpm policy->cur
* @cpu: CPU number
@@ -941,6 +976,7 @@ unsigned int cpufreq_quick_get(unsigned

        return (ret);

}
+#endif
EXPORT_SYMBOL(cpufreq_quick_get);

Index: linux-2.6.17/include/linux/cpufreq.h
===================================================================
@@ -268,6 +268,8 @@ static inline unsigned int cpufreq_quick
        return 0;
 }
 #endif
+int cpufreq_prepare_transition(struct oppoint *cur, struct oppoint *new);
+int cpufreq_finish_transition(struct oppoint *cur, struct oppoint *new);
 
 
 /*********************************************************************

Greg,
here's the patch that implements operating points for different frequencies
for the speedstep-centrino line of processors. Operating points are created in much the same manner that cpufreq tables are. This works for both simple implementations like the centrino and more complex SoC systems like the arm-pxa72x which has several clocks to control, and different clock divisors and multipliers.

David

 arch/i386/Kconfig                                |    2
 arch/i386/kernel/cpu/Makefile                    |    1
 arch/i386/kernel/cpu/power/Kconfig               |  168 ++++++++++
 arch/i386/kernel/cpu/power/Makefile              |    2
 arch/i386/kernel/cpu/power/centrino-on-the-fly.c |   72 ++++
 arch/i386/kernel/cpu/power/centrino-speedstep.c  |  368 +++++++++++++++++++++++
 arch/i386/kernel/i386_ksyms.c                    |    4
 7 files changed, 617 insertions(+)

Index: linux-2.6.17/arch/i386/kernel/cpu/Makefile
===================================================================
@@ -17,3 +17,4 @@ obj-$(CONFIG_X86_MCE) +=      mcheck/
 
 obj-$(CONFIG_MTRR)     +=      mtrr/
 obj-$(CONFIG_CPU_FREQ) +=      cpufreq/
+obj-$(CONFIG_PM)       +=      power/

Index: linux-2.6.17/arch/i386/kernel/i386_ksyms.c
===================================================================
@@ -28,3 +28,7 @@ EXPORT_SYMBOL(__read_lock_failed);
 #endif
 
 EXPORT_SYMBOL(csum_partial);
+#ifdef CONFIG_PM
+#include <linux/pm.h>
+EXPORT_SYMBOL(pm_states);
+#endif

Index: linux-2.6.17/arch/i386/Kconfig
===================================================================
@@ -964,6 +964,8 @@ config APM_REAL_MODE_POWER_OFF
 
 endmenu
 
+source "arch/i386/kernel/cpu/power/Kconfig"
+
 source "arch/i386/kernel/cpu/cpufreq/Kconfig"
 
 endmenu

Index: linux-2.6.17/arch/i386/kernel/cpu/power/Makefile
===================================================================
@@ -0,0 +1,2 @@
+obj-m                                  += centrino-on-the-fly.o
+obj-$(CONFIG_X86_SPEEDSTEP_CENTRINO)   += centrino-speedstep.o

struct cpu_id *x);
+
+void centrino_set_frequency(struct oppoint *op, uint freq, uint volt) +{

+       op->frequency = freq * 1000;
+       op->voltage = volt;
+       op->md_data = (void *)(((freq / 100) << 8) | (volt - 700) / 16);

+}
+EXPORT_SYMBOL(centrino_set_frequency);
+
+int centrino_transition(struct oppoint *cur, struct oppoint *new) +{
+ unsigned int msr, oldmsr = 0, h = 0;
+
+ if (cur == new)
+ return 0;
+
+ msr = (unsigned int)new->md_data;
+ rdmsr(MSR_IA32_PERF_CTL, oldmsr, h);
+

+       /* all but 16 LSB are reserved, treat them with care */
+       oldmsr &= ~0xffff;
+       msr &= 0xffff;
+       oldmsr |= msr;

+
+ wrmsr(MSR_IA32_PERF_CTL, oldmsr, h);
+
+ udelay(new->latency);
+
+ return 0;
+}
+EXPORT_SYMBOL(centrino_transition);
+
+#define _BANIAS(cpuid, max, name) \

+{      .cpu_id         = cpuid,        \
+       .model_name     = "Intel(R) Pentium(R) M processor " name "MHz", \
+       .max_freq       = (max)*1000,   \

+}
+#define BANIAS(max) _BANIAS(&cpu_ids[CPU_BANIAS], max, #max)
+
+/*
+ * CPU models, their operating frequency range, and freq/voltage
+ * operating points
+ */
+static struct cpu_model models[] =
+{

+       _BANIAS(&cpu_ids[CPU_BANIAS], 900, " 900"),
+       BANIAS(1000),
+       BANIAS(1100),
+       BANIAS(1200),
+       BANIAS(1300),
+       BANIAS(1400),
+       BANIAS(1500),
+       BANIAS(1600),
+       BANIAS(1700),
+
+       /* NULL model_name is a wildcard */
+       { &cpu_ids[CPU_DOTHAN_A1], NULL, 0},
+       { &cpu_ids[CPU_DOTHAN_A2], NULL, 0},
+       { &cpu_ids[CPU_DOTHAN_B0], NULL, 0},
+       { &cpu_ids[CPU_MP4HT_D0], NULL, 0},
+       { &cpu_ids[CPU_MP4HT_E0], NULL, 0},

+
+ { NULL, }
+};
+#undef _BANIAS
+#undef BANIAS
+
+static struct oppoint lowest = {

+       .name = "lowest",
+       .type = PM_FREQ_CHANGE,
+       .frequency = 0,
+       .voltage = 0,
+       .latency = 15,
+       .prepare_transition  = cpufreq_prepare_transition,
+       .transition = centrino_transition,
+       .finish_transition = cpufreq_finish_transition,
+};
+
+static struct oppoint low = {
+       .name = "low",
+       .type = PM_FREQ_CHANGE,
+       .latency = 15,
+       .prepare_transition  = cpufreq_prepare_transition,
+       .transition = centrino_transition,
+       .finish_transition = cpufreq_finish_transition,
+};
+
+static struct oppoint mediumlow = {
+       .name = "mediumlow",
+       .type = PM_FREQ_CHANGE,
+       .latency = 15,
+       .prepare_transition  = cpufreq_prepare_transition,
+       .transition = centrino_transition,
+       .finish_transition = cpufreq_finish_transition,
+};
+
+static struct oppoint medium = {
+       .name = "medium",
+       .type = PM_FREQ_CHANGE,
+       .latency = 15,
+       .prepare_transition  = cpufreq_prepare_transition,
+       .transition = centrino_transition,
+       .finish_transition = cpufreq_finish_transition,
+};
+
+static struct oppoint mediumhigh = {
+       .name = "mediumhigh",
+       .type = PM_FREQ_CHANGE,
+       .latency = 15,
+       .prepare_transition  = cpufreq_prepare_transition,
+       .transition = centrino_transition,
+       .finish_transition = cpufreq_finish_transition,
+};
+
+static struct oppoint high = {
+       .name = "high",
+       .type = PM_FREQ_CHANGE,
+       .latency = 15,
+       .prepare_transition  = cpufreq_prepare_transition,
+       .transition = centrino_transition,
+       .finish_transition = cpufreq_finish_transition,
+};
+
+static struct oppoint highest = {
+       .name = "highest",
+       .type = PM_FREQ_CHANGE,
+       .latency = 15,
+       .prepare_transition  = cpufreq_prepare_transition,
+       .transition = centrino_transition,
+       .finish_transition = cpufreq_finish_transition,

+};
+
+static int __init centrino_init_oppoint(void)
+{
+ struct cpuinfo_x86 *cpu = &cpu_data[0];
+ struct cpu_model *model;
+
+ for(model = models; model->cpu_id != NULL; model++) {
+ if (centrino_verify_cpu_id(cpu, model->cpu_id) &&
+ (model->model_name == NULL ||
+ strcmp(cpu->x86_model_id, model->model_name) == 0))
+ break;
+ }
+
+ if (model->cpu_id == NULL) {
+ /* No match at all */
+ printk("OpPoint: no support for CPU model %s\n",
+ cpu->x86_model_id);
+ return -ENOENT;
+ }
+
+ switch (model->max_freq) {
+ /* Ultra Low Voltage Intel Pentium M processor 900MHz (Banias) */
+ case (900000) :
+ {

+               centrino_set_frequency(&low, 600, 844);
+               centrino_set_frequency(&medium, 800, 988);
+               centrino_set_frequency(&high, 900, 1004);
+               break;

+ }
+ /* Ultra Low Voltage Intel Pentium M processor 1.00GHz (Banias) */
+ case (1000000) :
+ {

+               centrino_set_frequency(&low, 600, 844);
+               centrino_set_frequency(&medium, 800, 972);
+               centrino_set_frequency(&high, 900, 988);
+               centrino_set_frequency(&highest, 1000, 1004);
+               break;

+ }
+ /* Ultra Low Voltage Intel Pentium M processor 1.10GHz (Banias) */
+ case (1100000) :
+ {

+               centrino_set_frequency(&lowest, 600, 956);
+               centrino_set_frequency(&low, 800, 1020);
+               centrino_set_frequency(&medium, 900, 1100);
+               centrino_set_frequency(&high, 1000, 1164);
+               centrino_set_frequency(&highest, 1100, 1180);
+               break;

+ }
+ /* Ultra Low Voltage Intel Pentium M processor 1.10GHz (Banias) */
+ case (1200000) :
+ {

+               centrino_set_frequency(&lowest, 600, 956);
+               centrino_set_frequency(&low, 800, 1004);
+               centrino_set_frequency(&medium, 900, 1020);
+               centrino_set_frequency(&mediumhigh, 1000, 1100);
+               centrino_set_frequency(&high, 1100, 1164);
+               centrino_set_frequency(&highest, 1200, 1180);
+               break;

+ }
+ /* Ultra Low Voltage Intel Pentium M processor 1.10GHz (Banias) */
+ case (1300000) :
+ {

+               centrino_set_frequency(&lowest, 600, 956);
+               centrino_set_frequency(&low, 800, 1260);
+               centrino_set_frequency(&medium, 1000, 1292);
+               centrino_set_frequency(&high, 1200, 1356);
+               centrino_set_frequency(&highest, 1300, 1388);
+               break;

+ }
+ /* Ultra Low Voltage Intel Pentium M processor 1.10GHz (Banias) */
+ case (1400000) :
+ {

+               centrino_set_frequency(&lowest, 600, 956);
+               centrino_set_frequency(&low, 800, 1180);
+               centrino_set_frequency(&medium, 1000, 1308);
+               centrino_set_frequency(&high, 1200, 1436);
+               centrino_set_frequency(&highest, 1400, 1484);
+               break;

+ }
+ /* Ultra Low Voltage Intel Pentium M processor 1.10GHz (Banias) */
+ case (1500000) :
+ {

+               centrino_set_frequency(&lowest, 600, 956);
+               centrino_set_frequency(&low, 800, 1116);
+               centrino_set_frequency(&medium, 1000, 1228);
+               centrino_set_frequency(&mediumhigh, 1200, 1356);
+               centrino_set_frequency(&high, 1400, 1452);
+               centrino_set_frequency(&highest, 1500, 1484);
+               break;

+ }
+ /* Ultra Low Voltage Intel Pentium M processor 1.10GHz (Banias) */
+ case (1600000) :
+ {

+               centrino_set_frequency(&lowest, 600, 956);
+               centrino_set_frequency(&low, 800, 1036);
+               centrino_set_frequency(&medium, 1000, 1164);
+               centrino_set_frequency(&mediumhigh, 1200, 1276);
+               centrino_set_frequency(&high, 1400, 1420);
+               centrino_set_frequency(&highest, 1600, 1484);
+               break;

+ }
+ /* Ultra Low Voltage Intel Pentium M processor 1.10GHz (Banias) */
+ case (1700000) :
+ {

+               centrino_set_frequency(&lowest, 600, 956);
+               centrino_set_frequency(&low, 800, 1004);
+               centrino_set_frequency(&medium, 1000, 1116);
+               centrino_set_frequency(&mediumhigh, 1200, 1228);
+               centrino_set_frequency(&high, 1400, 1308);
+               centrino_set_frequency(&highest, 1700, 1484);
+               break;

+ }
+ }
+ if (lowest.frequency) {
+ register_operating_point(&lowest);
+ list_add_tail(&lowest.list, &pm_states.list);
+ }
+ if (low.frequency) {
+ register_operating_point(&low);
+ list_add_tail(&low.list, &pm_states.list);
+ }
+ if (mediumlow.frequency) {
+ register_operating_point(&mediumlow);
+ list_add_tail(&mediumlow.list, &pm_states.list);
+ }
+ if (medium.frequency) {
+ register_operating_point(&medium);
+ list_add_tail(&medium.list, &pm_states.list);
+ }
+ if (mediumhigh.frequency) {
+ register_operating_point(&mediumhigh);
+ list_add_tail(&mediumhigh.list, &pm_states.list);
+ }
+ if (high.frequency) {

+               register_operating_point(&high);
+               list_add_tail(&high.list, &pm_states.list);
+               current_state = &high;
+       }
+       if (highest.frequency) {
+               register_operating_point(&highest);
+               list_add_tail(&highest.list, &pm_states.list);
+               current_state = &highest;

+ }
+ return 0;
+}
+
+static void centrino_exit_oppoint(void)
+{
+ if (lowest.frequency)
+ list_del_init(&lowest.list);
+ if (low.frequency)
+ list_del_init(&low.list);
+ if (mediumlow.frequency)
+ list_del_init(&mediumlow.list);
+ if (medium.frequency)
+ list_del_init(&medium.list);
+ if (mediumhigh.frequency)
+ list_del_init(&mediumhigh.list);
+ if (high.frequency)
+ list_del_init(&high.list);
+ if (highest.frequency)
+ list_del_init(&highest.list);
+ return;
+}
+
+static int centrino_verify_cpu_id(const struct cpuinfo_x86 *c, const struct cpu_id *x)
+{
+ if ((c->x86 == x->x86) &&
+ (c->x86_model == x->x86_model) &&
+ (c->x86_mask == x->x86_mask))
+ return 1;
+ return 0;
+}
+
+MODULE_AUTHOR ("David Singleton <[email protected]>"); +MODULE_DESCRIPTION ("OpPoint operting points for Intel Pentium M processors."); +MODULE_LICENSE ("GPL");
+
+late_initcall(centrino_init_oppoint); +module_exit(centrino_exit_oppoint);

Heh, that shows pretty much that the approach itself is not good... And this is only beginning.

Vitaly

Yes it could. The centrino was the first platform I tested on and I used the existing speedstep-centrino code from cpufreq. The 1.86Ghz was not in
the cpufreq base. But you can see how easy it is to add new operating points
for a new cpu.

Adding new platform support is quite straight forward. It basically requires a function to transition to the new operating point and the parameters needed for the transition.

David

Hi!

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Maybe a compromise could be reached where a defined set of numbers maps to string names ala Unix init states. Many people (at least me) still invoke init 6 to reboot a system. A defined table would satisfy both the number and string camps.

Richard

PowerOP allows the platform to define the name. In our cpufreq integration patches, we reuse the same name that cpufreq centrino used.

Greg,
here's the arm-pxa7x patch which creates operating points for the much more
complex ARM platform. It illustrates the straight forward nature of creating operating points, both frequency and sleep states, for different and more complex architectures, and eliminates the need to have users pass in all the parameters needed to create an operating point.

arch/arm/Kconfig                      |    2
arch/arm/mach-pxa/Makefile            |    3
arch/arm/mach-pxa/mainstone_freq.c    |  211 +++
arch/arm/mach-pxa/mainstone_oppoint.c | 1910 ++++++++++++++++++++++++++++++++++
arch/arm/mach-pxa/mainstone_volt.c    |  363 ++++++
include/asm-arm/arch-pxa/oppoint.h    |  119 ++
include/asm-arm/arch-pxa/pxa-regs.h   |    8
7 files changed, 2614 insertions(+), 2 deletions(-)

Index: linux-2.6.17/arch/arm/mach-pxa/Makefile
===================================================================
@@ -10,7 +10,8 @@ obj-$(CONFIG_PXA27x) += pxa27x.o

# Specific board support
obj-$(CONFIG_ARCH_LUBBOCK) += lubbock.o obj-$(CONFIG_MACH_LOGICPD_PXA270) += lpd270.o -obj-$(CONFIG_MACH_MAINSTONE) += mainstone.o +obj-$(CONFIG_MACH_MAINSTONE) += mainstone.o mainstone_oppoint.o \
+mainstone_freq.o mainstone_volt.o obj-$(CONFIG_ARCH_PXA_IDP) += idp.o
obj-$(CONFIG_MACH_TRIZEPS4) += trizeps4.o
obj-$(CONFIG_PXA_SHARP_C7xx) += corgi.o corgi_ssp.o corgi_lcd.o
sharpsl_pm.o corgi_pm.o

Index: linux-2.6.17/arch/arm/mach-pxa/mainstone_freq.c
===================================================================
@@ -0,0 +1,211 @@
+/*
+ * linux/arch/arm/mach-pxa/mainstone_freq.c
+ *
+ * Functions to change CPU frequencies on the Bulverde processor
+ * adopted from Intel code for MontaVista Linux.
+ *
+ * Author: David Singleton <[email protected]>
+ *
+ * 2006 (c) MontaVista Software, Inc. This file is licensed under the
+ * terms of the GNU General Public License version 2. This program is
+ * licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/init.h>
+#include <linux/mm.h>
+#include <linux/hardirq.h>
+#include <asm/io.h>
+
+#include <asm/hardware.h>
+#include <asm/arch/oppoint.h>
+#include <asm/arch/pxa-regs.h>
+#include <asm/pgtable.h>
+#include <asm/pgalloc.h>
+
+#include <asm/arch/mainstone.h>
+
+/*
+ * Since CPDIS and PPDIS is always the same, we use only one definition here.
+ */
+#define        PDIS    0       /* Core PLL and Peripheral PLL is

enabled after FCS. */
+
+/*
+ * Available CPU frequency list for Bulverde.
+ */
+static unsigned int cpufreq_matrix[N_NUM][L_NUM + 1]; +static void mainstone_freq_debug_info(void); +static volatile int *ramstart;
+
+/*
+ * Init according to mainstone manual.
+ */
+static void mainstone_initialize_freq_matrix(void) +{
+ int n, l;
+
+ memset(&cpufreq_matrix, 0, sizeof(cpufreq_matrix));
+
+ for (n = 2; n < N_NUM + 2; n++) {
+ for (l = 2; l <= L_NUM; l++) {
+ cpufreq_matrix[n - 2][l - 2] = (13 * n * l / 2) * 1000;
+ if (cpufreq_matrix[n - 2][l - 2] > BLVD_MAX_FREQ)
+ cpufreq_matrix[n - 2][l - 2] = 0;
+ }
+ }
+}
+
+/*
+ * This should be called with a valid freq point that was
+ * obtained via mainstone_validate_speed
+ */
+void mainstone_set_freq(unsigned int CLKCFGValue) +{

+       unsigned long flags;
+       unsigned int unused;
+       volatile int v;

+
+ local_irq_save(flags);
+
+ /*

+        * force a tlb fault to get the mapping into the tlb
+        * (otherwise this will occur below when the sdram is turned off and
+        * something-bad(tm) will happen)
+        */

+ v = *(volatile unsigned long *)ramstart;
+ *(volatile unsigned long *)ramstart = v;
+
+ MST_LEDDAT1 = CLKCFGValue;
+
+ __asm__ __volatile__(" \n\

+               ldr     r4, [%1]                        @load MDREFR \n\
+               mcr     p14, 0, %2, c6, c0, 0           @ set CCLKCFG[FCS] \n\
+               ldr     r5, =0xe3dfefff \n\
+               and     r4, r4, r5      \n\
+               str     r4,  [%1]                       @restore \n\
+               ":"=&r"(unused)

+ :"r"(&MDREFR), "r"(CLKCFGValue), "r"(ramstart)
+ :"r4", "r5");
+
+ MST_LEDDAT1 = 0x0002;
+ /*
+ NOTE: if we don't turn off IRQs up top, there is no point
+ to restoring them here.
+ */
+ local_irq_restore(flags);
+
+ /* spit out some info about what happened */
+ mainstone_freq_debug_info();
+
+}
+
+extern void mainstone_get_current_info(struct md_opt *); +
+static void mainstone_freq_debug_info(void)
+{
+ unsigned int sysbus, run, t, turbo, mem, m = 1;
+ struct md_opt opt;
+
+ mainstone_get_current_info(&opt);
+

+       run = 13000 * opt.l;
+       turbo = (13000 * opt.l * opt.n) >> 1;
+       sysbus = (opt.b) ? run : (run / 2);
+       t = opt.regs.clkcfg & 0x1;

+
+ /* If CCCR[A] is on */
+ if (opt.cccra) {
+ mem = sysbus;
+ } else {
+ /* If A=0
+ m initialized to 1 (for l=2-10)
+ */
+ if (opt.l > 10)
+ m = 2; /* for l=11-20 */
+ if (opt.l > 20)
+ m = 4; /* for l=21-31 */
+ mem = run / m;
+ }
+}
+
+int mainstone_get_freq(void)
+{
+ unsigned int freq, n, l, ccsr;
+
+ ccsr = CCSR;
+
+ l = ccsr & CCCR_L_MASK; /* Get L */
+ n = (ccsr & CCCR_N_MASK) >> 7; /* Get 2N */
+
+ if (n < 2)
+ n = 2;
+
+ /* Shift to divide by 2 because N is really 2N */
+ freq = (13000 * l * n) >> 1; /* in kHz */
+
+ return freq;
+}
+
+unsigned int mainstone_read_clkcfg(void)
+{
+ unsigned int value = 0;
+ unsigned int un_used;
+
+ __asm__ __volatile__("mrc p14, 0, %0, c6, c0, 0":
"=r"(value) : "r"(un_used) );
+
+ return value;
+}
+
+static int mainstone_init_freqs(void)
+{
+ int cpu_ver;
+
+ asm volatile ("mrc%? p15, 0, %0, c0, c0":"=r" (cpu_ver));
+
+ /*

+          Bulverde     A0:     0x69054110,
+          A1 :         0x69054111

+ */
+ if ((cpu_ver & 0x0000f000) >> 12 == 4 &&
+ (cpu_ver & 0xffff0000) >> 16 == 0x6905) {
+ /* It is an xscale core bulverde chip. */
+ return 1;
+ }
+
+ return 0;
+}
+
+int mainstone_clk_init(void)
+{
+ unsigned int freq;
+
+ /*

+        * In order to turn the sdram back on (see below) we need to
+        * r/w the sdram.  We need to do this without the cache and
+        * write buffer in the way.  So, we temporarily ioremap the
+        * first page of sdram as uncached i/o memory and use the
+        * aliased address
+        */

+
+ /* map the first page of sdram to an uncached virtual page */
+ ramstart = (int *)ioremap(PHYS_OFFSET, 4096);
+
+ freq = mainstone_get_freq(); /* in kHz */
+ printk(KERN_INFO "Init freq: %dkHz.\n", freq);
+
+ mainstone_initialize_freq_matrix();
+
+ if (mainstone_init_freqs()) {
+ printk(KERN_INFO "CPU frequency change initialized.\n");
+ }
+ return 0;
+}
+
+void mainstone_freq_cleanup(void)
+{
+ /* unmap the page we used*/
+ iounmap((void *)ramstart);
+}

Index: linux-2.6.17/include/asm-arm/arch-pxa/pxa-regs.h
===================================================================
@@ -2042,6 +2042,14 @@

#define MDMRS __REG(0x48000040) /* MRS value to be written
to SDRAM */
#define BOOT_DEF __REG(0x48000044) /* Read-Only Boot-Time
Register. Contains BOOT_SEL and PKG_SEL */

+#define MDREFR_DRI 0xFFF
+#define MSC0_RDF (0xF << 20)
+#define MSC0_RDN (0xF << 24)
+#define MSC0_RRR (0x7 << 12)
+#define MDREFR_RFU 0xC0200000
+#define MDCNFG_DTC0 (0x3 << 8)
+#define MDCNFG_DTC2 (0x3 << 24)
+
/*
* More handy macros for PCMCIA
*

MDREFR_K2DB2);
+ }
+
+ /* clock increasing or decreasing? */
+ if (new_memclk > cur_memclk) scaling_up = 1;
+ }
+
+ /* set MDREFR if necessary */
+ if (new_mdrefr != cur_mdrefr){

+               set_mdrefr = 1;
+               /* also adjust timings  as long as we change MDREFR value */
+               assign_optimal_mem_timings(
+                                          &new_msc0,
+                                          &new_mdrefr,
+                                          &new_mdcnfg,
+                                          a,b,l
+                                          );

+ }
+
+ /* if memclk is scaling up, set MDREFR before freq change
+ * (2800002.pdf:6.5.1.4)
+ */
+ if (set_mdrefr && scaling_up) {

+               MSC0 = new_msc0;
+               set_mdrefr_value(new_mdrefr);
+               MDCNFG = new_mdcnfg;
+               read_mdrefr = MDREFR;

+ }
+
+ CCCR = regs->cccr;
+ mainstone_set_freq(regs->clkcfg);
+
+ /* if memclk is scaling down, set MDREFR after freq change
+ * (2800002.pdf:6.5.1.4)
+ */
+ if (set_mdrefr && !scaling_up) {

+               MSC0 = new_msc0;
+               set_mdrefr_value(new_mdrefr);
+               MDCNFG = new_mdcnfg;
+               read_mdrefr = MDREFR;

+ }
+}
+
+static void mainstone_scale_voltage(struct oppoint_regs *regs) +{
+ mainstone_set_voltage(regs->voltage);
+}
+
+static void mainstone_scale_voltage_coupled(struct oppoint_regs *regs) +{
+ mainstone_prep_set_voltage(regs->voltage);
+}
+
+static void calculate_lcd_freq(struct md_opt *opt)
+{
+ int k = 1; /* lcd divisor */
+
+ /* L is verified to be between PLL_L_MAX and PLL_L_MIN in */
+ if (opt->l == -1) {
+ opt->lcd = -1;
+ return;
+ }
+
+ if (opt->l > 16) {
+ /* When L=17-31, K=4 */
+ k = 4;
+ } else if (opt->l > 7) {
+ /* When L=8-16, K=2 */
+ k = 2;
+ }
+
+ /* Else, when L=2-7, K=1 */
+
+ opt->lcd = 13000 * opt->l / k;
+}
+
+static void calculate_reg_values(struct md_opt *opt)
+{

+       int f = 0;              /* frequency change bit */
+       int turbo = 0;          /* turbo mode bit; depends on N value */

+
+ opt->regs.voltage = opt->v;
+/*
+ CCCR

+    | 31| 30|29-28| 27| 26| 25|24-11| 10| 9 | 8 | 7 |6-5  | 4 | 3 | 2 | 1 | 0 |
+    | C | P |     | L | P |   |     |         |     |             |
+    | P | P |     | C | L |   |     |         |     |             |
+    | D | D |resrv| D | L | A |resrv|  2 * N   |resrv|    L          |
+    | I | I |     | 2 | . |   |     |         |     |             |
+    | S | S |     | 6 | . |   |     |         |     |             |

+
+ A: Alternate setting for MEMC clock
+ 0 = MEM clock frequency as specified in YB's table 3-7
+ 1 = MEM clock frq = System Bus Frequency
+
+
+ CLKCFG

+    | 31------------------------------------------- | 3 | 2  | 1 | 0 |
+    | --------------------------------------------- | B | HT | F | T |

+
+ B = Fast-Bus Mode 0: System Bus is half of run-mode
+ 1: System Bus is equal to run-mode
+ NOTE: only allowed when L <= 16
+
+ HT = Half-Turbo 0: core frequency = run or turbo, depending on T bit
+ 1: core frequency = turbo frequency / 2
+ NOTE: only allowed when 2N = 6 or 2N = 8
+
+ F = Frequency change
+ 0: No frequency change is performed
+ 1: Do frequency-change
+
+ T = Turbo Mode 0: CPU operates at run Frequency
+ 1: CPU operates at Turbo Frequency (when n2 > 2)
+
+*/
+ /* Set the CLKCFG with B, T, and HT */
+ if (opt->b != -1 && opt->n != -1) {
+ f = 1;
+
+ /*When N2=2, Turbo Mode equals Run Mode, so it
+ does not really matter if this is >2 or >=2
+ */
+ if (opt->n > 2) {
+ turbo = 0x1;
+ }
+ opt->regs.clkcfg = (opt->b << 3) + (f << 1) + turbo;
+ } else {
+ f = 0x1;
+ opt->regs.clkcfg = (f << 1);
+ }
+
+ /*
+ What about when n2=0 ... it is not defined by the yellow
+ book
+ */
+ if (opt->n != -1) {
+ /* N2 is 4 bits, L is 5 bits */
+ opt->regs.cccr = ((opt->n & 0xF) << 7) + (opt->l & 0x1F);
+ }
+
+ if (opt->cccra > 0) {
+ /* Turn on the CCCR[A] bit */
+ opt->regs.cccr |= (1 << 25);
+ }
+

+       /* 13M Mode */
+       if (opt->l == 1) {
+       }

+
+ if ( (opt->l > 1) && (opt->cpll_enabled == 0) ) {
+ printk(KERN_WARNING
+ "DPM: internal error if l>1 CPLL must be On\n");
+ }
+ if( (opt->cpll_enabled == 1) && (opt->ppll_enabled == 0) ){
+ printk(KERN_WARNING
+ "DPM: internal error CPLL=On PPLL=Off is NOT
supported in hardware\n");
+ }
+ if(opt->cpll_enabled == 0) {
+ opt->regs.cccr |= (CCCR_CPDIS_BIT_ON);
+ }
+ if(opt->ppll_enabled == 0) {
+ opt->regs.cccr |= (CCCR_PPDIS_BIT_ON);
+ }
+
+}
+
+/* This routine computes the "forward" frequency scaler flags
+ * for moving the system
+ * from the current operating point to the new operating point. The resulting
+ * fscaler is applied to the registers of the new operating point.
+ */
+void compute_fscaler_flags(struct md_opt *cur, struct md_opt *new) +{
+ int current_n, ccsr;
+

+       ccsr = CCSR;
+       current_n = (ccsr & CCCR_N_MASK) >> 7;
+       fscaler_flags = FSCALER_NOP;
+       /* If new CPU is 0, that means sleep, we do NOT switch PLLs

+ if going to sleep.
+ */
+ if (!new->cpu) {
+ if (new->sleep_mode == CPUMODE_DEEPSLEEP) {
+ fscaler_flags |= FSCALER_DEEPSLEEP;
+ } else if (new->sleep_mode == CPUMODE_STANDBY) {
+ fscaler_flags |= FSCALER_STANDBY;
+ } else {
+ fscaler_flags |= FSCALER_SLEEP;
+ }
+ } else {
+
+ /*

+                * If exiting 13M mode, set the flag so we can do the extra
+                * work to get out before the frequency change
+                */

+ if( ((cur->cpll_enabled == 0) && (new->cpll_enabled ==1)) ||
+ ((cur->ppll_enabled == 0) && (new->ppll_enabled ==1)) ){
+ fscaler_flags |= FSCALER_XPLLON;
+ }
+
+ }
+
+
+ /* if CPU is *something*, it means we are not going to sleep */
+ if ((new->cpu) &&
+ /* And something has indeed changed */
+ ((new->regs.cccr != cur->regs.cccr) ||
+ (new->regs.clkcfg != cur->regs.clkcfg))) {
+
+ /* Find out if it is *just* a turbo bit change */
+ if ((cur->l == new->l) &&

+                   (cur->cccra == new->cccra) &&
+                   (cur->b == new->b) &&
+                   (cur->half_turbo == new->half_turbo)) {
+                       /*
+                        * If the real, current N is a turbo freq and
+                        * the new N is not a turbo freq, then set
+                        * TURBO_OFF and do not change N
+                        */

+ if ((cur->n > 1) && (new->n == 2)) {
+ fscaler_flags |= FSCALER_TURBO_OFF;
+ }
+ /*

+                        * Else if the current operating point's N is
+                        * not-turbo and the new N is the desired
+                        * destination N, then set TURBO_ON
+                        */
+                       else if ((cur->n == 2) && (new->n == current_n)) {
+                               /*
+                                * Desired N must be what is current
+                                * set in the CCCR/CCSR
+                                */

+ fscaler_flags |= FSCALER_TURBO_ON;
+ }
+ /* Else, fall through to regular FCS */
+ }
+ if (!(fscaler_flags & FSCALER_TURBO)) {
+ /* It this is not a Turbo bit only change, it
+ must be a regular FCS
+ */
+ fscaler_flags |= FSCALER_CPUFREQ;
+ }
+ loops_per_jiffy = new->lpj;
+ }
+
+ if (new->half_turbo != cur->half_turbo) {
+ loops_per_jiffy = new->lpj;
+
+ if (new->half_turbo)
+ fscaler_flags |= FSCALER_HALFTURBO_ON;
+ else
+ fscaler_flags |= FSCALER_HALFTURBO_OFF;
+ }
+
+ if (new->regs.voltage != cur->regs.voltage)
+ fscaler_flags |= FSCALER_VOLTAGE;
+
+}
+
+static int pxa27x_transition(struct oppoint *cur, struct oppoint *new) +{

+       int rc = 0;
+       unsigned target_v;
+       struct md_opt *md_cur, *md_new;

+
+ pr_debug("%s: %s => %s\n", __FUNCTION__, cur->name, new->name);
+
+ md_cur = (struct md_opt *)cur->md_data;
+ md_new = (struct md_opt *)new->md_data;
+
+ /* fully define the new opt, if necessary, based on values
+ from the current opt
+ */
+ mainstone_fully_define_opt(cur, new);
+ target_v = md_new->v;
+
+ /* In accordance with Yellow Book section 3.7.6.3, "Coupling

+          Voltage Change with Frequency Change", always set the
+          voltage first (setting the FVC bit in the PCFR) and then do
+          the frequency change

+ */
+ rc = mainstone_fscale(cur, new);
+ if (rc == 0)
+ current_state = new;
+
+ udelay(new->latency);
+
+ return rc;
+}
+
+static int mainstone_fscale(struct oppoint *c, struct oppoint *n) +{
+ struct md_opt *cur = (struct md_opt *)c->md_data;
+ struct md_opt *new = (struct md_opt *)n->md_data;
+
+ compute_fscaler_flags(cur, new);
+
+ mainstone_fscaler(&new->regs);
+
+}
+
+void mainstone_fully_define_opt(struct oppoint *c, struct oppoint *n) +{
+ struct md_opt *cur = (struct md_opt *)c->md_data;
+ struct md_opt *new = (struct md_opt *)n->md_data;
+
+ if (new->v == -1)
+ new->v = cur->v;
+ if (new->l == -1)
+ new->l = cur->l;
+ if (new->n == -1)
+ new->n = cur->n;
+ if (new->b == -1)
+ new->b = cur->b;
+ if (new->half_turbo == -1)
+ new->half_turbo = cur->half_turbo;
+ if (new->cccra == -1)
+ new->cccra = cur->cccra;
+ if (new->cpll_enabled == -1)
+ new->cpll_enabled = cur->cpll_enabled;
+ if (new->ppll_enabled == -1)
+ new->ppll_enabled = cur->ppll_enabled;
+ if (new->sleep_mode == -1)
+ new->sleep_mode = cur->sleep_mode;
+
+#ifdef CONFIG_BULVERDE_B0
+ /* for "B0"-revision PLLs have the same value */
+ new->ppll_enabled = new->cpll_enabled;
+#endif
+ /* PXA27x manual ("Yellow book") 3.5.5 (Table 3-7) states that

+        * CPLL-"On" and PPLL-"Off"
+        * configuration is forbidden (all others seem to be OK for "B0")
+        * for "C0" boards we suppose that this configuration is also enabled.
+        * PXA27x manual ("Yellow book") also states at 3.5.7.1 (page 3-25)
+        * that "CCCR[PPDIS] and CCCR[CPDIS] must always be identical and
+        * changed together". "If PLLs are to be turned off using xPDIS then
+        * set xPDIS before frequency change and clear xPDIS after frequency
+        * change"
+        */

+
+ if (new->n > 2) {
+ new->turbo = 1;
+ /* turbo mode: 13K * L * (N/2)
+ Shift at the end to divide N by 2 for Turbo mode or
+ by 4 for Half-Turbo mode )
+ */
+ new->cpu = (13000 * new->l * new->n) >>
+ ((new->half_turbo == 1) ? 2 : 1);
+ } else {

+               new->turbo = 0;
+               /* run mode */
+               new->cpu = 13000 * new->l;
+       }
+       /* lcd freq is derived from L */
+       calculate_lcd_freq(new);
+       calculate_reg_values(new);
+       /* We want to keep a baseline loops_per_jiffy/cpu-freq ratio
+          to work off of for future calculations, especially when
+          emerging from sleep when there is no current cpu frequency
+          to calculate from (because cpu-freq of 0 means sleep).

+ */
+ if (!saved_loops_per_jiffy) {
+ saved_loops_per_jiffy = loops_per_jiffy;
+ saved_cpu_freq = cur->cpu;
+ }
+ if (!saved_cpu_freq) {
+ saved_cpu_freq = c->frequency;
+ }
+ /*
+ * a dedicated method for updating jiffies when frequency is changed
+ */
+ if (new->cpu) {
+ /* Normal change (not sleep), just compute. Always use
+ the "baseline" lpj and freq */
+ new->lpj = oppoint_compute_lpj(saved_loops_per_jiffy,
+ saved_cpu_freq, new->cpu);
+ } else {
+ /* If sleeping, keep the old LPJ */
+ new->lpj = loops_per_jiffy;
+ }
+}
+
+static void xpll_on(struct oppoint_regs *regs, int fscaler_flags) +{

+       int tmp_cccr, tmp_ccsr;
+       int new_cpllon=0, new_pllon=0, cur_cpllon=0;
+       int  cur_pllon=0, start_cpll=0, start_pll=0;

+
+ tmp_ccsr = CCSR;
+
+ if ((regs->cccr & CCCR_CPDIS_BIT_ON) == 0)
+ new_cpllon=1;
+ if ((regs->cccr & CCCR_PPDIS_BIT_ON) == 0)
+ new_pllon=1;
+ if (((tmp_ccsr >> 31) & 0x1) == 0)
+ cur_cpllon=1;
+ if (((tmp_ccsr >> 30) & 0x1) == 0)
+ cur_pllon=1;
+
+ if ((new_cpllon == 1) && (cur_cpllon == 0)) {
+ start_cpll=1;
+ }
+ if ((new_pllon == 1) && (cur_pllon == 0)) {
+ start_pll=1;
+ }
+
+ if (!(fscaler_flags & FSCALER_XPLLON)) {
+ return;
+ }
+ if ((start_cpll == 0) && (start_pll == 0)) {
+ return;
+ }
+ /* NOTE: the Yellow Book says that exiting 13M mode requires a

+          PLL relock, which takes at least 120uS, so the book suggests
+          the OS could use a timer to keep busy until it is time to
+          check the CCSR bits which must happen before changing the
+          frequency back.

+
+ For now, we'll just loop.
+ */
+
+ /* From Yellow Book, page 3-31, section 3.5.7.5 13M Mode
+
+ Exiting 13M Mode:
+

+          1. Remain in 13M mode, but early enable the PLL via
+          CCCR[CPDIS, PPDIS]=11, and CCCR[PLL_EARLY_EN]=1. Doing
+          so will allow the PLL to be started early.

+
+ 2. Read CCCR and compare to make sure that the data was
+ correctly written.
+

+          3. Check to see if CCS[CPLOCK] and CCSR[PPLOCK] bits are
+          both set. Once these bits are both high, the PLLs are
+          locked and you may move on.

+
+ 4. Note that the CPU is still in 13M mode, but the PLLs are
+ started.
+

+          5. Exit from 13M mode by writing CCCR[CPDIS, PPDIS]=00, but
+          maintain CCCR[PLL_EARLY_EN]=1. This bit will be cleared
+          by the imminent frequency change.
+        */
+
+       /* Step 1 */
+       tmp_cccr = CCCR;
+       if (start_cpll)

+ tmp_cccr |= CCCR_CPDIS_BIT_ON;
+ if (start_pll)
+ tmp_cccr |= CCCR_PPDIS_BIT_ON;
+ tmp_cccr |= CCCR_PLL_EARLY_EN_BIT_ON;
+
+ CCCR = tmp_cccr;
+
+ /* Step 2 */
+ tmp_cccr = CCCR;
+
+ if ((tmp_cccr & CCCR_PLL_EARLY_EN_BIT_ON) != CCCR_PLL_EARLY_EN_BIT_ON) {
+ printk(KERN_WARNING
+ "DPM: Warning: PLL_EARLY_EN is NOT on\n");
+ }
+ if ((start_cpll==1) &&
+ ((tmp_cccr & CCCR_CPDIS_BIT_ON) != CCCR_CPDIS_BIT_ON)) {
+ printk(KERN_WARNING
+ "DPM: Warning: CPDIS is NOT on\n");
+ }
+ if ((start_pll==1) &&
+ (tmp_cccr & CCCR_PPDIS_BIT_ON) != CCCR_PPDIS_BIT_ON) {
+ printk(KERN_WARNING
+ "DPM: Warning: PPDIS is NOT on\n");
+ }
+
+ /* Step 3 */
+ {
+ /* Note: the point of this is to "wait" for the lock

+                  bits to be set; the Yellow Book says this may take
+                  a while, but observation indicates that it is
+                  instantaneous

+ */
+
+ long volatile int i = 0;
+

+               int cpll_complete=1;
+               int pll_complete=1;
+               if (start_cpll==1)

+ cpll_complete=0;
+ if (start_pll==1)
+ pll_complete=0;
+
+ /*loop arbitrary big value to prevent looping forever */
+ for (i = 0; i < 999999; i++) {
+ tmp_ccsr = CCSR;
+
+ if ((tmp_ccsr & CCSR_CPLL_LOCKED) == CCSR_CPLL_LOCKED) {
+ /*CPLL locked*/
+ cpll_complete=1;
+ }
+ if ((tmp_ccsr & CCSR_PPLL_LOCKED) == CCSR_PPLL_LOCKED) {
+ /*PPLL locked*/
+ pll_complete=1;
+ }
+ if ((cpll_complete == 1) && (pll_complete == 1)) {
+ break;
+ }
+ }
+ }
+
+ /* Step 4: NOP */
+
+ /* Step 5
+ Clear the PLL disable bits - do NOT do it here.
+ */
+

+       /* But leave EARLY_EN on; it will be cleared by the frequency change */
+       regs->cccr |= CCCR_PLL_EARLY_EN_BIT_ON;
+       /* Do not set it now
+          Step 6: Now go continue on with frequency change
+          We do this step later as if voltage is too low,
+          we must ensure that it rised up  before entereng to higher
+          freq mode or simultaniously

+ */
+}
+
+static void mainstone_fscaler(struct oppoint_regs *regs) +{
+ unsigned int cccr, clkcfg = 0;
+ unsigned long s;
+
+ /* If no flags are set, don't waste time here, just return */
+ if (fscaler_flags == FSCALER_NOP)
+ return;
+
+ if (!(fscaler_flags & FSCALER_ANY_SLEEPMODE))
+ local_irq_save(s);
+
+ /* If exiting 13M mode (turn on PLL(s) ), do some extra work

+          before changing the CPU frequency or voltage.
+          We may turn on a combination of PLLs supported by hardware
+          only. Otherwise xpll_on(...) hang the system.

+ */
+ if (fscaler_flags & FSCALER_XPLLON)
+ xpll_on(regs, fscaler_flags);
+
+ /* if not sleeping, and have a voltage change
+ note that SLEEPMODE will handle voltage itself
+ */
+ if (((fscaler_flags & FSCALER_ANY_SLEEPMODE) == 0) &&
+ (fscaler_flags & FSCALER_VOLTAGE)) {
+ if (fscaler_flags & FSCALER_CPUFREQ) {
+ /* coupled voltage & freq change */
+ mainstone_scale_voltage_coupled(regs);
+ } else {
+ /* Scale CPU voltage un-coupled with freq */
+ mainstone_scale_voltage(regs);
+ }
+ }
+
+ if (fscaler_flags & FSCALER_CPUFREQ) /* Scale CPU freq */
+ mainstone_scale_cpufreq(regs);
+
+ if ((fscaler_flags & FSCALER_VOLTAGE) &&
+ (fscaler_flags & FSCALER_CPUFREQ))
+ PCFR &= ~PCFR_FVC;
+
+ if (fscaler_flags & FSCALER_TURBO) {
+
+ clkcfg = mainstone_read_clkcfg();
+
+ /* Section 3.5.7 of the Yellow Book says that the F
+ bit will be left on after a FCS, so we need to
+ explicitly clear it. But do not change the B bit
+ */
+ clkcfg &= ~(CLKCFG_F_BIT);
+
+ if (fscaler_flags & FSCALER_TURBO_ON) {
+ clkcfg = clkcfg | (CLKCFG_T_BIT);
+ } else {
+ clkcfg = clkcfg & ~(CLKCFG_T_BIT);
+ }
+
+ /* enable */
+ mainstone_set_freq(clkcfg);
+ }
+
+ if ((fscaler_flags & FSCALER_HALFTURBO_ON) ||
+ (fscaler_flags & FSCALER_HALFTURBO_OFF)) {
+ if ((fscaler_flags & FSCALER_CPUFREQ) ||
+ (fscaler_flags & FSCALER_VOLTAGE)) {
+
+ /*
+ From the Yellow Book, p 3-106:
+