[PATCH] [PM] Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP

[PATCH] [PM] Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP

[Dasgupta, Romit]

Alan,
         Here is a patch for you. Tested with

1) CONFIG_SUSPEND, CONFIG_HIBERNATION i.e. with CONFIG_PM_SLEEP
2) without CONFIG_PM_SLEEP but with CONFIG_PM
3) without CONFIG_PM

Thanks,
-Romit


Description: Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP.
Signed-off-by: Romit Dasgupta <romit@ti.com>
diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
index a3252c0..ad8112d 100644
--- a/drivers/base/power/power.h
+++ b/drivers/base/power/power.h
@@ -1,3 +1,25 @@
+#ifdef CONFIG_PM
+
+/*
+ * sysfs.c
+ */
+
+extern int dpm_sysfs_add(struct device *);
+extern void dpm_sysfs_remove(struct device *);
+
+#else /* CONFIG_PM */
+
+static inline int dpm_sysfs_add(struct device *dev)
+{
+       return 0;
+}
+
+static inline void dpm_sysfs_remove(struct device *dev)
+{
+}
+
+#endif
+
 #ifdef CONFIG_PM_SLEEP

 /*
@@ -16,29 +38,13 @@ extern void device_pm_remove(struct device *);

 #else /* CONFIG_PM_SLEEP */

-static inline int device_pm_add(struct device *dev) { return 0; }
-static inline void device_pm_remove(struct device *dev) {}
-
-#endif
-
-#ifdef CONFIG_PM
-
-/*
- * sysfs.c
- */
-
-extern int dpm_sysfs_add(struct device *);
-extern void dpm_sysfs_remove(struct device *);
-
-#else /* CONFIG_PM */
-
-static inline int dpm_sysfs_add(struct device *dev)
+static inline int device_pm_add(struct device *dev)
 {
-       return 0;
+       return dpm_sysfs_add(dev);
 }
-
-static inline void dpm_sysfs_remove(struct device *dev)
+static inline void device_pm_remove(struct device *dev)
 {
+       dpm_sysfs_remove(dev);
 }

 #endif

>-----Original Message-----
>From: Alan Stern [mailto:stern@rowland.harvard.edu]
>Sent: Sunday, August 03, 2008 5:16 AM
>To: Dasgupta, Romit
>Cc: David Engraf; Greg KH; linux-usb@vger.kernel.org
>Subject: RE: usb_create_sysfs_dev_files in hub.c:1586 fails on system boot
>
>On Fri, 1 Aug 2008, Dasgupta, Romit wrote:
>
>> CONFIG_PM_SLEEP needs to be enabled for power to be added as a sysfs and
>that is dependent on CONFIG_SUSPEND (Suspend to RAM and standby).
>
>You're right.  I thought we had fixed that some time ago, but it's
>still wrong.  In particular, the inline definition of device_pm_add()
>in drivers/base/power/power.h is wrong; it needs to call
>dpm_sysfs_add().
>
>I can't make up a patch now; I'll do it on Monday if nobody beats me to
>it.
>
>Alan Stern

Re: [PATCH] [PM] Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP

[Alan Stern]

On Sun, 3 Aug 2008, Dasgupta, Romit wrote:

> Alan,
>          Here is a patch for you. Tested with
> 
> 1) CONFIG_SUSPEND, CONFIG_HIBERNATION i.e. with CONFIG_PM_SLEEP
> 2) without CONFIG_PM_SLEEP but with CONFIG_PM
> 3) without CONFIG_PM

I'd prefer to use the patch below.  Although it's a little longer, the 
end result is slightly less complicated.  Also, it fixes two problems:

	dpm_sysfs_add/remove should be called whenever CONFIG_PM
	is defined, regardless of CONFIG_PM_SLEEP.

	dev->power.status should be initialized to DPM_ON even if
	CONFIG_PM isn't defined.

Can you make sure it compiles properly under all three configurations?

Rafael, any objections?

Alan Stern



Index: usb-2.6/drivers/base/power/power.h
===================================================================
--- usb-2.6.orig/drivers/base/power/power.h
+++ usb-2.6/drivers/base/power/power.h
@@ -1,3 +1,8 @@
+static inline void device_pm_init(struct device *dev)
+{
+	dev->power.status = DPM_ON;
+}
+
 #ifdef CONFIG_PM_SLEEP
 
 /*
Index: usb-2.6/drivers/base/core.c
===================================================================
--- usb-2.6.orig/drivers/base/core.c
+++ usb-2.6/drivers/base/core.c
@@ -542,6 +542,7 @@ void device_initialize(struct device *de
 	spin_lock_init(&dev->devres_lock);
 	INIT_LIST_HEAD(&dev->devres_head);
 	device_init_wakeup(dev, 0);
+	device_pm_init(dev);
 	set_dev_node(dev, -1);
 }
 
@@ -898,6 +899,9 @@ int device_add(struct device *dev)
 	error = bus_add_device(dev);
 	if (error)
 		goto BusError;
+	error = dpm_sysfs_add(dev);
+	if (error)
+		goto DPMError;
 	error = device_pm_add(dev);
 	if (error)
 		goto PMError;
@@ -922,6 +926,8 @@ int device_add(struct device *dev)
 	put_device(dev);
 	return error;
  PMError:
+	dpm_sysfs_remove(dev);
+ DPMError:
 	bus_remove_device(dev);
  BusError:
 	if (dev->bus)
@@ -1008,6 +1014,7 @@ void device_del(struct device *dev)
 	struct class_interface *class_intf;
 
 	device_pm_remove(dev);
+	dpm_sysfs_remove(dev);
 	if (parent)
 		klist_del(&dev->knode_parent);
 	if (MAJOR(dev->devt)) {
Index: usb-2.6/drivers/base/power/main.c
===================================================================
--- usb-2.6.orig/drivers/base/power/main.c
+++ usb-2.6/drivers/base/power/main.c
@@ -69,7 +69,7 @@ void device_pm_unlock(void)
  */
 int device_pm_add(struct device *dev)
 {
-	int error;
+	int error = 0;
 
 	pr_debug("PM: Adding info for %s:%s\n",
 		 dev->bus ? dev->bus->name : "No Bus",
@@ -89,11 +89,8 @@ int device_pm_add(struct device *dev)
 		 */
 		WARN_ON(true);
 	}
-	error = dpm_sysfs_add(dev);
-	if (!error) {
-		dev->power.status = DPM_ON;
-		list_add_tail(&dev->power.entry, &dpm_list);
-	}
+
+	list_add_tail(&dev->power.entry, &dpm_list);
 	mutex_unlock(&dpm_list_mtx);
 	return error;
 }
@@ -110,7 +107,6 @@ void device_pm_remove(struct device *dev
 		 dev->bus ? dev->bus->name : "No Bus",
 		 kobject_name(&dev->kobj));
 	mutex_lock(&dpm_list_mtx);
-	dpm_sysfs_remove(dev);
 	list_del_init(&dev->power.entry);
 	mutex_unlock(&dpm_list_mtx);
 }

Re: [PATCH] [PM] Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP

[Rafael J. Wysocki]

On Monday, 4 of August 2008, Alan Stern wrote:
> On Sun, 3 Aug 2008, Dasgupta, Romit wrote:
> 
> > Alan,
> >          Here is a patch for you. Tested with
> > 
> > 1) CONFIG_SUSPEND, CONFIG_HIBERNATION i.e. with CONFIG_PM_SLEEP
> > 2) without CONFIG_PM_SLEEP but with CONFIG_PM
> > 3) without CONFIG_PM
> 
> I'd prefer to use the patch below.  Although it's a little longer, the 
> end result is slightly less complicated.  Also, it fixes two problems:
> 
> 	dpm_sysfs_add/remove should be called whenever CONFIG_PM
> 	is defined, regardless of CONFIG_PM_SLEEP.
> 
> 	dev->power.status should be initialized to DPM_ON even if
> 	CONFIG_PM isn't defined.
> 
> Can you make sure it compiles properly under all three configurations?
> 
> Rafael, any objections?

Hm, well, one thing below.

> Index: usb-2.6/drivers/base/power/power.h
> ===================================================================
> --- usb-2.6.orig/drivers/base/power/power.h
> +++ usb-2.6/drivers/base/power/power.h
> @@ -1,3 +1,8 @@
> +static inline void device_pm_init(struct device *dev)
> +{
> +	dev->power.status = DPM_ON;
> +}
> +
>  #ifdef CONFIG_PM_SLEEP
>  
>  /*
> Index: usb-2.6/drivers/base/core.c
> ===================================================================
> --- usb-2.6.orig/drivers/base/core.c
> +++ usb-2.6/drivers/base/core.c
> @@ -542,6 +542,7 @@ void device_initialize(struct device *de
>  	spin_lock_init(&dev->devres_lock);
>  	INIT_LIST_HEAD(&dev->devres_head);
>  	device_init_wakeup(dev, 0);
> +	device_pm_init(dev);
>  	set_dev_node(dev, -1);
>  }
>  
> @@ -898,6 +899,9 @@ int device_add(struct device *dev)
>  	error = bus_add_device(dev);
>  	if (error)
>  		goto BusError;
> +	error = dpm_sysfs_add(dev);
> +	if (error)
> +		goto DPMError;
>  	error = device_pm_add(dev);
>
>  	if (error)
>  		goto PMError;
> @@ -922,6 +926,8 @@ int device_add(struct device *dev)
>  	put_device(dev);
>  	return error;
>   PMError:
> +	dpm_sysfs_remove(dev);
> + DPMError:
>  	bus_remove_device(dev);
>   BusError:
>  	if (dev->bus)
> @@ -1008,6 +1014,7 @@ void device_del(struct device *dev)
>  	struct class_interface *class_intf;
>  
>  	device_pm_remove(dev);
> +	dpm_sysfs_remove(dev);
>  	if (parent)
>  		klist_del(&dev->knode_parent);
>  	if (MAJOR(dev->devt)) {
> Index: usb-2.6/drivers/base/power/main.c
> ===================================================================
> --- usb-2.6.orig/drivers/base/power/main.c
> +++ usb-2.6/drivers/base/power/main.c
> @@ -69,7 +69,7 @@ void device_pm_unlock(void)
>   */

device_pm_add() can now be void and the error variable in there is not
necessary any more.

>  int device_pm_add(struct device *dev)
>  {
> -	int error;
> +	int error = 0;
>  
>  	pr_debug("PM: Adding info for %s:%s\n",
>  		 dev->bus ? dev->bus->name : "No Bus",
> @@ -89,11 +89,8 @@ int device_pm_add(struct device *dev)
>  		 */
>  		WARN_ON(true);
>  	}
> -	error = dpm_sysfs_add(dev);
> -	if (!error) {
> -		dev->power.status = DPM_ON;
> -		list_add_tail(&dev->power.entry, &dpm_list);
> -	}
> +
> +	list_add_tail(&dev->power.entry, &dpm_list);
>  	mutex_unlock(&dpm_list_mtx);
>  	return error;
>  }
> @@ -110,7 +107,6 @@ void device_pm_remove(struct device *dev
>  		 dev->bus ? dev->bus->name : "No Bus",
>  		 kobject_name(&dev->kobj));
>  	mutex_lock(&dpm_list_mtx);
> -	dpm_sysfs_remove(dev);
>  	list_del_init(&dev->power.entry);
>  	mutex_unlock(&dpm_list_mtx);
>  }

Thanks,
Rafael

Re: [PATCH] [PM] Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP

[Alan Stern]

On Mon, 4 Aug 2008, Rafael J. Wysocki wrote:

> > Rafael, any objections?
> 
> Hm, well, one thing below.

> device_pm_add() can now be void and the error variable in there is not
> necessary any more.

Do you think we will want to make device_pm_add() return an error at 
some point in the future?  For example, earlier you had it fail if the 
parent device was asleep, instead of just printing a warning.

Alan Stern

Re: [PATCH] [PM] Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP

[Rafael J. Wysocki]

On Monday, 4 of August 2008, Alan Stern wrote:
> On Mon, 4 Aug 2008, Rafael J. Wysocki wrote:
> 
> > > Rafael, any objections?
> > 
> > Hm, well, one thing below.
> 
> > device_pm_add() can now be void and the error variable in there is not
> > necessary any more.
> 
> Do you think we will want to make device_pm_add() return an error at 
> some point in the future?

Well, maybe, but not any time soon. :-)

> For example, earlier you had it fail if the parent device was asleep, instead
> of just printing a warning. 

Yes, I remeber it, but there still are some drivers that would break if we
did that.

Anyway, I think it won't be hard to change device_pm_add() to return error
codes again if we decide to in future.

Thanks,
Rafael

Re: [PATCH] [PM] Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP

[Alan Stern]

On Mon, 4 Aug 2008, Rafael J. Wysocki wrote:

> > Do you think we will want to make device_pm_add() return an error at 
> > some point in the future?
> 
> Well, maybe, but not any time soon. :-)

Okay, here's the updated patch.  Awaiting comments from the testers...

Alan Stern



Index: usb-2.6/drivers/base/power/power.h
===================================================================
--- usb-2.6.orig/drivers/base/power/power.h
+++ usb-2.6/drivers/base/power/power.h
@@ -1,3 +1,8 @@
+static inline void device_pm_init(struct device *dev)
+{
+	dev->power.status = DPM_ON;
+}
+
 #ifdef CONFIG_PM_SLEEP
 
 /*
@@ -11,12 +16,12 @@ static inline struct device *to_device(s
 	return container_of(entry, struct device, power.entry);
 }
 
-extern int device_pm_add(struct device *);
+extern void device_pm_add(struct device *);
 extern void device_pm_remove(struct device *);
 
 #else /* CONFIG_PM_SLEEP */
 
-static inline int device_pm_add(struct device *dev) { return 0; }
+static inline void device_pm_add(struct device *dev) {}
 static inline void device_pm_remove(struct device *dev) {}
 
 #endif
Index: usb-2.6/drivers/base/core.c
===================================================================
--- usb-2.6.orig/drivers/base/core.c
+++ usb-2.6/drivers/base/core.c
@@ -542,6 +542,7 @@ void device_initialize(struct device *de
 	spin_lock_init(&dev->devres_lock);
 	INIT_LIST_HEAD(&dev->devres_head);
 	device_init_wakeup(dev, 0);
+	device_pm_init(dev);
 	set_dev_node(dev, -1);
 }
 
@@ -898,9 +899,10 @@ int device_add(struct device *dev)
 	error = bus_add_device(dev);
 	if (error)
 		goto BusError;
-	error = device_pm_add(dev);
+	error = dpm_sysfs_add(dev);
 	if (error)
 		goto PMError;
+	device_pm_add(dev);
 	kobject_uevent(&dev->kobj, KOBJ_ADD);
 	bus_attach_device(dev);
 	if (parent)
@@ -1008,6 +1010,7 @@ void device_del(struct device *dev)
 	struct class_interface *class_intf;
 
 	device_pm_remove(dev);
+	dpm_sysfs_remove(dev);
 	if (parent)
 		klist_del(&dev->knode_parent);
 	if (MAJOR(dev->devt)) {
Index: usb-2.6/drivers/base/power/main.c
===================================================================
--- usb-2.6.orig/drivers/base/power/main.c
+++ usb-2.6/drivers/base/power/main.c
@@ -67,10 +67,8 @@ void device_pm_unlock(void)
  *	device_pm_add - add a device to the list of active devices
  *	@dev:	Device to be added to the list
  */
-int device_pm_add(struct device *dev)
+void device_pm_add(struct device *dev)
 {
-	int error;
-
 	pr_debug("PM: Adding info for %s:%s\n",
 		 dev->bus ? dev->bus->name : "No Bus",
 		 kobject_name(&dev->kobj));
@@ -89,13 +87,9 @@ int device_pm_add(struct device *dev)
 		 */
 		WARN_ON(true);
 	}
-	error = dpm_sysfs_add(dev);
-	if (!error) {
-		dev->power.status = DPM_ON;
-		list_add_tail(&dev->power.entry, &dpm_list);
-	}
+
+	list_add_tail(&dev->power.entry, &dpm_list);
 	mutex_unlock(&dpm_list_mtx);
-	return error;
 }
 
 /**
@@ -110,7 +104,6 @@ void device_pm_remove(struct device *dev
 		 dev->bus ? dev->bus->name : "No Bus",
 		 kobject_name(&dev->kobj));
 	mutex_lock(&dpm_list_mtx);
-	dpm_sysfs_remove(dev);
 	list_del_init(&dev->power.entry);
 	mutex_unlock(&dpm_list_mtx);
 }

RE: [PATCH] [PM] Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP

[Dasgupta, Romit]

A minor change below: Removed old goto label PMError and introduced SysFSError.
Tested with CONFIG_PM, CONFIG_PM_SLEEP, without CONFIG_PM and ofcourse with CONFIG_USB_SUSPEND. Works fine.


diff --git a/drivers/base/core.c b/drivers/base/core.c
index 068aa1c..9c77939 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -541,6 +541,7 @@ void device_initialize(struct device *dev)
        spin_lock_init(&dev->devres_lock);
        INIT_LIST_HEAD(&dev->devres_head);
        device_init_wakeup(dev, 0);
+       device_pm_init(dev);
        set_dev_node(dev, -1);
 }

@@ -897,9 +898,10 @@ int device_add(struct device *dev)
        error = bus_add_device(dev);
        if (error)
                goto BusError;
-       error = device_pm_add(dev);
+       error = dpm_sysfs_add(dev);
        if (error)
-               goto PMError;
+               goto SysFSError;
+       device_pm_add(dev);
        kobject_uevent(&dev->kobj, KOBJ_ADD);
        bus_attach_device(dev);
        if (parent)
@@ -920,7 +922,7 @@ int device_add(struct device *dev)
  Done:
        put_device(dev);
        return error;
- PMError:
+ SysFSError:
        bus_remove_device(dev);
  BusError:
        if (dev->bus)
@@ -1007,6 +1009,7 @@ void device_del(struct device *dev)
        struct class_interface *class_intf;

        device_pm_remove(dev);
+       dpm_sysfs_remove(dev);
        if (parent)
                klist_del(&dev->knode_parent);
        if (MAJOR(dev->devt)) {
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 3250c52..d1f752b 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -67,10 +67,8 @@ void device_pm_unlock(void)
  *     device_pm_add - add a device to the list of active devices
  *     @dev:   Device to be added to the list
  */
-int device_pm_add(struct device *dev)
+void device_pm_add(struct device *dev)
 {
-       int error;
-
        pr_debug("PM: Adding info for %s:%s\n",
                 dev->bus ? dev->bus->name : "No Bus",
                 kobject_name(&dev->kobj));
@@ -89,13 +87,8 @@ int device_pm_add(struct device *dev)
                 */
                WARN_ON(true);
        }
-       error = dpm_sysfs_add(dev);
-       if (!error) {
-               dev->power.status = DPM_ON;
-               list_add_tail(&dev->power.entry, &dpm_list);
-       }
+       list_add_tail(&dev->power.entry, &dpm_list);
        mutex_unlock(&dpm_list_mtx);
-       return error;
 }

 /**
@@ -110,7 +103,6 @@ void device_pm_remove(struct device *dev)
                 dev->bus ? dev->bus->name : "No Bus",
                 kobject_name(&dev->kobj));
        mutex_lock(&dpm_list_mtx);
-       dpm_sysfs_remove(dev);
        list_del_init(&dev->power.entry);
        mutex_unlock(&dpm_list_mtx);
 }
diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
index a3252c0..41f51fa 100644
--- a/drivers/base/power/power.h
+++ b/drivers/base/power/power.h
@@ -1,3 +1,8 @@
+static inline void device_pm_init(struct device *dev)
+{
+       dev->power.status = DPM_ON;
+}
+
 #ifdef CONFIG_PM_SLEEP

 /*
@@ -11,12 +16,12 @@ static inline struct device *to_device(struct list_head *entry)
        return container_of(entry, struct device, power.entry);
 }

-extern int device_pm_add(struct device *);
+extern void device_pm_add(struct device *);
 extern void device_pm_remove(struct device *);

 #else /* CONFIG_PM_SLEEP */

-static inline int device_pm_add(struct device *dev) { return 0; }
+static inline void device_pm_add(struct device *dev) {}
 static inline void device_pm_remove(struct device *dev) {}

 #endif

>-----Original Message-----
>From: Alan Stern [mailto:stern@rowland.harvard.edu]
>Sent: Monday, August 04, 2008 9:15 PM
>To: Rafael J. Wysocki
>Cc: Dasgupta, Romit; David Engraf; Greg KH; linux-usb@vger.kernel.org; linux-
>kernel@vger.kernel.org; Linux-pm mailing list
>Subject: Re: [PATCH] [PM] Fixes missing PM entries in sysfs without
>CONFIG_PM_SLEEP
>
>On Mon, 4 Aug 2008, Rafael J. Wysocki wrote:
>
>> > Do you think we will want to make device_pm_add() return an error at
>> > some point in the future?
>>
>> Well, maybe, but not any time soon. :-)
>
>Okay, here's the updated patch.  Awaiting comments from the testers...
>
>Alan Stern
>
>
>
>Index: usb-2.6/drivers/base/power/power.h
>================================================================
>===
>--- usb-2.6.orig/drivers/base/power/power.h
>+++ usb-2.6/drivers/base/power/power.h
>@@ -1,3 +1,8 @@
>+static inline void device_pm_init(struct device *dev)
>+{
>+       dev->power.status = DPM_ON;
>+}
>+
> #ifdef CONFIG_PM_SLEEP
>
> /*
>@@ -11,12 +16,12 @@ static inline struct device *to_device(s
>        return container_of(entry, struct device, power.entry);
> }
>
>-extern int device_pm_add(struct device *);
>+extern void device_pm_add(struct device *);
> extern void device_pm_remove(struct device *);
>
> #else /* CONFIG_PM_SLEEP */
>
>-static inline int device_pm_add(struct device *dev) { return 0; }
>+static inline void device_pm_add(struct device *dev) {}
> static inline void device_pm_remove(struct device *dev) {}
>
> #endif
>Index: usb-2.6/drivers/base/core.c
>================================================================
>===
>--- usb-2.6.orig/drivers/base/core.c
>+++ usb-2.6/drivers/base/core.c
>@@ -542,6 +542,7 @@ void device_initialize(struct device *de
>        spin_lock_init(&dev->devres_lock);
>        INIT_LIST_HEAD(&dev->devres_head);
>        device_init_wakeup(dev, 0);
>+       device_pm_init(dev);
>        set_dev_node(dev, -1);
> }
>
>@@ -898,9 +899,10 @@ int device_add(struct device *dev)
>        error = bus_add_device(dev);
>        if (error)
>                goto BusError;
>-       error = device_pm_add(dev);
>+       error = dpm_sysfs_add(dev);
>        if (error)
>                goto PMError;
>+       device_pm_add(dev);
>        kobject_uevent(&dev->kobj, KOBJ_ADD);
>        bus_attach_device(dev);
>        if (parent)
>@@ -1008,6 +1010,7 @@ void device_del(struct device *dev)
>        struct class_interface *class_intf;
>
>        device_pm_remove(dev);
>+       dpm_sysfs_remove(dev);
>        if (parent)
>                klist_del(&dev->knode_parent);
>        if (MAJOR(dev->devt)) {
>Index: usb-2.6/drivers/base/power/main.c
>================================================================
>===
>--- usb-2.6.orig/drivers/base/power/main.c
>+++ usb-2.6/drivers/base/power/main.c
>@@ -67,10 +67,8 @@ void device_pm_unlock(void)
>  *     device_pm_add - add a device to the list of active devices
>  *     @dev:   Device to be added to the list
>  */
>-int device_pm_add(struct device *dev)
>+void device_pm_add(struct device *dev)
> {
>-       int error;
>-
>        pr_debug("PM: Adding info for %s:%s\n",
>                 dev->bus ? dev->bus->name : "No Bus",
>                 kobject_name(&dev->kobj));
>@@ -89,13 +87,9 @@ int device_pm_add(struct device *dev)
>                 */
>                WARN_ON(true);
>        }
>-       error = dpm_sysfs_add(dev);
>-       if (!error) {
>-               dev->power.status = DPM_ON;
>-               list_add_tail(&dev->power.entry, &dpm_list);
>-       }
>+
>+       list_add_tail(&dev->power.entry, &dpm_list);
>        mutex_unlock(&dpm_list_mtx);
>-       return error;
> }
>
> /**
>@@ -110,7 +104,6 @@ void device_pm_remove(struct device *dev
>                 dev->bus ? dev->bus->name : "No Bus",
>                 kobject_name(&dev->kobj));
>        mutex_lock(&dpm_list_mtx);
>-       dpm_sysfs_remove(dev);
>        list_del_init(&dev->power.entry);
>        mutex_unlock(&dpm_list_mtx);
> }

Re: [PATCH] [PM] Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP

[David Engraf]

Alan's patch is working on my system with CONFIG_PM and without
CONFIG_PM_SLEEP/CONFIG_USB_SUSPEND. Romit, I couldn't check your patch
because I was getting errors from the patch program (maybe the tabs).

Thanks again, David Engraf

> A minor change below: Removed old goto label PMError and introduced SysFSError.
> Tested with CONFIG_PM, CONFIG_PM_SLEEP, without CONFIG_PM and ofcourse with CONFIG_USB_SUSPEND. Works fine.
>
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 068aa1c..9c77939 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -541,6 +541,7 @@ void device_initialize(struct device *dev)
>         spin_lock_init(&dev->devres_lock);
>         INIT_LIST_HEAD(&dev->devres_head);
>         device_init_wakeup(dev, 0);
> +       device_pm_init(dev);
>         set_dev_node(dev, -1);
>  }
>
> @@ -897,9 +898,10 @@ int device_add(struct device *dev)
>         error = bus_add_device(dev);
>         if (error)
>                 goto BusError;
> -       error = device_pm_add(dev);
> +       error = dpm_sysfs_add(dev);
>         if (error)
> -               goto PMError;
> +               goto SysFSError;
> +       device_pm_add(dev);
>         kobject_uevent(&dev->kobj, KOBJ_ADD);
>         bus_attach_device(dev);
>         if (parent)
> @@ -920,7 +922,7 @@ int device_add(struct device *dev)
>   Done:
>         put_device(dev);
>         return error;
> - PMError:
> + SysFSError:
>         bus_remove_device(dev);
>   BusError:
>         if (dev->bus)
> @@ -1007,6 +1009,7 @@ void device_del(struct device *dev)
>         struct class_interface *class_intf;
>
>         device_pm_remove(dev);
> +       dpm_sysfs_remove(dev);
>         if (parent)
>                 klist_del(&dev->knode_parent);
>         if (MAJOR(dev->devt)) {
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 3250c52..d1f752b 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -67,10 +67,8 @@ void device_pm_unlock(void)
>   *     device_pm_add - add a device to the list of active devices
>   *     @dev:   Device to be added to the list
>   */
> -int device_pm_add(struct device *dev)
> +void device_pm_add(struct device *dev)
>  {
> -       int error;
> -
>         pr_debug("PM: Adding info for %s:%s\n",
>                  dev->bus ? dev->bus->name : "No Bus",
>                  kobject_name(&dev->kobj));
> @@ -89,13 +87,8 @@ int device_pm_add(struct device *dev)
>                  */
>                 WARN_ON(true);
>         }
> -       error = dpm_sysfs_add(dev);
> -       if (!error) {
> -               dev->power.status = DPM_ON;
> -               list_add_tail(&dev->power.entry, &dpm_list);
> -       }
> +       list_add_tail(&dev->power.entry, &dpm_list);
>         mutex_unlock(&dpm_list_mtx);
> -       return error;
>  }
>
>  /**
> @@ -110,7 +103,6 @@ void device_pm_remove(struct device *dev)
>                  dev->bus ? dev->bus->name : "No Bus",
>                  kobject_name(&dev->kobj));
>         mutex_lock(&dpm_list_mtx);
> -       dpm_sysfs_remove(dev);
>         list_del_init(&dev->power.entry);
>         mutex_unlock(&dpm_list_mtx);
>  }
> diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
> index a3252c0..41f51fa 100644
> --- a/drivers/base/power/power.h
> +++ b/drivers/base/power/power.h
> @@ -1,3 +1,8 @@
> +static inline void device_pm_init(struct device *dev)
> +{
> +       dev->power.status = DPM_ON;
> +}
> +
>  #ifdef CONFIG_PM_SLEEP
>
>  /*
> @@ -11,12 +16,12 @@ static inline struct device *to_device(struct list_head *entry)
>         return container_of(entry, struct device, power.entry);
>  }
>
> -extern int device_pm_add(struct device *);
> +extern void device_pm_add(struct device *);
>  extern void device_pm_remove(struct device *);
>
>  #else /* CONFIG_PM_SLEEP */
>
> -static inline int device_pm_add(struct device *dev) { return 0; }
> +static inline void device_pm_add(struct device *dev) {}
>  static inline void device_pm_remove(struct device *dev) {}
>
>  #endif
>
>   
>> -----Original Message-----
>> From: Alan Stern [mailto:stern@rowland.harvard.edu]
>> Sent: Monday, August 04, 2008 9:15 PM
>> To: Rafael J. Wysocki
>> Cc: Dasgupta, Romit; David Engraf; Greg KH; linux-usb@vger.kernel.org; linux-
>> kernel@vger.kernel.org; Linux-pm mailing list
>> Subject: Re: [PATCH] [PM] Fixes missing PM entries in sysfs without
>> CONFIG_PM_SLEEP
>>
>> On Mon, 4 Aug 2008, Rafael J. Wysocki wrote:
>>
>>     
>>>> Do you think we will want to make device_pm_add() return an error at
>>>> some point in the future?
>>>>         
>>> Well, maybe, but not any time soon. :-)
>>>       
>> Okay, here's the updated patch.  Awaiting comments from the testers...
>>
>> Alan Stern
>>
>>
>>
>> Index: usb-2.6/drivers/base/power/power.h
>> ================================================================
>> ===
>> --- usb-2.6.orig/drivers/base/power/power.h
>> +++ usb-2.6/drivers/base/power/power.h
>> @@ -1,3 +1,8 @@
>> +static inline void device_pm_init(struct device *dev)
>> +{
>> +       dev->power.status = DPM_ON;
>> +}
>> +
>> #ifdef CONFIG_PM_SLEEP
>>
>> /*
>> @@ -11,12 +16,12 @@ static inline struct device *to_device(s
>>        return container_of(entry, struct device, power.entry);
>> }
>>
>> -extern int device_pm_add(struct device *);
>> +extern void device_pm_add(struct device *);
>> extern void device_pm_remove(struct device *);
>>
>> #else /* CONFIG_PM_SLEEP */
>>
>> -static inline int device_pm_add(struct device *dev) { return 0; }
>> +static inline void device_pm_add(struct device *dev) {}
>> static inline void device_pm_remove(struct device *dev) {}
>>
>> #endif
>> Index: usb-2.6/drivers/base/core.c
>> ================================================================
>> ===
>> --- usb-2.6.orig/drivers/base/core.c
>> +++ usb-2.6/drivers/base/core.c
>> @@ -542,6 +542,7 @@ void device_initialize(struct device *de
>>        spin_lock_init(&dev->devres_lock);
>>        INIT_LIST_HEAD(&dev->devres_head);
>>        device_init_wakeup(dev, 0);
>> +       device_pm_init(dev);
>>        set_dev_node(dev, -1);
>> }
>>
>> @@ -898,9 +899,10 @@ int device_add(struct device *dev)
>>        error = bus_add_device(dev);
>>        if (error)
>>                goto BusError;
>> -       error = device_pm_add(dev);
>> +       error = dpm_sysfs_add(dev);
>>        if (error)
>>                goto PMError;
>> +       device_pm_add(dev);
>>        kobject_uevent(&dev->kobj, KOBJ_ADD);
>>        bus_attach_device(dev);
>>        if (parent)
>> @@ -1008,6 +1010,7 @@ void device_del(struct device *dev)
>>        struct class_interface *class_intf;
>>
>>        device_pm_remove(dev);
>> +       dpm_sysfs_remove(dev);
>>        if (parent)
>>                klist_del(&dev->knode_parent);
>>        if (MAJOR(dev->devt)) {
>> Index: usb-2.6/drivers/base/power/main.c
>> ================================================================
>> ===
>> --- usb-2.6.orig/drivers/base/power/main.c
>> +++ usb-2.6/drivers/base/power/main.c
>> @@ -67,10 +67,8 @@ void device_pm_unlock(void)
>>  *     device_pm_add - add a device to the list of active devices
>>  *     @dev:   Device to be added to the list
>>  */
>> -int device_pm_add(struct device *dev)
>> +void device_pm_add(struct device *dev)
>> {
>> -       int error;
>> -
>>        pr_debug("PM: Adding info for %s:%s\n",
>>                 dev->bus ? dev->bus->name : "No Bus",
>>                 kobject_name(&dev->kobj));
>> @@ -89,13 +87,9 @@ int device_pm_add(struct device *dev)
>>                 */
>>                WARN_ON(true);
>>        }
>> -       error = dpm_sysfs_add(dev);
>> -       if (!error) {
>> -               dev->power.status = DPM_ON;
>> -               list_add_tail(&dev->power.entry, &dpm_list);
>> -       }
>> +
>> +       list_add_tail(&dev->power.entry, &dpm_list);
>>        mutex_unlock(&dpm_list_mtx);
>> -       return error;
>> }
>>
>> /**
>> @@ -110,7 +104,6 @@ void device_pm_remove(struct device *dev
>>                 dev->bus ? dev->bus->name : "No Bus",
>>                 kobject_name(&dev->kobj));
>>        mutex_lock(&dpm_list_mtx);
>> -       dpm_sysfs_remove(dev);
>>        list_del_init(&dev->power.entry);
>>        mutex_unlock(&dpm_list_mtx);
>> }
>>     
>
>   

RE: [PATCH] [PM] Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP

[Dasgupta, Romit]

I think it is my mailer program that is adding the tabs.
In Alan's patch I just changed the PMError label with SysFSError.


>-----Original Message-----
>From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-owner@vger.kernel.org]
>On Behalf Of David Engraf
>Sent: Tuesday, August 05, 2008 6:29 PM
>To: Dasgupta, Romit
>Cc: Alan Stern; Rafael J. Wysocki; Greg KH; linux-usb@vger.kernel.org; linux-
>kernel@vger.kernel.org; Linux-pm mailing list
>Subject: Re: [PATCH] [PM] Fixes missing PM entries in sysfs without
>CONFIG_PM_SLEEP
>
>Alan's patch is working on my system with CONFIG_PM and without
>CONFIG_PM_SLEEP/CONFIG_USB_SUSPEND. Romit, I couldn't check your patch
>because I was getting errors from the patch program (maybe the tabs).
>
>Thanks again, David Engraf
>
>> A minor change below: Removed old goto label PMError and introduced
>SysFSError.
>> Tested with CONFIG_PM, CONFIG_PM_SLEEP, without CONFIG_PM and
>ofcourse with CONFIG_USB_SUSPEND. Works fine.
>>
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index 068aa1c..9c77939 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -541,6 +541,7 @@ void device_initialize(struct device *dev)
>>         spin_lock_init(&dev->devres_lock);
>>         INIT_LIST_HEAD(&dev->devres_head);
>>         device_init_wakeup(dev, 0);
>> +       device_pm_init(dev);
>>         set_dev_node(dev, -1);
>>  }
>>
>> @@ -897,9 +898,10 @@ int device_add(struct device *dev)
>>         error = bus_add_device(dev);
>>         if (error)
>>                 goto BusError;
>> -       error = device_pm_add(dev);
>> +       error = dpm_sysfs_add(dev);
>>         if (error)
>> -               goto PMError;
>> +               goto SysFSError;
>> +       device_pm_add(dev);
>>         kobject_uevent(&dev->kobj, KOBJ_ADD);
>>         bus_attach_device(dev);
>>         if (parent)
>> @@ -920,7 +922,7 @@ int device_add(struct device *dev)
>>   Done:
>>         put_device(dev);
>>         return error;
>> - PMError:
>> + SysFSError:
>>         bus_remove_device(dev);
>>   BusError:
>>         if (dev->bus)
>> @@ -1007,6 +1009,7 @@ void device_del(struct device *dev)
>>         struct class_interface *class_intf;
>>
>>         device_pm_remove(dev);
>> +       dpm_sysfs_remove(dev);
>>         if (parent)
>>                 klist_del(&dev->knode_parent);
>>         if (MAJOR(dev->devt)) {
>> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
>> index 3250c52..d1f752b 100644
>> --- a/drivers/base/power/main.c
>> +++ b/drivers/base/power/main.c
>> @@ -67,10 +67,8 @@ void device_pm_unlock(void)
>>   *     device_pm_add - add a device to the list of active devices
>>   *     @dev:   Device to be added to the list
>>   */
>> -int device_pm_add(struct device *dev)
>> +void device_pm_add(struct device *dev)
>>  {
>> -       int error;
>> -
>>         pr_debug("PM: Adding info for %s:%s\n",
>>                  dev->bus ? dev->bus->name : "No Bus",
>>                  kobject_name(&dev->kobj));
>> @@ -89,13 +87,8 @@ int device_pm_add(struct device *dev)
>>                  */
>>                 WARN_ON(true);
>>         }
>> -       error = dpm_sysfs_add(dev);
>> -       if (!error) {
>> -               dev->power.status = DPM_ON;
>> -               list_add_tail(&dev->power.entry, &dpm_list);
>> -       }
>> +       list_add_tail(&dev->power.entry, &dpm_list);
>>         mutex_unlock(&dpm_list_mtx);
>> -       return error;
>>  }
>>
>>  /**
>> @@ -110,7 +103,6 @@ void device_pm_remove(struct device *dev)
>>                  dev->bus ? dev->bus->name : "No Bus",
>>                  kobject_name(&dev->kobj));
>>         mutex_lock(&dpm_list_mtx);
>> -       dpm_sysfs_remove(dev);
>>         list_del_init(&dev->power.entry);
>>         mutex_unlock(&dpm_list_mtx);
>>  }
>> diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
>> index a3252c0..41f51fa 100644
>> --- a/drivers/base/power/power.h
>> +++ b/drivers/base/power/power.h
>> @@ -1,3 +1,8 @@
>> +static inline void device_pm_init(struct device *dev)
>> +{
>> +       dev->power.status = DPM_ON;
>> +}
>> +
>>  #ifdef CONFIG_PM_SLEEP
>>
>>  /*
>> @@ -11,12 +16,12 @@ static inline struct device *to_device(struct list_head
>*entry)
>>         return container_of(entry, struct device, power.entry);
>>  }
>>
>> -extern int device_pm_add(struct device *);
>> +extern void device_pm_add(struct device *);
>>  extern void device_pm_remove(struct device *);
>>
>>  #else /* CONFIG_PM_SLEEP */
>>
>> -static inline int device_pm_add(struct device *dev) { return 0; }
>> +static inline void device_pm_add(struct device *dev) {}
>>  static inline void device_pm_remove(struct device *dev) {}
>>
>>  #endif
>>
>>
>>> -----Original Message-----
>>> From: Alan Stern [mailto:stern@rowland.harvard.edu]
>>> Sent: Monday, August 04, 2008 9:15 PM
>>> To: Rafael J. Wysocki
>>> Cc: Dasgupta, Romit; David Engraf; Greg KH; linux-usb@vger.kernel.org; linux-
>>> kernel@vger.kernel.org; Linux-pm mailing list
>>> Subject: Re: [PATCH] [PM] Fixes missing PM entries in sysfs without
>>> CONFIG_PM_SLEEP
>>>
>>> On Mon, 4 Aug 2008, Rafael J. Wysocki wrote:
>>>
>>>
>>>>> Do you think we will want to make device_pm_add() return an error at
>>>>> some point in the future?
>>>>>
>>>> Well, maybe, but not any time soon. :-)
>>>>
>>> Okay, here's the updated patch.  Awaiting comments from the testers...
>>>
>>> Alan Stern
>>>
>>>
>>>
>>> Index: usb-2.6/drivers/base/power/power.h
>>>
>================================================================
>>> ===
>>> --- usb-2.6.orig/drivers/base/power/power.h
>>> +++ usb-2.6/drivers/base/power/power.h
>>> @@ -1,3 +1,8 @@
>>> +static inline void device_pm_init(struct device *dev)
>>> +{
>>> +       dev->power.status = DPM_ON;
>>> +}
>>> +
>>> #ifdef CONFIG_PM_SLEEP
>>>
>>> /*
>>> @@ -11,12 +16,12 @@ static inline struct device *to_device(s
>>>        return container_of(entry, struct device, power.entry);
>>> }
>>>
>>> -extern int device_pm_add(struct device *);
>>> +extern void device_pm_add(struct device *);
>>> extern void device_pm_remove(struct device *);
>>>
>>> #else /* CONFIG_PM_SLEEP */
>>>
>>> -static inline int device_pm_add(struct device *dev) { return 0; }
>>> +static inline void device_pm_add(struct device *dev) {}
>>> static inline void device_pm_remove(struct device *dev) {}
>>>
>>> #endif
>>> Index: usb-2.6/drivers/base/core.c
>>>
>================================================================
>>> ===
>>> --- usb-2.6.orig/drivers/base/core.c
>>> +++ usb-2.6/drivers/base/core.c
>>> @@ -542,6 +542,7 @@ void device_initialize(struct device *de
>>>        spin_lock_init(&dev->devres_lock);
>>>        INIT_LIST_HEAD(&dev->devres_head);
>>>        device_init_wakeup(dev, 0);
>>> +       device_pm_init(dev);
>>>        set_dev_node(dev, -1);
>>> }
>>>
>>> @@ -898,9 +899,10 @@ int device_add(struct device *dev)
>>>        error = bus_add_device(dev);
>>>        if (error)
>>>                goto BusError;
>>> -       error = device_pm_add(dev);
>>> +       error = dpm_sysfs_add(dev);
>>>        if (error)
>>>                goto PMError;
>>> +       device_pm_add(dev);
>>>        kobject_uevent(&dev->kobj, KOBJ_ADD);
>>>        bus_attach_device(dev);
>>>        if (parent)
>>> @@ -1008,6 +1010,7 @@ void device_del(struct device *dev)
>>>        struct class_interface *class_intf;
>>>
>>>        device_pm_remove(dev);
>>> +       dpm_sysfs_remove(dev);
>>>        if (parent)
>>>                klist_del(&dev->knode_parent);
>>>        if (MAJOR(dev->devt)) {
>>> Index: usb-2.6/drivers/base/power/main.c
>>>
>================================================================
>>> ===
>>> --- usb-2.6.orig/drivers/base/power/main.c
>>> +++ usb-2.6/drivers/base/power/main.c
>>> @@ -67,10 +67,8 @@ void device_pm_unlock(void)
>>>  *     device_pm_add - add a device to the list of active devices
>>>  *     @dev:   Device to be added to the list
>>>  */
>>> -int device_pm_add(struct device *dev)
>>> +void device_pm_add(struct device *dev)
>>> {
>>> -       int error;
>>> -
>>>        pr_debug("PM: Adding info for %s:%s\n",
>>>                 dev->bus ? dev->bus->name : "No Bus",
>>>                 kobject_name(&dev->kobj));
>>> @@ -89,13 +87,9 @@ int device_pm_add(struct device *dev)
>>>                 */
>>>                WARN_ON(true);
>>>        }
>>> -       error = dpm_sysfs_add(dev);
>>> -       if (!error) {
>>> -               dev->power.status = DPM_ON;
>>> -               list_add_tail(&dev->power.entry, &dpm_list);
>>> -       }
>>> +
>>> +       list_add_tail(&dev->power.entry, &dpm_list);
>>>        mutex_unlock(&dpm_list_mtx);
>>> -       return error;
>>> }
>>>
>>> /**
>>> @@ -110,7 +104,6 @@ void device_pm_remove(struct device *dev
>>>                 dev->bus ? dev->bus->name : "No Bus",
>>>                 kobject_name(&dev->kobj));
>>>        mutex_lock(&dpm_list_mtx);
>>> -       dpm_sysfs_remove(dev);
>>>        list_del_init(&dev->power.entry);
>>>        mutex_unlock(&dpm_list_mtx);
>>> }
>>>
>>
>>
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] [PM] Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP

[Alan Stern]

On Tue, 5 Aug 2008, Dasgupta, Romit wrote:

> A minor change below: Removed old goto label PMError and introduced SysFSError.
> Tested with CONFIG_PM, CONFIG_PM_SLEEP, without CONFIG_PM and ofcourse with CONFIG_USB_SUSPEND. Works fine.

Strictly speaking, I suppose the statement label should be 
DPMSysfsError.

On the other hand, the corresponding label for
device_add_class_symlinks() is SymlinkError, not
DeviceAddClassSymlinksError.  The label for device_add_attrs() is
AttrsError, not DeviceAddAttrsError.  And the label for
bus_add_device() is BusError, not BusAddDeviceError.

Following those precedents, it seems appropriate that the label for
dpm_sysfs_add() should be DPMError.  I'll make the change and submit
the patch.

Alan Stern

Re: [PATCH] [PM] Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP

[David Engraf]

Is there an option to make a patch for the recent 2.6.26.1 kernel, so
that I can use the next 2.6.26.2 kernel instead of waiting a few months
for the next big release?

Thanks David Engraf


Alan Stern wrote:
> On Tue, 5 Aug 2008, Dasgupta, Romit wrote:
>
>   
>> A minor change below: Removed old goto label PMError and introduced SysFSError.
>> Tested with CONFIG_PM, CONFIG_PM_SLEEP, without CONFIG_PM and ofcourse with CONFIG_USB_SUSPEND. Works fine.
>>     
>
> Strictly speaking, I suppose the statement label should be 
> DPMSysfsError.
>
> On the other hand, the corresponding label for
> device_add_class_symlinks() is SymlinkError, not
> DeviceAddClassSymlinksError.  The label for device_add_attrs() is
> AttrsError, not DeviceAddAttrsError.  And the label for
> bus_add_device() is BusError, not BusAddDeviceError.
>
> Following those precedents, it seems appropriate that the label for
> dpm_sysfs_add() should be DPMError.  I'll make the change and submit
> the patch.
>
> Alan Stern
>
>   

Re: [PATCH] [PM] Fixes missing PM entries in sysfs without CONFIG_PM_SLEEP

[Alan Stern]

On Tue, 5 Aug 2008, David Engraf wrote:

> Is there an option to make a patch for the recent 2.6.26.1 kernel, so
> that I can use the next 2.6.26.2 kernel instead of waiting a few months
> for the next big release?

After the patch gets accepted, we can submit it for the 2.6.26-stable 
series.  Remind me in case I forget.

Alan Stern