[patch 2.6.18-rc2-mm 1/2] PM: make /sys/devices/.../power/state optional

[patch 2.6.18-rc2-mm 1/2] PM: make /sys/devices/.../power/state optional

[David Brownell]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

There seems to be a consensus on linux-pm that the sysfs power/state files
are the wrong approach to userspace having control over runtime PM.  Not
that there's agreement on what such controls _should_ look like, or even
that there should be such controls for purposes other than testing...

So here's a patch that just makes it easy to get rid of this misfeature.
It'll help flush out any userspace software that really thinks it needs that
mechanism; PCMCIA seems to no longer be in that camp, it has it's own
sysfs attribute.

- Dave


[-- Attachment #2: pm-sysfs.patch --]
[-- Type: text/x-diff, Size: 2060 bytes --]

Add a new PM_SYSFS_DEPRECATED config option to control whether or
not the /sys/devices/.../power/state files are provided.  This will
make it easier to get rid of that mechanism when the time comes,
and to verify that userspace tools work right without it.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>

Index: linux/kernel/power/Kconfig
===================================================================
--- linux.orig/kernel/power/Kconfig	2006-07-18 11:00:35.000000000 -0700
+++ linux/kernel/power/Kconfig	2006-07-18 11:14:57.000000000 -0700
@@ -53,6 +53,17 @@
 	CAUTION: this option will cause your machine's real-time clock to be
 	set to an invalid time after a resume.
 
+config PM_SYSFS_DEPRECATED
+	bool "Driver model /sys/devices/.../power/state files (DEPRECATED)"
+	depends on PM && SYSFS
+	default n
+	help
+	  The driver model started out with a sysfs file intended to provide
+	  a userspace hook for device power management.  This feature has never
+	  worked very well, except for limited testing purposes, and so it will
+	  be removed.   It's not clear that a generic mechanism could really
+	  handle the wide variability of device power states; any replacements
+	  are likely to be bus or driver specific.
 
 config SOFTWARE_SUSPEND
 	bool "Software Suspend"
Index: linux/drivers/base/power/sysfs.c
===================================================================
--- linux.orig/drivers/base/power/sysfs.c	2006-07-18 11:13:56.000000000 -0700
+++ linux/drivers/base/power/sysfs.c	2006-07-18 11:15:24.000000000 -0700
@@ -7,6 +7,8 @@
 #include "power.h"
 
 
+#ifdef	CONFIG_PM_SYSFS_DEPRECATED
+
 /**
  *	state - Control current power state of device
  *
@@ -67,6 +69,8 @@
 
 static DEVICE_ATTR(state, 0644, state_show, state_store);
 
+#endif	/* CONFIG_PM_SYSFS_DEPRECATED */
+
 /*
  *	wakeup - Report/change current wakeup option for device
  *
@@ -140,7 +144,9 @@
 
 
 static struct attribute * power_attrs[] = {
+#ifdef	CONFIG_PM_SYSFS_DEPRECATED
 	&dev_attr_state.attr,
+#endif
 	&dev_attr_wakeup.attr,
 	NULL,
 };

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

Re: [patch 2.6.18-rc2-mm 1/2] PM: make /sys/devices/.../power/state optional

[Pavel Machek]

HI!

> There seems to be a consensus on linux-pm that the sysfs power/state files
> are the wrong approach to userspace having control over runtime PM.  Not
> that there's agreement on what such controls _should_ look like, or even
> that there should be such controls for purposes other than testing...
> 
> So here's a patch that just makes it easy to get rid of this misfeature.
> It'll help flush out any userspace software that really thinks it needs that
> mechanism; PCMCIA seems to no longer be in that camp, it has it's own
> sysfs attribute.

> Add a new PM_SYSFS_DEPRECATED config option to control whether or
> not the /sys/devices/.../power/state files are provided.  This will
> make it easier to get rid of that mechanism when the time comes,
> and to verify that userspace tools work right without it.
> 
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>

Looks good to me, ACK.

(And sorry for long delay). 

I guess this should go with it...? OTOH andrew wanted alternative,
first, before removing the feature... but it is unclear to me if we
want the feature at all on the production system, so...

Signed-off-by: Pavel Machek <pavel@suse.cz>

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index 87851ef..4902bef 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -6,6 +6,16 @@ be removed from this file.
 
 ---------------------------
 
+What:	/sys/device/.../power/state
+When:	July 2007
+Files:	
+Why:	Because it takes integers, and different userland applications
+	expect different numbers to mean different things.
+	(Pcmcia expect 2 for off, some other code expects 3 for off).
+Who:	Pavel Machek <pavel@suse.cz>
+
+---------------------------
+
 What:	RAW driver (CONFIG_RAW_DRIVER)
 When:	December 2005
 Why:	declared obsolete since kernel 2.6.3

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

Re: [patch 2.6.18-rc2-mm 1/2] PM: make /sys/devices/.../power/state optional

[David Brownell]

On Wednesday 16 August 2006 4:57 am, Pavel Machek wrote:
> > Add a new PM_SYSFS_DEPRECATED config option to control whether or
> > not the /sys/devices/.../power/state files are provided.  ...
> 
> Looks good to me, ACK.
> 
> (And sorry for long delay). 

Greg still hasn't merged it; so, no prob.  :)

I updated patch 2/2  so that it doesn't test for the problematic and
incomplete prepare_suspend() method, and to remove prepare_suspend().
(That removal was discussed separately.)



> I guess this should go with it...? OTOH andrew wanted alternative,
> first, before removing the feature... but it is unclear to me if we
> want the feature at all on the production system, so...

How about this one instead?  The issue is not limited to that sysfs
file and its use of PM_EVENT_* codes, that's just the most visible
facet of the problem.

- Dave

Index: o26/Documentation/feature-removal-schedule.txt
===================================================================
--- o26.orig/Documentation/feature-removal-schedule.txt	2006-07-30 22:08:10.000000000 -0700
+++ o26/Documentation/feature-removal-schedule.txt	2006-08-16 11:34:54.000000000 -0700
@@ -6,6 +6,21 @@ be removed from this file.
 
 ---------------------------
 
+What:	/sys/devices/.../power/state
+	dev->power.power_state
+	dpm_runtime_{suspend,resume)()
+When:	July 2007
+Why:	Broken design for runtime control over driver power states, confusing
+	driver-internal runtime power management with:  mechanisms to support
+	system-wide sleep state transitions; event codes that distinguish
+	different phases of swsusp "sleep" transitions; and userspace policy
+	inputs.  This framework was never widely used, and most attempts to
+	use it were broken.  Drivers should instead be exposing domain-specific
+	interfaces either to kernel or to userspace, iff they are even needed.
+Who:	Pavel Machek <pavel@suse.cz>
+
+---------------------------
+
 What:	RAW driver (CONFIG_RAW_DRIVER)
 When:	December 2005
 Why:	declared obsolete since kernel 2.6.3

Re: [patch 2.6.18-rc2-mm 1/2] PM: make /sys/devices/.../power/state optional

[Pavel Machek]

Hi!

> > > Add a new PM_SYSFS_DEPRECATED config option to control whether or
> > > not the /sys/devices/.../power/state files are provided.  ...
> > 
> > Looks good to me, ACK.
> > 
> > (And sorry for long delay). 
> 
> Greg still hasn't merged it; so, no prob.  :)
> 
> I updated patch 2/2  so that it doesn't test for the problematic and
> incomplete prepare_suspend() method, and to remove prepare_suspend().
> (That removal was discussed separately.)
> 
> 
> 
> > I guess this should go with it...? OTOH andrew wanted alternative,
> > first, before removing the feature... but it is unclear to me if we
> > want the feature at all on the production system, so...
> 
> How about this one instead?  The issue is not limited to that sysfs
> file and its use of PM_EVENT_* codes, that's just the most visible
> facet of the problem.

Looks good to me.

> Index: o26/Documentation/feature-removal-schedule.txt
> ===================================================================
> --- o26.orig/Documentation/feature-removal-schedule.txt	2006-07-30 22:08:10.000000000 -0700
> +++ o26/Documentation/feature-removal-schedule.txt	2006-08-16 11:34:54.000000000 -0700
> @@ -6,6 +6,21 @@ be removed from this file.
>  
>  ---------------------------
>  
> +What:	/sys/devices/.../power/state
> +	dev->power.power_state
> +	dpm_runtime_{suspend,resume)()
> +When:	July 2007
> +Why:	Broken design for runtime control over driver power states, confusing
> +	driver-internal runtime power management with:  mechanisms to support
> +	system-wide sleep state transitions; event codes that distinguish
> +	different phases of swsusp "sleep" transitions; and userspace policy
> +	inputs.  This framework was never widely used, and most attempts to
> +	use it were broken.  Drivers should instead be exposing domain-specific
> +	interfaces either to kernel or to userspace, iff they are even needed.
> +Who:	Pavel Machek <pavel@suse.cz>
> +
> +---------------------------
> +
>  What:	RAW driver (CONFIG_RAW_DRIVER)
>  When:	December 2005
>  Why:	declared obsolete since kernel 2.6.3

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