All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] powermac suspend fixes
@ 2007-03-19 10:53 Johannes Berg
  2007-03-19 10:53 ` [PATCH 1/5] powerpc: generic time suspend/resume code Johannes Berg
                   ` (4 more replies)
  0 siblings, 5 replies; 32+ messages in thread
From: Johannes Berg @ 2007-03-19 10:53 UTC (permalink / raw)
  To: linuxppc-dev

This patch series fixes a few things with suspend on 32-bit powermac
machines. There is a bug with suspend to disk due to the pm_ops
misunderstanding, and for suspend to ram this introduces the ability to use
/sys/power/state properly.

johannes

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/5] powerpc: generic time suspend/resume code
  2007-03-19 10:53 [PATCH 0/5] powermac suspend fixes Johannes Berg
@ 2007-03-19 10:53 ` Johannes Berg
  2007-03-19 14:47   ` Benjamin Herrenschmidt
  2007-05-02  5:02   ` Michael Ellerman
  2007-03-19 10:53 ` [PATCH 2/5] powerpc: fix suspend states again Johannes Berg
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 32+ messages in thread
From: Johannes Berg @ 2007-03-19 10:53 UTC (permalink / raw)
  To: linuxppc-dev

This patch removes the time suspend/restore code that was done through
a PMU notifier in arch/platforms/powermac/time.c.

Instead, introduce arch/powerpc/sysdev/timer.c which creates a sys
device and handles time of day suspend/resume through that.

This should probably be replaced by using the generic RTC framework
but for now it gets rid of the arcane powermac specific hack.

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

---
 arch/powerpc/Kconfig                   |    5 ++
 arch/powerpc/platforms/powermac/time.c |   38 -----------------
 arch/powerpc/sysdev/Makefile           |    3 +
 arch/powerpc/sysdev/timer.c            |   70 +++++++++++++++++++++++++++++++++
 4 files changed, 78 insertions(+), 38 deletions(-)

--- linux-2.6.orig/arch/powerpc/platforms/powermac/time.c	2007-03-19 11:47:27.312413925 +0100
+++ linux-2.6/arch/powerpc/platforms/powermac/time.c	2007-03-19 11:47:37.512413925 +0100
@@ -297,49 +297,11 @@ int __init via_calibrate_decr(void)
 }
 #endif
 
-#ifdef CONFIG_PM
-/*
- * Reset the time after a sleep.
- */
-static int
-time_sleep_notify(struct pmu_sleep_notifier *self, int when)
-{
-	static unsigned long time_diff;
-	unsigned long flags;
-	unsigned long seq;
-	struct timespec tv;
-
-	switch (when) {
-	case PBOOK_SLEEP_NOW:
-		do {
-			seq = read_seqbegin_irqsave(&xtime_lock, flags);
-			time_diff = xtime.tv_sec - pmac_get_boot_time();
-		} while (read_seqretry_irqrestore(&xtime_lock, seq, flags));
-		break;
-	case PBOOK_WAKE:
-		tv.tv_sec = pmac_get_boot_time() + time_diff;
-		tv.tv_nsec = 0;
-		do_settimeofday(&tv);
-		break;
-	}
-	return PBOOK_SLEEP_OK;
-}
-
-static struct pmu_sleep_notifier time_sleep_notifier = {
-	time_sleep_notify, SLEEP_LEVEL_MISC,
-};
-#endif /* CONFIG_PM */
-
 /*
  * Query the OF and get the decr frequency.
  */
 void __init pmac_calibrate_decr(void)
 {
-#if defined(CONFIG_PM) && defined(CONFIG_ADB_PMU)
-	/* XXX why here? */
-	pmu_register_sleep_notifier(&time_sleep_notifier);
-#endif
-
 	generic_calibrate_decr();
 
 #ifdef CONFIG_PPC32
--- linux-2.6.orig/arch/powerpc/Kconfig	2007-03-19 11:47:27.422413925 +0100
+++ linux-2.6/arch/powerpc/Kconfig	2007-03-19 11:47:37.512413925 +0100
@@ -11,6 +11,11 @@ config PPC64
 	  This option selects whether a 32-bit or a 64-bit kernel
 	  will be built.
 
+config PPC_PM_NEEDS_RTC_LIB
+	bool
+	select RTC_LIB
+	default y if PM
+
 config PPC32
 	bool
 	default y if !PPC64
--- linux-2.6.orig/arch/powerpc/sysdev/Makefile	2007-03-19 11:47:27.532413925 +0100
+++ linux-2.6/arch/powerpc/sysdev/Makefile	2007-03-19 11:47:37.512413925 +0100
@@ -14,6 +14,9 @@ obj-$(CONFIG_FSL_SOC)		+= fsl_soc.o
 obj-$(CONFIG_TSI108_BRIDGE)	+= tsi108_pci.o tsi108_dev.o
 obj-$(CONFIG_QUICC_ENGINE)	+= qe_lib/
 
+# contains only the suspend handler for time
+obj-$(CONFIG_PM)		+= timer.o
+
 ifeq ($(CONFIG_PPC_MERGE),y)
 obj-$(CONFIG_PPC_I8259)		+= i8259.o
 obj-$(CONFIG_PPC_83xx)		+= ipic.o
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/powerpc/sysdev/timer.c	2007-03-19 11:47:37.512413925 +0100
@@ -0,0 +1,70 @@
+/*
+ * Common code to keep time when machine suspends.
+ *
+ * Copyright 2007	Johannes Berg <johannes@sipsolutions.net>
+ *
+ * GPLv2
+ */
+
+#include <linux/time.h>
+#include <asm/rtc.h>
+
+static unsigned long suspend_rtc_time;
+
+/*
+ * Reset the time after a sleep.
+ */
+static int timer_resume(struct sys_device *dev)
+{
+	struct timeval tv;
+	struct timespec ts;
+	struct rtc_time cur_rtc_tm;
+	unsigned long cur_rtc_time, diff;
+
+	/* get current RTC time and convert to seconds */
+	get_rtc_time(&cur_rtc_tm);
+	rtc_tm_to_time(&cur_rtc_tm, &cur_rtc_time);
+
+	diff = cur_rtc_time - suspend_rtc_time;
+
+	/* adjust time of day by seconds that elapsed while
+	 * we were suspended */
+	do_gettimeofday(&tv);
+	ts.tv_sec = tv.tv_sec + diff;
+	ts.tv_nsec = tv.tv_usec * NSEC_PER_USEC;
+	do_settimeofday(&ts);
+
+	return 0;
+}
+
+static int timer_suspend(struct sys_device *dev, pm_message_t state)
+{
+	struct rtc_time suspend_rtc_tm;
+	WARN_ON(!ppc_md.get_rtc_time);
+
+	get_rtc_time(&suspend_rtc_tm);
+	rtc_tm_to_time(&suspend_rtc_tm, &suspend_rtc_time);
+
+	return 0;
+}
+
+static struct sysdev_class timer_sysclass = {
+	.resume = timer_resume,
+	.suspend = timer_suspend,
+	set_kset_name("timer"),
+};
+
+static struct sys_device device_timer = {
+	.id = 0,
+	.cls = &timer_sysclass,
+};
+
+static int time_init_device(void)
+{
+	int error = sysdev_class_register(&timer_sysclass);
+	if (!error)
+		error = sysdev_register(&device_timer);
+	return error;
+}
+
+device_initcall(time_init_device);

--

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 2/5] powerpc: fix suspend states again
  2007-03-19 10:53 [PATCH 0/5] powermac suspend fixes Johannes Berg
  2007-03-19 10:53 ` [PATCH 1/5] powerpc: generic time suspend/resume code Johannes Berg
@ 2007-03-19 10:53 ` Johannes Berg
  2007-03-19 14:48   ` Benjamin Herrenschmidt
  2007-03-19 16:06   ` Benjamin Herrenschmidt
  2007-03-19 10:53 ` [PATCH 3/5] powermac: disallow pmu sleep notifiers from aborting sleep Johannes Berg
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 32+ messages in thread
From: Johannes Berg @ 2007-03-19 10:53 UTC (permalink / raw)
  To: linuxppc-dev

In commit 0fba3a1f39f8b0a50b56c8b068fa52131cbc84c2 (a very long time ago,
May 2006), I fixed a bug that caused powermacs to crash when you tried
entering standby/mem suspend states.

As I'm now getting more familiar with the suspend code I notice a few
more things:
 1. we previously misunderstood what pm_ops is for, it isn't supposed to be
    for doing platform dependent suspend/resume stuff that needs to be done
    for suspend to disk (as we currently try to use it!), it is instead for
    entering platform dependent suspend states ("standby", "mem").
 2. due to the first point, we never properly save FPU and altivec states
    when suspending to disk. It probably hasn't hurt yet because the process
    that writes the "disk" to /sys/power/state uses neither and its context
    is used.

This patch addresses these points as follows:
 1. remove all pm_ops from powermac, powermac suspend to ram isn't currently
    usable via /sys/power/state but is done via the PMU instead.
 2. move the code responsible for storing FPU/altivec state into a new
    arch_prepare_suspend function (previously, this was only present for
    32-bit platforms from asm-ppc.)

A follow-on patch will create new pm_ops for via-pmu.

I removed
	set_context(current->active_mm->context.id, current->active_mm->pgd);
because
 1. it works without and
 2. I don't see the point

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

---
 arch/powerpc/kernel/Makefile            |    1 
 arch/powerpc/kernel/swsusp.c            |   26 ++++++++++++
 arch/powerpc/platforms/powermac/setup.c |   65 --------------------------------
 include/asm-powerpc/suspend.h           |    9 ++++
 4 files changed, 36 insertions(+), 65 deletions(-)

--- linux-2.6.orig/arch/powerpc/platforms/powermac/setup.c	2007-03-19 11:47:26.622413925 +0100
+++ linux-2.6/arch/powerpc/platforms/powermac/setup.c	2007-03-19 11:47:39.132413925 +0100
@@ -420,76 +420,11 @@ static void __init find_boot_device(void
 #endif
 }
 
-/* TODO: Merge the suspend-to-ram with the common code !!!
- * currently, this is a stub implementation for suspend-to-disk
- * only
- */
-
-#ifdef CONFIG_SOFTWARE_SUSPEND
-
-static int pmac_pm_prepare(suspend_state_t state)
-{
-	printk(KERN_DEBUG "%s(%d)\n", __FUNCTION__, state);
-
-	return 0;
-}
-
-static int pmac_pm_enter(suspend_state_t state)
-{
-	printk(KERN_DEBUG "%s(%d)\n", __FUNCTION__, state);
-
-	/* 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 (cur_cpu_spec->cpu_features & CPU_FTR_ALTIVEC)
-		enable_kernel_altivec();
-#endif /* CONFIG_ALTIVEC */
-
-	return 0;
-}
-
-static int pmac_pm_finish(suspend_state_t state)
-{
-	printk(KERN_DEBUG "%s(%d)\n", __FUNCTION__, state);
-
-	/* Restore userland MMU context */
-	set_context(current->active_mm->context.id, current->active_mm->pgd);
-
-	return 0;
-}
-
-static int pmac_pm_valid(suspend_state_t state)
-{
-	switch (state) {
-	case PM_SUSPEND_DISK:
-		return 1;
-	/* can't do any other states via generic mechanism yet */
-	default:
-		return 0;
-	}
-}
-
-static struct pm_ops pmac_pm_ops = {
-	.pm_disk_mode	= PM_DISK_SHUTDOWN,
-	.prepare	= pmac_pm_prepare,
-	.enter		= pmac_pm_enter,
-	.finish		= pmac_pm_finish,
-	.valid		= pmac_pm_valid,
-};
-
-#endif /* CONFIG_SOFTWARE_SUSPEND */
-
 static int initializing = 1;
 
 static int pmac_late_init(void)
 {
 	initializing = 0;
-#ifdef CONFIG_SOFTWARE_SUSPEND
-	pm_set_ops(&pmac_pm_ops);
-#endif /* CONFIG_SOFTWARE_SUSPEND */
 	return 0;
 }
 
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/include/asm-powerpc/suspend.h	2007-03-19 11:47:39.132413925 +0100
@@ -0,0 +1,9 @@
+#ifndef __ASM_POWERPC_SUSPEND_H
+#define __ASM_POWERPC_SUSPEND_H
+
+static inline int arch_prepare_suspend(void) { return 0; }
+
+void save_processor_state(void);
+static inline void restore_processor_state(void) {}
+
+#endif /* __ASM_POWERPC_SUSPEND_H */
--- linux-2.6.orig/arch/powerpc/kernel/Makefile	2007-03-19 11:47:26.692413925 +0100
+++ linux-2.6/arch/powerpc/kernel/Makefile	2007-03-19 11:47:39.172413925 +0100
@@ -36,6 +36,7 @@ obj-$(CONFIG_GENERIC_TBSYNC)	+= smp-tbsy
 obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
 obj-$(CONFIG_6xx)		+= idle_6xx.o l2cr_6xx.o cpu_setup_6xx.o
 obj-$(CONFIG_TAU)		+= tau_6xx.o
+obj-$(CONFIG_SOFTWARE_SUSPEND)	+= swsusp.o
 obj32-$(CONFIG_SOFTWARE_SUSPEND) += swsusp_32.o
 obj32-$(CONFIG_MODULES)		+= module_32.o
 
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/powerpc/kernel/swsusp.c	2007-03-19 11:47:39.172413925 +0100
@@ -0,0 +1,26 @@
+/*
+ * Common powerpc suspend code for 32 and 64 bits
+ *
+ * Copyright 2007	Johannes Berg <johannes@sipsolutions.net>
+ *
+ * GPLv2
+ */
+
+#include <asm/cputable.h>
+#include <asm/system.h>
+
+
+int save_processor_state(void)
+{
+	/* 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 (cur_cpu_spec->cpu_features & CPU_FTR_ALTIVEC)
+		enable_kernel_altivec();
+#endif /* CONFIG_ALTIVEC */
+
+	return 0;
+}

--

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 3/5] powermac: disallow pmu sleep notifiers from aborting sleep
  2007-03-19 10:53 [PATCH 0/5] powermac suspend fixes Johannes Berg
  2007-03-19 10:53 ` [PATCH 1/5] powerpc: generic time suspend/resume code Johannes Berg
  2007-03-19 10:53 ` [PATCH 2/5] powerpc: fix suspend states again Johannes Berg
@ 2007-03-19 10:53 ` Johannes Berg
  2007-03-19 14:49   ` Benjamin Herrenschmidt
  2007-03-19 10:53 ` [PATCH 4/5] powermac: proper sleep management Johannes Berg
  2007-03-19 10:53 ` [PATCH 5/5] remove dead code in via-pmu68k Johannes Berg
  4 siblings, 1 reply; 32+ messages in thread
From: Johannes Berg @ 2007-03-19 10:53 UTC (permalink / raw)
  To: linuxppc-dev

Tracing through the code, no current PMU sleep notifier can abort sleep.
Since no new PMU sleep notifiers should be added, this patch simplifies the
code and removes the ability to abort sleep.

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

---
I did this mostly because it gets rid of code we don't need (any more)

---
 drivers/macintosh/adb.c             |   42 +++++++-----------------------------
 drivers/macintosh/apm_emu.c         |   13 +++--------
 drivers/macintosh/via-pmu-led.c     |    4 ---
 drivers/macintosh/via-pmu.c         |   36 +++++-------------------------
 include/linux/pmu.h                 |   12 +---------
 sound/oss/dmasound/dmasound_awacs.c |    5 +---
 6 files changed, 24 insertions(+), 88 deletions(-)

--- linux-2.6.orig/drivers/macintosh/adb.c	2007-03-19 11:47:25.572413925 +0100
+++ linux-2.6/drivers/macintosh/adb.c	2007-03-19 11:47:40.232413925 +0100
@@ -90,7 +90,7 @@ static int autopoll_devs;
 int __adb_probe_sync;
 
 #ifdef CONFIG_PM
-static int adb_notify_sleep(struct pmu_sleep_notifier *self, int when);
+static void adb_notify_sleep(struct pmu_sleep_notifier *self, int when);
 static struct pmu_sleep_notifier adb_sleep_notifier = {
 	adb_notify_sleep,
 	SLEEP_LEVEL_ADB,
@@ -340,11 +340,9 @@ __initcall(adb_init);
 /*
  * notify clients before sleep and reset bus afterwards
  */
-int
+void
 adb_notify_sleep(struct pmu_sleep_notifier *self, int when)
 {
-	int ret;
-	
 	switch (when) {
 	case PBOOK_SLEEP_REQUEST:
 		adb_got_sleep = 1;
@@ -353,22 +351,8 @@ adb_notify_sleep(struct pmu_sleep_notifi
 		/* Stop autopoll */
 		if (adb_controller->autopoll)
 			adb_controller->autopoll(0);
-		ret = blocking_notifier_call_chain(&adb_client_list,
-				ADB_MSG_POWERDOWN, NULL);
-		if (ret & NOTIFY_STOP_MASK) {
-			up(&adb_probe_mutex);
-			return PBOOK_SLEEP_REFUSE;
-		}
-		break;
-	case PBOOK_SLEEP_REJECT:
-		if (adb_got_sleep) {
-			adb_got_sleep = 0;
-			up(&adb_probe_mutex);
-			adb_reset_bus();
-		}
-		break;
-		
-	case PBOOK_SLEEP_NOW:
+		blocking_notifier_call_chain(&adb_client_list,
+			ADB_MSG_POWERDOWN, NULL);
 		break;
 	case PBOOK_WAKE:
 		adb_got_sleep = 0;
@@ -376,14 +360,13 @@ adb_notify_sleep(struct pmu_sleep_notifi
 		adb_reset_bus();
 		break;
 	}
-	return PBOOK_SLEEP_OK;
 }
 #endif /* CONFIG_PM */
 
 static int
 do_adb_reset_bus(void)
 {
-	int ret, nret;
+	int ret;
 	
 	if (adb_controller == NULL)
 		return -ENXIO;
@@ -391,13 +374,8 @@ do_adb_reset_bus(void)
 	if (adb_controller->autopoll)
 		adb_controller->autopoll(0);
 
-	nret = blocking_notifier_call_chain(&adb_client_list,
-			ADB_MSG_PRE_RESET, NULL);
-	if (nret & NOTIFY_STOP_MASK) {
-		if (adb_controller->autopoll)
-			adb_controller->autopoll(autopoll_devs);
-		return -EBUSY;
-	}
+	blocking_notifier_call_chain(&adb_client_list,
+		ADB_MSG_PRE_RESET, NULL);
 
 	if (sleepy_trackpad) {
 		/* Let the trackpad settle down */
@@ -427,10 +405,8 @@ do_adb_reset_bus(void)
 	}
 	up(&adb_handler_sem);
 
-	nret = blocking_notifier_call_chain(&adb_client_list,
-			ADB_MSG_POST_RESET, NULL);
-	if (nret & NOTIFY_STOP_MASK)
-		return -EBUSY;
+	blocking_notifier_call_chain(&adb_client_list,
+		ADB_MSG_POST_RESET, NULL);
 	
 	return ret;
 }
--- linux-2.6.orig/drivers/macintosh/via-pmu.c	2007-03-19 11:47:25.642413925 +0100
+++ linux-2.6/drivers/macintosh/via-pmu.c	2007-03-19 11:47:40.232413925 +0100
@@ -1769,35 +1769,21 @@ EXPORT_SYMBOL(pmu_unregister_sleep_notif
 #if defined(CONFIG_PM) && defined(CONFIG_PPC32)
 
 /* Sleep is broadcast last-to-first */
-static int
-broadcast_sleep(int when, int fallback)
+static void broadcast_sleep(int when)
 {
-	int ret = PBOOK_SLEEP_OK;
 	struct list_head *list;
 	struct pmu_sleep_notifier *notifier;
 
 	for (list = sleep_notifiers.prev; list != &sleep_notifiers;
 	     list = list->prev) {
 		notifier = list_entry(list, struct pmu_sleep_notifier, list);
-		ret = notifier->notifier_call(notifier, when);
-		if (ret != PBOOK_SLEEP_OK) {
-			printk(KERN_DEBUG "sleep %d rejected by %p (%p)\n",
-			       when, notifier, notifier->notifier_call);
-			for (; list != &sleep_notifiers; list = list->next) {
-				notifier = list_entry(list, struct pmu_sleep_notifier, list);
-				notifier->notifier_call(notifier, fallback);
-			}
-			return ret;
-		}
+		notifier->notifier_call(notifier, when);
 	}
-	return ret;
 }
 
 /* Wake is broadcast first-to-last */
-static int
-broadcast_wake(void)
+static void broadcast_wake(void)
 {
-	int ret = PBOOK_SLEEP_OK;
 	struct list_head *list;
 	struct pmu_sleep_notifier *notifier;
 
@@ -1806,7 +1792,6 @@ broadcast_wake(void)
 		notifier = list_entry(list, struct pmu_sleep_notifier, list);
 		notifier->notifier_call(notifier, PBOOK_WAKE);
 	}
-	return ret;
 }
 
 /*
@@ -2013,12 +1998,8 @@ pmac_suspend_devices(void)
 
 	pm_prepare_console();
 	
-	/* Notify old-style device drivers & userland */
-	ret = broadcast_sleep(PBOOK_SLEEP_REQUEST, PBOOK_SLEEP_REJECT);
-	if (ret != PBOOK_SLEEP_OK) {
-		printk(KERN_ERR "Sleep rejected by drivers\n");
-		return -EBUSY;
-	}
+	/* 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
@@ -2028,12 +2009,7 @@ pmac_suspend_devices(void)
 	 */
 	sys_sync();
 
-	/* Sleep can fail now. May not be very robust but useful for debugging */
-	ret = broadcast_sleep(PBOOK_SLEEP_NOW, PBOOK_WAKE);
-	if (ret != PBOOK_SLEEP_OK) {
-		printk(KERN_ERR "Driver sleep failed\n");
-		return -EBUSY;
-	}
+	broadcast_sleep(PBOOK_SLEEP_NOW);
 
 	/* Send suspend call to devices, hold the device core's dpm_sem */
 	ret = device_suspend(PMSG_SUSPEND);
--- linux-2.6.orig/include/linux/pmu.h	2007-03-19 11:47:25.902413925 +0100
+++ linux-2.6/include/linux/pmu.h	2007-03-19 11:47:40.242413925 +0100
@@ -168,24 +168,16 @@ extern int pmu_get_model(void);
 
 struct pmu_sleep_notifier
 {
-	int (*notifier_call)(struct pmu_sleep_notifier *self, int when);
+	void (*notifier_call)(struct pmu_sleep_notifier *self, int when);
 	int priority;
 	struct list_head list;
 };
 
 /* Code values for calling sleep/wakeup handlers
- *
- * Note: If a sleep request got cancelled, all drivers will get
- * the PBOOK_SLEEP_REJECT, even those who didn't get the PBOOK_SLEEP_REQUEST.
  */
 #define PBOOK_SLEEP_REQUEST	1
 #define PBOOK_SLEEP_NOW		2
-#define PBOOK_SLEEP_REJECT	3
-#define PBOOK_WAKE		4
-
-/* Result codes returned by the notifiers */
-#define PBOOK_SLEEP_OK		0
-#define PBOOK_SLEEP_REFUSE	-1
+#define PBOOK_WAKE		3
 
 /* priority levels in notifiers */
 #define SLEEP_LEVEL_VIDEO	100	/* Video driver (first wake) */
--- linux-2.6.orig/drivers/macintosh/via-pmu-led.c	2007-03-19 11:47:25.702413925 +0100
+++ linux-2.6/drivers/macintosh/via-pmu-led.c	2007-03-19 11:47:40.242413925 +0100
@@ -81,7 +81,7 @@ static struct led_classdev pmu_led = {
 };
 
 #ifdef CONFIG_PM
-static int pmu_led_sleep_call(struct pmu_sleep_notifier *self, int when)
+static void pmu_led_sleep_call(struct pmu_sleep_notifier *self, int when)
 {
 	unsigned long flags;
 
@@ -99,8 +99,6 @@ static int pmu_led_sleep_call(struct pmu
 		break;
 	}
 	spin_unlock_irqrestore(&pmu_blink_lock, flags);
-
-	return PBOOK_SLEEP_OK;
 }
 
 static struct pmu_sleep_notifier via_pmu_led_sleep_notif = {
--- linux-2.6.orig/drivers/macintosh/apm_emu.c	2007-03-19 11:47:25.802413925 +0100
+++ linux-2.6/drivers/macintosh/apm_emu.c	2007-03-19 11:47:40.242413925 +0100
@@ -96,7 +96,7 @@ static DECLARE_WAIT_QUEUE_HEAD(apm_waitq
 static DECLARE_WAIT_QUEUE_HEAD(apm_suspend_waitqueue);
 static struct apm_user *	user_list;
 
-static int apm_notify_sleep(struct pmu_sleep_notifier *self, int when);
+static void apm_notify_sleep(struct pmu_sleep_notifier *self, int when);
 static struct pmu_sleep_notifier apm_sleep_notifier = {
 	apm_notify_sleep,
 	SLEEP_LEVEL_USERLAND,
@@ -352,7 +352,7 @@ static int do_open(struct inode * inode,
  * doesn't provide a way to NAK, but this could be added
  * here.
  */
-static int wait_all_suspend(void)
+static void wait_all_suspend(void)
 {
 	DECLARE_WAITQUEUE(wait, current);
 
@@ -366,24 +366,19 @@ static int wait_all_suspend(void)
 	remove_wait_queue(&apm_suspend_waitqueue, &wait);
 
 	DBG("apm_emu: wait_all_suspend() - complete !\n");
-	
-	return 1;
 }
 
-static int apm_notify_sleep(struct pmu_sleep_notifier *self, int when)
+static void apm_notify_sleep(struct pmu_sleep_notifier *self, int when)
 {
 	switch(when) {
 		case PBOOK_SLEEP_REQUEST:
 			queue_event(APM_SYS_SUSPEND, NULL);
-			if (!wait_all_suspend())
-				return PBOOK_SLEEP_REFUSE;
+			wait_all_suspend();
 			break;
-		case PBOOK_SLEEP_REJECT:
 		case PBOOK_WAKE:
 			queue_event(APM_NORMAL_RESUME, NULL);
 			break;
 	}
-	return PBOOK_SLEEP_OK;
 }
 
 #define APM_CRITICAL		10
--- linux-2.6.orig/sound/oss/dmasound/dmasound_awacs.c	2007-03-19 11:47:26.032413925 +0100
+++ linux-2.6/sound/oss/dmasound/dmasound_awacs.c	2007-03-19 11:47:40.252413925 +0100
@@ -257,7 +257,7 @@ static volatile struct dbdma_cmd *emerge
 /*
  * Stuff for restoring after a sleep.
  */
-static int awacs_sleep_notify(struct pmu_sleep_notifier *self, int when);
+static void awacs_sleep_notify(struct pmu_sleep_notifier *self, int when);
 struct pmu_sleep_notifier awacs_sleep_notifier = {
 	awacs_sleep_notify, SLEEP_LEVEL_SOUND,
 };
@@ -1419,7 +1419,7 @@ load_awacs(void)
  * Save state when going to sleep, restore it afterwards.
  */
 /* FIXME: sort out disabling/re-enabling of read stuff as well */
-static int awacs_sleep_notify(struct pmu_sleep_notifier *self, int when)
+static void awacs_sleep_notify(struct pmu_sleep_notifier *self, int when)
 {
 	unsigned long flags;
 
@@ -1548,7 +1548,6 @@ static int awacs_sleep_notify(struct pmu
 		spin_unlock_irqrestore(&dmasound.lock, flags);
 		UNLOCK();
 	}
-	return PBOOK_SLEEP_OK;
 }
 #endif /* CONFIG_PM */
 

--

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 4/5] powermac: proper sleep management
  2007-03-19 10:53 [PATCH 0/5] powermac suspend fixes Johannes Berg
                   ` (2 preceding siblings ...)
  2007-03-19 10:53 ` [PATCH 3/5] powermac: disallow pmu sleep notifiers from aborting sleep Johannes Berg
@ 2007-03-19 10:53 ` Johannes Berg
  2007-03-19 14:50   ` Benjamin Herrenschmidt
  2007-03-19 23:44   ` Johannes Berg
  2007-03-19 10:53 ` [PATCH 5/5] remove dead code in via-pmu68k Johannes Berg
  4 siblings, 2 replies; 32+ messages in thread
From: Johannes Berg @ 2007-03-19 10:53 UTC (permalink / raw)
  To: linuxppc-dev

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>

---
Could use some testing on older powerbooks just to see if they get problems
with the slight reordering of the suspend/resume sequence. I doubt it though.

And before someone asks:
Yes, it is safe to remove the backlight ioctl restrictions
because the generic layer actually freezes processes before STR.

This updated version removes the sys_sync() call that can't be done with
processes frozen.

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

--- linux-2.6.orig/drivers/macintosh/via-pmu.c	2007-03-19 11:47:40.232413925 +0100
+++ linux-2.6/drivers/macintosh/via-pmu.c	2007-03-19 11:53:10.332413925 +0100
@@ -155,9 +155,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 +1988,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 +1998,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 +2064,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 +2073,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 +2082,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 +2152,6 @@ powerbook_sleep_Core99(void)
 	/* Restore LPJ, cpufreq will adjust the cpu frequency */
 	loops_per_jiffy /= 2;
 
-	pmac_wakeup_devices();
-
 	return 0;
 }
 
@@ -2308,7 +2161,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 +2179,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 +2222,6 @@ powerbook_sleep_3400(void)
 	while (asleep)
 		mb();
 
-	pmac_wakeup_devices();
 	pbook_free_pci_save();
 	iounmap(mem_ctrl);
 
@@ -2558,6 +2403,124 @@ 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);
+
+#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();
+
+	preempt_disable();
+
+	return 0;
+}
+
+static int powerbook_sleep(suspend_state_t state)
+{
+	int error = 0;
+
+	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));
+
+	/* 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);
+
+	/* Force a poll of ADB interrupts */
+	adb_int_pending = 1;
+	via_pmu_interrupt(0, NULL);
+
+	/* Restart jiffies & scheduling */
+	wakeup_decrementer();
+
+	return 0;
+}
+
+static int powerbook_finish_sleep(suspend_state_t state)
+{
+#ifdef CONFIG_PMAC_BACKLIGHT
+	/* Tell backlight code it can use the chip again */
+	pmu_backlight_set_sleep(0);
+#endif
+
+	preempt_enable();
+
+	/* Call platform functions marked "on wake" */
+	pmac_pfunc_base_resume();
+	pmac_pfunc_i2c_resume();
+
+	/* 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 = {
+	.pm_disk_mode = PM_DISK_PLATFORM,
+	.prepare = powerbook_prepare_sleep,
+	.finish = powerbook_finish_sleep,
+	.enter = powerbook_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 +2530,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 +2555,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 +2566,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-03-19 11:47:24.672413925 +0100
+++ linux-2.6/Documentation/feature-removal-schedule.txt	2007-03-19 11:47:41.162413925 +0100
@@ -324,3 +324,13 @@ Why:	the i8xx_tco watchdog driver has be
 Who:	Wim Van Sebroeck <wim@iguana.be>
 
 ---------------------------
+
+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>
+
+---------------------------

--

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 5/5] remove dead code in via-pmu68k
  2007-03-19 10:53 [PATCH 0/5] powermac suspend fixes Johannes Berg
                   ` (3 preceding siblings ...)
  2007-03-19 10:53 ` [PATCH 4/5] powermac: proper sleep management Johannes Berg
@ 2007-03-19 10:53 ` Johannes Berg
  2007-03-19 19:17   ` Brad Boyer
  4 siblings, 1 reply; 32+ messages in thread
From: Johannes Berg @ 2007-03-19 10:53 UTC (permalink / raw)
  To: linuxppc-dev

When suspend is ever implemented for pmu68k it really should follow the
generic pm_ops concept and not mirror the platform-specific /dev/pmu
device with ioctls on it. Hence, this patch removes the unused code there;
should the implementors need it they can look at via-pmu.c and/or the
history of the file.

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

---
 drivers/macintosh/via-pmu68k.c |  240 -----------------------------------------
 1 file changed, 240 deletions(-)

--- linux-2.6.orig/drivers/macintosh/via-pmu68k.c	2007-03-19 11:47:23.702413925 +0100
+++ linux-2.6/drivers/macintosh/via-pmu68k.c	2007-03-19 11:47:41.802413925 +0100
@@ -819,243 +819,3 @@ pmu_present(void)
 {
 	return (pmu_kind != PMU_UNKNOWN);
 }
-
-#if 0 /* needs some work for 68K */
-
-/*
- * This struct is used to store config register values for
- * PCI devices which may get powered off when we sleep.
- */
-static struct pci_save {
-	u16	command;
-	u16	cache_lat;
-	u16	intr;
-} *pbook_pci_saves;
-static int n_pbook_pci_saves;
-
-static inline void
-pbook_pci_save(void)
-{
-	int npci;
-	struct pci_dev *pd = NULL;
-	struct pci_save *ps;
-
-	npci = 0;
-	while ((pd = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, pd)) != NULL)
-		++npci;
-	n_pbook_pci_saves = npci;
-	if (npci == 0)
-		return;
-	ps = kmalloc(npci * sizeof(*ps), GFP_KERNEL);
-	pbook_pci_saves = ps;
-	if (ps == NULL)
-		return;
-
-	pd = NULL;
-	while ((pd = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, pd)) != NULL) {
-		pci_read_config_word(pd, PCI_COMMAND, &ps->command);
-		pci_read_config_word(pd, PCI_CACHE_LINE_SIZE, &ps->cache_lat);
-		pci_read_config_word(pd, PCI_INTERRUPT_LINE, &ps->intr);
-		++ps;
-		--npci;
-	}
-}
-
-static inline void
-pbook_pci_restore(void)
-{
-	u16 cmd;
-	struct pci_save *ps = pbook_pci_saves;
-	struct pci_dev *pd = NULL;
-	int j;
-
-	while ((pd = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, pd)) != NULL) {
-		if (ps->command == 0)
-			continue;
-		pci_read_config_word(pd, PCI_COMMAND, &cmd);
-		if ((ps->command & ~cmd) == 0)
-			continue;
-		switch (pd->hdr_type) {
-		case PCI_HEADER_TYPE_NORMAL:
-			for (j = 0; j < 6; ++j)
-				pci_write_config_dword(pd,
-					PCI_BASE_ADDRESS_0 + j*4,
-					pd->resource[j].start);
-			pci_write_config_dword(pd, PCI_ROM_ADDRESS,
-			       pd->resource[PCI_ROM_RESOURCE].start);
-			pci_write_config_word(pd, PCI_CACHE_LINE_SIZE,
-				ps->cache_lat);
-			pci_write_config_word(pd, PCI_INTERRUPT_LINE,
-				ps->intr);
-			pci_write_config_word(pd, PCI_COMMAND, ps->command);
-			break;
-			/* other header types not restored at present */
-		}
-	}
-}
-
-/*
- * Put the powerbook to sleep.
- */
-#define IRQ_ENABLE	((unsigned int *)0xf3000024)
-#define MEM_CTRL	((unsigned int *)0xf8000070)
-
-int powerbook_sleep(void)
-{
-	int ret, i, x;
-	static int save_backlight;
-	static unsigned int save_irqen;
-	unsigned long msr;
-	unsigned int hid0;
-	unsigned long p, wait;
-	struct adb_request sleep_req;
-
-	/* Notify device drivers */
-	ret = blocking_notifier_call_chain(&sleep_notifier_list,
-			PBOOK_SLEEP, NULL);
-	if (ret & NOTIFY_STOP_MASK)
-		return -EBUSY;
-
-	/* Sync the disks. */
-	/* XXX It would be nice to have some way to ensure that
-	 * nobody is dirtying any new buffers while we wait. */
-	sys_sync();
-
-	/* Turn off the display backlight */
-	save_backlight = backlight_enabled;
-	if (save_backlight)
-		pmu_enable_backlight(0);
-
-	/* Give the disks a little time to actually finish writing */
-	for (wait = jiffies + (HZ/4); time_before(jiffies, wait); )
-		mb();
-
-	/* Disable all interrupts except pmu */
-	save_irqen = in_le32(IRQ_ENABLE);
-	for (i = 0; i < 32; ++i)
-		if (i != vias->intrs[0].line && (save_irqen & (1 << i)))
-			disable_irq(i);
-	asm volatile("mtdec %0" : : "r" (0x7fffffff));
-
-	/* Save the state of PCI config space for some slots */
-	pbook_pci_save();
-
-	/* Set the memory controller to keep the memory refreshed
-	   while we're asleep */
-	for (i = 0x403f; i >= 0x4000; --i) {
-		out_be32(MEM_CTRL, i);
-		do {
-			x = (in_be32(MEM_CTRL) >> 16) & 0x3ff;
-		} while (x == 0);
-		if (x >= 0x100)
-			break;
-	}
-
-	/* Ask the PMU to put us to sleep */
-	pmu_request(&sleep_req, NULL, 5, PMU_SLEEP, 'M', 'A', 'T', 'T');
-	while (!sleep_req.complete)
-		mb();
-	/* displacement-flush the L2 cache - necessary? */
-	for (p = KERNELBASE; p < KERNELBASE + 0x100000; p += 0x1000)
-		i = *(volatile int *)p;
-	asleep = 1;
-
-	/* Put the CPU into sleep mode */
-	asm volatile("mfspr %0,1008" : "=r" (hid0) :);
-	hid0 = (hid0 & ~(HID0_NAP | HID0_DOZE)) | HID0_SLEEP;
-	asm volatile("mtspr 1008,%0" : : "r" (hid0));
-	local_save_flags(msr);
-	msr |= MSR_POW | MSR_EE;
-	local_irq_restore(msr);
-	udelay(10);
-
-	/* OK, we're awake again, start restoring things */
-	out_be32(MEM_CTRL, 0x3f);
-	pbook_pci_restore();
-
-	/* wait for the PMU interrupt sequence to complete */
-	while (asleep)
-		mb();
-
-	/* reenable interrupts */
-	for (i = 0; i < 32; ++i)
-		if (i != vias->intrs[0].line && (save_irqen & (1 << i)))
-			enable_irq(i);
-
-	/* Notify drivers */
-	blocking_notifier_call_chain(&sleep_notifier_list, PBOOK_WAKE, NULL);
-
-	/* reenable ADB autopoll */
-	pmu_adb_autopoll(adb_dev_map);
-
-	/* Turn on the screen backlight, if it was on before */
-	if (save_backlight)
-		pmu_enable_backlight(1);
-
-	/* Wait for the hard disk to spin up */
-
-	return 0;
-}
-
-/*
- * Support for /dev/pmu device
- */
-static int pmu_open(struct inode *inode, struct file *file)
-{
-	return 0;
-}
-
-static ssize_t pmu_read(struct file *file, char *buf,
-			size_t count, loff_t *ppos)
-{
-	return 0;
-}
-
-static ssize_t pmu_write(struct file *file, const char *buf,
-			 size_t count, loff_t *ppos)
-{
-	return 0;
-}
-
-static int pmu_ioctl(struct inode * inode, struct file *filp,
-		     u_int cmd, u_long arg)
-{
-	int error;
-	__u32 value;
-
-	switch (cmd) {
-	    case PMU_IOC_SLEEP:
-	    	return -ENOSYS;
-	    case PMU_IOC_GET_BACKLIGHT:
-		return put_user(backlight_level, (__u32 *)arg);
-	    case PMU_IOC_SET_BACKLIGHT:
-		error = get_user(value, (__u32 *)arg);
-		if (!error)
-			pmu_set_brightness(value);
-		return error;
-	    case PMU_IOC_GET_MODEL:
-	    	return put_user(pmu_kind, (__u32 *)arg);
-	}
-	return -EINVAL;
-}
-
-static const struct file_operations pmu_device_fops = {
-	.read		= pmu_read,
-	.write		= pmu_write,
-	.ioctl		= pmu_ioctl,
-	.open		= pmu_open,
-};
-
-static struct miscdevice pmu_device = {
-	PMU_MINOR, "pmu", &pmu_device_fops
-};
-
-void pmu_device_init(void)
-{
-	if (!via)
-		return;
-	if (misc_register(&pmu_device) < 0)
-		printk(KERN_ERR "via-pmu68k: cannot register misc device.\n");
-}
-#endif /* CONFIG_PMAC_PBOOK */
-

--

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/5] powerpc: generic time suspend/resume code
  2007-03-19 10:53 ` [PATCH 1/5] powerpc: generic time suspend/resume code Johannes Berg
