public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
* sysdev suspend/resume
@ 2005-07-26  5:42 Shaohua Li
  2005-07-26 10:14 ` Pavel Machek
  0 siblings, 1 reply; 9+ messages in thread
From: Shaohua Li @ 2005-07-26  5:42 UTC (permalink / raw)
  To: linux-pm

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

Hi,
The return value of sysdev's suspend/resume methods is ignored, is this
intended (it should not fail?) or just a bug? I'd like to abort the
suspend process if one device fails.

Thanks,
Shaohua


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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: sysdev suspend/resume
  2005-07-26  5:42 sysdev suspend/resume Shaohua Li
@ 2005-07-26 10:14 ` Pavel Machek
  2005-07-27  8:50   ` Shaohua Li
  0 siblings, 1 reply; 9+ messages in thread
From: Pavel Machek @ 2005-07-26 10:14 UTC (permalink / raw)
  To: Shaohua Li; +Cc: linux-pm

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

Hi!

> The return value of sysdev's suspend/resume methods is ignored, is this
> intended (it should not fail?) or just a bug? I'd like to abort the
> suspend process if one device fails.

I guess patch would be accepted... but please make sure you test those
error paths, and printk name of failing driver.

Hmmm, I wonder how you are going to handle resume failure? AFAICS, there's
no sane way to handle that.
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         


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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: sysdev suspend/resume
  2005-07-26 10:14 ` Pavel Machek
@ 2005-07-27  8:50   ` Shaohua Li
  2005-07-27  9:07     ` Pavel Machek
  0 siblings, 1 reply; 9+ messages in thread
From: Shaohua Li @ 2005-07-27  8:50 UTC (permalink / raw)
  To: Pavel Machek; +Cc: linux-pm

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

On Tue, 2005-07-26 at 12:14 +0200, Pavel Machek wrote:
> Hi!
> 
> > The return value of sysdev's suspend/resume methods is ignored, is this
> > intended (it should not fail?) or just a bug? I'd like to abort the
> > suspend process if one device fails.
> 
> I guess patch would be accepted... but please make sure you test those
> error paths, and printk name of failing driver.
It's a challenge to me, so many loops :). Below patch is a little ugly,
but should cover all error paths.

> Hmmm, I wonder how you are going to handle resume failure? AFAICS, there's
> no sane way to handle that.
We can't refuse to resume :).

Thanks,
Shaohua

Signed-off-by: Shaohua Li<shaohua.li@intel.com>
---

 linux-2.6.13-rc3-root/drivers/base/sys.c |  104 +++++++++++++++++++++++--------
 1 files changed, 80 insertions(+), 24 deletions(-)

diff -puN drivers/base/sys.c~sysdev drivers/base/sys.c
--- linux-2.6.13-rc3/drivers/base/sys.c~sysdev	2005-07-27 10:49:52.831538832 +0800
+++ linux-2.6.13-rc3-root/drivers/base/sys.c	2005-07-27 16:25:49.154312008 +0800
@@ -288,6 +288,22 @@ void sysdev_shutdown(void)
 	up(&sysdev_drivers_lock);
 }
 
+#define SYSDEV_RESUME(cls, dev, drv)			\
+	/* First, call the class-specific one */ 	\
+	if (cls->resume)				\
+		cls->resume(dev);			\
+							\
+	/* Call auxillary drivers next. */ 		\
+	list_for_each_entry(drv, &cls->drivers, entry) {\
+		if (drv->resume) 			\
+			drv->resume(dev); 		\
+	}						\
+							\
+	/* Call global drivers. */			\
+	list_for_each_entry(drv, &sysdev_drivers, entry) {\
+		if (drv->resume)			\
+			drv->resume(dev);		\
+	}
 
 /**
  *	sysdev_suspend - Suspend all system devices.
@@ -305,38 +321,93 @@ void sysdev_shutdown(void)
 int sysdev_suspend(pm_message_t state)
 {
 	struct sysdev_class * cls;
+	struct sys_device *sysdev, *err_dev;
+	struct sysdev_driver *drv, *err_drv;
+	int	ret;
 
 	pr_debug("Suspending System Devices\n");
 
 	list_for_each_entry_reverse(cls, &system_subsys.kset.list,
 				    kset.kobj.entry) {
-		struct sys_device * sysdev;
 
 		pr_debug("Suspending type '%s':\n",
 			 kobject_name(&cls->kset.kobj));
 
 		list_for_each_entry(sysdev, &cls->kset.list, kobj.entry) {
-			struct sysdev_driver * drv;
 			pr_debug(" %s\n", kobject_name(&sysdev->kobj));
 
 			/* Call global drivers first. */
 			list_for_each_entry(drv, &sysdev_drivers, entry) {
-				if (drv->suspend)
-					drv->suspend(sysdev, state);
+				if (drv->suspend) {
+					ret = drv->suspend(sysdev, state);
+					if (ret)
+						goto gbl_driver;
+				}
 			}
 
 			/* Call auxillary drivers next. */
 			list_for_each_entry(drv, &cls->drivers, entry) {
-				if (drv->suspend)
-					drv->suspend(sysdev, state);
+				if (drv->suspend) {
+					ret = drv->suspend(sysdev, state);
+					if (ret)
+						goto aux_driver;
+				}
 			}
 
 			/* Now call the generic one */
-			if (cls->suspend)
-				cls->suspend(sysdev, state);
+			if (cls->suspend) {
+				ret = cls->suspend(sysdev, state);
+				if (ret)
+					goto cls_driver;
+			}
 		}
 	}
 	return 0;
+	/* resume current sysdev */
+cls_driver:
+	drv = NULL;
+	printk(KERN_ERR "Clss suspend failed for %s\n",
+		kobject_name(&sysdev->kobj));
+
+aux_driver:
+	if (drv)
+		printk(KERN_ERR "Class driver suspend failed for %s\n",
+				kobject_name(&sysdev->kobj));
+	list_for_each_entry(err_drv, &cls->drivers, entry) {
+		if (err_drv == drv)
+			break;
+		if (err_drv->resume)
+			err_drv->resume(sysdev);
+	}
+	drv = NULL;
+
+gbl_driver:
+	if (drv)
+		printk(KERN_ERR "sysdev driver suspend failed for %s\n",
+				kobject_name(&sysdev->kobj));
+	list_for_each_entry(err_drv, &sysdev_drivers, entry) {
+		if (err_drv == drv)
+			break;
+		if (err_drv->resume)
+			err_drv->resume(sysdev);
+	}
+	/* resume other sysdevs in current class */
+	list_for_each_entry(err_dev, &cls->kset.list, kobj.entry) {
+		if (err_dev == sysdev)
+			break;
+		pr_debug(" %s\n", kobject_name(&err_dev->kobj));
+		SYSDEV_RESUME(cls, err_dev, err_drv);
+	}
+
+	/* resume other classes */
+	list_for_each_entry_continue(cls, &system_subsys.kset.list,
+					kset.kobj.entry) {
+		list_for_each_entry(err_dev, &cls->kset.list, kobj.entry) {
+			pr_debug(" %s\n", kobject_name(&err_dev->kobj));
+			SYSDEV_RESUME(cls, err_dev, err_drv);
+		}
+	}
+	return ret;
 }
 
 
@@ -365,22 +436,7 @@ int sysdev_resume(void)
 			struct sysdev_driver * drv;
 			pr_debug(" %s\n", kobject_name(&sysdev->kobj));
 
-			/* First, call the class-specific one */
-			if (cls->resume)
-				cls->resume(sysdev);
-
-			/* Call auxillary drivers next. */
-			list_for_each_entry(drv, &cls->drivers, entry) {
-				if (drv->resume)
-					drv->resume(sysdev);
-			}
-
-			/* Call global drivers. */
-			list_for_each_entry(drv, &sysdev_drivers, entry) {
-				if (drv->resume)
-					drv->resume(sysdev);
-			}
-
+			SYSDEV_RESUME(cls, sysdev, drv);
 		}
 	}
 	return 0;
