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

3 messages, 3 authors, 2011-01-19 · open the first message on its own page

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

From: Yoder Stuart-B08248 <hidden>
Date: 2011-01-18 20:21:33

From: Meador Inge <redacted>
Date: Mon, Jan 17, 2011 at 6:52 PM
Subject: [PATCH 1/2] powerpc: document the MPIC device tree binding
To: linuxppc-dev@lists.ozlabs.org
Cc: minge <redacted>,
devicetree-discuss@lists.ozlabs.org, "Blanchard, Hollis"
[off-list ref]
=20
=20
This binding documents several properties that have been in use for quite
some time, and adds one new property 'no-reset', which controls whether t=
he
MPIC should be reset during runtime initialization.
=20
Signed-off-by: Meador Inge <redacted>
CC: Hollis Blanchard <redacted>
---
=A0Documentation/powerpc/dts-bindings/mpic.txt | =A0 78
This is really the binding for an open-pic interrupt controller
and I think the name should reflect that-- open-pic.txt.
quoted hunk
+++++++++++++++++++++++++++
=A01 files changed, 78 insertions(+), 0 deletions(-)
=A0create mode 100644 Documentation/powerpc/dts-bindings/mpic.txt
=20
diff --git a/Documentation/powerpc/dts-bindings/mpic.txt
b/Documentation/powerpc/dts-bindings/mpic.txt
new file mode 100644
index 0000000..3a67919
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/mpic.txt
@@ -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. =A0This 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.=20
+
+** Required properties:
+
+ =A0 NOTE: Many of these descriptions were paraphrased from [1] to aid
+ =A0 =A0 =A0 =A0 readability.
+
+ =A0 - name : Specifies the name of the MPIC.
Drop this.  No DTS files use it.
+ =A0 - device_type : Specifies the device type of this MPIC. =A0The valu=
e
+ of this
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 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.
+ =A0 - reg : Specifies the base physical address(s) and size(s) of this
+ MPIC's
+ =A0 =A0 =A0 =A0 =A0 addressable register space.
+ =A0 - compatible : Specifies the compatibility list for the MPIC. =A0Th=
e
+ property
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0value 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.
+ =A0 - interrupt-controller : The presence of this property identifies
+ the node
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0as a MPIC. =A0No=
 property value should be
defined.
+ =A0 - #address-cells : Specifies the number of cells needed to encode a=
n
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0address. =A0The value of thi=
s property shall always
+ be 0
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0so that 'interrupt-map' node=
s do not have to
+ specify a
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0parent unit address.
+ =A0 - #interrupt-cells : Specifies the number of cells needed to encode
+ an
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0interrupt source.
Should be 2, correct?
+** Optional properties:
+
+ =A0 - no-reset : The presence of this property indicates that the MPIC
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0should not be reset during runtime initi=
alization.
+ =A0 - protected-sources : Specifies a list of interrupt sources that ar=
e
+ not
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 available for use and w=
hose corresponding
+ vectors
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 should not be initializ=
ed. =A0A typical use case
+ for
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 this property is in AMP=
 systems where multiple
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 independent operating s=
ystems need to share
+ the MPIC
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 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

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

From: Meador Inge <hidden>
Date: 2011-01-19 20:24:57

On 01/18/2011 02:21 PM, Yoder Stuart-B08248 wrote:
quoted
  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
+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.
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
+
+** 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
+   - 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
+   - 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
+   - 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.
Should be 2, correct?
Yup.
quoted
+** 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.
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.


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

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

From: Yoder Stuart-B08248 <hidden>
Date: 2011-01-19 20:38:51

-----Original Message-----
From: Meador Inge [mailto:meador_inge@mentor.com]
Sent: Wednesday, January 19, 2011 2:25 PM
To: Yoder Stuart-B08248
Cc: linuxppc-dev@lists.ozlabs.org; devicetree-discuss@lists.ozlabs.org;
Blanchard, Hollis
Subject: Re: [PATCH 1/2] powerpc: document the MPIC device tree binding
=20
On 01/18/2011 02:21 PM, Yoder Stuart-B08248 wrote:
quoted
quoted
  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.
=20
Yup, agreed.
=20
quoted
quoted
+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>

The proposed new properties really should go back into the ePAPR.
=20
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
quoted
+
+** 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.
=20
Done.
=20
quoted
quoted
+   - 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.
=20
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 a=
re
of the form:
=20
     np =3D of_find_node_by_type(NULL, "open-pic");
     if (np =3D=3D NULL)
        return;
=20
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=3D"open-pic", but in addition
we should accept the compatible=3D"open-pic".
quoted
quoted
+   - 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.
=20
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
quoted
+   - 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
quoted
+                      address.  The value of this property shall alwa=
ys
quoted
quoted
+ 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 encod=
e
quoted
quoted
+ an
+                        interrupt source.
Should be 2, correct?
=20
Yup.
=20
quoted
quoted
+** 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 a=
re
quoted
quoted
+ not
+                         available for use and whose corresponding
+ vectors
+                         should not be initialized.  A typical use ca=
se
quoted
quoted
+ for
+                         this property is in AMP systems where multip=
le
quoted
quoted
+                         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.
=20
OK, I will look into that.  Thanks.
I have a version 2 I hope to send out later today.

Stuart
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help