@ 2007-03-19 14:47   ` Benjamin Herrenschmidt
  2007-03-19 21:51     ` Guennadi Liakhovetski
  2007-05-02  5:02   ` Michael Ellerman
  1 sibling, 1 reply; 32+ messages in thread
From: Benjamin Herrenschmidt @ 2007-03-19 14:47 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev

On Mon, 2007-03-19 at 11:53 +0100, Johannes Berg wrote:
> plain text document attachment (001-time-resume.patch)
> This patch removes the time suspend/restore code that was done through
> a PMU notifier in arch/platforms/powermac/time.c.
> 
> Instead, introduce arch/powerpc/sysdev/timer.c which creates a sys
> device and handles time of day suspend/resume through that.
> 
> This should probably be replaced by using the generic RTC framework
> but for now it gets rid of the arcane powermac specific hack.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] powerpc: fix suspend states again
  2007-03-19 10:53 ` [PATCH 2/5] powerpc: fix suspend states again Johannes Berg
@ 2007-03-19 14:48   ` Benjamin Herrenschmidt
  2007-03-19 15:22     ` Johannes Berg
  2007-03-19 16:06   ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 32+ messages in thread
From: Benjamin Herrenschmidt @ 2007-03-19 14:48 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev

On Mon, 2007-03-19 at 11:53 +0100, Johannes Berg wrote:
> plain text document attachment (002-fix-powermac-suspend-again.patch)
> In commit 0fba3a1f39f8b0a50b56c8b068fa52131cbc84c2 (a very long time ago,
> May 2006), I fixed a bug that caused powermacs to crash when you tried
> entering standby/mem suspend states.
> 
> As I'm now getting more familiar with the suspend code I notice a few
> more things:
>  1. we previously misunderstood what pm_ops is for, it isn't supposed to be
>     for doing platform dependent suspend/resume stuff that needs to be done
>     for suspend to disk (as we currently try to use it!), it is instead for
>     entering platform dependent suspend states ("standby", "mem").
>  2. due to the first point, we never properly save FPU and altivec states
>     when suspending to disk. It probably hasn't hurt yet because the process
>     that writes the "disk" to /sys/power/state uses neither and its context
>     is used.
> 
> This patch addresses these points as follows:
>  1. remove all pm_ops from powermac, powermac suspend to ram isn't currently
>     usable via /sys/power/state but is done via the PMU instead.
>  2. move the code responsible for storing FPU/altivec state into a new
>     arch_prepare_suspend function (previously, this was only present for
>     32-bit platforms from asm-ppc.)
> 
> A follow-on patch will create new pm_ops for via-pmu.
> 
> I removed
> 	set_context(current->active_mm->context.id, current->active_mm->pgd);
> because
>  1. it works without and
>  2. I don't see the point