_



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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: sysdev suspend/resume
  2005-07-27  8:50   ` Shaohua Li
@ 2005-07-27  9:07     ` Pavel Machek
  2005-07-27  9:30       ` Shaohua Li
  0 siblings, 1 reply; 9+ messages in thread
From: Pavel Machek @ 2005-07-27  9:07 UTC (permalink / raw)
  To: Shaohua Li; +Cc: linux-pm

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

Hi!

> > > The return value of sysdev's suspend/resume methods is ignored, is this
> > > intended (it should not fail?) or just a bug? I'd like to abort the
> > > suspend process if one device fails.
> > 
> > I guess patch would be accepted... but please make sure you test those
> > error paths, and printk name of failing driver.
> It's a challenge to me, so many loops :). Below patch is a little ugly,
> but should cover all error paths.

It is indeed quite ugly. Could we get SYSDEV_RESUME inline function
instead of ugly macro?

> diff -puN drivers/base/sys.c~sysdev drivers/base/sys.c
> --- linux-2.6.13-rc3/drivers/base/sys.c~sysdev	2005-07-27 10:49:52.831538832 +0800
> +++ linux-2.6.13-rc3-root/drivers/base/sys.c	2005-07-27 16:25:49.154312008 +0800
> @@ -288,6 +288,22 @@ void sysdev_shutdown(void)
>  	up(&sysdev_drivers_lock);
>  }
>  
> +#define SYSDEV_RESUME(cls, dev, drv)			\
> +	/* First, call the class-specific one */ 	\
> +	if (cls->resume)				\
> +		cls->resume(dev);			\
> +							\
> +	/* Call auxillary drivers next. */ 		\
> +	list_for_each_entry(drv, &cls->drivers, entry) {\
> +		if (drv->resume) 			\
> +			drv->resume(dev); 		\
> +	}						\
> +							\
> +	/* Call global drivers. */			\
> +	list_for_each_entry(drv, &sysdev_drivers, entry) {\
> +		if (drv->resume)			\
> +			drv->resume(dev);		\
> +	}

Are you sure you are resuming already-resumed devices?

>  /**
>   *	sysdev_suspend - Suspend all system devices.
> @@ -305,38 +321,93 @@ void sysdev_shutdown(void)
>  int sysdev_suspend(pm_message_t state)
>  {
>  	struct sysdev_class * cls;
> +	struct sys_device *sysdev, *err_dev;
> +	struct sysdev_driver *drv, *err_drv;
> +	int	ret;
	~~~~~~~~~~~~
	use space not tab here. I see you got that habit from ACPI,
but it is just wrong.

> +cls_driver:
> +	drv = NULL;
> +	printk(KERN_ERR "Clss suspend failed for %s\n",
			 ~~~~

Mssng vwls.

								Pavel
-- 
teflon -- maybe it is a trademark, but it should not be.

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: sysdev suspend/resume
  2005-07-27  9:07     ` Pavel Machek
