2009-06-04 16:20:40

by Martin Schwidefsky

[permalink] [raw]
Subject: [patch 01/38] pm: Move nvs routines into a seperate file.

From: Cornelia Huck <[email protected]>

The *_nvs_* routines in swsusp.c make use of the io*map()
functions, which are only provided for HAS_IOMEM, thus
breaking compilation if HAS_IOMEM is not set. Fix this
by moving the *_nvs_* routines into nvs.c, which is only
compiled if HAS_IOMEM is set.

Signed-off-by: Cornelia Huck <[email protected]>
Signed-off-by: Martin Schwidefsky <[email protected]>
---
include/linux/suspend.h | 18 ++++--
kernel/power/Kconfig | 4 +
kernel/power/Makefile | 1
kernel/power/nvs.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++
kernel/power/swsusp.c | 122 --------------------------------------------
5 files changed, 147 insertions(+), 129 deletions(-)

Index: linux-2.6/kernel/power/nvs.c
===================================================================
--- /dev/null
+++ linux-2.6/kernel/power/nvs.c
@@ -0,0 +1,131 @@
+/*
+ * Routines for NVS memory handling
+ */
+
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/suspend.h>
+
+/*
+ * Platforms, like ACPI, may want us to save some memory used by them during
+ * hibernation and to restore the contents of this memory during the subsequent
+ * resume. The code below implements a mechanism allowing us to do that.
+ */
+
+struct nvs_page {
+ unsigned long phys_start;
+ unsigned int size;
+ void *kaddr;
+ void *data;
+ struct list_head node;
+};
+
+static LIST_HEAD(nvs_list);
+
+/**
+ * hibernate_nvs_register - register platform NVS memory region to save
+ * @start - physical address of the region
+ * @size - size of the region
+ *
+ * The NVS region need not be page-aligned (both ends) and we arrange
+ * things so that the data from page-aligned addresses in this region will
+ * be copied into separate RAM pages.
+ */
+int hibernate_nvs_register(unsigned long start, unsigned long size)
+{
+ struct nvs_page *entry, *next;
+
+ while (size > 0) {
+ unsigned int nr_bytes;
+
+ entry = kzalloc(sizeof(struct nvs_page), GFP_KERNEL);
+ if (!entry)
+ goto Error;
+
+ list_add_tail(&entry->node, &nvs_list);
+ entry->phys_start = start;
+ nr_bytes = PAGE_SIZE - (start & ~PAGE_MASK);
+ entry->size = (size < nr_bytes) ? size : nr_bytes;
+
+ start += entry->size;
+ size -= entry->size;
+ }
+ return 0;
+
+ Error:
+ list_for_each_entry_safe(entry, next, &nvs_list, node) {
+ list_del(&entry->node);
+ kfree(entry);
+ }
+ return -ENOMEM;
+}
+
+/**
+ * hibernate_nvs_free - free data pages allocated for saving NVS regions
+ */
+void hibernate_nvs_free(void)
+{
+ struct nvs_page *entry;
+
+ list_for_each_entry(entry, &nvs_list, node)
+ if (entry->data) {
+ free_page((unsigned long)entry->data);
+ entry->data = NULL;
+ if (entry->kaddr) {
+ iounmap(entry->kaddr);
+ entry->kaddr = NULL;
+ }
+ }
+}
+
+/**
+ * hibernate_nvs_alloc - allocate memory necessary for saving NVS regions
+ */
+int hibernate_nvs_alloc(void)
+{
+ struct nvs_page *entry;
+
+ list_for_each_entry(entry, &nvs_list, node) {
+ entry->data = (void *)__get_free_page(GFP_KERNEL);
+ if (!entry->data) {
+ hibernate_nvs_free();
+ return -ENOMEM;
+ }
+ }
+ return 0;
+}
+
+/**
+ * hibernate_nvs_save - save NVS memory regions
+ */
+void hibernate_nvs_save(void)
+{
+ struct nvs_page *entry;
+
+ printk(KERN_INFO "PM: Saving platform NVS memory\n");
+
+ list_for_each_entry(entry, &nvs_list, node)
+ if (entry->data) {
+ entry->kaddr = ioremap(entry->phys_start, entry->size);
+ memcpy(entry->data, entry->kaddr, entry->size);
+ }
+}
+
+/**
+ * hibernate_nvs_restore - restore NVS memory regions
+ *
+ * This function is going to be called with interrupts disabled, so it
+ * cannot iounmap the virtual addresses used to access the NVS region.
+ */
+void hibernate_nvs_restore(void)
+{
+ struct nvs_page *entry;
+
+ printk(KERN_INFO "PM: Restoring platform NVS memory\n");
+
+ list_for_each_entry(entry, &nvs_list, node)
+ if (entry->data)
+ memcpy(entry->kaddr, entry->data, entry->size);
+}
Index: linux-2.6/kernel/power/swsusp.c
===================================================================
--- linux-2.6.orig/kernel/power/swsusp.c
+++ linux-2.6/kernel/power/swsusp.c
@@ -262,125 +262,3 @@ int swsusp_shrink_memory(void)

return 0;
}
-
-/*
- * Platforms, like ACPI, may want us to save some memory used by them during
- * hibernation and to restore the contents of this memory during the subsequent
- * resume. The code below implements a mechanism allowing us to do that.
- */
-
-struct nvs_page {
- unsigned long phys_start;
- unsigned int size;
- void *kaddr;
- void *data;
- struct list_head node;
-};
-
-static LIST_HEAD(nvs_list);
-
-/**
- * hibernate_nvs_register - register platform NVS memory region to save
- * @start - physical address of the region
- * @size - size of the region
- *
- * The NVS region need not be page-aligned (both ends) and we arrange
- * things so that the data from page-aligned addresses in this region will
- * be copied into separate RAM pages.
- */
-int hibernate_nvs_register(unsigned long start, unsigned long size)
-{
- struct nvs_page *entry, *next;
-
- while (size > 0) {
- unsigned int nr_bytes;
-
- entry = kzalloc(sizeof(struct nvs_page), GFP_KERNEL);
- if (!entry)
- goto Error;
-
- list_add_tail(&entry->node, &nvs_list);
- entry->phys_start = start;
- nr_bytes = PAGE_SIZE - (start & ~PAGE_MASK);
- entry->size = (size < nr_bytes) ? size : nr_bytes;
-
- start += entry->size;
- size -= entry->size;
- }
- return 0;
-
- Error:
- list_for_each_entry_safe(entry, next, &nvs_list, node) {
- list_del(&entry->node);
- kfree(entry);
- }
- return -ENOMEM;
-}
-
-/**
- * hibernate_nvs_free - free data pages allocated for saving NVS regions
- */
-void hibernate_nvs_free(void)
-{
- struct nvs_page *entry;
-
- list_for_each_entry(entry, &nvs_list, node)
- if (entry->data) {
- free_page((unsigned long)entry->data);
- entry->data = NULL;
- if (entry->kaddr) {
- iounmap(entry->kaddr);
- entry->kaddr = NULL;
- }
- }
-}
-
-/**
- * hibernate_nvs_alloc - allocate memory necessary for saving NVS regions
- */
-int hibernate_nvs_alloc(void)
-{
- struct nvs_page *entry;
-
- list_for_each_entry(entry, &nvs_list, node) {
- entry->data = (void *)__get_free_page(GFP_KERNEL);
- if (!entry->data) {
- hibernate_nvs_free();
- return -ENOMEM;
- }
- }
- return 0;
-}
-
-/**
- * hibernate_nvs_save - save NVS memory regions
- */
-void hibernate_nvs_save(void)
-{
- struct nvs_page *entry;
-
- printk(KERN_INFO "PM: Saving platform NVS memory\n");
-
- list_for_each_entry(entry, &nvs_list, node)
- if (entry->data) {
- entry->kaddr = ioremap(entry->phys_start, entry->size);
- memcpy(entry->data, entry->kaddr, entry->size);
- }
-}
-
-/**
- * hibernate_nvs_restore - restore NVS memory regions
- *
- * This function is going to be called with interrupts disabled, so it
- * cannot iounmap the virtual addresses used to access the NVS region.
- */
-void hibernate_nvs_restore(void)
-{
- struct nvs_page *entry;
-
- printk(KERN_INFO "PM: Restoring platform NVS memory\n");
-
- list_for_each_entry(entry, &nvs_list, node)
- if (entry->data)
- memcpy(entry->kaddr, entry->data, entry->size);
-}
Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -116,9 +116,13 @@ config SUSPEND_FREEZER