No, you need that to restore the segment registers. I might work for you
by mere luck.

Ben.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/5] powermac: disallow pmu sleep notifiers from aborting sleep
  2007-03-19 10:53 ` [PATCH 3/5] powermac: disallow pmu sleep notifiers from aborting sleep Johannes Berg
@ 2007-03-19 14:49   ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 32+ messages in thread
From: Benjamin Herrenschmidt @ 2007-03-19 14:49 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev

On Mon, 2007-03-19 at 11:53 +0100, Johannes Berg wrote:
> plain text document attachment (003-less-pmu-sleep-notifier.patch)
> Tracing through the code, no current PMU sleep notifier can abort sleep.
> Since no new PMU sleep notifiers should be added, this patch simplifies the
> code and removes the ability to abort sleep.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Ben.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 4/5] powermac: proper sleep management
  2007-03-19 10:53 ` [PATCH 4/5] powermac: proper sleep management Johannes Berg
@ 2007-03-19 14:50   ` Benjamin Herrenschmidt
  2007-03-19 15:16     ` Johannes Berg
  2007-03-19 23:44   ` Johannes Berg
  1 sibling, 1 reply; 32+ messages in thread
From: Benjamin Herrenschmidt @ 2007-03-19 14:50 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev


> This updated version removes the sys_sync() call that can't be done with
> processes frozen.

Is the generic code still sync'ing beforehand ?

Ben.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 4/5] powermac: proper sleep management
  2007-03-19 14:50   ` Benjamin Herrenschmidt
@ 2007-03-19 15:16     ` Johannes Berg
  0 siblings, 0 replies; 32+ messages in thread
From: Johannes Berg @ 2007-03-19 15:16 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

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

On Mon, 2007-03-19 at 15:50 +0100, Benjamin Herrenschmidt wrote:
> > This updated version removes the sys_sync() call that can't be done with
> > processes frozen.
> 
> Is the generic code still sync'ing beforehand ?

Yes, Rafael told me that the generic code syncs right before freezing
processes.

johannes

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] powerpc: fix suspend states again
  2007-03-19 14:48   ` Benjamin Herrenschmidt
@ 2007-03-19 15:22     ` Johannes Berg
  2007-03-19 15:32       ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 32+ messages in thread
From: Johannes Berg @ 2007-03-19 15:22 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

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

On Mon, 2007-03-19 at 15:48 +0100, Benjamin Herrenschmidt wrote:

> > I removed
> > 	set_context(current->active_mm->context.id, current->active_mm->pgd);

> No, you need that to restore the segment registers. I might work for you
> by mere luck.

Hm, I thought they were saved by the lowlevel code, but yeah. I'll need
to see where to put this. Btw, it looks like it's missing in
powerbook_sleep_3400 as well?

johannes

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] powerpc: fix suspend states again
  2007-03-19 15:22     ` Johannes Berg
@ 2007-03-19 15:32       ` Benjamin Herrenschmidt
  2007-03-19 15:45         ` Johannes Berg
  0 siblings, 1 reply; 32+ messages in thread
From: Benjamin Herrenschmidt @ 2007-03-19 15:32 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev

On Mon, 2007-03-19 at 16:22 +0100, Johannes Berg wrote:
> On Mon, 2007-03-19 at 15:48 +0100, Benjamin Herrenschmidt wrote:
> 
> > > I removed
> > > 	set_context(current->active_mm->context.id, current->active_mm->pgd);
> 
> > No, you need that to restore the segment registers. I might work for you
> > by mere luck.
> 
> Hm, I thought they were saved by the lowlevel code, but yeah. I'll need
> to see where to put this. Btw, it looks like it's missing in
> powerbook_sleep_3400 as well?

No, on powerbook_sleep_3400, the CPU is never reset nor powered off.

Ben.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] powerpc: fix suspend states again
  2007-03-19 15:32       ` Benjamin Herrenschmidt
@ 2007-03-19 15:45         ` Johannes Berg
  2007-03-19 15:54           ` Johannes Berg
  0 siblings, 1 reply; 32+ messages in thread
From: Johannes Berg @ 2007-03-19 15:45 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

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

On Mon, 2007-03-19 at 16:32 +0100, Benjamin Herrenschmidt wrote:

> No, on powerbook_sleep_3400, the CPU is never reset nor powered off.

Oh ok, wasn't aware of that. I think the right place to put it is
restore_processor_state, new patch coming.

johannes

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 2/5] powerpc: fix suspend states again
  2007-03-19 15:45         ` Johannes Berg
