Re: [PATCH 3/3] PM / QoS: Add function dev_pm_qos_read_value() (v2)

[Jean Pihet]

Hi Rafael,

I have a few minor remarks, inlined below.

On Sat, Sep 24, 2011 at 11:26 PM, Rafael J. Wysocki <rjw@xxxxxxx> wrote:
> From: Rafael J. Wysocki <rjw@xxxxxxx>
>
> 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.
>
> 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.
It might be good to mention dev_pm_qos_mtx which is used internally by
the per-device PM QoS code.

>
> Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx>
Other then the minor remarks:
Acked-by: Jean Pihet <j-pihet@xxxxxx>

> ---
>  drivers/base/power/main.c |    4 -
>  drivers/base/power/qos.c  |  168 ++++++++++++++++++++++++++--------------------
>  include/linux/pm.h        |    8 --
>  include/linux/pm_qos.h    |    3
>  4 files changed, 103 insertions(+), 80 deletions(-)
>
> Index: linux/drivers/base/power/qos.c
> ===================================================================
> --- linux.orig/drivers/base/power/qos.c
> +++ linux/drivers/base/power/qos.c
...

>
> +static void __dev_pm_qos_constraints_init(struct device *dev)
I would change the name or add a comment stating that this is an
internal function which reinits the constraints field under the lock.

Thanks & regards,
Jean
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/