Review Request: New proposal for device tree clock binding.

[Grant Likely]

On Mon, Aug 9, 2010 at 1:05 AM, Li Yang-R58472 <r58472 at freescale.com> wrote:
>>>><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?
>
> Quoting your reply:
>
>> I've avoided requiring clock nodes to have a separate sub node for
>> each output because it is more verbose and it prevents clock providers
>> from having child nodes for other purposes.  Are you concerned that
>
> I don't see why there should be child nodes for other purposes under clock node.
>
>> having the <phandle>+output name pair will be difficult to manage?
>
> That's part of my concern.

I was concerned about this too until I found precedence for doing the
exact same thing in the pci binding (and ePAPR).  Mixing phandle and a
string in this way doesn't bother me anymore.

>  But my main concern is the inability of describing the properties of each clock in the device tree.  The clock stuff is much SoC related, which means it could be variable among chips even in a same family.  Having clock properties defined in device tree will make it easier to have an abstracted driver to handle clock operations.  That's why device trees are used in the first place, right?

You can do whatever you like for your specific clock source driver.
All the clock binding provides is a connection from a clock consumer
node to a specific named output from a clock provider node.  You can
add whatever properties (or subnodes) you need for the hardware you
are writing a binding for.  This binding doesn't prevent you from
doing anything.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.