[RFC] clocktree representation in the devicetree

[Sascha Hauer]

On Mon, Oct 17, 2011 at 06:11:56PM -0500, Rob Herring wrote:
> On 10/17/2011 01:43 PM, Sascha Hauer wrote:
> > On Mon, Oct 17, 2011 at 12:01:37PM -0500, Rob Herring wrote:
> >> Sascha,
> >>
> >> On 10/17/2011 05:29 AM, Sascha Hauer wrote:
> >>>
> >>> Hi All,
> >>>
> >>> The following is an attempt to represent the clocktree of a i.MX53 in
> >>> the devicetree. I created this to see how it would look like and to
> >>> start a discussion whether we want to move in this direction or not.
> >>>
> >>> Some things to consider:
> >>>
> >>> - It seems to be very flexible. A board can customize the clock tree
> >>>   by just adding some clk-parent=<phandle> properties to the muxers.
> >>> - clocks can easily be associated with devices.
> >>>
> >>> but:
> >>>
> >>> - The following example registers 127 new platform devices and it's
> >>>   not even complete. This adds significant overhead to initialization.
> >>>
> >>
> >> Why? You should only get platform devices if you declare the clocks
> >> block as a simple bus.
> >>
> >> I like the clk tree hierarchy reflected in the DT hierarchy. This would
> >> make init ordering easier. However, there is one major problem I see.
> >> You can only describe 1 configuration of the clock tree. How do you show
> >> all possible muxing options for clocks? We need to describe what the mux
> >> options are, but not what the current selection is as that is
> >> discoverable already.
> > 
> > The way I did it only dividers and gates are child nodes of their
> > parents. The muxes instead are located at the base of the clock tree and
> > have a parent property which describes all possible parents. A board
> > could then add a current-parent = <phandle> property in its board dts
> > (similar to the status = enabled property). Something like this:
> > 
> > imx53.dtsi:
> > 
> > ...
> > 		periph_apm: clkmux-periph_apm at 0x53fd4018 {
> > 			reg = <0x53fd4018 0x0>;
> > 			shift = <0x00000000>;
> > 			width = <0x00000002>;
> > 			parent = <&pll1 &pll3 &lp_apm 0>;
> > 			compatible = "fsl,imx51-clk-mux";
> > 		};
> > ...
> > 
> > imx53-qsb.dts:
> > 
> > 		periph_apm: clkmux-periph_apm at 0x53fd4018 {
> > 			current_parent = <&pll3>
> > 		};
> > 
> 
> Okay. Missed that not going far enough down into the dts...
> 
> So either a clock can have an explicit parent (or list) or can inherit
> from the parent node. That aligns pretty well with how interrupts are done.
> 
> Perhaps it should be "clock-parent" rather than just parent.

Yes. Also we should make the difference clear between the possible
parents of a mux and the one the user wants to select. So maybe

'clock-parent' for the possible parents of a mux

and

'selected-clock-parent' for the one the user wants to have.

> 
> > 
> > So the entry in imx53-qsb.dts is used to describe what a board wants
> > and not what the current status is.
> > 
> I worry that that could result in a lot of combinations of DT's that
> won't boot. If it's all generic code, how do you ensure things are done
> in the right order. There's lots of gotchas ensuring clocks stay in the
> right ranges and ratios when you change them. I don't think the clock
> hierarchy alone is enough information. As a simple example, what is the
> maximum frequency of internal bus clocks. That is one of the primary
> differences between MX51 and MX53 clocks.

If a user selects higher rates than allowed for a SoC he's doomed, just
like he's doomed when he screws up the devicetree in other ways
nowadays.

I agree that there will be problems when critical bus timings should be
changed via the devicetree. This would either need special handling in
the kernel or simply should be dissallowed.

> 
> Granted this problem exists already, but is it just making it easier to
> hang yourself?
> 
> >>
> >> Will clocks ever become generic enough that it makes sense to describe
> >> clocks in DT at the level of muxes, dividers, gates, etc.?
> > 
> > I think yes. On i.MX processors you only need dividers, gates, muxes and
> > plls. On other SoCs there may be table based dividers, power-of-2
> > dividers or similar stuff, but overall there should be a quite limited
> > set of features to be described.
> > 
> 
> So how do you bind a "fsl,imx51-clk-mux" to yet to be written generic
> clock mux code?

Well my first thought was to use normal (of-)platform devices, but as
mentioned this may have some unwanted overhead. In case of the mux
a generic mux could be used, so maybe compatible = "fsl,imx51-clk-mux",
"clk-mux" should do it. In case of a gate we'll probably need a i.MX5
specific variant as these SoCs use two bits for a single gate.

> More importantly, are the properties exposed sufficient?

No :)

At least there should be a propagates flag, but there may be more.

> 
> While we may ultimately want the compatible strings to be SOC specific,
> it would be good to start by generically defining bindings for dividers,
> muxes, and gates and ensure we have something that works for all SOCs.
> 
> >> Perhaps it
> >> makes more sense to just describe the clock controller to device
> >> connections and any board level clocks in the DT.
> > 
> > Describing the clock tree in the device tree also makes it possible for
> > a board to customize the divider/PLL/mux settings.
> > Consider a SoC with a PLL where several different devices can derive its
> > clock from.  One board may want to move all other devices away from this
> > PLL and use it exclusively for sound to get an exact rate. Another might
> > use it for the pixel clock and a third one selects a good compromise
> > between an exact sound clock and the pixel clock. Not describing this in
> > the device tree means that we need board specific code with
> > clk_get/clk_get_parent/clk_set_rate orgies. 
> > I gave up on creating clock code that magically tries to do everything
> > right based on clk_* functions. With the example above how should the
> > clock code know how to adjust the mentioned PLL? If you managed to get
> > it right for one SoC the next totally different SoC will already be in
> > the pipeline.
> > 
> Good point. I guess the board specifics can go all the way back up the
> clock tree.
> 
> It's good to see a real example. but it would be nice to see some
> documentation to update this:
> 
> http://www.devicetree.org/ClockBindings
> 
> Do you have any code using this? I've updated the OF clock support based
> on Mike's latest common clk code that I need to send out. But it's based
> on the above clock binding.

I wasn't aware of this page. As I see it I could rewrite my suggestion
so that it extends these clock bindings without really changing them.
I have no code working yet, I first wanted to see how the reactions are.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |