| Message ID | 201108310021.13800.rjw@sisk.pl (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi Rafael, On Wed, Aug 31, 2011 at 12:21 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > From: Rafael J. Wysocki <rjw@sisk.pl> > > To read the current PM QoS value for a given device we need to > make sure that the device's power.constraints object won't be > removed while we're doing that. For this reason, put the > operation under dev->power.lock and acquire the lock > around the initialization and removal of power.constraints. Ok. > Moreover, since we're using the value of power.constraints to > determine whether or not the object is present, the > power.constraints_state field isn't necessary any more and may be > removed. However, dev_pm_qos_add_request() needs to check if the > device is being removed from the system before allocating a new > PM QoS constraints object for it, so it has to use device_pm_lock() > and the device PM QoS initialization and destruction should be done > under device_pm_lock() as well. Ok that makes sense. The constraints_state field can be replaced by a combination of dev->power.constraints and list_empty(&dev->power.entry), which makes the code more compact and less redundant. > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > --- > drivers/base/power/main.c | 4 - > drivers/base/power/qos.c | 167 ++++++++++++++++++++++++++-------------------- > include/linux/pm.h | 8 -- > include/linux/pm_qos.h | 3 > 4 files changed, 101 insertions(+), 81 deletions(-) > > Index: linux/drivers/base/power/qos.c > =================================================================== > --- linux.orig/drivers/base/power/qos.c > +++ linux/drivers/base/power/qos.c > @@ -30,15 +30,6 @@ ... > > @@ -178,8 +202,8 @@ void dev_pm_qos_constraints_destroy(stru > * > * Returns 1 if the aggregated constraint value has changed, > * 0 if the aggregated constraint value has not changed, > - * -EINVAL in case of wrong parameters, -ENODEV if the device has been > - * removed from the system > + * -EINVAL in case of wrong parameters, -ENOMEM if there's not enough memory > + * to allocate for data structures. Why not use -ENODEV in case there is no device? > */ > int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req, > s32 value) > @@ -195,28 +219,35 @@ int dev_pm_qos_add_request(struct device > return -EINVAL; > } > > - mutex_lock(&dev_pm_qos_mtx); > req->dev = dev; > > - /* Return if the device has been removed */ > - if (req->dev->power.constraints_state == DEV_PM_QOS_NO_DEVICE) { > - ret = -ENODEV; > - goto out; > - } > + device_pm_lock(); > + mutex_lock(&dev_pm_qos_mtx); > > - /* > - * Allocate the constraints data on the first call to add_request, > - * i.e. only if the data is not already allocated and if the device has > - * not been removed > - */ > - if (dev->power.constraints_state == DEV_PM_QOS_DEVICE_PRESENT) > - ret = dev_pm_qos_constraints_allocate(dev); > + if (dev->power.constraints) { > + device_pm_unlock(); > + } else { > + if (list_empty(&dev->power.entry)) { > + /* The device has been removed from the system. */ > + device_pm_unlock(); > + goto out; 0 is silently returned in case the device has been removed. Is that the intention? > + } else { > + device_pm_unlock(); > + /* > + * Allocate the constraints data on the first call to > + * add_request, i.e. only if the data is not already > + * allocated and if the device has not been removed. > + */ > + ret = dev_pm_qos_constraints_allocate(dev); > + } > + } > > if (!ret) > ret = apply_constraint(req, PM_QOS_ADD_REQ, value); > > -out: > + out: > mutex_unlock(&dev_pm_qos_mtx); > + > return ret; > } > EXPORT_SYMBOL_GPL(dev_pm_qos_add_request); ... Thanks, Jean -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Thursday, September 01, 2011, Jean Pihet wrote: > Hi Rafael, > > On Wed, Aug 31, 2011 at 12:21 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > From: Rafael J. Wysocki <rjw@sisk.pl> > > > > To read the current PM QoS value for a given device we need to > > make sure that the device's power.constraints object won't be > > removed while we're doing that. For this reason, put the > > operation under dev->power.lock and acquire the lock > > around the initialization and removal of power.constraints. > Ok. > > > Moreover, since we're using the value of power.constraints to > > determine whether or not the object is present, the > > power.constraints_state field isn't necessary any more and may be > > removed. However, dev_pm_qos_add_request() needs to check if the > > device is being removed from the system before allocating a new > > PM QoS constraints object for it, so it has to use device_pm_lock() > > and the device PM QoS initialization and destruction should be done > > under device_pm_lock() as well. > Ok that makes sense. > The constraints_state field can be replaced by a combination of > dev->power.constraints and list_empty(&dev->power.entry), which makes > the code more compact and less redundant. > > > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > > --- > > drivers/base/power/main.c | 4 - > > drivers/base/power/qos.c | 167 ++++++++++++++++++++++++++-------------------- > > include/linux/pm.h | 8 -- > > include/linux/pm_qos.h | 3 > > 4 files changed, 101 insertions(+), 81 deletions(-) > > > > Index: linux/drivers/base/power/qos.c > > =================================================================== > > --- linux.orig/drivers/base/power/qos.c > > +++ linux/drivers/base/power/qos.c > > @@ -30,15 +30,6 @@ > ... > > > > > @@ -178,8 +202,8 @@ void dev_pm_qos_constraints_destroy(stru > > * > > * Returns 1 if the aggregated constraint value has changed, > > * 0 if the aggregated constraint value has not changed, > > - * -EINVAL in case of wrong parameters, -ENODEV if the device has been > > - * removed from the system > > + * -EINVAL in case of wrong parameters, -ENOMEM if there's not enough memory > > + * to allocate for data structures. > Why not use -ENODEV in case there is no device? I don't think it's useful for the caller. If the device is gone, the constraing simply doesn't matter, so there's no error to handle. > > */ > > int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req, > > s32 value) > > @@ -195,28 +219,35 @@ int dev_pm_qos_add_request(struct device > > return -EINVAL; > > } > > > > - mutex_lock(&dev_pm_qos_mtx); > > req->dev = dev; > > > > - /* Return if the device has been removed */ > > - if (req->dev->power.constraints_state == DEV_PM_QOS_NO_DEVICE) { > > - ret = -ENODEV; > > - goto out; > > - } > > + device_pm_lock(); > > + mutex_lock(&dev_pm_qos_mtx); > > > > - /* > > - * Allocate the constraints data on the first call to add_request, > > - * i.e. only if the data is not already allocated and if the device has > > - * not been removed > > - */ > > - if (dev->power.constraints_state == DEV_PM_QOS_DEVICE_PRESENT) > > - ret = dev_pm_qos_constraints_allocate(dev); > > + if (dev->power.constraints) { > > + device_pm_unlock(); > > + } else { > > + if (list_empty(&dev->power.entry)) { > > + /* The device has been removed from the system. */ > > + device_pm_unlock(); > > + goto out; > 0 is silently returned in case the device has been removed. Is that > the intention? Pretty much it is. Is that a problem? Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 2, 2011 at 12:07 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > Hi, > > On Thursday, September 01, 2011, Jean Pihet wrote: >> Hi Rafael, >> >> On Wed, Aug 31, 2011 at 12:21 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> > From: Rafael J. Wysocki <rjw@sisk.pl> >> > >> > To read the current PM QoS value for a given device we need to >> > make sure that the device's power.constraints object won't be >> > removed while we're doing that. For this reason, put the >> > operation under dev->power.lock and acquire the lock >> > around the initialization and removal of power.constraints. >> Ok. >> >> > Moreover, since we're using the value of power.constraints to >> > determine whether or not the object is present, the >> > power.constraints_state field isn't necessary any more and may be >> > removed. However, dev_pm_qos_add_request() needs to check if the >> > device is being removed from the system before allocating a new >> > PM QoS constraints object for it, so it has to use device_pm_lock() >> > and the device PM QoS initialization and destruction should be done >> > under device_pm_lock() as well. >> Ok that makes sense. >> The constraints_state field can be replaced by a combination of >> dev->power.constraints and list_empty(&dev->power.entry), which makes >> the code more compact and less redundant. >> >> > >> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> >> > --- >> > drivers/base/power/main.c | 4 - >> > drivers/base/power/qos.c | 167 ++++++++++++++++++++++++++-------------------- >> > include/linux/pm.h | 8 -- >> > include/linux/pm_qos.h | 3 >> > 4 files changed, 101 insertions(+), 81 deletions(-) >> > >> > Index: linux/drivers/base/power/qos.c >> > =================================================================== >> > --- linux.orig/drivers/base/power/qos.c >> > +++ linux/drivers/base/power/qos.c >> > @@ -30,15 +30,6 @@ >> ... >> >> > >> > @@ -178,8 +202,8 @@ void dev_pm_qos_constraints_destroy(stru >> > * >> > * Returns 1 if the aggregated constraint value has changed, >> > * 0 if the aggregated constraint value has not changed, >> > - * -EINVAL in case of wrong parameters, -ENODEV if the device has been >> > - * removed from the system >> > + * -EINVAL in case of wrong parameters, -ENOMEM if there's not enough memory >> > + * to allocate for data structures. >> Why not use -ENODEV in case there is no device? > > I don't think it's useful for the caller. If the device is gone, the > constraing simply doesn't matter, so there's no error to handle. > >> > */ >> > int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req, >> > s32 value) >> > @@ -195,28 +219,35 @@ int dev_pm_qos_add_request(struct device >> > return -EINVAL; >> > } >> > >> > - mutex_lock(&dev_pm_qos_mtx); >> > req->dev = dev; >> > >> > - /* Return if the device has been removed */ >> > - if (req->dev->power.constraints_state == DEV_PM_QOS_NO_DEVICE) { >> > - ret = -ENODEV; >> > - goto out; >> > - } >> > + device_pm_lock(); >> > + mutex_lock(&dev_pm_qos_mtx); >> > >> > - /* >> > - * Allocate the constraints data on the first call to add_request, >> > - * i.e. only if the data is not already allocated and if the device has >> > - * not been removed >> > - */ >> > - if (dev->power.constraints_state == DEV_PM_QOS_DEVICE_PRESENT) >> > - ret = dev_pm_qos_constraints_allocate(dev); >> > + if (dev->power.constraints) { >> > + device_pm_unlock(); >> > + } else { >> > + if (list_empty(&dev->power.entry)) { >> > + /* The device has been removed from the system. */ >> > + device_pm_unlock(); >> > + goto out; >> 0 is silently returned in case the device has been removed. Is that >> the intention? > > Pretty much it is. Is that a problem? I think the caller needs to know if the constraint has been applied correctly or not. That is why I changed the API functions to int. Note: I still need to come with an API documentation patch after the code is settled down. > > Rafael > Thanks, Jean -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday, September 02, 2011, Jean Pihet wrote: > On Fri, Sep 2, 2011 at 12:07 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > Hi, > > > > On Thursday, September 01, 2011, Jean Pihet wrote: > >> Hi Rafael, > >> > >> On Wed, Aug 31, 2011 at 12:21 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > >> > From: Rafael J. Wysocki <rjw@sisk.pl> > >> > > >> > To read the current PM QoS value for a given device we need to > >> > make sure that the device's power.constraints object won't be > >> > removed while we're doing that. For this reason, put the > >> > operation under dev->power.lock and acquire the lock > >> > around the initialization and removal of power.constraints. > >> Ok. > >> > >> > Moreover, since we're using the value of power.constraints to > >> > determine whether or not the object is present, the > >> > power.constraints_state field isn't necessary any more and may be > >> > removed. However, dev_pm_qos_add_request() needs to check if the > >> > device is being removed from the system before allocating a new > >> > PM QoS constraints object for it, so it has to use device_pm_lock() > >> > and the device PM QoS initialization and destruction should be done > >> > under device_pm_lock() as well. > >> Ok that makes sense. > >> The constraints_state field can be replaced by a combination of > >> dev->power.constraints and list_empty(&dev->power.entry), which makes > >> the code more compact and less redundant. > >> > >> > > >> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > >> > --- > >> > drivers/base/power/main.c | 4 - > >> > drivers/base/power/qos.c | 167 ++++++++++++++++++++++++++-------------------- > >> > include/linux/pm.h | 8 -- > >> > include/linux/pm_qos.h | 3 > >> > 4 files changed, 101 insertions(+), 81 deletions(-) > >> > > >> > Index: linux/drivers/base/power/qos.c > >> > =================================================================== > >> > --- linux.orig/drivers/base/power/qos.c > >> > +++ linux/drivers/base/power/qos.c > >> > @@ -30,15 +30,6 @@ > >> ... > >> > >> > > >> > @@ -178,8 +202,8 @@ void dev_pm_qos_constraints_destroy(stru > >> > * > >> > * Returns 1 if the aggregated constraint value has changed, > >> > * 0 if the aggregated constraint value has not changed, > >> > - * -EINVAL in case of wrong parameters, -ENODEV if the device has been > >> > - * removed from the system > >> > + * -EINVAL in case of wrong parameters, -ENOMEM if there's not enough memory > >> > + * to allocate for data structures. > >> Why not use -ENODEV in case there is no device? > > > > I don't think it's useful for the caller. If the device is gone, the > > constraing simply doesn't matter, so there's no error to handle. > > > >> > */ > >> > int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req, > >> > s32 value) > >> > @@ -195,28 +219,35 @@ int dev_pm_qos_add_request(struct device > >> > return -EINVAL; > >> > } > >> > > >> > - mutex_lock(&dev_pm_qos_mtx); > >> > req->dev = dev; > >> > > >> > - /* Return if the device has been removed */ > >> > - if (req->dev->power.constraints_state == DEV_PM_QOS_NO_DEVICE) { > >> > - ret = -ENODEV; > >> > - goto out; > >> > - } > >> > + device_pm_lock(); > >> > + mutex_lock(&dev_pm_qos_mtx); > >> > > >> > - /* > >> > - * Allocate the constraints data on the first call to add_request, > >> > - * i.e. only if the data is not already allocated and if the device has > >> > - * not been removed > >> > - */ > >> > - if (dev->power.constraints_state == DEV_PM_QOS_DEVICE_PRESENT) > >> > - ret = dev_pm_qos_constraints_allocate(dev); > >> > + if (dev->power.constraints) { > >> > + device_pm_unlock(); > >> > + } else { > >> > + if (list_empty(&dev->power.entry)) { > >> > + /* The device has been removed from the system. */ > >> > + device_pm_unlock(); > >> > + goto out; > >> 0 is silently returned in case the device has been removed. Is that > >> the intention? > > > > Pretty much it is. Is that a problem? > I think the caller needs to know if the constraint has been applied > correctly or not. However, the removal of the device is a special case. What would the caller be supposed to do when it learned that the device had been removed while it had been trying to add a QoS constraing for it? Not much I guess. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rafael, On Sat, Sep 3, 2011 at 1:55 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Friday, September 02, 2011, Jean Pihet wrote: >> On Fri, Sep 2, 2011 at 12:07 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: ... >> >> > */ >> >> > int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req, >> >> > s32 value) >> >> > @@ -195,28 +219,35 @@ int dev_pm_qos_add_request(struct device >> >> > return -EINVAL; >> >> > } >> >> > >> >> > - mutex_lock(&dev_pm_qos_mtx); >> >> > req->dev = dev; >> >> > >> >> > - /* Return if the device has been removed */ >> >> > - if (req->dev->power.constraints_state == DEV_PM_QOS_NO_DEVICE) { >> >> > - ret = -ENODEV; >> >> > - goto out; >> >> > - } >> >> > + device_pm_lock(); >> >> > + mutex_lock(&dev_pm_qos_mtx); >> >> > >> >> > - /* >> >> > - * Allocate the constraints data on the first call to add_request, >> >> > - * i.e. only if the data is not already allocated and if the device has >> >> > - * not been removed >> >> > - */ >> >> > - if (dev->power.constraints_state == DEV_PM_QOS_DEVICE_PRESENT) >> >> > - ret = dev_pm_qos_constraints_allocate(dev); >> >> > + if (dev->power.constraints) { >> >> > + device_pm_unlock(); >> >> > + } else { >> >> > + if (list_empty(&dev->power.entry)) { >> >> > + /* The device has been removed from the system. */ >> >> > + device_pm_unlock(); >> >> > + goto out; >> >> 0 is silently returned in case the device has been removed. Is that >> >> the intention? >> > >> > Pretty much it is. Is that a problem? >> I think the caller needs to know if the constraint has been applied >> correctly or not. > > However, the removal of the device is a special case. What would the caller > be supposed to do when it learned that the device had been removed while it had > been trying to add a QoS constraing for it? Not much I guess. When the device is removed from the system the constraints list id flushed and the requests are zero'ed using memset. In that case the caller needs to be aware of that otherwise the next call to the API will throw an error. > > Thanks, > Rafael > Thanks, Jean -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: linux/drivers/base/power/qos.c =================================================================== --- linux.orig/drivers/base/power/qos.c +++ linux/drivers/base/power/qos.c @@ -30,15 +30,6 @@ * . To minimize the data usage by the per-device constraints, the data struct * is only allocated at the first call to dev_pm_qos_add_request. * . The data is later free'd when the device is removed from the system. - * . The constraints_state variable from dev_pm_info tracks the data struct - * allocation state: - * DEV_PM_QOS_NO_DEVICE: No device present or device removed, no data - * allocated, - * DEV_PM_QOS_DEVICE_PRESENT: Device present, data not allocated and will be - * allocated at the first call to dev_pm_qos_add_request, - * DEV_PM_QOS_ALLOCATED: Device present, data allocated. The per-device - * PM QoS constraints framework is operational and constraints can be - * added, updated or removed using the dev_pm_qos_* API. * . A global mutex protects the constraints users from the data being * allocated and free'd. */ @@ -51,8 +42,30 @@ static DEFINE_MUTEX(dev_pm_qos_mtx); + static BLOCKING_NOTIFIER_HEAD(dev_pm_notifiers); +/** + * dev_pm_qos_read_value - Get PM QoS constraint for a given device. + * @dev: Device to get the PM QoS constraint value for. + */ +s32 dev_pm_qos_read_value(struct device *dev) +{ + struct pm_qos_constraints *c; + unsigned long flags; + s32 ret = 0; + + spin_lock_irqsave(&dev->power.lock, flags); + + c = dev->power.constraints; + if (c) + ret = pm_qos_read_value(c); + + spin_unlock_irqrestore(&dev->power.lock, flags); + + return ret; +} + /* * apply_constraint * @req: constraint request to apply @@ -105,27 +118,37 @@ static int dev_pm_qos_constraints_alloca } BLOCKING_INIT_NOTIFIER_HEAD(n); + plist_head_init(&c->list); + c->target_value = PM_QOS_DEV_LAT_DEFAULT_VALUE; + c->default_value = PM_QOS_DEV_LAT_DEFAULT_VALUE; + c->type = PM_QOS_MIN; + c->notifiers = n; + + spin_lock_irq(&dev->power.lock); dev->power.constraints = c; - plist_head_init(&dev->power.constraints->list); - dev->power.constraints->target_value = PM_QOS_DEV_LAT_DEFAULT_VALUE; - dev->power.constraints->default_value = PM_QOS_DEV_LAT_DEFAULT_VALUE; - dev->power.constraints->type = PM_QOS_MIN; - dev->power.constraints->notifiers = n; - dev->power.constraints_state = DEV_PM_QOS_ALLOCATED; + spin_unlock_irq(&dev->power.lock); return 0; } +static void __dev_pm_qos_constraints_init(struct device *dev) +{ + spin_lock_irq(&dev->power.lock); + dev->power.constraints = NULL; + spin_unlock_irq(&dev->power.lock); +} + /** - * dev_pm_qos_constraints_init + * dev_pm_qos_constraints_init - Initalize device's PM QoS constraints pointer. * @dev: target device * - * Called from the device PM subsystem at device insertion + * Called from the device PM subsystem at device insertion under + * device_pm_lock(). */ void dev_pm_qos_constraints_init(struct device *dev) { mutex_lock(&dev_pm_qos_mtx); - dev->power.constraints_state = DEV_PM_QOS_DEVICE_PRESENT; + dev->power.constraints = NULL; mutex_unlock(&dev_pm_qos_mtx); } @@ -133,34 +156,35 @@ void dev_pm_qos_constraints_init(struct * dev_pm_qos_constraints_destroy * @dev: target device * - * Called from the device PM subsystem at device removal + * Called from the device PM subsystem at device removal under device_pm_lock(). */ void dev_pm_qos_constraints_destroy(struct device *dev) { struct dev_pm_qos_request *req, *tmp; + struct pm_qos_constraints *c; mutex_lock(&dev_pm_qos_mtx); - if (dev->power.constraints_state == DEV_PM_QOS_ALLOCATED) { - /* Flush the constraints list for the device */ - plist_for_each_entry_safe(req, tmp, - &dev->power.constraints->list, - node) { - /* - * Update constraints list and call the notification - * callbacks if needed - */ - apply_constraint(req, PM_QOS_REMOVE_REQ, - PM_QOS_DEFAULT_VALUE); - memset(req, 0, sizeof(*req)); - } + c = dev->power.constraints; + if (!c) + goto out; - kfree(dev->power.constraints->notifiers); - kfree(dev->power.constraints); - dev->power.constraints = NULL; + /* Flush the constraints list for the device */ + plist_for_each_entry_safe(req, tmp, &c->list, node) { + /* + * Update constraints list and call the notification + * callbacks if needed + */ + apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE); + memset(req, 0, sizeof(*req)); } - dev->power.constraints_state = DEV_PM_QOS_NO_DEVICE; + __dev_pm_qos_constraints_init(dev); + + kfree(c->notifiers); + kfree(c); + + out: mutex_unlock(&dev_pm_qos_mtx); } @@ -178,8 +202,8 @@ void dev_pm_qos_constraints_destroy(stru * * Returns 1 if the aggregated constraint value has changed, * 0 if the aggregated constraint value has not changed, - * -EINVAL in case of wrong parameters, -ENODEV if the device has been - * removed from the system + * -EINVAL in case of wrong parameters, -ENOMEM if there's not enough memory + * to allocate for data structures. */ int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req, s32 value) @@ -195,28 +219,35 @@ int dev_pm_qos_add_request(struct device return -EINVAL; } - mutex_lock(&dev_pm_qos_mtx); req->dev = dev; - /* Return if the device has been removed */ - if (req->dev->power.constraints_state == DEV_PM_QOS_NO_DEVICE) { - ret = -ENODEV; - goto out; - } + device_pm_lock(); + mutex_lock(&dev_pm_qos_mtx); - /* - * Allocate the constraints data on the first call to add_request, - * i.e. only if the data is not already allocated and if the device has - * not been removed - */ - if (dev->power.constraints_state == DEV_PM_QOS_DEVICE_PRESENT) - ret = dev_pm_qos_constraints_allocate(dev); + if (dev->power.constraints) { + device_pm_unlock(); + } else { + if (list_empty(&dev->power.entry)) { + /* The device has been removed from the system. */ + device_pm_unlock(); + goto out; + } else { + device_pm_unlock(); + /* + * Allocate the constraints data on the first call to + * add_request, i.e. only if the data is not already + * allocated and if the device has not been removed. + */ + ret = dev_pm_qos_constraints_allocate(dev); + } + } if (!ret) ret = apply_constraint(req, PM_QOS_ADD_REQ, value); -out: + out: mutex_unlock(&dev_pm_qos_mtx); + return ret; } EXPORT_SYMBOL_GPL(dev_pm_qos_add_request); @@ -252,13 +283,13 @@ int dev_pm_qos_update_request(struct dev mutex_lock(&dev_pm_qos_mtx); - if (req->dev->power.constraints_state == DEV_PM_QOS_ALLOCATED) { + if (req->dev->power.constraints) { if (new_value != req->node.prio) ret = apply_constraint(req, PM_QOS_UPDATE_REQ, new_value); } else { /* Return if the device has been removed */ - ret = -ENODEV; + ret = -EINVAL; } mutex_unlock(&dev_pm_qos_mtx); @@ -293,7 +324,7 @@ int dev_pm_qos_remove_request(struct dev mutex_lock(&dev_pm_qos_mtx); - if (req->dev->power.constraints_state == DEV_PM_QOS_ALLOCATED) { + if (req->dev->power.constraints) { ret = apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE); memset(req, 0, sizeof(*req)); @@ -323,15 +354,12 @@ int dev_pm_qos_add_notifier(struct devic mutex_lock(&dev_pm_qos_mtx); - /* Silently return if the device has been removed */ - if (dev->power.constraints_state != DEV_PM_QOS_ALLOCATED) - goto out; - - retval = blocking_notifier_chain_register( - dev->power.constraints->notifiers, - notifier); + /* Silently return if the constraints object is not present. */ + if (dev->power.constraints) + retval = blocking_notifier_chain_register( + dev->power.constraints->notifiers, + notifier); -out: mutex_unlock(&dev_pm_qos_mtx); return retval; } @@ -354,15 +382,12 @@ int dev_pm_qos_remove_notifier(struct de mutex_lock(&dev_pm_qos_mtx); - /* Silently return if the device has been removed */ - if (dev->power.constraints_state != DEV_PM_QOS_ALLOCATED) - goto out; - - retval = blocking_notifier_chain_unregister( - dev->power.constraints->notifiers, - notifier); + /* Silently return if the constraints object is not present. */ + if (dev->power.constraints) + retval = blocking_notifier_chain_unregister( + dev->power.constraints->notifiers, + notifier); -out: mutex_unlock(&dev_pm_qos_mtx); return retval; } Index: linux/include/linux/pm_qos.h =================================================================== --- linux.orig/include/linux/pm_qos.h +++ linux/include/linux/pm_qos.h @@ -77,6 +77,7 @@ int pm_qos_remove_notifier(int pm_qos_cl int pm_qos_request_active(struct pm_qos_request *req); s32 pm_qos_read_value(struct pm_qos_constraints *c); +s32 dev_pm_qos_read_value(struct device *dev); int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req, s32 value); int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value); @@ -117,6 +118,8 @@ static inline int pm_qos_request_active( static inline s32 pm_qos_read_value(struct pm_qos_constraints *c) { return 0; } +static inline s32 dev_pm_qos_read_value(struct device *dev) + { return 0; } static inline int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req, s32 value) Index: linux/drivers/base/power/main.c =================================================================== --- linux.orig/drivers/base/power/main.c +++ linux/drivers/base/power/main.c @@ -98,8 +98,8 @@ void device_pm_add(struct device *dev) dev_warn(dev, "parent %s should not be sleeping\n", dev_name(dev->parent)); list_add_tail(&dev->power.entry, &dpm_list); - mutex_unlock(&dpm_list_mtx); dev_pm_qos_constraints_init(dev); + mutex_unlock(&dpm_list_mtx); } /** @@ -110,9 +110,9 @@ void device_pm_remove(struct device *dev { pr_debug("PM: Removing info for %s:%s\n", dev->bus ? dev->bus->name : "No Bus", dev_name(dev)); - dev_pm_qos_constraints_destroy(dev); complete_all(&dev->power.completion); mutex_lock(&dpm_list_mtx); + dev_pm_qos_constraints_destroy(dev); list_del_init(&dev->power.entry); mutex_unlock(&dpm_list_mtx); device_wakeup_disable(dev); Index: linux/include/linux/pm.h =================================================================== --- linux.orig/include/linux/pm.h +++ linux/include/linux/pm.h @@ -421,13 +421,6 @@ enum rpm_request { RPM_REQ_RESUME, }; -/* Per-device PM QoS constraints data struct state */ -enum dev_pm_qos_state { - DEV_PM_QOS_NO_DEVICE, /* No device present */ - DEV_PM_QOS_DEVICE_PRESENT, /* Device present, data not allocated */ - DEV_PM_QOS_ALLOCATED, /* Device present, data allocated */ -}; - struct wakeup_source; struct pm_domain_data { @@ -489,7 +482,6 @@ struct dev_pm_info { #endif struct pm_subsys_data *subsys_data; /* Owned by the subsystem. */ struct pm_qos_constraints *constraints; - enum dev_pm_qos_state constraints_state; }; extern void update_pm_runtime_accounting(struct device *dev);