@ 2007-03-19 15:54           ` Johannes Berg
  2007-03-19 16:22             ` Johannes Berg
  0 siblings, 1 reply; 32+ messages in thread
From: Johannes Berg @ 2007-03-19 15:54 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

In commit 0fba3a1f39f8b0a50b56c8b068fa52131cbc84c2 (a very long time ago,
May 2006), I fixed a bug that caused powermacs to crash when you tried
entering standby/mem suspend states.

As I'm now getting more familiar with the suspend code I notice a few
more things:
 1. we previously misunderstood what pm_ops is for, it isn't supposed to be
    for doing platform dependent suspend/resume stuff that needs to be done
    for suspend to disk (as we currently try to use it!), it is instead for
    entering platform dependent suspend states ("standby", "mem").
 2. due to the first point, we never properly save FPU and altivec states
    when suspending to disk. It probably hasn't hurt yet because the process
    that writes the "disk" to /sys/power/state uses neither and its context
    is used.

This patch addresses these points as follows:
 1. remove all pm_ops from powermac, powermac suspend to ram isn't currently
    usable via /sys/power/state but is done via the PMU instead.
 2. move the code responsible for storing FPU/altivec state into
    save_processor_state and the set_context() call to restore_processor_state.

It may look like there is some code removal missing but that is actually because
the new suspend.h file overrides the ppc/suspend.h one which was previously used.

A follow-on patch will create new pm_ops for via-pmu.

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

---
 arch/powerpc/kernel/Makefile            |    1 
 arch/powerpc/kernel/swsusp.c            |   34 ++++++++++++++++
 arch/powerpc/platforms/powermac/setup.c |   65 --------------------------------
 include/asm-powerpc/suspend.h           |    9 ++++
 4 files changed, 44 insertions(+), 65 deletions(-)

--- linux-2.6.orig/arch/powerpc/platforms/powermac/setup.c	2007-03-19 16:49:57.723321419 +0100
+++ linux-2.6/arch/powerpc/platforms/powermac/setup.c	2007-03-19 16:50:00.983321419 +0100
@@ -420,76 +420,11 @@ static void __init find_boot_device(void
 #endif
 }
 
-/* TODO: Merge the suspend-to-ram with the common code !!!
- * currently, this is a stub implementation for suspend-to-disk
- * only
- */
-
-#ifdef CONFIG_SOFTWARE_SUSPEND
-
-static int pmac_pm_prepare(suspend_state_t state)
-{
-	printk(KERN_DEBUG "%s(%d)\n", __FUNCTION__, state);
-
-	return 0;
-}
-
-static int pmac_pm_enter(suspend_state_t state)
-{
-	printk(KERN_DEBUG "%s(%d)\n", __FUNCTION__, state);
-
-	/* 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 (cur_cpu_spec->cpu_features & CPU_FTR_ALTIVEC)
-		enable_kernel_altivec();
-#endif /* CONFIG_ALTIVEC */
-
-	return 0;
-}
-
-static int pmac_pm_finish(suspend_state_t state)
-{
-	printk(KERN_DEBUG "%s(%d)\n", __FUNCTION__, state);
-
-	/* Restore userland MMU context */
-	set_context(current->active_mm->context.id, current->active_mm->pgd);
-
-	return 0;
-}
-
-static int pmac_pm_valid(suspend_state_t state)
-{
-	switch (state) {
-	case PM_SUSPEND_DISK:
-		return 1;
-	/* can't do any other states via generic mechanism yet */
-	default:
-		return 0;
-	}
-}
-
-static struct pm_ops pmac_pm_ops = {
-	.pm_disk_mode	= PM_DISK_SHUTDOWN,
-	.prepare	= pmac_pm_prepare,
-	.enter		= pmac_pm_enter,
-	.finish		= pmac_pm_finish,
-	.valid		= pmac_pm_valid,
-};
-
-#endif /* CONFIG_SOFTWARE_SUSPEND */
-
 static int initializing = 1;
 
 static int pmac_late_init(void)
 {
 	initializing = 0;
-#ifdef CONFIG_SOFTWARE_SUSPEND
-	pm_set_ops(&pmac_pm_ops);
-#endif /* CONFIG_SOFTWARE_SUSPEND */
 	return 0;
 }
 
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/include/asm-powerpc/suspend.h	2007-03-19 16:50:00.993321419 +0100
@@ -0,0 +1,9 @@
+#ifndef __ASM_POWERPC_SUSPEND_H
+#define __ASM_POWERPC_SUSPEND_H
+
+static inline int arch_prepare_suspend(void) { return 0; }
+
+void save_processor_state(void);
+void restore_processor_state(void);
+
+#endif /* __ASM_POWERPC_SUSPEND_H */
--- linux-2.6.orig/arch/powerpc/kernel/Makefile	2007-03-19 16:49:57.743321419 +0100
+++ linux-2.6/arch/powerpc/kernel/Makefile	2007-03-19 16:50:00.993321419 +0100
@@ -36,6 +36,7 @@ obj-$(CONFIG_GENERIC_TBSYNC)	+= smp-tbsy
 obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
 obj-$(CONFIG_6xx)		+= idle_6xx.o l2cr_6xx.o cpu_setup_6xx.o
 obj-$(CONFIG_TAU)		+= tau_6xx.o
+obj-$(CONFIG_SOFTWARE_SUSPEND)	+= swsusp.o
 obj32-$(CONFIG_SOFTWARE_SUSPEND) += swsusp_32.o
 obj32-$(CONFIG_MODULES)		+= module_32.o
 
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/powerpc/kernel/swsusp.c	2007-03-19 16:50:00.993321419 +0100
@@ -0,0 +1,34 @@
+/*
+ * Common powerpc suspend code for 32 and 64 bits
+ *
+ * Copyright 2007	Johannes Berg <johannes@sipsolutions.net>
+ *
+ * GPLv2
+ */
+
+#include <linux/sched.h>
+#include <asm/suspend.h>
+#include <asm/cputable.h>
+#include <asm/system.h>
+#include <asm/current.h>
+#include <asm/mmu_context.h>
+
+void save_processor_state(void)
+{
+	/* 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 (cur_cpu_spec->cpu_features & CPU_FTR_ALTIVEC)
+		enable_kernel_altivec();
+#endif /* CONFIG_ALTIVEC */
+}
+
+void restore_processor_state(void)
+{
+#ifdef CONFIG_PPC32
+	set_context(current->active_mm->context.id, current->active_mm->pgd);
+#endif
+}

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] powerpc: fix suspend states again
  2007-03-19 10:53 ` [PATCH 2/5] powerpc: fix suspend states again Johannes Berg
  2007-03-19 14:48   ` Benjamin Herrenschmidt
@ 2007-03-19 16:06   ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 32+ messages in thread
From: Benjamin Herrenschmidt @ 2007-03-19 16:06 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev


> +
> +int save_processor_state(void)
> +{
> +	/* 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 (cur_cpu_spec->cpu_features & CPU_FTR_ALTIVEC)
> +		enable_kernel_altivec();
> +#endif /* CONFIG_ALTIVEC */
> +
> +	return 0;
> +}

You need to check if there's an FPU I suppose for enable_kernel_fp()
since this is generic code potentially used with embedded CPUs as well.
In fact, it would be good to also add some of the freescale stuff.

Ben.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 2/5] powerpc: fix suspend states again
  2007-03-19 15:54           ` Johannes Berg
@ 2007-03-19 16:22             ` Johannes Berg
  2007-03-19 16:39               ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 32+ messages in thread
From: Johannes Berg @ 2007-03-19 16:22 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

In commit 0fba3a1f39f8b0a50b56c8b068fa52131cbc84c2 (a very long time ago,
May 2006), I fixed a bug that caused powermacs to crash when you tried
entering standby/mem suspend states.

As I'm now getting more familiar with the suspend code I notice a few
more things:
 1. we previously misunderstood what pm_ops is for, it isn't supposed to be
    for doing platform dependent suspend/resume stuff that needs to be done
    for suspend to disk (as we currently try to use it!), it is instead for
    entering platform dependent suspend states ("standby", "mem").
 2. due to the first point, we never properly save FPU and altivec states
    when suspending to disk. It probably hasn't hurt yet because the process
    that writes the "disk" to /sys/power/state uses neither and its context
    is used.

This patch addresses these points as follows:
 1. remove all pm_ops from powermac, powermac suspend to ram isn't currently
    usable via /sys/power/state but is done via the PMU instead.
 2. move the code responsible for storing FPU/altivec state into
    save_processor_state and the set_context() call to restore_processor_state.

It also adds a call to kernel_enable_spe() but I don't have any machines that
have that to see if it actually works anyway.

It may look like there is some code removal missing but that is actually because
the new suspend.h file overrides the ppc/suspend.h one which was previously used.

A follow-on patch will create new pm_ops for via-pmu.

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

---
 arch/powerpc/kernel/Makefile            |    1 
 arch/powerpc/kernel/swsusp.c            |   42 ++++++++++++++++++++
 arch/powerpc/platforms/powermac/setup.c |   65 --------------------------------
 include/asm-powerpc/suspend.h           |    9 ++++
 4 files changed, 52 insertions(+), 65 deletions(-)

--- linux-2.6.orig/arch/powerpc/platforms/powermac/setup.c	2007-03-19 16:49:57.723321419 +0100
+++ linux-2.6/arch/powerpc/platforms/powermac/setup.c	2007-03-19 16:50:00.983321419 +0100
@@ -420,76 +420,11 @@ static void __init find_boot_device(void
 #endif
 }
 
-/* TODO: Merge the suspend-to-ram with the common code !!!
- * currently, this is a stub implementation for suspend-to-disk
- * only
- */
-
-#ifdef CONFIG_SOFTWARE_SUSPEND
-
-static int pmac_pm_prepare(suspend_state_t state)
-{
-	printk(KERN_DEBUG "%s(%d)\n", __FUNCTION__, state);
-
-	return 0;
-}
-
-static int pmac_pm_enter(suspend_state_t state)
-{
-	printk(KERN_DEBUG "%s(%d)\n", __FUNCTION__, state);
-
-	/* 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 (cur_cpu_spec->cpu_features & CPU_FTR_ALTIVEC)
-		enable_kernel_altivec();
-#endif /* CONFIG_ALTIVEC */
-
-	return 0;
-}
-
-static int pmac_pm_finish(suspend_state_t state)
-{
-	printk(KERN_DEBUG "%s(%d)\n", __FUNCTION__, state);
-
-	/* Restore userland MMU context */
-	set_context(current->active_mm->context.id, current->active_mm->pgd);
-
-	return 0;
-}
-
-static int pmac_pm_valid(suspend_state_t state)
-{
-	switch (state) {
-	case PM_SUSPEND_DISK:
-		return 1;
-	/* can't do any other states via generic mechanism yet */
-	default:
-		return 0;
-	}
-}
-
-static struct pm_ops pmac_pm_ops = {
-	.pm_disk_mode	= PM_DISK_SHUTDOWN,
-	.prepare	= pmac_pm_prepare,
-	.enter		= pmac_pm_enter,
-	.finish		= pmac_pm_finish,
-	.valid		= pmac_pm_valid,
-};
-
-#endif /* CONFIG_SOFTWARE_SUSPEND */
-
 static int initializing = 1;
 
 static int pmac_late_init(void)
 {
 	initializing = 0;
-#ifdef CONFIG_SOFTWARE_SUSPEND
-	pm_set_ops(&pmac_pm_ops);
-#endif /* CONFIG_SOFTWARE_SUSPEND */
 	return 0;
 }
 
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/include/asm-powerpc/suspend.h	2007-03-19 16:50:00.993321419 +0100
@@ -0,0 +1,9 @@
+#ifndef __ASM_POWERPC_SUSPEND_H
+#define __ASM_POWERPC_SUSPEND_H
+
+static inline int arch_prepare_suspend(void) { return 0; }
+
+void save_processor_state(void);
+void restore_processor_state(void);
+
+#endif /* __ASM_POWERPC_SUSPEND_H */
--- linux-2.6.orig/arch/powerpc/kernel/Makefile	2007-03-19 16:49:57.743321419 +0100
+++ linux-2.6/arch/powerpc/kernel/Makefile	2007-03-19 16:50:00.993321419 +0100
@@ -36,6 +36,7 @@ obj-$(CONFIG_GENERIC_TBSYNC)	+= smp-tbsy
 obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
 obj-$(CONFIG_6xx)		+= idle_6xx.o l2cr_6xx.o cpu_setup_6xx.o
 obj-$(CONFIG_TAU)		+= tau_6xx.o
