[PATCH] powermac: proper sleep management

[PATCH] powermac: proper sleep management

[Johannes Berg]

After having removed the power management ops from powermac completely, this
patch adds them back for PMU based machines, directly in the PMU driver.
This finally allows suspending via /sys/power/state on powerbooks.

The patch also replaces the PMU ioctl with a simple call to
pm_suspend(PM_SUSPEND_MEM) and puts the sleep-related PMU ioctls onto the
feature-removal schedule.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>

---
This was tested on slightly older machines by David Woodhouse and by me
on my PowerBook5,6. It will apply, build and suspend-to-ram will work
against current kernels (the arch suspend hooks have been merged via
Greg KH) but it will break suspend to disk if working. This is addressed
by a patch to the generic code that Andrew Morton has and will be
sending soon under the title "rework pm_ops pm_disk_mode, kill misuse".

---
 Documentation/feature-removal-schedule.txt |   10 
 drivers/macintosh/via-pmu.c                |  329 +++++++++++++----------------
 2 files changed, 159 insertions(+), 180 deletions(-)

--- linux-2.6.orig/drivers/macintosh/via-pmu.c	2007-04-27 23:56:19.176021121 +0200
+++ linux-2.6/drivers/macintosh/via-pmu.c	2007-04-28 09:45:25.746883531 +0200
@@ -61,6 +61,10 @@
 #include <asm/cputable.h>
 #include <asm/time.h>
 #include <asm/backlight.h>
+#include <asm/suspend.h>
+
+/* remove when we don't need kernel_power_off any more */
+#include <linux/reboot.h>
 
 #include "via-pmu-event.h"
 
@@ -155,9 +159,6 @@ static int drop_interrupts;
 #if defined(CONFIG_PM) && defined(CONFIG_PPC32)
 static int option_lid_wakeup = 1;
 #endif /* CONFIG_PM && CONFIG_PPC32 */
-#if (defined(CONFIG_PM)&&defined(CONFIG_PPC32))||defined(CONFIG_PMAC_BACKLIGHT_LEGACY)
-static int sleep_in_progress;
-#endif
 static unsigned long async_req_locks;
 static unsigned int pmu_irq_stats[11];
 
@@ -1991,132 +1992,6 @@ restore_via_state(void)
 
 extern void pmu_backlight_set_sleep(int sleep);
 
-static int
-pmac_suspend_devices(void)
-{
-	int ret;
-
-	pm_prepare_console();
-	
-	/* Notify old-style device drivers */
-	broadcast_sleep(PBOOK_SLEEP_REQUEST);
-
-	/* Sync the disks. */
-	/* XXX It would be nice to have some way to ensure that
-	 * nobody is dirtying any new buffers while we wait. That
-	 * could be achieved using the refrigerator for processes
-	 * that swsusp uses
-	 */
-	sys_sync();
-
-	broadcast_sleep(PBOOK_SLEEP_NOW);
-
-	/* Send suspend call to devices, hold the device core's dpm_sem */
-	ret = device_suspend(PMSG_SUSPEND);
-	if (ret) {
-		broadcast_wake();
-		printk(KERN_ERR "Driver sleep failed\n");
-		return -EBUSY;
-	}
-
-#ifdef CONFIG_PMAC_BACKLIGHT
-	/* Tell backlight code not to muck around with the chip anymore */
-	pmu_backlight_set_sleep(1);
-#endif
-
-	/* Call platform functions marked "on sleep" */
-	pmac_pfunc_i2c_suspend();
-	pmac_pfunc_base_suspend();
-
-	/* Stop preemption */
-	preempt_disable();
-
-	/* Make sure the decrementer won't interrupt us */
-	asm volatile("mtdec %0" : : "r" (0x7fffffff));
-	/* Make sure any pending DEC interrupt occurring while we did
-	 * the above didn't re-enable the DEC */
-	mb();
-	asm volatile("mtdec %0" : : "r" (0x7fffffff));
-
-	/* We can now disable MSR_EE. This code of course works properly only
-	 * on UP machines... For SMP, if we ever implement sleep, we'll have to
-	 * stop the "other" CPUs way before we do all that stuff.
-	 */
-	local_irq_disable();
-
-	/* Broadcast power down irq
-	 * This isn't that useful in most cases (only directly wired devices can
-	 * use this but still... This will take care of sysdev's as well, so
-	 * we exit from here with local irqs disabled and PIC off.
-	 */
-	ret = device_power_down(PMSG_SUSPEND);
-	if (ret) {
-		wakeup_decrementer();
-		local_irq_enable();
-		preempt_enable();
-		device_resume();
-		broadcast_wake();
-		printk(KERN_ERR "Driver powerdown failed\n");
-		return -EBUSY;
-	}
-
-	/* Wait for completion of async requests */
-	while (!batt_req.complete)
-		pmu_poll();
-
-	/* Giveup the lazy FPU & vec so we don't have to back them
-	 * up from the low level code
-	 */
-	enable_kernel_fp();
-
-#ifdef CONFIG_ALTIVEC
-	if (cpu_has_feature(CPU_FTR_ALTIVEC))
-		enable_kernel_altivec();
-#endif /* CONFIG_ALTIVEC */
-
-	return 0;
-}
-
-static int
-pmac_wakeup_devices(void)
-{
-	mdelay(100);
-
-#ifdef CONFIG_PMAC_BACKLIGHT
-	/* Tell backlight code it can use the chip again */
-	pmu_backlight_set_sleep(0);
-#endif
-
-	/* Power back up system devices (including the PIC) */
-	device_power_up();
-
-	/* Force a poll of ADB interrupts */
-	adb_int_pending = 1;
-	via_pmu_interrupt(0, NULL);
-
-	/* Restart jiffies & scheduling */
-	wakeup_decrementer();
-
-	/* Re-enable local CPU interrupts */
-	local_irq_enable();
-	mdelay(10);
-	preempt_enable();
-
-	/* Call platform functions marked "on wake" */
-	pmac_pfunc_base_resume();
-	pmac_pfunc_i2c_resume();
-
-	/* Resume devices */
-	device_resume();
-
-	/* Notify old style drivers */
-	broadcast_wake();
-
-	pm_restore_console();
-
-	return 0;
-}
-
 #define	GRACKLE_PM	(1<<7)
 #define GRACKLE_DOZE	(1<<5)
 #define	GRACKLE_NAP	(1<<4)
@@ -2127,19 +2002,12 @@ static int powerbook_sleep_grackle(void)
 	unsigned long save_l2cr;
 	unsigned short pmcr1;
 	struct adb_request req;
