Re: [PATCH 1/2] Add a common struct clk

From: Grant Likely
Date: Sun Jan 16 2011 - 02:27:40 EST


On Mon, Jan 10, 2011 at 5:54 PM, Jeremy Kerr <jeremy.kerr@xxxxxxxxxxxxx> wrote:
> Hi Russell,
>
>> Unless the locking problems can be resolved, the patches aren't ready.
>>
>> From what I've seen there's still quite a problem with what kind of
>> lock to use in the clock - mutex or spinlock.
>
> Yes, the clock driver may either use a spinlock or mutex.
>
> However, this exactly the same as the current clock code, my patches do not
> alter what we currently have.
>
> I do agree that we should define some specific semantics for the clock API
> with regards to sleeping, and I'll start a new thread about that. But we
> should definitely separate that issue from the problem of having multiple
> definitions for struct clk, which is what these patches address.

Given that each current struct clk implementation makes it's own
decisions about how to handle locking, would it not make sense to omit
locking entirely? At least as a first step? Let the clock ops
implement their own locking. Make enable count management their
responsibility too. That's all that the lock protects anyway. The
clk_* apis can fall directly through to the ops hooks with no
manipulation or locking. The way v3 of the patch worked.

Just having a common struct clk definition (without the locking) is
valuable on its own to get closer to supporting multiple platforms
with a single kernel on ARM. It certainly shouldn't be worse that the
current state. Locking consolidation can be implemented in follow-on
patches.

g.
--
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/