Kevin Hilman had written, on 10/05/2010 03:50 PM, the following:
"Rafael J. Wysocki" <rjw@xxxxxxx> 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
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.
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