Turning OFF this setting is NOT recommended! If in doubt, say Y.

+config NVS
+ bool
+
config HIBERNATION
bool "Hibernation (aka 'suspend to disk')"
depends on PM && SWAP && ARCH_HIBERNATION_POSSIBLE
+ select NVS if HAS_IOMEM
---help---
Enable the suspend to disk (STD) functionality, which is usually
called "hibernation" in user interfaces. STD checkpoints the
Index: linux-2.6/kernel/power/Makefile
===================================================================
--- linux-2.6.orig/kernel/power/Makefile
+++ linux-2.6/kernel/power/Makefile
@@ -7,5 +7,6 @@ obj-$(CONFIG_PM) += main.o
obj-$(CONFIG_PM_SLEEP) += console.o
obj-$(CONFIG_FREEZER) += process.o
obj-$(CONFIG_HIBERNATION) += swsusp.o disk.o snapshot.o swap.o user.o
+obj-$(CONFIG_NVS) += nvs.o

obj-$(CONFIG_MAGIC_SYSRQ) += poweroff.o
Index: linux-2.6/include/linux/suspend.h
===================================================================
--- linux-2.6.orig/include/linux/suspend.h
+++ linux-2.6/include/linux/suspend.h
@@ -245,11 +245,6 @@ extern unsigned long get_safe_page(gfp_t

extern void hibernation_set_ops(struct platform_hibernation_ops *ops);
extern int hibernate(void);
-extern int hibernate_nvs_register(unsigned long start, unsigned long size);
-extern int hibernate_nvs_alloc(void);
-extern void hibernate_nvs_free(void);
-extern void hibernate_nvs_save(void);
-extern void hibernate_nvs_restore(void);
extern bool system_entering_hibernation(void);
#else /* CONFIG_HIBERNATION */
static inline int swsusp_page_is_forbidden(struct page *p) { return 0; }
@@ -258,6 +253,16 @@ static inline void swsusp_unset_page_fre

static inline void hibernation_set_ops(struct platform_hibernation_ops *ops) {}
static inline int hibernate(void) { return -ENOSYS; }
+static inline bool system_entering_hibernation(void) { return false; }
+#endif /* CONFIG_HIBERNATION */
+
+#ifdef CONFIG_NVS
+extern int hibernate_nvs_register(unsigned long start, unsigned long size);
+extern int hibernate_nvs_alloc(void);
+extern void hibernate_nvs_free(void);
+extern void hibernate_nvs_save(void);
+extern void hibernate_nvs_restore(void);
+#else /* CONFIG_NVS */
static inline int hibernate_nvs_register(unsigned long a, unsigned long b)
{
return 0;
@@ -266,8 +271,7 @@ static inline int hibernate_nvs_alloc(vo
static inline void hibernate_nvs_free(void) {}
static inline void hibernate_nvs_save(void) {}
static inline void hibernate_nvs_restore(void) {}
-static inline bool system_entering_hibernation(void) { return false; }
-#endif /* CONFIG_HIBERNATION */
+#endif /* CONFIG_NVS */

#ifdef CONFIG_PM_SLEEP
void save_processor_state(void);

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.


2009-06-08 15:20:44

by Pavel Machek

[permalink] [raw]
Subject: Re: [patch 01/38] pm: Move nvs routines into a seperate file.

On Thu 2009-06-04 18:18:48, Martin Schwidefsky wrote:
> From: Cornelia Huck <[email protected]>
>
> The *_nvs_* routines in swsusp.c make use of the io*map()
> functions, which are only provided for HAS_IOMEM, thus
> breaking compilation if HAS_IOMEM is not set. Fix this
> by moving the *_nvs_* routines into nvs.c, which is only
> compiled if HAS_IOMEM is set.
>
> Signed-off-by: Cornelia Huck <[email protected]>
> Signed-off-by: Martin Schwidefsky <[email protected]>
> ---
> include/linux/suspend.h | 18 ++++--
> kernel/power/Kconfig | 4 +
> kernel/power/Makefile | 1
> kernel/power/nvs.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++
> kernel/power/swsusp.c | 122 --------------------------------------------
> 5 files changed, 147 insertions(+), 129 deletions(-)
>
> Index: linux-2.6/kernel/power/nvs.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6/kernel/power/nvs.c

Ideally, filename would be a bit more descriptive.

> @@ -0,0 +1,131 @@
> +/*
> + * Routines for NVS memory handling
> + */

If you copy&pasted code, you need to copy&paste copyright notices, too.

> --- linux-2.6.orig/kernel/power/Makefile
> +++ linux-2.6/kernel/power/Makefile
> @@ -7,5 +7,6 @@ obj-$(CONFIG_PM) += main.o
> obj-$(CONFIG_PM_SLEEP) += console.o
> obj-$(CONFIG_FREEZER) += process.o
> obj-$(CONFIG_HIBERNATION) += swsusp.o disk.o snapshot.o swap.o user.o
> +obj-$(CONFIG_NVS) += nvs.o
>
> obj-$(CONFIG_MAGIC_SYSRQ) += poweroff.o

CONFIG_NVS is definitely not descriptive enough. C_HIBERNATION_NVS?

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

2009-06-08 15:37:37

by Cornelia Huck

[permalink] [raw]
Subject: Re: [patch 01/38] pm: Move nvs routines into a seperate file.

On Mon, 8 Jun 2009 08:35:01 +0200,
Pavel Machek <[email protected]> wrote:

> On Thu 2009-06-04 18:18:48, Martin Schwidefsky wrote:
> > From: Cornelia Huck <[email protected]>
> >
> > The *_nvs_* routines in swsusp.c make use of the io*map()
> > functions, which are only provided for HAS_IOMEM, thus
> > breaking compilation if HAS_IOMEM is not set. Fix this
> > by moving the *_nvs_* routines into nvs.c, which is only
> > compiled if HAS_IOMEM is set.
> >
> > Signed-off-by: Cornelia Huck <[email protected]>
> > Signed-off-by: Martin Schwidefsky <[email protected]>
> > ---
> > include/linux/suspend.h | 18 ++++--
> > kernel/power/Kconfig | 4 +
> > kernel/power/Makefile | 1
> > kernel/power/nvs.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++
> > kernel/power/swsusp.c | 122 --------------------------------------------
> > 5 files changed, 147 insertions(+), 129 deletions(-)
> >
> > Index: linux-2.6/kernel/power/nvs.c
> > ===================================================================
> > --- /dev/null
> > +++ linux-2.6/kernel/power/nvs.c
>
> Ideally, filename would be a bit more descriptive.

hibernate_nvs.c?

>
> > @@ -0,0 +1,131 @@
> > +/*
> > + * Routines for NVS memory handling
> > + */
>
> If you copy&pasted code, you need to copy&paste copyright notices, too.

Which ones? Rafael, it seems you wrote the NVS code - which copyright
should I use?

>
> > --- linux-2.6.orig/kernel/power/Makefile
> > +++ linux-2.6/kernel/power/Makefile
> > @@ -7,5 +7,6 @@ obj-$(CONFIG_PM) += main.o
> > obj-$(CONFIG_PM_SLEEP) += console.o
> > obj-$(CONFIG_FREEZER) += process.o
> > obj-$(CONFIG_HIBERNATION) += swsusp.o disk.o snapshot.o swap.o user.o
> > +obj-$(CONFIG_NVS) += nvs.o
> >
> > obj-$(CONFIG_MAGIC_SYSRQ) += poweroff.o
>
> CONFIG_NVS is definitely not descriptive enough. C_HIBERNATION_NVS?

It's a hidden config variable - but I can change it to
CONFIG_HIBERNATION_NVS.

2009-06-08 18:48:26

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [patch 01/38] pm: Move nvs routines into a seperate file.

On Monday 08 June 2009, Cornelia Huck wrote:
> On Mon, 8 Jun 2009 08:35:01 +0200,
> Pavel Machek <[email protected]> wrote:
>
> > On Thu 2009-06-04 18:18:48, Martin Schwidefsky wrote:
> > > From: Cornelia Huck <[email protected]>
> > >
> > > The *_nvs_* routines in swsusp.c make use of the io*map()
> > > functions, which are only provided for HAS_IOMEM, thus
> > > breaking compilation if HAS_IOMEM is not set. Fix this
> > > by moving the *_nvs_* routines into nvs.c, which is only
> > > compiled if HAS_IOMEM is set.
> > >
> > > Signed-off-by: Cornelia Huck <[email protected]>
> > > Signed-off-by: Martin Schwidefsky <[email protected]>
> > > ---
> > > include/linux/suspend.h | 18 ++++--
> > > kernel/power/Kconfig | 4 +
> > > kernel/power/Makefile | 1
> > > kernel/power/nvs.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > kernel/power/swsusp.c | 122 --------------------------------------------
> > > 5 files changed, 147 insertions(+), 129 deletions(-)
> > >
> > > Index: linux-2.6/kernel/power/nvs.c
> > > ===================================================================
> > > --- /dev/null
> > > +++ linux-2.6/kernel/power/nvs.c
> >
> > Ideally, filename would be a bit more descriptive.
>
> hibernate_nvs.c?

Yes, please.

> > > @@ -0,0 +1,131 @@
> > > +/*
> > > + * Routines for NVS memory handling
> > > + */
> >
> > If you copy&pasted code, you need to copy&paste copyright notices, too.
>
> Which ones? Rafael, it seems you wrote the NVS code - which copyright
> should I use?

Same as in kernel/irq/pm.c , please.

> > > --- linux-2.6.orig/kernel/power/Makefile
> > > +++ linux-2.6/kernel/power/Makefile
> > > @@ -7,5 +7,6 @@ obj-$(CONFIG_PM) += main.o
> > > obj-$(CONFIG_PM_SLEEP) += console.o
> > > obj-$(CONFIG_FREEZER) += process.o
> > > obj-$(CONFIG_HIBERNATION) += swsusp.o disk.o snapshot.o swap.o user.o
> > > +obj-$(CONFIG_NVS) += nvs.o
> > >
> > > obj-$(CONFIG_MAGIC_SYSRQ) += poweroff.o
> >
> > CONFIG_NVS is definitely not descriptive enough. C_HIBERNATION_NVS?
>
> It's a hidden config variable - but I can change it to
> CONFIG_HIBERNATION_NVS.

Please do that too.

When you're done, please send the patch to me and I'll put it into the suspend
tree for 2.6.31.

Best,
Rafael

2009-06-09 08:41:16

by Cornelia Huck

[permalink] [raw]
Subject: [PATCH v2] pm: Move nvs routines into a seperate file.

The *_nvs_* routines in swsusp.c make use of the io*map()
functions, which are only provided for HAS_IOMEM, thus
breaking compilation if HAS_IOMEM is not set. Fix this
by moving the *_nvs_* routines into hibernation_nvs.c, which
is only compiled if HAS_IOMEM is set.

Signed-off-by: Cornelia Huck <[email protected]>

---
include/linux/suspend.h | 18 +++--
kernel/power/Kconfig | 4 +
kernel/power/Makefile | 1
kernel/power/hibernation_nvs.c | 135 +++++++++++++++++++++++++++++++++++++++++
kernel/power/swsusp.c | 122 -------------------------------------
5 files changed, 151 insertions(+), 129 deletions(-)

--- /dev/null
+++ linux-suspend/kernel/power/hibernation_nvs.c
@@ -0,0 +1,135 @@
+/*
+ * linux/kernel/power/hibernation_nvs.c
+ *
+ * Copyright (C) 2008,2009 Rafael J. Wysocki <[email protected]>, Novell Inc.
+ *
+ * Routines for NVS memory handling
+ */
+
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/suspend.h>
+
+/*
+ * Platforms, like ACPI, may want us to save some memory used by them during
+ * hibernation and to restore the contents of this memory during the subsequent
+ * resume. The code below implements a mechanism allowing us to do that.
+ */
+
+struct nvs_page {
+ unsigned long phys_start;
+ unsigned int size;
+ void *kaddr;
+ void *data;
+ struct list_head node;
+};
+
+static LIST_HEAD(nvs_list);
+
+/**
+ * hibernate_nvs_register - register platform NVS memory region to save
+ * @start - physical address of the region
+ * @size - size of the region
+ *
+ * The NVS region need not be page-aligned (both ends) and we arrange
+ * things so that the data from page-aligned addresses in this region will
+ * be copied into separate RAM pages.
+ */
+int hibernate_nvs_register(unsigned long start, unsigned long size)
+{
+ struct nvs_page *entry, *next;
+
+ while (size > 0) {
+ unsigned int nr_bytes;
+
+ entry = kzalloc(sizeof(struct nvs_page), GFP_KERNEL);
+ if (!entry)
+ goto Error;
+
+ list_add_tail(&entry->node, &nvs_list);
+ entry->phys_start = start;
+ nr_bytes = PAGE_SIZE - (start & ~PAGE_MASK);
+ entry->size = (size < nr_bytes) ? size : nr_bytes;
+
+ start += entry->size;
+ size -= entry->size;
+ }
+ return 0;
+
+ Error:
+ list_for_each_entry_safe(entry, next, &nvs_list, node) {
+ list_del(&entry->node);
+ kfree(entry);
+ }
+ return -ENOMEM;
+}
+
+/**
+ * hibernate_nvs_free - free data pages allocated for saving NVS regions
+ */
+void hibernate_nvs_free(void)
+{
+ struct nvs_page *entry;
+
+ list_for_each_entry(entry, &nvs_list, node)
+ if (entry->data) {
+ free_page((unsigned long)entry->data);
+ entry->data = NULL;
+ if (entry->kaddr) {
+ iounmap(entry->kaddr);
+ entry->kaddr = NULL;
+ }
+ }
+}
+
+/**
+ * hibernate_nvs_alloc - allocate memory necessary for saving NVS regions
+ */
+int hibernate_nvs_alloc(void)
+{
+ struct nvs_page *entry;
+
+ list_for_each_entry(entry, &nvs_list, node) {
+ entry->data = (void *)__get_free_page(GFP_KERNEL);
+ if (!entry->data) {
+ hibernate_nvs_free();
+ return -ENOMEM;
+ }
+ }
+ return 0;
+}
+
+/**
+ * hibernate_nvs_save - save NVS memory regions
+ */
+void hibernate_nvs_save(void)
+{
+ struct nvs_page *entry;
+
+ printk(KERN_INFO "PM: Saving platform NVS memory\n");
+
+ list_for_each_entry(entry, &nvs_list, node)
+ if (entry->data) {
+ entry->kaddr = ioremap(entry->phys_start, entry->size);
+ memcpy(entry->data, entry->kaddr, entry->size);
+ }
+}
+
+/**
+ * hibernate_nvs_restore - restore NVS memory regions
+ *
+ * This function is going to be called with interrupts disabled, so it
+ * cannot iounmap the virtual addresses used to access the NVS region.
+ */
+void hibernate_nvs_restore(void)
+{
+ struct nvs_page *entry;
+
+ printk(KERN_INFO "PM: Restoring platform NVS memory\n");
+
+ list_for_each_entry(entry, &nvs_list, node)
+ if (entry->data)
+ memcpy(entry->kaddr, entry->data, entry->size);
+}
--- linux-suspend.orig/kernel/power/swsusp.c
+++ linux-suspend/kernel/power/swsusp.c
@@ -186,125 +186,3 @@ void swsusp_show_speed(struct timeval *s
centisecs / 100, centisecs % 100,
kps / 1000, (kps % 1000) / 10);
}
-
-/*
- * Platforms, like ACPI, may want us to save some memory used by them during
- * hibernation and to restore the contents of this memory during the subsequent
- * resume. The code below implements a mechanism allowing us to do that.
- */
-
-struct nvs_page {
- unsigned long phys_start;
- unsigned int size;
- void *kaddr;
- void *data;
- struct list_head node;
-};
-
-static LIST_HEAD(nvs_list);
-
-/**
- * hibernate_nvs_register - register platform NVS memory region to save
- * @start - physical address of the region
- * @size - size of the region
- *
- * The NVS region need not be page-aligned (both ends) and we arrange
- * things so that the data from page-aligned addresses in this region will
- * be copied into separate RAM pages.
- */
-int hibernate_nvs_register(unsigned long start, unsigned long size)
-{
- struct nvs_page *entry, *next;
-
- while (size > 0) {
- unsigned int nr_bytes;
-
- entry = kzalloc(sizeof(struct nvs_page), GFP_KERNEL);
- if (!entry)
- goto Error;
-
- list_add_tail(&entry->node, &nvs_list);
- entry->phys_start = start;
- nr_bytes = PAGE_SIZE - (start & ~PAGE_MASK);
- entry->size = (size < nr_bytes) ? size : nr_bytes;
-
- start += entry->size;
- size -= entry->size;
- }
- return 0;
-
- Error:
- list_for_each_entry_safe(entry, next, &nvs_list, node) {
- list_del(&entry->node);
- kfree(entry);
- }
- return -ENOMEM;
-}
-
-/**
- * hibernate_nvs_free - free data pages allocated for saving NVS regions
- */
-void hibernate_nvs_free(void)
-{
- struct nvs_page *entry;
-
- list_for_each_entry(entry, &nvs_list, node)
- if (entry->data) {
- free_page((unsigned long)entry->data);
- entry->data = NULL;
- if (entry->kaddr) {
- iounmap(entry->kaddr);
- entry->kaddr = NULL;
- }
- }
-}
-
-/**
- * hibernate_nvs_alloc - allocate memory necessary for saving NVS regions
- */
-int hibernate_nvs_alloc(void)
-{
- struct nvs_page *entry;
-
- list_for_each_entry(entry, &nvs_list, node) {
- entry->data = (void *)__get_free_page(GFP_KERNEL);
- if (!entry->data) {
- hibernate_nvs_free();
- return -ENOMEM;
- }
- }
- return 0;
-}
-
-/**
- * hibernate_nvs_save - save NVS memory regions
- */
-void hibernate_nvs_save(void)
-{
- struct nvs_page *entry;
-
- printk(KERN_INFO "PM: Saving platform NVS memory\n");
-
- list_for_each_entry(entry, &nvs_list, node)
- if (entry->data) {
- entry->kaddr = ioremap(entry->phys_start, entry->size);
- memcpy(entry->data, entry->kaddr, entry->size);
- }
-}
-
-/**
- * hibernate_nvs_restore - restore NVS memory regions
- *
- * This function is going to be called with interrupts disabled, so it
- * cannot iounmap the virtual addresses used to access the NVS region.
- */
-void hibernate_nvs_restore(void)
-{
- struct nvs_page *entry;
-
- printk(KERN_INFO "PM: Restoring platform NVS memory\n");
-
- list_for_each_entry(entry, &nvs_list, node)
- if (entry->data)
- memcpy(entry->kaddr, entry->data, entry->size);
-}
--- linux-suspend.orig/kernel/power/Kconfig
+++ linux-suspend/kernel/power/Kconfig
@@ -116,9 +116,13 @@ config SUSPEND_FREEZER

Turning OFF this setting is NOT recommended! If in doubt, say Y.

+config HIBERNATION_NVS
+ bool
+
config HIBERNATION
bool "Hibernation (aka 'suspend to disk')"
depends on PM && SWAP && ARCH_HIBERNATION_POSSIBLE
+ select HIBERNATION_NVS if HAS_IOMEM
---help---
Enable the suspend to disk (STD) functionality, which is usually
called "hibernation" in user interfaces. STD checkpoints the
--- linux-suspend.orig/kernel/power/Makefile
+++ linux-suspend/kernel/power/Makefile
@@ -7,5 +7,6 @@ obj-$(CONFIG_PM) += main.o
obj-$(CONFIG_PM_SLEEP) += console.o
obj-$(CONFIG_FREEZER) += process.o
obj-$(CONFIG_HIBERNATION) += swsusp.o disk.o snapshot.o swap.o user.o
+obj-$(CONFIG_HIBERNATION_NVS) += hibernation_nvs.o

obj-$(CONFIG_MAGIC_SYSRQ) += poweroff.o
--- linux-suspend.orig/include/linux/suspend.h
+++ linux-suspend/include/linux/suspend.h
@@ -245,11 +245,6 @@ extern unsigned long get_safe_page(gfp_t

extern void hibernation_set_ops(struct platform_hibernation_ops *ops);
extern int hibernate(void);
-extern int hibernate_nvs_register(unsigned long start, unsigned long size);
-extern int hibernate_nvs_alloc(void);
-extern void hibernate_nvs_free(void);
-extern void hibernate_nvs_save(void);
-extern void hibernate_nvs_restore(void);
extern bool system_entering_hibernation(void);
#else /* CONFIG_HIBERNATION */
static inline int swsusp_page_is_forbidden(struct page *p) { return 0; }
@@ -258,6 +253,16 @@ static inline void swsusp_unset_page_fre

static inline void hibernation_set_ops(struct platform_hibernation_ops *ops) {}
static inline int hibernate(void) { return -ENOSYS; }
+static inline bool system_entering_hibernation(void) { return false; }
+#endif /* CONFIG_HIBERNATION */
+
+#ifdef CONFIG_HIBERNATION_NVS
+extern int hibernate_nvs_register(unsigned long start, unsigned long size);
+extern int hibernate_nvs_alloc(void);
+extern void hibernate_nvs_free(void);
+extern void hibernate_nvs_save(void);
+extern void hibernate_nvs_restore(void);
+#else /* CONFIG_HIBERNATION_NVS */
static inline int hibernate_nvs_register(unsigned long a, unsigned long b)
{
return 0;
@@ -266,8 +271,7 @@ static inline int hibernate_nvs_alloc(vo
static inline void hibernate_nvs_free(void) {}
static inline void hibernate_nvs_save(void) {}
static inline void hibernate_nvs_restore(void) {}
-static inline bool system_entering_hibernation(void) { return false; }
-#endif /* CONFIG_HIBERNATION */
+#endif /* CONFIG_HIBERNATION_NVS */

#ifdef CONFIG_PM_SLEEP
void save_processor_state(void);

2009-06-09 19:58:28

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v2] pm: Move nvs routines into a seperate file.

On Tue 2009-06-09 10:40:03, Cornelia Huck wrote:
> The *_nvs_* routines in swsusp.c make use of the io*map()
> functions, which are only provided for HAS_IOMEM, thus
> breaking compilation if HAS_IOMEM is not set. Fix this
> by moving the *_nvs_* routines into hibernation_nvs.c, which
> is only compiled if HAS_IOMEM is set.
>
> Signed-off-by: Cornelia Huck <[email protected]>

Acked-by: Pavel Machek <[email protected]>

> @@ -0,0 +1,135 @@
> +/*
> + * linux/kernel/power/hibernation_nvs.c
> + *
> + * Copyright (C) 2008,2009 Rafael J. Wysocki <[email protected]>, Novell Inc.
> + *
> + * Routines for NVS memory handling
> + */

Specifying GPLv2 here would be nice. If it contains copyright, it
should contain license, too.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2009-06-09 23:09:55

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2] pm: Move nvs routines into a seperate file.

On Tuesday 09 June 2009, Cornelia Huck wrote:
> The *_nvs_* routines in swsusp.c make use of the io*map()
> functions, which are only provided for HAS_IOMEM, thus
> breaking compilation if HAS_IOMEM is not set. Fix this
> by moving the *_nvs_* routines into hibernation_nvs.c, which
> is only compiled if HAS_IOMEM is set.
>
> Signed-off-by: Cornelia Huck <[email protected]>

Thanks, I added the GPLv2 line to the header comment and changed the name
of the file to hibernate_nvs.c (to match the other changes in the works).

I'll carry out some compilation testing on it and put it into the tree shortly.

Best,
Rafael

> ---
> include/linux/suspend.h | 18 +++--
> kernel/power/Kconfig | 4 +
> kernel/power/Makefile | 1
> kernel/power/hibernation_nvs.c | 135 +++++++++++++++++++++++++++++++++++++++++
> kernel/power/swsusp.c | 122 -------------------------------------
> 5 files changed, 151 insertions(+), 129 deletions(-)
>
> --- /dev/null
> +++ linux-suspend/kernel/power/hibernation_nvs.c
> @@ -0,0 +1,135 @@
> +/*
> + * linux/kernel/power/hibernation_nvs.c
> + *
> + * Copyright (C) 2008,2009 Rafael J. Wysocki <[email protected]>, Novell Inc.
> + *
> + * Routines for NVS memory handling
> + */
> +
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/mm.h>
> +#include <linux/suspend.h>
> +
> +/*
> + * Platforms, like ACPI, may want us to save some memory used by them during
> + * hibernation and to restore the contents of this memory during the subsequent
> + * resume. The code below implements a mechanism allowing us to do that.
> + */
> +
> +struct nvs_page {
> + unsigned long phys_start;
> + unsigned int size;
> + void *kaddr;
> + void *data;
> + struct list_head node;
> +};
> +
> +static LIST_HEAD(nvs_list);
> +
> +/**
> + * hibernate_nvs_register - register platform NVS memory region to save
> + * @start - physical address of the region
> + * @size - size of the region
> + *
> + * The NVS region need not be page-aligned (both ends) and we arrange
> + * things so that the data from page-aligned addresses in this region will
> + * be copied into separate RAM pages.
> + */
> +int hibernate_nvs_register(unsigned long start, unsigned long size)
> +{
> + struct nvs_page *entry, *next;
> +
> + while (size > 0) {
> + unsigned int nr_bytes;
> +
> + entry = kzalloc(sizeof(struct nvs_page), GFP_KERNEL);
> + if (!entry)
> + goto Error;
> +
> + list_add_tail(&entry->node, &nvs_list);
> + entry->phys_start = start;
> + nr_bytes = PAGE_SIZE - (start & ~PAGE_MASK);
> + entry->size = (size < nr_bytes) ? size : nr_bytes;
> +
> + start += entry->size;
> + size -= entry->size;
> + }
> + return 0;
> +
> + Error:
> + list_for_each_entry_safe(entry, next, &nvs_list, node) {
> + list_del(&entry->node);
> + kfree(entry);
> + }
> + return -ENOMEM;
> +}
> +
> +/**
> + * hibernate_nvs_free - free data pages allocated for saving NVS regions
> + */
> +void hibernate_nvs_free(void)
> +{
> + struct nvs_page *entry;
> +
> + list_for_each_entry(entry, &nvs_list, node)
> + if (entry->data) {
> + free_page((unsigned long)entry->data);
> + entry->data = NULL;
> + if (entry->kaddr) {
> + iounmap(entry->kaddr);
> + entry->kaddr = NULL;
> + }
> + }
> +}
> +
> +/**
> + * hibernate_nvs_alloc - allocate memory necessary for saving NVS regions
> + */
> +int hibernate_nvs_alloc(void)
> +{
> + struct nvs_page *entry;
> +
> + list_for_each_entry(entry, &nvs_list, node) {
> + entry->data = (void *)__get_free_page(GFP_KERNEL);
> + if (!entry->data) {
> + hibernate_nvs_free();
> + return -ENOMEM;
> + }
> + }
> + return 0;
> +}
> +
> +/**
> + * hibernate_nvs_save - save NVS memory regions
> + */
> +void hibernate_nvs_save(void)
> +{
> + struct nvs_page *entry;
> +
> + printk(KERN_INFO "PM: Saving platform NVS memory\n");
> +
> + list_for_each_entry(entry, &nvs_list, node)
> + if (entry->data) {
> + entry->kaddr = ioremap(entry->phys_start, entry->size);
> + memcpy(entry->data, entry->kaddr, entry->size);
> + }
> +}
> +
> +/**
> + * hibernate_nvs_restore - restore NVS memory regions
> + *
> + * This function is going to be called with interrupts disabled, so it
> + * cannot iounmap the virtual addresses used to access the NVS region.
> + */
> +void hibernate_nvs_restore(void)
> +{
> + struct nvs_page *entry;
> +
> + printk(KERN_INFO "PM: Restoring platform NVS memory\n");
> +
> + list_for_each_entry(entry, &nvs_list, node)
> + if (entry->data)
> + memcpy(entry->kaddr, entry->data, entry->size);
> +}
> --- linux-suspend.orig/kernel/power/swsusp.c
> +++ linux-suspend/kernel/power/swsusp.c
> @@ -186,125 +186,3 @@ void swsusp_show_speed(struct timeval *s
> centisecs / 100, centisecs % 100,
> kps / 1000, (kps % 1000) / 10);
> }
> -
> -/*
> - * Platforms, like ACPI, may want us to save some memory used by them during
> - * hibernation and to restore the contents of this memory during the subsequent
> - * resume. The code below implements a mechanism allowing us to do that.
> - */
> -
> -struct nvs_page {
> - unsigned long phys_start;
> - unsigned int size;
> - void *kaddr;
> - void *data;
> - struct list_head node;
> -};
> -
> -static LIST_HEAD(nvs_list);
> -
> -/**
> - * hibernate_nvs_register - register platform NVS memory region to save
> - * @start - physical address of the region
> - * @size - size of the region
> - *
> - * The NVS region need not be page-aligned (both ends) and we arrange
> - * things so that the data from page-aligned addresses in this region will
> - * be copied into separate RAM pages.
> - */
> -int hibernate_nvs_register(unsigned long start, unsigned long size)
> -{
> - struct nvs_page *entry, *next;
> -
> - while (size > 0) {
> - unsigned int nr_bytes;
> -
> - entry = kzalloc(sizeof(struct nvs_page), GFP_KERNEL);
> - if (!entry)
> - goto Error;
> -
> - list_add_tail(&entry->node, &nvs_list);
> - entry->phys_start = start;
> - nr_bytes = PAGE_SIZE - (start & ~PAGE_MASK);
> - entry->size = (size < nr_bytes) ? size : nr_bytes;
> -
> - start += entry->size;
> - size -= entry->size;
> - }
> - return 0;
> -
> - Error:
> - list_for_each_entry_safe(entry, next, &nvs_list, node) {
> - list_del(&entry->node);
> - kfree(entry);
> - }
> - return -ENOMEM;
> -}
> -
> -/**
> - * hibernate_nvs_free - free data pages allocated for saving NVS regions
> - */
> -void hibernate_nvs_free(void)
> -{
> - struct nvs_page *entry;
> -
> - list_for_each_entry(entry, &nvs_list, node)
> - if (entry->data) {
> - free_page((unsigned long)entry->data);
> - entry->data = NULL;
> - if (entry->kaddr) {
> - iounmap(entry->kaddr);
> - entry->kaddr = NULL;
> - }
> - }
> -}
> -
> -/**
> - * hibernate_nvs_alloc - allocate memory necessary for saving NVS regions
> - */
> -int hibernate_nvs_alloc(void)
> -{
> - struct nvs_page *entry;
> -
> - list_for_each_entry(entry, &nvs_list, node) {
> - entry->data = (void *)__get_free_page(GFP_KERNEL);
> - if (!entry->data) {
> - hibernate_nvs_free();
> - return -ENOMEM;
> - }
> - }
> - return 0;
> -}
> -
> -/**
> - * hibernate_nvs_save - save NVS memory regions
> - */
> -void hibernate_nvs_save(void)
> -{
> - struct nvs_page *entry;
> -
> - printk(KERN_INFO "PM: Saving platform NVS memory\n");
> -
> - list_for_each_entry(entry, &nvs_list, node)
> - if (entry->data) {
> - entry->kaddr = ioremap(entry->phys_start, entry->size);
> - memcpy(entry->data, entry->kaddr, entry->size);
> - }
> -}
> -
> -/**
> - * hibernate_nvs_restore - restore NVS memory regions
> - *
> - * This function is going to be called with interrupts disabled, so it
> - * cannot iounmap the virtual addresses used to access the NVS region.
> - */
> -void hibernate_nvs_restore(void)
> -{
> - struct nvs_page *entry;
> -
> - printk(KERN_INFO "PM: Restoring platform NVS memory\n");
> -
> - list_for_each_entry(entry, &nvs_list, node)
> - if (entry->data)
> - memcpy(entry->kaddr, entry->data, entry->size);
> -}
> --- linux-suspend.orig/kernel/power/Kconfig
> +++ linux-suspend/kernel/power/Kconfig
> @@ -116,9 +116,13 @@ config SUSPEND_FREEZER
>
> Turning OFF this setting is NOT recommended! If in doubt, say Y.
>
> +config HIBERNATION_NVS
> + bool
> +
> config HIBERNATION
> bool "Hibernation (aka 'suspend to disk')"
> depends on PM && SWAP && ARCH_HIBERNATION_POSSIBLE
> + select HIBERNATION_NVS if HAS_IOMEM
> ---help---
> Enable the suspend to disk (STD) functionality, which is usually
> called "hibernation" in user interfaces. STD checkpoints the
> --- linux-suspend.orig/kernel/power/Makefile
> +++ linux-suspend/kernel/power/Makefile
> @@ -7,5 +7,6 @@ obj-$(CONFIG_PM) += main.o
> obj-$(CONFIG_PM_SLEEP) += console.o
> obj-$(CONFIG_FREEZER) += process.o
> obj-$(CONFIG_HIBERNATION) += swsusp.o disk.o snapshot.o swap.o user.o
> +obj-$(CONFIG_HIBERNATION_NVS) += hibernation_nvs.o
>
> obj-$(CONFIG_MAGIC_SYSRQ) += poweroff.o
> --- linux-suspend.orig/include/linux/suspend.h
> +++ linux-suspend/include/linux/suspend.h
> @@ -245,11 +245,6 @@ extern unsigned long get_safe_page(gfp_t
>
> extern void hibernation_set_ops(struct platform_hibernation_ops *ops);
> extern int hibernate(void);
> -extern int hibernate_nvs_register(unsigned long start, unsigned long size);
> -extern int hibernate_nvs_alloc(void);
> -extern void hibernate_nvs_free(void);
> -extern void hibernate_nvs_save(void);
> -extern void hibernate_nvs_restore(void);
> extern bool system_entering_hibernation(void);
> #else /* CONFIG_HIBERNATION */
> static inline int swsusp_page_is_forbidden(struct page *p) { return 0; }
> @@ -258,6 +253,16 @@ static inline void swsusp_unset_page_fre
>
> static inline void hibernation_set_ops(struct platform_hibernation_ops *ops) {}
> static inline int hibernate(void) { return -ENOSYS; }
> +static inline bool system_entering_hibernation(void) { return false; }
> +#endif /* CONFIG_HIBERNATION */
> +
> +#ifdef CONFIG_HIBERNATION_NVS
> +extern int hibernate_nvs_register(unsigned long start, unsigned long size);
> +extern int hibernate_nvs_alloc(void);
> +extern void hibernate_nvs_free(void);
> +extern void hibernate_nvs_save(void);
> +extern void hibernate_nvs_restore(void);
> +#else /* CONFIG_HIBERNATION_NVS */
> static inline int hibernate_nvs_register(unsigned long a, unsigned long b)
> {
> return 0;
> @@ -266,8 +271,7 @@ static inline int hibernate_nvs_alloc(vo
> static inline void hibernate_nvs_free(void) {}
> static inline void hibernate_nvs_save(void) {}
> static inline void hibernate_nvs_restore(void) {}
> -static inline bool system_entering_hibernation(void) { return false; }
> -#endif /* CONFIG_HIBERNATION */
> +#endif /* CONFIG_HIBERNATION_NVS */
>
> #ifdef CONFIG_PM_SLEEP
> void save_processor_state(void);
>
>


--
Everyone knows that debugging is twice as hard as writing a program
in the first place. So if you're as clever as you can be when you write it,
how will you ever debug it? --- Brian Kernighan

2009-06-11 13:33:18

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH v2] pm: Move nvs routines into a seperate file.

On Wed, Jun 10, 2009 at 01:09:19AM +0200, Rafael J. Wysocki wrote:
> On Tuesday 09 June 2009, Cornelia Huck wrote:
> > The *_nvs_* routines in swsusp.c make use of the io*map()
> > functions, which are only provided for HAS_IOMEM, thus
> > breaking compilation if HAS_IOMEM is not set. Fix this
> > by moving the *_nvs_* routines into hibernation_nvs.c, which
> > is only compiled if HAS_IOMEM is set.
> >
> > Signed-off-by: Cornelia Huck <[email protected]>
>
> Thanks, I added the GPLv2 line to the header comment and changed the name
> of the file to hibernate_nvs.c (to match the other changes in the works).
>
> I'll carry out some compilation testing on it and put it into the tree shortly.

Rafael, could you add the patch below as well?
Or should that go in via git390?

Subject: [PATCH] PM: add empty suspend/resume device irq functions

From: Heiko Carstens <[email protected]>

git commit 0a0c5168 "PM: Introduce functions for suspending and resuming
device interrupts" introduced some helper functions. However these
functions are only available for architectures which support
GENERIC_HARDIRQS.

Other architectures will see this build error:

drivers/built-in.o: In function `sysdev_suspend':
(.text+0x15138): undefined reference to `check_wakeup_irqs'
drivers/built-in.o: In function `device_power_up':
(.text+0x1cb66): undefined reference to `resume_device_irqs'
drivers/built-in.o: In function `device_power_down':
(.text+0x1cb92): undefined reference to `suspend_device_irqs'

To fix this add some empty inline functions for !GENERIC_HARDIRQS.

Signed-off-by: Heiko Carstens <[email protected]>
---

include/linux/interrupt.h | 6 ++++++
1 file changed, 6 insertions(+)

diff -urpN linux-2.6/include/linux/interrupt.h linux-2.6-patched/include/linux/interrupt.h
--- linux-2.6/include/linux/interrupt.h 2009-06-11 13:07:46.000000000 +0200
+++ linux-2.6-patched/include/linux/interrupt.h 2009-06-11 13:08:31.000000000 +0200
@@ -183,6 +183,7 @@ extern void disable_irq(unsigned int irq
extern void enable_irq(unsigned int irq);

/* The following three functions are for the core kernel use only. */
+#ifdef CONFIG_GENERIC_HARDIRQS
extern void suspend_device_irqs(void);
extern void resume_device_irqs(void);
#ifdef CONFIG_PM_SLEEP
@@ -190,6 +191,11 @@ extern int check_wakeup_irqs(void);
#else
static inline int check_wakeup_irqs(void) { return 0; }
#endif
+#else
+static inline void suspend_device_irqs(void) { };
+static inline void resume_device_irqs(void) { };
+static inline int check_wakeup_irqs(void) { return 0; }
+#endif

#if defined(CONFIG_SMP) && defined(CONFIG_GENERIC_HARDIRQS)

2009-06-11 19:57:40

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2] pm: Move nvs routines into a seperate file.

On Thursday 11 June 2009, Heiko Carstens wrote:
> On Wed, Jun 10, 2009 at 01:09:19AM +0200, Rafael J. Wysocki wrote:
> > On Tuesday 09 June 2009, Cornelia Huck wrote:
> > > The *_nvs_* routines in swsusp.c make use of the io*map()
> > > functions, which are only provided for HAS_IOMEM, thus
> > > breaking compilation if HAS_IOMEM is not set. Fix this
> > > by moving the *_nvs_* routines into hibernation_nvs.c, which
> > > is only compiled if HAS_IOMEM is set.
> > >
> > > Signed-off-by: Cornelia Huck <[email protected]>
> >
> > Thanks, I added the GPLv2 line to the header comment and changed the name
> > of the file to hibernate_nvs.c (to match the other changes in the works).
> >
> > I'll carry out some compilation testing on it and put it into the tree shortly.
>
> Rafael, could you add the patch below as well?
> Or should that go in via git390?
>
> Subject: [PATCH] PM: add empty suspend/resume device irq functions
>
> From: Heiko Carstens <[email protected]>
>
> git commit 0a0c5168 "PM: Introduce functions for suspending and resuming
> device interrupts" introduced some helper functions. However these
> functions are only available for architectures which support
> GENERIC_HARDIRQS.
>
> Other architectures will see this build error:
>
> drivers/built-in.o: In function `sysdev_suspend':
> (.text+0x15138): undefined reference to `check_wakeup_irqs'
> drivers/built-in.o: In function `device_power_up':
> (.text+0x1cb66): undefined reference to `resume_device_irqs'
> drivers/built-in.o: In function `device_power_down':
> (.text+0x1cb92): undefined reference to `suspend_device_irqs'
>
> To fix this add some empty inline functions for !GENERIC_HARDIRQS.
>
> Signed-off-by: Heiko Carstens <[email protected]>
> ---
>
> include/linux/interrupt.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff -urpN linux-2.6/include/linux/interrupt.h linux-2.6-patched/include/linux/interrupt.h
> --- linux-2.6/include/linux/interrupt.h 2009-06-11 13:07:46.000000000 +0200
> +++ linux-2.6-patched/include/linux/interrupt.h 2009-06-11 13:08:31.000000000 +0200
> @@ -183,6 +183,7 @@ extern void disable_irq(unsigned int irq
> extern void enable_irq(unsigned int irq);
>
> /* The following three functions are for the core kernel use only. */
> +#ifdef CONFIG_GENERIC_HARDIRQS
> extern void suspend_device_irqs(void);
> extern void resume_device_irqs(void);
> #ifdef CONFIG_PM_SLEEP
> @@ -190,6 +191,11 @@ extern int check_wakeup_irqs(void);
> #else
> static inline int check_wakeup_irqs(void) { return 0; }
> #endif
> +#else
> +static inline void suspend_device_irqs(void) { };
> +static inline void resume_device_irqs(void) { };
> +static inline int check_wakeup_irqs(void) { return 0; }
> +#endif
>
> #if defined(CONFIG_SMP) && defined(CONFIG_GENERIC_HARDIRQS)

Added, thanks a lot for the patch!

Best,
Rafael

2009-06-11 21:12:19

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v2] pm: Move nvs routines into a seperate file.

On Thu 2009-06-11 15:32:18, Heiko Carstens wrote:
> On Wed, Jun 10, 2009 at 01:09:19AM +0200, Rafael J. Wysocki wrote:
> > On Tuesday 09 June 2009, Cornelia Huck wrote:
> > > The *_nvs_* routines in swsusp.c make use of the io*map()
> > > functions, which are only provided for HAS_IOMEM, thus
> > > breaking compilation if HAS_IOMEM is not set. Fix this
> > > by moving the *_nvs_* routines into hibernation_nvs.c, which
> > > is only compiled if HAS_IOMEM is set.
> > >
> > > Signed-off-by: Cornelia Huck <[email protected]>
> >
> > Thanks, I added the GPLv2 line to the header comment and changed the name
> > of the file to hibernate_nvs.c (to match the other changes in the works).
> >
> > I'll carry out some compilation testing on it and put it into the tree shortly.
>
> Rafael, could you add the patch below as well?
> Or should that go in via git390?
>
> Subject: [PATCH] PM: add empty suspend/resume device irq functions
>
> From: Heiko Carstens <[email protected]>
>
> git commit 0a0c5168 "PM: Introduce functions for suspending and resuming
> device interrupts" introduced some helper functions. However these
> functions are only available for architectures which support
> GENERIC_HARDIRQS.
>
> Other architectures will see this build error:
>
> drivers/built-in.o: In function `sysdev_suspend':
> (.text+0x15138): undefined reference to `check_wakeup_irqs'
> drivers/built-in.o: In function `device_power_up':
> (.text+0x1cb66): undefined reference to `resume_device_irqs'
> drivers/built-in.o: In function `device_power_down':
> (.text+0x1cb92): undefined reference to `suspend_device_irqs'
>
> To fix this add some empty inline functions for !GENERIC_HARDIRQS.

I don't think that's right fix. If architecture does not use
GENERIC_HARDIRQS, it may want to implement *_device_irqs()
itself. Before your patch, it could, after your patch, it can not.

Better put those empty functions in arch/s390/include?
Pavel

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

2009-06-11 21:46:03

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2] pm: Move nvs routines into a seperate file.

On Thursday 11 June 2009, Pavel Machek wrote:
> On Thu 2009-06-11 15:32:18, Heiko Carstens wrote:
> > On Wed, Jun 10, 2009 at 01:09:19AM +0200, Rafael J. Wysocki wrote:
> > > On Tuesday 09 June 2009, Cornelia Huck wrote:
> > > > The *_nvs_* routines in swsusp.c make use of the io*map()
> > > > functions, which are only provided for HAS_IOMEM, thus
> > > > breaking compilation if HAS_IOMEM is not set. Fix this
> > > > by moving the *_nvs_* routines into hibernation_nvs.c, which
> > > > is only compiled if HAS_IOMEM is set.
> > > >
> > > > Signed-off-by: Cornelia Huck <[email protected]>
> > >
> > > Thanks, I added the GPLv2 line to the header comment and changed the name
> > > of the file to hibernate_nvs.c (to match the other changes in the works).
> > >
> > > I'll carry out some compilation testing on it and put it into the tree shortly.
> >
> > Rafael, could you add the patch below as well?
> > Or should that go in via git390?
> >
> > Subject: [PATCH] PM: add empty suspend/resume device irq functions
> >
> > From: Heiko Carstens <[email protected]>
> >
> > git commit 0a0c5168 "PM: Introduce functions for suspending and resuming
> > device interrupts" introduced some helper functions. However these
> > functions are only available for architectures which support
> > GENERIC_HARDIRQS.
> >
> > Other architectures will see this build error:
> >
> > drivers/built-in.o: In function `sysdev_suspend':
> > (.text+0x15138): undefined reference to `check_wakeup_irqs'
> > drivers/built-in.o: In function `device_power_up':
> > (.text+0x1cb66): undefined reference to `resume_device_irqs'
> > drivers/built-in.o: In function `device_power_down':
> > (.text+0x1cb92): undefined reference to `suspend_device_irqs'
> >
> > To fix this add some empty inline functions for !GENERIC_HARDIRQS.
>
> I don't think that's right fix. If architecture does not use
> GENERIC_HARDIRQS, it may want to implement *_device_irqs()
> itself. Before your patch, it could, after your patch, it can not.
>
> Better put those empty functions in arch/s390/include?

If any of the affected architectures wants to implement *_device_irqs()
itself, it can do the appropriate change in future. For now, let's not break
compilation on them, shall we?

Thanks,
Rafael

2009-06-11 22:05:41

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v2] pm: Move nvs routines into a seperate file.

On Thu 2009-06-11 23:46:27, Rafael J. Wysocki wrote:
> On Thursday 11 June 2009, Pavel Machek wrote:
> > On Thu 2009-06-11 15:32:18, Heiko Carstens wrote:
> > > On Wed, Jun 10, 2009 at 01:09:19AM +0200, Rafael J. Wysocki wrote:
> > > > On Tuesday 09 June 2009, Cornelia Huck wrote:
> > > > > The *_nvs_* routines in swsusp.c make use of the io*map()
> > > > > functions, which are only provided for HAS_IOMEM, thus
> > > > > breaking compilation if HAS_IOMEM is not set. Fix this
> > > > > by moving the *_nvs_* routines into hibernation_nvs.c, which
> > > > > is only compiled if HAS_IOMEM is set.
> > > > >
> > > > > Signed-off-by: Cornelia Huck <[email protected]>
> > > >
> > > > Thanks, I added the GPLv2 line to the header comment and changed the name
> > > > of the file to hibernate_nvs.c (to match the other changes in the works).
> > > >
> > > > I'll carry out some compilation testing on it and put it into the tree shortly.
> > >
> > > Rafael, could you add the patch below as well?
> > > Or should that go in via git390?
> > >
> > > Subject: [PATCH] PM: add empty suspend/resume device irq functions
> > >
> > > From: Heiko Carstens <[email protected]>
> > >
> > > git commit 0a0c5168 "PM: Introduce functions for suspending and resuming
> > > device interrupts" introduced some helper functions. However these
> > > functions are only available for architectures which support
> > > GENERIC_HARDIRQS.
> > >
> > > Other architectures will see this build error:
> > >
> > > drivers/built-in.o: In function `sysdev_suspend':
> > > (.text+0x15138): undefined reference to `check_wakeup_irqs'
> > > drivers/built-in.o: In function `device_power_up':
> > > (.text+0x1cb66): undefined reference to `resume_device_irqs'
> > > drivers/built-in.o: In function `device_power_down':
> > > (.text+0x1cb92): undefined reference to `suspend_device_irqs'
> > >
> > > To fix this add some empty inline functions for !GENERIC_HARDIRQS.
> >
> > I don't think that's right fix. If architecture does not use
> > GENERIC_HARDIRQS, it may want to implement *_device_irqs()
> > itself. Before your patch, it could, after your patch, it can not.
> >
> > Better put those empty functions in arch/s390/include?
>
> If any of the affected architectures wants to implement *_device_irqs()
> itself, it can do the appropriate change in future. For now, let's not break
> compilation on them, shall we?

Well, if one of those architectures will want to implement
*_device_irqs(), it will have to either modify s390, and all other
!GENERIC_HARDIRQS architectures. Not exactly easy task; better do it
right from beggining?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2009-06-11 22:29:35

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2] pm: Move nvs routines into a seperate file.

On Friday 12 June 2009, Pavel Machek wrote:
> On Thu 2009-06-11 23:46:27, Rafael J. Wysocki wrote:
> > On Thursday 11 June 2009, Pavel Machek wrote:
> > > On Thu 2009-06-11 15:32:18, Heiko Carstens wrote:
> > > > On Wed, Jun 10, 2009 at 01:09:19AM +0200, Rafael J. Wysocki wrote:
> > > > > On Tuesday 09 June 2009, Cornelia Huck wrote:
> > > > > > The *_nvs_* routines in swsusp.c make use of the io*map()
> > > > > > functions, which are only provided for HAS_IOMEM, thus
> > > > > > breaking compilation if HAS_IOMEM is not set. Fix this
> > > > > > by moving the *_nvs_* routines into hibernation_nvs.c, which
> > > > > > is only compiled if HAS_IOMEM is set.
> > > > > >
> > > > > > Signed-off-by: Cornelia Huck <[email protected]>
> > > > >
> > > > > Thanks, I added the GPLv2 line to the header comment and changed the name
> > > > > of the file to hibernate_nvs.c (to match the other changes in the works).
> > > > >
> > > > > I'll carry out some compilation testing on it and put it into the tree shortly.
> > > >
> > > > Rafael, could you add the patch below as well?
> > > > Or should that go in via git390?
> > > >
> > > > Subject: [PATCH] PM: add empty suspend/resume device irq functions
> > > >
> > > > From: Heiko Carstens <[email protected]>
> > > >
> > > > git commit 0a0c5168 "PM: Introduce functions for suspending and resuming
> > > > device interrupts" introduced some helper functions. However these
> > > > functions are only available for architectures which support
> > > > GENERIC_HARDIRQS.
> > > >
> > > > Other architectures will see this build error:
> > > >
> > > > drivers/built-in.o: In function `sysdev_suspend':
> > > > (.text+0x15138): undefined reference to `check_wakeup_irqs'
> > > > drivers/built-in.o: In function `device_power_up':
> > > > (.text+0x1cb66): undefined reference to `resume_device_irqs'
> > > > drivers/built-in.o: In function `device_power_down':
> > > > (.text+0x1cb92): undefined reference to `suspend_device_irqs'
> > > >
> > > > To fix this add some empty inline functions for !GENERIC_HARDIRQS.
> > >
> > > I don't think that's right fix. If architecture does not use
> > > GENERIC_HARDIRQS, it may want to implement *_device_irqs()
> > > itself. Before your patch, it could, after your patch, it can not.
> > >
> > > Better put those empty functions in arch/s390/include?
> >
> > If any of the affected architectures wants to implement *_device_irqs()
> > itself, it can do the appropriate change in future. For now, let's not break
> > compilation on them, shall we?
>
> Well, if one of those architectures will want to implement
> *_device_irqs(), it will have to either modify s390, and all other
> !GENERIC_HARDIRQS architectures.

Why will it? I think it will be sufficient to modify the header changed by
this patch and the architecture in question.

Best,
Rafael

2009-06-11 23:22:39

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v2] pm: Move nvs routines into a seperate file.

