[PATCH] Device Tree Bindings for Freescale TDM controller

From: Poonam Aggrwal <redacted> 

This TDM controller is available in various Freescale SOCs like MPC8315, P1020,
P1022, P1010.

Signed-off-by: Sandeep Singh <redacted>
Signed-off-by: Poonam Aggrwal <redacted>
---
 Documentation/devicetree/bindings/tdm/fsl-tdm.txt |   71 +++++++++++++++++++++
 1 files changed, 71 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/tdm/fsl-tdm.txt

diff --git a/Documentation/devicetree/bindings/tdm/fsl-tdm.txt b/Documentation/devicetree/bindings/tdm/fsl-tdm.txt
new file mode 100644
index 0000000..61431e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/tdm/fsl-tdm.txt

@@ -0,0 +1,71 @@
+=====================================================================
+TDM Device Tree Binding
+Copyright (C) 2012 Freescale Semiconductor Inc.
+
+NOTE: The bindings described in this document are preliminary
+and subject to change.
+
+=====================================================================
+TDM (Time Division Multiplexing)
+
+DESCRIPTION
+
+The TDM is full duplex serial port designed to allow various devices including
+digital signal processors (DSPs) to communicate with a variety of serial devices
+including industry standard framers, codecs, other DSPs and microprocessors.
+
+The below properties describe the device tree bindings for Freescale TDM
+controller.
+This TDM controller is available on various Freescale Processors like
+MPC8313, P1020, P1022 and P1010.
+
+PROPERTIES
+
+  - compatible
+      Usage: required
+      Value type: <string>
+      Definition: Should contain "fsl,mpc8315-tdm".
+	  So mpc8313 will have compatible = "fsl,mpc8315-tdm";
+	  p1010 will have compatible "fsl,p1010-tdm", "fsl,mpc8315-tdm";
+
+  - reg
+      Usage: required
+      Value type: <tdm-reg-offset tdm-reg-size dmac-reg-offset dmac-reg-size>
+      Definition: A standard property. Specifies the physical address
+	  offset and length of the TDM registers and TDM DMAC registers for
+	  the device.
+
+  - clock-frequency
+      Usage: optional
+      Value type: <u32>
+      Definition: The frequency at which the TDM block is operating.
+
+  - interrupts
+      Usage: required
+      Value type: <tdm-err-intr tdm-err-intr-type dmac-intr dmac-intr-type>
+      Definition: This field defines two interrupt specifiers namely interrupt
+	  number and interrupt type for TDM error and TDM DMAC.
+
+  - phy-handle
+      Usage: optional
+      Value type: <phandle>
+      Definition: Phandle of the line controller node or framer node eg. SLIC,
+	  E1\T1 etc.
+
+  - fsl-max-time-slots
+      Usage: required
+      Value type: <u32>
+      Definition: Maximum number of 8-bit time slots in one TDM frame.
+	  This is the maximum number which TDM hardware supports.
+
+EXAMPLE
+
+	tdm@16000 {
+		device_type = "tdm";
+		compatible = "fsl,p1010-tdm", "fsl,mpc8315-tdm";
+		reg = <0x16000 0x200 0x2c000 0x2000>;
+		clock-frequency = <0>;
+		interrupts = <16 8 62 8>;
+		phy-handle = <zarlink1>
+		fsl-max-time-slots = <128>
+	};

-- 
1.5.6.5

On 03/15/2012 08:30 PM, Poonam Aggrwal wrote:

Shouldn't mpc8313 have:
compatible = "fsl,mpc8313-tdm", "fsl,mpc8315-tdm"?

I thought we were going to use 8313 as the canonical implementation, not
8315.

Just say there's two reg resources, and that the first is the TDM
registers and the second is the TDM DMAC registers.

It's typically not going to be the actual physical address, but rather
an offset that gets translated through a parent node's ranges.

Remove "value type"; it's standard.

Will this frequency ever need to be > 4GHz?

Might want to specify as u32 or u64, as ePAPR suggests.

What is "tdm-err-intr-type"?  The interrupt specifier encoding is
defined by the interrupt controller.  There might be one cell, two
cells, four cells, etc.  Remove "value type", it's standard.

Use a forward slash -- this isn't a Windows filesystem path. :-)

fsl,tdm-max-time-slots

No device_type

Show a real clock-frequency, perhaps with a comment saying it's
typically filled in by boot software.

+		interrupts = <16 8 62 8>;
+		phy-handle = <zarlink1>

That phy-handle is invalid syntax, perhaps you meant:

	phy-handle = <&zarlink1>;

+		fsl-max-time-slots = <128>

Missing semicolons on the last two properties.

-Scott

Thanks Scott for the review.

Will send an updated revision with the comments taken care.

Regards
Poonam

-----Original Message-----
From: Wood Scott-B07421
Sent: Saturday, March 17, 2012 12:00 AM
To: Aggrwal Poonam-B10812
Cc: devicetree-discuss@lists.ozlabs.org; linuxppc-dev@lists.ozlabs.org;
Singh Sandeep-B37400
Subject: Re: [PATCH] Device Tree Bindings for Freescale TDM controller
=20
On 03/15/2012 08:30 PM, Poonam Aggrwal wrote:

quoted

From: Poonam Aggrwal <redacted>

This TDM controller is available in various Freescale SOCs like
MPC8315, P1020, P1022, P1010.

Signed-off-by: Sandeep Singh <redacted>
Signed-off-by: Poonam Aggrwal <redacted>
---
 Documentation/devicetree/bindings/tdm/fsl-tdm.txt |   71

+++++++++++++++++++++

quoted

 1 files changed, 71 insertions(+), 0 deletions(-)  create mode 100644
Documentation/devicetree/bindings/tdm/fsl-tdm.txt

diff --git a/Documentation/devicetree/bindings/tdm/fsl-tdm.txt

b/Documentation/devicetree/bindings/tdm/fsl-tdm.txt
new file mode 100644
index 0000000..61431e3

--- /dev/null
+++ b/Documentation/devicetree/bindings/tdm/fsl-tdm.txt

@@ -0,0 +1,71 @@
+=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

quoted

+TDM Device Tree Binding
+Copyright (C) 2012 Freescale Semiconductor Inc.
+
+NOTE: The bindings described in this document are preliminary and
+subject to change.
+
+=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

quoted

+TDM (Time Division Multiplexing)
+
+DESCRIPTION
+
+The TDM is full duplex serial port designed to allow various devices
+including digital signal processors (DSPs) to communicate with a
+variety of serial devices including industry standard framers, codecs,

other DSPs and microprocessors.

quoted

+
+The below properties describe the device tree bindings for Freescale
+TDM controller.
+This TDM controller is available on various Freescale Processors like
+MPC8313, P1020, P1022 and P1010.
+
+PROPERTIES
+
+  - compatible
+      Usage: required
+      Value type: <string>
+      Definition: Should contain "fsl,mpc8315-tdm".
+	  So mpc8313 will have compatible =3D "fsl,mpc8315-tdm";
+	  p1010 will have compatible "fsl,p1010-tdm", "fsl,mpc8315-tdm";

=20
Shouldn't mpc8313 have:
compatible =3D "fsl,mpc8313-tdm", "fsl,mpc8315-tdm"?
=20
I thought we were going to use 8313 as the canonical implementation, not
8315.

MPC8315 was the first FSL platform to have this controller.
MPC8313 does not have TDM.

=20

quoted

+  - reg
+      Usage: required
+      Value type: <tdm-reg-offset tdm-reg-size dmac-reg-offset dmac-

reg-size>

quoted

+      Definition: A standard property. Specifies the physical address
+	  offset and length of the TDM registers and TDM DMAC registers for
+	  the device.

=20
Just say there's two reg resources, and that the first is the TDM
registers and the second is the TDM DMAC registers.
=20
It's typically not going to be the actual physical address, but rather an
offset that gets translated through a parent node's ranges.

Okay, I think we missed this comment, you already gave this earlier.
Sorry for that.

=20
Remove "value type"; it's standard.
=20

Okay. So just definition must suffice, right?

quoted

+  - clock-frequency
+      Usage: optional
+      Value type: <u32>
+      Definition: The frequency at which the TDM block is operating.

=20
Will this frequency ever need to be > 4GHz?

Don't think so, at max this will be CCB, not sure if CCB on our platforms m=
ay get bigger than 4G ever.

=20
Might want to specify as u32 or u64, as ePAPR suggests.

Means Value type: <u32 or u64>?
In this case the driver must always use 64bit data structure to read this. =
Is this correct?

=20

quoted

+  - interrupts
+      Usage: required
+      Value type: <tdm-err-intr tdm-err-intr-type dmac-intr dmac-intr-

type>

quoted

+      Definition: This field defines two interrupt specifiers namely

interrupt

quoted

+	  number and interrupt type for TDM error and TDM DMAC.

=20
What is "tdm-err-intr-type"?  The interrupt specifier encoding is defined
by the interrupt controller.  There might be one cell, two cells, four
cells, etc.  Remove "value type", it's standard.
=20

okay

quoted

+  - phy-handle
+      Usage: optional
+      Value type: <phandle>
+      Definition: Phandle of the line controller node or framer node

eg. SLIC,

quoted

+	  E1\T1 etc.

=20
Use a forward slash -- this isn't a Windows filesystem path. :-)
=20

Okay, agreed.

quoted

+  - fsl-max-time-slots
+      Usage: required
+      Value type: <u32>
+      Definition: Maximum number of 8-bit time slots in one TDM frame.
+	  This is the maximum number which TDM hardware supports.

=20
fsl,tdm-max-time-slots

Sure. This again got missed.

=20

quoted

+
+EXAMPLE
+
+	tdm@16000 {
+		device_type =3D "tdm";

=20
No device_type

Okay.

=20

quoted

+		compatible =3D "fsl,p1010-tdm", "fsl,mpc8315-tdm";
+		reg =3D <0x16000 0x200 0x2c000 0x2000>;
+		clock-frequency =3D <0>;

=20
Show a real clock-frequency, perhaps with a comment saying it's typically
filled in by boot software.

Okay.

=20

quoted

+		interrupts =3D <16 8 62 8>;
+		phy-handle =3D <zarlink1>

=20
That phy-handle is invalid syntax, perhaps you meant:
=20
	phy-handle =3D <&zarlink1>;

Yes. Will correct it.

=20

quoted

+		fsl-max-time-slots =3D <128>

=20
Missing semicolons on the last two properties.
=20

Ok.

-Scott