[PATCH] power management: change /sys/power/disk display

[PATCH] power management: change /sys/power/disk display

[Johannes Berg]

This patch changes /sys/power/disk to display all valid modes
as well as the currently selected one in a fashion known from
the LED subsystem.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---
This changes userspace API, but it is apparently not used much (we asked
some userspace developers)

 Documentation/power/interface.txt |    8 ++++++--
 kernel/power/disk.c               |   34 +++++++++++++++++++++++++++++++---
 2 files changed, 37 insertions(+), 5 deletions(-)

--- wireless-dev.orig/kernel/power/disk.c	2007-04-11 18:01:08.389481358 +0200
+++ wireless-dev/kernel/power/disk.c	2007-04-11 18:01:10.109481358 +0200
@@ -322,7 +322,34 @@ static const char * const pm_disk_modes[
 
 static ssize_t disk_show(struct subsystem * subsys, char * buf)
 {
-	return sprintf(buf, "%s\n", pm_disk_modes[pm_disk_mode]);
+	int i;
+	char *start = buf;
+
+	for (i = PM_DISK_PLATFORM; i < PM_DISK_MAX; i++) {
+		if (!pm_disk_modes[i])
+			continue;
+		switch (i) {
+		case PM_DISK_SHUTDOWN:
+		case PM_DISK_REBOOT:
+		case PM_DISK_TEST:
+		case PM_DISK_TESTPROC:
+			break;
+		default:
+			if (pm_ops && pm_ops->enter &&
+			    (i == pm_ops->pm_disk_mode))
+				break;
+			/* not a valid mode, continue with loop */
+			continue;
+		}
+		if (i == pm_disk_mode)
+			buf += sprintf(buf, "[%s]", pm_disk_modes[i]);
+		else
+			buf += sprintf(buf, "%s", pm_disk_modes[i]);
+		if (i+1 != PM_DISK_MAX)
+			buf += sprintf(buf, " ");
+	}
+	buf += sprintf(buf, "\n");
+	return buf-start;
 }
 
 
@@ -363,8 +390,9 @@ static ssize_t disk_store(struct subsyst
 		error = -EINVAL;
 	}
 
-	pr_debug("PM: suspend-to-disk mode set to '%s'\n",
-		 pm_disk_modes[mode]);
+	if (!error)
+		pr_debug("PM: suspend-to-disk mode set to '%s'\n",
+			 pm_disk_modes[mode]);
 	mutex_unlock(&pm_mutex);
 	return error ? error : n;
 }
--- wireless-dev.orig/Documentation/power/interface.txt	2007-04-11 18:01:08.389481358 +0200
+++ wireless-dev/Documentation/power/interface.txt	2007-04-11 18:01:10.109481358 +0200
@@ -34,8 +34,12 @@ for 5 seconds, resume devices, unfreeze 
 we are able to look in the log messages and work out, for example, which code
 is being slow and which device drivers are misbehaving.
 
-Reading from this file will display what the mode is currently set
-to. Writing to this file will accept one of
+Reading from this file will display all supported modes and the currently
+selected one in brackets, for example
+
+	[shutdown] reboot test testproc
+
+Writing to this file will accept one of
 
        'platform' (only if the platform supports it)
        'shutdown'

Re: [PATCH] power management: change /sys/power/disk display

[Rafael J. Wysocki]

On Thursday, 12 April 2007 02:37, Johannes Berg wrote:
> This patch changes /sys/power/disk to display all valid modes
> as well as the currently selected one in a fashion known from
> the LED subsystem.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

Looks good to me, but ... (see below)