-	int ret;
 	struct pci_dev *grackle;
 
 	grackle = pci_find_slot(0, 0);
 	if (!grackle)
 		return -ENODEV;
 
-	ret = pmac_suspend_devices();
-	if (ret) {
-		printk(KERN_ERR "Sleep rejected by devices\n");
-		return ret;
-	}
-	
 	/* Turn off various things. Darwin does some retry tests here... */
 	pmu_request(&req, NULL, 2, PMU_POWER_CTRL0, PMU_POW0_OFF|PMU_POW0_HARD_DRIVE);
 	pmu_wait_complete(&req);
@@ -2200,8 +2068,6 @@ static int powerbook_sleep_grackle(void)
 			PMU_POW_ON|PMU_POW_BACKLIGHT|PMU_POW_CHARGER|PMU_POW_IRLED|PMU_POW_MEDIABAY);
 	pmu_wait_complete(&req);
 
-	pmac_wakeup_devices();
-
 	return 0;
 }
 
@@ -2211,7 +2077,6 @@ powerbook_sleep_Core99(void)
 	unsigned long save_l2cr;
 	unsigned long save_l3cr;
 	struct adb_request req;
-	int ret;
 	
 	if (pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,-1) < 0) {
 		printk(KERN_ERR "Sleep mode not supported on this machine\n");
@@ -2221,12 +2086,6 @@ powerbook_sleep_Core99(void)
 	if (num_online_cpus() > 1 || cpu_is_offline(0))
 		return -EAGAIN;
 
-	ret = pmac_suspend_devices();
-	if (ret) {
-		printk(KERN_ERR "Sleep rejected by devices\n");
-		return ret;
-	}
-
 	/* Stop environment and ADB interrupts */
 	pmu_request(&req, NULL, 2, PMU_SET_INTR_MASK, 0);
 	pmu_wait_complete(&req);
@@ -2297,8 +2156,6 @@ powerbook_sleep_Core99(void)
 	/* Restore LPJ, cpufreq will adjust the cpu frequency */
 	loops_per_jiffy /= 2;
 
-	pmac_wakeup_devices();
-
 	return 0;
 }
 
@@ -2308,7 +2165,7 @@ powerbook_sleep_Core99(void)
 static int
 powerbook_sleep_3400(void)
 {
-	int ret, i, x;
+	int i, x;
 	unsigned int hid0;
 	unsigned long p;
 	struct adb_request sleep_req;
@@ -2326,13 +2183,6 @@ powerbook_sleep_3400(void)
 	/* Allocate room for PCI save */
 	pbook_alloc_pci_save();
 
-	ret = pmac_suspend_devices();
-	if (ret) {
-		pbook_free_pci_save();
-		printk(KERN_ERR "Sleep rejected by devices\n");
-		return ret;
-	}
-
 	/* Save the state of PCI config space for some slots */
 	pbook_pci_save();
 
@@ -2376,7 +2226,6 @@ powerbook_sleep_3400(void)
 	while (asleep)
 		mb();
 
-	pmac_wakeup_devices();
 	pbook_free_pci_save();
 	iounmap(mem_ctrl);
 
@@ -2558,6 +2407,142 @@ pmu_release(struct inode *inode, struct 
 	return 0;
 }
 
+#if defined(CONFIG_PM) && defined(CONFIG_PPC32)
+static int powerbook_prepare_sleep(suspend_state_t state)
+{
+	/* Notify old-style device drivers */
+	broadcast_sleep(PBOOK_SLEEP_REQUEST);
+	broadcast_sleep(PBOOK_SLEEP_NOW);
+
+	return 0;
+}
+
+/*
+ * overrides the weak arch_suspend_disable_irqs in kernel/power/main.c
+ */
+void arch_suspend_disable_irqs(void)
+{
+#ifdef CONFIG_PMAC_BACKLIGHT
+	/* Tell backlight code not to muck around with the chip anymore */
+	pmu_backlight_set_sleep(1);
+#endif
+
+	/* Call platform functions marked "on sleep" */
+	pmac_pfunc_i2c_suspend();
+	pmac_pfunc_base_suspend();
+
+	/* Stop preemption */
+	preempt_disable();
+
+	/* Make sure the decrementer won't interrupt us */
+	asm volatile("mtdec %0" : : "r" (0x7fffffff));
+	/* Make sure any pending DEC interrupt occurring while we did
+	 * the above didn't re-enable the DEC */
+	mb();
+	asm volatile("mtdec %0" : : "r" (0x7fffffff));
+
+	local_irq_disable();
+}
+
+static int powerbook_sleep(suspend_state_t state)
+{
+	int error = 0;
+
+	/* Wait for completion of async requests */
+	while (!batt_req.complete)
+		pmu_poll();
+
+	/* Giveup the lazy FPU & vec so we don't have to back them
+	 * up from the low level code
+	 */
+	enable_kernel_fp();
+
+#ifdef CONFIG_ALTIVEC
+	if (cpu_has_feature(CPU_FTR_ALTIVEC))
+		enable_kernel_altivec();
+#endif /* CONFIG_ALTIVEC */
+
+	switch (pmu_kind) {
+	case PMU_OHARE_BASED:
+		error = powerbook_sleep_3400();
+		break;
+	case PMU_HEATHROW_BASED:
+	case PMU_PADDINGTON_BASED:
+		error = powerbook_sleep_grackle();
+		break;
+	case PMU_KEYLARGO_BASED:
+		error = powerbook_sleep_Core99();
+		break;
+	default:
+		return -ENOSYS;
+	}
+
+	if (error)
+		return error;
+
+	mdelay(100);
+
+#ifdef CONFIG_PMAC_BACKLIGHT
+	/* Tell backlight code it can use the chip again */
+	pmu_backlight_set_sleep(0);
+#endif
+
+	return 0;
+}
+
+/*
+ * overrides the weak arch_suspend_enable_irqs in kernel/power/main.c
+ */
+void arch_suspend_enable_irqs(void)
+{
+	/* Force a poll of ADB interrupts */
+	adb_int_pending = 1;
+	via_pmu_interrupt(0, NULL);
+
+	/* Restart jiffies & scheduling */
+	wakeup_decrementer();
+
+	/* Re-enable local CPU interrupts */
+	local_irq_enable();
+	mdelay(10);
+	preempt_enable();
+
+	/* Call platform functions marked "on wake" */
+	pmac_pfunc_base_resume();
+	pmac_pfunc_i2c_resume();
+}
+
+static int powerbook_finish_sleep(suspend_state_t state)
+{
+	/* Notify old style drivers */
+	broadcast_wake();
+
+	return 0;
+}
+
+static int pmu_sleep_valid(suspend_state_t state)
+{
+	return state == PM_SUSPEND_MEM
+		&& (pmac_call_feature(PMAC_FTR_SLEEP_STATE, NULL, 0, -1) >= 0);
+}
+
+static struct pm_ops pmu_pm_ops = {
+	.prepare = powerbook_prepare_sleep,
+	.enter = powerbook_sleep,
+	.finish = powerbook_finish_sleep,
+	.valid = pmu_sleep_valid,
+};
+
+static int register_pmu_pm_ops(void)
+{
+	pm_set_ops(&pmu_pm_ops);
+
+	return 0;
+}
+
+device_initcall(register_pmu_pm_ops);
+#endif
+
 static int
 pmu_ioctl(struct inode * inode, struct file *filp,
 		     u_int cmd, u_long arg)
@@ -2567,29 +2552,19 @@ pmu_ioctl(struct inode * inode, struct f
 
 	switch (cmd) {
 #if defined(CONFIG_PM) && defined(CONFIG_PPC32)
+	/* just provided for compatibility */
 	case PMU_IOC_SLEEP:
 		if (!capable(CAP_SYS_ADMIN))
 			return -EACCES;
-		if (sleep_in_progress)
-			return -EBUSY;
-		sleep_in_progress = 1;
-		switch (pmu_kind) {
-		case PMU_OHARE_BASED:
-			error = powerbook_sleep_3400();
-			break;
-		case PMU_HEATHROW_BASED:
-		case PMU_PADDINGTON_BASED:
-			error = powerbook_sleep_grackle();
-			break;
-		case PMU_KEYLARGO_BASED:
-			error = powerbook_sleep_Core99();
-			break;
-		default:
-			error = -ENOSYS;
-		}
-		sleep_in_progress = 0;
+		printk(KERN_INFO "via-pmu: the PMU_IOC_SLEEP ioctl is deprecated.\n");
+		printk(KERN_INFO "via-pmu: use \"echo mem > /sys/power/state\" instead!\n");
+		printk(KERN_INFO "via-pmu: this ioctl will be removed soon.\n");
+		error = pm_suspend(PM_SUSPEND_MEM);
 		break;
 	case PMU_IOC_CAN_SLEEP:
+		printk(KERN_INFO "via-pmu: the PMU_IOC_CAN_SLEEP ioctl is deprecated.\n");
+		printk(KERN_INFO "via-pmu: use \"grep mem /sys/power/state\" instead!\n");
+		printk(KERN_INFO "via-pmu: this ioctl will be removed soon.\n");
 		if (pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,-1) < 0)
 			return put_user(0, argp);
 		else
@@ -2602,9 +2577,6 @@ pmu_ioctl(struct inode * inode, struct f
 	{
 		int brightness;
 
-		if (sleep_in_progress)
-			return -EBUSY;
-
 		brightness = pmac_backlight_get_legacy_brightness();
 		if (brightness < 0)
 			return brightness;
@@ -2616,9 +2588,6 @@ pmu_ioctl(struct inode * inode, struct f
 	{
 		int brightness;
 
-		if (sleep_in_progress)
-			return -EBUSY;
-
 		error = get_user(brightness, argp);
 		if (error)
 			return error;
--- linux-2.6.orig/Documentation/feature-removal-schedule.txt	2007-04-27 23:56:19.196021121 +0200
+++ linux-2.6/Documentation/feature-removal-schedule.txt	2007-04-28 00:52:59.482010117 +0200
@@ -302,3 +302,13 @@ Why:	Code was merged, then submitter imm
 Who:	David S. Miller <davem@davemloft.net>
 
 ---------------------------
+
+What:	/dev/pmu suspend/can-suspend ioctls
+When:	2.6.24
+Files:	drivers/macintosh/via-pmu.c
+Why:	powermac supports proper generic pm_ops now and can suspend with
+	"echo mem > /sys/power/state" instead of the ioctl, checking if
+	it can suspend can be done by reading /sys/power/state.
+Who:	Johannes Berg <johannes@sipsolutions.net>
+
+---------------------------

Re: [PATCH] powermac: proper sleep management

[Paul Mackerras]

Johannes Berg writes:

> +What:	/dev/pmu suspend/can-suspend ioctls
> +When:	2.6.24

No way.  You're proposing to remove a kernel/user ABI which is
actively being used by many users.  It needs to stick around for at
least 2 years before we make it non-functional.

Paul.

Re: [PATCH] powermac: proper sleep management

[Benjamin Herrenschmidt]

On Sat, 2007-04-28 at 09:49 +0200, Johannes Berg wrote:
> After having removed the power management ops from powermac completely, this
> patch adds them back for PMU based machines, directly in the PMU driver.
> This finally allows suspending via /sys/power/state on powerbooks.
> 
> The patch also replaces the PMU ioctl with a simple call to
> pm_suspend(PM_SUSPEND_MEM) and puts the sleep-related PMU ioctls onto the
> feature-removal schedule.

What happened to moving that code out of the PMU driver ?

Cheers,
Ben.

Re: [PATCH] powermac: proper sleep management

[Paul Mackerras]

Johannes Berg writes:

> After having removed the power management ops from powermac completely, this
> patch adds them back for PMU based machines, directly in the PMU driver.
> This finally allows suspending via /sys/power/state on powerbooks.

I would very much rather that we keep the existing code path intact
and add the code to suspend via /sys/power/state as a separate code
path.  I am not confident enough about the state of the generic
suspend/resume code to commit to using it as the only way to suspend
to RAM, given how many bugs it seems to have (as evidenced by the
number of suspend-related bugs in Adrian Bunk's regression lists, for
instance).

I don't mind code being factored out into helpers, or moved from one
file to another, as long as the control flow for the old sleep ioctl
remains essentially unchanged, for now.

Paul.

Re: [PATCH] powermac: proper sleep management

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 235 bytes --]

On Sat, 2007-04-28 at 18:38 +1000, Benjamin Herrenschmidt wrote:

> What happened to moving that code out of the PMU driver ?

It was a separate patch and also buggy and had feature regressions. So I
left it for now.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH] powermac: proper sleep management

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 480 bytes --]

On Sat, 2007-04-28 at 18:08 +1000, Paul Mackerras wrote:
> Johannes Berg writes:
> 
> > +What:	/dev/pmu suspend/can-suspend ioctls
> > +When:	2.6.24
> 
> No way.  You're proposing to remove a kernel/user ABI which is
> actively being used by many users.  It needs to stick around for at
> least 2 years before we make it non-functional.

Heh, ok, let's push the timeline out then. Don't really care anyway
since the feature now has about 10 lines of code.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH] powermac: proper sleep management

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 998 bytes --]

On Sat, 2007-04-28 at 22:01 +1000, Paul Mackerras wrote:

> I would very much rather that we keep the existing code path intact
> and add the code to suspend via /sys/power/state as a separate code
> path. 

Are you serious? The only thing the generic code does here is invoke our
stuff in the right order. That shouldn't be too much of a constraint. We
don't even have multiple CPUs or anything that was recently giving
problems.

> I am not confident enough about the state of the generic
> suspend/resume code to commit to using it as the only way to suspend
> to RAM, given how many bugs it seems to have (as evidenced by the
> number of suspend-related bugs in Adrian Bunk's regression lists, for
> instance).

Of the suspend related bugs Adrian's regression list most aren't really
related to the generic code we rely on, especially since all machines
this code applies to are single-CPU. Besides, relying on it is the only
way to ensure it stays compatible.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH] powermac: proper sleep management

[Paul Mackerras]

Johannes Berg writes:

> Are you serious? The only thing the generic code does here is invoke our
> stuff in the right order. That shouldn't be too much of a constraint. We

Yes I'm serious.  At a quick look, the generic code is calling
freeze_processes and shrink_all_memory, and I don't see where it's
doing a sync.  I really don't like the process freezer; as Linus
pointed out, it has caused more deadlocks than it solved.

Also, you're now calling pbook_alloc_pci_save after interrupts are
disabled, and it does a kmalloc(..., GFP_KERNEL).  Oops.

Part of the problem is exactly what Linus pointed out: that the
generic code tries to use the same code paths for suspend to RAM and
suspend to disk, but they are two totally different things.

I have no objection to adding code to enable the generic code to do
the (mostly) right thing when you write "mem" into /sys/power/state.
I have no objection to code being refactored to eliminate
duplication.  All I ask is that the PMU ioctls continue to do
essentially the same things in the same order.

Paul.

Re: [PATCH] powermac: proper sleep management

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 2921 bytes --]

On Mon, 2007-04-30 at 15:31 +1000, Paul Mackerras wrote:

> At a quick look, the generic code is calling
> freeze_processes and shrink_all_memory, and I don't see where it's
> doing a sync.

The sync is done after userspace has been frozen, in freeze_processes. I
agree that shrink_all_memory is bogus, I'll talk to Rafael about it;
we're working on splitting out the hibernate and suspend code paths
anyway so it should be possible to remove this cleanly. It should also
be easy to invoke it only for suspend to disk for now.

> Also, you're now calling pbook_alloc_pci_save after interrupts are
> disabled, and it does a kmalloc(..., GFP_KERNEL).  Oops.

Oops, yes, thanks for pointing that out, it needs to go into the prepare
callback. I just sent an updated patch that moves it there and verified
that no other code was misplaced.

> Part of the problem is exactly what Linus pointed out: that the
> generic code tries to use the same code paths for suspend to RAM and
> suspend to disk, but they are two totally different things.

Actually, the stuff Linus pointed out in the recent thread was about
device drivers and the current PMU code uses the same driver/device
suspend routines that the generic code uses. No difference there.

> I have no objection to adding code to enable the generic code to do
> the (mostly) right thing when you write "mem" into /sys/power/state.
> I have no objection to code being refactored to eliminate
> duplication.  All I ask is that the PMU ioctls continue to do
> essentially the same things in the same order.

I disagree with that, it'll get us no closer to keeping the generic code
working for us. It does in fact work now, I've been using it for a long
time and a few other people have also tried on older powerbooks. It's
also very hard to convince anybody that we need changes in the generic
code if at the same time we do our own stuff because "the generic code
(you wrote) is not good enough for us anyway."

Considering the options from userspace, there are currently 3 programs
(that I know of) that actually use the ioctl:
 * hal via some helper or via pm-utils (almost all gnome/kde programs
   use it)
 * pbbuttonsd
 * pmud (?)

I know that the hal/pm-utils folks would love to get rid of the pmu
specific stuff and I think this and the battery (?) are the last things.
I don't even pretend to know what pbbuttonsd/pmud will do. Regardless of
that, however, the vast majority of users of modern desktop distros will
be using hal and hence the new code simply because that is integrated
into their desktop. If we fragment the user base into these two sets
we're making life more difficult because suddenly "sleep" or "suspend to
RAM" no longer identifies uniquely what the user did. Also, if one of
them breaks people will switch to the other one instead of helping
identify and fix the problem.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH] powermac: proper sleep management

[Paul Mackerras]

Johannes Berg writes:

> I disagree with that, it'll get us no closer to keeping the generic code
> working for us. It does in fact work now, I've been using it for a long
> time and a few other people have also tried on older powerbooks. It's
> also very hard to convince anybody that we need changes in the generic
> code if at the same time we do our own stuff because "the generic code
> (you wrote) is not good enough for us anyway."

I think that once we have /sys/power/state working the distros will
start using it.  I want the old way to keep doing what it was doing
while the new way is tested and the bugs are shaken out.

Bottom line is I'm not going to apply a patch that makes pmu_ioctl
call pm_suspend.

Paul.

Re: [PATCH] powermac: proper sleep management

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 230 bytes --]

On Tue, 2007-05-01 at 22:14 +1000, Paul Mackerras wrote:

> Bottom line is I'm not going to apply a patch that makes pmu_ioctl
> call pm_suspend.

Alright, I'll drop the patch. What about suspend to disk for G5?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

[PATCH] powermac: proper sleep management

[Johannes Berg]

This adds platform_suspend_ops for PMU based machines, directly in
the PMU driver. This finally allows suspending via /sys/power/state
on powerbooks.

The patch also replaces the PMU ioctl with a simple call to
pm_suspend(PM_SUSPEND_MEM) and puts the sleep-related PMU ioctls onto
the feature-removal schedule, to be removed in early 2010 (just
over two years from now).

Additionally, it cleans up some debug code.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 Documentation/feature-removal-schedule.txt |   10 
 drivers/macintosh/Kconfig                  |   12 
 drivers/macintosh/via-pmu.c                |  462 +++++++++++++----------------
 3 files changed, 241 insertions(+), 243 deletions(-)

--- everything.orig/drivers/macintosh/via-pmu.c	2007-11-13 20:16:51.196263726 +0100
+++ everything/drivers/macintosh/via-pmu.c	2007-11-13 20:24:29.056252331 +0100
@@ -10,13 +10,13 @@
  *
  * Copyright (C) 1998 Paul Mackerras and Fabio Riccardi.
  * Copyright (C) 2001-2002 Benjamin Herrenschmidt
