Re: [PATCH 5/5] ARM: OMAP: remove unnecessary locking in clk_get_rate()

[Tony Lindgren <tony@atomide.com>]

* Kevin Hilman <khilman@mvista.com> [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@mvista.com>
>>
>> Acked-by: Paul Walmsley <paul@pwsan.com>
>>
>> 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?

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.

Tony