[PATCH -mm 0/7] PM: Remove unused and unnecessary features from suspend and resume core

[PATCH -mm 0/7] PM: Remove unused and unnecessary features from suspend and resume core

[Rafael J. Wysocki]

Hi,

The following series of patches removes some unused and unnecessary features
from the suspend and resume core code.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

[PATCH -mm 1/7] PM: Remove pm_parent from struct dev_pm_info

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The pm_parent member of struct dev_pm_info (defined in include/linux/pm.h) is
only used to check if the device's parent is in the right state while the
device is being suspended or resumed.  However, this can be done just as well
with the help of the parent pointer in struct device, so pm_parent can be
removed along with some code that handles it.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: David Brownell <dbrownell@users.sourceforge.net>
---
 drivers/base/power/main.c    |   30 ++++--------------------------
 drivers/base/power/resume.c  |    7 +++----
 drivers/base/power/suspend.c |    7 +++----
 include/linux/pm.h           |    3 ---
 4 files changed, 10 insertions(+), 37 deletions(-)

Index: linux-2.6.22-rc4-mm2/drivers/base/power/main.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/base/power/main.c
+++ linux-2.6.22-rc4-mm2/drivers/base/power/main.c
@@ -33,28 +33,7 @@ DEFINE_MUTEX(dpm_list_mtx);
 
 int (*platform_enable_wakeup)(struct device *dev, int is_on);
 
-
-/**
- *	device_pm_set_parent - Specify power dependency.
- *	@dev:		Device who needs power.
- *	@parent:	Device that supplies power.
- *
- *	This function is used to manually describe a power-dependency
- *	relationship. It may be used to specify a transversal relationship
- *	(where the power supplier is not the physical (or electrical)
- *	ancestor of a specific device.
- *	The effect of this is that the supplier will not be powered down
- *	before the power dependent.
- */
-
-void device_pm_set_parent(struct device * dev, struct device * parent)
-{
-	put_device(dev->power.pm_parent);
-	dev->power.pm_parent = get_device(parent);
-}
-EXPORT_SYMBOL_GPL(device_pm_set_parent);
-
-int device_pm_add(struct device * dev)
+int device_pm_add(struct device *dev)
 {
 	int error;
 
@@ -63,21 +42,20 @@ int device_pm_add(struct device * dev)
 		 kobject_name(&dev->kobj));
 	mutex_lock(&dpm_list_mtx);
 	list_add_tail(&dev->power.entry, &dpm_active);
-	device_pm_set_parent(dev, dev->parent);
-	if ((error = dpm_sysfs_add(dev)))
+	error = dpm_sysfs_add(dev);
+	if (error)
 		list_del(&dev->power.entry);
 	mutex_unlock(&dpm_list_mtx);
 	return error;
 }
 
-void device_pm_remove(struct device * dev)
+void device_pm_remove(struct device *dev)
 {
 	pr_debug("PM: Removing info for %s:%s\n",
 		 dev->bus ? dev->bus->name : "No Bus",
 		 kobject_name(&dev->kobj));
 	mutex_lock(&dpm_list_mtx);
 	dpm_sysfs_remove(dev);
-	put_device(dev->power.pm_parent);
 	list_del_init(&dev->power.entry);
 	mutex_unlock(&dpm_list_mtx);
 }
Index: linux-2.6.22-rc4-mm2/drivers/base/power/resume.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/base/power/resume.c
+++ linux-2.6.22-rc4-mm2/drivers/base/power/resume.c
@@ -29,12 +29,11 @@ int resume_device(struct device * dev)
 
 	down(&dev->sem);
 