> > > > > Other architectures will see this build error:
> > > > >
> > > > > drivers/built-in.o: In function `sysdev_suspend':
> > > > > (.text+0x15138): undefined reference to `check_wakeup_irqs'
> > > > > drivers/built-in.o: In function `device_power_up':
> > > > > (.text+0x1cb66): undefined reference to `resume_device_irqs'
> > > > > drivers/built-in.o: In function `device_power_down':
> > > > > (.text+0x1cb92): undefined reference to `suspend_device_irqs'
> > > > >
> > > > > To fix this add some empty inline functions for !GENERIC_HARDIRQS.
> > > >
> > > > I don't think that's right fix. If architecture does not use
> > > > GENERIC_HARDIRQS, it may want to implement *_device_irqs()
> > > > itself. Before your patch, it could, after your patch, it can not.
> > > >
> > > > Better put those empty functions in arch/s390/include?
> > >
> > > If any of the affected architectures wants to implement *_device_irqs()
> > > itself, it can do the appropriate change in future. For now, let's not break
> > > compilation on them, shall we?
> >
> > Well, if one of those architectures will want to implement
> > *_device_irqs(), it will have to either modify s390, and all other
> > !GENERIC_HARDIRQS architectures.
>
> Why will it? I think it will be sufficient to modify the header changed by
> this patch and the architecture in question.

Hmm, how? Putting #ifndef MY_ARCH into generic header? Inventing
CONFIG_NON_GENERIC_HARDIRQS_BUT_I_NEED_DEVICE_IRQS?
Pavel

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

2009-06-11 23:28:22

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v2] pm: Move nvs routines into a seperate file.

> > > > > > To fix this add some empty inline functions for !GENERIC_HARDIRQS.
> > > > >
> > > > > I don't think that's right fix. If architecture does not use
> > > > > GENERIC_HARDIRQS, it may want to implement *_device_irqs()
> > > > > itself. Before your patch, it could, after your patch, it can not.
> > > > >
> > > > > Better put those empty functions in arch/s390/include?
> > > >
> > > > If any of the affected architectures wants to implement *_device_irqs()
> > > > itself, it can do the appropriate change in future. For now, let's not break
> > > > compilation on them, shall we?
> > >
> > > Well, if one of those architectures will want to implement
> > > *_device_irqs(), it will have to either modify s390, and all other
> > > !GENERIC_HARDIRQS architectures.
> >
> > Why will it? I think it will be sufficient to modify the header changed by
> > this patch and the architecture in question.
>
> Hmm, how? Putting #ifndef MY_ARCH into generic header? Inventing
> CONFIG_NON_GENERIC_HARDIRQS_BUT_I_NEED_DEVICE_IRQS?

Maybe playing with attribute((weak)) is the cleanest solution?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html