> ---
> This changes userspace API, but it is apparently not used much (we asked
> some userspace developers)
> 
>  Documentation/power/interface.txt |    8 ++++++--
>  kernel/power/disk.c               |   34 +++++++++++++++++++++++++++++++---
>  2 files changed, 37 insertions(+), 5 deletions(-)
> 
> --- wireless-dev.orig/kernel/power/disk.c	2007-04-11 18:01:08.389481358 +0200
> +++ wireless-dev/kernel/power/disk.c	2007-04-11 18:01:10.109481358 +0200
> @@ -322,7 +322,34 @@ static const char * const pm_disk_modes[
>  
>  static ssize_t disk_show(struct subsystem * subsys, char * buf)
>  {
> -	return sprintf(buf, "%s\n", pm_disk_modes[pm_disk_mode]);
> +	int i;
> +	char *start = buf;
> +
> +	for (i = PM_DISK_PLATFORM; i < PM_DISK_MAX; i++) {
> +		if (!pm_disk_modes[i])
> +			continue;
> +		switch (i) {
> +		case PM_DISK_SHUTDOWN:
> +		case PM_DISK_REBOOT:
> +		case PM_DISK_TEST:
> +		case PM_DISK_TESTPROC:
> +			break;
> +		default:
> +			if (pm_ops && pm_ops->enter &&
> +			    (i == pm_ops->pm_disk_mode))
> +				break;
> +			/* not a valid mode, continue with loop */
> +			continue;
> +		}
> +		if (i == pm_disk_mode)
> +			buf += sprintf(buf, "[%s]", pm_disk_modes[i]);
> +		else
> +			buf += sprintf(buf, "%s", pm_disk_modes[i]);
> +		if (i+1 != PM_DISK_MAX)
> +			buf += sprintf(buf, " ");
> +	}
> +	buf += sprintf(buf, "\n");
> +	return buf-start;
>  }
>  
>  
> @@ -363,8 +390,9 @@ static ssize_t disk_store(struct subsyst
>  		error = -EINVAL;
>  	}
>  
> -	pr_debug("PM: suspend-to-disk mode set to '%s'\n",
> -		 pm_disk_modes[mode]);
> +	if (!error)
> +		pr_debug("PM: suspend-to-disk mode set to '%s'\n",
> +			 pm_disk_modes[mode]);
>  	mutex_unlock(&pm_mutex);
>  	return error ? error : n;
>  }

.. this is an additional bugfix, right?  If so, can you please separate it from
this patch?

> --- wireless-dev.orig/Documentation/power/interface.txt	2007-04-11 18:01:08.389481358 +0200
> +++ wireless-dev/Documentation/power/interface.txt	2007-04-11 18:01:10.109481358 +0200
> @@ -34,8 +34,12 @@ for 5 seconds, resume devices, unfreeze 
>  we are able to look in the log messages and work out, for example, which code
>  is being slow and which device drivers are misbehaving.
>  
> -Reading from this file will display what the mode is currently set
> -to. Writing to this file will accept one of
> +Reading from this file will display all supported modes and the currently
> +selected one in brackets, for example
> +
> +	[shutdown] reboot test testproc
> +
> +Writing to this file will accept one of
>  
>         'platform' (only if the platform supports it)
>         'shutdown'

Greetings,
Rafael

Re: [PATCH] power management: change /sys/power/disk display

[Johannes Berg]

[-- Attachment #1.1: Type: text/plain, Size: 561 bytes --]

On Thu, 2007-04-12 at 12:41 +0200, Rafael J. Wysocki wrote:
> > -	pr_debug("PM: suspend-to-disk mode set to '%s'\n",
> > -		 pm_disk_modes[mode]);
> > +	if (!error)
> > +		pr_debug("PM: suspend-to-disk mode set to '%s'\n",
> > +			 pm_disk_modes[mode]);
> >  	mutex_unlock(&pm_mutex);
> >  	return error ? error : n;
> >  }
> 
> .. this is an additional bugfix, right?  If so, can you please separate it from
> this patch?

Huh, yes, it's separate I guess. This must have slipped in from the
original patch where I had an extra file.

johannes

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH v2] power management: change /sys/power/disk display

[Johannes Berg]

This patch changes /sys/power/disk to display all valid modes
as well as the currently selected one in a fashion known from
the LED subsystem.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---
 Documentation/power/interface.txt |    8 ++++++--
 kernel/power/disk.c               |   29 ++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 3 deletions(-)