+obj-$(CONFIG_SOFTWARE_SUSPEND)	+= swsusp.o
 obj32-$(CONFIG_SOFTWARE_SUSPEND) += swsusp_32.o
 obj32-$(CONFIG_MODULES)		+= module_32.o
 
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/powerpc/kernel/swsusp.c	2007-03-19 17:18:25.323321419 +0100
@@ -0,0 +1,42 @@
+/*
+ * Common powerpc suspend code for 32 and 64 bits
+ *
+ * Copyright 2007	Johannes Berg <johannes@sipsolutions.net>
+ *
+ * GPLv2
+ */
+
+#include <linux/sched.h>
+#include <asm/suspend.h>
+#include <asm/cputable.h>
+#include <asm/system.h>
+#include <asm/current.h>
+#include <asm/mmu_context.h>
+
+#ifdef CONFIG_SPE
+extern void enable_kernel_spe(void);
+#endif
+
+void save_processor_state(void)
+{
+	/* 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 (cur_cpu_spec->cpu_features & CPU_FTR_ALTIVEC)
+		enable_kernel_altivec();
+#endif /* CONFIG_ALTIVEC */
+
+#ifdef CONFIG_SPE
+	enable_kernel_spe();
+#endif
+}
+
+void restore_processor_state(void)
+{
+#ifdef CONFIG_PPC32
+	set_context(current->active_mm->context.id, current->active_mm->pgd);
+#endif
+}

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] powerpc: fix suspend states again
  2007-03-19 16:22             ` Johannes Berg
@ 2007-03-19 16:39               ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 32+ messages in thread
From: Benjamin Herrenschmidt @ 2007-03-19 16:39 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev

On Mon, 2007-03-19 at 17:22 +0100, Johannes Berg wrote:
> In commit 0fba3a1f39f8b0a50b56c8b068fa52131cbc84c2 (a very long time ago,
> May 2006), I fixed a bug that caused powermacs to crash when you tried
> entering standby/mem suspend states.
> 
> As I'm now getting more familiar with the suspend code I notice a few
> more things:
>  1. we previously misunderstood what pm_ops is for, it isn't supposed to be
>     for doing platform dependent suspend/resume stuff that needs to be done
>     for suspend to disk (as we currently try to use it!), it is instead for
>     entering platform dependent suspend states ("standby", "mem").
>  2. due to the first point, we never properly save FPU and altivec states
>     when suspending to disk. It probably hasn't hurt yet because the process
>     that writes the "disk" to /sys/power/state uses neither and its context
>     is used.
> 
> This patch addresses these points as follows:
>  1. remove all pm_ops from powermac, powermac suspend to ram isn't currently
>     usable via /sys/power/state but is done via the PMU instead.
>  2. move the code responsible for storing FPU/altivec state into
>     save_processor_state and the set_context() call to restore_processor_state.
> 
> It also adds a call to kernel_enable_spe() but I don't have any machines that
> have that to see if it actually works anyway.
> 
> It may look like there is some code removal missing but that is actually because
> the new suspend.h file overrides the ppc/suspend.h one which was previously used.
> 
> A follow-on patch will create new pm_ops for via-pmu.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 5/5] remove dead code in via-pmu68k
  2007-03-19 10:53 ` [PATCH 5/5] remove dead code in via-pmu68k Johannes Berg
@ 2007-03-19 19:17   ` Brad Boyer
  0 siblings, 0 replies; 32+ messages in thread
From: Brad Boyer @ 2007-03-19 19:17 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev, linux-m68k


Patches for the 68k drivers should probably be copied to the linux-m68k
mailing list. I've copied that list on this reply.

	Brad Boyer
	flar@allandria.com

On Mon, Mar 19, 2007 at 11:53:57AM +0100, Johannes Berg wrote:
> When suspend is ever implemented for pmu68k it really should follow the
> generic pm_ops concept and not mirror the platform-specific /dev/pmu
> device with ioctls on it. Hence, this patch removes the unused code there;
> should the implementors need it they can look at via-pmu.c and/or the
> history of the file.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> ---
>  drivers/macintosh/via-pmu68k.c |  240 -----------------------------------------
>  1 file changed, 240 deletions(-)
> 
> --- linux-2.6.orig/drivers/macintosh/via-pmu68k.c	2007-03-19 11:47:23.702413925 +0100
> +++ linux-2.6/drivers/macintosh/via-pmu68k.c	2007-03-19 11:47:41.802413925 +0100
> @@ -819,243 +819,3 @@ pmu_present(void)
>  {
>  	return (pmu_kind != PMU_UNKNOWN);
>  }
> -
> -#if 0 /* needs some work for 68K */
> -
> -/*
> - * This struct is used to store config register values for
> - * PCI devices which may get powered off when we sleep.
> - */
> -static struct pci_save {
> -	u16	command;
> -	u16	cache_lat;
> -	u16	intr;
> -} *pbook_pci_saves;
> -static int n_pbook_pci_saves;
> -
> -static inline void
> -pbook_pci_save(void)
> -{
> -	int npci;
> -	struct pci_dev *pd = NULL;
> -	struct pci_save *ps;
> -
> -	npci = 0;
> -	while ((pd = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, pd)) != NULL)
> -		++npci;
> -	n_pbook_pci_saves = npci;
> -	if (npci == 0)
> -		return;
> -	ps = kmalloc(npci * sizeof(*ps), GFP_KERNEL);
> -	pbook_pci_saves = ps;
> -	if (ps == NULL)
> -		return;
> -
> -	pd = NULL;
> -	while ((pd = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, pd)) != NULL) {
> -		pci_read_config_word(pd, PCI_COMMAND, &ps->command);
> -		pci_read_config_word(pd, PCI_CACHE_LINE_SIZE, &ps->cache_lat);
> -		pci_read_config_word(pd, PCI_INTERRUPT_LINE, &ps->intr);
> -		++ps;
> -		--npci;
> -	}
> -}
> -
> -static inline void
> -pbook_pci_restore(void)
> -{
> -	u16 cmd;
> -	struct pci_save *ps = pbook_pci_saves;
> -	struct pci_dev *pd = NULL;
> -	int j;
> -
> -	while ((pd = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, pd)) != NULL) {
> -		if (ps->command == 0)
> -			continue;
> -		pci_read_config_word(pd, PCI_COMMAND, &cmd);
> -		if ((ps->command & ~cmd) == 0)
> -			continue;
> -		switch (pd->hdr_type) {
> -		case PCI_HEADER_TYPE_NORMAL:
> -			for (j = 0; j < 6; ++j)
> -				pci_write_config_dword(pd,
> -					PCI_BASE_ADDRESS_0 + j*4,
> -					pd->resource[j].start);
> -			pci_write_config_dword(pd, PCI_ROM_ADDRESS,
> -			       pd->resource[PCI_ROM_RESOURCE].start);
> -			pci_write_config_word(pd, PCI_CACHE_LINE_SIZE,
> -				ps->cache_lat);
> -			pci_write_config_word(pd, PCI_INTERRUPT_LINE,
> -				ps->intr);
> -			pci_write_config_word(pd, PCI_COMMAND, ps->command);
> -			break;
> -			/* other header types not restored at present */
> -		}
> -	}
> -}
> -
> -/*
> - * Put the powerbook to sleep.
> - */
> -#define IRQ_ENABLE	((unsigned int *)0xf3000024)
> -#define MEM_CTRL	((unsigned int *)0xf8000070)
> -
> -int powerbook_sleep(void)
> -{
> -	int ret, i, x;
> -	static int save_backlight;
> -	static unsigned int save_irqen;
> -	unsigned long msr;
> -	unsigned int hid0;
> -	unsigned long p, wait;
> -	struct adb_request sleep_req;
> -
> -	/* Notify device drivers */
> -	ret = blocking_notifier_call_chain(&sleep_notifier_list,
> -			PBOOK_SLEEP, NULL);
> -	if (ret & NOTIFY_STOP_MASK)
> -		return -EBUSY;
> -
> -	/* Sync the disks. */
> -	/* XXX It would be nice to have some way to ensure that
> -	 * nobody is dirtying any new buffers while we wait. */
> -	sys_sync();
> -
> -	/* Turn off the display backlight */
> -	save_backlight = backlight_enabled;
> -	if (save_backlight)
> -		pmu_enable_backlight(0);
> -
> -	/* Give the disks a little time to actually finish writing */
> -	for (wait = jiffies + (HZ/4); time_before(jiffies, wait); )
> -		mb();
> -
> -	/* Disable all interrupts except pmu */
> -	save_irqen = in_le32(IRQ_ENABLE);
> -	for (i = 0; i < 32; ++i)
> -		if (i != vias->intrs[0].line && (save_irqen & (1 << i)))
> -			disable_irq(i);
> -	asm volatile("mtdec %0" : : "r" (0x7fffffff));
> -
> -	/* Save the state of PCI config space for some slots */
> -	pbook_pci_save();
> -
> -	/* Set the memory controller to keep the memory refreshed
> -	   while we're asleep */
> -	for (i = 0x403f; i >= 0x4000; --i) {
> -		out_be32(MEM_CTRL, i);
> -		do {
> -			x = (in_be32(MEM_CTRL) >> 16) & 0x3ff;
> -		} while (x == 0);
> -		if (x >= 0x100)
> -			break;
> -	}
> -
> -	/* Ask the PMU to put us to sleep */
> -	pmu_request(&sleep_req, NULL, 5, PMU_SLEEP, 'M', 'A', 'T', 'T');
> -	while (!sleep_req.complete)
> -		mb();
> -	/* displacement-flush the L2 cache - necessary? */
> -	for (p = KERNELBASE; p < KERNELBASE + 0x100000; p += 0x1000)
> -		i = *(volatile int *)p;
> -	asleep = 1;
> -
> -	/* Put the CPU into sleep mode */
> -	asm volatile("mfspr %0,1008" : "=r" (hid0) :);
> -	hid0 = (hid0 & ~(HID0_NAP | HID0_DOZE)) | HID0_SLEEP;
> -	asm volatile("mtspr 1008,%0" : : "r" (hid0));
> -	local_save_flags(msr);
> -	msr |= MSR_POW | MSR_EE;
> -	local_irq_restore(msr);
> -	udelay(10);
> -
> -	/* OK, we're awake again, start restoring things */
> -	out_be32(MEM_CTRL, 0x3f);
> -	pbook_pci_restore();
> -
> -	/* wait for the PMU interrupt sequence to complete */
> -	while (asleep)
> -		mb();
> -
> -	/* reenable interrupts */
> -	for (i = 0; i < 32; ++i)
> -		if (i != vias->intrs[0].line && (save_irqen & (1 << i)))
> -			enable_irq(i);
> -
> -	/* Notify drivers */
> -	blocking_notifier_call_chain(&sleep_notifier_list, PBOOK_WAKE, NULL);
> -
> -	/* reenable ADB autopoll */
> -	pmu_adb_autopoll(adb_dev_map);
> -
> -	/* Turn on the screen backlight, if it was on before */
> -	if (save_backlight)
> -		pmu_enable_backlight(1);
> -
> -	/* Wait for the hard disk to spin up */
> -
> -	return 0;
> -}
> -
> -/*
> - * Support for /dev/pmu device
> - */
> -static int pmu_open(struct inode *inode, struct file *file)
> -{
> -	return 0;
> -}
> -
> -static ssize_t pmu_read(struct file *file, char *buf,
> -			size_t count, loff_t *ppos)
> -{
> -	return 0;
> -}
> -
> -static ssize_t pmu_write(struct file *file, const char *buf,
> -			 size_t count, loff_t *ppos)
> -{
> -	return 0;
> -}
> -
> -static int pmu_ioctl(struct inode * inode, struct file *filp,
> -		     u_int cmd, u_long arg)
> -{
> -	int error;
> -	__u32 value;
> -
> -	switch (cmd) {
> -	    case PMU_IOC_SLEEP:
> -	    	return -ENOSYS;
> -	    case PMU_IOC_GET_BACKLIGHT:
> -		return put_user(backlight_level, (__u32 *)arg);
> -	    case PMU_IOC_SET_BACKLIGHT:
> -		error = get_user(value, (__u32 *)arg);
> -		if (!error)
> -			pmu_set_brightness(value);
> -		return error;
> -	    case PMU_IOC_GET_MODEL:
> -	    	return put_user(pmu_kind, (__u32 *)arg);
> -	}
> -	return -EINVAL;
> -}
> -
> -static const struct file_operations pmu_device_fops = {
> -	.read		= pmu_read,
> -	.write		= pmu_write,
> -	.ioctl		= pmu_ioctl,
> -	.open		= pmu_open,
> -};
> -
> -static struct miscdevice pmu_device = {
> -	PMU_MINOR, "pmu", &pmu_device_fops
> -};
> -
> -void pmu_device_init(void)
> -{
> -	if (!via)
> -		return;
> -	if (misc_register(&pmu_device) < 0)
> -		printk(KERN_ERR "via-pmu68k: cannot register misc device.\n");
> -}
> -#endif /* CONFIG_PMAC_PBOOK */
> -
> 
> --
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/5] powerpc: generic time suspend/resume code
  2007-03-19 14:47   ` Benjamin Herrenschmidt
@ 2007-03-19 21:51     ` Guennadi Liakhovetski
  2007-03-19 22:11       ` Johannes Berg
  2007-03-21 20:47       ` David Brownell
  0 siblings, 2 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2007-03-19 21:51 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: David Brownell, linuxppc-dev, Johannes Berg

On Mon, 19 Mar 2007, Benjamin Herrenschmidt wrote:

> On Mon, 2007-03-19 at 11:53 +0100, Johannes Berg wrote:
> > plain text document attachment (001-time-resume.patch)
> > This patch removes the time suspend/restore code that was done through
> > a PMU notifier in arch/platforms/powermac/time.c.
> > 
> > Instead, introduce arch/powerpc/sysdev/timer.c which creates a sys
> > device and handles time of day suspend/resume through that.
> > 
> > This should probably be replaced by using the generic RTC framework
> > but for now it gets rid of the arcane powermac specific hack.
> > 
> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> 
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Not sure I'm convinced it's a good thing to do...

As far as I understand it duplicates the functionality proposed in this 
thread:

http://lists.linux-foundation.org/pipermail/linux-pm/2007-February/thread.html#4949

and by committing it now we'll delay the powerpc migration to generic rtc. 
How about this: you give me a week before committing this (and any 
depending) patches, in 1 week I send an email with an estimate whether I'd 
be able to do the convertion (without being able to test - others will 
have to do that) and how long I expect to need for that. If we find it 
acceptible, we'll do that, if not - in one week we still can commit these 
patches - 2.6.21 either will not be there yet by then or the window will 
still be open...

David, what's the status of that your patch?

Notice also, that my linkstation suspend patch in its present form also 
uses the previous version of this patch from Johannes', but I'd rather 
delay its acceptance upstream and directly go for the ultimate solution.

Thanks
Guennadi
---
Guennadi Liakhovetski

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/5] powerpc: generic time suspend/resume code
  2007-03-19 21:51     ` Guennadi Liakhovetski
@ 2007-03-19 22:11       ` Johannes Berg
  2007-03-19 23:21         ` Guennadi Liakhovetski
  2007-03-21 20:47       ` David Brownell
  1 sibling, 1 reply; 32+ messages in thread
From: Johannes Berg @ 2007-03-19 22:11 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: David Brownell, linuxppc-dev

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

On Mon, 2007-03-19 at 22:51 +0100, Guennadi Liakhovetski wrote:

> Notice also, that my linkstation suspend patch in its present form also 
> uses the previous version of this patch from Johannes', but I'd rather 
> delay its acceptance upstream and directly go for the ultimate solution.

I just posted a whole other series of cleanup patches that depend on the
removal of the arcane pmu notifier in the timer code. I don't see the
point in holding that for much longer just because there's some big
structural change possibly coming up for the rtc code.

It's not just the generic RTC suspend code that is relevant but also the
ppc_md rtc hooks, ultimately they probably should be removed totally
once powerpc completely migrates to genrtc everywhere. In the patch you
showed me to this file you actually circumvented the ppc_md rtc hooks
because you're already using genrtc, in that case the genrtc suspend
code is obviously much more appropriate.

Since currently only 32-bit powermac even implements suspend in
mainline, the whole issue is sort of moot anyway. Once the genrtc
suspend patches is in this file can be removed with the patch that
migrates powermac to genrtc, and then new platforms wanting to implement
suspend will need to use genrtc.

johannes

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/5] powerpc: generic time suspend/resume code
  2007-03-19 22:11       ` Johannes Berg
@ 2007-03-19 23:21         ` Guennadi Liakhovetski
  0 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2007-03-19 23:21 UTC (permalink / raw)
  To: Johannes Berg; +Cc: David Brownell, linuxppc-dev

On Mon, 19 Mar 2007, Johannes Berg wrote:

> It's not just the generic RTC suspend code that is relevant but also the
> ppc_md rtc hooks, ultimately they probably should be removed totally
> once powerpc completely migrates to genrtc everywhere. In the patch you
> showed me to this file you actually circumvented the ppc_md rtc hooks
> because you're already using genrtc, in that case the genrtc suspend
> code is obviously much more appropriate.

Just to make it easier for everyone to follow below is my patch I showed 
Johannes today on IRC. I use it atm on the top of his patch to save / 
restore system time. Notice, it is not thought for upstream in any way - 
just for reference.

Another point I forgot to mention in my first reply - why does this patch 
use a special mechanism to restore the time like saving the rtc time on 
suspend, re-reading it on resume, calculating the difference and adding it 
to the current timeofday, which is, presumably, equal to suspend time? 
Isn't re-using the same method as on poweron of setting the timeofday from 
rtc more logical? Which is exactly what David's patch does?

Thanks
Guennadi
---
Guennadi Liakhovetski

(not for inclusion, for reference only, hence no "Signed-off-by")

--- a/arch/powerpc/sysdev/timer.c	2007-03-19 22:26:12.000000000 +0100
+++ b/arch/powerpc/sysdev/timer.c	2007-03-03 01:19:01.000000000 +0100
@@ -11,6 +11,24 @@
 
 static unsigned long suspend_rtc_time;
 
+static int timer_get_rtc_time(struct rtc_time *time)
+{
+	int err = 0;
+
+	if (ppc_md.get_rtc_time)
+		get_rtc_time(time);
+	else {
+		struct class_device *class_dev = rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
+
+		WARN_ON(!class_dev);
+
+		err = rtc_read_time(class_dev, time);
+		rtc_class_close(class_dev);
+	}
+
+	return err;
+}
+
 /*
  * Reset the time after a sleep.
  */
@@ -20,9 +38,12 @@
 	struct timespec ts;
 	struct rtc_time cur_rtc_tm;
 	unsigned long cur_rtc_time, diff;
+	int err;
 
 	/* get current RTC time and convert to seconds */
-	get_rtc_time(&cur_rtc_tm);
+	err = timer_get_rtc_time(&cur_rtc_tm);
+	if (err)
+		return err;
 	rtc_tm_to_time(&cur_rtc_tm, &cur_rtc_time);
 
 	diff = cur_rtc_time - suspend_rtc_time;
@@ -40,9 +61,11 @@
 static int timer_suspend(struct sys_device *dev, pm_message_t state)
 {
 	struct rtc_time suspend_rtc_tm;
-	WARN_ON(!ppc_md.get_rtc_time);
+	int err;
 
-	get_rtc_time(&suspend_rtc_tm);
+	err = timer_get_rtc_time(&suspend_rtc_tm);
+	if (err)
+		return err;
 	rtc_tm_to_time(&suspend_rtc_tm, &suspend_rtc_time);
 
 	return 0;

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 4/5] powermac: proper sleep management
  2007-03-19 10:53 ` [PATCH 4/5] powermac: proper sleep management Johannes Berg
  2007-03-19 14:50   ` Benjamin Herrenschmidt
@ 2007-03-19 23:44   ` Johannes Berg
  2007-03-20  0:11     ` [PATCH 4/5 v2] " Johannes Berg
  1 sibling, 1 reply; 32+ messages in thread
From: Johannes Berg @ 2007-03-19 23:44 UTC (permalink / raw)
  To: linuxppc-dev

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

On Mon, 2007-03-19 at 11:53 +0100, Johannes Berg wrote:
> 
> +static struct pm_ops pmu_pm_ops = {
> +       .pm_disk_mode = PM_DISK_PLATFORM, 

That shouldn't be there, not sure how it slipped it.

johannes

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 4/5 v2] powermac: proper sleep management
  2007-03-19 23:44   ` Johannes Berg
@ 2007-03-20  0:11     ` Johannes Berg
  2007-03-20  0:48       ` Johannes Berg
  0 siblings, 1 reply; 32+ messages in thread
From: Johannes Berg @ 2007-03-20  0:11 UTC (permalink / raw)
  To: linuxppc-dev

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>

---
Could use some testing on older powerbooks just to see if they get problems
with the slight reordering of the suspend/resume sequence. I doubt it though.

And before someone asks:
Yes, it is safe to remove the backlight ioctl restrictions
because the generic layer actually freezes processes before STR.

This updated version removes the sys_sync() call that can't be done with
processes frozen. This version was updated again to not allow dual
suspend (suspend to ram after writing things to disk) because that is
not supported yet.

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

--- linux-2.6.orig/drivers/macintosh/via-pmu.c	2007-03-19 19:15:06.553321419 +0100
+++ linux-2.6/drivers/macintosh/via-pmu.c	2007-03-20 00:46:53.166994679 +0100
@@ -155,9 +155,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 +1988,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 +1998,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 +2064,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 +2073,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 +2082,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 +2152,6 @@ powerbook_sleep_Core99(void)
 	/* Restore LPJ, cpufreq will adjust the cpu frequency */
 	loops_per_jiffy /= 2;
 
-	pmac_wakeup_devices();
-
 	return 0;
 }
 
@@ -2308,7 +2161,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 +2179,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 +2222,6 @@ powerbook_sleep_3400(void)
 	while (asleep)
 		mb();
 
-	pmac_wakeup_devices();
 	pbook_free_pci_save();
 	iounmap(mem_ctrl);
 
