Re: [PATCH 07/13] PM: wakelock: Add /sys/power/request_state

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Thursday 05 February 2009, Arve Hjønnevåg wrote:
> This is a non-blocking interface that specifies which suspend state to
> enter when no wakelocks are locked. A special state, "on", stops the
> process by locking the "main" wakelock.
> 
> If early-suspend support is enabled, early suspend handlers will be called
> when a state other than "on" is written. When writing "on", the corresonding
> resume handlers are called.
> 
> Signed-off-by: Arve Hjønnevåg <arve@android.com>
> ---
>  kernel/power/main.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 74 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index e2aae3e..676a8b0 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -22,6 +22,7 @@
>  #include <linux/freezer.h>
>  #include <linux/vmstat.h>
>  #include <linux/syscalls.h>
> +#include <linux/wakelock.h>
>  
>  #include "power.h"
>  
> @@ -388,6 +389,9 @@ static void suspend_finish(void)
>  
>  
>  static const char * const pm_states[PM_SUSPEND_MAX] = {
> +#ifdef CONFIG_WAKELOCK
> +	[PM_SUSPEND_ON]		= "on",
> +#endif
>  	[PM_SUSPEND_STANDBY]	= "standby",
>  	[PM_SUSPEND_MEM]	= "mem",
>  };
> @@ -545,6 +549,73 @@ static ssize_t state_store(struct kobject *kobj, struct kobj_attribute *attr,
>  power_attr(state);
>  #endif
>  
> +/**
> + *	request_state - control system power state.
> + *
> + *	This is similar to state, but it does not block until the system
> + *	resumes, and it will try to re-enter the state until another state is
> + *	requsted. Wakelocks are respected and the requested state will only
> + *	be entered when no wakelocks are held. Write "on" to cancel.
> + *
> + *	If CONFIG_EARLYSUSPEND is set, early_suspend hooks are called when
> + *	the requested state changes to or from "on"
> + */
> +
> +#ifdef CONFIG_WAKELOCK
> +static ssize_t request_state_show(struct kobject *kobj, struct kobj_attribute *attr,
> +			  char *buf)
> +{
> +	char *s = buf;
> +	int i;
> +
> +	for (i = 0; i < PM_SUSPEND_MAX; i++) {
> +		if (pm_states[i] && valid_state(i))
> +			s += sprintf(s,"%s ", pm_states[i]);
> +	}
> +	if (s != buf)
> +		/* convert the last space to a newline */
> +		*(s-1) = '\n';
> +	return (s - buf);
> +}

Isn't the above duplicated code?

> +
> +static ssize_t request_state_store(struct kobject *kobj, struct kobj_attribute *attr,
> +			   const char *buf, size_t n)
> +{
> +	suspend_state_t state = PM_SUSPEND_ON;
> +	const char * const *s;
> +	char *p;
> +	int len;
> +	int error = -EINVAL;
> +
> +	p = memchr(buf, '\n', n);
> +	len = p ? p - buf : n;
> +
> +	for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) {
> +		if (*s && len == strlen(*s) && !strncmp(buf, *s, len))
> +			break;
> +	}
> +	if (state < PM_SUSPEND_MAX && *s)
> +		if (state == PM_SUSPEND_ON || valid_state(state)) {
> +			error = 0;
> +#ifdef CONFIG_EARLYSUSPEND
> +			request_suspend_state(state);
> +#else

Remove this #ifdef please.

> +			if (!mutex_trylock(&pm_mutex))
> +				return -EBUSY;
> +			requested_suspend_state = state;
> +			if (state == PM_SUSPEND_ON)
> +				wake_lock(&main_wake_lock);
> +			else
> +				wake_unlock(&main_wake_lock);

What's the main_wake_lock and what exactly is its role?

> +			mutex_unlock(&pm_mutex);
> +#endif
> +		}
> +	return error ? error : n;
> +}
> +
> +power_attr(request_state);
> +#endif
> +
>  #ifdef CONFIG_PM_TRACE
>  int pm_trace_enabled;
>  
> @@ -574,6 +645,9 @@ static struct attribute * g[] = {
>  #ifndef CONFIG_DISABLE_SYS_POWER_STATE
>  	&state_attr.attr,
>  #endif
> +#ifdef CONFIG_WAKELOCK
> +	&request_state_attr.attr,
> +#endif
>  #ifdef CONFIG_PM_TRACE
>  	&pm_trace_attr.attr,
>  #endif

I don't think all of this is really necessary.  IMO we should create bus
type-specific interfaces for putting devices into low power states at run time,
that will work along with the existing suspend-resume infrastructure,
instead of this.

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