Re: [PATCH 01/11] PM: Add wake lock 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>
> ---
>  include/linux/wakelock.h |   91 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 91 insertions(+), 0 deletions(-)
>  create mode 100755 include/linux/wakelock.h
> 
> diff --git a/include/linux/wakelock.h b/include/linux/wakelock.h
> new file mode 100755
> index 0000000..a096d24
> --- /dev/null
> +++ b/include/linux/wakelock.h
> @@ -0,0 +1,91 @@
> +/* include/linux/wakelock.h
> + *
> + * Copyright (C) 2007-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.
> + *
> + */
> +
> +#ifndef _LINUX_WAKELOCK_H
> +#define _LINUX_WAKELOCK_H
> +
> +#include <linux/list.h>
> +#include <linux/ktime.h>
> +
> +/* A wake_lock prevents the system from entering suspend or other low power
> + * states when active. If the type is set to WAKE_LOCK_SUSPEND, the wake_lock
> + * prevents a full system suspend. If the type is WAKE_LOCK_IDLE, low power

Does this include hibernation? If so, you might like to say 'sleep'
instead of suspend.

> + * states that cause large interrupt latencies or that disable a set of
> + * interrupts will not entered from idle until the wake_locks are released.

This makes me wonder if 'wake_locks' is an appropriate name - you're
locking against sleeping rather than waking.

> + */
> +
> +enum {
> +	WAKE_LOCK_SUSPEND, /* Prevent suspend */
> +	WAKE_LOCK_IDLE,    /* Prevent low power idle */
> +	WAKE_LOCK_TYPE_COUNT
> +};
> +
> +struct wake_lock {
> +#ifdef CONFIG_HAS_WAKELOCK
> +	struct list_head    link;
> +	int                 flags;
> +	const char         *name;
> +	unsigned long       expires;
> +#ifdef CONFIG_WAKELOCK_STAT
> +	struct {
> +		int             count;
> +		int             expire_count;
> +		int             wakeup_count;
> +		ktime_t         total_time;
> +		ktime_t         prevent_suspend_time;
> +		ktime_t         max_time;
> +		ktime_t         last_time;
> +	} stat;
> +#endif
> +#endif
> +};

If CONFIG_HAS_WAKELOCK and CONFIG_WAKELOCK_STAT are both off, you've got
an empty struct wake_lock definition. It wouldn't have any users, would
it? (And therefore doesn't need to be defined at all).

> +
> +#ifdef CONFIG_HAS_WAKELOCK
> +
> +void wake_lock_init(struct wake_lock *lock, int type, const char *name);
> +void wake_lock_destroy(struct wake_lock *lock);
> +void wake_lock(struct wake_lock *lock);
> +void wake_lock_timeout(struct wake_lock *lock, long timeout);
> +void wake_unlock(struct wake_lock *lock);
> +
> +/* wake_lock_active returns a non-zero value if the wake_lock is currently
> + * locked. If the wake_lock has a timeout, it does not check the timeout
> + * but if the timeout had aready been checked it will return 0.

s/aready/already/

What does it mean for the timeout to already have been checked? Is that
the same as the timeout having already expired?

> + */
> +int wake_lock_active(struct wake_lock *lock);
> +
> +/* has_wake_lock returns 0 if no wake locks of the specified type are active,
> + * and non-zero if one or more wake locks are held. Specifically it returns
> + * -1 if one or more wake locks with no timeout are active or the
> + * number of jiffies until all active wake locks time out.
> + */
> +long has_wake_lock(int type);
> +
> +#else
> +
> +static inline void wake_lock_init(struct wake_lock *lock, int type,
> +					const char *name) {}
> +static inline void wake_lock_destroy(struct wake_lock *lock) {}
> +static inline void wake_lock(struct wake_lock *lock) {}
> +static inline void wake_lock_timeout(struct wake_lock *lock, long timeout) {}
> +static inline void wake_unlock(struct wake_lock *lock) {}
> +
> +static inline int wake_lock_active(struct wake_lock *lock) { return 0; }
> +static inline long has_wake_lock(int type) { return 0; }
> +
> +#endif
> +
> +#endif
> +

Regards,

Nigel

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