@ 2005-07-27  9:30       ` Shaohua Li
  2005-07-27  9:40         ` Pavel Machek
  2005-07-27 17:43         ` Patrick Mochel
  0 siblings, 2 replies; 9+ messages in thread
From: Shaohua Li @ 2005-07-27  9:30 UTC (permalink / raw)
  To: Pavel Machek; +Cc: linux-pm

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

Hi,
On Wed, 2005-07-27 at 11:07 +0200, Pavel Machek wrote:
> > > > The return value of sysdev's suspend/resume methods is ignored, is this
> > > > intended (it should not fail?) or just a bug? I'd like to abort the
> > > > suspend process if one device fails.
> > > 
> > > I guess patch would be accepted... but please make sure you test those
> > > error paths, and printk name of failing driver.
> > It's a challenge to me, so many loops :). Below patch is a little ugly,
> > but should cover all error paths.
> 
> It is indeed quite ugly. Could we get SYSDEV_RESUME inline function
> instead of ugly macro?
It make senses. Thanks for your comments.

> 
> > diff -puN drivers/base/sys.c~sysdev drivers/base/sys.c
> > --- linux-2.6.13-rc3/drivers/base/sys.c~sysdev	2005-07-27 10:49:52.831538832 +0800
> > +++ linux-2.6.13-rc3-root/drivers/base/sys.c	2005-07-27 16:25:49.154312008 +0800
> > @@ -288,6 +288,22 @@ void sysdev_shutdown(void)
> >  	up(&sysdev_drivers_lock);
> >  }
> >  
> > +#define SYSDEV_RESUME(cls, dev, drv)			\
> > +	/* First, call the class-specific one */ 	\
> > +	if (cls->resume)				\
> > +		cls->resume(dev);			\
> > +							\
> > +	/* Call auxillary drivers next. */ 		\
> > +	list_for_each_entry(drv, &cls->drivers, entry) {\
> > +		if (drv->resume) 			\
> > +			drv->resume(dev); 		\
> > +	}						\
> > +							\
> > +	/* Call global drivers. */			\
> > +	list_for_each_entry(drv, &sysdev_drivers, entry) {\
> > +		if (drv->resume)			\
> > +			drv->resume(dev);		\
> > +	}
> 
> Are you sure you are resuming already-resumed devices?
What's your point? if sysdev_suspend failed, sysdev_resume isn't called,
so it will not be resumed twice.

Thanks,
Shaohua


Signed-off-by: Shaohua Li<shaohua.li@intel.com>

---

 linux-2.6.13-rc3-root/drivers/base/sys.c |  107 ++++++++++++++++++++++++-------
 1 files changed, 83 insertions(+), 24 deletions(-)

diff -puN drivers/base/sys.c~sysdev drivers/base/sys.c
--- linux-2.6.13-rc3/drivers/base/sys.c~sysdev	2005-07-27 10:49:52.831538832 +0800
+++ linux-2.6.13-rc3-root/drivers/base/sys.c	2005-07-27 17:22:57.447132280 +0800
@@ -288,6 +288,25 @@ void sysdev_shutdown(void)
 	up(&sysdev_drivers_lock);
 }
 
+static void inline do_sysdev_resume(struct sysdev_class *cls,
+	struct sys_device *dev, struct sysdev_driver *drv)
+{
+	/* First, call the class-specific one */
+	if (cls->resume)
+		cls->resume(dev);
+
+	/* Call auxillary drivers next. */
+	list_for_each_entry(drv, &cls->drivers, entry) {
+		if (drv->resume)
+			drv->resume(dev);
+	}
+
+	/* Call global drivers. */
+	list_for_each_entry(drv, &sysdev_drivers, entry) {
+		if (drv->resume)
+			drv->resume(dev);
+	}
+}
 
 /**
  *	sysdev_suspend - Suspend all system devices.
@@ -305,38 +324,93 @@ void sysdev_shutdown(void)
 int sysdev_suspend(pm_message_t state)
 {
 	struct sysdev_class * cls;
+	struct sys_device *sysdev, *err_dev;
+	struct sysdev_driver *drv, *err_drv;
+	int ret;
 
 	pr_debug("Suspending System Devices\n");
 
 	list_for_each_entry_reverse(cls, &system_subsys.kset.list,
 				    kset.kobj.entry) {
-		struct sys_device * sysdev;
 
 		pr_debug("Suspending type '%s':\n",
 			 kobject_name(&cls->kset.kobj));
 
 		list_for_each_entry(sysdev, &cls->kset.list, kobj.entry) {
-			struct sysdev_driver * drv;
 			pr_debug(" %s\n", kobject_name(&sysdev->kobj));
 
 			/* Call global drivers first. */
 			list_for_each_entry(drv, &sysdev_drivers, entry) {
-				if (drv->suspend)
-					drv->suspend(sysdev, state);
+				if (drv->suspend) {
+					ret = drv->suspend(sysdev, state);
+					if (ret)
+						goto gbl_driver;
+				}
 			}
 
 			/* Call auxillary drivers next. */
 			list_for_each_entry(drv, &cls->drivers, entry) {
-				if (drv->suspend)
-					drv->suspend(sysdev, state);
+				if (drv->suspend) {
+					ret = drv->suspend(sysdev, state);
+					if (ret)
+						goto aux_driver;
+				}
 			}
 
 			/* Now call the generic one */
-			if (cls->suspend)
-				cls->suspend(sysdev, state);
+			if (cls->suspend) {
+				ret = cls->suspend(sysdev, state);
+				if (ret)
+					goto cls_driver;
+			}
 		}
 	}
 	return 0;
+	/* resume current sysdev */
+cls_driver:
+	drv = NULL;
+	printk(KERN_ERR "Class suspend failed for %s\n",
+		kobject_name(&sysdev->kobj));
+
+aux_driver:
+	if (drv)
+		printk(KERN_ERR "Class driver suspend failed for %s\n",
+				kobject_name(&sysdev->kobj));
+	list_for_each_entry(err_drv, &cls->drivers, entry) {
+		if (err_drv == drv)
+			break;
+		if (err_drv->resume)
+			err_drv->resume(sysdev);
+	}
+	drv = NULL;
+
+gbl_driver:
+	if (drv)
+		printk(KERN_ERR "sysdev driver suspend failed for %s\n",
+				kobject_name(&sysdev->kobj));
+	list_for_each_entry(err_drv, &sysdev_drivers, entry) {
+		if (err_drv == drv)
+			break;
+		if (err_drv->resume)
+			err_drv->resume(sysdev);
+	}
+	/* resume other sysdevs in current class */
+	list_for_each_entry(err_dev, &cls->kset.list, kobj.entry) {
+		if (err_dev == sysdev)
+			break;
+		pr_debug(" %s\n", kobject_name(&err_dev->kobj));
+		do_sysdev_resume(cls, err_dev, err_drv);
+	}
+
+	/* resume other classes */
+	list_for_each_entry_continue(cls, &system_subsys.kset.list,
+					kset.kobj.entry) {
+		list_for_each_entry(err_dev, &cls->kset.list, kobj.entry) {
+			pr_debug(" %s\n", kobject_name(&err_dev->kobj));
+			do_sysdev_resume(cls, err_dev, err_drv);
+		}
+	}
+	return ret;
 }
 
 
@@ -365,22 +439,7 @@ int sysdev_resume(void)
 			struct sysdev_driver * drv;
 			pr_debug(" %s\n", kobject_name(&sysdev->kobj));
 
-			/* First, call the class-specific one */
-			if (cls->resume)
-				cls->resume(sysdev);
-
-			/* Call auxillary drivers next. */
-			list_for_each_entry(drv, &cls->drivers, entry) {
-				if (drv->resume)
-					drv->resume(sysdev);
-			}
-
-			/* Call global drivers. */
-			list_for_each_entry(drv, &sysdev_drivers, entry) {
-				if (drv->resume)
-					drv->resume(sysdev);
-			}
-
+			do_sysdev_resume(cls, sysdev, drv);
 		}
 	}
 	return 0;
_



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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: sysdev suspend/resume
  2005-07-27  9:30       ` Shaohua Li
@ 2005-07-27  9:40         ` Pavel Machek
  2005-07-27 17:43         ` Patrick Mochel
  1 sibling, 0 replies; 9+ messages in thread
From: Pavel Machek @ 2005-07-27  9:40 UTC (permalink / raw)
  To: Shaohua Li; +Cc: linux-pm

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

Hi!

> > > +#define SYSDEV_RESUME(cls, dev, drv)			\
> > > +	/* First, call the class-specific one */ 	\
> > > +	if (cls->resume)				\
> > > +		cls->resume(dev);			\
> > > +							\
> > > +	/* Call auxillary drivers next. */ 		\
> > > +	list_for_each_entry(drv, &cls->drivers, entry) {\
> > > +		if (drv->resume) 			\
> > > +			drv->resume(dev); 		\
> > > +	}						\
> > > +							\
> > > +	/* Call global drivers. */			\
> > > +	list_for_each_entry(drv, &sysdev_drivers, entry) {\
> > > +		if (drv->resume)			\
> > > +			drv->resume(dev);		\
> > > +	}
> > 
> > Are you sure you are resuming already-resumed devices?
> What's your point? if sysdev_suspend failed, sysdev_resume isn't called,
> so it will not be resumed twice.

Probably nothing then, sorry for noise.
									Pavel

-- 
teflon -- maybe it is a trademark, but it should not be.

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: sysdev suspend/resume
  2005-07-27  9:30       ` Shaohua Li
  2005-07-27  9:40         ` Pavel Machek
@ 2005-07-27 17:43         ` Patrick Mochel
  2005-07-27 18:50           ` Patrick Mochel
  2005-07-28  3:10           ` Shaohua Li
  1 sibling, 2 replies; 9+ messages in thread
From: Patrick Mochel @ 2005-07-27 17:43 UTC (permalink / raw)
  To: Shaohua Li; +Cc: linux-pm, Pavel Machek

[-- Attachment #1: Type: TEXT/PLAIN, Size: 921 bytes --]


On Wed, 27 Jul 2005, Shaohua Li wrote:

The patch looks Ok, except for this function.

> diff -puN drivers/base/sys.c~sysdev drivers/base/sys.c
> --- linux-2.6.13-rc3/drivers/base/sys.c~sysdev	2005-07-27 10:49:52.831538832 +0800
> +++ linux-2.6.13-rc3-root/drivers/base/sys.c	2005-07-27 17:22:57.447132280 +0800
> @@ -288,6 +288,25 @@ void sysdev_shutdown(void)
>  	up(&sysdev_drivers_lock);
>  }
>
> +static void inline do_sysdev_resume(struct sysdev_class *cls,
> +	struct sys_device *dev, struct sysdev_driver *drv)
> +{


* It does not need to be inlined.

* Internal helpers are prefix'd with "__" instead of "do_", so this would
  be '__sysdev_resume()'.

* drv is a temporary variable

* cls can be ascertained from the sys_device


So, the first few lines should be

static void __sysdev_resume(struct sys_device * dev)
{
	struct sysdev_class * cls = dev->cls;
	struct sysdev_driver * dev;
	...


Thanks,


	Pat

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: sysdev suspend/resume
  2005-07-27 17:43         ` Patrick Mochel
