* 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? 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 -- 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
