[PATCH v5] power: introduce library for device-specific OPPs
nm at ti.com
Tue Oct 5 17:03:44 EDT 2010
Kevin Hilman had written, on 10/05/2010 03:50 PM, the following:
> "Rafael J. Wysocki" <rjw at sisk.pl> writes:
>> On Tuesday, October 05, 2010, Nishanth Menon wrote:
>>> Rafael J. Wysocki had written, on 10/04/2010 05:36 PM, the following:
>>>> On Friday, October 01, 2010, Nishanth Menon wrote:
>>>> I'm not really sure why so many mutexes are needed here. I don't think you
>>>> need a separate mutex in every struct device_opp object. I'd just use
>>>> dev_opp_list_lock for everything.
>>> I did consider using dev_opp_list_lock to lock everything *but* here is
>>> the contention:
>>> dev_opp_list_lock locks modification for addition of domains device.
>>> This operation happens usually during init stage.
>>> each domain device has multiple opps, new opps can be added, but the
>>> more often usage will probably be opp_enable and disable. domain are
>>> usually modifiable independent of each other - device_opp->lock provides
>>> device level lock allowing for each domain device opp list to be
>>> modified independent of each other. e.g. on thermal overage we may
>>> choose to lower mpu domain while a coprocessor driver in parallel might
>>> choose to disable co-processor domain in parallel.
>>> Wondering why you'd like a single lock for all domains and restrict
>> Because of the simplicity, mostly. If there's only a relatively short period
>> when the lock will be contended for, that still is not too bad and it's much
>> easier to get the synchronization right with just one lock for starters.
> FWIW, I agree with Rafael
> These are not going be highly contended locks, and the lock durations
> are very short, so simplifying the locking is a big win for readability.
Fair enough. we can relook if the lock becomes a contended lock in the
future. I do agree that simplifying the locking will benefit
readability. Will post a v6 with a singular lock and updated
documentation for the same.
More information about the linux-arm-kernel