+ * Copyright (C) 2006-2007 Johannes Berg
  *
  * THIS DRIVER IS BECOMING A TOTAL MESS !
  *  - Cleanup atomically disabling reply to PMU events after
  *    a sleep or a freq. switch
- *  - Move sleep code out of here to pmac_pm, merge into new
- *    common PM infrastructure
- *  - Save/Restore PCI space properly
+ *  - check if powerbook 3400 really needs the extra PCI
+ *    save/restore code we have
  *
  */
 #include <stdarg.h>
@@ -64,7 +64,7 @@
 #include "via-pmu-event.h"
 
 /* Some compile options */
-#define DEBUG_SLEEP
+#undef DEBUG_SLEEP
 
 /* Misc minor number allocated for /dev/pmu */
 #define PMU_MINOR		154
@@ -149,12 +149,9 @@ static spinlock_t pmu_lock;
 static u8 pmu_intr_mask;
 static int pmu_version;
 static int drop_interrupts;
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_PPC32)
+#if defined(CONFIG_SUSPEND) && defined(CONFIG_PPC32)
 static int option_lid_wakeup = 1;
-#endif /* CONFIG_PM_SLEEP && CONFIG_PPC32 */
-#if (defined(CONFIG_PM_SLEEP)&&defined(CONFIG_PPC32))||defined(CONFIG_PMAC_BACKLIGHT_LEGACY)
-static int sleep_in_progress;
-#endif
+#endif /* CONFIG_SUSPEND && CONFIG_PPC32 */
 static unsigned long async_req_locks;
 static unsigned int pmu_irq_stats[11];
 
@@ -220,7 +217,7 @@ extern void enable_kernel_fp(void);
 
 #ifdef DEBUG_SLEEP
 int pmu_polled_request(struct adb_request *req);
-int pmu_wink(struct adb_request *req);
+void pmu_blink(int n);
 #endif
 
 /*
@@ -871,7 +868,7 @@ proc_read_options(char *page, char **sta
 {
 	char *p = page;
 
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_PPC32)
+#if defined(CONFIG_SUSPEND) && defined(CONFIG_PPC32)
 	if (pmu_kind == PMU_KEYLARGO_BASED &&
 	    pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,-1) >= 0)
 		p += sprintf(p, "lid_wakeup=%d\n", option_lid_wakeup);
@@ -912,7 +909,7 @@ proc_write_options(struct file *file, co
 	*(val++) = 0;
 	while(*val == ' ')
 		val++;
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_PPC32)
+#if defined(CONFIG_SUSPEND) && defined(CONFIG_PPC32)
 	if (pmu_kind == PMU_KEYLARGO_BASED &&
 	    pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,-1) >= 0)
 		if (!strcmp(label, "lid_wakeup"))
@@ -1718,7 +1715,7 @@ pmu_present(void)
 	return via != 0;
 }
 
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_PPC32)
+#if defined(CONFIG_SUSPEND) && defined(CONFIG_PPC32)
 /*
  * This struct is used to store config register values for
  * PCI devices which may get powered off when we sleep.
@@ -1821,42 +1818,6 @@ pbook_pci_restore(void)
 	}
 }
 
-#ifdef DEBUG_SLEEP
-/* N.B. This doesn't work on the 3400 */
-void 
-pmu_blink(int n)
-{
-	struct adb_request req;
-
-	memset(&req, 0, sizeof(req));
-
-	for (; n > 0; --n) {
-		req.nbytes = 4;
-		req.done = NULL;
-		req.data[0] = 0xee;
-		req.data[1] = 4;
-		req.data[2] = 0;
-		req.data[3] = 1;
-		req.reply[0] = ADB_RET_OK;
-		req.reply_len = 1;
-		req.reply_expected = 0;
-		pmu_polled_request(&req);
-		mdelay(50);
-		req.nbytes = 4;
-		req.done = NULL;
-		req.data[0] = 0xee;
-		req.data[1] = 4;
-		req.data[2] = 0;
-		req.data[3] = 0;
-		req.reply[0] = ADB_RET_OK;
-		req.reply_len = 1;
-		req.reply_expected = 0;
-		pmu_polled_request(&req);
-		mdelay(50);
-	}
-	mdelay(50);
-}
-#endif
 
 /*
  * Put the powerbook to sleep.
@@ -1894,122 +1855,6 @@ restore_via_state(void)
 
 extern void pmu_backlight_set_sleep(int sleep);
 
-static int
-pmac_suspend_devices(void)
-{
-	int ret;
-
-	pm_prepare_console();
-	
-	/* Sync the disks. */
-	/* XXX It would be nice to have some way to ensure that
-	 * nobody is dirtying any new buffers while we wait. That
-	 * could be achieved using the refrigerator for processes
-	 * that swsusp uses
-	 */
-	sys_sync();
-
-	/* Send suspend call to devices, hold the device core's dpm_sem */
-	ret = device_suspend(PMSG_SUSPEND);
-	if (ret) {
-		printk(KERN_ERR "Driver sleep failed\n");
-		return -EBUSY;
-	}
-
-#ifdef CONFIG_PMAC_BACKLIGHT
-	/* Tell backlight code not to muck around with the chip anymore */
-	pmu_backlight_set_sleep(1);
-#endif
-
-	/* Call platform functions marked "on sleep" */
-	pmac_pfunc_i2c_suspend();
-	pmac_pfunc_base_suspend();
-
-	/* Stop preemption */
-	preempt_disable();
-
-	/* Make sure the decrementer won't interrupt us */
-	asm volatile("mtdec %0" : : "r" (0x7fffffff));
-	/* Make sure any pending DEC interrupt occurring while we did
-	 * the above didn't re-enable the DEC */
-	mb();
-	asm volatile("mtdec %0" : : "r" (0x7fffffff));
-
-	/* We can now disable MSR_EE. This code of course works properly only
-	 * on UP machines... For SMP, if we ever implement sleep, we'll have to
-	 * stop the "other" CPUs way before we do all that stuff.
-	 */
-	local_irq_disable();
-
-	/* Broadcast power down irq
-	 * This isn't that useful in most cases (only directly wired devices can
-	 * use this but still... This will take care of sysdev's as well, so
-	 * we exit from here with local irqs disabled and PIC off.
-	 */
-	ret = device_power_down(PMSG_SUSPEND);
-	if (ret) {
-		wakeup_decrementer();
-		local_irq_enable();
-		preempt_enable();
-		device_resume();
-		printk(KERN_ERR "Driver powerdown failed\n");
-		return -EBUSY;
-	}
-
-	/* Wait for completion of async requests */
-	while (!batt_req.complete)
-		pmu_poll();
-
-	/* Giveup the lazy FPU & vec so we don't have to back them
-	 * up from the low level code
-	 */
-	enable_kernel_fp();
-
-#ifdef CONFIG_ALTIVEC
-	if (cpu_has_feature(CPU_FTR_ALTIVEC))
-		enable_kernel_altivec();
-#endif /* CONFIG_ALTIVEC */
-
-	return 0;
-}
-
-static int
-pmac_wakeup_devices(void)
-{
-	mdelay(100);
-
-#ifdef CONFIG_PMAC_BACKLIGHT
-	/* Tell backlight code it can use the chip again */
-	pmu_backlight_set_sleep(0);
-#endif
-
-	/* Power back up system devices (including the PIC) */
-	device_power_up();
-
-	/* Force a poll of ADB interrupts */
-	adb_int_pending = 1;
-	via_pmu_interrupt(0, NULL);
-
-	/* Restart jiffies & scheduling */
-	wakeup_decrementer();
-
-	/* Re-enable local CPU interrupts */
-	local_irq_enable();
-	mdelay(10);
-	preempt_enable();
-
-	/* Call platform functions marked "on wake" */
-	pmac_pfunc_base_resume();
-	pmac_pfunc_i2c_resume();
-
-	/* Resume devices */
-	device_resume();
-
-	pm_restore_console();
-
-	return 0;
-}
-
 #define	GRACKLE_PM	(1<<7)
 #define GRACKLE_DOZE	(1<<5)
 #define	GRACKLE_NAP	(1<<4)
@@ -2020,19 +1865,12 @@ static int powerbook_sleep_grackle(void)
 	unsigned long save_l2cr;
 	unsigned short pmcr1;
 	struct adb_request req;
-	int ret;
 	struct pci_dev *grackle;
 
 	grackle = pci_get_bus_and_slot(0, 0);
 	if (!grackle)
 		return -ENODEV;
 
-	ret = pmac_suspend_devices();
-	if (ret) {
-		printk(KERN_ERR "Sleep rejected by devices\n");
-		return ret;
-	}
-	
 	/* Turn off various things. Darwin does some retry tests here... */
 	pmu_request(&req, NULL, 2, PMU_POWER_CTRL0, PMU_POW0_OFF|PMU_POW0_HARD_DRIVE);
 	pmu_wait_complete(&req);
@@ -2095,8 +1933,6 @@ static int powerbook_sleep_grackle(void)
 			PMU_POW_ON|PMU_POW_BACKLIGHT|PMU_POW_CHARGER|PMU_POW_IRLED|PMU_POW_MEDIABAY);
 	pmu_wait_complete(&req);
 
-	pmac_wakeup_devices();
-
 	return 0;
 }
 
@@ -2106,7 +1942,6 @@ powerbook_sleep_Core99(void)
 	unsigned long save_l2cr;
 	unsigned long save_l3cr;
 	struct adb_request req;
-	int ret;
 	
 	if (pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,-1) < 0) {
 		printk(KERN_ERR "Sleep mode not supported on this machine\n");
@@ -2116,12 +1951,6 @@ powerbook_sleep_Core99(void)
 	if (num_online_cpus() > 1 || cpu_is_offline(0))
 		return -EAGAIN;
 
-	ret = pmac_suspend_devices();
-	if (ret) {
-		printk(KERN_ERR "Sleep rejected by devices\n");
-		return ret;
-	}
-
 	/* Stop environment and ADB interrupts */
 	pmu_request(&req, NULL, 2, PMU_SET_INTR_MASK, 0);
 	pmu_wait_complete(&req);
@@ -2192,41 +2021,24 @@ powerbook_sleep_Core99(void)
 	/* Restore LPJ, cpufreq will adjust the cpu frequency */
 	loops_per_jiffy /= 2;
 
-	pmac_wakeup_devices();
-
 	return 0;
 }
 
 #define PB3400_MEM_CTRL		0xf8000000
 #define PB3400_MEM_CTRL_SLEEP	0x70
 
+static void __iomem *pb3400_mem_ctrl;
+
 static int
 powerbook_sleep_3400(void)
 {
-	int ret, i, x;
+	int i, x;
 	unsigned int hid0;
 	unsigned long p;
 	struct adb_request sleep_req;
-	void __iomem *mem_ctrl;
 	unsigned int __iomem *mem_ctrl_sleep;
 
-	/* first map in the memory controller registers */
-	mem_ctrl = ioremap(PB3400_MEM_CTRL, 0x100);
-	if (mem_ctrl == NULL) {
-		printk("powerbook_sleep_3400: ioremap failed\n");
-		return -ENOMEM;
-	}
-	mem_ctrl_sleep = mem_ctrl + PB3400_MEM_CTRL_SLEEP;
-
-	/* Allocate room for PCI save */
-	pbook_alloc_pci_save();
-
-	ret = pmac_suspend_devices();
-	if (ret) {
-		pbook_free_pci_save();
-		printk(KERN_ERR "Sleep rejected by devices\n");
-		return ret;
-	}
+	mem_ctrl_sleep = pb3400_mem_ctrl + PB3400_MEM_CTRL_SLEEP;
 
 	/* Save the state of PCI config space for some slots */
 	pbook_pci_save();
@@ -2271,14 +2083,10 @@ powerbook_sleep_3400(void)
 	while (asleep)
 		mb();
 
-	pmac_wakeup_devices();
-	pbook_free_pci_save();
-	iounmap(mem_ctrl);
-
 	return 0;
 }
 
