Re: Locking in the clk API, part 2: clk_prepare/clk_unprepare

[Jassi Brar]

2011/2/2 Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx>:
> On Tue, Feb 01, 2011 at 04:22:03PM +0100, Uwe Kleine-KÃnig wrote:
>> Full ack. Â(I wonder if you misunderstood me or wanted to put my
>> statement into more words. ÂJassi didn't like that a clk_enable without
>> a previous clk_prepare worked on some platforms and on others it
>> doesn't. ÂWith BUG_ON(clk->ops->prepare && !clk->prepare_count) in
>> clk_enable we have exactly this situation.)
>
> Even with a NULL clk->ops->prepare function, we still want drivers to
> have called clk_prepare(). ÂSo we can do something like:
>
> Â Â Â Âif (WARN_ON(clk->prepare_count == 0))
> Â Â Â Â Â Â Â Âreturn -EINVAL;
>
> in clk_enable() should be sufficient and noisy enough not to be missed.
>
> I'd avoid BUG_ON() here as that will take the system down, which may
> increase the chances of getting useful bug reports.

Having thought about it, I think it's not necessary to immediately catch
drivers that work on some platforms and not on others -- a mere comment
'please add clk_prepare' during code review or a patch adding 'clk_prepare'
later upon stumbling across a platform on which the driver doesn't work,
should be OK. Let us not fret about it.

That leaves us with only having to ensure that :-
a) No two calls to clk_prepare/unprepare _hooks_ are consecutive.
b) clk_prepare is done on the clock (not necessarily by the driver
    under consideration) before calls to clk_enable.

I think (a) is already easily managed by having the prepare_count,
and (b) can be reasonably managed by what Russell suggests above.

So, FWIW, I am for the idea.

Njoi!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/