Review Request: New proposal for device tree clock binding.

[Li Yang-R58472]

Hi Grant,

I have some comment on this proposal.

>Subject: Review Request: New proposal for device tree clock binding.
>
>Hi Ben (well, hello to everyone, but I'm particularly interested in Ben's
>feedback),
>
>Jeremy and I have been kicking around the clock binding, and we've come up
>with a new proposal that doesn't feel quite as forced to me.
>Please take a look and let me know what you think.  The link to the
>binding is below[1], but I've also copied the full text so that you can
>reply and comment.  The rational for the new binding can be found in talk
>page[2].
>
>[1] http://www.devicetree.org/ClockBindings
>[2] http://www.devicetree.org/Talk:ClockBindings
>
>---
>
>This page descibes the proposed OF clock bindings. These are a work-in-
>progress, and are based on some [http://patchwork.ozlabs.org/patch/31551/
>experimental work by benh].
>
>==Clock providers==
>
>Sources of clock signal can be represented by any node in the device tree.
>A mandatory "<tt>clock-outputs</tt>" property describes the clock outputs
>from this device.
>
>{|border=1
>!property
>!format
>!notes
>|-
>|<tt>clock-outputs</tt>
>|list of strings
>|specifies output clock signal names.
>|}
>
>For example:
>
>    oscillator {
>        clock-outputs = "ckil", "ckih";
>    };
>
>- this node defines a device with two clock outputs, the first named
>"ckil" and the second named "ckih".  Consumer nodes always reference
>clocks by name.  The names should reflect the clock output signal names
>for the device.
>
>==Clock consumers==
>
>A device connected to a clock signal needs a *-clock property for each
>clock that it is connected to.
>
>{|border=1
>!property
>!format
>!notes
>|-
>|<tt>*-clock</tt>
>|1 cell phandle to the clock provider, followed by a string containing
>the clock output name.
>|The name of this property should be the name of the clock input
>signal with a "-clock" suffix.
>|}
>
><tt>*-clock</tt> is named for the signal name for the ''clock input''
>of the device. it should describe the function of the signal for that
>device, rather than the name of the system-wide clock line. For example, a
>UART with two clocks - one for baud-rate clocking, and the other for
>register clocking - may have clock input properties named "baud-clock" and
>"register-clock".  The property value is a tuple containing the phandle to
>the clock provider and the name of the clock output signal.
>
>For example:
>
>    uart {
>        baud-clock = <&osc>, "ckil";
>        register-clock = <&ref>, "bus";
>    };
>
>
>This represents a device with two clock inputs, named "baud" and
>"register". The baud clock is connected to the "ckil" output of the "osc"
>device, and the register clock is connected to the "bus" output of the
>"ref" device.


Instead of having two items to identify a clock, I would suggest to have a node for each clock.  So that clock can be referenced by one handle.  Also we can have clock specific information defined in the clock node.  Here is the example I am planning to use on 85xx PMC.


                power at e0070{
                        compatible = "fsl,mpc8548-pmc", "fsl,p2020-pmc";
                        reg = <0xe0070 0x20>;

                        etsec1_clk: soc-clk at 24{
                                fsl,pmcdr-mask = <0x00000080>;
                        };
                        etsec2_clk: soc-clk at 25{
                                fsl,pmcdr-mask = <0x00000040>;
                        };
                        etsec3_clk: soc-clk at 26{
                                fsl,pmcdr-mask = <0x00000020>;
                        };
                };

                enet0: ethernet at 24000 {
                  ......
                        master-clock = <&etsec1_clk>;
			......


What do you think?

- Leo