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

[Saravana Kannan <skannan@codeaurora.org>]

On 02/01/2011 07:28 AM, Russell King - ARM Linux wrote:
> 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.

This code will only catch the error when it actually happens and will 
even miss catching some of them (if timed right -- unprepare happens in 
the other core after this check is executed).

I really wish there was something better we could do to help driver devs 
catch errors of calling enable without calling prepare(). Some thing 
like spin lock debug, or the might_sleeps() inside mutexes, etc.

Hmm... Jeremy, how about doing a similar check in the unprepare code? 
You could WARN/BUG ON the prepare count going to zero when the enable 
count is still non-zero?

Thanks,
Saravana

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.