Re: [linux-usb-devel] [RFC][PATCH -mm 1/7] PM: Remove pm_parent from struct dev_pm_info

2007-06-11 Thread Alan Stern
On Mon, 11 Jun 2007, Rafael J. Wysocki wrote:

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

 @@ -61,21 +40,26 @@ int device_pm_add(struct device * dev)
kobject_name(dev-kobj));
   mutex_lock(dpm_list_mtx);
   list_add_tail(dev-power.entry, dpm_active);
 - device_pm_set_parent(dev, dev-parent);
 - if ((error = dpm_sysfs_add(dev)))
 + /*
 +  * The device's parent must not be released until the device itself is
 +  * removed from the dpm_active list.
 +  */
 + get_device(dev-parent);
 + error = dpm_sysfs_add(dev);
 + if (error)
   list_del(dev-power.entry);
   mutex_unlock(dpm_list_mtx);
   return error;
  }

The error pathway here does an unbalanced get_device on dev-parent.

Anyway, I don't think you need to do this get_device at all (or the
coresponding put in device_pm_remove).  As long as a device is
registered it retains a reference to its parent, and unregistration
always calls device_pm_remove.  The reason it was there in the first 
place was because people recognized that dev-power.pm_parent wouldn't 
be one of dev's ancestors in the device hierarchy.

Alan Stern


-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel


Re: [linux-usb-devel] [RFC][PATCH -mm 1/7] PM: Remove pm_parent from struct dev_pm_info

2007-06-11 Thread Rafael J. Wysocki
On Monday, 11 June 2007 17:59, Alan Stern wrote:
 On Mon, 11 Jun 2007, Rafael J. Wysocki wrote:
 
  From: Rafael J. Wysocki [EMAIL PROTECTED]
  
  The pm_parent member of struct dev_pm_info (defined in include/linux/pm.h) 
  is
  only used to check if the device's parent is in the right state while the
  device is being suspended or resumed.  However, this can be done just as 
  well
  with the help of the parent pointer in struct device, so pm_parent can be
  removed along with some code that handles it.
 
  @@ -61,21 +40,26 @@ int device_pm_add(struct device * dev)
   kobject_name(dev-kobj));
  mutex_lock(dpm_list_mtx);
  list_add_tail(dev-power.entry, dpm_active);
  -   device_pm_set_parent(dev, dev-parent);
  -   if ((error = dpm_sysfs_add(dev)))
  +   /*
  +* The device's parent must not be released until the device itself is
  +* removed from the dpm_active list.
  +*/
  +   get_device(dev-parent);
  +   error = dpm_sysfs_add(dev);
  +   if (error)
  list_del(dev-power.entry);
  mutex_unlock(dpm_list_mtx);
  return error;
   }
 
 The error pathway here does an unbalanced get_device on dev-parent.
 
 Anyway, I don't think you need to do this get_device at all (or the
 coresponding put in device_pm_remove).  As long as a device is
 registered it retains a reference to its parent, and unregistration
 always calls device_pm_remove.

Yes, I've just come to the same conclusion.  I'll remove the
get_device(dev-parent) and the correspondint put_device(dev-parent)
from device_pm_remove().

Greetings,
Rafael


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

-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel


Re: [linux-usb-devel] [RFC][PATCH -mm 1/7] PM: Remove pm_parent from struct dev_pm_info

