On Tue, 2010-06-01 at 23:31 +0200, Rafael J. Wysocki wrote:
> On Tuesday 01 June 2010, Maxim Levitsky wrote:
> > Saving platform non-volatile state may be required for suspend to RAM as
> > well as hibernation. Move it to more generic code.
> >
> > Signed-off-by: Matthew Garrett <mjg@xxxxxxxxxx>
> > Signed-off-by: Matthew Garrett <maximlevitsky@xxxxxxxxx>
>
> You made a mistake here.
>
> Also, why are you resending the Matthews patches? I think Len has seen them
> already.
Yea, a copypaste.
(I was told that if one submits modified patch, it adds his
Signed-off-by.)
I rebased these on top of
ACPI / EC / PM: Fix race between EC transactions and system suspend'
To be honest, I just want to get some feedback on this.
This was major issue that kept me from using otherwise prefect suspend
to ram.
Thus I am thinking that maybe ready to apply patch will have more
chances to be reviewed....
(Athough I guess I need to wait until the oil spill in gulf of Mexico is
plugged, because it seems to continue to feed the never ending
discussion/flamewar on android suspend issues)....
Best regards,
Maxim Levitsky
>
> Rafael
>
>
> > ---
> > arch/x86/kernel/e820.c | 2 +-
> > drivers/acpi/sleep.c | 12 ++--
> > include/linux/suspend.h | 26 ++++----
> > kernel/power/Kconfig | 9 ++-
> > kernel/power/Makefile | 2 +-
> > kernel/power/hibernate_nvs.c | 136 ------------------------------------------
> > kernel/power/nvs.c | 136 ++++++++++++++++++++++++++++++++++++++++++
> > kernel/power/suspend.c | 6 ++
> > 8 files changed, 168 insertions(+), 161 deletions(-)
> > delete mode 100644 kernel/power/hibernate_nvs.c
> > create mode 100644 kernel/power/nvs.c
> >
> > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> > index 7bca3c6..0d6fc71 100644
> > --- a/arch/x86/kernel/e820.c
> > +++ b/arch/x86/kernel/e820.c
> > @@ -729,7 +729,7 @@ static int __init e820_mark_nvs_memory(void)
> > struct e820entry *ei = &e820.map[i];
> >
> > if (ei->type == E820_NVS)
> > - hibernate_nvs_register(ei->addr, ei->size);
> > + suspend_nvs_register(ei->addr, ei->size);
> > }
> >
> > return 0;
> > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> > index 3fb4bde..bdb306f 100644
> > --- a/drivers/acpi/sleep.c
> > +++ b/drivers/acpi/sleep.c
> > @@ -404,7 +404,7 @@ static int acpi_hibernation_begin(void)
> > {
> > int error;
> >
> > - error = s4_no_nvs ? 0 : hibernate_nvs_alloc();
> > + error = s4_no_nvs ? 0 : suspend_nvs_alloc();
> > if (!error) {
> > acpi_target_sleep_state = ACPI_STATE_S4;
> > acpi_sleep_tts_switch(acpi_target_sleep_state);
> > @@ -418,7 +418,7 @@ static int acpi_hibernation_pre_snapshot(void)
> > int error = acpi_pm_prepare();
> >
> > if (!error)
> > - hibernate_nvs_save();
> > + suspend_nvs_save();
> >
> > return error;
> > }
> > @@ -443,7 +443,7 @@ static int acpi_hibernation_enter(void)
> >
> > static void acpi_hibernation_finish(void)
> > {
> > - hibernate_nvs_free();
> > + suspend_nvs_free();
> > acpi_ec_unblock_transactions();
> > acpi_pm_finish();
> > }
> > @@ -464,7 +464,7 @@ static void acpi_hibernation_leave(void)
> > panic("ACPI S4 hardware signature mismatch");
> > }
> > /* Restore the NVS memory area */
> > - hibernate_nvs_restore();
> > + suspend_nvs_restore();
> > /* Allow EC transactions to happen. */
> > acpi_ec_unblock_transactions_early();
> > }
> > @@ -507,7 +507,7 @@ static int acpi_hibernation_begin_old(void)
> >
> > if (!error) {
> > if (!s4_no_nvs)
> > - error = hibernate_nvs_alloc();
> > + error = suspend_nvs_alloc();
> > if (!error)
> > acpi_target_sleep_state = ACPI_STATE_S4;
> > }
> > @@ -517,7 +517,7 @@ static int acpi_hibernation_begin_old(void)
> > static int acpi_hibernation_pre_snapshot_old(void)
> > {
> > acpi_pm_freeze();
> > - hibernate_nvs_save();
> > + suspend_nvs_save();
> > return 0;
> > }
> >
> > diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> > index 5e781d8..bc7d6bb 100644
> > --- a/include/linux/suspend.h
> > +++ b/include/linux/suspend.h
> > @@ -256,22 +256,22 @@ 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)
> > +#ifdef CONFIG_SUSPEND_NVS
> > +extern int suspend_nvs_register(unsigned long start, unsigned long size);
> > +extern int suspend_nvs_alloc(void);
> > +extern void suspend_nvs_free(void);
> > +extern void suspend_nvs_save(void);
> > +extern void suspend_nvs_restore(void);
> > +#else /* CONFIG_SUSPEND_NVS */
> > +static inline int suspend_nvs_register(unsigned long a, unsigned long b)
> > {
> > return 0;
> > }
> > -static inline int hibernate_nvs_alloc(void) { return 0; }
> > -static inline void hibernate_nvs_free(void) {}
> > -static inline void hibernate_nvs_save(void) {}
> > -static inline void hibernate_nvs_restore(void) {}
> > -#endif /* CONFIG_HIBERNATION_NVS */
> > +static inline int suspend_nvs_alloc(void) { return 0; }
> > +static inline void suspend_nvs_free(void) {}
> > +static inline void suspend_nvs_save(void) {}
> > +static inline void suspend_nvs_restore(void) {}
> > +#endif /* CONFIG_SUSPEND_NVS */
> >
> > #ifdef CONFIG_PM_SLEEP
> > void save_processor_state(void);
> > diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> > index 5c36ea9..ca6066a 100644
> > --- a/kernel/power/Kconfig
> > +++ b/kernel/power/Kconfig
> > @@ -99,9 +99,13 @@ config PM_SLEEP_ADVANCED_DEBUG
> > depends on PM_ADVANCED_DEBUG
> > default n
> >
> > +config SUSPEND_NVS
> > + bool
> > +
> > config SUSPEND
> > bool "Suspend to RAM and standby"
> > depends on PM && ARCH_SUSPEND_POSSIBLE
> > + select SUSPEND_NVS if HAS_IOMEM
> > default y
> > ---help---
> > Allow the system to enter sleep states in which main memory is
> > @@ -130,13 +134,10 @@ 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
> > + select SUSPEND_NVS if HAS_IOMEM
> > ---help---
> > Enable the suspend to disk (STD) functionality, which is usually
> > called "hibernation" in user interfaces. STD checkpoints the
> > diff --git a/kernel/power/Makefile b/kernel/power/Makefile
> > index 524e058..f9063c6 100644
> > --- a/kernel/power/Makefile
> > +++ b/kernel/power/Makefile
> > @@ -10,6 +10,6 @@ obj-$(CONFIG_SUSPEND) += suspend.o
> > obj-$(CONFIG_PM_TEST_SUSPEND) += suspend_test.o
> > obj-$(CONFIG_HIBERNATION) += hibernate.o snapshot.o swap.o user.o \
> > block_io.o
> > -obj-$(CONFIG_HIBERNATION_NVS) += hibernate_nvs.o
> > +obj-$(CONFIG_SUSPEND_NVS) += nvs.o
> >
> > obj-$(CONFIG_MAGIC_SYSRQ) += poweroff.o
> > diff --git a/kernel/power/hibernate_nvs.c b/kernel/power/hibernate_nvs.c
> > deleted file mode 100644
> > index fdcad9e..0000000
> > --- a/kernel/power/hibernate_nvs.c
> > +++ /dev/null
> > @@ -1,136 +0,0 @@
> > -/*
> > - * linux/kernel/power/hibernate_nvs.c - Routines for handling NVS memory
> > - *
> > - * Copyright (C) 2008,2009 Rafael J. Wysocki <rjw@xxxxxxx>, Novell Inc.
> > - *
> > - * This file is released under the GPLv2.
> > - */
> > -
> > -#include <linux/io.h>
> > -#include <linux/kernel.h>
> > -#include <linux/list.h>
> > -#include <linux/mm.h>
> > -#include <linux/slab.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);
> > -}
> > diff --git a/kernel/power/nvs.c b/kernel/power/nvs.c
> > new file mode 100644
> > index 0000000..30c662f
> > --- /dev/null
> > +++ b/kernel/power/nvs.c
> > @@ -0,0 +1,136 @@
> > +/*
> > + * linux/kernel/power/nvs.c - Routines for handling NVS memory
> > + *
> > + * Copyright (C) 2008,2009 Rafael J. Wysocki <rjw@xxxxxxx>, Novell Inc.
> > + *
> > + * This file is released under the GPLv2.
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/list.h>
> > +#include <linux/mm.h>
> > +#include <linux/slab.h>
> > +#include <linux/suspend.h>
> > +
> > +/*
> > + * Platforms, like ACPI, may want us to save some memory used by them during
> > + * suspend 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);
> > +
> > +/**
> > + * suspend_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 suspend_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;
> > +}
> > +
> > +/**
> > + * suspend_nvs_free - free data pages allocated for saving NVS regions
> > + */
> > +void suspend_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;
> > + }
> > + }
> > +}
> > +
> > +/**
> > + * suspend_nvs_alloc - allocate memory necessary for saving NVS regions
> > + */
> > +int suspend_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) {
> > + suspend_nvs_free();
> > + return -ENOMEM;
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > +/**
> > + * suspend_nvs_save - save NVS memory regions
> > + */
> > +void suspend_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);
> > + }
> > +}
> > +
> > +/**
> > + * suspend_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 suspend_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);
> > +}
> > diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> > index 56e7dbb..f37cb7d 100644
> > --- a/kernel/power/suspend.c
> > +++ b/kernel/power/suspend.c
> > @@ -16,6 +16,12 @@
> > #include <linux/cpu.h>
> > #include <linux/syscalls.h>
> > #include <linux/gfp.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/list.h>
> > +#include <linux/mm.h>
> > +#include <linux/slab.h>
> > +#include <linux/suspend.h>
> >
> > #include "power.h"
> >
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
