Re: [PATCH v5] power: introduce library for device-specific OPPs

[Kevin Hilman <khilman@deeprootsystems.com>]

"Rafael J. Wysocki" <rjw@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 
>> parallelization?
>
> 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.

Kevin