2007-06-11 Thread Rafael J. Wysocki
On Monday, 11 June 2007 20:52, Rafael J. Wysocki wrote:
 On Monday, 11 June 2007 17:59, Alan Stern wrote:
  On Mon, 11 Jun 2007, Rafael J. Wysocki wrote:
  
   From: Rafael J. Wysocki [EMAIL PROTECTED]
   
   The pm_parent member of struct dev_pm_info (defined in 
   include/linux/pm.h) is
   only used to check if the device's parent is in the right state while the
   device is being suspended or resumed.  However, this can be done just as 
   well
   with the help of the parent pointer in struct device, so pm_parent can be
   removed along with some code that handles it.
  
   @@ -61,21 +40,26 @@ int device_pm_add(struct device * dev)
  kobject_name(dev-kobj));
 mutex_lock(dpm_list_mtx);
 list_add_tail(dev-power.entry, dpm_active);
   - device_pm_set_parent(dev, dev-parent);
   - if ((error = dpm_sysfs_add(dev)))
   + /*
   +  * The device's parent must not be released until the device itself is
   +  * removed from the dpm_active list.
   +  */
   + get_device(dev-parent);
   + error = dpm_sysfs_add(dev);
   + if (error)
 list_del(dev-power.entry);
 mutex_unlock(dpm_list_mtx);
 return error;
}
  
  The error pathway here does an unbalanced get_device on dev-parent.
  
  Anyway, I don't think you need to do this get_device at all (or the
  coresponding put in device_pm_remove).  As long as a device is
  registered it retains a reference to its parent, and unregistration
  always calls device_pm_remove.
 
 Yes, I've just come to the same conclusion.  I'll remove the
 get_device(dev-parent) and the correspondint put_device(dev-parent)
 from device_pm_remove().

OK

The updated patch follows.

Greetings,
Rafael

---
From: Rafael J. Wysocki [EMAIL PROTECTED]

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

Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
---
 drivers/base/power/main.c|   30 --
 drivers/base/power/resume.c  |7 +++
 drivers/base/power/suspend.c |7 +++
 include/linux/pm.h   |3 ---
 4 files changed, 10 insertions(+), 37 deletions(-)

Index: linux-2.6.22-rc4/drivers/base/power/main.c
===
--- linux-2.6.22-rc4.orig/drivers/base/power/main.c 2007-06-10 
19:35:49.0 +0200
+++ linux-2.6.22-rc4/drivers/base/power/main.c  2007-06-11 21:05:10.0 
+0200
@@ -31,28 +31,7 @@ DEFINE_MUTEX(dpm_list_mtx);
 
 int (*platform_enable_wakeup)(struct device *dev, int is_on);
 
-
-/**
- * device_pm_set_parent - Specify power dependency.
- * @dev:   Device who needs power.
- * @parent:Device that supplies power.
- *
- * This function is used to manually describe a power-dependency
- * relationship. It may be used to specify a transversal relationship
- * (where the power supplier is not the physical (or electrical)
- * ancestor of a specific device.
- * The effect of this is that the supplier will not be powered down
- * before the power dependent.
- */
-
-void device_pm_set_parent(struct device * dev, struct device * parent)
-{
-   put_device(dev-power.pm_parent);
-   dev-power.pm_parent = get_device(parent);
-}
-EXPORT_SYMBOL_GPL(device_pm_set_parent);
-
-int device_pm_add(struct device * dev)
+int device_pm_add(struct device *dev)
 {
int error;
 
@@ -61,21 +40,20 @@ int device_pm_add(struct device * dev)
 kobject_name(dev-kobj));
mutex_lock(dpm_list_mtx);
list_add_tail(dev-power.entry, dpm_active);
-   device_pm_set_parent(dev, dev-parent);
-   if ((error = dpm_sysfs_add(dev)))
+   error = dpm_sysfs_add(dev);
+   if (error)
list_del(dev-power.entry);
mutex_unlock(dpm_list_mtx);
return error;
 }
 
-void device_pm_remove(struct device * dev)
+void device_pm_remove(struct device *dev)
 {
pr_debug(PM: Removing info for %s:%s\n,
 dev-bus ? dev-bus-name : No Bus,
 kobject_name(dev-kobj));
mutex_lock(dpm_list_mtx);
dpm_sysfs_remove(dev);
-   put_device(dev-power.pm_parent);
list_del_init(dev-power.entry);
mutex_unlock(dpm_list_mtx);
 }
Index: linux-2.6.22-rc4/drivers/base/power/resume.c
===
--- linux-2.6.22-rc4.orig/drivers/base/power/resume.c   2007-06-10 
19:35:49.0 +0200
+++ linux-2.6.22-rc4/drivers/base/power/resume.c2007-06-11 
21:04:44.0 +0200
@@ -29,12 +29,11 @@ int resume_device(struct device * dev)
 
down(dev-sem);
 
-   if (dev-power.pm_parent
-dev-power.pm_parent-power.power_state.event) {