[PATCH V3] PM / Domains: Remove intermediate states from the power off sequence

Ulf Hansson ulf.hansson at linaro.org
Thu Jun 25 01:09:35 PDT 2015


On 24 June 2015 at 19:57, Kevin Hilman <khilman at kernel.org> wrote:
> Ulf Hansson <ulf.hansson at linaro.org> writes:
>
> [...]
>
>>> Minor nit: I think you should leave these 3 helpers and just simplify
>>> them.  It will make the changes below easier to read as well.
>>
>> I would rather like to remove them. The reason is to create consistency.
>>
>> For the locking part, there are currently mixtures of
>> mutex_lock|unlock() and genpd_acquire|release_lock(). Following your
>> suggestion will leave around 6-7 places where mutex_lock() will remain
>> used (additionally for mutex_unlock()). So removing the helper
>> functions creates a consistent behaviour.
>
> OK, that makes more sense.
>
>> For the genpd->status, it's currently being assigned at various places
>> without using a helper function. Again I wanted to create a consistent
>> behaviour and make the code more readable.
>>
>>>
>>> Also, for the locking, Lina will be adding these locking functions back
>>> anyways, so let's just avoid the extra churn.
>>
>> Actually I think it becomes more evident what Lina's patchset does if
>> she re-introduces an API to deal with the locking. Moreover we can
>> "force" that patchset to not break consistently around the locking.
>>
>>>
>>> Otherwise, I think this version is looking really good.
>>>
>>> With the above tweaks, feel free to add
>>>
>>> Reviewed-by: Kevin Hilman <khilman at linaro.org>
>>>
>>> Kevin
>>
>> Thanks a lot for reviewing!
>>
>> If you have a strong opinion about your suggestions, I will happily
>> adapt to them, please let me know.
>
> No strong option, you convinced me your way will actually make things
> more consistent, and Lina prefers them gone as well, so I'm fine with
> them gone (as you've done in V4).
>
> Kevin
>
>

Kevin, thanks for your review!

I haven't applied your Reviewed-by tag to V4, as there where one more
minor additional change added, regarding IRQ safe devices. I
appreciate if could have a look, whenever you have some time.

Kind regards
Uffe



More information about the linux-arm-kernel mailing list