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

[Jean Pihet]

Hi Rafael,

On Thu, Sep 29, 2011 at 10:33 PM, Rafael J. Wysocki <rjw@xxxxxxx> wrote:
> Hi,
>
> On Thursday, September 29, 2011, Jean Pihet wrote:
>> Hi Rafael,
>>
>> I have a few minor remarks, inlined below.
>
> In the meantime it turned out that the patch caused a build failure
> for CONFIG_PM_SLEEP unset (your original patches won't really work
> in this case too), so I posted a patch to fix it.  However, since
> you had remarks anyway, I've simply folded the fix into the next
> version of the $subject patch, which is appended.
OK to me.

I have 2 remarks though:
- some drivers are using the power_state field in the suspend/resume
operation, IIUC without conflict. Can you confirm?
- the power_state field is scheduled for removal, cf.
Documentation/feature-removal-schedule.txt. That does not look
correct.

...
>> > +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.
>
> This function was used only once, so it wasn't really necessary.  I simply
> removed it.
>
> Thanks,
> Rafael
>
> ---
> From: Rafael J. Wysocki <rjw@xxxxxxx>
> Subject: PM / QoS: Add function dev_pm_qos_read_value() (v3)
>
> 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 make it use the
> power.power_state field of struct device for this purpose.
>
> Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx>

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/