On Mon, Nov 21, 2011 at 05:40:42PM -0800, Mike Turquette wrote: [...]
.the most notable change is the removal of struct clk_hw.
Happy to see that.
This extra layer of abstraction is only necessary if we want hide the definition of struct clk from platform code. Many developers expressed the need to know some details of the generic struct clk in the platform layer, and rightly so. Now struct clk is defined in include/linux/clk.h, protected by #ifdef CONFIG_GENERIC_CLK.
.flags have been introduced to struct clk, with several of them defined and used in the common code. These flags protect against changing clk rates or switching the clk parent while that clk is enabled; another flag is used to signal to clk_set_rate that it should ask the parent to change it's rate too.
.speaking of which, clk_set_rate has been overhauled and is now recursive. *collective groan*. clk_set_rate is still simple for the common case of simply setting a single clk's rate. But if your clk has the CLK_PARENT_SET_RATE flag and the .round_rate callback recommends changing the parent rate, then clk_set_rate will recurse upwards to the parent and try it all over again. In the event of a failure everything unwinds and all the clks go out for drinks.
I smell that this will be the part which makes the whole series risky of being accepted in a desired time frame, with saying rate change notifications are still missing for now.
.clk_register has been replaced by clk_init, which does NOT allocate memory for you. Platforms should allocate their own clk_hw_whatever structure which contains struct clk. clk_init is still necessary to initialize struct clk internals. clk_init also accepts struct device *dev as an argument, but does nothing with it. This is in anticipation of device tree support.
I would say that we do not necessarily need to have 'struct device' for each clk (for imx6 example, we have 110 clks, and I heard some omap support has 225 clks?). The device tree support can work out everything it needs from the 'struct device_node', which can be a clock blob constraining multiple clks (thanks to 'clock-cells'). That said you may want to add the 'dev' argument for other reasons, but I never used it when converting imx6 clock to device tree.
.Documentation! I'm sure somebody reads it.
.sysfs support. Visualize your clk tree at /sys/clk! Where would be a better place to put the clk tree besides the root of /sys/? When a consensus on this is reached I'll submit the proper changes to Documentation/ABI/testing/.
What's missing?
.per tree locking. I implemented this at the Linaro Connect conference but the implementation was unpopular, so it didn't make the cut. There needs to be better understanding of everyone's needs for this to work.
.rate change notifications. I simply didn't want to delay getting these patches to the list any longer, so the notifiers didn't make it in. I'll submit them to the list soon, or roll them into the V4 patchset. There are comments in the clk API definitions for where PRECHANGE, POSTCHANGE and ABORT propagation will go.
.basic mux clk, divider and dummy clk implementations. I think others have some code lying around to implement these, so I left them out.
.device tree support. I haven't looked much at the on-going discussions on the dt clk bindings. How compatible (or not) are the device tree clk bindings and the way these patches want to initialize clks?
I have just converted imx6 clock to device tree based on this series and Grant's of-clk series with one minor change on clk-basic which technically is not part of the core support. So I would say, do not worry, it's perfectly compatible with device tree :)
.what is the overlap between common clk and clkdev? We're essentially tracking the clks in two places (common clk's tree and clkdevs's list), which feels a bit wasteful.
I do not see the overlap between these two. The clkdev is nothing but a list maintaining the mapping between necessary 'struct clk' and its consumer's 'struct dev'. It has no clock tree topology, and common clk's tree has no that mapping.