[lkml]   [2011]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: Locking in the clk API, part 2: clk_prepare/clk_unprepare
On Tue, Feb 01, 2011 at 11:54:49AM +0100, Uwe Kleine-König wrote:
> Alternatively don't force the sleep in clk_prepare (e.g. by protecting
> prepare_count by a spinlock (probably enable_lock)) and call clk_prepare
> before calling clk->ops->enable?

That's a completely bad idea. I assume you haven't thought about this
very much.

There's two ways I can think of doing what you're suggesting:

int clk_prepare(struct clk *clk)
unsigned long flags;
int ret = 0;


spin_lock_irqsave(&clk->enable_lock, flags);
if (clk->prepare_count++ == 0)
ret = clk->ops->prepare(clk);
spin_unlock_irqrestore(&clk->enable_clock, flags);

return ret;

The problem is that clk->ops->prepare() is called in a non-sleepable
context. So this breaks the whole idea of clk_prepare(), and so isn't
a solution.

The other solution is:

int clk_prepare(struct clk *clk)
unsigned long flags;
int ret = 0;
bool first;


spin_lock_irqsave(clk->enable_lock, flags);
first = clk->prepare_count++ == 0;
spin_unlock_irqrestore(clk->enable_clock, flags);

if (first)
ret = clk->ops->prepare(clk);

return ret;

The problem with this is that you now don't have any sane locking on
the prepare callback, and the circumstances under which it's called
are very indefinite. For example, consider a preempt-enabled system:

thread 1 thread 2 prepare_count
clk_prepare 0
clk->prepare_count++ 1
<thread switch>
clk_prepare 1
clk->prepare_count++ 2
clk_prepare returns 2

clk_enable 2
<explodes as clock is not prepared>
<thread switch>

So really, what you're suggesting is completely broken.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

 \ /
  Last update: 2011-02-01 14:19    [W:0.501 / U:7.440 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site