Tony Lindgren wrote:
* Kevin Hilman <khilman@xxxxxxxxxx> [080604 09:37]:
Paul Walmsley wrote:
Hi Kevin,
On Tue, 20 May 2008, Kevin Hilman wrote:
> The locking in the get_rate() hook is unnecessary, and causes problems
> when used with the -rt patch, since it may be called recursively.
>
> Signed-off-by: Kevin Hilman <khilman@xxxxxxxxxx>
Acked-by: Paul Walmsley <paul@xxxxxxxxx>
BTW, looks like we could probably get rid of some of the other spinlocking
in plat-omap/clock.c also -- clk_get_usecount() ?
I agree, most of the locking in there seems unnecessary, but this was
the only one that could be removed without looking closer at at all the
platform specific clock code.
Basically, I think there minimal (maybe no) locking in the generic
layer, and the locking should be done if necessary in the platform
specific parts.
To me it seems locking is needed. What if something is changing a clock's
rate and then something else calls get_rate() while the other thread is
changing the rate?
Then the caller of get_rate() has an out of date value.
The same can happen even with the locking. If someone asks for the
rate, and then another thread changes the rate right after, the thread
who asked for the rate has an out-of-date value.
That being said, there may be some locks that need to stay, I haven't
done a thorough analysis of this. I just think that there is currently
to much unnecessary locking.
If get_rate() is not locking, get_rate() may get invalid rate between
something else programming the hardware and changing clk->rate. I guess
just locking set_rate() would still work with local irq disabled,
but would not be SMP safe.
Anyways, ideally the spin_lock would be specific to the clocks accessing
the same clock registers... But that would mean adding yet another field
to struct clk or clock domains. Anybody got better ideas?
And of course anything going through the clocks list needs to be protected
by the mutex.
Agreed, the mutex needs to stay.
Kevin
--
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
