Re: [PATCH 04/11] PM: Implement early suspend api

[Nigel Cunningham <ncunningham@crca.org.au>]

Hi.

On Tue, 2009-01-13 at 17:27 -0800, Arve Hjønnevåg wrote:
> Signed-off-by: Arve Hjønnevåg <arve@android.com>
> ---
>  kernel/power/Kconfig        |   12 +++
>  kernel/power/Makefile       |    1 +
>  kernel/power/earlysuspend.c |  178 +++++++++++++++++++++++++++++++++++++++++++
>  kernel/power/power.h        |    6 ++
>  4 files changed, 197 insertions(+), 0 deletions(-)
>  create mode 100644 kernel/power/earlysuspend.c
> 
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 6e3da6e..50690d8 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -119,6 +119,9 @@ config SUSPEND_FREEZER
>  config HAS_WAKELOCK
>  	bool
>  
> +config HAS_EARLYSUSPEND
> +	bool
> +
>  config WAKELOCK
>  	bool "Wake lock"
>  	depends on PM && RTC_CLASS
> @@ -135,6 +138,15 @@ config WAKELOCK_STAT
>  	---help---
>  	  Report wake lock stats in /proc/wakelocks
>  
> +config EARLYSUSPEND
> +	bool "Early suspend"
> +	depends on WAKELOCK
> +	default y
> +	select HAS_EARLYSUSPEND
> +	---help---
> +	  Call early suspend handlers when the user requested sleep state
> +	  changes.
> +
>  config HIBERNATION
>  	bool "Hibernation (aka 'suspend to disk')"
>  	depends on PM && SWAP && ARCH_HIBERNATION_POSSIBLE
> diff --git a/kernel/power/Makefile b/kernel/power/Makefile
> index 401ddfe..f0f7b15 100644
> --- a/kernel/power/Makefile
> +++ b/kernel/power/Makefile
> @@ -6,6 +6,7 @@ endif
>  obj-y				:= main.o
>  obj-$(CONFIG_PM_SLEEP)		+= process.o console.o
>  obj-$(CONFIG_WAKELOCK)		+= wakelock.o
> +obj-$(CONFIG_EARLYSUSPEND)	+= earlysuspend.o
>  obj-$(CONFIG_HIBERNATION)	+= swsusp.o disk.o snapshot.o swap.o user.o
>  
>  obj-$(CONFIG_MAGIC_SYSRQ)	+= poweroff.o
> diff --git a/kernel/power/earlysuspend.c b/kernel/power/earlysuspend.c
> new file mode 100644
> index 0000000..84bed51
> --- /dev/null
> +++ b/kernel/power/earlysuspend.c
> @@ -0,0 +1,178 @@
> +/* kernel/power/earlysuspend.c
> + *
> + * Copyright (C) 2005-2008 Google, Inc.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/earlysuspend.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/rtc.h>
> +#include <linux/syscalls.h> /* sys_sync */
> +#include <linux/wakelock.h>
> +#include <linux/workqueue.h>
> +
> +#include "power.h"
> +
> +enum {
> +	DEBUG_USER_STATE = 1U << 0,
> +	DEBUG_SUSPEND = 1U << 2,
> +};

Is there a reason DEBUG_SUSPEND isn't 1U << 1? If so, it might be good
to document that here.

> +static int debug_mask = DEBUG_USER_STATE;
> +module_param_named(debug_mask, debug_mask, int, S_IRUGO | S_IWUSR | S_IWGRP);
> +
> +static DEFINE_MUTEX(early_suspend_lock);
> +static LIST_HEAD(early_suspend_handlers);
> +static void early_suspend(struct work_struct *work);
> +static void late_resume(struct work_struct *work);
> +static DECLARE_WORK(early_suspend_work, early_suspend);
> +static DECLARE_WORK(late_resume_work, late_resume);
> +static DEFINE_SPINLOCK(state_lock);
> +enum {
> +	SUSPEND_REQUESTED = 0x1,
> +	SUSPENDED = 0x2,
> +	SUSPEND_REQUESTED_AND_SUSPENDED = SUSPEND_REQUESTED | SUSPENDED,
> +};
> +static int state;
> +
> +void register_early_suspend(struct early_suspend *handler)
> +{
> +	struct list_head *pos;
> +
> +	mutex_lock(&early_suspend_lock);
> +	list_for_each(pos, &early_suspend_handlers) {
> +		struct early_suspend *e;
> +		e = list_entry(pos, struct early_suspend, link);

list_for_each_entry?

> +		if (e->level > handler->level)
> +			break;
> +	}
> +	list_add_tail(&handler->link, pos);
> +	if ((state & SUSPENDED) && handler->suspend)
> +		handler->suspend(handler);
> +	mutex_unlock(&early_suspend_lock);
> +}
> +EXPORT_SYMBOL(register_early_suspend);
> +
> +void unregister_early_suspend(struct early_suspend *handler)
> +{
> +	mutex_lock(&early_suspend_lock);
> +	list_del(&handler->link);
> +	mutex_unlock(&early_suspend_lock);
> +}
> +EXPORT_SYMBOL(unregister_early_suspend);
> +
> +static void early_suspend(struct work_struct *work)
> +{
> +	struct early_suspend *pos;
> +	unsigned long irqflags;
> +	int abort = 0;
> +
> +	mutex_lock(&early_suspend_lock);
> +	spin_lock_irqsave(&state_lock, irqflags);
> +	if (state == SUSPEND_REQUESTED)
> +		state |= SUSPENDED;
> +	else
> +		abort = 1;
> +	spin_unlock_irqrestore(&state_lock, irqflags);
> +
> +	if (abort) {
> +		if (debug_mask & DEBUG_SUSPEND)
> +			pr_info("early_suspend: abort, state %d\n", state);
> +		mutex_unlock(&early_suspend_lock);
> +		goto abort;
> +	}
> +
> +	if (debug_mask & DEBUG_SUSPEND)
> +		pr_info("early_suspend: call handlers\n");
> +	list_for_each_entry(pos, &early_suspend_handlers, link) {
> +		if (pos->suspend != NULL)

Just "if (pos->suspend)" is sufficient.

> +			pos->suspend(pos);
> +	}
> +	mutex_unlock(&early_suspend_lock);
> +
> +	if (debug_mask & DEBUG_SUSPEND)
> +		pr_info("early_suspend: sync\n");
> +
> +	sys_sync();

Why the sync here?

> +abort:
> +	spin_lock_irqsave(&state_lock, irqflags);
> +	if (state == SUSPEND_REQUESTED_AND_SUSPENDED)
> +		wake_unlock(&main_wake_lock);
> +	spin_unlock_irqrestore(&state_lock, irqflags);
> +}
> +
> +static void late_resume(struct work_struct *work)
> +{
> +	struct early_suspend *pos;
> +	unsigned long irqflags;
> +	int abort = 0;
> +
> +	mutex_lock(&early_suspend_lock);
> +	spin_lock_irqsave(&state_lock, irqflags);
> +	if (state == SUSPENDED)
> +		state &= ~SUSPENDED;

Why not just say state = 0?

> +	else
> +		abort = 1;
> +	spin_unlock_irqrestore(&state_lock, irqflags);
> +
> +	if (abort) {
> +		if (debug_mask & DEBUG_SUSPEND)
> +			pr_info("late_resume: abort, state %d\n", state);
> +		goto abort;
> +	}
> +	if (debug_mask & DEBUG_SUSPEND)
> +		pr_info("late_resume: call handlers\n");
> +	list_for_each_entry_reverse(pos, &early_suspend_handlers, link)
> +		if (pos->resume != NULL)

if (pos->resume)

> +			pos->resume(pos);
> +	if (debug_mask & DEBUG_SUSPEND)
> +		pr_info("late_resume: done\n");
> +abort:
> +	mutex_unlock(&early_suspend_lock);
> +}
> +
> +void request_suspend_state(suspend_state_t new_state)
> +{
> +	unsigned long irqflags;
> +	int old_sleep;
> +
> +	spin_lock_irqsave(&state_lock, irqflags);
> +	old_sleep = state & SUSPEND_REQUESTED;
> +	if (debug_mask & DEBUG_USER_STATE) {
> +		struct timespec ts;
> +		struct rtc_time tm;
> +		getnstimeofday(&ts);
> +		rtc_time_to_tm(ts.tv_sec, &tm);
> +		pr_info("request_suspend_state: %s (%d->%d) at %lld "
> +			"(%d-%02d-%02d %02d:%02d:%02d.%09lu UTC)\n",
> +			new_state != PM_SUSPEND_ON ? "sleep" : "wakeup",
> +			requested_suspend_state, new_state,
> +			ktime_to_ns(ktime_get()),
> +			tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
> +			tm.tm_hour, tm.tm_min, tm.tm_sec, ts.tv_nsec);
> +	}
> +	if (!old_sleep && new_state != PM_SUSPEND_ON) {
> +		state |= SUSPEND_REQUESTED;
> +		queue_work(suspend_work_queue, &early_suspend_work);
> +	} else if (old_sleep && new_state == PM_SUSPEND_ON) {
> +		state &= ~SUSPEND_REQUESTED;
> +		wake_lock(&main_wake_lock);
> +		queue_work(suspend_work_queue, &late_resume_work);
> +	}
> +	requested_suspend_state = new_state;
> +	spin_unlock_irqrestore(&state_lock, irqflags);
> +}
> +
> +suspend_state_t get_suspend_state(void)
> +{
> +	return requested_suspend_state;
> +}
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 1527174..7ce9637 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -230,3 +230,9 @@ extern struct workqueue_struct *suspend_work_queue;
>  extern struct wake_lock main_wake_lock;
>  extern suspend_state_t requested_suspend_state;
>  #endif
> +
> +#ifdef CONFIG_EARLYSUSPEND
> +/* kernel/power/earlysuspend.c */
> +void request_suspend_state(suspend_state_t state);
> +suspend_state_t get_suspend_state(void);
> +#endif
> -- 
> 1.6.1

Regards,

Nigel

_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm