Re: [PATCH 02/11] PM: Add early suspend api.

Nigel Cunningham
2009-01-14T09:17:58Z

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/earlysuspend.h | 56 ++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 56 insertions(+), 0 deletions(-)
> create mode 100755 include/linux/earlysuspend.h
>
> diff --git a/include/linux/earlysuspend.h b/include/linux/earlysuspend.h
> new file mode 100755
> index 0000000..8343b81
> --- /dev/null
> +++ b/include/linux/earlysuspend.h
> @@ -0,0 +1,56 @@
> +/* include/linux/earlysuspend.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_EARLYSUSPEND_H
> +#define _LINUX_EARLYSUSPEND_H
> +
> +#ifdef CONFIG_HAS_EARLYSUSPEND
> +#include <linux/list.h>
> +#endif

Just #include anyway - it doesn't matter if it doesn't get used in some
circumstances.

> +
> +/* The early_suspend structure defines suspend and resume hooks to be called
> + * when the user visible sleep state of the system changes, and a level to
> + * control the order. They can be used to turn off the screen and input
> + * devices that are not used for wakeup.
> + * Suspend handlers are called in low to high level order, resume handlers are
> + * called in the opposite order. If, when calling register_early_suspend,
> + * the suspend handlers have already been called without a matching call to the
> + * resume handlers, the suspend handler will be called directly from
> + * register_early_suspend. This direct call can violate the normal level order.
> + */

Registering can happen while we're suspending and resuming? That
potential variation in ordering sounds dangerous as far as opportunities
for race conditions goes.

> +enum {
> + EARLY_SUSPEND_LEVEL_BLANK_SCREEN = 50,
> + EARLY_SUSPEND_LEVEL_STOP_DRAWING = 100,
> + EARLY_SUSPEND_LEVEL_DISABLE_FB = 150,
> +};
> +struct early_suspend {
> +#ifdef CONFIG_HAS_EARLYSUSPEND
> + struct list_head link;
> + int level;
> + void (*suspend)(struct early_suspend *h);
> + void (*resume)(struct early_suspend *h);

Could these functions ever potentially fail in way that you'd want
callers to know? If so, you might want a non void return value.

> +#endif
> +};
> +
> +#ifdef CONFIG_HAS_EARLYSUSPEND
> +void register_early_suspend(struct early_suspend *handler);
> +void unregister_early_suspend(struct early_suspend *handler);
> +#else
> +#define register_early_suspend(handler) do { } while (0)
> +#define unregister_early_suspend(handler) do { } while (0)
> +#endif
> +
> +#endif
> +

Regards,

Nigel

_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org