[PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer document

[PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer document

[Dongsheng.wang-KZfg59tc24xl57MIdRCFDg]

From: Wang Dongsheng <Dongsheng.Wang-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

Add a description of the OPEN-PIC global timer in the OPEN-PIC document.

Moidfy mpic-timer document. 1.Add a TFRR register region. This register
is written by software to report the clocking frequency of the PIC timers.
2.Add a device_type. The global timer in line with the OPEN-PIC specification.

Signed-off-by: Wang Dongsheng <Dongsheng.Wang-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Signed-off-by: Li Yang <leoli-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 Documentation/devicetree/bindings/open-pic.txt     |   46 ++++++++++++++++++++
 .../devicetree/bindings/powerpc/fsl/mpic-timer.txt |   21 +++++----
 arch/powerpc/boot/dts/fsl/pq3-mpic-timer-B.dtsi    |    7 ++-
 arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi            |    7 ++-
 4 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/open-pic.txt b/Documentation/devicetree/bindings/open-pic.txt
index 909a902..045c2e9 100644
--- a/Documentation/devicetree/bindings/open-pic.txt
+++ b/Documentation/devicetree/bindings/open-pic.txt
@@ -92,6 +92,52 @@ Example 2:
 
 * References
 
+* Open PIC global timers
+
+Required properties:
+- compatible: "open-pic,global-timer"
+
+- reg : Contains two regions.  The first is the timer frequency reporting
+  register for the group.  The second is the main timer register bank
+  (GTCCR, GTBCR, GTVPR, GTDR).
+
+- available-ranges: use <start count> style section to define which
+  timer interrupts can be used.  This property is optional; without this,
+  all timers within the group can be used.
+
+- interrupts: one interrupt per timer in the group, in order, starting
+  with timer zero.  If available-ranges is present, only the interrupts
+  that correspond to available timers shall be present.
+
+* Examples
+
+Example 1:
+
+	/* Note that this requires #interrupt-cells to be 4 */
+	timer: timer@010f0 {
+		compatible = "open-pic,global-timer";
+		device_type = "open-pic";
+		reg = <0x010f0 4 0x01100 0x100>;
+
+		/* Another AMP partition is using timer */
+		available-ranges = <2 2>;
+
+		interrupts = <2 0 3 0
+		              3 0 3 0>;
+	};
+
+Example 2:
+
+	timer: timer@010f0 {
+		compatible = "open-pic,global-timer";
+		device_type = "open-pic";
+		reg = <0x010f0 4 0x01100 0x100>;
+		interrupts = <0 0 3 0
+			      1 0 3 0
+			      2 0 3 0
+		              3 0 3 0>;
+	};
+
 [1] Power.org (TM) Standard for Embedded Power Architecture (TM) Platform
     Requirements (ePAPR), Version 1.0, July 2008.
     (http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf)
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt b/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
index df41958..5aafca0 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
@@ -1,13 +1,14 @@
 * Freescale MPIC timers
 
 Required properties:
-- compatible: "fsl,mpic-global-timer"
+- compatible: "fsl,global-timer"
 
-- reg : Contains two regions.  The first is the main timer register bank
-  (GTCCRxx, GTBCRxx, GTVPRxx, GTDRxx).  The second is the timer control
+- reg : Contains three regions.  The first is the timer frequency reporting
+  register (TFRRx) for the group.  The second is the main timer register
+  bank (GTCCRxx, GTBCRxx, GTVPRxx, GTDRxx).  The third is the timer control
   register (TCRx) for the group.
 
-- fsl,available-ranges: use <start count> style section to define which
+- available-ranges: use <start count> style section to define which
   timer interrupts can be used.  This property is optional; without this,
   all timers within the group can be used.
 
@@ -18,19 +19,21 @@ Required properties:
 Example:
 	/* Note that this requires #interrupt-cells to be 4 */
 	timer0: timer@41100 {
-		compatible = "fsl,mpic-global-timer";
-		reg = <0x41100 0x100 0x41300 4>;
+		compatible = "fsl,global-timer";
+		device_type = "open-pic";
+		reg = <0x410f0 4 0x41100 0x100 0x41300 4>;
 
 		/* Another AMP partition is using timers 0 and 1 */
-		fsl,available-ranges = <2 2>;
+		available-ranges = <2 2>;
 
 		interrupts = <2 0 3 0
 		              3 0 3 0>;
 	};
 
 	timer1: timer@42100 {
-		compatible = "fsl,mpic-global-timer";
-		reg = <0x42100 0x100 0x42300 4>;
+		compatible = "fsl,global-timer";
+		device_type = "open-pic";
+		reg = <0x420f0 4 0x42100 0x100 0x42300 4>;
 		interrupts = <4 0 3 0
 		              5 0 3 0
 		              6 0 3 0
diff --git a/arch/powerpc/boot/dts/fsl/pq3-mpic-timer-B.dtsi b/arch/powerpc/boot/dts/fsl/pq3-mpic-timer-B.dtsi
index 8734cff..01cd33c 100644
--- a/arch/powerpc/boot/dts/fsl/pq3-mpic-timer-B.dtsi
+++ b/arch/powerpc/boot/dts/fsl/pq3-mpic-timer-B.dtsi
@@ -32,9 +32,10 @@
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-timer@42100 {
-	compatible = "fsl,mpic-global-timer";
-	reg = <0x42100 0x100 0x42300 4>;
+timer@420f0 {
+	compatible = "fsl,global-timer";
+	device_type = "open-pic";
+	reg = <0x420f0 4 0x42100 0x100 0x42300 4>;
 	interrupts = <4 0 3 0
 		      5 0 3 0
 		      6 0 3 0
diff --git a/arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi b/arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi
index 71c30eb..c71d8e0 100644
--- a/arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi
+++ b/arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi
@@ -44,9 +44,10 @@ mpic: pic@40000 {
 	last-interrupt-source = <255>;
 };
 
-timer@41100 {
-	compatible = "fsl,mpic-global-timer";
-	reg = <0x41100 0x100 0x41300 4>;
+timer@410f0 {
+	compatible = "fsl,global-timer";
+	device_type = "open-pic";
+	reg = <0x410f0 4 0x41100 0x100 0x41300 4>;
 	interrupts = <0 0 3 0
 		      1 0 3 0
 		      2 0 3 0
-- 
1.7.5.1

Re: [PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer document

[Gala Kumar-B11780]

On Aug 10, 2012, at 12:53 AM, <Dongsheng.wang-KZfg59tc24xl57MIdRCFDg@public.gmane.org> <Dongsheng.wang-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:

> From: Wang Dongsheng <Dongsheng.Wang-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> 
> Add a description of the OPEN-PIC global timer in the OPEN-PIC document.
> 
> Moidfy mpic-timer document. 1.Add a TFRR register region. This register
> is written by software to report the clocking frequency of the PIC timers.
> 2.Add a device_type. The global timer in line with the OPEN-PIC specification.
> 
> Signed-off-by: Wang Dongsheng <Dongsheng.Wang-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Signed-off-by: Li Yang <leoli-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
> Documentation/devicetree/bindings/open-pic.txt     |   46 ++++++++++++++++++++

Let's separate out the open-pic.txt timer binding change into its own patch from the FSL timer binding & dtsi updates.

> .../devicetree/bindings/powerpc/fsl/mpic-timer.txt |   21 +++++----
> arch/powerpc/boot/dts/fsl/pq3-mpic-timer-B.dtsi    |    7 ++-
> arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi            |    7 ++-
> 4 files changed, 66 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/open-pic.txt b/Documentation/devicetree/bindings/open-pic.txt
> index 909a902..045c2e9 100644
> --- a/Documentation/devicetree/bindings/open-pic.txt
> +++ b/Documentation/devicetree/bindings/open-pic.txt
> @@ -92,6 +92,52 @@ Example 2:
> 
> * References
> 
> +* Open PIC global timers
> +
> +Required properties:
> +- compatible: "open-pic,global-timer"
> +
> +- reg : Contains two regions.  The first is the timer frequency reporting
> +  register for the group.  The second is the main timer register bank
> +  (GTCCR, GTBCR, GTVPR, GTDR).
> +
> +- available-ranges: use <start count> style section to define which
> +  timer interrupts can be used.  This property is optional; without this,
> +  all timers within the group can be used.
> +
> +- interrupts: one interrupt per timer in the group, in order, starting
> +  with timer zero.  If available-ranges is present, only the interrupts
> +  that correspond to available timers shall be present.
> +

If we are going to require device_type property it should be in the binding.

Based on the comments in ePAPR, I recommend dropping device_type from the timer binding.

> +* Examples
> +
> +Example 1:
> +
> +	/* Note that this requires #interrupt-cells to be 4 */
> +	timer: timer@010f0 {
> +		compatible = "open-pic,global-timer";
> +		device_type = "open-pic";
> +		reg = <0x010f0 4 0x01100 0x100>;
> +
> +		/* Another AMP partition is using timer */
> +		available-ranges = <2 2>;
> +
> +		interrupts = <2 0 3 0
> +		              3 0 3 0>;
> +	};
> +
> +Example 2:
> +
> +	timer: timer@010f0 {
> +		compatible = "open-pic,global-timer";
> +		device_type = "open-pic";
> +		reg = <0x010f0 4 0x01100 0x100>;
> +		interrupts = <0 0 3 0
> +			      1 0 3 0
> +			      2 0 3 0
> +		              3 0 3 0>;
> +	};
> +
> [1] Power.org (TM) Standard for Embedded Power Architecture (TM) Platform
>     Requirements (ePAPR), Version 1.0, July 2008.
>     (http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf)
> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt b/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
> index df41958..5aafca0 100644
> --- a/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
> +++ b/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
> @@ -1,13 +1,14 @@
> * Freescale MPIC timers
> 
> Required properties:
> -- compatible: "fsl,mpic-global-timer"
> +- compatible: "fsl,global-timer"

Why are renaming?.. also use of fsl,global-timer is to generic of a name for the this.

> 
> -- reg : Contains two regions.  The first is the main timer register bank
> -  (GTCCRxx, GTBCRxx, GTVPRxx, GTDRxx).  The second is the timer control
> +- reg : Contains three regions.  The first is the timer frequency reporting
> +  register (TFRRx) for the group.  The second is the main timer register
> +  bank (GTCCRxx, GTBCRxx, GTVPRxx, GTDRxx).  The third is the timer control
>   register (TCRx) for the group.
> 
> -- fsl,available-ranges: use <start count> style section to define which
> +- available-ranges: use <start count> style section to define which
>   timer interrupts can be used.  This property is optional; without this,
>   all timers within the group can be used.
> 
> @@ -18,19 +19,21 @@ Required properties:
> Example:
> 	/* Note that this requires #interrupt-cells to be 4 */
> 	timer0: timer@41100 {
> -		compatible = "fsl,mpic-global-timer";
> -		reg = <0x41100 0x100 0x41300 4>;
> +		compatible = "fsl,global-timer";
> +		device_type = "open-pic";
> +		reg = <0x410f0 4 0x41100 0x100 0x41300 4>;
> 
> 		/* Another AMP partition is using timers 0 and 1 */
> -		fsl,available-ranges = <2 2>;
> +		available-ranges = <2 2>;
> 
> 		interrupts = <2 0 3 0
> 		              3 0 3 0>;
> 	};
> 
> 	timer1: timer@42100 {
> -		compatible = "fsl,mpic-global-timer";
> -		reg = <0x42100 0x100 0x42300 4>;
> +		compatible = "fsl,global-timer";
> +		device_type = "open-pic";
> +		reg = <0x420f0 4 0x42100 0x100 0x42300 4>;
> 		interrupts = <4 0 3 0
> 		              5 0 3 0
> 		              6 0 3 0
> diff --git a/arch/powerpc/boot/dts/fsl/pq3-mpic-timer-B.dtsi b/arch/powerpc/boot/dts/fsl/pq3-mpic-timer-B.dtsi
> index 8734cff..01cd33c 100644
> --- a/arch/powerpc/boot/dts/fsl/pq3-mpic-timer-B.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/pq3-mpic-timer-B.dtsi
> @@ -32,9 +32,10 @@
>  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  */
> 
> -timer@42100 {
> -	compatible = "fsl,mpic-global-timer";
> -	reg = <0x42100 0x100 0x42300 4>;
> +timer@420f0 {
> +	compatible = "fsl,global-timer";

Shouldn't this be:

	compatible = "fsl,mpic-global-timer", "open-pic,global-timer";

> +	device_type = "open-pic";
> +	reg = <0x420f0 4 0x42100 0x100 0x42300 4>;
> 	interrupts = <4 0 3 0
> 		      5 0 3 0
> 		      6 0 3 0
> diff --git a/arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi b/arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi
> index 71c30eb..c71d8e0 100644
> --- a/arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi
> @@ -44,9 +44,10 @@ mpic: pic@40000 {
> 	last-interrupt-source = <255>;
> };
> 
> -timer@41100 {
> -	compatible = "fsl,mpic-global-timer";
> -	reg = <0x41100 0x100 0x41300 4>;
> +timer@410f0 {
> +	compatible = "fsl,global-timer";

same as above.

> +	device_type = "open-pic";
> +	reg = <0x410f0 4 0x41100 0x100 0x41300 4>;
> 	interrupts = <0 0 3 0
> 		      1 0 3 0
> 		      2 0 3 0
> -- 
> 1.7.5.1
> 
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer document

[Scott Wood]

On 08/10/2012 12:53 AM, Dongsheng.wang-KZfg59tc24xl57MIdRCFDg@public.gmane.org wrote:
> From: Wang Dongsheng <Dongsheng.Wang-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> 
> Add a description of the OPEN-PIC global timer in the OPEN-PIC document.
> 
> Moidfy mpic-timer document. 1.Add a TFRR register region. This register
> is written by software to report the clocking frequency of the PIC timers.
> 2.Add a device_type. The global timer in line with the OPEN-PIC specification.
> 
> Signed-off-by: Wang Dongsheng <Dongsheng.Wang-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Signed-off-by: Li Yang <leoli-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/open-pic.txt     |   46 ++++++++++++++++++++
>  .../devicetree/bindings/powerpc/fsl/mpic-timer.txt |   21 +++++----
>  arch/powerpc/boot/dts/fsl/pq3-mpic-timer-B.dtsi    |    7 ++-
>  arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi            |    7 ++-
>  4 files changed, 66 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/open-pic.txt b/Documentation/devicetree/bindings/open-pic.txt
> index 909a902..045c2e9 100644
> --- a/Documentation/devicetree/bindings/open-pic.txt
> +++ b/Documentation/devicetree/bindings/open-pic.txt
> @@ -92,6 +92,52 @@ Example 2:
>  
>  * References
>  
> +* Open PIC global timers
> +
> +Required properties:
> +- compatible: "open-pic,global-timer"

open-pic isn't a vendor (or software project that acts like a
pseudo-vendor) -- I'd go with "open-pic-global-timer".

> +- reg : Contains two regions.  The first is the timer frequency reporting
> +  register for the group.  The second is the main timer register bank
> +  (GTCCR, GTBCR, GTVPR, GTDR).

Why not just put clock-frequency in the node, instead of describing
TFRR?  I don't think U-Boot currently sets TFRR.

> +- available-ranges: use <start count> style section to define which
> +  timer interrupts can be used.  This property is optional; without this,
> +  all timers within the group can be used.
> +
> +- interrupts: one interrupt per timer in the group, in order, starting
> +  with timer zero.  If available-ranges is present, only the interrupts
> +  that correspond to available timers shall be present.
> +
> +* Examples
> +
> +Example 1:
> +
> +	/* Note that this requires #interrupt-cells to be 4 */
> +	timer: timer@010f0 {

Unit addres shouldn't have leading zeroes.

> +		compatible = "open-pic,global-timer";
> +		device_type = "open-pic";

Remove device_type.  Not only is it deprecated outside of real OF, it's
wrong -- this isn't an openpic, it's just a subset of it.

> +		reg = <0x010f0 4 0x01100 0x100>;
> +
> +		/* Another AMP partition is using timer */
> +		available-ranges = <2 2>;
>
> +
> +		interrupts = <2 0 3 0
> +		              3 0 3 0>;
> +	};
> +
> +Example 2:
> +
> +	timer: timer@010f0 {
> +		compatible = "open-pic,global-timer";
> +		device_type = "open-pic";
> +		reg = <0x010f0 4 0x01100 0x100>;
> +		interrupts = <0 0 3 0
> +			      1 0 3 0
> +			      2 0 3 0
> +		              3 0 3 0>;
> +	};

4-cell interrupt specifiers are specific to Freescale MPICs.  This means
there's no way to describe the timer interrupt on a non-Freescale
openpic.  Again, I suggest we not bother with this in the absence of an
actual need to support the timer on non-Freescale openpic in partitioned
scenarios.  The existing openpic node is sufficient to describe the
hardware in the absence of partitioning.   We could have an
"openpic-no-timer" property to indicate that we're describing it
separately, so that the absence of a timer node isn't ambiguous as to
whether it's an old tree or a partitioned scenario.  An fsl,mpic
compatible would imply openpic-no-timer.

Note that I believe many of the non-Freescale openpic nodes are going to
be found on systems with real Open Firmware, so we can't go changing the
device tree for them.

-Scott

RE: [PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer document

[Wang Dongsheng-B40534]

> 
> > a/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
> > b/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
> > index df41958..5aafca0 100644
> > --- a/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
> > +++ b/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
> > @@ -1,13 +1,14 @@
> > * Freescale MPIC timers
> >
> > Required properties:
> > -- compatible: "fsl,mpic-global-timer"
> > +- compatible: "fsl,global-timer"
> 
> Why are renaming?.. also use of fsl,global-timer is to generic of a name
> for the this.
> 
[Wang Dongsheng] "fsl,global-timer" is generic of a name.

RE: [PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer document

[Wang Dongsheng-B40534]

> > diff --git a/Documentation/devicetree/bindings/open-pic.txt
> > b/Documentation/devicetree/bindings/open-pic.txt
> > index 909a902..045c2e9 100644
> > --- a/Documentation/devicetree/bindings/open-pic.txt
> > +++ b/Documentation/devicetree/bindings/open-pic.txt
> > @@ -92,6 +92,52 @@ Example 2:
> >
> >  * References
> >
> > +* Open PIC global timers
> > +
> > +Required properties:
> > +- compatible: "open-pic,global-timer"
> 
> open-pic isn't a vendor (or software project that acts like a
> pseudo-vendor) -- I'd go with "open-pic-global-timer".
> 
[Wang Dongsheng] yes, "open-pic-global-timer" looks good.

> > +- reg : Contains two regions.  The first is the timer frequency
> > +reporting
> > +  register for the group.  The second is the main timer register bank
> > +  (GTCCR, GTBCR, GTVPR, GTDR).
> 
> Why not just put clock-frequency in the node, instead of describing TFRR?
> I don't think U-Boot currently sets TFRR.
> 
[Wang Dongsheng] If during startup U-Boot do not set TFRR that is unreasonable.
In Open-PIC this is not allowed. We used is the CCB frequency in FSL chip.
We do not need to care about the value of this register. But we must support 
TFRR.

> > +		reg = <0x010f0 4 0x01100 0x100>;
> > +
> > +		/* Another AMP partition is using timer */
> > +		available-ranges = <2 2>;
> >
> > +
> > +		interrupts = <2 0 3 0
> > +		              3 0 3 0>;
> > +	};
> > +
> > +Example 2:
> > +
> > +	timer: timer@010f0 {
> > +		compatible = "open-pic,global-timer";
> > +		device_type = "open-pic";
> > +		reg = <0x010f0 4 0x01100 0x100>;
> > +		interrupts = <0 0 3 0
> > +			      1 0 3 0
> > +			      2 0 3 0
> > +		              3 0 3 0>;
> > +	};
> 
> 4-cell interrupt specifiers are specific to Freescale MPICs.  This means
> there's no way to describe the timer interrupt on a non-Freescale openpic.
> Again, I suggest we not bother with this in the absence of an actual need
> to support the timer on non-Freescale openpic in partitioned scenarios.
> The existing openpic node is sufficient to describe the
> hardware in the absence of partitioning.   We could have an
> "openpic-no-timer" property to indicate that we're describing it
> separately, so that the absence of a timer node isn't ambiguous as to
> whether it's an old tree or a partitioned scenario.  An fsl,mpic
> compatible would imply openpic-no-timer.
> 
> Note that I believe many of the non-Freescale openpic nodes are going to
> be found on systems with real Open Firmware, so we can't go changing the
> device tree for them.
[Wang Dongsheng] In the Open-PIC specification, there are four timer.
		interrupts = <0 0 3 0
			      1 0 3 0
			      2 0 3 0
		              3 0 3 0>;

The "interrupts" just let user know there are four timers. Usage based "interrupts"
binding to change dts.

> 
> -Scott

Re: [PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer document

[Scott Wood]

On 08/13/2012 12:40 AM, Wang Dongsheng-B40534 wrote:
>>> diff --git a/Documentation/devicetree/bindings/open-pic.txt
>>> b/Documentation/devicetree/bindings/open-pic.txt
>>> index 909a902..045c2e9 100644
>>> --- a/Documentation/devicetree/bindings/open-pic.txt
>>> +++ b/Documentation/devicetree/bindings/open-pic.txt
>>> @@ -92,6 +92,52 @@ Example 2:
>>>
>>>  * References
>>>
>>> +* Open PIC global timers
>>> +
>>> +Required properties:
>>> +- compatible: "open-pic,global-timer"
>>
>> open-pic isn't a vendor (or software project that acts like a
>> pseudo-vendor) -- I'd go with "open-pic-global-timer".
>>
> [Wang Dongsheng] yes, "open-pic-global-timer" looks good.
> 
>>> +- reg : Contains two regions.  The first is the timer frequency
>>> +reporting
>>> +  register for the group.  The second is the main timer register bank
>>> +  (GTCCR, GTBCR, GTVPR, GTDR).
>>
>> Why not just put clock-frequency in the node, instead of describing TFRR?
>> I don't think U-Boot currently sets TFRR.
>>
> [Wang Dongsheng] If during startup U-Boot do not set TFRR that is unreasonable.

Too bad, it's what happens and we're not going to force users to update
U-Boot because of this.

>>> +Example 2:
>>> +
>>> +	timer: timer@010f0 {
>>> +		compatible = "open-pic,global-timer";
>>> +		device_type = "open-pic";
>>> +		reg = <0x010f0 4 0x01100 0x100>;
>>> +		interrupts = <0 0 3 0
>>> +			      1 0 3 0
>>> +			      2 0 3 0
>>> +		              3 0 3 0>;
>>> +	};
>>
>> 4-cell interrupt specifiers are specific to Freescale MPICs.  This means
>> there's no way to describe the timer interrupt on a non-Freescale openpic.
>> Again, I suggest we not bother with this in the absence of an actual need
>> to support the timer on non-Freescale openpic in partitioned scenarios.
>> The existing openpic node is sufficient to describe the
>> hardware in the absence of partitioning.   We could have an
>> "openpic-no-timer" property to indicate that we're describing it
>> separately, so that the absence of a timer node isn't ambiguous as to
>> whether it's an old tree or a partitioned scenario.  An fsl,mpic
>> compatible would imply openpic-no-timer.
>>
>> Note that I believe many of the non-Freescale openpic nodes are going to
>> be found on systems with real Open Firmware, so we can't go changing the
>> device tree for them.
> [Wang Dongsheng] In the Open-PIC specification, there are four timer.
> 		interrupts = <0 0 3 0
> 			      1 0 3 0
> 			      2 0 3 0
> 		              3 0 3 0>;
> 
> The "interrupts" just let user know there are four timers. Usage based "interrupts"
> binding to change dts.

I can't understand the above or how it's a response to what I wrote.

-Scott

RE: [PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer document

[Wang Dongsheng-B40534]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, August 14, 2012 1:40 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; benh@kernel.crashing.org; paulus@samba.org;
> linuxppc-dev@lists.ozlabs.org; devicetree-discuss@lists.ozlabs.org; Gala
> Kumar-B11780; Li Yang-R58472
> Subject: Re: [PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer
> document
> 
> On 08/13/2012 12:40 AM, Wang Dongsheng-B40534 wrote:
> >>> diff --git a/Documentation/devicetree/bindings/open-pic.txt
> >>> b/Documentation/devicetree/bindings/open-pic.txt
> >>> index 909a902..045c2e9 100644
> >>> --- a/Documentation/devicetree/bindings/open-pic.txt
> >>> +++ b/Documentation/devicetree/bindings/open-pic.txt
> >>> @@ -92,6 +92,52 @@ Example 2:
> >>>
> >>>  * References
> >>>
> >>> +* Open PIC global timers
> >>> +
> >>> +Required properties:
> >>> +- compatible: "open-pic,global-timer"
> >>
> >> open-pic isn't a vendor (or software project that acts like a
> >> pseudo-vendor) -- I'd go with "open-pic-global-timer".
> >>
> > [Wang Dongsheng] yes, "open-pic-global-timer" looks good.
> >
> >>> +- reg : Contains two regions.  The first is the timer frequency
> >>> +reporting
> >>> +  register for the group.  The second is the main timer register
> >>> +bank
> >>> +  (GTCCR, GTBCR, GTVPR, GTDR).
> >>
> >> Why not just put clock-frequency in the node, instead of describing
> TFRR?
> >> I don't think U-Boot currently sets TFRR.
> >>
> > [Wang Dongsheng] If during startup U-Boot do not set TFRR that is
> unreasonable.
> 
> Too bad, it's what happens and we're not going to force users to update
> U-Boot because of this.
> 
> >>> +Example 2:
> >>> +
> >>> +	timer: timer@010f0 {
> >>> +		compatible = "open-pic,global-timer";
> >>> +		device_type = "open-pic";
> >>> +		reg = <0x010f0 4 0x01100 0x100>;
> >>> +		interrupts = <0 0 3 0
> >>> +			      1 0 3 0
> >>> +			      2 0 3 0
> >>> +		              3 0 3 0>;
> >>> +	};
> >>
> >> 4-cell interrupt specifiers are specific to Freescale MPICs.  This
> >> means there's no way to describe the timer interrupt on a non-
> Freescale openpic.
> >> Again, I suggest we not bother with this in the absence of an actual
> >> need to support the timer on non-Freescale openpic in partitioned
> scenarios.
> >> The existing openpic node is sufficient to describe the
> >> hardware in the absence of partitioning.   We could have an
> >> "openpic-no-timer" property to indicate that we're describing it
> >> separately, so that the absence of a timer node isn't ambiguous as to
> >> whether it's an old tree or a partitioned scenario.  An fsl,mpic
> >> compatible would imply openpic-no-timer.
> >>
> >> Note that I believe many of the non-Freescale openpic nodes are going
> >> to be found on systems with real Open Firmware, so we can't go
> >> changing the device tree for them.
> > [Wang Dongsheng] In the Open-PIC specification, there are four timer.
> > 		interrupts = <0 0 3 0
> > 			      1 0 3 0
> > 			      2 0 3 0
> > 		              3 0 3 0>;
> >
> > The "interrupts" just let user know there are four timers. Usage based
> "interrupts"
> > binding to change dts.
> 
> I can't understand the above or how it's a response to what I wrote.
> 
[Wang Dongsheng] I mean this just to tell how many timers to support in Open-PIC
specification. If someone needs to write "interrupts" into dts, this must comply
with the specification of the interrupt to write. this is based on the pic driver
should be changed in different platforms.

> -Scott

Re: [PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer document

[Scott Wood]

On 08/13/2012 09:40 PM, Wang Dongsheng-B40534 wrote:
>>>> +Example 2:
>>>>> +
>>>>> +	timer: timer@010f0 {
>>>>> +		compatible = "open-pic,global-timer";
>>>>> +		device_type = "open-pic";
>>>>> +		reg = <0x010f0 4 0x01100 0x100>;
>>>>> +		interrupts = <0 0 3 0
>>>>> +			      1 0 3 0
>>>>> +			      2 0 3 0
>>>>> +		              3 0 3 0>;
>>>>> +	};
>>>>
>>>> 4-cell interrupt specifiers are specific to Freescale MPICs.  This
>>>> means there's no way to describe the timer interrupt on a non-
>> Freescale openpic.
>>>> Again, I suggest we not bother with this in the absence of an actual
>>>> need to support the timer on non-Freescale openpic in partitioned
>> scenarios.
>>>> The existing openpic node is sufficient to describe the
>>>> hardware in the absence of partitioning.   We could have an
>>>> "openpic-no-timer" property to indicate that we're describing it
>>>> separately, so that the absence of a timer node isn't ambiguous as to
>>>> whether it's an old tree or a partitioned scenario.  An fsl,mpic
>>>> compatible would imply openpic-no-timer.
>>>>
>>>> Note that I believe many of the non-Freescale openpic nodes are going
>>>> to be found on systems with real Open Firmware, so we can't go
>>>> changing the device tree for them.
>>> [Wang Dongsheng] In the Open-PIC specification, there are four timer.
>>> 		interrupts = <0 0 3 0
>>> 			      1 0 3 0
>>> 			      2 0 3 0
>>> 		              3 0 3 0>;
>>>
>>> The "interrupts" just let user know there are four timers. Usage based
>> "interrupts"
>>> binding to change dts.
>>
>> I can't understand the above or how it's a response to what I wrote.
>>
> [Wang Dongsheng] I mean this just to tell how many timers to support in Open-PIC
> specification. If someone needs to write "interrupts" into dts, this must comply
> with the specification of the interrupt to write. this is based on the pic driver
> should be changed in different platforms.

My point (beyond that examples provided should be valid for *some*
system) is there is no valid thing to put in the interrupts property
here when the interrupt controller is not "fsl,mpic", so this doesn't work.

-Scott

RE: [PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer document

[Wang Dongsheng-B40534]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, August 15, 2012 5:19 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; benh@kernel.crashing.org; paulus@samba.org;
> linuxppc-dev@lists.ozlabs.org; devicetree-discuss@lists.ozlabs.org; Gala
> Kumar-B11780; Li Yang-R58472
> Subject: Re: [PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer
> document
> 
> On 08/13/2012 09:40 PM, Wang Dongsheng-B40534 wrote:
> >>>> +Example 2:
> >>>>> +
> >>>>> +	timer: timer@010f0 {
> >>>>> +		compatible = "open-pic,global-timer";
> >>>>> +		device_type = "open-pic";
> >>>>> +		reg = <0x010f0 4 0x01100 0x100>;
> >>>>> +		interrupts = <0 0 3 0
> >>>>> +			      1 0 3 0
> >>>>> +			      2 0 3 0
> >>>>> +		              3 0 3 0>;
> >>>>> +	};
> >>>>
> >>>> 4-cell interrupt specifiers are specific to Freescale MPICs.  This
> >>>> means there's no way to describe the timer interrupt on a non-
> >> Freescale openpic.
> >>>> Again, I suggest we not bother with this in the absence of an actual
> >>>> need to support the timer on non-Freescale openpic in partitioned
> >> scenarios.
> >>>> The existing openpic node is sufficient to describe the
> >>>> hardware in the absence of partitioning.   We could have an
> >>>> "openpic-no-timer" property to indicate that we're describing it
> >>>> separately, so that the absence of a timer node isn't ambiguous as
> to
> >>>> whether it's an old tree or a partitioned scenario.  An fsl,mpic
> >>>> compatible would imply openpic-no-timer.
> >>>>
> >>>> Note that I believe many of the non-Freescale openpic nodes are
> going
> >>>> to be found on systems with real Open Firmware, so we can't go
> >>>> changing the device tree for them.
> >>> [Wang Dongsheng] In the Open-PIC specification, there are four timer.
> >>> 		interrupts = <0 0 3 0
> >>> 			      1 0 3 0
> >>> 			      2 0 3 0
> >>> 		              3 0 3 0>;
> >>>
> >>> The "interrupts" just let user know there are four timers. Usage
> based
> >> "interrupts"
> >>> binding to change dts.
> >>
> >> I can't understand the above or how it's a response to what I wrote.
> >>
> > [Wang Dongsheng] I mean this just to tell how many timers to support in
> Open-PIC
> > specification. If someone needs to write "interrupts" into dts, this
> must comply
> > with the specification of the interrupt to write. this is based on the
> pic driver
> > should be changed in different platforms.
> 
> My point (beyond that examples provided should be valid for *some*
> system) is there is no valid thing to put in the interrupts property
> here when the interrupt controller is not "fsl,mpic", so this doesn't
> work.
> 
[Wang Dongsheng] Fine, I will remove this document of Open-PIC global timer.
We only support mpic timer. Driver will be compatible with OPEN-PIC
specification. Let someone who cares about ordinary OpenPIC drivers add
support?

> -Scott