On 03/20/2012 04:53 PM, Turquette, Mike wrote:

On Tue, Mar 20, 2012 at 10:46 AM, Saravana Kannan
[off-list ref]  wrote:

quoted

On Tue, March 20, 2012 7:02 am, Shawn Guo wrote:

quoted

On Thu, Mar 15, 2012 at 11:11:19PM -0700, Mike Turquette wrote:
...

quoted

+struct clk_ops {
+    int             (*prepare)(struct clk_hw *hw);
+    void            (*unprepare)(struct clk_hw *hw);
+    int             (*enable)(struct clk_hw *hw);
+    void            (*disable)(struct clk_hw *hw);
+    int             (*is_enabled)(struct clk_hw *hw);
+    unsigned long   (*recalc_rate)(struct clk_hw *hw,
+                                    unsigned long parent_rate);

I believe I have heard people love the interface with parent_rate
passed in.  I love that too.  But I would like to ask the same thing
on .round_rate and .set_rate as well for the same reason why we have
it for .recalc_rate.

In my case, for most clocks, set rate involves reparenting. So, what does
passing parent_rate for these even mean? Passing parent_rate seems more
apt for recalc_rate since it's called when the parent rate changes -- so,
the actual parent itself is not expected to change.

 From my conversations with folks across many platforms, I think that
the way your clock tree expects to change rates is the exception, not
the rule.  As such you should just ignore the parent_rate parameter as
it useless to you.

quoted

I could ignore the parameter, but just wondering how many of the others
see value in this. And if we do add this parameter, it shouldn't be made
mandatory for the platform driver to use it (due to other assumptions the
clock framework might make).

 From my rough census of folks that actually need .set_rate support, I
think that everyone except MSM could benefit from this.  Your concept
of clk_set_rate is everyone else's clk_set_parent.

To clarify, MSM's set parent is a subset of steps needed to be done to 
finish set parent. So, it's not that we just randomly decided to swap 
the meanings of these two functions.

Also, I think don't think the difference in view of set_rate is due to 
the difference in the clock trees between platforms with complicated 
trees. I think it's more because of who actually decides the rates for 
the clock tree. It looks like some platforms pick a top-down approach to 
deciding the rates of the clock tre while others pick a bottom-up approach.

Ignoring the new parameter should cause you no harm.

As long as this is guaranteed, I have no problems with Shawn's suggestion.

It does make me
wonder if it would be a good idea to pass in the parent rate for
.set_parent, which is analogous to .set_rate in many ways.

I need to think a bit more about this.

-Saravana

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