--- wireless-dev.orig/kernel/power/disk.c	2007-04-12 11:58:14.275366513 +0200
+++ wireless-dev/kernel/power/disk.c	2007-04-12 12:49:07.965366513 +0200
@@ -322,7 +322,34 @@ static const char * const pm_disk_modes[
 
 static ssize_t disk_show(struct subsystem * subsys, char * buf)
 {
-	return sprintf(buf, "%s\n", pm_disk_modes[pm_disk_mode]);
+	int i;
+	char *start = buf;
+
+	for (i = PM_DISK_PLATFORM; i < PM_DISK_MAX; i++) {
+		if (!pm_disk_modes[i])
+			continue;
+		switch (i) {
+		case PM_DISK_SHUTDOWN:
+		case PM_DISK_REBOOT:
+		case PM_DISK_TEST:
+		case PM_DISK_TESTPROC:
+			break;
+		default:
+			if (pm_ops && pm_ops->enter &&
+			    (i == pm_ops->pm_disk_mode))
+				break;
+			/* not a valid mode, continue with loop */
+			continue;
+		}
+		if (i == pm_disk_mode)
+			buf += sprintf(buf, "[%s]", pm_disk_modes[i]);
+		else
+			buf += sprintf(buf, "%s", pm_disk_modes[i]);
+		if (i+1 != PM_DISK_MAX)
+			buf += sprintf(buf, " ");
+	}
+	buf += sprintf(buf, "\n");
+	return buf-start;
 }
 
 
--- wireless-dev.orig/Documentation/power/interface.txt	2007-04-12 11:58:14.285366513 +0200
+++ wireless-dev/Documentation/power/interface.txt	2007-04-12 11:58:15.895366513 +0200
@@ -34,8 +34,12 @@ for 5 seconds, resume devices, unfreeze 
 we are able to look in the log messages and work out, for example, which code
 is being slow and which device drivers are misbehaving.
 
-Reading from this file will display what the mode is currently set
-to. Writing to this file will accept one of
+Reading from this file will display all supported modes and the currently
+selected one in brackets, for example
+
+	[shutdown] reboot test testproc
+
+Writing to this file will accept one of
 
        'platform' (only if the platform supports it)
        'shutdown'

Re: [PATCH v2] power management: change /sys/power/disk display

[Rafael J. Wysocki]

On Thursday, 12 April 2007 12:49, Johannes Berg wrote:
> This patch changes /sys/power/disk to display all valid modes
> as well as the currently selected one in a fashion known from
> the LED subsystem.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

Looks OK to me.  Pavel?

> ---
>  Documentation/power/interface.txt |    8 ++++++--
>  kernel/power/disk.c               |   29 ++++++++++++++++++++++++++++-
>  2 files changed, 34 insertions(+), 3 deletions(-)
> 
> --- wireless-dev.orig/kernel/power/disk.c	2007-04-12 11:58:14.275366513 +0200
> +++ wireless-dev/kernel/power/disk.c	2007-04-12 12:49:07.965366513 +0200
> @@ -322,7 +322,34 @@ static const char * const pm_disk_modes[
>  
>  static ssize_t disk_show(struct subsystem * subsys, char * buf)
>  {
> -	return sprintf(buf, "%s\n", pm_disk_modes[pm_disk_mode]);
> +	int i;
> +	char *start = buf;
> +
> +	for (i = PM_DISK_PLATFORM; i < PM_DISK_MAX; i++) {
> +		if (!pm_disk_modes[i])
> +			continue;
> +		switch (i) {
> +		case PM_DISK_SHUTDOWN:
> +		case PM_DISK_REBOOT:
> +		case PM_DISK_TEST:
> +		case PM_DISK_TESTPROC:
> +			break;
> +		default:
> +			if (pm_ops && pm_ops->enter &&
> +			    (i == pm_ops->pm_disk_mode))
> +				break;
> +			/* not a valid mode, continue with loop */
> +			continue;
> +		}
> +		if (i == pm_disk_mode)
> +			buf += sprintf(buf, "[%s]", pm_disk_modes[i]);
> +		else
> +			buf += sprintf(buf, "%s", pm_disk_modes[i]);
> +		if (i+1 != PM_DISK_MAX)
> +			buf += sprintf(buf, " ");
> +	}
> +	buf += sprintf(buf, "\n");
> +	return buf-start;
>  }
>  
>  
> --- wireless-dev.orig/Documentation/power/interface.txt	2007-04-12 11:58:14.285366513 +0200
> +++ wireless-dev/Documentation/power/interface.txt	2007-04-12 11:58:15.895366513 +0200
> @@ -34,8 +34,12 @@ for 5 seconds, resume devices, unfreeze 
>  we are able to look in the log messages and work out, for example, which code
>  is being slow and which device drivers are misbehaving.
>  
> -Reading from this file will display what the mode is currently set
> -to. Writing to this file will accept one of
> +Reading from this file will display all supported modes and the currently
> +selected one in brackets, for example
> +
> +	[shutdown] reboot test testproc
> +
> +Writing to this file will accept one of
>  
>         'platform' (only if the platform supports it)
>         'shutdown'

Re: [PATCH v2] power management: change /sys/power/disk display

[Pavel Machek]

Hi!

> > This patch changes /sys/power/disk to display all valid modes
> > as well as the currently selected one in a fashion known from
> > the LED subsystem.
> > 
> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> 
> Looks OK to me.  Pavel?

ACK.

						Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html