RE: Review Request: New proposal for device tree clock binding.

quoted

quoted

<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". =A0The property value is a tuple
containing the phandle to the clock provider and the name of the =

clock

output signal.

quoted

quoted

For example:

=A0 =A0uart {
=A0 =A0 =A0 =A0baud-clock =3D <&osc>, "ckil";
=A0 =A0 =A0 =A0register-clock =3D <&ref>, "bus";
=A0 =A0};


This represents a device with two clock inputs, named "baud" and
"register". The baud clock is connected to the "ckil" output of the

"osc"

quoted

quoted

device, and the register clock is connected to the "bus" output of =

the

quoted

quoted

"ref" device.

Instead of having two items to identify a clock, I would suggest to =

have

a node for each clock. =A0So that clock can be referenced by one
handle. =A0Also we can have clock specific information defined in the =

clock

node. =A0Here is the example I am planning to use on 85xx PMC.

quoted

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0power@e0070{
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0compatible =3D =

"fsl,mpc8548-pmc",

quoted

"fsl,p2020-pmc";
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D <0xe0070 =

0x20>;

quoted

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0etsec1_clk: =

soc-clk@24{

quoted

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

=A0fsl,pmcdr-mask =3D <0x00000080>;

quoted

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0};
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0etsec2_clk: =

soc-clk@25{

quoted

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

=A0fsl,pmcdr-mask =3D <0x00000040>;

quoted

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0};
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0etsec3_clk: =

soc-clk@26{

quoted

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

=A0fsl,pmcdr-mask =3D <0x00000020>;

quoted

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0};
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0};

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0enet0: ethernet@24000 {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0......
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0master-clock =3D =

<&etsec1_clk>;

quoted

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0......


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.  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?

- Leo