@ 2005-07-27 18:50           ` Patrick Mochel
  2005-07-28  3:10           ` Shaohua Li
  1 sibling, 0 replies; 9+ messages in thread
From: Patrick Mochel @ 2005-07-27 18:50 UTC (permalink / raw)
  To: Shaohua Li; +Cc: linux-pm, Pavel Machek

[-- Attachment #1: Type: TEXT/PLAIN, Size: 340 bytes --]


One small correction that one eagle-eyed participant noticed:

On Wed, 27 Jul 2005, Patrick Mochel wrote:

> So, the first few lines should be
>
> static void __sysdev_resume(struct sys_device * dev)
> {
> 	struct sysdev_class * cls = dev->cls;
> 	struct sysdev_driver * dev;

That should be:

	struct sysdev_driver * drv;

Thanks,


	Pat

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: sysdev suspend/resume
  2005-07-27 17:43         ` Patrick Mochel
  2005-07-27 18:50           ` Patrick Mochel
@ 2005-07-28  3:10           ` Shaohua Li
  1 sibling, 0 replies; 9+ messages in thread
From: Shaohua Li @ 2005-07-28  3:10 UTC (permalink / raw)
  To: Patrick Mochel; +Cc: linux-pm, Pavel Machek

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

Hi,
On Wed, 2005-07-27 at 10:43 -0700, Patrick Mochel wrote:
> > +static void inline do_sysdev_resume(struct sysdev_class *cls,
> > +	struct sys_device *dev, struct sysdev_driver *drv)
> > +{
> 
> 
> * It does not need to be inlined.
> 
> * Internal helpers are prefix'd with "__" instead of "do_", so this would
>   be '__sysdev_resume()'.
> 
> * drv is a temporary variable
> 
> * cls can be ascertained from the sys_device
> 
> 
> So, the first few lines should be
> 
> static void __sysdev_resume(struct sys_device * dev)
> {
> 	struct sysdev_class * cls = dev->cls;
> 	struct sysdev_driver * dev;
> 	...
Thanks. A updated one.


Signed-off-by: Shaohua Li<shaohua.li@intel.com>
---

 linux-2.6.13-rc3-root/drivers/base/sys.c |  110 +++++++++++++++++++++++--------
 1 files changed, 85 insertions(+), 25 deletions(-)

diff -puN drivers/base/sys.c~sysdev drivers/base/sys.c
--- linux-2.6.13-rc3/drivers/base/sys.c~sysdev	2005-07-27 10:49:52.000000000 +0800
+++ linux-2.6.13-rc3-root/drivers/base/sys.c	2005-07-28 10:56:10.373347264 +0800
@@ -288,6 +288,27 @@ void sysdev_shutdown(void)
 	up(&sysdev_drivers_lock);
 }
 
+static void __sysdev_resume(struct sys_device *dev)
+{
+	struct sysdev_class *cls = dev->cls;
+	struct sysdev_driver *drv;
+
+	/* First, call the class-specific one */
+	if (cls->resume)
+		cls->resume(dev);
+
+	/* Call auxillary drivers next. */
+	list_for_each_entry(drv, &cls->drivers, entry) {
+		if (drv->resume)
+			drv->resume(dev);
+	}
+
+	/* Call global drivers. */
+	list_for_each_entry(drv, &sysdev_drivers, entry) {
+		if (drv->resume)
+			drv->resume(dev);
+	}
+}
 
 /**
  *	sysdev_suspend - Suspend all system devices.
@@ -305,38 +326,93 @@ void sysdev_shutdown(void)
 int sysdev_suspend(pm_message_t state)
 {
 	struct sysdev_class * cls;
+	struct sys_device *sysdev, *err_dev;
+	struct sysdev_driver *drv, *err_drv;
+	int ret;
 
 	pr_debug("Suspending System Devices\n");
 
 	list_for_each_entry_reverse(cls, &system_subsys.kset.list,
 				    kset.kobj.entry) {
-		struct sys_device * sysdev;
 
 		pr_debug("Suspending type '%s':\n",
 			 kobject_name(&cls->kset.kobj));
 
 		list_for_each_entry(sysdev, &cls->kset.list, kobj.entry) {
-			struct sysdev_driver * drv;
 			pr_debug(" %s\n", kobject_name(&sysdev->kobj));
 
 			/* Call global drivers first. */
 			list_for_each_entry(drv, &sysdev_drivers, entry) {
-				if (drv->suspend)
-					drv->suspend(sysdev, state);
+				if (drv->suspend) {
+					ret = drv->suspend(sysdev, state);
+					if (ret)
+						goto gbl_driver;
+				}
 			}
 
 			/* Call auxillary drivers next. */
 			list_for_each_entry(drv, &cls->drivers, entry) {
-				if (drv->suspend)
-					drv->suspend(sysdev, state);
+				if (drv->suspend) {
+					ret = drv->suspend(sysdev, state);
+					if (ret)
+						goto aux_driver;
+				}
 			}
 
 			/* Now call the generic one */
-			if (cls->suspend)
-				cls->suspend(sysdev, state);
+			if (cls->suspend) {
+				ret = cls->suspend(sysdev, state);
+				if (ret)
+					goto cls_driver;
+			}
 		}
 	}
 	return 0;
+	/* resume current sysdev */
+cls_driver:
+	drv = NULL;
+	printk(KERN_ERR "Class suspend failed for %s\n",
+		kobject_name(&sysdev->kobj));
+
+aux_driver:
+	if (drv)
+		printk(KERN_ERR "Class driver suspend failed for %s\n",
+				kobject_name(&sysdev->kobj));
+	list_for_each_entry(err_drv, &cls->drivers, entry) {
+		if (err_drv == drv)
+			break;
+		if (err_drv->resume)
+			err_drv->resume(sysdev);
+	}
+	drv = NULL;
+
+gbl_driver:
+	if (drv)
+		printk(KERN_ERR "sysdev driver suspend failed for %s\n",
+				kobject_name(&sysdev->kobj));
+	list_for_each_entry(err_drv, &sysdev_drivers, entry) {
+		if (err_drv == drv)
+			break;
+		if (err_drv->resume)
+			err_drv->resume(sysdev);
+	}
+	/* resume other sysdevs in current class */
+	list_for_each_entry(err_dev, &cls->kset.list, kobj.entry) {
+		if (err_dev == sysdev)
+			break;
+		pr_debug(" %s\n", kobject_name(&err_dev->kobj));
+		__sysdev_resume(err_dev);
+	}
+
+	/* resume other classes */
+	list_for_each_entry_continue(cls, &system_subsys.kset.list,
+					kset.kobj.entry) {
+		list_for_each_entry(err_dev, &cls->kset.list, kobj.entry) {
+			pr_debug(" %s\n", kobject_name(&err_dev->kobj));
+			__sysdev_resume(err_dev);
+		}
+	}
+	return ret;
 }
 
 
@@ -362,25 +438,9 @@ int sysdev_resume(void)
 			 kobject_name(&cls->kset.kobj));
 
 		list_for_each_entry(sysdev, &cls->kset.list, kobj.entry) {
-			struct sysdev_driver * drv;
 			pr_debug(" %s\n", kobject_name(&sysdev->kobj));
 
-			/* First, call the class-specific one */
-			if (cls->resume)
-				cls->resume(sysdev);
-
-			/* Call auxillary drivers next. */
-			list_for_each_entry(drv, &cls->drivers, entry) {
-				if (drv->resume)
-					drv->resume(sysdev);
-			}
-
-			/* Call global drivers. */
-			list_for_each_entry(drv, &sysdev_drivers, entry) {
-				if (drv->resume)
-					drv->resume(sysdev);
-			}
-
+			__sysdev_resume(sysdev);
 		}
 	}
 	return 0;
_



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



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2005-07-28  3:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-07-26  5:42 sysdev suspend/resume Shaohua Li
2005-07-26 10:14 ` Pavel Machek
2005-07-27  8:50   ` Shaohua Li
2005-07-27  9:07     ` Pavel Machek
2005-07-27  9:30       ` Shaohua Li
2005-07-27  9:40         ` Pavel Machek
2005-07-27 17:43         ` Patrick Mochel
2005-07-27 18:50           ` Patrick Mochel
2005-07-28  3:10           ` Shaohua Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox