[PATCH 0/3] Device tree docs updates for FSL periphs

[PATCH 0/3] Device tree docs updates for FSL periphs

[Kumar Gala]

All,

Please review these updates to booting-without-of.txt for the FSL periphs:
* SATA
* RapidIO
* DMA

They are intended to encompass patches posted before with some changes
that I've made.

- k

[PATCH 1/3] [POWERPC] Add docs for Freescale 83xx SATA device tree nodes

[Kumar Gala]

---
 Documentation/powerpc/booting-without-of.txt |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index e9a3cb1..eb14dd5 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -2514,6 +2514,29 @@ platforms are moved over to use the flattened-device-tree model.
       Requred properties:
        - current-speed : Baud rate of uartlite

+    * Freescale 8xxx/3.0 Gb/s SATA nodes
+
+    SATA nodes are defined to describe on-chip Serial ATA controllers.
+    Each SATA port should have its own node.
+
+    Required properties:
+    - compatible        : compatible list, contains 2 entries, first is
+			 "fsl,sata-CHIP", where CHIP is the processor
+			 (mpc8315, mpc8379, etc.) and the second is
+			 "fsl,sata-pq2pro"
+    - interrupts        : <interrupt mapping for SATA IRQ>
+    - interrupt-parent  : optional, if needed for interrupt mapping
+    - reg               : <registers mapping>
+
+   Example:
+
+	sata@19000 {
+		compatible = "fsl,mpc8315-sata", "fsl,sata-pq2pro;
+		reg = <19000 1000>;
+		interrupts = <2d 8>;
+		interrupt-parent = < &ipic >;
+        };
+
    More devices will be defined as this spec matures.

 VII - Specifying interrupt information for devices
-- 
1.5.3.4

Re: [PATCH 1/3] [POWERPC] Add docs for Freescale 83xx SATA device tree nodes

[Arnd Bergmann]

On Wednesday 21 November 2007, Kumar Gala wrote:
> + =A0 =A0* Freescale 8xxx/3.0 Gb/s SATA nodes
> +
> + =A0 =A0SATA nodes are defined to describe on-chip Serial ATA controller=
s.
> + =A0 =A0Each SATA port should have its own node.
> +
> + =A0 =A0Required properties:
> + =A0 =A0- compatible =A0 =A0 =A0 =A0: compatible list, contains 2 entrie=
s, first is
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 "f=
sl,sata-CHIP", where CHIP is the processor
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 (m=
pc8315, mpc8379, etc.) and the second is
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 "f=
sl,sata-pq2pro"
> + =A0 =A0- interrupts =A0 =A0 =A0 =A0: <interrupt mapping for SATA IRQ>
> + =A0 =A0- interrupt-parent =A0: optional, if needed for interrupt mapping
> + =A0 =A0- reg =A0 =A0 =A0 =A0 =A0 =A0 =A0 : <registers mapping>
> +

Should this maybe also mandate a compatible property that is defined
in a way to match the generic (p)ata_of_platform driver?

	Arnd <><

Re: [PATCH 1/3] [POWERPC] Add docs for Freescale 83xx SATA device tree nodes

[Kumar Gala]

On Nov 25, 2007, at 4:22 PM, Arnd Bergmann wrote:

> On Wednesday 21 November 2007, Kumar Gala wrote:
>> +    * Freescale 8xxx/3.0 Gb/s SATA nodes
>> +
>> +    SATA nodes are defined to describe on-chip Serial ATA  
>> controllers.
>> +    Each SATA port should have its own node.
>> +
>> +    Required properties:
>> +    - compatible        : compatible list, contains 2 entries,  
>> first is
>> +                        "fsl,sata-CHIP", where CHIP is the processor
>> +                        (mpc8315, mpc8379, etc.) and the second is
>> +                        "fsl,sata-pq2pro"
>> +    - interrupts        : <interrupt mapping for SATA IRQ>
>> +    - interrupt-parent  : optional, if needed for interrupt mapping
>> +    - reg               : <registers mapping>
>> +
>
> Should this maybe also mandate a compatible property that is defined
> in a way to match the generic (p)ata_of_platform driver?

Is there something about the (p)ata_of_platform driver you think we  
can use.  The SATA controller here is a unique piece of HW and  
requires a unique driver so I'm not sure what we get from  
(p)ata_of_platform.

- k

Re: [PATCH 1/3] [POWERPC] Add docs for Freescale 83xx SATA device tree nodes

[Arnd Bergmann]

On Monday 26 November 2007, you wrote:
> On Nov 25, 2007, at 4:22 PM, Arnd Bergmann wrote:

> > Should this maybe also mandate a compatible property that is defined
> > in a way to match the generic (p)ata_of_platform driver?
>=20
> Is there something about the (p)ata_of_platform driver you think we =A0
> can use. =A0The SATA controller here is a unique piece of HW and =A0
> requires a unique driver so I'm not sure what we get from =A0
> (p)ata_of_platform.

All ata controllers I've dealt with so far are to some degree compatible
to the old PC style controllers. My point was that this should ideally
be reflected in the device tree so that you could in theory use the
ata_of_platform driver, even if you normally would prefer the new
driver for performance reasons.

If your controller doesn't have the legacy register set, this clearly
doesn't make any sense and you should not list it as compatible with
something generic.

	Arnd <><

RE: [PATCH 1/3] [POWERPC] Add docs for Freescale 83xx SATA device tree nodes

[Li Yang]

> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]=20
> Sent: Wednesday, November 21, 2007 1:13 PM
> To: linuxppc-dev@ozlabs.org
> Cc: Segher Boessenkool; Li Yang
> Subject: [PATCH 1/3] [POWERPC] Add docs for Freescale 83xx=20
> SATA device tree nodes
>=20
> ---
>  Documentation/powerpc/booting-without-of.txt |   23=20
> +++++++++++++++++++++++
>  1 files changed, 23 insertions(+), 0 deletions(-)
>=20
> diff --git a/Documentation/powerpc/booting-without-of.txt=20
> b/Documentation/powerpc/booting-without-of.txt
> index e9a3cb1..eb14dd5 100644
> --- a/Documentation/powerpc/booting-without-of.txt
> +++ b/Documentation/powerpc/booting-without-of.txt
> @@ -2514,6 +2514,29 @@ platforms are moved over to use the=20
> flattened-device-tree model.
>        Requred properties:
>         - current-speed : Baud rate of uartlite
>=20
> +    * Freescale 8xxx/3.0 Gb/s SATA nodes
> +
> +    SATA nodes are defined to describe on-chip Serial ATA=20
> controllers.
> +    Each SATA port should have its own node.
> +
> +    Required properties:
> +    - compatible        : compatible list, contains 2=20
> entries, first is
> +			 "fsl,sata-CHIP", where CHIP is the processor

Should be "fsl,CHIP-sata"

> +			 (mpc8315, mpc8379, etc.) and the second is
> +			 "fsl,sata-pq2pro"

Why not use "pq2pro-sata" to be consistent with the CHIP version?