@@ -2558,6 +2403,123 @@ 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);
+
+#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();
+
+	preempt_disable();
+
+	return 0;
+}
+
+static int powerbook_sleep(suspend_state_t state)
+{
+	int error = 0;
+
+	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));
+
+	/* 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);
+
+	/* Force a poll of ADB interrupts */
+	adb_int_pending = 1;
+	via_pmu_interrupt(0, NULL);
+
+	/* Restart jiffies & scheduling */
+	wakeup_decrementer();
+
+	return 0;
+}
+
+static int powerbook_finish_sleep(suspend_state_t state)
+{
+#ifdef CONFIG_PMAC_BACKLIGHT
+	/* Tell backlight code it can use the chip again */
+	pmu_backlight_set_sleep(0);
+#endif
+
+	preempt_enable();
+
+	/* Call platform functions marked "on wake" */
+	pmac_pfunc_base_resume();
+	pmac_pfunc_i2c_resume();
+
+	/* 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,
+	.finish = powerbook_finish_sleep,
+	.enter = powerbook_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 +2529,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 +2554,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 +2565,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-03-19 19:15:00.773321419 +0100
+++ linux-2.6/Documentation/feature-removal-schedule.txt	2007-03-19 19:15:07.323321419 +0100
@@ -324,3 +324,13 @@ Why:	the i8xx_tco watchdog driver has be
 Who:	Wim Van Sebroeck <wim@iguana.be>
 
 ---------------------------
+
+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>
+
+---------------------------

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 4/5 v2] powermac: proper sleep management
  2007-03-20  0:11     ` [PATCH 4/5 v2] " Johannes Berg
@ 2007-03-20  0:48       ` Johannes Berg
  2007-03-20  2:19         ` Johannes Berg
  0 siblings, 1 reply; 32+ messages in thread
From: Johannes Berg @ 2007-03-20  0:48 UTC (permalink / raw)
  To: linuxppc-dev

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

Grmbl. Breakage in the pm_ops interface means I can't do it this way
either. Hold off on this until I get an answer from Rafael...

johannes

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 4/5 v2] powermac: proper sleep management
  2007-03-20  0:48       ` Johannes Berg
@ 2007-03-20  2:19         ` Johannes Berg
  0 siblings, 0 replies; 32+ messages in thread
From: Johannes Berg @ 2007-03-20  2:19 UTC (permalink / raw)
  To: linuxppc-dev

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

On Tue, 2007-03-20 at 01:48 +0100, Johannes Berg wrote:
> Grmbl. Breakage in the pm_ops interface means I can't do it this way
> either. Hold off on this until I get an answer from Rafael...

Nah, this patch is correct, the generic code is just totally borked.

johannes

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/5] powerpc: generic time suspend/resume code
  2007-03-19 21:51     ` Guennadi Liakhovetski
  2007-03-19 22:11       ` Johannes Berg
@ 2007-03-21 20:47       ` David Brownell
  2007-03-21 22:48         ` Guennadi Liakhovetski
  1 sibling, 1 reply; 32+ messages in thread
From: David Brownell @ 2007-03-21 20:47 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: Johannes Berg, linuxppc-dev

On Monday 19 March 2007 2:51 pm, Guennadi Liakhovetski wrote:

> David, what's the status of that your patch?

Those RTC framework changes are now in the MM tree, and I expect they will
merge soon after the 2.6.22 merge window opens.


One related topic to consider:  the I2C stack is also getting updated
to (finally!!) leverage about the driver model.  If those changes aren't
in the MM tree yet, they will be soonish.  This means that the RTCs based
on I2C will be converted to use a different config scheme(*) ... and that
board init code will need updates to support it.

Boards using RTCs that hook up without I2C could convert to the generic RTC
framework at any time after those RTC core changes merge.  But when I2C is
involved, some other conversions will be in the mix too.  Me, I have no idea
what PPC platforms combine PM, RTC, and I2C.  :)

- Dave

(*) Most of the I2C based RTCs rely on kernel command line parameters,
    because of the vagaries of I2C.  "New style" I2C drivers will replace
    command line declarations ("rv5c386 RTC on bus 0 at address 0x32")
    with ones in the board specific init logic.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/5] powerpc: generic time suspend/resume code
  2007-03-21 20:47       ` David Brownell
@ 2007-03-21 22:48         ` Guennadi Liakhovetski
  0 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2007-03-21 22:48 UTC (permalink / raw)
  To: David Brownell; +Cc: Johannes Berg, linuxppc-dev

On Wed, 21 Mar 2007, David Brownell wrote:

> (*) Most of the I2C based RTCs rely on kernel command line parameters,
>     because of the vagaries of I2C.  "New style" I2C drivers will replace
>     command line declarations ("rv5c386 RTC on bus 0 at address 0x32")
>     with ones in the board specific init logic.

[OT] I know very well what you mean! After my recent patch i2c-pxa driver 
on PXA27x CPUs supports both i2c buses. Now, there are chips, like pcf8574 
that specify 16 (!) addresses in their normal list. Which means, to use it 
on a pxa27x with one such chip on 1 address and to avoid scanning them all 
and flooding your log with terrifying error messages (smth like "retries 
exhausted", I think), you have to specify an "ignore" list on the kernel 
command line / modprobe parameter with a list of (16 - 1)*2 + 16*2 = 62 
comma separated values (e.g., if it's on 0,0x20: 
ignore=0,0x21,...1,0x20,1,0x21,...)!... Which even exceeds max length of 
the ignore array. So, I had to double its length. Of course, changes like 
that I don't submit to the mainline:-)))

Looking forward to your reworked i2c.

Thanks
Guennadi
---
Guennadi Liakhovetski

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/5] powerpc: generic time suspend/resume code
  2007-03-19 10:53 ` [PATCH 1/5] powerpc: generic time suspend/resume code Johannes Berg
  2007-03-19 14:47   ` Benjamin Herrenschmidt
@ 2007-05-02  5:02   ` Michael Ellerman
  2007-05-02  8:25     ` Johannes Berg
  1 sibling, 1 reply; 32+ messages in thread
From: Michael Ellerman @ 2007-05-02  5:02 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev

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

On Mon, 2007-03-19 at 11:53 +0100, Johannes Berg wrote:
> plain text document attachment (001-time-resume.patch)
> This patch removes the time suspend/restore code that was done through
> a PMU notifier in arch/platforms/powermac/time.c.
> 
> Instead, introduce arch/powerpc/sysdev/timer.c which creates a sys
> device and handles time of day suspend/resume through that.
> 
> This should probably be replaced by using the generic RTC framework
> but for now it gets rid of the arcane powermac specific hack.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>

This seems to break prep_defconfig:

timer.c:(.text+0x28c): undefined reference to `rtc_tm_to_time'
timer.c:(.text+0x38c): undefined reference to `rtc_tm_to_time'
make[1]: *** [.tmp_vmlinux1] Error 1

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/5] powerpc: generic time suspend/resume code
  2007-05-02  5:02   ` Michael Ellerman
@ 2007-05-02  8:25     ` Johannes Berg
  2007-05-02 11:06       ` Michael Ellerman
  0 siblings, 1 reply; 32+ messages in thread
From: Johannes Berg @ 2007-05-02  8:25 UTC (permalink / raw)
  To: michael; +Cc: linuxppc-dev

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

On Wed, 2007-05-02 at 15:02 +1000, Michael Ellerman wrote:

> This seems to break prep_defconfig:
> 
> timer.c:(.text+0x28c): undefined reference to `rtc_tm_to_time'
> timer.c:(.text+0x38c): undefined reference to `rtc_tm_to_time'
> make[1]: *** [.tmp_vmlinux1] Error 1

Odd. I put this into Kconfig:
config PPC_PM_NEEDS_RTC_LIB
        bool
        select RTC_LIB
        default y if PM

and that should cause the rtc lib that contains the rtc_tm_to_time
function to be compiled in if you have PM enabled (and otherwise timer.c
isn't compiled). Or maybe defconfig doesn't invoke oldconfig?

johannes

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/5] powerpc: generic time suspend/resume code
  2007-05-02  8:25     ` Johannes Berg
@ 2007-05-02 11:06       ` Michael Ellerman
  2007-05-02 14:04         ` Johannes Berg
  0 siblings, 1 reply; 32+ messages in thread
From: Michael Ellerman @ 2007-05-02 11:06 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev

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

On Wed, 2007-05-02 at 10:25 +0200, Johannes Berg wrote:
> On Wed, 2007-05-02 at 15:02 +1000, Michael Ellerman wrote:
> 
> > This seems to break prep_defconfig:
> > 
> > timer.c:(.text+0x28c): undefined reference to `rtc_tm_to_time'
> > timer.c:(.text+0x38c): undefined reference to `rtc_tm_to_time'
> > make[1]: *** [.tmp_vmlinux1] Error 1
> 
> Odd. I put this into Kconfig:
> config PPC_PM_NEEDS_RTC_LIB
>         bool
>         select RTC_LIB
>         default y if PM
> 
> and that should cause the rtc lib that contains the rtc_tm_to_time
> function to be compiled in if you have PM enabled (and otherwise timer.c
> isn't compiled). Or maybe defconfig doesn't invoke oldconfig?

Ah sorry, I didn't mention prep_defconfig is arch/__ppc__. 

See arch/ppc/Makefile:

core-y                          += arch/ppc/kernel/ arch/powerpc/kernel/ \
                                   arch/ppc/platforms/ \
                                   arch/ppc/mm/ arch/ppc/lib/ \
                                   arch/ppc/syslib/ arch/powerpc/sysdev/ \
                                   arch/powerpc/lib/


So I guess we need to add the Kconfig fragment on arch/ppc also?

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/5] powerpc: generic time suspend/resume code
  2007-05-02 11:06       ` Michael Ellerman
@ 2007-05-02 14:04         ` Johannes Berg
  0 siblings, 0 replies; 32+ messages in thread
From: Johannes Berg @ 2007-05-02 14:04 UTC (permalink / raw)
  To: michael; +Cc: linuxppc-dev

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

On Wed, 2007-05-02 at 21:06 +1000, Michael Ellerman wrote:

> Ah sorry, I didn't mention prep_defconfig is arch/__ppc__. 

Ahh, ok.

> See arch/ppc/Makefile:
> 
> core-y                          += arch/ppc/kernel/ arch/powerpc/kernel/ \
>                                    arch/ppc/platforms/ \
>                                    arch/ppc/mm/ arch/ppc/lib/ \
>                                    arch/ppc/syslib/ arch/powerpc/sysdev/ \
>                                    arch/powerpc/lib/
> 
> 
> So I guess we need to add the Kconfig fragment on arch/ppc also?

Yeah, but let me first see later today if I can go without rtc lib as
Paul has suggested.

johannes

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2007-05-02 14:03 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-03-19 10:53 [PATCH 0/5] powermac suspend fixes Johannes Berg
2007-03-19 10:53 ` [PATCH 1/5] powerpc: generic time suspend/resume code Johannes Berg
2007-03-19 14:47   ` Benjamin Herrenschmidt
2007-03-19 21:51     ` Guennadi Liakhovetski
2007-03-19 22:11       ` Johannes Berg
2007-03-19 23:21         ` Guennadi Liakhovetski
2007-03-21 20:47       ` David Brownell
2007-03-21 22:48         ` Guennadi Liakhovetski
2007-05-02  5:02   ` Michael Ellerman
2007-05-02  8:25     ` Johannes Berg
2007-05-02 11:06       ` Michael Ellerman
2007-05-02 14:04         ` Johannes Berg
2007-03-19 10:53 ` [PATCH 2/5] powerpc: fix suspend states again Johannes Berg
2007-03-19 14:48   ` Benjamin Herrenschmidt
2007-03-19 15:22     ` Johannes Berg
2007-03-19 15:32       ` Benjamin Herrenschmidt
2007-03-19 15:45         ` Johannes Berg
2007-03-19 15:54           ` Johannes Berg
2007-03-19 16:22             ` Johannes Berg
2007-03-19 16:39               ` Benjamin Herrenschmidt
2007-03-19 16:06   ` Benjamin Herrenschmidt
2007-03-19 10:53 ` [PATCH 3/5] powermac: disallow pmu sleep notifiers from aborting sleep Johannes Berg
2007-03-19 14:49   ` Benjamin Herrenschmidt
2007-03-19 10:53 ` [PATCH 4/5] powermac: proper sleep management Johannes Berg
2007-03-19 14:50   ` Benjamin Herrenschmidt
2007-03-19 15:16     ` Johannes Berg
2007-03-19 23:44   ` Johannes Berg
2007-03-20  0:11     ` [PATCH 4/5 v2] " Johannes Berg
2007-03-20  0:48       ` Johannes Berg
2007-03-20  2:19         ` Johannes Berg
2007-03-19 10:53 ` [PATCH 5/5] remove dead code in via-pmu68k Johannes Berg
2007-03-19 19:17   ` Brad Boyer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.