Power Mangement Interfaces

I am now turning my attention to handling wakeup events - in particular, events that we can set at run-time. My thoughts on the matter are detailed here:

http://wiki.laptop.org/go/Power_Management_Interface

Quoted 5 lines

I am now turning my attention to handling wakeup events - in particular, events that we can set at run-time. My thoughts on the matter are detailed here:

http://wiki.laptop.org/go/Power_Management_Interface

Quoted 1 line · Johannes Berg

Quoted 1 line

I am now turning my attention to handling wakeup events - in

particular,

Quoted 4 lines

Quoted 4 lines

events that we can set at run-time. My thoughts on the matter are detailed here:

http://wiki.laptop.org/go/Power_Management_Interface

Hm, another thing. The RTC alarm wakeup event, wouldn't that require programming the time too? And if so, should that be done through the power interface to it?

Quoted 1 line

Quoted 1 line

http://wiki.laptop.org/go/Power_Management_Interface

Hm, another thing. The RTC alarm wakeup event, wouldn't that require programming the time too? And if so, should that be done through the power interface to it?

Hm, another thing. The RTC alarm wakeup event, wouldn't that require programming the time too? And if so, should that be done through the power interface to it?

On Fri, 2007-03-30 at 17:57 -0600, Jordan Crouse wrote:

I think the sources file isn't really useful, a
grep -l 1 /sys/power/wakeup/*
should do.

About different possible states: I think each of those can have different possible wakeup sources, ACPI can afaik go to S4 and still be able to configure the wakeup sources. So I suppose this really needs to be something like /sys/power/wakeup/<state>/<event> then where <state> is one of (currently) "mem", "disk" and "standby". And then change the interface of pm_register_wakeup_source to include the state.

Also, I'm not sure I like the interface with the name of the wakeup event set by the platform driver. That will probably lead to inconsistencies, a centrally maintained file with items could be nicer.

About test mode: there is such a thing for suspend to disk. In fact, if you just want to test driver suspend you could use that. See the /sys/power/disk file.

Quoted 3 lines

I think the sources file isn't really useful, a
grep -l 1 /sys/power/wakeup/*
should do.

I was just trying to be friendly to folks who may be used to the ACPI method.

On Fri, 2007-03-30 at 17:57 -0600, Jordan Crouse wrote:

About different possible states: I think each of those can have different possible wakeup sources, ACPI can afaik go to S4 and still be able to configure the wakeup sources. So I suppose this really needs to be something like /sys/power/wakeup/<state>/<event> then where <state> is one of (currently) "mem", "disk" and "standby".

And then change the
interface of pm_register_wakeup_source to include the state.

To the contrary, it seems enough to just expose a boolean flag saying whether a given device should try to be a wakeup event source. If the hardware won't allow that in the target system state, so be it; there's no way software can change that.

How would I go about representing the lid as a device? It's an honest question, I'd actually like to do and replace the /proc/pmu/options file it but don't know where I should put it etc.

I'm not sure I understand the question. ACPI has a special "acpi_device" node (not backed by any "real" device, PNPACPI etc) with a driver that hooks up to the input subsystem. Same is true for a couple other fixed function buttons. I'd expect non-ACPI platforms would do something very similar, except using platform_device and looking less cryptic.

Quoted 5 lines · David Brownell

I'm not sure I understand the question. ACPI has a special "acpi_device" node (not backed by any "real" device, PNPACPI etc) with a driver that hooks up to the input subsystem. Same is true for a couple other fixed function buttons. I'd expect non-ACPI platforms would do something very similar, except using platform_device and looking less cryptic.

No, the thing is more: how would we represent the lid as a device in sysfs?

Currently, our PMU only has a sysdev because it doesn't need more. Would you suggest doing a PMU-bus and than hanging "lid" off of that?

Quoted 2 lines

No, the thing is more: how would we represent the lid as a device in sysfs?

As a platform_device.

Quoted 4 lines · Johannes Berg

How would I go about representing the lid as a device? It's an honest question, I'd actually like to do and replace the /proc/pmu/options file it but don't know where I should put it etc.

I'm not sure I understand the question. ACPI has a special "acpi_device" node (not backed by any "real" device, PNPACPI etc) with a driver that hooks up to the input subsystem. Same is true for a couple other fixed function buttons. I'd expect non-ACPI platforms would do something very similar, except using platform_device and looking less cryptic.

As for hooking things up to the driver model wakeup event stuff, ACPI doesn't do that yet. I've got some patches in the works;

Hey all -

I'm happy to report that the OLPC power management effort is proceeding nicely. We have suspend to RAM functional, and the system is resuming back to the framebuffer console.

We have the usual blips (USB), but
those will be resolved in the fullness of time.

I am now turning my attention to handling wakeup events - in particular, events that we can set at run-time. My thoughts on the matter are detailed here:

http://wiki.laptop.org/go/Power_Management_Interface

I use the ACPI wakeup infrastructure as an example because a) it exists, and b) it works.

However, it doesn't share nicely, and it is a /proc
file, so I sat down and thought of a more /sysfs friendly and generic model. I did keep APCI in mind, figuring that if a generic framework was designed well enough, they could slowly transition over as well (which is why the ACPI list is CCed).

USB once worked fine (I spent more than enough time testing it in all kinds of configs) but usbcore has changed a lot since then ... I hear that some of the root hub stuff got broken.

Quoted 6 lines

I am now turning my attention to handling wakeup events - in particular, events that we can set at run-time. My thoughts on the matter are detailed here:

http://wiki.laptop.org/go/Power_Management_Interface

You seem to be completely ignoring the /sys/devices/..../power/wakeup event attributes. Why?

Quoted 2 lines · David Brownell

You seem to be completely ignoring the /sys/devices/..../power/wakeup event attributes. Why?

Complete and total ignorance that they even existed. I guess I should
pay more attention to linux-pm then I do.. :(. My impression is that
this infrastructure allows the devices to configure themselves to wake up the system,

but it doesn't seem to address enabling the system to
allow those events to wake it. It is the latter that I am concerned about.

The Geode (and most other platforms, I presume) allows us to configure which wakeup events are actually allowed to wake the system. Currently, we are permanently enabling the power button as a wakeup source, but I want to eventually be able to pick and chose from any of our main wakeup sources:

* Power button
* RTC Alarm
* LID event (GPIO26)
* wake event from the EC (GPIO27)

Of the four, the only device that can actually be configured to do anything is the RTC alarm, and for that we would use the /sys/devices/../power/wakeup infrastructure, but we would still need something in place to allow the software to tell the hardware to enable the RTC event to wake the system.

I suppose we could just globally enable all sources, but that seems silly, seeing as though we do have very fine tuned control over the wake sources. I'm sure other platforms have this concern as well - what do they do?

The other part is the platform code. On embedded hardware that's usually just calling device_init_wakeup() before registering the platform devices, and supporting enable_irq_wake(). On x86 that gets messy; ACPI is there, and PCI initialization can't set device_init_wakeup() because of conflicts with how PPC initializes PCI.

As in "echo enabled > /sys/devices/.../power/wakeup" (to get the default), or "echo disabled > ..." to disable it. For PCs, with ACPI, that involves two patches I just posted to the RTC list (and CC'd Len on), and replacing the ancient/legacy/ACPI-only /proc/acpi/alarm interface.

Quoted 5 lines · David Brownell

The other part is the platform code. On embedded hardware that's usually just calling device_init_wakeup() before registering the platform devices, and supporting enable_irq_wake(). On x86 that gets messy; ACPI is there, and PCI initialization can't set device_init_wakeup() because of conflicts with how PPC initializes PCI.

enable_irq_wake() looks like it integrates very well with the AT91, since it seems that there is a 1:1 mapping of interrupts to wakeup sources.

The
story is much more muddled for x86 - there is no direct mapping of interrupts to wakeup sources, and the PIC has nothing to do with handling wake events.

On the Geode, we have 17 possible wake sources, all of which are ORed

together to form the SCI interrupt. Some of the events, like the RTC, UART, and USB wakeup sources are associated with devices that have their own interrupts, but there is no correlation between the RTC interrupt, for example, and enabling it as a wakeup source. It seems overly complex to try to map these to individual interrupts, and certainly seems wasteful to get the PIC involved in something it just doesn't understand.

Quoted 4 lines

As in "echo enabled > /sys/devices/.../power/wakeup" (to get the default), or "echo disabled > ..." to disable it. For PCs, with ACPI, that involves two patches I just posted to the RTC list (and CC'd Len on), and replacing the ancient/legacy/ACPI-only /proc/acpi/alarm interface.

The link is here for the interested:

http://groups.google.com/group/rtc-linux/browse_thread/thread/ae7fe3436e01e7fa

This looks very good, and is pretty close to what I was proposing. Replace the acpi_*_event() calls with a generic pm_ infrastructure, and

add hooks to the pm_ops for each individual PM system to handle the wakeups in whatever way they see fit, and we're there. It would be slightly more complex for AT91 and friends to match this (since they have the advantage of a 1:1 mapping right now), but in the long run, I think everybody would benefit.

The only other issue then, is how we could define and manage wakeup events for events that aren't associated with specific devices, like power button and lid events. We'll need some way to control those somewhere in sysfs - if not in /sys/power/wakeup like I had proposed, then somewhere under the platform or system hierarchy .

Quoted 5 lines · David Brownell

The other part is the platform code. On embedded hardware that's usually just calling device_init_wakeup() before registering the platform devices, and supporting enable_irq_wake(). On x86 that gets messy; ACPI is there, and PCI initialization can't set device_init_wakeup() because of conflicts with how PPC initializes PCI.

enable_irq_wake() looks like it integrates very well with the AT91, since it seems that there is a 1:1 mapping of interrupts to wakeup sources.

The
story is much more muddled for x86 - there is no direct mapping of interrupts to wakeup sources, and the PIC has nothing to do with handling wake events.

On the Geode, we have 17 possible wake sources, all of which are ORed

together to form the SCI interrupt. Some of the events, like the RTC, UART, and USB wakeup sources are associated with devices that have their own interrupts, but there is no correlation between the RTC interrupt, for example, and enabling it as a wakeup source. It seems overly complex to try to map these to individual interrupts, and certainly seems wasteful to get the PIC involved in something it just doesn't understand.

Quoted 4 lines

As in "echo enabled > /sys/devices/.../power/wakeup" (to get the default), or "echo disabled > ..." to disable it. For PCs, with ACPI, that involves two patches I just posted to the RTC list (and CC'd Len on), and replacing the ancient/legacy/ACPI-only /proc/acpi/alarm interface.

The link is here for the interested:

http://groups.google.com/group/rtc-linux/browse_thread/thread/ae7fe3436e01e7fa

This looks very good, and is pretty close to what I was proposing. Replace the acpi_*_event() calls with a generic pm_ infrastructure, and

add hooks to the pm_ops for each individual PM system to handle the wakeups in whatever way they see fit, and we're there. It would be slightly more complex for AT91 and friends to match this (since they have the advantage of a 1:1 mapping right now), but in the long run, I think everybody would benefit.

The only other issue then, is how we could define and manage wakeup events for events that aren't associated with specific devices, like power button and lid events. We'll need some way to control those somewhere in sysfs - if not in /sys/power/wakeup like I had proposed, then somewhere under the platform or system hierarchy .

Quoted 3 lines

The
story is much more muddled for x86 - there is no direct mapping of interrupts to wakeup sources, and the PIC has nothing to do with handling wake events.

Right. Plus there often seems to be an assumption that some "embedded (micro)controller" (EC) manages system bringup/wakeup/runtime.

Quoted 2 lines

On the Geode, we have 17 possible wake sources, all of which are ORed

What are those? You mentioned four before. (RTC alarm, two GPIOs, and the power button.) I'd guess various PCI devices can also issue wakeups. (And if USB is one of them, up to 126 USB devices hooked up to each USB host controller!)

Quoted 2 lines

This looks very good, and is pretty close to what I was proposing. Replace the acpi_*_event() calls with a generic pm_ infrastructure, and

Inside ACPI ... why?

Quoted 5 lines

add hooks to the pm_ops for each individual PM system to handle the wakeups in whatever way they see fit, and we're there. It would be slightly more complex for AT91 and friends to match this (since they have the advantage of a 1:1 mapping right now), but in the long run, I think everybody would benefit.

I guess I don't see why this isn't already sufficiently generic ...

As a rule, all those embedded drivers are platform-specific and have no need for more than a few enable_irq_wake() calls, in addition to whatever controller setup and clock management they do. Since they can't be very portable, they don't need to generalize such stuff.

And for PCI based things, pci_enable_wake() encapsulates everything that needs doing. Not that ACPI actually *does* anything yet! But the stuff that writing /proc/acpi/wakeup enables should happen in that routine ... and eventually PCI runtime wake events should work too. (So: no drivers would ever call ACPI directly, and they already have the generic call that ACPI should hook.)

That seems to cover most drivers in Linux, without a need for any new generic PM infrastructure. Did I overlook something important?

Quoted 5 lines

The only other issue then, is how we could define and manage wakeup events for events that aren't associated with specific devices, like power button and lid events. We'll need some way to control those somewhere in sysfs - if not in /sys/power/wakeup like I had proposed, then somewhere under the platform or system hierarchy .

I see /sys/devices/acpi_system:00/button_power:00 on this system; and /sys/devices/acpi_system:00/device:00/PNP0C0D:00 has path \_SB_.LID_ ... such device nodes already exist, even though they're not really hooked up to anything much. Notably, their "wakeup" state is not initialized.

And while it seems that the three USB controllers on this system show up as /sys/devices/acpi_system:00/device:00/PNP0A03:00/device:{01,02,03} I have no idea which one is /sys/devices/pci0000:00/0000:00:02.2 versus 0000:00:02.1 or 0000:00:02.0 ... I know that USB0 is device:01 and so on (by reading "path"), but associating one with a PCI device seems to involve pure guesswork.

A few of your comments made me wonder if I haven't been clear enough - despite this being an x86 platform, we are not using ACPI for power management, for a variety of reasons, including the fact that we have our own open source firmware and a very specific hardware platform.

So, for better or worse, we are implementing a new power management manger. Here is the code:

http://dev.laptop.org/git.do?p=olpc-2.6;a=blob;h=e6e19ac32a881be962db7190f2cb460e9f60a9da;hb=powermgmt;f=arch/i386/kernel/olpc-pm.c

Quoted 6 lines

Quoted 1 line

On the Geode, we have 17 possible wake sources, all of which are ORed

What are those? You mentioned four before. (RTC alarm, two GPIOs, and the power button.) I'd guess various PCI devices can also issue wakeups. (And if USB is one of them, up to 126 USB devices hooked up to each USB host controller!)

The silicon can handle wakeups from RTC, a sleep button, a power button, 8 GPIO groups, the USB controller (covering any and all USB events), UART1, UART2, SMbus and the PIC.

OLPC really only cares about 4 of those, because our only sleep state is ~S3, and the PIC, smbus, UARTs and USB are off in S3. We also only have the power button, and all the rest of the wakeup sources come from the GPIOs.

Quoted 4 lines

Quoted 2 lines

This looks very good, and is pretty close to what I was proposing. Replace the acpi_*_event() calls with a generic pm_ infrastructure, and

Inside ACPI ... why?

I'm not saying inside ACPI - I'm saying at the driver level.

Something
like this (where name could be a string like ACPI does, but it would be more portable if it was a #define'ed value).

pm_set_event(name) {
   pm_ops->set_event(name, 1);
}

pm_clear_event(name) {
   pm_ops->set_event(name, state);
}

And then each subsystem that defines a struct pm_ops adds a set_event() that does the right thing.

In OLPC's case we would do something like this:

olpc_pm_set_event(name, state) {
	switch(name) {
		case PM_WAKEUP_RTC:
			pm1_wakeup |= (1 << 9);
			break;

			...
	}
}

And so forth. Like the AT91, we enable the wakeup events prior to suspending. I'm willing to bet that other PM methods like ACPI could pick this up pretty quickly too.

Quoted 6 lines

Quoted 5 lines

add hooks to the pm_ops for each individual PM system to handle the wakeups in whatever way they see fit, and we're there. It would be slightly more complex for AT91 and friends to match this (since they have the advantage of a 1:1 mapping right now), but in the long run, I think everybody would benefit.

Quoted 6 lines

I guess I don't see why this isn't already sufficiently generic ...

As a rule, all those embedded drivers are platform-specific and have no need for more than a few enable_irq_wake() calls, in addition to whatever controller setup and clock management they do. Since they can't be very portable, they don't need to generalize such stuff.

enable_irq_wake() just doesn't work for x86, unfortunately.

It seems
to me to be more logical to move the wakeup intelligence to the PM subsystem, and then let that code distribute it to where it is needed. In the case of x86, the logic would stay in the PM method, for other processors, it might involve a call to the interrupt mapper.

Quoted 7 lines

And for PCI based things, pci_enable_wake() encapsulates everything that needs doing. Not that ACPI actually *does* anything yet! But the stuff that writing /proc/acpi/wakeup enables should happen in that routine ... and eventually PCI runtime wake events should work too. (So: no drivers would ever call ACPI directly, and they already have the generic call that ACPI should hook.)

Quoted 2 lines

That seems to cover most drivers in Linux, without a need for any new generic PM infrastructure. Did I overlook something important?

No, I don't think so - we're very close on agreeing here. I just don't know if enable_irq_wake() is the best way to provide the generic call.

Moving
it into the PM infrastructure seems the best way to handle everybody equally, regardless of the relative intelligence or stupidity of their hardware implementation.

Quoted 11 lines

Quoted 5 lines

The only other issue then, is how we could define and manage wakeup events for events that aren't associated with specific devices, like power button and lid events. We'll need some way to control those somewhere in sysfs - if not in /sys/power/wakeup like I had proposed, then somewhere under the platform or system hierarchy .

I see /sys/devices/acpi_system:00/button_power:00 on this system; and /sys/devices/acpi_system:00/device:00/PNP0C0D:00 has path \_SB_.LID_ ... such device nodes already exist, even though they're not really hooked up to anything much. Notably, their "wakeup" state is not initialized.

Quoted 6 lines

And while it seems that the three USB controllers on this system show up as /sys/devices/acpi_system:00/device:00/PNP0A03:00/device:{01,02,03} I have no idea which one is /sys/devices/pci0000:00/0000:00:02.2 versus 0000:00:02.1 or 0000:00:02.0 ... I know that USB0 is device:01 and so on (by reading "path"), but associating one with a PCI device seems to involve pure guesswork.

I guess we'll probably have to do something similar for our OLPC PM code
- but thats the sort of platform specific fragmentation thing I was trying to avoid.

A few of your comments made me wonder if I haven't been clear enough - despite this being an x86 platform, we are not using ACPI for power management, for a variety of reasons, including the fact that we have our own open source firmware and a very specific hardware platform.

So, for better or worse, we are implementing a new power management manger. Here is the code:

http://dev.laptop.org/git.do?p=olpc-2.6;a=blob;h=e6e19ac32a881be962db7190f2cb460e9f60a9da;hb=powermgmt;f=arch/i386/kernel/olpc-pm.c

Quoted 6 lines

Quoted 1 line

On the Geode, we have 17 possible wake sources, all of which are ORed

What are those? You mentioned four before. (RTC alarm, two GPIOs, and the power button.) I'd guess various PCI devices can also issue wakeups. (And if USB is one of them, up to 126 USB devices hooked up to each USB host controller!)

The silicon can handle wakeups from RTC, a sleep button, a power button, 8 GPIO groups, the USB controller (covering any and all USB events), UART1, UART2, SMbus and the PIC.

OLPC really only cares about 4 of those, because our only sleep state is ~S3, and the PIC, smbus, UARTs and USB are off in S3. We also only have the power button, and all the rest of the wakeup sources come from the GPIOs.

Quoted 4 lines

Quoted 2 lines

This looks very good, and is pretty close to what I was proposing. Replace the acpi_*_event() calls with a generic pm_ infrastructure, and

Inside ACPI ... why?

I'm not saying inside ACPI - I'm saying at the driver level.

Something
like this (where name could be a string like ACPI does, but it would be more portable if it was a #define'ed value).

pm_set_event(name) {
   pm_ops->set_event(name, 1);
}

pm_clear_event(name) {
   pm_ops->set_event(name, state);
}

And then each subsystem that defines a struct pm_ops adds a set_event() that does the right thing.

In OLPC's case we would do something like this:

olpc_pm_set_event(name, state) {
	switch(name) {
		case PM_WAKEUP_RTC:
			pm1_wakeup |= (1 << 9);
			break;

			...
	}
}

And so forth. Like the AT91, we enable the wakeup events prior to suspending. I'm willing to bet that other PM methods like ACPI could pick this up pretty quickly too.

Quoted 6 lines

Quoted 5 lines

add hooks to the pm_ops for each individual PM system to handle the wakeups in whatever way they see fit, and we're there. It would be slightly more complex for AT91 and friends to match this (since they have the advantage of a 1:1 mapping right now), but in the long run, I think everybody would benefit.

Quoted 6 lines

I guess I don't see why this isn't already sufficiently generic ...

As a rule, all those embedded drivers are platform-specific and have no need for more than a few enable_irq_wake() calls, in addition to whatever controller setup and clock management they do. Since they can't be very portable, they don't need to generalize such stuff.

enable_irq_wake() just doesn't work for x86, unfortunately.

It seems
to me to be more logical to move the wakeup intelligence to the PM subsystem, and then let that code distribute it to where it is needed. In the case of x86, the logic would stay in the PM method, for other processors, it might involve a call to the interrupt mapper.

Quoted 7 lines

And for PCI based things, pci_enable_wake() encapsulates everything that needs doing. Not that ACPI actually *does* anything yet! But the stuff that writing /proc/acpi/wakeup enables should happen in that routine ... and eventually PCI runtime wake events should work too. (So: no drivers would ever call ACPI directly, and they already have the generic call that ACPI should hook.)

Quoted 2 lines

That seems to cover most drivers in Linux, without a need for any new generic PM infrastructure. Did I overlook something important?

No, I don't think so - we're very close on agreeing here. I just don't know if enable_irq_wake() is the best way to provide the generic call.

Moving
it into the PM infrastructure seems the best way to handle everybody equally, regardless of the relative intelligence or stupidity of their hardware implementation.

Quoted 11 lines

Quoted 5 lines

The only other issue then, is how we could define and manage wakeup events for events that aren't associated with specific devices, like power button and lid events. We'll need some way to control those somewhere in sysfs - if not in /sys/power/wakeup like I had proposed, then somewhere under the platform or system hierarchy .

I see /sys/devices/acpi_system:00/button_power:00 on this system; and /sys/devices/acpi_system:00/device:00/PNP0C0D:00 has path \_SB_.LID_ ... such device nodes already exist, even though they're not really hooked up to anything much. Notably, their "wakeup" state is not initialized.

Quoted 6 lines

And while it seems that the three USB controllers on this system show up as /sys/devices/acpi_system:00/device:00/PNP0A03:00/device:{01,02,03} I have no idea which one is /sys/devices/pci0000:00/0000:00:02.2 versus 0000:00:02.1 or 0000:00:02.0 ... I know that USB0 is device:01 and so on (by reading "path"), but associating one with a PCI device seems to involve pure guesswork.

I guess we'll probably have to do something similar for our OLPC PM code
- but thats the sort of platform specific fragmentation thing I was trying to avoid.

Quoted 5 lines

So, for better or worse, we are implementing a new power management manger. Here is the code:

http://dev.laptop.org/git.do?p=olpc-2.6;a=blob;h=e6e19ac32a881be962db7190f2cb460e9f60a9da;hb=powermgmt;f=arch/i386/kernel/olpc-pm.c

Does that work yet with CONFIG_NO_HZ, so that the system idle loop enters some "almost everyting is turned off" low power state? Or would that be something other than a "PM Manager"?

Quoted 7 lines

Quoted 6 lines

Quoted 1 line

On the Geode, we have 17 possible wake sources, all of which are ORed

What are those? You mentioned four before. (RTC alarm, two GPIOs, and the power button.) I'd guess various PCI devices can also issue wakeups. (And if USB is one of them, up to 126 USB devices hooked up to each USB host controller!)

Quoted 8 lines

The silicon can handle wakeups from RTC, a sleep button, a power button, 8 GPIO groups, the USB controller (covering any and all USB events), UART1, UART2, SMbus and the PIC.

OLPC really only cares about 4 of those, because our only sleep state is ~S3, and the PIC, smbus, UARTs and USB are off in S3. We also only have the power button, and all the rest of the wakeup sources come from the GPIOs.

So it's a slightly deeper "S3" than I've usually seen, but not by much.

And correct me if I'm wrong, but your RTC can work with "rtc-cmos", and the wake hooks I've posted (and which will be in the next MM release) sound like they'll work just fine (given OLPC-specific implementation, which is clearly not included in the code above).

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

This looks very good, and is pretty close to what I was proposing. Replace the acpi_*_event() calls with a generic pm_ infrastructure, and

Inside ACPI ... why?

I'm not saying inside ACPI - I'm saying at the driver level.

Whereas in that patch, the only ACPI calls were inside ACPI itself; and that driver only had a callback. FWIW that's the first time I've ever needed to have indirection on wakeup event handling ...

Quoted 3 lines

And so forth. Like the AT91, we enable the wakeup events prior to suspending. I'm willing to bet that other PM methods like ACPI could pick this up pretty quickly too.

With AT91 (and most ARMs) the wake events are normal IRQs, and the irq infrastructure already handles wake events cleanly. So I'd claim they don't _want_ to change something that's inexpensive, already deployed (training, code, testing, etc), and working.

So I hope you'll tolerate me if I view this as less of a "how do we get a generic solution" and more of a "how does _this_ x86-ish issue get solved cleanly".

Quoted 4 lines

Moving
it into the PM infrastructure seems the best way to handle everybody equally, regardless of the relative intelligence or stupidity of their hardware implementation.

I guess that's where we disagree: you're assuming "one call to rule them all", where I'm thinking that IRQ calls should manage IRQs, PCI calls should manage PME#, and so on.

Quoted 3 lines

I guess we'll probably have to do something similar for our OLPC PM code
- but thats the sort of platform specific fragmentation thing I was trying to avoid.

Since OLPC isn't using ACPI, it can be more like embedded Linux, and just Do The Right Thing ... create platform devices, etc. :)

My point above was that ACPI isn't yet integrating with the things it needs to be integrating with. If you're concerned about how to work with wakeup events in Linux, don't even bother looking at ACPI. Look at the embedded implementations; look at USB. AT91 is by far the cleanest and most complete (simplest hardware too!!), but some of the other systems (OMAP, PXA, SA1100, etc) also implement wakeup using board-specific code.

OLPC _could_ use that same "board-specific hacks" approach found in most embedded platforms. I'm glad you're thinking about how to avoid doing that.

Quoted 9 lines · David Brownell

Quoted 3 lines

And so forth. Like the AT91, we enable the wakeup events prior to suspending. I'm willing to bet that other PM methods like ACPI could pick this up pretty quickly too.

With AT91 (and most ARMs) the wake events are normal IRQs, and the irq infrastructure already handles wake events cleanly. So I'd claim they don't _want_ to change something that's inexpensive, already deployed (training, code, testing, etc), and working.

Quoted 3 lines

So I hope you'll tolerate me if I view this as less of a "how do we get a generic solution" and more of a "how does _this_ x86-ish issue get solved cleanly".

Thats fair. x86 does live in a different world then the other embedded chips. I think its reasonable for the x86 folks to get their story straight and then move on from there.

Quoted 5 lines

Quoted 4 lines

Moving
it into the PM infrastructure seems the best way to handle everybody equally, regardless of the relative intelligence or stupidity of their hardware implementation.

Quoted 3 lines

I guess that's where we disagree: you're assuming "one call to rule them all", where I'm thinking that IRQ calls should manage IRQs, PCI calls should manage PME#, and so on.

Thats fine - I am not sold on a "one call to rule them all", but I do want to avoid any #ifdef CONFIG_PM_ACPI #elsif CONFIG_OLPC_PM #else nonsense.

We are keenly aware that we're committing a minor sin by implementing yet another PM interface for x86.

But we believe that our heart is in the
right place, and we are dedicated to doing the right thing, and trying our hardest not to contribute further to fragmenting the PM subsystem with implementation specific code. In many cases we're looking to help bring the others into the fold, especially our fellow x86 implementations that suffer from the same issues that we do.

I appreciate your comments, and the rtc-cmos code will go to good use.

Quoted 9 lines · David Brownell

Quoted 3 lines

And so forth. Like the AT91, we enable the wakeup events prior to suspending. I'm willing to bet that other PM methods like ACPI could pick this up pretty quickly too.

With AT91 (and most ARMs) the wake events are normal IRQs, and the irq infrastructure already handles wake events cleanly. So I'd claim they don't _want_ to change something that's inexpensive, already deployed (training, code, testing, etc), and working.

Quoted 3 lines

So I hope you'll tolerate me if I view this as less of a "how do we get a generic solution" and more of a "how does _this_ x86-ish issue get solved cleanly".

Thats fair. x86 does live in a different world then the other embedded chips. I think its reasonable for the x86 folks to get their story straight and then move on from there.

Quoted 5 lines

Quoted 4 lines

Moving
it into the PM infrastructure seems the best way to handle everybody equally, regardless of the relative intelligence or stupidity of their hardware implementation.

Quoted 3 lines

I guess that's where we disagree: you're assuming "one call to rule them all", where I'm thinking that IRQ calls should manage IRQs, PCI calls should manage PME#, and so on.

Thats fine - I am not sold on a "one call to rule them all", but I do want to avoid any #ifdef CONFIG_PM_ACPI #elsif CONFIG_OLPC_PM #else nonsense.

We are keenly aware that we're committing a minor sin by implementing yet another PM interface for x86.

But we believe that our heart is in the
right place, and we are dedicated to doing the right thing, and trying our hardest not to contribute further to fragmenting the PM subsystem with implementation specific code. In many cases we're looking to help bring the others into the fold, especially our fellow x86 implementations that suffer from the same issues that we do.

I appreciate your comments, and the rtc-cmos code will go to good use.

Quoted 9 lines · David Brownell

Quoted 5 lines

So, for better or worse, we are implementing a new power management manger. Here is the code:

http://dev.laptop.org/git.do?p=olpc-2.6;a=blob;h=e6e19ac32a881be962db7190f2cb460e9f60a9da;hb=powermgmt;f=arch/i386/kernel/olpc-pm.c

Does that work yet with CONFIG_NO_HZ, so that the system idle loop enters some "almost everyting is turned off" low power state? Or would that be something other than a "PM Manager"?

I think eventually we'll get into entering deeper states in the idle loop. For now, we're just going into the classic C1 / hlt state, which is actually quite thrifty on the Geode, due to automatic hardware clock gating that happens throughout the chip(s).

Quoted 16 lines

Quoted 7 lines

Quoted 6 lines

Quoted 1 line

On the Geode, we have 17 possible wake sources, all of which are ORed

What are those? You mentioned four before. (RTC alarm, two GPIOs, and the power button.) I'd guess various PCI devices can also issue wakeups. (And if USB is one of them, up to 126 USB devices hooked up to each USB host controller!)

Quoted 8 lines

The silicon can handle wakeups from RTC, a sleep button, a power button, 8 GPIO groups, the USB controller (covering any and all USB events), UART1, UART2, SMbus and the PIC.

OLPC really only cares about 4 of those, because our only sleep state is ~S3, and the PIC, smbus, UARTs and USB are off in S3. We also only have the power button, and all the rest of the wakeup sources come from the GPIOs.

Quoted 6 lines

So it's a slightly deeper "S3" than I've usually seen, but not by much.

And correct me if I'm wrong, but your RTC can work with "rtc-cmos", and the wake hooks I've posted (and which will be in the next MM release) sound like they'll work just fine (given OLPC-specific implementation, which is clearly not included in the code above).

Indeed, and thank you much for that work - we'll be sure to integrate it.

Quoted 10 lines

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

This looks very good, and is pretty close to what I was proposing. Replace the acpi_*_event() calls with a generic pm_ infrastructure, and

Inside ACPI ... why?

I'm not saying inside ACPI - I'm saying at the driver level.

Whereas in that patch, the only ACPI calls were inside ACPI itself; and that driver only had a callback. FWIW that's the first time I've ever needed to have indirection on wakeup event handling ...

This is the first real "embedded" scenario to pop its head up in x86 land, up to this point, most of the boards have just been really weak desktops.

Quoted 4 lines

Quoted 3 lines

And so forth. Like the AT91, we enable the wakeup events prior to suspending. I'm willing to bet that other PM methods like ACPI could pick this up pretty quickly too.

Quoted 8 lines

With AT91 (and most ARMs) the wake events are normal IRQs, and the irq infrastructure already handles wake events cleanly. So I'd claim they don't _want_ to change something that's inexpensive, already deployed (training, code, testing, etc), and working.

So I hope you'll tolerate me if I view this as less of a "how do we get a generic solution" and more of a "how does _this_ x86-ish issue get solved cleanly".

Thats fair. x86 does live in a different world then the other embedded chips. I think its reasonable for the x86 folks to get their story straight and then move on from there.

Quoted 8 lines

Quoted 4 lines

Moving
it into the PM infrastructure seems the best way to handle everybody equally, regardless of the relative intelligence or stupidity of their hardware implementation.

I guess that's where we disagree: you're assuming "one call to rule them all", where I'm thinking that IRQ calls should manage IRQs, PCI calls should manage PME#, and so on.

Thats fine - I am not sold on a "one call to rule them all", but I do want to avoid any #ifdef CONFIG_PM_ACPI #elsif CONFIG_OLPC_PM #else nonsense.

Quoted 15 lines

Quoted 3 lines

I guess we'll probably have to do something similar for our OLPC PM code
- but thats the sort of platform specific fragmentation thing I was trying to avoid.

Since OLPC isn't using ACPI, it can be more like embedded Linux, and just Do The Right Thing ... create platform devices, etc. :)

My point above was that ACPI isn't yet integrating with the things it needs to be integrating with. If you're concerned about how to work with wakeup events in Linux, don't even bother looking at ACPI. Look at the embedded implementations; look at USB. AT91 is by far the cleanest and most complete (simplest hardware too!!), but some of the other systems (OMAP, PXA, SA1100, etc) also implement wakeup using board-specific code.

Quoted 3 lines

OLPC _could_ use that same "board-specific hacks" approach found in most embedded platforms. I'm glad you're thinking about how to avoid doing that.

We are keenly aware that we're committing a minor sin by implementing yet another PM interface for x86. But we believe that our heart is in the right place, and we are dedicated to doing the right thing, and trying our hardest not to contribute further to fragmenting the PM subsystem with implementation specific code. In many cases we're looking to help bring the others into the fold, especially our fellow x86 implementations that suffer from the same issues that we do.

I appreciate your comments, and the rtc-cmos code will go to good use.

David,

Is there any reason why programs using RTC_AIE/RTC_ALM_SET ioctls are unable to arm alarm irq's?

I'm aware of the new RTC_WKALM_SET... but shouldnt backward compatibility be kept?

The code seems to imply that its not supported anymore, however ioctl returns success but no alarm is fired (for instance alarm.enabled is set to 0):

        case RTC_ALM_SET:
                if (copy_from_user(&alarm.time, uarg, sizeof(tm)))
                        return -EFAULT;

                alarm.enabled = 0;
                alarm.pending = 0;
                alarm.time.tm_wday = -1;
                alarm.time.tm_yday = -1;
                alarm.time.tm_isdst = -1;

David,

Is there any reason why programs using RTC_AIE/RTC_ALM_SET ioctls are unable to arm alarm irq's?

I'm aware of the new RTC_WKALM_SET... but shouldnt backward compatibility be kept?

The code seems to imply that its not supported anymore, however ioctl returns success but no alarm is fired (for instance alarm.enabled is set to 0):

        case RTC_ALM_SET:
                if (copy_from_user(&alarm.time, uarg, sizeof(tm)))
                        return -EFAULT;

                alarm.enabled = 0;
                alarm.pending = 0;
                alarm.time.tm_wday = -1;
                alarm.time.tm_yday = -1;
                alarm.time.tm_isdst = -1;

Quoted 5 lines · Marcelo Tosatti

David,

Is there any reason why programs using RTC_AIE/RTC_ALM_SET ioctls are unable to arm alarm irq's?

The ALM_SET request never did! And I'm pretty sure AIE_ON does indeed set it ... the test cases use ALM_SET then AIE_ON, which is the "traditional" idiom (i.e. legacy drivers/char/rtc.c idiom).

Quoted 2 lines

I'm aware of the new RTC_WKALM_SET... but shouldnt backward compatibility be kept?

According to <linux/rtc.h> the ALM_SET ioctl takes an rtc_time, while WKALM_SET takes an rtc_wkalm ... which differs from the former by including the flag controlling wheter the alarm will be enabled (or not) when setting the alarm. (Model inherited from EFI.)

Quoted 5 lines

The code seems to imply that its not supported anymore, however ioctl returns success but no alarm is fired (for instance alarm.enabled is set to 0):

Quoted 9 lines

        case RTC_ALM_SET:
                if (copy_from_user(&alarm.time, uarg, sizeof(tm)))
                        return -EFAULT;

                alarm.enabled = 0;
                alarm.pending = 0;
                alarm.time.tm_wday = -1;
                alarm.time.tm_yday = -1;
                alarm.time.tm_isdst = -1;

That code is generic RTC framework code (rtc-dev.c), and not code specific to rtc-cmos ... the comment immediately following that (2.6.22-rc7 and previous) should clarify many of the issues.

I think the current manpage does the same. (If it mentions the /dev/rtc0, /dev/rtc1, etc files, it should be current enough.)

What it does is just stuff the rtc_time into the expanded rtc_wkalm. The idea is that the RTC drivers only support one alarm-setting method, but the /dev/rtcN ioctl interface uses it in two modes:

- "legacy", up to 24 hours in the future, and requiring the alarm to be enabled later with AIE_ON;

- "EFI-style", into the arbitrary future, and bundling the flag saying whether to enable the alarm.

Since ALM_SET is the legacy mode, it requires separate AIE_ON (from the RTC driver directly). But RTC_WKALM_SET doesn't.

- Dave

My point is that RTC_ALM_SET after RTC_AIE_ON works with drivers/char/rtc.c but not rtc-cmos:

        ret = ioctl(fd, RTC_AIE_ON);
        if (ret) {
                perror("ioctl RTC_AIE_ON");
                exit(0);
        }

        ret = ioctl(fd, RTC_ALM_SET, &rtctime);
        if (ret) {
                perror("ioctl RTC_ALM_SET");
                exit(0);
        }

I think its fine to consider such applications broken anyway?

I think so ... although that's unfortunately another difference between the legacy x86-mostly code and the newer RTC framework.

Quoted 3 lines · David Brownell

I think so ... although that's unfortunately another difference between the legacy x86-mostly code and the newer RTC framework.

(sorry for hijacking the thread)

Is this the interface should stuff like HAL use to do:

* Suspend for 10 minutes
* auto wakeup and then hibernate...

I figure we can do a suspend setting the rtc using the ioctls and then we wakeup, and HAL has to know that we woke up from the alarm rather than from a lid event or keypress.

Is this something we can do (or should do) for OLPC and general ACPI?

Quoted 6 lines · Richard Hughes

Quoted 3 lines · David Brownell

I think so ... although that's unfortunately another difference between the legacy x86-mostly code and the newer RTC framework.

(sorry for hijacking the thread)

I changed $SUBJECT ...

Quoted 2 lines

Is this the interface should stuff like HAL use to do:

Quoted 2 lines

* Suspend for 10 minutes
* auto wakeup and then hibernate...

That is, "Suspend-to-RAM" or "standby"? Yes, assuming that works on this particular system. Arguably that would be a direction for cpuidle to think about too, but I think alarm-driven wakeup is more ready-to-use at this point.

Quoted 3 lines

I figure we can do a suspend setting the rtc using the ioctls and then we wakeup, and HAL has to know that we woke up from the alarm rather than from a lid event or keypress.

... although I don't know whether that particular distinction is made to userspace right now. ACPI provides a bit like that, and at least a few other systems can do something analagous.

That is, we may want to provide a bit more information about the specific event which triggered wakeup. I don't believe there is such an interface, in general.

Plus, the notion seems kind of racey to me. (If you press a key right while the wakealarm fires, you don't want hibernation..)

Quoted 1 line

Is this something we can do (or should do) for OLPC and general ACPI?

I'd certainly rather see laptops doing that than what they do now:
running the battery out, and needing filesystem recovery!!

Quoted 5 lines · David Brownell

On Sunday 08 July 2007, Richard Hughes wrote:

That is, we may want to provide a bit more information about the specific event which triggered wakeup. I don't believe there is such an interface, in general.

What would be a nice interface? Perhaps an additional file under /sys/devices/.../power/wakeup_fired or something (only for devices with can_wakeup set).

Quoted 2 lines

Plus, the notion seems kind of racey to me. (If you press a key right while the wakealarm fires, you don't want hibernation..)

Then you check if the any key or other wakeup event has happened other than RTC... I don't see any problem with that.

Quoted 4 lines

Quoted 1 line

Is this something we can do (or should do) for OLPC and general ACPI?

I'd certainly rather see laptops doing that than what they do now:
running the battery out, and needing filesystem recovery!!

Yep.

Quoted 5 lines · David Brownell

On Sunday 08 July 2007, Richard Hughes wrote:

That is, we may want to provide a bit more information about the specific event which triggered wakeup. I don't believe there is such an interface, in general.

What would be a nice interface? Perhaps an additional file under /sys/devices/.../power/wakeup_fired or something (only for devices with can_wakeup set).

Quoted 2 lines

Plus, the notion seems kind of racey to me. (If you press a key right while the wakealarm fires, you don't want hibernation..)

Then you check if the any key or other wakeup event has happened other than RTC... I don't see any problem with that.

Quoted 4 lines

Quoted 1 line

Is this something we can do (or should do) for OLPC and general ACPI?

I'd certainly rather see laptops doing that than what they do now:
running the battery out, and needing filesystem recovery!!

Yep.

Quoted 7 lines · Marcelo Tosatti

Quoted 5 lines · David Brownell

On Sunday 08 July 2007, Richard Hughes wrote:

That is, we may want to provide a bit more information about the specific event which triggered wakeup. I don't believe there is such an interface, in general.

Quoted 3 lines

What would be a nice interface? Perhaps an additional file under /sys/devices/.../power/wakeup_fired or something (only for devices with can_wakeup set).

Better a /sys/power/wakeup_event (or whatever) that's more easily found. It could link to the device issuing the event.

Quoted 5 lines

Quoted 2 lines

Plus, the notion seems kind of racey to me. (If you press a key right while the wakealarm fires, you don't want hibernation..)

Then you check if the any key or other wakeup event has happened other than RTC... I don't see any problem with that.

It might be best not to need to re-activate the whole system from STR though ... it ought to be possible to just power off, having already written the image to disk. Right?

Yes, there's all that stuff about how to quiesce the system properly, freezer/icebox/flamesuits/etc. Regardless, going from STR to fully operational and then hibernating would take time and energy that might not be available...

Quoted 7 lines · Marcelo Tosatti

Quoted 5 lines · David Brownell

On Sunday 08 July 2007, Richard Hughes wrote:

That is, we may want to provide a bit more information about the specific event which triggered wakeup. I don't believe there is such an interface, in general.

Quoted 3 lines

What would be a nice interface? Perhaps an additional file under /sys/devices/.../power/wakeup_fired or something (only for devices with can_wakeup set).

Better a /sys/power/wakeup_event (or whatever) that's more easily found. It could link to the device issuing the event.

Quoted 5 lines

Quoted 2 lines

Plus, the notion seems kind of racey to me. (If you press a key right while the wakealarm fires, you don't want hibernation..)

Then you check if the any key or other wakeup event has happened other than RTC... I don't see any problem with that.

It might be best not to need to re-activate the whole system from STR though ... it ought to be possible to just power off, having already written the image to disk. Right?

Yes, there's all that stuff about how to quiesce the system properly, freezer/icebox/flamesuits/etc. Regardless, going from STR to fully operational and then hibernating would take time and energy that might not be available...

Quoted 7 lines · Marcelo Tosatti

Quoted 5 lines · David Brownell

On Sunday 08 July 2007, Richard Hughes wrote:

That is, we may want to provide a bit more information about the specific event which triggered wakeup. I don't believe there is such an interface, in general.

Quoted 3 lines

What would be a nice interface? Perhaps an additional file under /sys/devices/.../power/wakeup_fired or something (only for devices with can_wakeup set).

Better a /sys/power/wakeup_event (or whatever) that's more easily found. It could link to the device issuing the event.

Quoted 5 lines

Quoted 2 lines

Plus, the notion seems kind of racey to me. (If you press a key right while the wakealarm fires, you don't want hibernation..)

Then you check if the any key or other wakeup event has happened other than RTC... I don't see any problem with that.

It might be best not to need to re-activate the whole system from STR though ... it ought to be possible to just power off, having already written the image to disk. Right?

Yes, there's all that stuff about how to quiesce the system properly, freezer/icebox/flamesuits/etc. Regardless, going from STR to fully operational and then hibernating would take time and energy that might not be available...

- Dave

Quoted 7 lines

Quoted 4 lines

Quoted 1 line

Is this something we can do (or should do) for OLPC and general ACPI?

I'd certainly rather see laptops doing that than what they do now:
running the battery out, and needing filesystem recovery!!

Yep.

Quoted 2 lines · David Brownell

Better a /sys/power/wakeup_event (or whatever) that's more easily found. It could link to the device issuing the event.

As you mentioned, there might be two wakeup sources (RTC and power button for instance) or even more at the same time.

Do you think its fine to simply but the device separated by spaces in the wakeup_event file?

Sounds fine by me, but what about the one-value-per-file sysfs rule?

Quoted 2 lines · David Brownell

Better a /sys/power/wakeup_event (or whatever) that's more easily found. It could link to the device issuing the event.

As you mentioned, there might be two wakeup sources (RTC and power button for instance) or even more at the same time.

Do you think its fine to simply but the device separated by spaces in the wakeup_event file?

Sounds fine by me, but what about the one-value-per-file sysfs rule?

Quoted 7 lines · Marcelo Tosatti

Quoted 2 lines · David Brownell

Better a /sys/power/wakeup_event (or whatever) that's more easily found. It could link to the device issuing the event.

As you mentioned, there might be two wakeup sources (RTC and power button for instance) or even more at the same time.

Actually I though I said that there would be races. Though come to think of it, one way they'd show up on a typical embedded system would be to have multiple wake IRQs pending ... you couldn't tell which one came first. (The typical case would be a single event, of course.) ACPI-ish systems would do that with GPEs and the magic "rtc woke" flag.

And then there are shared IRQ lines serving as wake sources. There could be three wake-enabled devices on that line (plus others that aren't wake-enabled); the drivers returning IRQ_HANDLED should likely be reported as having been wake sources, but not the ones returning IRQ_NONE ...

... so yes, systems might need to present multiple wake events.

Quoted 3 lines

Do you think its fine to simply but the device separated by spaces in the wakeup_event file?

Quoted 1 line

Sounds fine by me, but what about the one-value-per-file sysfs rule?

Better to have that node be a directory of links then, rather than a single link.

Note that I'm just throwing ideas out there. I suspect that in the general case it may not be easy to map from wake event to device, without infrastructure that's now missing.

Hi.

Quoted 28 lines · On Tuesday 10 July 2007 02:26:32 David Brownell wrote:

Quoted 7 lines · Marcelo Tosatti

Quoted 2 lines · David Brownell

Better a /sys/power/wakeup_event (or whatever) that's more easily found. It could link to the device issuing the event.

As you mentioned, there might be two wakeup sources (RTC and power button for instance) or even more at the same time.

Actually I though I said that there would be races. Though come to think of it, one way they'd show up on a typical embedded system would be to have multiple wake IRQs pending ... you couldn't tell which one came first. (The typical case would be a single event, of course.) ACPI-ish systems would do that with GPEs and the magic "rtc woke" flag.

And then there are shared IRQ lines serving as wake sources. There could be three wake-enabled devices on that line (plus others that aren't wake-enabled); the drivers returning IRQ_HANDLED should likely be reported as having been wake sources, but not the ones returning IRQ_NONE ...

... so yes, systems might need to present multiple wake events.

Quoted 3 lines

Do you think its fine to simply but the device separated by spaces in the wakeup_event file?

Quoted 8 lines

Quoted 1 line

Sounds fine by me, but what about the one-value-per-file sysfs rule?

Better to have that node be a directory of links then, rather than a single link.

Note that I'm just throwing ideas out there. I suspect that in the general case it may not be easy to map from wake event to device, without infrastructure that's now missing.

FWIW, I've recently added code to Suspend2 to allow it to (optionally) set the RTC wake alarm when it's finished writing the image and check the lid switch when waking, entering a different sleep state if the lid switch is still closed. It achieves this by letting the user set the name of an rtc alarm to use (the 'rtc0' in /sys/class/rtc/rtc0/*) and of a button to use (lid/LID in /proc/acpi/button/lid/LID/*). It then opens the button directory's state file and the rtc directory's since_epoch and wakealarm files, and uses them to determine read the time since epoch and set the wakealarm and determine whether the lid button is still closed.

I don't really like the opening /proc and /sysfs files in this way - whatever solution you come up with, could you consider exposing some way for kernel code to do this more neatly?

Hi.

Quoted 28 lines · On Tuesday 10 July 2007 02:26:32 David Brownell wrote:

Quoted 7 lines · Marcelo Tosatti

Quoted 2 lines · David Brownell

Better a /sys/power/wakeup_event (or whatever) that's more easily found. It could link to the device issuing the event.

As you mentioned, there might be two wakeup sources (RTC and power button for instance) or even more at the same time.

Actually I though I said that there would be races. Though come to think of it, one way they'd show up on a typical embedded system would be to have multiple wake IRQs pending ... you couldn't tell which one came first. (The typical case would be a single event, of course.) ACPI-ish systems would do that with GPEs and the magic "rtc woke" flag.

And then there are shared IRQ lines serving as wake sources. There could be three wake-enabled devices on that line (plus others that aren't wake-enabled); the drivers returning IRQ_HANDLED should likely be reported as having been wake sources, but not the ones returning IRQ_NONE ...

... so yes, systems might need to present multiple wake events.

Quoted 3 lines

Do you think its fine to simply but the device separated by spaces in the wakeup_event file?

Quoted 8 lines

Quoted 1 line

Sounds fine by me, but what about the one-value-per-file sysfs rule?

Better to have that node be a directory of links then, rather than a single link.

Note that I'm just throwing ideas out there. I suspect that in the general case it may not be easy to map from wake event to device, without infrastructure that's now missing.

FWIW, I've recently added code to Suspend2 to allow it to (optionally) set the RTC wake alarm when it's finished writing the image and check the lid switch when waking, entering a different sleep state if the lid switch is still closed. It achieves this by letting the user set the name of an rtc alarm to use (the 'rtc0' in /sys/class/rtc/rtc0/*) and of a button to use (lid/LID in /proc/acpi/button/lid/LID/*). It then opens the button directory's state file and the rtc directory's since_epoch and wakealarm files, and uses them to determine read the time since epoch and set the wakealarm and determine whether the lid button is still closed.

I don't really like the opening /proc and /sysfs files in this way - whatever solution you come up with, could you consider exposing some way for kernel code to do this more neatly?

Quoted 33 lines · Nigel Cunningham

Hi.

Quoted 30 lines · On Tuesday 10 July 2007 02:26:32 David Brownell wrote:

Quoted 7 lines · Marcelo Tosatti

Quoted 2 lines · David Brownell

Better a /sys/power/wakeup_event (or whatever) that's more easily found. It could link to the device issuing the event.

As you mentioned, there might be two wakeup sources (RTC and power button for instance) or even more at the same time.

Actually I though I said that there would be races. Though come to think of it, one way they'd show up on a typical embedded system would be to have multiple wake IRQs pending ... you couldn't tell which one came first. (The typical case would be a single event, of course.) ACPI-ish systems would do that with GPEs and the magic "rtc woke" flag.

And then there are shared IRQ lines serving as wake sources. There could be three wake-enabled devices on that line (plus others that aren't wake-enabled); the drivers returning IRQ_HANDLED should likely be reported as having been wake sources, but not the ones returning IRQ_NONE ...

... so yes, systems might need to present multiple wake events.

Quoted 4 lines

Do you think its fine to simply but the device separated by spaces in the wakeup_event file?

Sounds fine by me, but what about the one-value-per-file sysfs rule?

Quoted 8 lines

Quoted 6 lines

Better to have that node be a directory of links then, rather than a single link.

Note that I'm just throwing ideas out there. I suspect that in the general case it may not be easy to map from wake event to device, without infrastructure that's now missing.

FWIW, I've recently added code to Suspend2 to allow it to (optionally) set

Quoted 1 line

RTC wake alarm when it's finished writing the image and check the lid

Quoted 2 lines

when waking, entering a different sleep state if the lid switch is still closed. It achieves this by letting the user set the name of an rtc alarm

Quoted 2 lines

use (the 'rtc0' in /sys/class/rtc/rtc0/*) and of a button to use (lid/LID in /proc/acpi/button/lid/LID/*). It then opens the button directory's

Quoted 1 line

file and the rtc directory's since_epoch and wakealarm files, and uses

Quoted 2 lines

to determine read the time since epoch and set the wakealarm and determine whether the lid button is still closed.

Interesting...

Quoted 1 line

I don't really like the opening /proc and /sysfs files in this way -

Quoted 2 lines

solution you come up with, could you consider exposing some way for kernel code to do this more neatly?

You're supposed to be able to use /sys/... files this way!

But that's not true of /proc/... files.

Yeah, the bit I consider to be ugly is opening the files from within the kernel, but it seemed to be necessary in order to provide the functionality without having to rely on userspace or do some sort of messy work to figure out how to access the lid button and so on.

Yeah, the bit I consider to be ugly is opening the files from within the kernel, but it seemed to be necessary in order to provide the functionality without having to rely on userspace or do some sort of messy work to figure out how to access the lid button and so on.

Quoted 2 lines · Nigel Cunningham

Yeah, the bit I consider to be ugly is opening the files from within the kernel, but it seemed to be necessary in order to provide the

Quoted 1 line

without having to rely on userspace or do some sort of messy work to

Quoted 1 line

out how to access the lid button and so on.

How are you going to shift into suspend to disk without going via userspace? It's quite plausible that people will want different configuration at that point (or, realistically, need a different set of workarounds...)

Quoted 4 lines · On Wednesday 11 July 2007 10:45:50 Matthew Garrett wrote:

How are you going to shift into suspend to disk without going via userspace? It's quite plausible that people will want different configuration at that point (or, realistically, need a different set of workarounds...)

This is done after writing the image, from kernel space. We do the suspend-to-ram, and if/when we wake from that, we look at the lid switch state before removing the image. If it's still closed, the kernel code powers down again (this time properly) without userspace ever seeing the light of day.

Quoted 4 lines · On Wednesday 11 July 2007 10:45:50 Matthew Garrett wrote:

How are you going to shift into suspend to disk without going via userspace? It's quite plausible that people will want different configuration at that point (or, realistically, need a different set of workarounds...)

This is done after writing the image, from kernel space. We do the suspend-to-ram, and if/when we wake from that, we look at the lid switch state before removing the image. If it's still closed, the kernel code powers down again (this time properly) without userspace ever seeing the light of day.

Quoted 9 lines · Nigel Cunningham

Quoted 4 lines · On Wednesday 11 July 2007 10:45:50 Matthew Garrett wrote:

How are you going to shift into suspend to disk without going via userspace? It's quite plausible that people will want different configuration at that point (or, realistically, need a different set of workarounds...)

This is done after writing the image, from kernel space. We do the suspend-to-ram, and if/when we wake from that, we look at the lid switch state before removing the image. If it's still closed, the kernel code

Quoted 2 lines

down again (this time properly) without userspace ever seeing the light of day.

Yes. I'm sort of struggling to see how this is done especially reliably
- if we have separate hibernation and suspend to ram pathways, which is executed in this scenario?

Yeah, that is a bit confusing. At the moment, I'm doing the suspend to ram platform dependent preparation and cleanup in this scenario. That's definitely the right thing to do in the case where we write an image, then suspend to ram, wake and continue working without running running out of battery (writing the image is redundant in that case). Where we end up properly powering down after suspending to ram, I believe we don't run the pm_ops->finish after doing the atomic restore when resuming the image.

Yeah, that is a bit confusing. At the moment, I'm doing the suspend to ram platform dependent preparation and cleanup in this scenario. That's definitely the right thing to do in the case where we write an image, then suspend to ram, wake and continue working without running running out of battery (writing the image is redundant in that case). Where we end up properly powering down after suspending to ram, I believe we don't run the pm_ops->finish after doing the atomic restore when resuming the image.

Quoted 7 lines · Nigel Cunningham

Yeah, that is a bit confusing. At the moment, I'm doing the suspend to ram platform dependent preparation and cleanup in this scenario. That's definitely the right thing to do in the case where we write an image, then suspend to ram, wake and continue working without running running out of battery (writing the image is redundant in that case). Where we end up properly powering down after suspending to ram, I believe we don't run the pm_ops->finish after doing the atomic restore when resuming the image.

I'm not convinced this can work terribly well. It's not unlikely that hardware will need different state stored over different types of suspend. Can you separate out the saving of kernel memory and userspace memory, then resume/suspend/save the new kernel state without touching the userspace state?

Hi.

Quoted 10 lines · On Wednesday 11 July 2007 11:59:48 Matthew Garrett wrote:

Quoted 7 lines · Nigel Cunningham

Yeah, that is a bit confusing. At the moment, I'm doing the suspend to ram platform dependent preparation and cleanup in this scenario. That's definitely the right thing to do in the case where we write an image, then suspend to ram, wake and continue working without running running out of battery (writing the image is redundant in that case). Where we end up properly powering down after suspending to ram, I believe we don't run the pm_ops->finish after doing the atomic restore when resuming the image.

Quoted 5 lines

I'm not convinced this can work terribly well. It's not unlikely that hardware will need different state stored over different types of suspend. Can you separate out the saving of kernel memory and userspace memory, then resume/suspend/save the new kernel state without touching the userspace state?

Yeah, we could redo and resave the atomic copy, but it doesn't seem to be necessary at the moment; it has been working reliably, regardless of which combination of events occurs. If/when I come across a case where we have problems, I'll give resaving the atomic copy a go.

(Thinks some more). Ah, I think we're already doing the right thing, if I'm recalling the order of actions right. If I'm remembering correctly, prior to the atomic copy, we do hibernation prep, then after the atomic copy, hibernation cleanup. Then, if suspending to ram, we do the prep/enter/cleanup after the image has finished writing. If we lose power from suspend to ram, it doesn't matter because we're just doing a normal resume then, with the hibernation cleanup post atomic restore machine the prep that was done prior to the atomic copy.

To summarise:

Hibernate + STR + full wake.
Hibernation prep
(Atomic copy)
Hibernation cleanup
STR prep
STR enter
STR cleanup
Remove hibernation image

Hibernate + STR + poweroff + hibernate resume:

Hibernation prep
(Atomic copy)
Hibernation cleanup
STR prep
STR enter
<power out> or <STR wake + power off> (STR prep/enter no longer matters) (Fresh boot)
Atomic restore
Hibernation cleanup (matching prep at start)
Remove hibernation image

Quoted 4 lines · Nigel Cunningham

Quoted 3 lines · On Wednesday 11 July 2007 11:59:48 Matthew Garrett wrote:

Quoted 1 line · Nigel Cunningham

Yeah, that is a bit confusing. At the moment, I'm doing the suspend to

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

platform dependent preparation and cleanup in this scenario. That's definitely the right thing to do in the case where we write an image,

Quoted 1 line

Quoted 1 line

Quoted 1 line

suspend to ram, wake and continue working without running running out

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

battery (writing the image is redundant in that case). Where we end up properly powering down after suspending to ram, I believe we don't run

Quoted 8 lines

Quoted 7 lines

Quoted 1 line

pm_ops->finish after doing the atomic restore when resuming the image.

I'm not convinced this can work terribly well. It's not unlikely that hardware will need different state stored over different types of suspend. Can you separate out the saving of kernel memory and userspace memory, then resume/suspend/save the new kernel state without touching the userspace state?

Quoted 6 lines

Yeah, we could redo and resave the atomic copy, but it doesn't seem to be necessary at the moment; it has been working reliably, regardless of which combination of events occurs. If/when I come across a case where we have problems, I'll give resaving the atomic copy a go.

(Thinks some more). Ah, I think we're already doing the right thing, if

Quoted 1 line

recalling the order of actions right. If I'm remembering correctly, prior

Quoted 2 lines

the atomic copy, we do hibernation prep, then after the atomic copy, hibernation cleanup. Then, if suspending to ram, we do the

Quoted 1 line

after the image has finished writing. If we lose power from suspend to

Quoted 2 lines

it doesn't matter because we're just doing a normal resume then, with the hibernation cleanup post atomic restore machine the prep that was done

Quoted 15 lines

to the atomic copy.

To summarise:

Hibernate + STR + full wake.
Hibernation prep
(Atomic copy)
Hibernation cleanup
STR prep
STR enter
STR cleanup
Remove hibernation image

Hibernate + STR + poweroff + hibernate resume:

Quoted 10 lines

Hibernation prep
(Atomic copy)
Hibernation cleanup
STR prep
STR enter
<power out> or <STR wake + power off> (STR prep/enter no longer matters) (Fresh boot)
Atomic restore
Hibernation cleanup (matching prep at start)
Remove hibernation image

Yes, I think that this is the right ordering, but for some graphics adapters we need to do some tricks from the user space before 'STR prep' and after 'STR cleanup', which is theoretically possible with uswsusp.

Hi.

Quoted 5 lines · On Wednesday 11 July 2007 20:09:04 Rafael J. Wysocki wrote:

Quoted 4 lines · Nigel Cunningham

Quoted 3 lines · On Wednesday 11 July 2007 11:59:48 Matthew Garrett wrote:

Quoted 1 line · Nigel Cunningham

Yeah, that is a bit confusing. At the moment, I'm doing the suspend to

ram

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

platform dependent preparation and cleanup in this scenario. That's definitely the right thing to do in the case where we write an image,

then

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

suspend to ram, wake and continue working without running running out

of

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

battery (writing the image is redundant in that case). Where we end up properly powering down after suspending to ram, I believe we don't run

the

Quoted 8 lines

Quoted 8 lines

Quoted 7 lines

Quoted 1 line

pm_ops->finish after doing the atomic restore when resuming the image.

I'm not convinced this can work terribly well. It's not unlikely that hardware will need different state stored over different types of suspend. Can you separate out the saving of kernel memory and userspace memory, then resume/suspend/save the new kernel state without touching the userspace state?

Quoted 6 lines

Quoted 6 lines

Yeah, we could redo and resave the atomic copy, but it doesn't seem to be necessary at the moment; it has been working reliably, regardless of which combination of events occurs. If/when I come across a case where we have problems, I'll give resaving the atomic copy a go.

(Thinks some more). Ah, I think we're already doing the right thing, if

I'm

Quoted 1 line

Quoted 1 line

recalling the order of actions right. If I'm remembering correctly, prior

to

Quoted 2 lines

Quoted 2 lines

the atomic copy, we do hibernation prep, then after the atomic copy, hibernation cleanup. Then, if suspending to ram, we do the

prep/enter/cleanup

Quoted 1 line

Quoted 1 line

after the image has finished writing. If we lose power from suspend to

ram,

Quoted 2 lines

Quoted 2 lines

it doesn't matter because we're just doing a normal resume then, with the hibernation cleanup post atomic restore machine the prep that was done

prior

Quoted 26 lines

Quoted 25 lines

to the atomic copy.

To summarise:

Hibernate + STR + full wake.
Hibernation prep
(Atomic copy)
Hibernation cleanup
STR prep
STR enter
STR cleanup
Remove hibernation image

Hibernate + STR + poweroff + hibernate resume:

Hibernation prep
(Atomic copy)
Hibernation cleanup
STR prep
STR enter
<power out> or <STR wake + power off> (STR prep/enter no longer matters) (Fresh boot)
Atomic restore
Hibernation cleanup (matching prep at start)
Remove hibernation image

Quoted 3 lines

Yes, I think that this is the right ordering, but for some graphics adapters we need to do some tricks from the user space before 'STR prep' and after 'STR cleanup', which is theoretically possible with uswsusp.

Yeah. I haven't done it yet, but intend to implement that using userui.

Hi.

Quoted 5 lines · On Wednesday 11 July 2007 20:09:04 Rafael J. Wysocki wrote:

Quoted 4 lines · Nigel Cunningham

Quoted 3 lines · On Wednesday 11 July 2007 11:59:48 Matthew Garrett wrote:

Quoted 1 line · Nigel Cunningham

Yeah, that is a bit confusing. At the moment, I'm doing the suspend to

ram

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

platform dependent preparation and cleanup in this scenario. That's definitely the right thing to do in the case where we write an image,

then

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

suspend to ram, wake and continue working without running running out

of

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

battery (writing the image is redundant in that case). Where we end up properly powering down after suspending to ram, I believe we don't run

the

Quoted 8 lines

Quoted 8 lines

Quoted 7 lines

Quoted 1 line

pm_ops->finish after doing the atomic restore when resuming the image.

I'm not convinced this can work terribly well. It's not unlikely that hardware will need different state stored over different types of suspend. Can you separate out the saving of kernel memory and userspace memory, then resume/suspend/save the new kernel state without touching the userspace state?

Quoted 6 lines

Quoted 6 lines

Yeah, we could redo and resave the atomic copy, but it doesn't seem to be necessary at the moment; it has been working reliably, regardless of which combination of events occurs. If/when I come across a case where we have problems, I'll give resaving the atomic copy a go.

(Thinks some more). Ah, I think we're already doing the right thing, if

I'm

Quoted 1 line

Quoted 1 line

recalling the order of actions right. If I'm remembering correctly, prior

to

Quoted 2 lines

Quoted 2 lines

the atomic copy, we do hibernation prep, then after the atomic copy, hibernation cleanup. Then, if suspending to ram, we do the

prep/enter/cleanup

Quoted 1 line

Quoted 1 line

after the image has finished writing. If we lose power from suspend to

ram,

Quoted 2 lines

Quoted 2 lines

it doesn't matter because we're just doing a normal resume then, with the hibernation cleanup post atomic restore machine the prep that was done

prior

Quoted 26 lines

Quoted 25 lines

to the atomic copy.

To summarise:

Hibernate + STR + full wake.
Hibernation prep
(Atomic copy)
Hibernation cleanup
STR prep
STR enter
STR cleanup
Remove hibernation image

Hibernate + STR + poweroff + hibernate resume:

Hibernation prep
(Atomic copy)
Hibernation cleanup
STR prep
STR enter
<power out> or <STR wake + power off> (STR prep/enter no longer matters) (Fresh boot)
Atomic restore
Hibernation cleanup (matching prep at start)
Remove hibernation image

Quoted 3 lines

Yes, I think that this is the right ordering, but for some graphics adapters we need to do some tricks from the user space before 'STR prep' and after 'STR cleanup', which is theoretically possible with uswsusp.

Yeah. I haven't done it yet, but intend to implement that using userui.

Quoted 4 lines · Nigel Cunningham

Quoted 3 lines · On Wednesday 11 July 2007 11:59:48 Matthew Garrett wrote:

Quoted 1 line · Nigel Cunningham

Yeah, that is a bit confusing. At the moment, I'm doing the suspend to

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

platform dependent preparation and cleanup in this scenario. That's definitely the right thing to do in the case where we write an image,

Quoted 1 line

Quoted 1 line

Quoted 1 line

suspend to ram, wake and continue working without running running out

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

battery (writing the image is redundant in that case). Where we end up properly powering down after suspending to ram, I believe we don't run

Quoted 8 lines

Quoted 7 lines

Quoted 1 line

pm_ops->finish after doing the atomic restore when resuming the image.

I'm not convinced this can work terribly well. It's not unlikely that hardware will need different state stored over different types of suspend. Can you separate out the saving of kernel memory and userspace memory, then resume/suspend/save the new kernel state without touching the userspace state?

Quoted 6 lines

Yeah, we could redo and resave the atomic copy, but it doesn't seem to be necessary at the moment; it has been working reliably, regardless of which combination of events occurs. If/when I come across a case where we have problems, I'll give resaving the atomic copy a go.

(Thinks some more). Ah, I think we're already doing the right thing, if

Quoted 1 line

recalling the order of actions right. If I'm remembering correctly, prior

Quoted 2 lines

the atomic copy, we do hibernation prep, then after the atomic copy, hibernation cleanup. Then, if suspending to ram, we do the

Quoted 1 line

after the image has finished writing. If we lose power from suspend to

Quoted 2 lines

it doesn't matter because we're just doing a normal resume then, with the hibernation cleanup post atomic restore machine the prep that was done

Quoted 15 lines

to the atomic copy.

To summarise:

Hibernate + STR + full wake.
Hibernation prep
(Atomic copy)
Hibernation cleanup
STR prep
STR enter
STR cleanup
Remove hibernation image

Hibernate + STR + poweroff + hibernate resume:

Quoted 10 lines

Hibernation prep
(Atomic copy)
Hibernation cleanup
STR prep
STR enter
<power out> or <STR wake + power off> (STR prep/enter no longer matters) (Fresh boot)
Atomic restore
Hibernation cleanup (matching prep at start)
Remove hibernation image

Yes, I think that this is the right ordering, but for some graphics adapters we need to do some tricks from the user space before 'STR prep' and after 'STR cleanup', which is theoretically possible with uswsusp.

Hi.

Quoted 10 lines · On Wednesday 11 July 2007 11:59:48 Matthew Garrett wrote:

Quoted 7 lines · Nigel Cunningham

Yeah, that is a bit confusing. At the moment, I'm doing the suspend to ram platform dependent preparation and cleanup in this scenario. That's definitely the right thing to do in the case where we write an image, then suspend to ram, wake and continue working without running running out of battery (writing the image is redundant in that case). Where we end up properly powering down after suspending to ram, I believe we don't run the pm_ops->finish after doing the atomic restore when resuming the image.

Quoted 5 lines

I'm not convinced this can work terribly well. It's not unlikely that hardware will need different state stored over different types of suspend. Can you separate out the saving of kernel memory and userspace memory, then resume/suspend/save the new kernel state without touching the userspace state?

Yeah, we could redo and resave the atomic copy, but it doesn't seem to be necessary at the moment; it has been working reliably, regardless of which combination of events occurs. If/when I come across a case where we have problems, I'll give resaving the atomic copy a go.

(Thinks some more). Ah, I think we're already doing the right thing, if I'm recalling the order of actions right. If I'm remembering correctly, prior to the atomic copy, we do hibernation prep, then after the atomic copy, hibernation cleanup. Then, if suspending to ram, we do the prep/enter/cleanup after the image has finished writing. If we lose power from suspend to ram, it doesn't matter because we're just doing a normal resume then, with the hibernation cleanup post atomic restore machine the prep that was done prior to the atomic copy.

To summarise:

Hibernate + STR + full wake.
Hibernation prep
(Atomic copy)
Hibernation cleanup
STR prep
STR enter
STR cleanup
Remove hibernation image

Hibernate + STR + poweroff + hibernate resume:

Hibernation prep
(Atomic copy)
Hibernation cleanup
STR prep
STR enter
<power out> or <STR wake + power off> (STR prep/enter no longer matters) (Fresh boot)
Atomic restore
Hibernation cleanup (matching prep at start)
Remove hibernation image

Quoted 7 lines · Nigel Cunningham

Yeah, that is a bit confusing. At the moment, I'm doing the suspend to ram platform dependent preparation and cleanup in this scenario. That's definitely the right thing to do in the case where we write an image, then suspend to ram, wake and continue working without running running out of battery (writing the image is redundant in that case). Where we end up properly powering down after suspending to ram, I believe we don't run the pm_ops->finish after doing the atomic restore when resuming the image.

I'm not convinced this can work terribly well. It's not unlikely that hardware will need different state stored over different types of suspend. Can you separate out the saving of kernel memory and userspace memory, then resume/suspend/save the new kernel state without touching the userspace state?

Quoted 9 lines · Nigel Cunningham

Quoted 4 lines · On Wednesday 11 July 2007 10:45:50 Matthew Garrett wrote:

How are you going to shift into suspend to disk without going via userspace? It's quite plausible that people will want different configuration at that point (or, realistically, need a different set of workarounds...)

This is done after writing the image, from kernel space. We do the suspend-to-ram, and if/when we wake from that, we look at the lid switch state before removing the image. If it's still closed, the kernel code

Quoted 2 lines

down again (this time properly) without userspace ever seeing the light of day.

Yes. I'm sort of struggling to see how this is done especially reliably
- if we have separate hibernation and suspend to ram pathways, which is executed in this scenario?

Quoted 2 lines · Nigel Cunningham

Yeah, the bit I consider to be ugly is opening the files from within the kernel, but it seemed to be necessary in order to provide the

Quoted 1 line

without having to rely on userspace or do some sort of messy work to

Quoted 1 line

out how to access the lid button and so on.

How are you going to shift into suspend to disk without going via userspace? It's quite plausible that people will want different configuration at that point (or, realistically, need a different set of workarounds...)

Yeah, the bit I consider to be ugly is opening the files from within the kernel, but it seemed to be necessary in order to provide the functionality without having to rely on userspace or do some sort of messy work to figure out how to access the lid button and so on.

Re proc files, are the button files already exposed under sysfs and I just don't know? If so, I'll happily shift to using them.

Yeah, the bit I consider to be ugly is opening the files from within the kernel, but it seemed to be necessary in order to provide the functionality without having to rely on userspace or do some sort of messy work to figure out how to access the lid button and so on.

Re proc files, are the button files already exposed under sysfs and I just don't know? If so, I'll happily shift to using them.

Quoted 2 lines · Nigel Cunningham

Yeah, the bit I consider to be ugly is opening the files from within the kernel, but it seemed to be necessary in order to provide the

Quoted 1 line

without having to rely on userspace or do some sort of messy work to

Quoted 1 line

out how to access the lid button and so on.

Perhaps there should be a formal API to such buttons.

Quoted 2 lines

Re proc files, are the button files already exposed under sysfs and I just don't know? If so, I'll happily shift to using them.

ascent:/sys/devices/acpi_system:00/device:00/PNP0C0D:00$ ls -l 
total 0
lrwxrwxrwx 1 root root    0 2007-07-11 08:58 bus -> ../../../../bus/acpi/
lrwxrwxrwx 1 root root    0 2007-07-11 08:58 

-r--r--r-- 1 root root 4096 2007-07-11 08:58 hid
lrwxrwxrwx 1 root root 0 2007-07-11 08:58

lrwxrwxrwx 1 root root 0 2007-07-11 08:58

-r--r--r-- 1 root root 4096 2007-07-11 08:58 path
drwxr-xr-x 2 root root 0 2007-07-11 08:58 power/
lrwxrwxrwx 1 root root 0 2007-07-11 08:58

-rw-r--r-- 1 root root 4096 2007-07-11 08:58 uevent

db@ascent:/sys/devices/acpi_system:00/device:00/PNP0C0D:00$ cat path
\_SB_.LID_
ascent:/sys/devices/acpi_system:00/device:00/PNP0C0D:00$ 

If the generic kernel doesn't show that, then it's likely a consequence of a submitted-but-not-yet-merged patch.

Quoted 2 lines · Nigel Cunningham

Yeah, the bit I consider to be ugly is opening the files from within the kernel, but it seemed to be necessary in order to provide the

Quoted 1 line

without having to rely on userspace or do some sort of messy work to

Quoted 1 line

out how to access the lid button and so on.

Perhaps there should be a formal API to such buttons.

Quoted 2 lines

Re proc files, are the button files already exposed under sysfs and I just don't know? If so, I'll happily shift to using them.

ascent:/sys/devices/acpi_system:00/device:00/PNP0C0D:00$ ls -l 
total 0
lrwxrwxrwx 1 root root    0 2007-07-11 08:58 bus -> ../../../../bus/acpi/
lrwxrwxrwx 1 root root    0 2007-07-11 08:58 

-r--r--r-- 1 root root 4096 2007-07-11 08:58 hid
lrwxrwxrwx 1 root root 0 2007-07-11 08:58

lrwxrwxrwx 1 root root 0 2007-07-11 08:58

-r--r--r-- 1 root root 4096 2007-07-11 08:58 path
drwxr-xr-x 2 root root 0 2007-07-11 08:58 power/
lrwxrwxrwx 1 root root 0 2007-07-11 08:58

-rw-r--r-- 1 root root 4096 2007-07-11 08:58 uevent

db@ascent:/sys/devices/acpi_system:00/device:00/PNP0C0D:00$ cat path
\_SB_.LID_
ascent:/sys/devices/acpi_system:00/device:00/PNP0C0D:00$ 

If the generic kernel doesn't show that, then it's likely a consequence of a submitted-but-not-yet-merged patch.

Quoted 33 lines · Nigel Cunningham

Hi.

Quoted 30 lines · On Tuesday 10 July 2007 02:26:32 David Brownell wrote:

Quoted 7 lines · Marcelo Tosatti

Quoted 2 lines · David Brownell

Better a /sys/power/wakeup_event (or whatever) that's more easily found. It could link to the device issuing the event.

As you mentioned, there might be two wakeup sources (RTC and power button for instance) or even more at the same time.

Actually I though I said that there would be races. Though come to think of it, one way they'd show up on a typical embedded system would be to have multiple wake IRQs pending ... you couldn't tell which one came first. (The typical case would be a single event, of course.) ACPI-ish systems would do that with GPEs and the magic "rtc woke" flag.

And then there are shared IRQ lines serving as wake sources. There could be three wake-enabled devices on that line (plus others that aren't wake-enabled); the drivers returning IRQ_HANDLED should likely be reported as having been wake sources, but not the ones returning IRQ_NONE ...

... so yes, systems might need to present multiple wake events.

Quoted 4 lines

Do you think its fine to simply but the device separated by spaces in the wakeup_event file?

Sounds fine by me, but what about the one-value-per-file sysfs rule?

Quoted 8 lines

Quoted 6 lines

Better to have that node be a directory of links then, rather than a single link.

Note that I'm just throwing ideas out there. I suspect that in the general case it may not be easy to map from wake event to device, without infrastructure that's now missing.

FWIW, I've recently added code to Suspend2 to allow it to (optionally) set

Quoted 1 line

RTC wake alarm when it's finished writing the image and check the lid

Quoted 2 lines

when waking, entering a different sleep state if the lid switch is still closed. It achieves this by letting the user set the name of an rtc alarm

Quoted 2 lines

use (the 'rtc0' in /sys/class/rtc/rtc0/*) and of a button to use (lid/LID in /proc/acpi/button/lid/LID/*). It then opens the button directory's

Quoted 1 line

file and the rtc directory's since_epoch and wakealarm files, and uses

Quoted 2 lines

to determine read the time since epoch and set the wakealarm and determine whether the lid button is still closed.

Interesting...

Quoted 1 line

I don't really like the opening /proc and /sysfs files in this way -

Quoted 2 lines

solution you come up with, could you consider exposing some way for kernel code to do this more neatly?

You're supposed to be able to use /sys/... files this way!

But that's not true of /proc/... files.

Quoted 7 lines · Marcelo Tosatti

Quoted 2 lines · David Brownell

Better a /sys/power/wakeup_event (or whatever) that's more easily found. It could link to the device issuing the event.

As you mentioned, there might be two wakeup sources (RTC and power button for instance) or even more at the same time.

Actually I though I said that there would be races. Though come to think of it, one way they'd show up on a typical embedded system would be to have multiple wake IRQs pending ... you couldn't tell which one came first. (The typical case would be a single event, of course.) ACPI-ish systems would do that with GPEs and the magic "rtc woke" flag.

And then there are shared IRQ lines serving as wake sources. There could be three wake-enabled devices on that line (plus others that aren't wake-enabled); the drivers returning IRQ_HANDLED should likely be reported as having been wake sources, but not the ones returning IRQ_NONE ...

... so yes, systems might need to present multiple wake events.

Quoted 3 lines

Do you think its fine to simply but the device separated by spaces in the wakeup_event file?

Quoted 1 line

Sounds fine by me, but what about the one-value-per-file sysfs rule?

Better to have that node be a directory of links then, rather than a single link.

Note that I'm just throwing ideas out there. I suspect that in the general case it may not be easy to map from wake event to device, without infrastructure that's now missing.

Quoted 7 lines · Marcelo Tosatti

Quoted 5 lines · David Brownell

On Sunday 08 July 2007, Richard Hughes wrote:

That is, we may want to provide a bit more information about the specific event which triggered wakeup. I don't believe there is such an interface, in general.

Quoted 3 lines

What would be a nice interface? Perhaps an additional file under /sys/devices/.../power/wakeup_fired or something (only for devices with can_wakeup set).

Better a /sys/power/wakeup_event (or whatever) that's more easily found. It could link to the device issuing the event.

Quoted 5 lines

Quoted 2 lines

Plus, the notion seems kind of racey to me. (If you press a key right while the wakealarm fires, you don't want hibernation..)

Then you check if the any key or other wakeup event has happened other than RTC... I don't see any problem with that.

It might be best not to need to re-activate the whole system from STR though ... it ought to be possible to just power off, having already written the image to disk. Right?

Yes, there's all that stuff about how to quiesce the system properly, freezer/icebox/flamesuits/etc. Regardless, going from STR to fully operational and then hibernating would take time and energy that might not be available...

- Dave

Quoted 7 lines

Quoted 4 lines

Quoted 1 line

Is this something we can do (or should do) for OLPC and general ACPI?

I'd certainly rather see laptops doing that than what they do now:
running the battery out, and needing filesystem recovery!!

Yep.

Quoted 6 lines · Richard Hughes

Quoted 3 lines · David Brownell

I think so ... although that's unfortunately another difference between the legacy x86-mostly code and the newer RTC framework.

(sorry for hijacking the thread)

I changed $SUBJECT ...

Quoted 2 lines

Is this the interface should stuff like HAL use to do:

Quoted 2 lines

* Suspend for 10 minutes
* auto wakeup and then hibernate...

That is, "Suspend-to-RAM" or "standby"? Yes, assuming that works on this particular system. Arguably that would be a direction for cpuidle to think about too, but I think alarm-driven wakeup is more ready-to-use at this point.

Quoted 3 lines

I figure we can do a suspend setting the rtc using the ioctls and then we wakeup, and HAL has to know that we woke up from the alarm rather than from a lid event or keypress.

... although I don't know whether that particular distinction is made to userspace right now. ACPI provides a bit like that, and at least a few other systems can do something analagous.

That is, we may want to provide a bit more information about the specific event which triggered wakeup. I don't believe there is such an interface, in general.

Plus, the notion seems kind of racey to me. (If you press a key right while the wakealarm fires, you don't want hibernation..)

Quoted 1 line

Is this something we can do (or should do) for OLPC and general ACPI?

I'd certainly rather see laptops doing that than what they do now:
running the battery out, and needing filesystem recovery!!

Quoted 3 lines · David Brownell

I think so ... although that's unfortunately another difference between the legacy x86-mostly code and the newer RTC framework.

(sorry for hijacking the thread)

Is this the interface should stuff like HAL use to do:

* Suspend for 10 minutes
* auto wakeup and then hibernate...

I figure we can do a suspend setting the rtc using the ioctls and then we wakeup, and HAL has to know that we woke up from the alarm rather than from a lid event or keypress.

Is this something we can do (or should do) for OLPC and general ACPI?

I think so ... although that's unfortunately another difference between the legacy x86-mostly code and the newer RTC framework.

My point is that RTC_ALM_SET after RTC_AIE_ON works with drivers/char/rtc.c but not rtc-cmos:

        ret = ioctl(fd, RTC_AIE_ON);
        if (ret) {
                perror("ioctl RTC_AIE_ON");
                exit(0);
        }

        ret = ioctl(fd, RTC_ALM_SET, &rtctime);
        if (ret) {
                perror("ioctl RTC_ALM_SET");
                exit(0);
        }

I think its fine to consider such applications broken anyway?

Quoted 5 lines · Marcelo Tosatti

David,

Is there any reason why programs using RTC_AIE/RTC_ALM_SET ioctls are unable to arm alarm irq's?

The ALM_SET request never did! And I'm pretty sure AIE_ON does indeed set it ... the test cases use ALM_SET then AIE_ON, which is the "traditional" idiom (i.e. legacy drivers/char/rtc.c idiom).

Quoted 2 lines

I'm aware of the new RTC_WKALM_SET... but shouldnt backward compatibility be kept?

According to <linux/rtc.h> the ALM_SET ioctl takes an rtc_time, while WKALM_SET takes an rtc_wkalm ... which differs from the former by including the flag controlling wheter the alarm will be enabled (or not) when setting the alarm. (Model inherited from EFI.)

Quoted 5 lines

The code seems to imply that its not supported anymore, however ioctl returns success but no alarm is fired (for instance alarm.enabled is set to 0):

Quoted 9 lines

        case RTC_ALM_SET:
                if (copy_from_user(&alarm.time, uarg, sizeof(tm)))
                        return -EFAULT;

                alarm.enabled = 0;
                alarm.pending = 0;
                alarm.time.tm_wday = -1;
                alarm.time.tm_yday = -1;
                alarm.time.tm_isdst = -1;

That code is generic RTC framework code (rtc-dev.c), and not code specific to rtc-cmos ... the comment immediately following that (2.6.22-rc7 and previous) should clarify many of the issues.

I think the current manpage does the same. (If it mentions the /dev/rtc0, /dev/rtc1, etc files, it should be current enough.)

What it does is just stuff the rtc_time into the expanded rtc_wkalm. The idea is that the RTC drivers only support one alarm-setting method, but the /dev/rtcN ioctl interface uses it in two modes:

- "legacy", up to 24 hours in the future, and requiring the alarm to be enabled later with AIE_ON;

- "EFI-style", into the arbitrary future, and bundling the flag saying whether to enable the alarm.

Since ALM_SET is the legacy mode, it requires separate AIE_ON (from the RTC driver directly). But RTC_WKALM_SET doesn't.

- Dave

Quoted 9 lines · David Brownell

Quoted 5 lines

So, for better or worse, we are implementing a new power management manger. Here is the code:

http://dev.laptop.org/git.do?p=olpc-2.6;a=blob;h=e6e19ac32a881be962db7190f2cb460e9f60a9da;hb=powermgmt;f=arch/i386/kernel/olpc-pm.c

Does that work yet with CONFIG_NO_HZ, so that the system idle loop enters some "almost everyting is turned off" low power state? Or would that be something other than a "PM Manager"?

I think eventually we'll get into entering deeper states in the idle loop. For now, we're just going into the classic C1 / hlt state, which is actually quite thrifty on the Geode, due to automatic hardware clock gating that happens throughout the chip(s).

Quoted 16 lines

Quoted 7 lines

Quoted 6 lines

Quoted 1 line

On the Geode, we have 17 possible wake sources, all of which are ORed

What are those? You mentioned four before. (RTC alarm, two GPIOs, and the power button.) I'd guess various PCI devices can also issue wakeups. (And if USB is one of them, up to 126 USB devices hooked up to each USB host controller!)

Quoted 8 lines

The silicon can handle wakeups from RTC, a sleep button, a power button, 8 GPIO groups, the USB controller (covering any and all USB events), UART1, UART2, SMbus and the PIC.

OLPC really only cares about 4 of those, because our only sleep state is ~S3, and the PIC, smbus, UARTs and USB are off in S3. We also only have the power button, and all the rest of the wakeup sources come from the GPIOs.

Quoted 6 lines

So it's a slightly deeper "S3" than I've usually seen, but not by much.

And correct me if I'm wrong, but your RTC can work with "rtc-cmos", and the wake hooks I've posted (and which will be in the next MM release) sound like they'll work just fine (given OLPC-specific implementation, which is clearly not included in the code above).

Indeed, and thank you much for that work - we'll be sure to integrate it.

Quoted 10 lines

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

This looks very good, and is pretty close to what I was proposing. Replace the acpi_*_event() calls with a generic pm_ infrastructure, and

Inside ACPI ... why?

I'm not saying inside ACPI - I'm saying at the driver level.

Whereas in that patch, the only ACPI calls were inside ACPI itself; and that driver only had a callback. FWIW that's the first time I've ever needed to have indirection on wakeup event handling ...

This is the first real "embedded" scenario to pop its head up in x86 land, up to this point, most of the boards have just been really weak desktops.

Quoted 4 lines

Quoted 3 lines

And so forth. Like the AT91, we enable the wakeup events prior to suspending. I'm willing to bet that other PM methods like ACPI could pick this up pretty quickly too.

Quoted 8 lines

With AT91 (and most ARMs) the wake events are normal IRQs, and the irq infrastructure already handles wake events cleanly. So I'd claim they don't _want_ to change something that's inexpensive, already deployed (training, code, testing, etc), and working.

So I hope you'll tolerate me if I view this as less of a "how do we get a generic solution" and more of a "how does _this_ x86-ish issue get solved cleanly".

Thats fair. x86 does live in a different world then the other embedded chips. I think its reasonable for the x86 folks to get their story straight and then move on from there.

Quoted 8 lines

Quoted 4 lines

Moving
it into the PM infrastructure seems the best way to handle everybody equally, regardless of the relative intelligence or stupidity of their hardware implementation.

I guess that's where we disagree: you're assuming "one call to rule them all", where I'm thinking that IRQ calls should manage IRQs, PCI calls should manage PME#, and so on.

Thats fine - I am not sold on a "one call to rule them all", but I do want to avoid any #ifdef CONFIG_PM_ACPI #elsif CONFIG_OLPC_PM #else nonsense.

Quoted 15 lines

Quoted 3 lines

I guess we'll probably have to do something similar for our OLPC PM code
- but thats the sort of platform specific fragmentation thing I was trying to avoid.

Since OLPC isn't using ACPI, it can be more like embedded Linux, and just Do The Right Thing ... create platform devices, etc. :)

My point above was that ACPI isn't yet integrating with the things it needs to be integrating with. If you're concerned about how to work with wakeup events in Linux, don't even bother looking at ACPI. Look at the embedded implementations; look at USB. AT91 is by far the cleanest and most complete (simplest hardware too!!), but some of the other systems (OMAP, PXA, SA1100, etc) also implement wakeup using board-specific code.

Quoted 3 lines

OLPC _could_ use that same "board-specific hacks" approach found in most embedded platforms. I'm glad you're thinking about how to avoid doing that.

We are keenly aware that we're committing a minor sin by implementing yet another PM interface for x86. But we believe that our heart is in the right place, and we are dedicated to doing the right thing, and trying our hardest not to contribute further to fragmenting the PM subsystem with implementation specific code. In many cases we're looking to help bring the others into the fold, especially our fellow x86 implementations that suffer from the same issues that we do.

I appreciate your comments, and the rtc-cmos code will go to good use.

From my understand ACPI fills a cmos_rtc_board_info with pointers for

From my understand ACPI fills a cmos_rtc_board_info with pointers for wake_on/wake_off callbacks and registers that at dev->platform_data.

So use that to retrieve the callback pointers.

Am I missing something?

drivers/rtc/rtc-cmos.c +3 -2

index e24ea82..5bfab23 100644
@@ -549,6 +549,7 @@ #ifdef	CONFIG_PM
 static int cmos_suspend(struct device *dev, pm_message_t mesg)
 {
 	struct cmos_rtc	*cmos = dev_get_drvdata(dev);
+	struct cmos_rtc_board_info *board_info = dev->platform_data;
 	int		do_wake = device_may_wakeup(dev);
 	unsigned char	tmp;
 

@@ -572,8 +573,8 @@ static int cmos_suspend(struct device *d
 
 	if (tmp & RTC_AIE) {
 		cmos->enabled_wake = 1;
-		if (cmos->wake_on)
-			cmos->wake_on(dev);
+		if (board_info->wake_on)
+			board_info->wake_on(dev);
 		else
 			enable_irq_wake(cmos->irq);
 	}

From my understand ACPI fills a cmos_rtc_board_info with pointers for wake_on/wake_off callbacks and registers that at dev->platform_data.

So use that to retrieve the callback pointers.

Am I missing something?

drivers/rtc/rtc-cmos.c +3 -2

index e24ea82..5bfab23 100644
@@ -549,6 +549,7 @@ #ifdef	CONFIG_PM
 static int cmos_suspend(struct device *dev, pm_message_t mesg)
 {
 	struct cmos_rtc	*cmos = dev_get_drvdata(dev);
+	struct cmos_rtc_board_info *board_info = dev->platform_data;
 	int		do_wake = device_may_wakeup(dev);
 	unsigned char	tmp;
 

@@ -572,8 +573,8 @@ static int cmos_suspend(struct device *d
 
 	if (tmp & RTC_AIE) {
 		cmos->enabled_wake = 1;
-		if (cmos->wake_on)
-			cmos->wake_on(dev);
+		if (board_info->wake_on)
+			board_info->wake_on(dev);
 		else
 			enable_irq_wake(cmos->irq);
 	}

From my understand ACPI fills a cmos_rtc_board_info with pointers for

Hi,

The following patch implements the hooks necessary for OLPC to use the rtc-cmos driver. This is necessary since we do not want CONFIG_PNP.

This makes it possible to control rtc wakeup via
/sys/devices/platform/rtc_cmos/power/wakeup.

Comments?

Hi,

The following patch implements the hooks necessary for OLPC to use the rtc-cmos driver. This is necessary since we do not want CONFIG_PNP.

This makes it possible to control rtc wakeup via
/sys/devices/platform/rtc_cmos/power/wakeup.

Comments?

Quoted 10 lines · Marcelo Tosatti

Hi,

The following patch implements the hooks necessary for OLPC to use the rtc-cmos driver. This is necessary since we do not want CONFIG_PNP.

This makes it possible to control rtc wakeup via
/sys/devices/platform/rtc_cmos/power/wakeup.

Comments?

Looks almost right ... though I'd have expected the RTC to support alarms more than just 24 hours into the future!

It's more complicated than necessary. It'd be much simpler to just declare a static platform_device with pre-initted platform_data. Then call device_init_wakeup() on that before calling the usual platform_device_register() routine.

That way you wouldn't need to have two different init sections just to set up one device, where one of them is spending most of its time working around the possibility that the other one failed. And you'd also pretty much eliminate the possibility that the (now) single init section fails. :)

Like this?

+static int olpc_rtc_init(void)

arch/i386/kernel/olpc-pm.c +2 -0

+#endif /* CONFIG_RTC_DRV_CMOS */
+
 static void olpc_pm_exit(void)
 {
 	/* Clear any pending events, and disable them */

Quoted 1 line · Marcelo Tosatti

Like this?

Not quite ...

Review

Quoted 5 lines

(quoted diff) +5 -0

+struct resource rtc_platform_irq = {
+	.flags = IORESOURCE_IRQ,
+	.start = 8,
+	.end = 8,
+};

Unused, right?

Quoted 1 line

+static int olpc_rtc_init(void)

... should be marked __init ...

Review

Quoted 6 lines

(quoted diff) +6 -0

+{
+	rtc_info.rtc_day_alarm = 0;
+	rtc_info.rtc_mon_alarm = 0;
+	rtc_info.rtc_century = 0;
+	rtc_info.wake_on = rtc_wake_on;
+	rtc_info.wake_off = rtc_wake_off;

... that can all just be static init ...

Review

Quoted 4 lines

(quoted diff) +4 -0

+
+	(void)platform_device_register(&olpc_rtc_device);
+
+	device_init_wakeup(&olpc_rtc_device.dev, 1);

... do the init_wakeup before registering the device, so there can never be confusion about whether a probe() will see that part of device config ...

Quoted 8 lines

arch/i386/kernel/olpc-pm.c static int __init olpc_pm_init(void) · line 603

Quoted 4 lines

arch/i386/kernel/olpc-pm.c +3 -0

+	(void)platform_device_register(&olpc_rtc_device);
+
+	device_init_wakeup(&olpc_rtc_device.dev, 1);

... do the init_wakeup before registering the device, so there can never be confusion about whether a probe() will see that part of device config ...

For some reason doing things in that order causes wake_on/wake_off to not be called...

Thanks again for the comments.

Quoted 8 lines

arch/i386/kernel/olpc-pm.c static int __init olpc_pm_init(void) · line 603

Quoted 4 lines

arch/i386/kernel/olpc-pm.c +3 -0

+	(void)platform_device_register(&olpc_rtc_device);
+
+	device_init_wakeup(&olpc_rtc_device.dev, 1);

... do the init_wakeup before registering the device, so there can never be confusion about whether a probe() will see that part of device config ...

For some reason doing things in that order causes wake_on/wake_off to not be called...

Thanks again for the comments.

Quoted 1 line · Marcelo Tosatti

Like this?

Not quite ...

Review

Quoted 5 lines

(quoted diff) +5 -0

+struct resource rtc_platform_irq = {
+	.flags = IORESOURCE_IRQ,
+	.start = 8,
+	.end = 8,
+};

Unused, right?

Quoted 1 line

+static int olpc_rtc_init(void)

... should be marked __init ...

Review

Quoted 6 lines

(quoted diff) +6 -0

+{
+	rtc_info.rtc_day_alarm = 0;
+	rtc_info.rtc_mon_alarm = 0;
+	rtc_info.rtc_century = 0;
+	rtc_info.wake_on = rtc_wake_on;
+	rtc_info.wake_off = rtc_wake_off;

... that can all just be static init ...

Review

Quoted 4 lines

(quoted diff) +4 -0

+
+	(void)platform_device_register(&olpc_rtc_device);
+
+	device_init_wakeup(&olpc_rtc_device.dev, 1);

... do the init_wakeup before registering the device, so there can never be confusion about whether a probe() will see that part of device config ...

Like this?

+static int olpc_rtc_init(void)

arch/i386/kernel/olpc-pm.c +2 -0

+#endif /* CONFIG_RTC_DRV_CMOS */
+
 static void olpc_pm_exit(void)
 {
 	/* Clear any pending events, and disable them */

Quoted 10 lines · Marcelo Tosatti

Hi,

The following patch implements the hooks necessary for OLPC to use the rtc-cmos driver. This is necessary since we do not want CONFIG_PNP.

This makes it possible to control rtc wakeup via
/sys/devices/platform/rtc_cmos/power/wakeup.

Comments?

Looks almost right ... though I'd have expected the RTC to support alarms more than just 24 hours into the future!

It's more complicated than necessary. It'd be much simpler to just declare a static platform_device with pre-initted platform_data. Then call device_init_wakeup() on that before calling the usual platform_device_register() routine.

That way you wouldn't need to have two different init sections just to set up one device, where one of them is spending most of its time working around the possibility that the other one failed. And you'd also pretty much eliminate the possibility that the (now) single init section fails. :)

Quoted 5 lines

So, for better or worse, we are implementing a new power management manger. Here is the code:

http://dev.laptop.org/git.do?p=olpc-2.6;a=blob;h=e6e19ac32a881be962db7190f2cb460e9f60a9da;hb=powermgmt;f=arch/i386/kernel/olpc-pm.c

Does that work yet with CONFIG_NO_HZ, so that the system idle loop enters some "almost everyting is turned off" low power state? Or would that be something other than a "PM Manager"?

Quoted 7 lines

Quoted 6 lines

Quoted 1 line

On the Geode, we have 17 possible wake sources, all of which are ORed

What are those? You mentioned four before. (RTC alarm, two GPIOs, and the power button.) I'd guess various PCI devices can also issue wakeups. (And if USB is one of them, up to 126 USB devices hooked up to each USB host controller!)

Quoted 8 lines

The silicon can handle wakeups from RTC, a sleep button, a power button, 8 GPIO groups, the USB controller (covering any and all USB events), UART1, UART2, SMbus and the PIC.

OLPC really only cares about 4 of those, because our only sleep state is ~S3, and the PIC, smbus, UARTs and USB are off in S3. We also only have the power button, and all the rest of the wakeup sources come from the GPIOs.

So it's a slightly deeper "S3" than I've usually seen, but not by much.

And correct me if I'm wrong, but your RTC can work with "rtc-cmos", and the wake hooks I've posted (and which will be in the next MM release) sound like they'll work just fine (given OLPC-specific implementation, which is clearly not included in the code above).

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

This looks very good, and is pretty close to what I was proposing. Replace the acpi_*_event() calls with a generic pm_ infrastructure, and

Inside ACPI ... why?

I'm not saying inside ACPI - I'm saying at the driver level.

Whereas in that patch, the only ACPI calls were inside ACPI itself; and that driver only had a callback. FWIW that's the first time I've ever needed to have indirection on wakeup event handling ...

Quoted 3 lines

And so forth. Like the AT91, we enable the wakeup events prior to suspending. I'm willing to bet that other PM methods like ACPI could pick this up pretty quickly too.

With AT91 (and most ARMs) the wake events are normal IRQs, and the irq infrastructure already handles wake events cleanly. So I'd claim they don't _want_ to change something that's inexpensive, already deployed (training, code, testing, etc), and working.

So I hope you'll tolerate me if I view this as less of a "how do we get a generic solution" and more of a "how does _this_ x86-ish issue get solved cleanly".

Quoted 4 lines

Moving
it into the PM infrastructure seems the best way to handle everybody equally, regardless of the relative intelligence or stupidity of their hardware implementation.

I guess that's where we disagree: you're assuming "one call to rule them all", where I'm thinking that IRQ calls should manage IRQs, PCI calls should manage PME#, and so on.

Quoted 3 lines

I guess we'll probably have to do something similar for our OLPC PM code
- but thats the sort of platform specific fragmentation thing I was trying to avoid.

Since OLPC isn't using ACPI, it can be more like embedded Linux, and just Do The Right Thing ... create platform devices, etc. :)

My point above was that ACPI isn't yet integrating with the things it needs to be integrating with. If you're concerned about how to work with wakeup events in Linux, don't even bother looking at ACPI. Look at the embedded implementations; look at USB. AT91 is by far the cleanest and most complete (simplest hardware too!!), but some of the other systems (OMAP, PXA, SA1100, etc) also implement wakeup using board-specific code.

OLPC _could_ use that same "board-specific hacks" approach found in most embedded platforms. I'm glad you're thinking about how to avoid doing that.

Quoted 6 lines

It seems
to me to be more logical to move the wakeup intelligence to the PM subsystem, and then let that code distribute it to where it is needed. In the case of x86, the logic would stay in the PM method, for other processors, it might involve a call to the interrupt mapper.

But why should a "PM subsystem" be created to abstract something that already works just fine?

It'd make much more sense to me to come up with a good way to handle the x86 issues first, and only then think about whether to try making that work on other systems.

Quoted 7 lines

Quoted 7 lines

And for PCI based things, pci_enable_wake() encapsulates everything that needs doing. Not that ACPI actually *does* anything yet! But the stuff that writing /proc/acpi/wakeup enables should happen in that routine ... and eventually PCI runtime wake events should work too. (So: no drivers would ever call ACPI directly, and they already have the generic call that ACPI should hook.)

Quoted 5 lines

Quoted 2 lines

That seems to cover most drivers in Linux, without a need for any new generic PM infrastructure. Did I overlook something important?

No, I don't think so - we're very close on agreeing here. I just don't know if enable_irq_wake() is the best way to provide the generic call.

It's not, and I didn't propose doing that either.

Having all drivers try to use the same calls to manage wakeup mechanisms doesn't seem like the right model. When the mechanism is the same -- wakeup irqs, say -- then yes the calls should be the same. But when the mechanisms differ -- PCI PME# is very different from wakeup IRQS, only one per device etc -- then the calls can reasonably be different.

Quoted 4 lines

Moving
it into the PM infrastructure seems the best way to handle everybody equally, regardless of the relative intelligence or stupidity of their hardware implementation.

I guess that's where we disagree: you're assuming "one call to rule them all", where I'm thinking that IRQ calls should manage IRQs, PCI calls should manage PME#, and so on.

Quoted 18 lines

Quoted 17 lines

Quoted 5 lines

The only other issue then, is how we could define and manage wakeup events for events that aren't associated with specific devices, like power button and lid events. We'll need some way to control those somewhere in sysfs - if not in /sys/power/wakeup like I had proposed, then somewhere under the platform or system hierarchy .

I see /sys/devices/acpi_system:00/button_power:00 on this system; and /sys/devices/acpi_system:00/device:00/PNP0C0D:00 has path \_SB_.LID_ ... such device nodes already exist, even though they're not really hooked up to anything much. Notably, their "wakeup" state is not initialized.

And while it seems that the three USB controllers on this system show up as /sys/devices/acpi_system:00/device:00/PNP0A03:00/device:{01,02,03} I have no idea which one is /sys/devices/pci0000:00/0000:00:02.2 versus 0000:00:02.1 or 0000:00:02.0 ... I know that USB0 is device:01 and so on (by reading "path"), but associating one with a PCI device seems to involve pure guesswork.

Quoted 3 lines

I guess we'll probably have to do something similar for our OLPC PM code
- but thats the sort of platform specific fragmentation thing I was trying to avoid.

Since OLPC isn't using ACPI, it can be more like embedded Linux, and just Do The Right Thing ... create platform devices, etc. :)

My point above was that ACPI isn't yet integrating with the things it needs to be integrating with. If you're concerned about how to work with wakeup events in Linux, don't even bother looking at ACPI. Look at the embedded implementations; look at USB. AT91 is by far the cleanest and most complete (simplest hardware too!!), but some of the other systems (OMAP, PXA, SA1100, etc) also implement wakeup using board-specific code.

OLPC _could_ use that same "board-specific hacks" approach found in most embedded platforms. I'm glad you're thinking about how to avoid doing that.

- Dave
_______________________________________________
Devel mailing list
[email protected]
http://mailman.laptop.org/mailman/listinfo/devel

Quoted 12 lines

Quoted 12 lines

Quoted 11 lines

I see /sys/devices/acpi_system:00/button_power:00 on this system; and /sys/devices/acpi_system:00/device:00/PNP0C0D:00 has path \_SB_.LID_ ... such device nodes already exist, even though they're not really hooked up to anything much. Notably, their "wakeup" state is not initialized.

And while it seems that the three USB controllers on this system show up as /sys/devices/acpi_system:00/device:00/PNP0A03:00/device:{01,02,03} I have no idea which one is /sys/devices/pci0000:00/0000:00:02.2 versus 0000:00:02.1 or 0000:00:02.0 ... I know that USB0 is device:01 and so on (by reading "path"), but associating one with a PCI device seems to involve pure guesswork.

Quoted 6 lines

Quoted 3 lines

I guess we'll probably have to do something similar for our OLPC PM code
- but thats the sort of platform specific fragmentation thing I was trying to avoid.

Since OLPC isn't using ACPI, it can be more like embedded Linux, and just Do The Right Thing ... create platform devices, etc. :)

As you know we want user configuration of enabled wakeup events (unlike embedded platforms where this is hardcoded). It seems that the current interface available for that is /sys/devices/../power/wakeup hook (when !ACPI).

However there are several wakeup capable devices in OLPC which do not have drivers, thus no platform devices:

- power button
- lid

It seems that creating platform devices for these two just for the purpose of a having an interface for enabling wakeup events is overkill.

Quoted 12 lines

Quoted 12 lines

Quoted 11 lines

I see /sys/devices/acpi_system:00/button_power:00 on this system; and /sys/devices/acpi_system:00/device:00/PNP0C0D:00 has path \_SB_.LID_ ... such device nodes already exist, even though they're not really hooked up to anything much. Notably, their "wakeup" state is not initialized.

And while it seems that the three USB controllers on this system show up as /sys/devices/acpi_system:00/device:00/PNP0A03:00/device:{01,02,03} I have no idea which one is /sys/devices/pci0000:00/0000:00:02.2 versus 0000:00:02.1 or 0000:00:02.0 ... I know that USB0 is device:01 and so on (by reading "path"), but associating one with a PCI device seems to involve pure guesswork.

Quoted 6 lines

Quoted 3 lines

I guess we'll probably have to do something similar for our OLPC PM code
- but thats the sort of platform specific fragmentation thing I was trying to avoid.

Since OLPC isn't using ACPI, it can be more like embedded Linux, and just Do The Right Thing ... create platform devices, etc. :)

As you know we want user configuration of enabled wakeup events (unlike embedded platforms where this is hardcoded). It seems that the current interface available for that is /sys/devices/../power/wakeup hook (when !ACPI).

However there are several wakeup capable devices in OLPC which do not have drivers, thus no platform devices:

- power button
- lid

It seems that creating platform devices for these two just for the purpose of a having an interface for enabling wakeup events is overkill.

As you know we want user configuration of enabled wakeup events (unlike embedded platforms where this is hardcoded). It seems that the current interface available for that is /sys/devices/../power/wakeup hook (when !ACPI).

However there are several wakeup capable devices in OLPC which do not have drivers, thus no platform devices:

- power button
- lid

It seems that creating platform devices for these two just for the purpose of a having an interface for enabling wakeup events is overkill.

Given that, we probably want something similar to what was initially described in http://wiki.laptop.org/go/Power_Management_Interface ?

The downside of doing that is a non-unified interface: platform devices via /sys/devices/../power/wakeup and otherwise /sys/whatever/wakeup/source?

Quoted 3 lines

Since OLPC isn't using ACPI, it can be more like embedded Linux, and just Do The Right Thing ... create platform devices, etc. :)

As you know we want user configuration of enabled wakeup events (unlike embedded platforms where this is hardcoded). It seems that the current interface available for that is /sys/devices/../power/wakeup hook (when !ACPI).

However there are several wakeup capable devices in OLPC which do not have drivers, thus no platform devices:

- power button
- lid

It seems that creating platform devices for these two just for the purpose of a having an interface for enabling wakeup events is overkill.

Given that, we probably want something similar to what was initially described in http://wiki.laptop.org/go/Power_Management_Interface ?

The downside of doing that is a non-unified interface: platform devices via /sys/devices/../power/wakeup and otherwise /sys/whatever/wakeup/source?

Quoted 3 lines

Since OLPC isn't using ACPI, it can be more like embedded Linux, and just Do The Right Thing ... create platform devices, etc. :)

As you know we want user configuration of enabled wakeup events (unlike embedded platforms where this is hardcoded). It seems that the current interface available for that is /sys/devices/../power/wakeup hook (when !ACPI).

However there are several wakeup capable devices in OLPC which do not have drivers, thus no platform devices:

- power button
- lid

It seems that creating platform devices for these two just for the purpose of a having an interface for enabling wakeup events is overkill.

Given that, we probably want something similar to what was initially described in http://wiki.laptop.org/go/Power_Management_Interface ?

The downside of doing that is a non-unified interface: platform devices via /sys/devices/../power/wakeup and otherwise /sys/whatever/wakeup/source?