[PATCH RFC] clk: add support for automatic parent handling

Saravana Kannan
2011-05-03T02:44:24Z

On 04/29/2011 06:26 AM, Russell King - ARM Linux wrote:
> On Fri, Apr 29, 2011 at 02:13:06PM +0200, Thomas Gleixner wrote:
>> This are all well defined semantical problems and should be solved in
>> common code rather than having different buggy implementations in each
>> SoC clock "framework".
>
> Why are you associating functional elements in drivers/clk with the
> SoC clock framework?
>
> I think that's the root cause of this misunderstanding, and until you
> start realising that these building blocks are not part of the SoC
> implementation, we're going to continue arguing about this.

Ok, the discussion went down hill real quick after this point. So, I
will start from here.

I will try and propose what I think might be a reasonable compromise
based on what I have seen so far.

I think Thomas mostly cares about having the tree handling done at the
core code level. Russell's main point seems to be that the parent can
change for reasons other than "clk_set_parent()" and hence we can have
the core code manage all of the parent handling.

The parent of a clock can really change for only two reasons (based on
my experience with clocks -- in no way extensive, but not insignificant
either):
* clk_set_rate() -- where the set rate code takes care of selecting the
right parent for a given rate.
* clk_set_parent() -- where the "clk" is just a mux of a random bunch of
inputs and we just allow the caller to pick the mux input. The clock
framework might not even know what the rate/characteristic (jitter, duty
cycle, etc) of the input are since some of them could come from an
external source that's board specific.

A simple compromise might be to have the generic/base clk structure have
a "parent" field. Then the generic clk_set_parent() and clk_set_rate()
would grab the clock's mutex (currently named prepare_lock) before
calling the clk->ops->set_rate/set_parent. I already suggested grabbing
this mutex when Jeremy's patches were being review, but it was put off
for later/Soc specific code (forgot what the decision was). That's why
we see a might_sleep() in his clk_set_rate().

With the above suggestion, the Soc specific code can change the parent
all it wants and the core code can still do the tree handling in
clk_prepare/clk_unprepare (they already grab the mutex).

Although this might not be what each side wants, I think this is a
reasonable compromise.

Thomas/Russell, Does it sound acceptable to both of you?

We can go to the per-clock vs. per-tree locking after we can agree on
this. My current opinion (can change easily) is that while per-tree
locking might be nice, it might be hard/ugly to capture it in the data
structures. Per-clock locking will allow the soc-specific code which has
a better idea of the tree to do per-tree locking internally while
allowing the core code to handle propagating enable/disable up the tree.

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.