-	if (dev->power.pm_parent
-			&& dev->power.pm_parent->power.power_state.event) {
+	if (dev->parent && dev->parent->power.power_state.event) {
 		dev_err(dev, "PM: resume from %d, parent %s still %d\n",
 			dev->power.power_state.event,
-			dev->power.pm_parent->bus_id,
-			dev->power.pm_parent->power.power_state.event);
+			dev->parent->bus_id,
+			dev->parent->power.power_state.event);
 	}
 
 	if (dev->bus && dev->bus->resume) {
Index: linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/base/power/suspend.c
+++ linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
@@ -55,13 +55,12 @@ int suspend_device(struct device * dev, 
 		dev_dbg(dev, "PM: suspend %d-->%d\n",
 			dev->power.power_state.event, state.event);
 	}
-	if (dev->power.pm_parent
-			&& dev->power.pm_parent->power.power_state.event) {
+	if (dev->parent && dev->parent->power.power_state.event) {
 		dev_err(dev,
 			"PM: suspend %d->%d, parent %s already %d\n",
 			dev->power.power_state.event, state.event,
-			dev->power.pm_parent->bus_id,
-			dev->power.pm_parent->power.power_state.event);
+			dev->parent->bus_id,
+			dev->parent->power.power_state.event);
 	}
 
 	dev->power.prev_state = dev->power.power_state;
Index: linux-2.6.22-rc4-mm2/include/linux/pm.h
===================================================================
--- linux-2.6.22-rc4-mm2.orig/include/linux/pm.h
+++ linux-2.6.22-rc4-mm2/include/linux/pm.h
@@ -237,13 +237,10 @@ struct dev_pm_info {
 	unsigned		should_wakeup:1;
 	pm_message_t		prev_state;
 	void			* saved_state;
-	struct device		* pm_parent;
 	struct list_head	entry;
 #endif
 };
 
-extern void device_pm_set_parent(struct device * dev, struct device * parent);
-
 extern int device_power_down(pm_message_t state);
 extern void device_power_up(void);
 extern void device_resume(void);

[PATCH -mm 2/7] PM: Remove saved_state from struct dev_pm_info

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The saved_state member of struct dev_pm_info, defined in include/linux/pm.h, is
not used anywhere, so it can be removed.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 include/linux/pm.h |    1 -
 1 file changed, 1 deletion(-)

Index: linux-2.6.22-rc4-mm2/include/linux/pm.h
===================================================================
--- linux-2.6.22-rc4-mm2.orig/include/linux/pm.h
+++ linux-2.6.22-rc4-mm2/include/linux/pm.h
@@ -236,7 +236,6 @@ struct dev_pm_info {
 #ifdef	CONFIG_PM
 	unsigned		should_wakeup:1;
 	pm_message_t		prev_state;
-	void			* saved_state;
 	struct list_head	entry;
 #endif
 };

[PATCH -mm 3/7] PM: Simplify suspend_device

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Reduce code duplication in drivers/base/suspend.c by introducing a separate
function for printing diagnostic messages.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/base/power/suspend.c |   49 +++++++++++++++----------------------------
 1 file changed, 18 insertions(+), 31 deletions(-)

Index: linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/base/power/suspend.c
+++ linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
@@ -40,6 +40,14 @@ static inline char *suspend_verb(u32 eve
 }
 
 
+static void
+suspend_device_dbg(struct device * dev, pm_message_t state, char *info)
+{
+	dev_dbg(dev, "%s%s%s\n", info, suspend_verb(state.event),
+		((state.event == PM_EVENT_SUSPEND) && device_may_wakeup(dev)) ?
+		", may wakeup" : "");
+}
+
 /**
  *	suspend_device - Save state of one device.
  *	@dev:	Device.
@@ -66,37 +74,21 @@ int suspend_device(struct device * dev, 
 	dev->power.prev_state = dev->power.power_state;
 
 	if (dev->class && dev->class->suspend && !dev->power.power_state.event) {
-		dev_dbg(dev, "class %s%s\n",
-			suspend_verb(state.event),
-			((state.event == PM_EVENT_SUSPEND)
-					&& device_may_wakeup(dev))
-				? ", may wakeup"
-				: ""
-			);
+		suspend_device_dbg(dev, state, "class ");
 		error = dev->class->suspend(dev, state);
 		suspend_report_result(dev->class->suspend, error);
 	}
 
-	if (!error && dev->type && dev->type->suspend && !dev->power.power_state.event) {
-		dev_dbg(dev, "%s%s\n",
-			suspend_verb(state.event),
-			((state.event == PM_EVENT_SUSPEND)
-					&& device_may_wakeup(dev))
-				? ", may wakeup"
-				: ""
-			);
+	if (!error && dev->type && dev->type->suspend
+	    && !dev->power.power_state.event) {
+		suspend_device_dbg(dev, state, "type ");
 		error = dev->type->suspend(dev, state);
 		suspend_report_result(dev->type->suspend, error);
 	}
 
-	if (!error && dev->bus && dev->bus->suspend && !dev->power.power_state.event) {
-		dev_dbg(dev, "%s%s\n",
-			suspend_verb(state.event),
-			((state.event == PM_EVENT_SUSPEND)
-					&& device_may_wakeup(dev))
-				? ", may wakeup"
-				: ""
-			);
+	if (!error && dev->bus && dev->bus->suspend
+	    && !dev->power.power_state.event) {
+		suspend_device_dbg(dev, state, "");
 		error = dev->bus->suspend(dev, state);
 		suspend_report_result(dev->bus->suspend, error);
 	}
@@ -114,14 +106,9 @@ static int suspend_device_late(struct de
 {
 	int error = 0;
 
-	if (dev->bus && dev->bus->suspend_late && !dev->power.power_state.event) {
-		dev_dbg(dev, "LATE %s%s\n",
-			suspend_verb(state.event),
-			((state.event == PM_EVENT_SUSPEND)
-					&& device_may_wakeup(dev))
-				? ", may wakeup"
-				: ""
-			);
+	if (dev->bus && dev->bus->suspend_late
+	    && !dev->power.power_state.event) {
+		suspend_device_dbg(dev, state, "LATE ");
 		error = dev->bus->suspend_late(dev, state);
 		suspend_report_result(dev->bus->suspend_late, error);
 	}

[PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The suspend and resume support in struct device_type (include/linux/device.h)
is not used anywhere.  It is also undocumented, so it can be removed.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/resume.c  |    5 -----
 drivers/base/power/suspend.c |    7 -------
 include/linux/device.h       |    2 --
 3 files changed, 14 deletions(-)

Index: linux-2.6.22-rc4-mm2/drivers/base/power/resume.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/base/power/resume.c
+++ linux-2.6.22-rc4-mm2/drivers/base/power/resume.c
@@ -41,11 +41,6 @@ int resume_device(struct device * dev)
 		error = dev->bus->resume(dev);
 	}
 
-	if (!error && dev->type && dev->type->resume) {
-		dev_dbg(dev,"resuming\n");
-		error = dev->type->resume(dev);
-	}
-
 	if (!error && dev->class && dev->class->resume) {
 		dev_dbg(dev,"class resume\n");
 		error = dev->class->resume(dev);
Index: linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/base/power/suspend.c
+++ linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
@@ -79,13 +79,6 @@ int suspend_device(struct device * dev, 
 		suspend_report_result(dev->class->suspend, error);
 	}
 
-	if (!error && dev->type && dev->type->suspend
-	    && !dev->power.power_state.event) {
-		suspend_device_dbg(dev, state, "type ");
-		error = dev->type->suspend(dev, state);
-		suspend_report_result(dev->type->suspend, error);
-	}
-
 	if (!error && dev->bus && dev->bus->suspend
 	    && !dev->power.power_state.event) {
 		suspend_device_dbg(dev, state, "");
Index: linux-2.6.22-rc4-mm2/include/linux/device.h
===================================================================
--- linux-2.6.22-rc4-mm2.orig/include/linux/device.h
+++ linux-2.6.22-rc4-mm2/include/linux/device.h
@@ -340,8 +340,6 @@ struct device_type {
 	int (*uevent)(struct device *dev, char **envp, int num_envp,
 		      char *buffer, int buffer_size);
 	void (*release)(struct device *dev);
-	int (*suspend)(struct device * dev, pm_message_t state);
-	int (*resume)(struct device * dev);
 };
 
 /* interface for exporting device attributes */

Re: [PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[Kay Sievers]

Dmitry, you added this recently, is this used in any code you plan to
merge soon?

Thanks,
Kay

On 6/13/07, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> The suspend and resume support in struct device_type (include/linux/device.h)
> is not used anywhere.  It is also undocumented, so it can be removed.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/base/power/resume.c  |    5 -----
>  drivers/base/power/suspend.c |    7 -------
>  include/linux/device.h       |    2 --
>  3 files changed, 14 deletions(-)
>
> Index: linux-2.6.22-rc4-mm2/drivers/base/power/resume.c
> ===================================================================
> --- linux-2.6.22-rc4-mm2.orig/drivers/base/power/resume.c
> +++ linux-2.6.22-rc4-mm2/drivers/base/power/resume.c
> @@ -41,11 +41,6 @@ int resume_device(struct device * dev)
>                 error = dev->bus->resume(dev);
>         }
>
> -       if (!error && dev->type && dev->type->resume) {
> -               dev_dbg(dev,"resuming\n");
> -               error = dev->type->resume(dev);
> -       }
> -
>         if (!error && dev->class && dev->class->resume) {
>                 dev_dbg(dev,"class resume\n");
>                 error = dev->class->resume(dev);
> Index: linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
> ===================================================================
> --- linux-2.6.22-rc4-mm2.orig/drivers/base/power/suspend.c
> +++ linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
> @@ -79,13 +79,6 @@ int suspend_device(struct device * dev,
>                 suspend_report_result(dev->class->suspend, error);
>         }
>
> -       if (!error && dev->type && dev->type->suspend
> -           && !dev->power.power_state.event) {
> -               suspend_device_dbg(dev, state, "type ");
> -               error = dev->type->suspend(dev, state);
> -               suspend_report_result(dev->type->suspend, error);
> -       }
> -
>         if (!error && dev->bus && dev->bus->suspend
>             && !dev->power.power_state.event) {
>                 suspend_device_dbg(dev, state, "");
> Index: linux-2.6.22-rc4-mm2/include/linux/device.h
> ===================================================================
> --- linux-2.6.22-rc4-mm2.orig/include/linux/device.h
> +++ linux-2.6.22-rc4-mm2/include/linux/device.h
> @@ -340,8 +340,6 @@ struct device_type {
>         int (*uevent)(struct device *dev, char **envp, int num_envp,
>                       char *buffer, int buffer_size);
>         void (*release)(struct device *dev);
> -       int (*suspend)(struct device * dev, pm_message_t state);
> -       int (*resume)(struct device * dev);
>  };
>
>  /* interface for exporting device attributes */
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[Dmitry Torokhov]

On Wednesday 13 June 2007 18:20, Kay Sievers wrote:
> Dmitry, you added this recently, is this used in any code you plan to
> merge soon?
>

Yes, I will need it to implement input device resume (mainly to restore LED
state and repeat rate for keyboards).
 
-- 
Dmitry

Re: [PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[Rafael J. Wysocki]

On Thursday, 14 June 2007 06:10, Dmitry Torokhov wrote:
> On Wednesday 13 June 2007 18:20, Kay Sievers wrote:
> > Dmitry, you added this recently, is this used in any code you plan to
> > merge soon?
> >
> 
> Yes, I will need it to implement input device resume (mainly to restore LED
> state and repeat rate for keyboards).

Would that be a big problem to reintroduce it along with the user?

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[Dmitry Torokhov]

On 6/14/07, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Thursday, 14 June 2007 06:10, Dmitry Torokhov wrote:
> > On Wednesday 13 June 2007 18:20, Kay Sievers wrote:
> > > Dmitry, you added this recently, is this used in any code you plan to
> > > merge soon?
> > >
> >
> > Yes, I will need it to implement input device resume (mainly to restore LED
> > state and repeat rate for keyboards).
>
> Would that be a big problem to reintroduce it along with the user?
>

Yes because that part is in Greg's domain so I am trying to set
infrastructure up before I can commit my patches into my tree so that
Andrew can safely pull from me into -mm. Greg normally adds such stuff
during merge window so that means if we remove it now we'd need ~2
releases to get it and the user back in.

-- 
Dmitry

Re: [PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[Greg KH]

On Thu, Jun 14, 2007 at 08:59:20AM -0400, Dmitry Torokhov wrote:
>  On 6/14/07, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Thursday, 14 June 2007 06:10, Dmitry Torokhov wrote:
> > > On Wednesday 13 June 2007 18:20, Kay Sievers wrote:
> > > > Dmitry, you added this recently, is this used in any code you plan to
> > > > merge soon?
> > > >
> > >
> > > Yes, I will need it to implement input device resume (mainly to restore 
> > LED
> > > state and repeat rate for keyboards).
> >
> > Would that be a big problem to reintroduce it along with the user?
> >
> 
>  Yes because that part is in Greg's domain so I am trying to set
>  infrastructure up before I can commit my patches into my tree so that
>  Andrew can safely pull from me into -mm. Greg normally adds such stuff
>  during merge window so that means if we remove it now we'd need ~2
>  releases to get it and the user back in.

Yes, don't worry, I'm not going to remove this as I know you rely on it.

Sorry for the delay in getting to these patches, I'm in meetings down in
CA for the rest of the week...

thanks,

greg k-h

Re: [PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[Rafael J. Wysocki]

On Thursday, 14 June 2007 19:46, Greg KH wrote:
> On Thu, Jun 14, 2007 at 08:59:20AM -0400, Dmitry Torokhov wrote:
> >  On 6/14/07, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > On Thursday, 14 June 2007 06:10, Dmitry Torokhov wrote:
> > > > On Wednesday 13 June 2007 18:20, Kay Sievers wrote:
> > > > > Dmitry, you added this recently, is this used in any code you plan to
> > > > > merge soon?
> > > > >
> > > >
> > > > Yes, I will need it to implement input device resume (mainly to restore 
> > > LED
> > > > state and repeat rate for keyboards).
> > >
> > > Would that be a big problem to reintroduce it along with the user?
> > >
> > 
> >  Yes because that part is in Greg's domain so I am trying to set
> >  infrastructure up before I can commit my patches into my tree so that
> >  Andrew can safely pull from me into -mm. Greg normally adds such stuff
> >  during merge window so that means if we remove it now we'd need ~2
> >  releases to get it and the user back in.
> 
> Yes, don't worry, I'm not going to remove this as I know you rely on it.

Hm, in that case I'd have to rework the patches 5-7.  Perhaps I should resend
the entire series once again, without the $subject patch?

> Sorry for the delay in getting to these patches, I'm in meetings down in
> CA for the rest of the week...

No big deal. :-)

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[Greg KH]

On Fri, Jun 15, 2007 at 12:32:19AM +0200, Rafael J. Wysocki wrote:
> On Thursday, 14 June 2007 19:46, Greg KH wrote:
> > On Thu, Jun 14, 2007 at 08:59:20AM -0400, Dmitry Torokhov wrote:
> > >  On 6/14/07, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > > On Thursday, 14 June 2007 06:10, Dmitry Torokhov wrote:
> > > > > On Wednesday 13 June 2007 18:20, Kay Sievers wrote:
> > > > > > Dmitry, you added this recently, is this used in any code you plan to
> > > > > > merge soon?
> > > > > >
> > > > >
> > > > > Yes, I will need it to implement input device resume (mainly to restore 
> > > > LED
> > > > > state and repeat rate for keyboards).
> > > >
> > > > Would that be a big problem to reintroduce it along with the user?
> > > >
> > > 
> > >  Yes because that part is in Greg's domain so I am trying to set
> > >  infrastructure up before I can commit my patches into my tree so that
> > >  Andrew can safely pull from me into -mm. Greg normally adds such stuff
> > >  during merge window so that means if we remove it now we'd need ~2
> > >  releases to get it and the user back in.
> > 
> > Yes, don't worry, I'm not going to remove this as I know you rely on it.
> 
> Hm, in that case I'd have to rework the patches 5-7.  Perhaps I should resend
> the entire series once again, without the $subject patch?

You do?  Conflicts are that bad?

Let me try and apply them and let you know what I've accepted in my
tree...

thanks,

greg k-h

Re: [PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[Greg KH]

On Fri, Jun 15, 2007 at 12:32:19AM +0200, Rafael J. Wysocki wrote:
> On Thursday, 14 June 2007 19:46, Greg KH wrote:
> > On Thu, Jun 14, 2007 at 08:59:20AM -0400, Dmitry Torokhov wrote:
> > >  On 6/14/07, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > > On Thursday, 14 June 2007 06:10, Dmitry Torokhov wrote:
> > > > > On Wednesday 13 June 2007 18:20, Kay Sievers wrote:
> > > > > > Dmitry, you added this recently, is this used in any code you plan to
> > > > > > merge soon?
> > > > > >
> > > > >
> > > > > Yes, I will need it to implement input device resume (mainly to restore 
> > > > LED
> > > > > state and repeat rate for keyboards).
> > > >
> > > > Would that be a big problem to reintroduce it along with the user?
> > > >
> > > 
> > >  Yes because that part is in Greg's domain so I am trying to set
> > >  infrastructure up before I can commit my patches into my tree so that
> > >  Andrew can safely pull from me into -mm. Greg normally adds such stuff
> > >  during merge window so that means if we remove it now we'd need ~2
> > >  releases to get it and the user back in.
> > 
> > Yes, don't worry, I'm not going to remove this as I know you rely on it.
> 
> Hm, in that case I'd have to rework the patches 5-7.  Perhaps I should resend
> the entire series once again, without the $subject patch?

Ok, yeah, can you rework those three patches and resend them?  I took
patches 1-3 already.

thanks,

greg k-h

Re: [PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[Rafael J. Wysocki]

On Friday, 15 June 2007 01:14, Greg KH wrote:
> On Fri, Jun 15, 2007 at 12:32:19AM +0200, Rafael J. Wysocki wrote:
> > On Thursday, 14 June 2007 19:46, Greg KH wrote:
> > > On Thu, Jun 14, 2007 at 08:59:20AM -0400, Dmitry Torokhov wrote:
> > > >  On 6/14/07, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > > > On Thursday, 14 June 2007 06:10, Dmitry Torokhov wrote:
> > > > > > On Wednesday 13 June 2007 18:20, Kay Sievers wrote:
> > > > > > > Dmitry, you added this recently, is this used in any code you plan to
> > > > > > > merge soon?
> > > > > > >
> > > > > >
> > > > > > Yes, I will need it to implement input device resume (mainly to restore 
> > > > > LED
> > > > > > state and repeat rate for keyboards).
> > > > >
> > > > > Would that be a big problem to reintroduce it along with the user?
> > > > >
> > > > 
> > > >  Yes because that part is in Greg's domain so I am trying to set
> > > >  infrastructure up before I can commit my patches into my tree so that
> > > >  Andrew can safely pull from me into -mm. Greg normally adds such stuff
> > > >  during merge window so that means if we remove it now we'd need ~2
> > > >  releases to get it and the user back in.
> > > 
> > > Yes, don't worry, I'm not going to remove this as I know you rely on it.
> > 
> > Hm, in that case I'd have to rework the patches 5-7.  Perhaps I should resend
> > the entire series once again, without the $subject patch?
> 
> Ok, yeah, can you rework those three patches and resend them?

Sure, I will.

> I took patches 1-3 already.

Thanks!

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

[PATCH -mm 0/3] PM: Remove unused and unnecessary features from core suspend code (continued)

[Rafael J. Wysocki]

Hi,

The next messages contain the remaining three patches from the previous series
"PM: Remove unused and unnecessary features from suspend and resume core"
that I have promised to resend.

The patches apply to 2.6.22-rc4-mm2, which is particularly important for the
first one, as it is on top of some recent USB changes that right now are
mm-only.  If there are any problems with that, please let me know.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

[PATCH -mm 1/3] PM: Remove prev_state from struct dev_pm_info

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The prev_state member of struct dev_pm_info (defined in include/linux/pm.h) is
only used during a resume to check if the device's state before the suspend was
'off', in which case the device is not resumed.  However, in such cases the
decision whether or not to resume the device should be made on the driver level
and the resume callbacks from the device's bus and class should be executed
anyway (the may be needed for some things other than just powering on the
device).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/resume.c  |    3 +--
 drivers/base/power/suspend.c |    2 --
 drivers/usb/core/hub.c       |    5 -----
 drivers/usb/host/ohci-pci.c  |    2 --
 include/linux/pm.h           |    1 -
 5 files changed, 1 insertion(+), 12 deletions(-)

Index: linux-2.6.22-rc4-mm2/drivers/base/power/resume.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/base/power/resume.c
+++ linux-2.6.22-rc4-mm2/drivers/base/power/resume.c
@@ -88,8 +88,7 @@ void dpm_resume(void)
 		list_move_tail(entry, &dpm_active);
 
 		mutex_unlock(&dpm_list_mtx);
-		if (!dev->power.prev_state.event)
-			resume_device(dev);
+		resume_device(dev);
 		mutex_lock(&dpm_list_mtx);
 		put_device(dev);
 	}
Index: linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/base/power/suspend.c
+++ linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
@@ -71,8 +71,6 @@ int suspend_device(struct device * dev, 
 			dev->parent->power.power_state.event);
 	}
 
-	dev->power.prev_state = dev->power.power_state;
-
 	if (dev->class && dev->class->suspend && !dev->power.power_state.event) {
 		suspend_device_dbg(dev, state, "class ");
 		error = dev->class->suspend(dev, state);
Index: linux-2.6.22-rc4-mm2/drivers/usb/core/hub.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/usb/core/hub.c
+++ linux-2.6.22-rc4-mm2/drivers/usb/core/hub.c
@@ -623,7 +623,6 @@ static void mark_children_for_reset_resu
 
 		if (child) {
 			child->reset_resume = 1;
-			child->dev.power.prev_state.event = PM_EVENT_ON;
 			clear_port_feature(hdev, port1,
 					USB_PORT_FEAT_C_CONNECTION);
 		}
@@ -1190,10 +1189,6 @@ void usb_root_hub_lost_power(struct usb_
 	dev_warn(&rhdev->dev, "root hub lost power or was reset\n");
 	rhdev->reset_resume = 1;
 
-	/* Force the root hub to be resumed, even if it was suspended,
-	 * so that no wakeup events will get lost.
-	 */
-	rhdev->dev.power.prev_state.event = PM_EVENT_ON;
 }
 EXPORT_SYMBOL_GPL(usb_root_hub_lost_power);
 
Index: linux-2.6.22-rc4-mm2/include/linux/pm.h
===================================================================
--- linux-2.6.22-rc4-mm2.orig/include/linux/pm.h
+++ linux-2.6.22-rc4-mm2/include/linux/pm.h
@@ -235,7 +235,6 @@ struct dev_pm_info {
 	unsigned		can_wakeup:1;
 #ifdef	CONFIG_PM
 	unsigned		should_wakeup:1;
-	pm_message_t		prev_state;
 	struct list_head	entry;
 #endif
 };
Index: linux-2.6.22-rc4-mm2/drivers/usb/host/ohci-pci.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/usb/host/ohci-pci.c
+++ linux-2.6.22-rc4-mm2/drivers/usb/host/ohci-pci.c
@@ -281,8 +281,6 @@ static int ohci_pci_resume (struct usb_h
 	/* FIXME: we should try to detect loss of VBUS power here */
 	prepare_for_handover(hcd);
 
-	/* Force the PM core to resume the root hub */
-	hcd_to_bus(hcd)->root_hub->dev.power.prev_state.event = PM_EVENT_ON;
 	return 0;
 }
 

[PATCH -mm 2/3] PM: Remove power_state.event checks from suspend core code

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The suspend routines should be called for every device during a system sleep
transition, regardless of the device's state, so that drivers can regard these
method calls as notifications that the system is about to go to sleep, rather
than as directives to put their devices into the 'off' state.

This is documented in Documentation/power/devices.txt and is already done in
the core resume code, so it seems reasonable to make the core suspend code
behave accordingly.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/suspend.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Index: linux-2.6.22-rc4/drivers/base/power/suspend.c
===================================================================
--- linux-2.6.22-rc4.orig/drivers/base/power/suspend.c	2007-06-16 01:08:13.000000000 +0200
+++ linux-2.6.22-rc4/drivers/base/power/suspend.c	2007-06-16 01:12:04.000000000 +0200
@@ -71,21 +71,19 @@ int suspend_device(struct device * dev, 
 			dev->parent->power.power_state.event);
 	}
 
-	if (dev->class && dev->class->suspend && !dev->power.power_state.event) {
+	if (dev->class && dev->class->suspend) {
 		suspend_device_dbg(dev, state, "class ");
 		error = dev->class->suspend(dev, state);
 		suspend_report_result(dev->class->suspend, error);
 	}
 
-	if (!error && dev->type && dev->type->suspend
-	    && !dev->power.power_state.event) {
+	if (!error && dev->type && dev->type->suspend) {
 		suspend_device_dbg(dev, state, "type ");
 		error = dev->type->suspend(dev, state);
 		suspend_report_result(dev->type->suspend, error);
 	}
 
-	if (!error && dev->bus && dev->bus->suspend
-	    && !dev->power.power_state.event) {
+	if (!error && dev->bus && dev->bus->suspend) {
 		suspend_device_dbg(dev, state, "");
 		error = dev->bus->suspend(dev, state);
 		suspend_report_result(dev->bus->suspend, error);
@@ -104,8 +102,7 @@ static int suspend_device_late(struct de
 {
 	int error = 0;
 
-	if (dev->bus && dev->bus->suspend_late
-	    && !dev->power.power_state.event) {
+	if (dev->bus && dev->bus->suspend_late) {
 		suspend_device_dbg(dev, state, "LATE ");
 		error = dev->bus->suspend_late(dev, state);
 		suspend_report_result(dev->bus->suspend_late, error);

[PATCH -mm 3/3] PM: Do not check parent state in suspend and resume core code

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The checks if the device's parent is in the right state done in
drivers/base/power/suspend.c and drivers/base/power/resume.c serve no particular
purpose, since if the parent is in a wrong power state, the device's suspend or
resume callbacks are supposed to return an error anyway.  Moreover, they are
also useless from the sanity checking point of view, because they rely on the
code being checked to set dev->parent->power.power_state.event appropriately,
which need not happen if that code is buggy.  For these reasons they can be
removed.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: David Brownell <dbrownell@users.sourceforge.net>
---
 drivers/base/power/resume.c  |    7 -------
 drivers/base/power/suspend.c |    7 -------
 2 files changed, 14 deletions(-)

Index: linux-2.6.22-rc4/drivers/base/power/resume.c
===================================================================
--- linux-2.6.22-rc4.orig/drivers/base/power/resume.c	2007-06-16 01:08:13.000000000 +0200
+++ linux-2.6.22-rc4/drivers/base/power/resume.c	2007-06-16 01:12:53.000000000 +0200
@@ -29,13 +29,6 @@ int resume_device(struct device * dev)
 
 	down(&dev->sem);
 
-	if (dev->parent && dev->parent->power.power_state.event) {
-		dev_err(dev, "PM: resume from %d, parent %s still %d\n",
-			dev->power.power_state.event,
-			dev->parent->bus_id,
-			dev->parent->power.power_state.event);
-	}
-
 	if (dev->bus && dev->bus->resume) {
 		dev_dbg(dev,"resuming\n");
 		error = dev->bus->resume(dev);
Index: linux-2.6.22-rc4/drivers/base/power/suspend.c
===================================================================
--- linux-2.6.22-rc4.orig/drivers/base/power/suspend.c	2007-06-16 01:12:04.000000000 +0200
+++ linux-2.6.22-rc4/drivers/base/power/suspend.c	2007-06-16 01:12:53.000000000 +0200
@@ -63,13 +63,6 @@ int suspend_device(struct device * dev, 
 		dev_dbg(dev, "PM: suspend %d-->%d\n",
 			dev->power.power_state.event, state.event);
 	}
-	if (dev->parent && dev->parent->power.power_state.event) {
-		dev_err(dev,
-			"PM: suspend %d->%d, parent %s already %d\n",
-			dev->power.power_state.event, state.event,
-			dev->parent->bus_id,
-			dev->parent->power.power_state.event);
-	}
 
 	if (dev->class && dev->class->suspend) {
 		suspend_device_dbg(dev, state, "class ");

Re: [PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[David Brownell]

On Wednesday 13 June 2007, Dmitry Torokhov wrote:
> On Wednesday 13 June 2007 18:20, Kay Sievers wrote:
> > Dmitry, you added this recently, is this used in any code you plan to
> > merge soon?
> >
> 
> Yes, I will need it to implement input device resume (mainly to restore LED
> state and repeat rate for keyboards).

Why wouldn't that use the class device suspend/resume mechanism?

Re: [PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[Dmitry Torokhov]

On 6/14/07, David Brownell <david-b@pacbell.net> wrote:
> On Wednesday 13 June 2007, Dmitry Torokhov wrote:
> > On Wednesday 13 June 2007 18:20, Kay Sievers wrote:
> > > Dmitry, you added this recently, is this used in any code you plan to
> > > merge soon?
> > >
> >
> > Yes, I will need it to implement input device resume (mainly to restore LED
> > state and repeat rate for keyboards).
>
> Why wouldn't that use the class device suspend/resume mechanism?
>

Because they are not class devices anymore.

-- 
Dmitry

Re: [PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[David Brownell]

On Thursday 14 June 2007, Dmitry Torokhov wrote:
> On 6/14/07, David Brownell <david-b@pacbell.net> wrote:
> > On Wednesday 13 June 2007, Dmitry Torokhov wrote:
> > > On Wednesday 13 June 2007 18:20, Kay Sievers wrote:
> > > > Dmitry, you added this recently, is this used in any code you plan to
> > > > merge soon?
> > > >
> > >
> > > Yes, I will need it to implement input device resume (mainly to restore LED
> > > state and repeat rate for keyboards).
> >
> > Why wouldn't that use the class device suspend/resume mechanism?
> >
> 
> Because they are not class devices anymore.

Perhaps you mis-understood me:  the class level device suspend/resume
hooks don't rely on class_device.  I observe that /sys/class/input
still exists (2.6.22-rc4-git), so if all you mean is that it's no
longer using class_device, that's good.  One part of the reason to
stop using class_device is specifically to let framework code (like
input, network, or rtc) receive class level suspend/resume calls.

ISTR the RTC framework was the first to do that, but there's some
work afoot to teach the network stack to use that mechanism too.

Or are you referring to some input patches which I've not yet seen,
which cause /sys/class/input to vanish?

- Dave

Re: [PATCH -mm 4/7] PM: Remove suspend and resume support from struct device_type

[Dmitry Torokhov]

On 6/14/07, David Brownell <david-b@pacbell.net> wrote:
> On Thursday 14 June 2007, Dmitry Torokhov wrote:
> > On 6/14/07, David Brownell <david-b@pacbell.net> wrote:
> > > On Wednesday 13 June 2007, Dmitry Torokhov wrote:
> > > > On Wednesday 13 June 2007 18:20, Kay Sievers wrote:
> > > > > Dmitry, you added this recently, is this used in any code you plan to
> > > > > merge soon?
> > > > >
> > > >
> > > > Yes, I will need it to implement input device resume (mainly to restore LED
> > > > state and repeat rate for keyboards).
> > >
> > > Why wouldn't that use the class device suspend/resume mechanism?
> > >
> >
> > Because they are not class devices anymore.
>
> Perhaps you mis-understood me:  the class level device suspend/resume
> hooks don't rely on class_device.  I observe that /sys/class/input
> still exists (2.6.22-rc4-git), so if all you mean is that it's no
> longer using class_device, that's good.

Yes, I did misunderstood you.

The reason I can't use class-wide resume support is that since we
can't have class hierarchy we have mess of different objects dumped
into single pool (input_dev, evdev. mousedev, joydev, tsdev all relate
to /sys/class/input). And so instead of having class define standard
interface for all objects in it we have to rely on something else
(struct device_type - not visible fom userspace) to separate them and
add tweaks to interface. Can you see that I am not a fan of the
design? ;)

As far as input concerned - I need to resume input_dev but not evdev,
mousedev, tsdev, etc. as they are all just different representations
of the same object.

> One part of the reason to
> stop using class_device is specifically to let framework code (like
> input, network, or rtc) receive class level suspend/resume calls.

It could be done within the class_device infrastructure as well.

> ISTR the RTC framework was the first to do that, but there's some
> work afoot to teach the network stack to use that mechanism too.
>
> Or are you referring to some input patches which I've not yet seen,
> which cause /sys/class/input to vanish?
>

No, it will stay.

-- 
Dmitry

[PATCH -mm 5/7] PM: Remove prev_state from struct dev_pm_info

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The prev_state member of struct dev_pm_info (defined in include/linux/pm.h) is
only used during a resume to check if the device's state before the suspend was
'off', in which case the device is not resumed.  However, in such cases the
decision whether or not to resume the device should be made on the driver level
and the resume callbacks from the device's bus and class should be executed
anyway (the may be needed for some things other than just powering on the
device).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/resume.c  |    3 +--
 drivers/base/power/suspend.c |    2 --
 drivers/usb/core/hub.c       |    5 -----
 drivers/usb/host/ohci-pci.c  |    2 --
 include/linux/pm.h           |    1 -
 5 files changed, 1 insertion(+), 12 deletions(-)

Index: linux-2.6.22-rc4-mm2/drivers/base/power/resume.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/base/power/resume.c
+++ linux-2.6.22-rc4-mm2/drivers/base/power/resume.c
@@ -83,8 +83,7 @@ void dpm_resume(void)
 		list_move_tail(entry, &dpm_active);
 
 		mutex_unlock(&dpm_list_mtx);
-		if (!dev->power.prev_state.event)
-			resume_device(dev);
+		resume_device(dev);
 		mutex_lock(&dpm_list_mtx);
 		put_device(dev);
 	}
Index: linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/base/power/suspend.c
+++ linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
@@ -71,8 +71,6 @@ int suspend_device(struct device * dev, 
 			dev->parent->power.power_state.event);
 	}
 
-	dev->power.prev_state = dev->power.power_state;
-
 	if (dev->class && dev->class->suspend && !dev->power.power_state.event) {
 		suspend_device_dbg(dev, state, "class ");
 		error = dev->class->suspend(dev, state);
Index: linux-2.6.22-rc4-mm2/drivers/usb/core/hub.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/usb/core/hub.c
+++ linux-2.6.22-rc4-mm2/drivers/usb/core/hub.c
@@ -623,7 +623,6 @@ static void mark_children_for_reset_resu
 
 		if (child) {
 			child->reset_resume = 1;
-			child->dev.power.prev_state.event = PM_EVENT_ON;
 			clear_port_feature(hdev, port1,
 					USB_PORT_FEAT_C_CONNECTION);
 		}
@@ -1190,10 +1189,6 @@ void usb_root_hub_lost_power(struct usb_
 	dev_warn(&rhdev->dev, "root hub lost power or was reset\n");
 	rhdev->reset_resume = 1;
 
-	/* Force the root hub to be resumed, even if it was suspended,
-	 * so that no wakeup events will get lost.
-	 */
-	rhdev->dev.power.prev_state.event = PM_EVENT_ON;
 }
 EXPORT_SYMBOL_GPL(usb_root_hub_lost_power);
 
Index: linux-2.6.22-rc4-mm2/include/linux/pm.h
===================================================================
--- linux-2.6.22-rc4-mm2.orig/include/linux/pm.h
+++ linux-2.6.22-rc4-mm2/include/linux/pm.h
@@ -235,7 +235,6 @@ struct dev_pm_info {
 	unsigned		can_wakeup:1;
 #ifdef	CONFIG_PM
 	unsigned		should_wakeup:1;
-	pm_message_t		prev_state;
 	struct list_head	entry;
 #endif
 };
Index: linux-2.6.22-rc4-mm2/drivers/usb/host/ohci-pci.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/usb/host/ohci-pci.c
+++ linux-2.6.22-rc4-mm2/drivers/usb/host/ohci-pci.c
@@ -281,8 +281,6 @@ static int ohci_pci_resume (struct usb_h
 	/* FIXME: we should try to detect loss of VBUS power here */
 	prepare_for_handover(hcd);
 
-	/* Force the PM core to resume the root hub */
-	hcd_to_bus(hcd)->root_hub->dev.power.prev_state.event = PM_EVENT_ON;
 	return 0;
 }
 

[PATCH -mm 6/7] PM: Remove power_state.event checks from suspend core code

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The suspend routines should be called for every device during a system sleep
transition, regardless of the device's state, so that drivers can regard these
method calls as notifications that the system is about to go to sleep, rather
than as directives to put their devices into the 'off' state.

This is documented in Documentation/power/devices.txt and is already done in
the core resume code, so it seems reasonable to make the core suspend code
behave accordingly.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/suspend.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Index: linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/base/power/suspend.c
+++ linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
@@ -71,14 +71,13 @@ int suspend_device(struct device * dev, 
 			dev->parent->power.power_state.event);
 	}
 
-	if (dev->class && dev->class->suspend && !dev->power.power_state.event) {
+	if (dev->class && dev->class->suspend) {
 		suspend_device_dbg(dev, state, "class ");
 		error = dev->class->suspend(dev, state);
 		suspend_report_result(dev->class->suspend, error);
 	}
 
-	if (!error && dev->bus && dev->bus->suspend
-	    && !dev->power.power_state.event) {
+	if (!error && dev->bus && dev->bus->suspend) {
 		suspend_device_dbg(dev, state, "");
 		error = dev->bus->suspend(dev, state);
 		suspend_report_result(dev->bus->suspend, error);
@@ -97,8 +96,7 @@ static int suspend_device_late(struct de
 {
 	int error = 0;
 
-	if (dev->bus && dev->bus->suspend_late
-	    && !dev->power.power_state.event) {
+	if (dev->bus && dev->bus->suspend_late) {
 		suspend_device_dbg(dev, state, "LATE ");
 		error = dev->bus->suspend_late(dev, state);
 		suspend_report_result(dev->bus->suspend_late, error);

Re: [PATCH -mm 6/7] PM: Remove power_state.event checks from suspend core code

[David Brownell]

On Wednesday 13 June 2007, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The suspend routines should be called for every device during a system sleep
> transition, regardless of the device's state, so that drivers can regard these
> method calls as notifications that the system is about to go to sleep, rather
> than as directives to put their devices into the 'off' state.

Did you audit all the drivers to make sure this won't break things?
Like for example through inappropriate pci_save_state() calls?

I'd really expect this patch would break things...

Re: [PATCH -mm 6/7] PM: Remove power_state.event checks from suspend core code

[Rafael J. Wysocki]

On Thursday, 14 June 2007 16:21, David Brownell wrote:
> On Wednesday 13 June 2007, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > The suspend routines should be called for every device during a system sleep
> > transition, regardless of the device's state, so that drivers can regard these
> > method calls as notifications that the system is about to go to sleep, rather
> > than as directives to put their devices into the 'off' state.
> 
> Did you audit all the drivers to make sure this won't break things?
> Like for example through inappropriate pci_save_state() calls?

I did, but not very carefully.

> I'd really expect this patch would break things...

Well, in that case I'll have a closer look at them.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH -mm 6/7] PM: Remove power_state.event checks from suspend core code

[Alan Stern]

On Fri, 15 Jun 2007, Rafael J. Wysocki wrote:

> On Thursday, 14 June 2007 16:21, David Brownell wrote:
> > On Wednesday 13 June 2007, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > 
> > > The suspend routines should be called for every device during a system sleep
> > > transition, regardless of the device's state, so that drivers can regard these
> > > method calls as notifications that the system is about to go to sleep, rather
> > > than as directives to put their devices into the 'off' state.
> > 
> > Did you audit all the drivers to make sure this won't break things?
> > Like for example through inappropriate pci_save_state() calls?
> 
> I did, but not very carefully.
> 
> > I'd really expect this patch would break things...
> 
> Well, in that case I'll have a closer look at them.

It might not be all that bad.  One would expect problems to occur only 
in cases where devices were already suspended at the time of a system 
sleep transition.  Since relatively few drivers currently implement 
runtime PM -- and those that do are likely to be more careful about 
not blindly making state changes -- there might not be too much 
trouble.

Alan Stern

Re: [PATCH -mm 6/7] PM: Remove power_state.event checks from suspend core code

[Rafael J. Wysocki]

On Friday, 15 June 2007 04:00, Alan Stern wrote:
> On Fri, 15 Jun 2007, Rafael J. Wysocki wrote:
> 
> > On Thursday, 14 June 2007 16:21, David Brownell wrote:
> > > On Wednesday 13 June 2007, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > > 
> > > > The suspend routines should be called for every device during a system sleep
> > > > transition, regardless of the device's state, so that drivers can regard these
> > > > method calls as notifications that the system is about to go to sleep, rather
> > > > than as directives to put their devices into the 'off' state.
> > > 
> > > Did you audit all the drivers to make sure this won't break things?
> > > Like for example through inappropriate pci_save_state() calls?
> > 
> > I did, but not very carefully.
> > 
> > > I'd really expect this patch would break things...
> > 
> > Well, in that case I'll have a closer look at them.
> 
> It might not be all that bad.  One would expect problems to occur only 
> in cases where devices were already suspended at the time of a system 
> sleep transition.  Since relatively few drivers currently implement 
> runtime PM -- and those that do are likely to be more careful about 
> not blindly making state changes -- there might not be too much 
> trouble.

Yes, in fact I've had no problems related to that so far (tested the patch on
four different machines).

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH -mm 6/7] PM: Remove power_state.event checks from suspend core code

[Rafael J. Wysocki]

On Friday, 15 June 2007 23:57, Rafael J. Wysocki wrote:
> On Friday, 15 June 2007 04:00, Alan Stern wrote:
> > On Fri, 15 Jun 2007, Rafael J. Wysocki wrote:
> > 
> > > On Thursday, 14 June 2007 16:21, David Brownell wrote:
> > > > On Wednesday 13 June 2007, Rafael J. Wysocki wrote:
> > > > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > > > 
> > > > > The suspend routines should be called for every device during a system sleep
> > > > > transition, regardless of the device's state, so that drivers can regard these
> > > > > method calls as notifications that the system is about to go to sleep, rather
> > > > > than as directives to put their devices into the 'off' state.
> > > > 
> > > > Did you audit all the drivers to make sure this won't break things?
> > > > Like for example through inappropriate pci_save_state() calls?
> > > 
> > > I did, but not very carefully.
> > > 
> > > > I'd really expect this patch would break things...
> > > 
> > > Well, in that case I'll have a closer look at them.
> > 
> > It might not be all that bad.  One would expect problems to occur only 
> > in cases where devices were already suspended at the time of a system 
> > sleep transition.  Since relatively few drivers currently implement 
> > runtime PM -- and those that do are likely to be more careful about 
> > not blindly making state changes -- there might not be too much 
> > trouble.
> 
> Yes, in fact I've had no problems related to that so far (tested the patch on
> four different machines).

It seems the drivers for which that could be relevant do the checks as needed.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

[PATCH -mm 7/7] PM: Do not check parent state in suspend and resume core code

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The checks if the device's parent is in the right state done in
drivers/base/power/suspend.c and drivers/base/power/resume.c serve no particular
purpose, since if the parent is in a wrong power state, the device's suspend or
resume callbacks are supposed to return an error anyway.  Moreover, they are
also useless from the sanity checking point of view, because they rely on the
code being checked to set dev->parent->power.power_state.event appropriately,
which need not happen if that code is buggy.  For these reasons they can be
removed.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: David Brownell <dbrownell@users.sourceforge.net>
---
 drivers/base/power/resume.c  |    7 -------
 drivers/base/power/suspend.c |    7 -------
 2 files changed, 14 deletions(-)

Index: linux-2.6.22-rc4-mm2/drivers/base/power/resume.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/base/power/resume.c
+++ linux-2.6.22-rc4-mm2/drivers/base/power/resume.c
@@ -29,13 +29,6 @@ int resume_device(struct device * dev)
 
 	down(&dev->sem);
 
-	if (dev->parent && dev->parent->power.power_state.event) {
-		dev_err(dev, "PM: resume from %d, parent %s still %d\n",
-			dev->power.power_state.event,
-			dev->parent->bus_id,
-			dev->parent->power.power_state.event);
-	}
-
 	if (dev->bus && dev->bus->resume) {
 		dev_dbg(dev,"resuming\n");
 		error = dev->bus->resume(dev);
Index: linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
===================================================================
--- linux-2.6.22-rc4-mm2.orig/drivers/base/power/suspend.c
+++ linux-2.6.22-rc4-mm2/drivers/base/power/suspend.c
@@ -63,13 +63,6 @@ int suspend_device(struct device * dev, 
 		dev_dbg(dev, "PM: suspend %d-->%d\n",
 			dev->power.power_state.event, state.event);
 	}
-	if (dev->parent && dev->parent->power.power_state.event) {
-		dev_err(dev,
-			"PM: suspend %d->%d, parent %s already %d\n",
-			dev->power.power_state.event, state.event,
-			dev->parent->bus_id,
-			dev->parent->power.power_state.event);
-	}
 
 	if (dev->class && dev->class->suspend) {
 		suspend_device_dbg(dev, state, "class ");

Re: [PATCH -mm 0/7] PM: Remove unused and unnecessary features from suspend and resume core

[Greg KH]

On Wed, Jun 13, 2007 at 03:51:48PM +0200, Rafael J. Wysocki wrote:
> Hi,
> 
> The following series of patches removes some unused and unnecessary features
> from the suspend and resume core code.

These are the same as you sent out the other day, right?

If so, I'll put them in my tree, trying to catch up on my queue right
now.

thanks,

greg k-h

Re: [PATCH -mm 0/7] PM: Remove unused and unnecessary features from suspend and resume core

[Rafael J. Wysocki]

On Wednesday, 13 June 2007 23:59, Greg KH wrote:
> On Wed, Jun 13, 2007 at 03:51:48PM +0200, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > The following series of patches removes some unused and unnecessary features
> > from the suspend and resume core code.
> 
> These are the same as you sent out the other day, right?

Almost.

The "PM: Remove pm_parent from struct dev_pm_info" (1/7) has been updated.

The "PM: Remove prev_state from struct dev_pm_info" patch (5/7) has been
rediffed against 2.6.22-rc4-mm2 (to synchronize with the USB changes in there).

The others are the same.

> If so, I'll put them in my tree, trying to catch up on my queue right
> now.

OK

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH -mm 0/7] PM: Remove unused and unnecessary features from suspend and resume core

[Pavel Machek]

Hi!

> The following series of patches removes some unused and unnecessary features
> from the suspend and resume core code.

Hmm, patches 1-4 are pretty much obvious cleanups, I guess they can go
in.

Patches 5+ change semantics of suspend/resume callbacks in subtle way,
I guess they need to stay in -mm for a while...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html