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

[skannan@codeaurora.org (Saravana Kannan)]

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.