On 03/20/2012 04:53 PM, Turquette, Mike wrote:
On Tue, Mar 20, 2012 at 10:46 AM, Saravana Kannan skannan@codeaurora.org wrote:
On Tue, March 20, 2012 7:02 am, Shawn Guo wrote:
On Thu, Mar 15, 2012 at 11:11:19PM -0700, Mike Turquette wrote: ...
+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.
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