-#endif /* CONFIG_PM_SLEEP && CONFIG_PPC32 */
+#endif /* CONFIG_SUSPEND && CONFIG_PPC32 */
 
 /*
  * Support for /dev/pmu device
@@ -2451,6 +2259,157 @@ pmu_release(struct inode *inode, struct 
 	return 0;
 }
 
+#if defined(CONFIG_SUSPEND) && defined(CONFIG_PPC32)
+static int powerbook_prepare_sleep(void)
+{
+	if (pmu_kind == PMU_OHARE_BASED) {
+		/* first map in the memory controller registers */
+		pb3400_mem_ctrl = ioremap(PB3400_MEM_CTRL, 0x100);
+		if (!pb3400_mem_ctrl) {
+			printk(KERN_ERR "powerbook_sleep_3400: "
+			       "ioremap failed\n");
+			return -ENOMEM;
+		}
+
+		/* Allocate room for PCI save */
+		pbook_alloc_pci_save();
+	}
+
+	return 0;
+}
+
+/*
+ * overrides the weak arch_suspend_disable_irqs in kernel/power/main.c
+ *
+ * XXX: Once Scott Wood's patch is merged, this needs to use the ppc_md
+ *	hooks that patch adds!
+ */
+void arch_suspend_disable_irqs(void)
+{
+#ifdef CONFIG_PMAC_BACKLIGHT
+	/* Tell backlight code not to muck around with the chip anymore */
+	pmu_backlight_set_sleep(1);
+#endif
+
+	/* Call platform functions marked "on sleep" */
+	pmac_pfunc_i2c_suspend();
+	pmac_pfunc_base_suspend();
+
+	/* Stop preemption */
+	preempt_disable();
+
+	/* Make sure the decrementer won't interrupt us */
+	asm volatile("mtdec %0" : : "r" (0x7fffffff));
+	/* Make sure any pending DEC interrupt occurring while we did
+	 * the above didn't re-enable the DEC */
+	mb();
+	asm volatile("mtdec %0" : : "r" (0x7fffffff));
+
+	local_irq_disable();
+}
+
+static int powerbook_sleep(suspend_state_t state)
+{
+	int error = 0;
+
+	/* Wait for completion of async requests */
+	while (!batt_req.complete)
+		pmu_poll();
+
+	/* Giveup the lazy FPU & vec so we don't have to back them
+	 * up from the low level code
+	 */
+	enable_kernel_fp();
+
+#ifdef CONFIG_ALTIVEC
+	if (cpu_has_feature(CPU_FTR_ALTIVEC))
+		enable_kernel_altivec();
+#endif /* CONFIG_ALTIVEC */
+
+	switch (pmu_kind) {
+	case PMU_OHARE_BASED:
+		error = powerbook_sleep_3400();
+		break;
+	case PMU_HEATHROW_BASED:
+	case PMU_PADDINGTON_BASED:
+		error = powerbook_sleep_grackle();
+		break;
+	case PMU_KEYLARGO_BASED:
+		error = powerbook_sleep_Core99();
+		break;
+	default:
+		return -ENOSYS;
+	}
+
+	if (error)
+		return error;
+
+	mdelay(100);
+
+#ifdef CONFIG_PMAC_BACKLIGHT
+	/* Tell backlight code it can use the chip again */
+	pmu_backlight_set_sleep(0);
+#endif
+
+	return 0;
+}
+
+/*
+ * overrides the weak arch_suspend_enable_irqs in kernel/power/main.c
+ *
+ * XXX: Once Scott Wood's patch is merged, this needs to use the ppc_md
+ *	hooks that patch adds!
+ */
+void arch_suspend_enable_irqs(void)
+{
+	/* Force a poll of ADB interrupts */
+	adb_int_pending = 1;
+	via_pmu_interrupt(0, NULL);
+
+	/* Restart jiffies & scheduling */
+	wakeup_decrementer();
+
+	/* Re-enable local CPU interrupts */
+	local_irq_enable();
+	mdelay(10);
+	preempt_enable();
+
+	/* Call platform functions marked "on wake" */
+	pmac_pfunc_base_resume();
+	pmac_pfunc_i2c_resume();
+}
+
+static void powerbook_finish_sleep(void)
+{
+	if (pmu_kind == PMU_OHARE_BASED) {
+		pbook_free_pci_save();
+		iounmap(pb3400_mem_ctrl);
+	}
+}
+
+static int pmu_sleep_valid(suspend_state_t state)
+{
+	return state == PM_SUSPEND_MEM
+		&& (pmac_call_feature(PMAC_FTR_SLEEP_STATE, NULL, 0, -1) >= 0);
+}
+
+static struct platform_suspend_ops pmu_pm_ops = {
+	.prepare = powerbook_prepare_sleep,
+	.enter = powerbook_sleep,
+	.finish = powerbook_finish_sleep,
+	.valid = pmu_sleep_valid,
+};
+
+static int register_pmu_pm_ops(void)
+{
+	suspend_set_ops(&pmu_pm_ops);
+
+	return 0;
+}
+
+device_initcall(register_pmu_pm_ops);
+#endif
+
 static int
 pmu_ioctl(struct inode * inode, struct file *filp,
 		     u_int cmd, u_long arg)
@@ -2459,35 +2418,30 @@ pmu_ioctl(struct inode * inode, struct f
 	int error = -EINVAL;
 
 	switch (cmd) {
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_PPC32)
+#if defined(CONFIG_DEPRECATED_PMU_SLEEP_IOCTL)
 	case PMU_IOC_SLEEP:
 		if (!capable(CAP_SYS_ADMIN))
 			return -EACCES;
-		if (sleep_in_progress)
-			return -EBUSY;
-		sleep_in_progress = 1;
-		switch (pmu_kind) {
-		case PMU_OHARE_BASED:
-			error = powerbook_sleep_3400();
-			break;
-		case PMU_HEATHROW_BASED:
-		case PMU_PADDINGTON_BASED:
-			error = powerbook_sleep_grackle();
-			break;
-		case PMU_KEYLARGO_BASED:
-			error = powerbook_sleep_Core99();
-			break;
-		default:
-			error = -ENOSYS;
-		}
-		sleep_in_progress = 0;
+		printk(KERN_INFO
+		       "via-pmu: the PMU_IOC_SLEEP ioctl is deprecated.\n");
+		printk(KERN_INFO "via-pmu: use \"echo mem >"
+		       " /sys/power/state\"  instead!\n");
+		printk(KERN_INFO
+		       "via-pmu: this ioctl will be removed soon.\n");
+		error = pm_suspend(PM_SUSPEND_MEM);
 		break;
 	case PMU_IOC_CAN_SLEEP:
+		printk(KERN_INFO
+		       "via-pmu: the PMU_IOC_CAN_SLEEP ioctl is deprecated.\n");
+		printk(KERN_INFO
+		       "via-pmu: use \"grep mem /sys/power/state\" instead!\n");
+		printk(KERN_INFO
+		       "via-pmu: this ioctl will be removed soon.\n");
 		if (pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,-1) < 0)
 			return put_user(0, argp);
 		else
 			return put_user(1, argp);
-#endif /* CONFIG_PM_SLEEP && CONFIG_PPC32 */
+#endif /* CONFIG_DEPRECATED_PMU_SLEEP_IOCTL */
 
 #ifdef CONFIG_PMAC_BACKLIGHT_LEGACY
 	/* Compatibility ioctl's for backlight */
@@ -2495,9 +2449,6 @@ pmu_ioctl(struct inode * inode, struct f
 	{
 		int brightness;
 
-		if (sleep_in_progress)
-			return -EBUSY;
-
 		brightness = pmac_backlight_get_legacy_brightness();
 		if (brightness < 0)
 			return brightness;
@@ -2509,9 +2460,6 @@ pmu_ioctl(struct inode * inode, struct f
 	{
 		int brightness;
 
-		if (sleep_in_progress)
-			return -EBUSY;
-
 		error = get_user(brightness, argp);
 		if (error)
 			return error;
@@ -2638,15 +2586,43 @@ pmu_polled_request(struct adb_request *r
 	local_irq_restore(flags);
 	return 0;
 }
-#endif /* DEBUG_SLEEP */
 
+/* N.B. This doesn't work on the 3400 */
+void pmu_blink(int n)
+{
+	struct adb_request req;
 
-/* FIXME: This is a temporary set of callbacks to enable us
- * to do suspend-to-disk.
- */
+	memset(&req, 0, sizeof(req));
 
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_PPC32)
+	for (; n > 0; --n) {
+		req.nbytes = 4;
+		req.done = NULL;
+		req.data[0] = 0xee;
+		req.data[1] = 4;
+		req.data[2] = 0;
+		req.data[3] = 1;
+		req.reply[0] = ADB_RET_OK;
+		req.reply_len = 1;
+		req.reply_expected = 0;
+		pmu_polled_request(&req);
+		mdelay(50);
+		req.nbytes = 4;
+		req.done = NULL;
+		req.data[0] = 0xee;
+		req.data[1] = 4;
+		req.data[2] = 0;
+		req.data[3] = 0;
+		req.reply[0] = ADB_RET_OK;
+		req.reply_len = 1;
+		req.reply_expected = 0;
+		pmu_polled_request(&req);
+		mdelay(50);
+	}
+	mdelay(50);
+}
+#endif /* DEBUG_SLEEP */
 
+#if defined(CONFIG_SUSPEND) && defined(CONFIG_PPC32)
 int pmu_sys_suspended;
 
 static int pmu_sys_suspend(struct sys_device *sysdev, pm_message_t state)
@@ -2680,7 +2656,7 @@ static int pmu_sys_resume(struct sys_dev
 	return 0;
 }
 
-#endif /* CONFIG_PM_SLEEP && CONFIG_PPC32 */
+#endif /* CONFIG_SUSPEND && CONFIG_PPC32 */
 
 static ssize_t show_kind(struct sys_device *sysdev, char *buf)
 {
@@ -2736,10 +2712,10 @@ static struct sys_device device_pmu = {
 };
 
 static struct sysdev_driver driver_pmu = {
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_PPC32)
+#if defined(CONFIG_SUSPEND) && defined(CONFIG_PPC32)
 	.suspend	= &pmu_sys_suspend,
 	.resume		= &pmu_sys_resume,
-#endif /* CONFIG_PM_SLEEP && CONFIG_PPC32 */
+#endif /* CONFIG_SUSPEND && CONFIG_PPC32 */
 	.add		= &pmu_sys_add,
 };
 
@@ -2775,10 +2751,10 @@ EXPORT_SYMBOL(pmu_wait_complete);
 EXPORT_SYMBOL(pmu_suspend);
 EXPORT_SYMBOL(pmu_resume);
 EXPORT_SYMBOL(pmu_unlock);
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_PPC32)
+#if defined(CONFIG_SUSPEND) && defined(CONFIG_PPC32)
 EXPORT_SYMBOL(pmu_enable_irled);
 EXPORT_SYMBOL(pmu_battery_count);
 EXPORT_SYMBOL(pmu_batteries);
 EXPORT_SYMBOL(pmu_power_flags);
-#endif /* CONFIG_PM_SLEEP && CONFIG_PPC32 */
+#endif /* CONFIG_SUSPEND && CONFIG_PPC32 */
 
--- everything.orig/Documentation/feature-removal-schedule.txt	2007-11-13 20:16:51.276261773 +0100
+++ everything/Documentation/feature-removal-schedule.txt	2007-11-13 20:17:08.966262261 +0100
@@ -350,3 +350,13 @@ Why:	The PMU information can be obtained
 Who:	Johannes Berg <johannes@sipsolutions.net>
 
 ---------------------------
+
+What:	/dev/pmu suspend/can-suspend ioctls
+When:	January 2010
+Files:	drivers/macintosh/via-pmu.c
+Why:	powermac supports proper generic pm_ops now and can suspend with
+	"echo mem > /sys/power/state" instead of the ioctl, checking if
+	it can suspend can be done by reading /sys/power/state.
+Who:	Johannes Berg <johannes@sipsolutions.net>
+
+---------------------------
--- everything.orig/drivers/macintosh/Kconfig	2007-11-13 20:16:51.226265462 +0100
+++ everything/drivers/macintosh/Kconfig	2007-11-13 20:17:08.966262261 +0100
@@ -98,6 +98,18 @@ config DEPRECATED_PMU_INFO_IOCTLS
 
 	  If in doubt, say Y even if you will not use the ioctl.
 
+config DEPRECATED_PMU_SLEEP_IOCTL
+	bool "Support deprecated PMU sleep ioctl"
+	depends on ADB_PMU && SUSPEND && PPC32
+	default y
+	help
+	  The PMU code used to support suspending the machine via an ioctl
+	  before Linux had a generic suspend framework. Now the PMU supports
+	  suspending via the generic framework, this option adds the old
+	  PMU specific ioctl to the kernel.
+
+	  If in doubt, say Y even if you will not use the ioctl.
+
 config ADB_PMU_LED
 	bool "Support for the Power/iBook front LED"
 	depends on ADB_PMU

Re: [PATCH] powermac: proper sleep management

[Paul Mackerras]

Johannes Berg writes:

> +		printk(KERN_INFO
> +		       "via-pmu: the PMU_IOC_SLEEP ioctl is deprecated.\n");
> +		printk(KERN_INFO "via-pmu: use \"echo mem >"
> +		       " /sys/power/state\"  instead!\n");
> +		printk(KERN_INFO
> +		       "via-pmu: this ioctl will be removed soon.\n");

I don't like this.  I would rather keep the ioctls indefinitely and
not make any noise about programs using them.  The ioctls are part of
the kernel/user ABI, and thus need to be preserved, especially since
it only takes about 6 or 7 lines of code to implement them.

Paul.