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 <[email protected]>
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:[email protected]]
>Sent: Sunday, August 03, 2008 5:16 AM
>To: Dasgupta, Romit
>Cc: David Engraf; Greg KH; [email protected]
>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
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);
}
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
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
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
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);
}
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:[email protected]]
>Sent: Monday, August 04, 2008 9:15 PM
>To: Rafael J. Wysocki
>Cc: Dasgupta, Romit; David Engraf; Greg KH; [email protected]; linux-
>[email protected]; 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);
> }
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:[email protected]]
>> Sent: Monday, August 04, 2008 9:15 PM
>> To: Rafael J. Wysocki
>> Cc: Dasgupta, Romit; David Engraf; Greg KH; [email protected]; linux-
>> [email protected]; 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);
>> }
>>
>
>
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: [email protected] [mailto:[email protected]]
>On Behalf Of David Engraf
>Sent: Tuesday, August 05, 2008 6:29 PM
>To: Dasgupta, Romit
>Cc: Alan Stern; Rafael J. Wysocki; Greg KH; [email protected]; linux-
>[email protected]; 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:[email protected]]
>>> Sent: Monday, August 04, 2008 9:15 PM
>>> To: Rafael J. Wysocki
>>> Cc: Dasgupta, Romit; David Engraf; Greg KH; [email protected]; linux-
>>> [email protected]; 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 [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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
>
>
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