Skip to contentSkip to conversation

powerpc: document the MPIC device tree binding

PATCH In review Meador Inge 5 participants 9 messages Jan 18, 2011 – Jan 27, 2011 4 messages not in the archive Documentation

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 powerpc: document the MPIC device tree binding

This binding documents several properties that have been in use for quite some time, and adds one new property 'no-reset', which controls whether the MPIC should be reset during runtime initialization.

CC Hollis Blanchard <[email protected]>
---
 Documentation/powerpc/dts-bindings/mpic.txt |   78 
+++++++++++++++++++++++++++
 1 files changed, 78 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/powerpc/dts-bindings/mpic.txt
Documentation/powerpc/dts-bindings/mpic.txt new file +78 -0
new file mode 100644
index 0000000..3a67919
@@ -0,0 +1,78 @@
+* MPIC Binding
+
+This binding specifies what properties and child nodes must be available on
+the device tree representation of the "MPIC" interrupt controller.  This
+binding is based on the binding defined for Open PIC in [1] and is a superset
+of that binding.
+
+** Required properties:
+
+   NOTE: Many of these descriptions were paraphrased from [1] to aid
+         readability.
+
+   - name : Specifies the name of the MPIC.
+   - device_type : Specifies the device type of this MPIC.  The value of this
+                   property shall be "open-pic".
+   - reg : Specifies the base physical address(s) and size(s) of this MPIC's
+           addressable register space.
+   - compatible : Specifies the compatibility list for the MPIC.  The property
+                  value shall include "chrp,open-pic".
+   - interrupt-controller : The presence of this property identifies the node
+                            as a MPIC.  No property value should be defined.
+   - #address-cells : Specifies the number of cells needed to encode an
+                      address.  The value of this property shall always be 0
+                      so that 'interrupt-map' nodes do not have to specify a
+                      parent unit address.
+   - #interrupt-cells : Specifies the number of cells needed to encode an
+                        interrupt source.
+
+** Optional properties:
+
+   - no-reset : The presence of this property indicates that the MPIC
+                should not be reset during runtime initialization.
+   - protected-sources : Specifies a list of interrupt sources that are not
+                         available for use and whose corresponding vectors
+                         should not be initialized.  A typical use case for
+                         this property is in AMP systems where multiple
+                         independent operating systems need to share the MPIC
+                         without clobbering each other.
+
+** Example:
+
+	mpic: pic@40000 {
+        // This is an interrupt controller node.
+		interrupt-controller;
+
+        // No address cells so that 'interrupt-map' nodes which reference
+        // this MPIC node do not need a parent address specifier.
+		#address-cells = <0>;
+
+        // Two cell to encode interrupt sources.
+		#interrupt-cells = <2>;
+
+        // Offset address of 0x40000 and size of 0x40000.
+		reg = <0x40000 0x40000>;
+
+        // Compatible with Open PIC.
+		compatible = "chrp,open-pic";
+
+        // An Open PIC device.
+		device_type = "open-pic";
+
+        // The sources 0xb1 and 0xb2 are off limits for use and should not be
+        // initialized by the OS.
+		protected-sources = <0xb1 0xb2>;
+
+        // The MPIC should not be reset.
+		no-reset;
+	};
+
+* References
+
+[1] PowerPC Microprocessor Common Hardware Reference Platform (CHRP) Binding,
+    Version 1.8, 1998. Published by the Open Firmware Working Group.
+    (http://playground.sun.com/1275/bindings/chrp/chrp1_8a.ps)
+[2] Open Firmware Recommended Practice: Interrupt Mapping, Version 0.9. 1996.
+    Published by the Open Firmware Working Group.
+    (http://playground.sun.com/1275/practice/imap/imap0_9d.pdf)
+

-- 1.6.3.3

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): AANLkTi=QX4BfLvPfQDMOgmh90TtX4MQqio6AOZR8JKas-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#3 lore raw
Quoted 9 lines

From: Meador Inge <[email protected]>
Date: Mon, Jan 17, 2011 at 6:52 PM
Subject: [PATCH 1/2] powerpc: document the MPIC device tree binding
To: linuxppc-dev-uLR06cmDAlY/[email protected]
Cc: minge <[email protected]>, devicetree-discuss-uLR06cmDAlY/[email protected], "Blanchard, Hollis" <[email protected]>

Quoted 8 lines

This binding documents several properties that have been in use for quite some time, and adds one new property 'no-reset', which controls whether the MPIC should be reset during runtime initialization.

CC Hollis Blanchard <[email protected]>

---
 Documentation/powerpc/dts-bindings/mpic.txt |   78

This is really the binding for an open-pic interrupt controller and I think the name should reflect that-- open-pic.txt.

Quoted 11 lines

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

 1 files changed, 78 insertions(+), 0 deletions(-)

 create mode 100644 Documentation/powerpc/dts-bindings/mpic.txt

Documentation/powerpc/dts-bindings/mpic.txt new file +0 -0
new file mode 100644
index 0000000..3a67919

Documentation/powerpc/dts-bindings/mpic.txt line 6

Quoted 6 lines
Documentation/powerpc/dts-bindings/mpic.txt new file +6 -0
@@ -0,0 +1,78 @@
+* MPIC Binding
+
+This binding specifies what properties and child nodes must be
+available on the device tree representation of the "MPIC" interrupt
+controller.  This binding is based on the binding defined for Open PIC
+in [1] and is a superset of that binding.

I think it would be better to base this on the ePAPR binding which was based on the original chrp binding. Properties like "name" and "device_type" are deprecated not being used in flat device trees.

<http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf>

The proposed new properties really should go back into the ePAPR.

Documentation/powerpc/dts-bindings/mpic.txt line 13

Quoted 7 lines
Documentation/powerpc/dts-bindings/mpic.txt new file +7 -0
+
+** Required properties:
+
+   NOTE: Many of these descriptions were paraphrased from [1] to aid
+         readability.
+
+   - name : Specifies the name of the MPIC.

Drop this. No DTS files use it.

Documentation/powerpc/dts-bindings/mpic.txt line 16

Quoted 3 lines
Documentation/powerpc/dts-bindings/mpic.txt new file +3 -0
+   - device_type : Specifies the device type of this MPIC.  The value
+ of this
+                   property shall be "open-pic".

device_type is deprecated, since this is not real open-firmware. In practice the kernel is matching on device_type, but we want to move away from that to match on "compatible", just hasn't been implemented yet.

Documentation/powerpc/dts-bindings/mpic.txt line 22

Quoted 6 lines
Documentation/powerpc/dts-bindings/mpic.txt new file +6 -0
+   - reg : Specifies the base physical address(s) and size(s) of this
+ MPIC's
+           addressable register space.
+   - compatible : Specifies the compatibility list for the MPIC.  The
+ property
+                  value shall include "chrp,open-pic".

In the ePAPR we modified this to just "open-pic", because this has
nothing to do with chrp anymore. I think just "open-pic" is
what we want.

Quoted 5 lines
Documentation/powerpc/dts-bindings/mpic.txt new file +3 -0
+   - interrupt-controller : The presence of this property identifies
+ the node
+                            as a MPIC.  No property value should be

defined.
+   - #address-cells : Specifies the number of cells needed to encode an

Quoted 8 lines

+                      address.  The value of this property shall always
+ be 0
+                      so that 'interrupt-map' nodes do not have to
+ specify a
+                      parent unit address.
+   - #interrupt-cells : Specifies the number of cells needed to encode
+ an
+                        interrupt source.

Should be 2, correct?

Quoted 4 lines
Documentation/powerpc/dts-bindings/mpic.txt new file +4 -0
+** Optional properties:
+
+   - no-reset : The presence of this property indicates that the MPIC
+                should not be reset during runtime initialization.

Documentation/powerpc/dts-bindings/mpic.txt

Quoted 10 lines
Documentation/powerpc/dts-bindings/mpic.txt new file +10 -0
+   - protected-sources : Specifies a list of interrupt sources that are
+ not
+                         available for use and whose corresponding
+ vectors
+                         should not be initialized.  A typical use case
+ for
+                         this property is in AMP systems where multiple
+                         independent operating systems need to share
+ the MPIC
+                         without clobbering each other.

I do think you need to include the definition of interrupt
specifiers here. Feel free to cut/paste text from my
Freescale mpic binding.

Stuart

#4 lore raw
Quoted 4 lines · On 01/18/2011 02:21 PM, Yoder Stuart-B08248 wrote:
Quoted 1 line
  Documentation/powerpc/dts-bindings/mpic.txt |   78

This is really the binding for an open-pic interrupt controller and I think the name should reflect that-- open-pic.txt.

Yup, agreed.

Quoted 9 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+This binding specifies what properties and child nodes must be
+available on the device tree representation of the "MPIC" interrupt
+controller.  This binding is based on the binding defined for Open PIC
+in [1] and is a superset of that binding.

I think it would be better to base this on the ePAPR binding which was based on the original chrp binding. Properties like "name" and "device_type" are deprecated not being used in flat device trees.

Quoted 3 lines

<http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf>

The proposed new properties really should go back into the ePAPR.

I read portions of ePAPR while writing this binding and considered that.
My only worry was that ePAPR is focused on embedded systems and this
binding will have to cover non-embedded systems that exist in the kernel. However, perhaps that is not a legitimate concern?

Quoted 9 lines

Documentation/powerpc/dts-bindings/mpic.txt line 13

Quoted 7 lines
Documentation/powerpc/dts-bindings/mpic.txt new file +7 -0
+
+** Required properties:
+
+   NOTE: Many of these descriptions were paraphrased from [1] to aid
+         readability.
+
+   - name : Specifies the name of the MPIC.

Drop this. No DTS files use it.

Done.

Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+   - device_type : Specifies the device type of this MPIC.  The value
+ of this
+                   property shall be "open-pic".

device_type is deprecated, since this is not real open-firmware. In practice the kernel is matching on device_type, but we want to move away from that to match on "compatible", just hasn't been implemented yet.

I will drop this property with the expectation that the kernel will be fixed. From a quick grep of '.../arch/powerpc' it looks like most uses are of the form:

    np = of_find_node_by_type(NULL, "open-pic");
    if (np == NULL)
       return;

In most of these cases I suppose the 'of_find_node_by_type' calls could just be replaced with calls to 'of_find_compatible_node(NULL, "open-pic")'.

Quoted 10 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+   - reg : Specifies the base physical address(s) and size(s) of this
+ MPIC's
+           addressable register space.
+   - compatible : Specifies the compatibility list for the MPIC.  The
+ property
+                  value shall include "chrp,open-pic".

In the ePAPR we modified this to just "open-pic", because this has
nothing to do with chrp anymore. I think just "open-pic" is
what we want.

OK, but as a migration path we should allow the kernel to accept both (Scott mentioned this in another reply), but "open-pic" is the documented correct way.

Quoted 14 lines

Review

Quoted 13 lines
(quoted diff) +3 -0
+   - interrupt-controller : The presence of this property identifies
+ the node
+                            as a MPIC.  No property value should be

defined.

(quoted diff) +9 -0
+   - #address-cells : Specifies the number of cells needed to encode an
+                      address.  The value of this property shall always
+ be 0
+                      so that 'interrupt-map' nodes do not have to
+ specify a
+                      parent unit address.
+   - #interrupt-cells : Specifies the number of cells needed to encode
+ an
+                        interrupt source.
Quoted 1 line

Should be 2, correct?

Yup.

Quoted 10 lines
Quoted 10 lines
(quoted diff) +10 -0
+** Optional properties:
+
+   - no-reset : The presence of this property indicates that the MPIC
+                should not be reset during runtime initialization.
+   - protected-sources : Specifies a list of interrupt sources that are
+ not
+                         available for use and whose corresponding
+ vectors
+                         should not be initialized.  A typical use case
+ for
Quoted 8 lines

Documentation/powerpc/dts-bindings/mpic.txt line 38

Quoted 4 lines
Documentation/powerpc/dts-bindings/mpic.txt new file +4 -0
+                         this property is in AMP systems where multiple
+                         independent operating systems need to share
+ the MPIC
+                         without clobbering each other.

I do think you need to include the definition of interrupt
specifiers here. Feel free to cut/paste text from my
Freescale mpic binding.

OK, I will look into that. Thanks.

Signature · 2 lines
Meador Inge     | meador_inge AT mentor.com
Mentor Embedded | http://www.mentor.com/embedded-software
#5 lore raw
Quoted 30 lines

-----Original Message-----
From: Meador Inge [mailto:[email protected]]
Sent: Wednesday, January 19, 2011 2:25 PM
To: Yoder Stuart-B08248
Cc: [email protected]; [email protected]; Blanchard, Hollis
Subject: Re: [PATCH 1/2] powerpc: document the MPIC device tree binding

Quoted 4 lines · On 01/18/2011 02:21 PM, Yoder Stuart-B08248 wrote:
Quoted 1 line
  Documentation/powerpc/dts-bindings/mpic.txt |   78

This is really the binding for an open-pic interrupt controller and I think the name should reflect that-- open-pic.txt.

Yup, agreed.

Quoted 12 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+This binding specifies what properties and child nodes must be
+available on the device tree representation of the "MPIC" interrupt
+controller.  This binding is based on the binding defined for Open
+PIC in [1] and is a superset of that binding.

I think it would be better to base this on the ePAPR binding which was based on the original chrp binding. Properties like "name" and "device_type" are deprecated not being used in flat device trees.

<http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pd
f>

Quoted 6 lines
Quoted 1 line

The proposed new properties really should go back into the ePAPR.

I read portions of ePAPR while writing this binding and considered that.
My only worry was that ePAPR is focused on embedded systems and this
binding will have to cover non-embedded systems that exist in the kernel. However, perhaps that is not a legitimate concern?

The ePAPR tried to codify what was previously implemented in Linux, so I don't think lack of things like "name" and "device_type" in the binding are an issue.

Quoted 25 lines
Quoted 9 lines

Documentation/powerpc/dts-bindings/mpic.txt line 13

Quoted 7 lines
Documentation/powerpc/dts-bindings/mpic.txt new file +7 -0
+
+** Required properties:
+
+   NOTE: Many of these descriptions were paraphrased from [1] to aid
+         readability.
+
+   - name : Specifies the name of the MPIC.

Drop this. No DTS files use it.

Done.

Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+   - device_type : Specifies the device type of this MPIC.  The
+ value of this
+                   property shall be "open-pic".

device_type is deprecated, since this is not real open-firmware. In practice the kernel is matching on device_type, but we want to move away from that to match on "compatible", just hasn't been implemented yet.

I will drop this property with the expectation that the kernel will be fixed. From a quick grep of '.../arch/powerpc' it looks like most uses are of the form:

Quoted 6 lines
     np = of_find_node_by_type(NULL, "open-pic");
     if (np == NULL)
        return;

In most of these cases I suppose the 'of_find_node_by_type' calls could just be replaced with calls to 'of_find_compatible_node(NULL, "open-pic")'.

For backwards compatibility, we should continue to accept the old/deprecated device_type="open-pic", but in addition we should accept the compatible="open-pic".

Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
(quoted diff) +6 -0
+   - reg : Specifies the base physical address(s) and size(s) of this
+ MPIC's
+           addressable register space.
+   - compatible : Specifies the compatibility list for the MPIC.  The
+ property
+                  value shall include "chrp,open-pic".
Quoted 7 lines
Quoted 3 lines

In the ePAPR we modified this to just "open-pic", because this has
nothing to do with chrp anymore. I think just "open-pic" is
what we want.

OK, but as a migration path we should allow the kernel to accept both (Scott mentioned this in another reply), but "open-pic" is the documented correct way.

Right.

Quoted 37 lines
Quoted 14 lines

Review

Quoted 13 lines
(quoted diff) +3 -0
+   - interrupt-controller : The presence of this property identifies
+ the node
+                            as a MPIC.  No property value should be

defined.

(quoted diff) +9 -0
+   - #address-cells : Specifies the number of cells needed to encode an
+                      address.  The value of this property shall always
+ be 0
+                      so that 'interrupt-map' nodes do not have to
+ specify a
+                      parent unit address.
+   - #interrupt-cells : Specifies the number of cells needed to encode
+ an
+                        interrupt source.
Quoted 1 line

Should be 2, correct?

Yup.

Quoted 10 lines
Quoted 10 lines
(quoted diff) +10 -0
+** Optional properties:
+
+   - no-reset : The presence of this property indicates that the MPIC
+                should not be reset during runtime initialization.
+   - protected-sources : Specifies a list of interrupt sources that are
+ not
+                         available for use and whose corresponding
+ vectors
+                         should not be initialized.  A typical use case
+ for
Quoted 8 lines

Documentation/powerpc/dts-bindings/mpic.txt line 38

Quoted 4 lines
Documentation/powerpc/dts-bindings/mpic.txt new file +4 -0
+                         this property is in AMP systems where multiple
+                         independent operating systems need to share
+ the MPIC
+                         without clobbering each other.

I do think you need to include the definition of interrupt
specifiers here. Feel free to cut/paste text from my
Freescale mpic binding.

Quoted 1 line

OK, I will look into that. Thanks.

I have a version 2 I hope to send out later today.

Stuart

#6 lore raw
In reply to a message not in the archive
Quoted 5 lines
(quoted diff) +4 -0
+** Optional properties:
+
+   - no-reset : The presence of this property indicates that the MPIC
+                should not be reset during runtime initialization.

Review

Quoted 10 lines
(quoted diff) +10 -0
+   - protected-sources : Specifies a list of interrupt sources that are
+ not
+                         available for use and whose corresponding
+ vectors
+                         should not be initialized.  A typical use case
+ for
+                         this property is in AMP systems where multiple
+                         independent operating systems need to share
+ the MPIC
+                         without clobbering each other.

Is "protected-sources" really needed for AMP systems to tell the OSes not to clobber each other? Won't each OS be given a device tree with only its interrupt sources? ...so you know what you are allowed to touch.

Stuart

A message that is not in the archive (sent off-list or not yet received): 9F6FE96B71CF29479FF1CDC8046E150306BF11-TcFNo7jSaXM0vywKSws3iq4g8xLGJsHaLnY5E4hWTkheoWH0uzbU5w@public.gmane.org

#8 lore raw
Quoted 12 lines · On 01/19/2011 04:14 PM, Yoder Stuart-B08248 wrote:
Quoted 12 lines
(quoted diff) +11 -0
+** Optional properties:
+
+   - no-reset : The presence of this property indicates that the MPIC
+                should not be reset during runtime initialization.
+   - protected-sources : Specifies a list of interrupt sources that are
+ not
+                         available for use and whose corresponding
+ vectors
+                         should not be initialized.  A typical use case
+ for
+                         this property is in AMP systems where multiple
Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+                         independent operating systems need to share
+ the MPIC
+                         without clobbering each other.

Is "protected-sources" really needed for AMP systems to tell the OSes not to clobber each other? Won't each OS be given a device tree with only its interrupt sources? ...so you know what you are allowed to touch.

This was discussed a little bit already [1, 2]. The MPIC driver currently initializes the VECPRI register for all interrupt sources, which can lead to the aforementioned clobbering.

Signature · 5 lines
Meador Inge     | meador_inge AT mentor.com
Mentor Embedded | http://www.mentor.com/embedded-software

[1] http://lists.ozlabs.org/pipermail/devicetree-discuss/2010-December/003850.html
[2] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/003936.html
#9 lore raw
Quoted 32 lines

-----Original Message-----
From: Meador Inge [mailto:[email protected]]
Sent: Wednesday, January 19, 2011 6:08 PM
To: Yoder Stuart-B08248
Cc: Wood Scott-B07421; [email protected]; devicetree- [email protected]; Blanchard, Hollis
Subject: Re: [PATCH 1/2] powerpc: document the MPIC device tree binding

Quoted 12 lines · On 01/19/2011 04:14 PM, Yoder Stuart-B08248 wrote:
Quoted 12 lines
(quoted diff) +11 -0
+** Optional properties:
+
+   - no-reset : The presence of this property indicates that the MPIC
+                should not be reset during runtime initialization.
+   - protected-sources : Specifies a list of interrupt sources that
+ are not
+                         available for use and whose corresponding
+ vectors
+                         should not be initialized.  A typical use
+ case for
+                         this property is in AMP systems where multiple
Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+                         independent operating systems need to share
+ the MPIC
+                         without clobbering each other.

Is "protected-sources" really needed for AMP systems to tell the OSes not to clobber each other? Won't each OS be given a device tree with only its interrupt sources? ...so you know what you are allowed to touch.

Quoted 3 lines

This was discussed a little bit already [1, 2]. The MPIC driver currently initializes the VECPRI register for all interrupt sources, which can lead to the aforementioned clobbering.

For sources that are protected and not to be touched, it seems that the other need is for the OS to know (be guaranteed?) that those sources have been put in state where the source is masked
or directed to other cores. You can't have interrupts occurring
on sources that you are not allowed to initialize.

So the "no-reset" property could potentially cover this as well-- if it was defined to mean "don't reset the mpic" and boot firmware has put all sources
in a sane initial state. And we wouldn't need the "protected-sources"
property any longer.

Stuart

Right...so it would seem that the no-reset property if prop

A message that is not in the archive (sent off-list or not yet received): 9F6FE96B71CF29479FF1CDC8046E150307A5C8-TcFNo7jSaXM0vywKSws3iq4g8xLGJsHaLnY5E4hWTkheoWH0uzbU5w@public.gmane.org

#11 lore raw
Quoted 42 lines · On 01/20/2011 09:50 AM, Yoder Stuart-B08248 wrote:
Quoted 32 lines

-----Original Message-----
From: Meador Inge [mailto:[email protected]]
Sent: Wednesday, January 19, 2011 6:08 PM
To: Yoder Stuart-B08248
Cc: Wood Scott-B07421; linuxppc-dev-uLR06cmDAlY/[email protected]; devicetree- discuss-uLR06cmDAlY/[email protected]; Blanchard, Hollis
Subject: Re: [PATCH 1/2] powerpc: document the MPIC device tree binding

Quoted 12 lines · On 01/19/2011 04:14 PM, Yoder Stuart-B08248 wrote:
Quoted 12 lines
(quoted diff) +11 -0
+** Optional properties:
+
+   - no-reset : The presence of this property indicates that the MPIC
+                should not be reset during runtime initialization.
+   - protected-sources : Specifies a list of interrupt sources that
+ are not
+                         available for use and whose corresponding
+ vectors
+                         should not be initialized.  A typical use
+ case for
+                         this property is in AMP systems where multiple
Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+                         independent operating systems need to share
+ the MPIC
+                         without clobbering each other.

Is "protected-sources" really needed for AMP systems to tell the OSes not to clobber each other? Won't each OS be given a device tree with only its interrupt sources? ...so you know what you are allowed to touch.

Quoted 3 lines

This was discussed a little bit already [1, 2]. The MPIC driver currently initializes the VECPRI register for all interrupt sources, which can lead to the aforementioned clobbering.

For sources that are protected and not to be touched, it seems that the other need is for the OS to know (be guaranteed?) that those sources have been put in state where the source is masked
or directed to other cores. You can't have interrupts occurring
on sources that you are not allowed to initialize.

Quoted 5 lines

So the "no-reset" property could potentially cover this as well-- if it was defined to mean "don't reset the mpic" and boot firmware has put all sources
in a sane initial state. And we wouldn't need the "protected-sources"
property any longer.

This seems reasonable to me. If "no-reset" is there, then we will not reset the MPIC and *only* sources explicitly listed in "interrupts" properties are up for any sort of initialization (e.g. the VECPRI init).
If "no-reset" is not there, then anything is free game.

In terms of implementation, I think we can (1) pull the protected sources code, (2) keep the VECPRI initialization in 'mpic_init' from happening when "no-reset" is present, and (3) "lazily" perform the VECPRI initialization in 'mpic_host_map' (this way only sources mentioned in the device tree are initialized).

I will send out a patch with these updates tomorrow. I also CC'd Ben, who wrote the original protected sources work, to make sure something about the original use case is not being missed.

Signature · 2 lines
Meador Inge     | meador_inge AT mentor.com
Mentor Embedded | http://www.mentor.com/embedded-software
#12 lore raw
In reply to Meador Inge

Documentation/powerpc/dts-bindings/mpic.txt line 13

Quoted 6 lines · On Mon, 17 Jan 2011 18:52:24 -0600 Meador Inge <[email protected]> wrote:
Documentation/powerpc/dts-bindings/mpic.txt new file +6 -0
+** Required properties:
+
+   NOTE: Many of these descriptions were paraphrased from [1] to aid
+         readability.
+
+   - name : Specifies the name of the MPIC.

"name" isn't really a property with flat trees. The appropriate node name, according to the Generic Names recommendation and ePAPR, is interrupt-controller.

Quoted 3 lines

+ - device_type : Specifies the device type of this MPIC. The value
of this
+ property shall be "open-pic".

Can we drop device_type, and fix the kernel to look for compatible instead?

Quoted 3 lines

+ - compatible : Specifies the compatibility list for the MPIC. The
property
+ value shall include "chrp,open-pic".

ePAPR wants just "open-pic". And while chrp,open-pic is common in existing trees, only one platform currently checks for it.

I'd make in "open-pic" in the binding, and have the kernel accept either one.

Review

Quoted 13 lines
Documentation/powerpc/dts-bindings/mpic.txt new file +5 -0
+** Optional properties:
+
+   - no-reset : The presence of this property indicates that the MPIC
+                should not be reset during runtime initialization.
+   - protected-sources : Specifies a list of interrupt sources that are 

not

Documentation/powerpc/dts-bindings/mpic.txt new file +4 -0
+                         available for use and whose corresponding vectors
+                         should not be initialized.  A typical use case for
+                         this property is in AMP systems where multiple
+                         independent operating systems need to share 

the MPIC

(quoted diff) +2 -0
+                         without clobbering each other.
+

Can we define no-reset as meaning that all vectors are in a sane state (either directed at other cores, or masked)?

If we do that, maybe we can get rid of protected-sources.

-Scott