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

2011/2/2 Russell King - ARM Linux [off-list ref]:

On Tue, Feb 01, 2011 at 04:22:03PM +0100, Uwe Kleine-König wrote:

quoted

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!