> +    - interrupts        : <interrupt mapping for SATA IRQ>
> +    - interrupt-parent  : optional, if needed for interrupt mapping
> +    - reg               : <registers mapping>
> +
> +   Example:
> +
> +	sata@19000 {
> +		compatible =3D "fsl,mpc8315-sata", "fsl,sata-pq2pro;

Quotation mark missing.

- Leo

[PATCH 2/3] [POWERPC] Add docs for Freescale RapidIO device tree node

[Kumar Gala]

---
 Documentation/powerpc/booting-without-of.txt |   47 ++++++++++++++++++++++++--
 1 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index eb14dd5..3d959d6 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -2521,9 +2521,9 @@ platforms are moved over to use the flattened-device-tree model.

     Required properties:
     - compatible        : compatible list, contains 2 entries, first is
-			 "fsl,sata-CHIP", where CHIP is the processor
+			 "fsl,CHIP-sata", where CHIP is the processor
 			 (mpc8315, mpc8379, etc.) and the second is
-			 "fsl,sata-pq2pro"
+			 "fsl,pq2pro-sata"
     - interrupts        : <interrupt mapping for SATA IRQ>
     - interrupt-parent  : optional, if needed for interrupt mapping
     - reg               : <registers mapping>
@@ -2531,12 +2531,53 @@ platforms are moved over to use the flattened-device-tree model.
    Example:

 	sata@19000 {
-		compatible = "fsl,mpc8315-sata", "fsl,sata-pq2pro;
+		compatible = "fsl,mpc8315-sata", "fsl,pq2pro-sata;
 		reg = <19000 1000>;
 		interrupts = <2d 8>;
 		interrupt-parent = < &ipic >;
         };

+   * Frescale Serial RapidIO bus controller
+
+   RapidIO is a definition of a system interconnect. This node add
+   the support for RapidIO processor in kernel. The node name is
+   suggested to be 'rapidio'.
+
+   Required properties:
+
+    - compatible        : compatible list, contains 2 entries, first is
+			 "fsl,CHIP-rapidio", where CHIP is the processor
+			 (mpc8572, mpc8641, etc.) and the second is
+			 "fsl,delta-rapidio"
+    - interrupts	: interrupts are expected in the following
+			  order:
+      				* Error/port write
+				* Outbound Doorbell
+				* Inbound Doorbell
+				* Outbound Msg Unit 1
+				* Inbound Msg Unit 1
+				* Outbound Msg Unit 2
+				* Inbound Msg Unit 2
+    - interrupt-parent  : optional, if needed for interrupt mapping
+    - reg               : <registers mapping>
+    - #address-cells	: Should always be 2
+    - #size--cells	: Should always be 2
+    - ranges		: Details processor physical addr to RapidIO address
+
+  Example:
+
+	rapidio@c0000 {
+		compatible = "fsl,mpc8641-rapidio", "fsl,delta-rapidio;
+		#address-cells = <2>;
+		#size-cells = <2>;
+		reg = <c0000 20000>;
+		ranges = <0 0 c0000000 20000000>;
+		interrupt-parent = <&mpic>;
+		/* err_irq bell_outb_irq bell_inb_irq msg1_tx_irq msg1_rx_irq
+						      msg2_tx_irq msg2_rx_irq */
+		interrupts = <30 2 31 2 32 2 35 2 36 2 37 2 38 2>;
+	};
+
    More devices will be defined as this spec matures.

 VII - Specifying interrupt information for devices
-- 
1.5.3.4

Re: [PATCH 2/3] [POWERPC] Add docs for Freescale RapidIO device tree node

[Scott Wood]

On Tue, Nov 20, 2007 at 11:13:58PM -0600, Kumar Gala wrote:
>      Required properties:
>      - compatible        : compatible list, contains 2 entries, first is
> -			 "fsl,sata-CHIP", where CHIP is the processor
> +			 "fsl,CHIP-sata", where CHIP is the processor
>  			 (mpc8315, mpc8379, etc.) and the second is
> -			 "fsl,sata-pq2pro"
> +			 "fsl,pq2pro-sata"
>      - interrupts        : <interrupt mapping for SATA IRQ>
>      - interrupt-parent  : optional, if needed for interrupt mapping
>      - reg               : <registers mapping>
> @@ -2531,12 +2531,53 @@ platforms are moved over to use the flattened-device-tree model.
>     Example:
> 
>  	sata@19000 {
> -		compatible = "fsl,mpc8315-sata", "fsl,sata-pq2pro;
> +		compatible = "fsl,mpc8315-sata", "fsl,pq2pro-sata;

I think you meant to merge these changes with the previous patch...

-Scott

Re: [PATCH 2/3] [POWERPC] Add docs for Freescale RapidIO device tree node

[Kumar Gala]

On Nov 21, 2007, at 11:29 AM, Scott Wood wrote:

> On Tue, Nov 20, 2007 at 11:13:58PM -0600, Kumar Gala wrote:
>>     Required properties:
>>     - compatible        : compatible list, contains 2 entries,  
>> first is
>> -			 "fsl,sata-CHIP", where CHIP is the processor
>> +			 "fsl,CHIP-sata", where CHIP is the processor
>> 			 (mpc8315, mpc8379, etc.) and the second is
>> -			 "fsl,sata-pq2pro"
>> +			 "fsl,pq2pro-sata"
>>     - interrupts        : <interrupt mapping for SATA IRQ>
>>     - interrupt-parent  : optional, if needed for interrupt mapping
>>     - reg               : <registers mapping>
>> @@ -2531,12 +2531,53 @@ platforms are moved over to use the  
>> flattened-device-tree model.
>>    Example:
>>
>> 	sata@19000 {
>> -		compatible = "fsl,mpc8315-sata", "fsl,sata-pq2pro;
>> +		compatible = "fsl,mpc8315-sata", "fsl,pq2pro-sata;
>
> I think you meant to merge these changes with the previous patch...

Yeah a merge issue, I'll look into it.

- k

[PATCH 3/3] [POWERPC] Add docs for Freescale DMA & DMA channel device tree nodes

[Kumar Gala]

---
 Documentation/powerpc/booting-without-of.txt |  113 ++++++++++++++++++++++++++
 1 files changed, 113 insertions(+), 0 deletions(-)

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index 3d959d6..161fb0d 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -2578,6 +2578,119 @@ platforms are moved over to use the flattened-device-tree model.
 		interrupts = <30 2 31 2 32 2 35 2 36 2 37 2 38 2>;
 	};

+   * Freescale 83xx DMA Controller
+
+    Freescale PowerPC 83xx have on chip general purpose DMA controllers.
+
+    Required properties:
+
+    - compatible        : compatible list, contains 2 entries, first is
+			 "fsl,CHIP-dma", where CHIP is the processor
+			 (mpc8349, mpc8360, etc.) and the second is
+			 "fsl,elo-dma"
+    - reg               : <registers mapping for DMA general status reg>
+    - ranges 		: Should be defined as specified in 1) to describe the
+			  DMA controller channels.
+    - interrupts        : <interrupt mapping for DMA IRQ>
+    - interrupt-parent  : optional, if needed for interrupt mapping
+
+
+    - DMA channel nodes:
+	    - compatible        : compatible list, contains 2 entries, first is
+				 "fsl,CHIP-dma-channel", where CHIP is the processor
+				 (mpc8349, mpc8350, etc.) and the second is
+				 "fsl,elo-dma-channel"
+	    - reg               : <registers mapping for channel>
+
+    Optional properties:
+	    - interrupts        : <interrupt mapping for DMA channel IRQ>
+				  (on 83xx this is expected to be identical to
+				   the interrupts property of the parent node)
+	    - interrupt-parent  : optional, if needed for interrupt mapping
+
+  Example:
+	dma@21000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "fsl,mpc8349-dma", "fsl,elo-dma";
+		reg = <21300 4>;
+		ranges = <0 21100 200>;
+		interrupt-parent = <&ipic>;
+		interrupts = <14 2>;
+		dma-channel@0 {
+			compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
+			reg = <0 80>;
+		};
+		dma-channel@80 {
+			compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
+			reg = <80 80>;
+		};
+		dma-channel@100 {
+			compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
+			reg = <100 80>;
+		};
+		dma-channel@180 {
+			compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
+			reg = <180 80>;
+		};
+	};
+
+   * Freescale 85xx DMA Controller
+
+    Freescale PowerPC 85xx have on chip general purpose DMA controllers.
+
+    Required properties:
+
+    - compatible        : compatible list, contains 2 entries, first is
+			 "fsl,CHIP-dma", where CHIP is the processor
+			 (mpc8540, mpc8540, etc.) and the second is
+			 "fsl,eloplus-dma"
+    - reg               : <registers mapping for DMA general status reg>
+    - ranges 		: Should be defined as specified in 1) to describe the
+			  DMA controller channels.
+
+    - DMA channel nodes:
+	    - compatible        : compatible list, contains 2 entries, first is
+				 "fsl,CHIP-dma-channel", where CHIP is the processor
+				 (mpc8540, mpc8560, etc.) and the second is
+				 "fsl,eloplus-dma-channel"
+	    - reg               : <registers mapping for channel>
+	    - interrupts        : <interrupt mapping for DMA channel IRQ>
+	    - interrupt-parent  : optional, if needed for interrupt mapping
+
+  Example:
+	dma@21000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "fsl,mpc8540-dma", "fsl,eloplus-dma";
+		reg = <21300 4>;
+		ranges = <0 21100 200>;
+		dma-channel@0 {
+			compatible = "fsl,mpc8540-dma-channel", "fsl,eloplus-dma-channel";
+			reg = <0 80>;
+			interrupt-parent = <&mpic>;
+			interrupts = <14 2>;
+		};
+		dma-channel@80 {
+			compatible = "fsl,mpc8540-dma-channel", "fsl,eloplus-dma-channel";
+			reg = <80 80>;
+			interrupt-parent = <&mpic>;
+			interrupts = <15 2>;
+		};
+		dma-channel@100 {
+			compatible = "fsl,mpc8540-dma-channel", "fsl,eloplus-dma-channel";
+			reg = <100 80>;
+			interrupt-parent = <&mpic>;
+			interrupts = <16 2>;
+		};
+		dma-channel@180 {
+			compatible = "fsl,mpc8540-dma-channel", "fsl,eloplus-dma-channel";
+			reg = <180 80>;
+			interrupt-parent = <&mpic>;
+			interrupts = <17 2>;
+		};
+	};
+
    More devices will be defined as this spec matures.

 VII - Specifying interrupt information for devices
-- 
1.5.3.4

Re: [PATCH 3/3] [POWERPC] Add docs for Freescale DMA & DMA channel device tree nodes

[Timur Tabi]

Kumar Gala wrote:

> +   * Freescale 83xx DMA Controller
> +
> +    Freescale PowerPC 83xx have on chip general purpose DMA controllers.
> +
> +    Required properties:
> +
> +    - compatible        : compatible list, contains 2 entries, first is
> +			 "fsl,CHIP-dma", where CHIP is the processor
> +			 (mpc8349, mpc8360, etc.) and the second is
> +			 "fsl,elo-dma"

Shouldn't we put some text somewhere that we're calling it the Elo controller 
even though that word isn't used in the reference manual?

> +   * Freescale 85xx DMA Controller

And 86xx.

> +
> +    Freescale PowerPC 85xx have on chip general purpose DMA controllers.
> +
> +    Required properties:
> +
> +    - compatible        : compatible list, contains 2 entries, first is
> +			 "fsl,CHIP-dma", where CHIP is the processor
> +			 (mpc8540, mpc8540, etc.) and the second is
> +			 "fsl,eloplus-dma"
> +    - reg               : <registers mapping for DMA general status reg>
> +    - ranges 		: Should be defined as specified in 1) to describe the
> +			  DMA controller channels.
> +
> +    - DMA channel nodes:
> +	    - compatible        : compatible list, contains 2 entries, first is
> +				 "fsl,CHIP-dma-channel", where CHIP is the processor
> +				 (mpc8540, mpc8560, etc.) and the second is
> +				 "fsl,eloplus-dma-channel"
> +	    - reg               : <registers mapping for channel>
> +	    - interrupts        : <interrupt mapping for DMA channel IRQ>
> +	    - interrupt-parent  : optional, if needed for interrupt mapping
> +
> +  Example:
> +	dma@21000 {

Shouldn't this be dma@21300?

> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "fsl,mpc8540-dma", "fsl,eloplus-dma";
> +		reg = <21300 4>;
> +		ranges = <0 21100 200>;
> +		dma-channel@0 {
> +			compatible = "fsl,mpc8540-dma-channel", "fsl,eloplus-dma-channel";
> +			reg = <0 80>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <14 2>;
> +		};

The DMA controller and the DMA channels need a "device-id", so that they can 
be identified by number.  Some peripherals, like the SSI, can only use the 
controller and channel number.  This is what I have in my 8610 DTS:

                 dma@21300 {
                         #address-cells = <1>;
                         #size-cells = <1>;
                         compatible = "fsl,mpc8610-dma", "fsl,mpc8540-dma";
         -->             device-id = <0>;
                         reg = <21300 4>; /* DMA general status register */
                         ranges = <0 21100 200>;

                         dma-channel@0 {
                                 compatible = "fsl,mpc8610-dma-channel",
                                         "fsl,mpc8540-dma-channel";
         -->                     device-id = <0>;
                                 reg = <0 80>;
                                 interrupt-parent = <&mpic>;
                                 interrupts = <14 2>;
                         };

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [PATCH 3/3] [POWERPC] Add docs for Freescale DMA & DMA channel device tree nodes

[Kumar Gala]

On Nov 21, 2007, at 8:59 AM, Timur Tabi wrote:

> Kumar Gala wrote:
>
>> +   * Freescale 83xx DMA Controller
>> +
>> +    Freescale PowerPC 83xx have on chip general purpose DMA  
>> controllers.
>> +
>> +    Required properties:
>> +
>> +    - compatible        : compatible list, contains 2 entries,  
>> first is
>> +			 "fsl,CHIP-dma", where CHIP is the processor
>> +			 (mpc8349, mpc8360, etc.) and the second is
>> +			 "fsl,elo-dma"
>
> Shouldn't we put some text somewhere that we're calling it the Elo  
> controller even though that word isn't used in the reference manual?

we don't really have a place to put that.  its effectively documented  
right here.

>
>
>> +   * Freescale 85xx DMA Controller
>
> And 86xx.

yes, true.

>> +
>> +    Freescale PowerPC 85xx have on chip general purpose DMA  
>> controllers.
>> +
>> +    Required properties:
>> +
>> +    - compatible        : compatible list, contains 2 entries,  
>> first is
>> +			 "fsl,CHIP-dma", where CHIP is the processor
>> +			 (mpc8540, mpc8540, etc.) and the second is
>> +			 "fsl,eloplus-dma"
>> +    - reg               : <registers mapping for DMA general  
>> status reg>
>> +    - ranges 		: Should be defined as specified in 1) to describe  
>> the
>> +			  DMA controller channels.
>> +
>> +    - DMA channel nodes:
>> +	    - compatible        : compatible list, contains 2 entries,  
>> first is
>> +				 "fsl,CHIP-dma-channel", where CHIP is the processor
>> +				 (mpc8540, mpc8560, etc.) and the second is
>> +				 "fsl,eloplus-dma-channel"
>> +	    - reg               : <registers mapping for channel>
>> +	    - interrupts        : <interrupt mapping for DMA channel IRQ>
>> +	    - interrupt-parent  : optional, if needed for interrupt mapping
>> +
>> +  Example:
>> +	dma@21000 {
>
> Shouldn't this be dma@21300?

its an example that has not basis is reality :)

>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		compatible = "fsl,mpc8540-dma", "fsl,eloplus-dma";
>> +		reg = <21300 4>;
>> +		ranges = <0 21100 200>;
>> +		dma-channel@0 {
>> +			compatible = "fsl,mpc8540-dma-channel", "fsl,eloplus-dma- 
>> channel";
>> +			reg = <0 80>;
>> +			interrupt-parent = <&mpic>;
>> +			interrupts = <14 2>;
>> +		};
>
> The DMA controller and the DMA channels need a "device-id", so that  
> they can be identified by number.  Some peripherals, like the SSI,  
> can only use the controller and channel number.  This is what I have  
> in my 8610 DTS:

Why not use reg for this?  I don't see any reason to add another  
"unique id" when there is already one.

>                dma@21300 {
>                        #address-cells = <1>;
>                        #size-cells = <1>;
>                        compatible = "fsl,mpc8610-dma", "fsl,mpc8540- 
> dma";
>        -->             device-id = <0>;
>                        reg = <21300 4>; /* DMA general status  
> register */
>                        ranges = <0 21100 200>;
>
>                        dma-channel@0 {
>                                compatible = "fsl,mpc8610-dma-channel",
>                                        "fsl,mpc8540-dma-channel";
>        -->                     device-id = <0>;
>                                reg = <0 80>;
>                                interrupt-parent = <&mpic>;
>                                interrupts = <14 2>;
>                        };
>

- k

Re: [PATCH 3/3] [POWERPC] Add docs for Freescale DMA & DMA channel device tree nodes

[Scott Wood]

On Wed, Nov 21, 2007 at 09:33:05AM -0600, Kumar Gala wrote:
> On Nov 21, 2007, at 8:59 AM, Timur Tabi wrote:
> >> +  Example:
> >> +	dma@21000 {
> >
> > Shouldn't this be dma@21300?
> 
> its an example that has not basis is reality :)

But it should at least be internally consistent with this:

> >> +		reg = <21300 4>;

[snip]
> > The DMA controller and the DMA channels need a "device-id", so that  
> > they can be identified by number.  Some peripherals, like the SSI,  
> > can only use the controller and channel number.  This is what I have  
> > in my 8610 DTS:
> 
> Why not use reg for this?  I don't see any reason to add another  
> "unique id" when there is already one.

A cell-index property would be useful here for indexing into the summary
status register.

> 
> >                dma@21300 {
> >                        #address-cells = <1>;
> >                        #size-cells = <1>;
> >                        compatible = "fsl,mpc8610-dma", "fsl,mpc8540- 
> > dma";
> >        -->             device-id = <0>;

I don't see any justification for having such a property in the parent node,
though.

-Scott

Re: [PATCH 3/3] [POWERPC] Add docs for Freescale DMA & DMA channel device tree nodes

[Kumar Gala]

On Nov 21, 2007, at 11:35 AM, Scott Wood wrote:

> On Wed, Nov 21, 2007 at 09:33:05AM -0600, Kumar Gala wrote:
>> On Nov 21, 2007, at 8:59 AM, Timur Tabi wrote:
>>>> +  Example:
>>>> +	dma@21000 {
>>>
>>> Shouldn't this be dma@21300?
>>
>> its an example that has not basis is reality :)
>
> But it should at least be internally consistent with this:
>
>>>> +		reg = <21300 4>;

ahh, i see.. yes I'll fix that.

> [snip]
>>> The DMA controller and the DMA channels need a "device-id", so that
>>> they can be identified by number.  Some peripherals, like the SSI,
>>> can only use the controller and channel number.  This is what I have
>>> in my 8610 DTS:
>>
>> Why not use reg for this?  I don't see any reason to add another
>> "unique id" when there is already one.
>
> A cell-index property would be useful here for indexing into the  
> summary
> status register.

Divide by 0x80.

>>>               dma@21300 {
>>>                       #address-cells = <1>;
>>>                       #size-cells = <1>;
>>>                       compatible = "fsl,mpc8610-dma", "fsl,mpc8540-
>>> dma";
>>>       -->             device-id = <0>;
>
> I don't see any justification for having such a property in the  
> parent node,
> though.

huh?
- k

Re: [PATCH 3/3] [POWERPC] Add docs for Freescale DMA & DMA channel device tree nodes

[Scott Wood]

Kumar Gala wrote:
> On Nov 21, 2007, at 11:35 AM, Scott Wood wrote:
>> A cell-index property would be useful here for indexing into the summary
>> status register.
> 
> Divide by 0x80.

:-P

Using cell-index for things like this is reasonably common, and endorsed 
by current ePAPR drafts.

>>>>               dma@21300 {
>>>>                       #address-cells = <1>;
>>>>                       #size-cells = <1>;
>>>>                       compatible = "fsl,mpc8610-dma", "fsl,mpc8540-
>>>> dma";
>>>>       -->             device-id = <0>;
>>
>> I don't see any justification for having such a property in the parent 
>> node,
>> though.
> 
> huh?

Timur put device-id properties in both the parent and the channel nodes. 
  I was wondering why he did the former.

-Scott

Re: [PATCH 3/3] [POWERPC] Add docs for Freescale DMA & DMA channel device tree nodes

[David Gibson]

On Wed, Nov 21, 2007 at 01:27:03PM -0600, Scott Wood wrote:
> Kumar Gala wrote:
> > On Nov 21, 2007, at 11:35 AM, Scott Wood wrote:
> >> A cell-index property would be useful here for indexing into the summary
> >> status register.
> > 
> > Divide by 0x80.
> 
> :-P
> 
> Using cell-index for things like this is reasonably common, and endorsed 
> by current ePAPR drafts.

Indeed, indexing or writing into shared registers is exactly what
cell-index is for.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 3/3] [POWERPC] Add docs for Freescale DMA & DMA channel device tree nodes

[Timur Tabi]

David Gibson wrote:

> Indeed, indexing or writing into shared registers is exactly what
> cell-index is for.

I don't care whether it's cell-index or device-id, but I need to know which 
DMA controller is #0 and which one is #1, and I need to know which channel is 
#0, which one is #1, etc.  Dividing register offsets by 0x80 is not 
acceptable, because what if we have an elo-plus-plus that has 0x100 bytes per 
register, where the additional 0x20 bytes are for enhanced features?

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [PATCH 3/3] [POWERPC] Add docs for Freescale DMA & DMA channel device tree nodes

[Scott Wood]

Timur Tabi wrote:
> David Gibson wrote:
> 
>> Indeed, indexing or writing into shared registers is exactly what
>> cell-index is for.
> 
> I don't care whether it's cell-index or device-id, but I need to know 
> which DMA controller is #0 and which one is #1, and I need to know which 
> channel is #0, which one is #1, etc.

OK, cell-index makes sense there too, then.

-Scott

Re: [PATCH 3/3] [POWERPC] Add docs for Freescale DMA & DMA channel device tree nodes

[Timur Tabi]

Scott Wood wrote:

> I don't see any justification for having such a property in the parent node,
> though.

The SSI needs to know which DMA controller is #0 and which one is #1.

I literally program the SSI and the GUTS registers with the DMA controller and 
channels numbers.  I need to know which one is which!

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [PATCH 3/3] [POWERPC] Add docs for Freescale DMA & DMA channel device tree nodes

[Timur Tabi]

Kumar Gala wrote:

>> Shouldn't we put some text somewhere that we're calling it the Elo 
>> controller even though that word isn't used in the reference manual?
> 
> we don't really have a place to put that.  its effectively documented 
> right here.

I still think we need something.  Otherwise, people are going to be confused. 
  I know I would.  I'd be searching the RM for the string "ELO" and wonder why 
it wasn't there.


>>> +  Example:
>>> +    dma@21000 {
>>
>> Shouldn't this be dma@21300?
> 
> its an example that has not basis is reality :)

Eh?

>> The DMA controller and the DMA channels need a "device-id", so that 
>> they can be identified by number.  Some peripherals, like the SSI, can 
>> only use the controller and channel number.  This is what I have in my 
>> 8610 DTS:
> 
> Why not use reg for this?  I don't see any reason to add another "unique 
> id" when there is already one.

There isn't one.  Why should the driver assume that reg/80 == channel #? 
Besides, I still can't differentiate between DMA controller 0 and DMA 
controller 1 that way.  No, we need a device-id.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [PATCH 3/3] [POWERPC] Add docs for Freescale DMA & DMA channel device tree nodes

[Scott Wood]

On Tue, Nov 20, 2007 at 11:14:40PM -0600, Kumar Gala wrote:
> +    - compatible        : compatible list, contains 2 entries, first is
> +			 "fsl,CHIP-dma", where CHIP is the processor
> +			 (mpc8540, mpc8540, etc.) and the second is
> +			 "fsl,eloplus-dma"

So if the DMA register set gets tweaked again, will we have eloplusplus? :-)
Maybe elo2 would be better.

Do we really need completely separate descriptions of the two, or
can we just describe the difference in the compatible section?

-Scott

Re: [PATCH 3/3] [POWERPC] Add docs for Freescale DMA & DMA channel device tree nodes

[Kumar Gala]

On Nov 21, 2007, at 11:33 AM, Scott Wood wrote:

> On Tue, Nov 20, 2007 at 11:14:40PM -0600, Kumar Gala wrote:
>> +    - compatible        : compatible list, contains 2 entries,  
>> first is
>> +			 "fsl,CHIP-dma", where CHIP is the processor
>> +			 (mpc8540, mpc8540, etc.) and the second is
>> +			 "fsl,eloplus-dma"
>
> So if the DMA register set gets tweaked again, will we have  
> eloplusplus? :-)
> Maybe elo2 would be better.

Seem unlikely for the forseeable future.

> Do we really need completely separate descriptions of the two, or
> can we just describe the difference in the compatible section?

it seemed easier to duplicate and fix.

- k