linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] powerpc: document the MPIC device tree binding
@ 2011-01-18  0:52 Meador Inge
  2011-01-18 20:31 ` Scott Wood
       [not found] ` <AANLkTi=QX4BfLvPfQDMOgmh90TtX4MQqio6AOZR8JKas@mail.gmail.com>
  0 siblings, 2 replies; 9+ messages in thread
From: Meador Inge @ 2011-01-18  0:52 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: minge, devicetree-discuss, Blanchard, Hollis

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.

Signed-off-by: Meador Inge <meador_inge@mentor.com>
CC: Hollis Blanchard <hollis_blanchard@mentor.com>
---
  Documentation/powerpc/dts-bindings/mpic.txt |   78 
+++++++++++++++++++++++++++
  1 files changed, 78 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/powerpc/dts-bindings/mpic.txt

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.  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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* RE: [PATCH 1/2] powerpc: document the MPIC device tree binding
       [not found] ` <AANLkTi=QX4BfLvPfQDMOgmh90TtX4MQqio6AOZR8JKas@mail.gmail.com>
@ 2011-01-18 20:21   ` Yoder Stuart-B08248
  2011-01-19 20:24     ` Meador Inge
  2011-01-19 22:14   ` Yoder Stuart-B08248
  1 sibling, 1 reply; 9+ messages in thread
From: Yoder Stuart-B08248 @ 2011-01-18 20:21 UTC (permalink / raw)
  To: Meador Inge; +Cc: Blanchard, Hollis, devicetree-discuss, linuxppc-dev

> From: Meador Inge <meador_inge@mentor.com>
> 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 <meador_inge@mentor.com>,
> devicetree-discuss@lists.ozlabs.org, "Blanchard, Hollis"
> <Hollis_Blanchard@mentor.com>
>=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 <meador_inge@mentor.com>
> CC: Hollis Blanchard <hollis_blanchard@mentor.com>
> ---
> =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.

> +++++++++++++++++++++++++++
> =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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] powerpc: document the MPIC device tree binding
  2011-01-18  0:52 [PATCH 1/2] powerpc: document the MPIC device tree binding Meador Inge
@ 2011-01-18 20:31 ` Scott Wood
       [not found] ` <AANLkTi=QX4BfLvPfQDMOgmh90TtX4MQqio6AOZR8JKas@mail.gmail.com>
  1 sibling, 0 replies; 9+ messages in thread
From: Scott Wood @ 2011-01-18 20:31 UTC (permalink / raw)
  To: Meador Inge; +Cc: Blanchard, Hollis, devicetree-discuss, linuxppc-dev

On Mon, 17 Jan 2011 18:52:24 -0600
Meador Inge <meador_inge@mentor.com> wrote:

> +** 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.

> +   - 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?

> +   - 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.

> +** 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.
> +

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] powerpc: document the MPIC device tree binding
  2011-01-18 20:21   ` Yoder Stuart-B08248
@ 2011-01-19 20:24     ` Meador Inge
  2011-01-19 20:38       ` Yoder Stuart-B08248
  0 siblings, 1 reply; 9+ messages in thread
From: Meador Inge @ 2011-01-19 20:24 UTC (permalink / raw)
  To: Yoder Stuart-B08248; +Cc: Blanchard, Hollis, devicetree-discuss, linuxppc-dev

On 01/18/2011 02:21 PM, Yoder Stuart-B08248 wrote:
>>   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.

>> +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?

>> +
>> +** 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.

>> +   - 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")'.


>> +   - 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.

>> +   - 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.

>> +** 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 1/2] powerpc: document the MPIC device tree binding
  2011-01-19 20:24     ` Meador Inge
@ 2011-01-19 20:38       ` Yoder Stuart-B08248
  0 siblings, 0 replies; 9+ messages in thread
From: Yoder Stuart-B08248 @ 2011-01-19 20:38 UTC (permalink / raw)
  To: Meador Inge; +Cc: Blanchard, Hollis, devicetree-discuss, linuxppc-dev



> -----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:
> >>   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
> >> +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.

> >> +
> >> +** 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
> >> +   - 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".

> >> +   - 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.

> >> +   - 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 alwa=
ys
> >> + 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
> >> + an
> >> +                        interrupt source.
> >
> > Should be 2, correct?
>=20
> Yup.
>=20
> >> +** 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
> >> + not
> >> +                         available for use and whose corresponding
> >> + vectors
> >> +                         should not be initialized.  A typical use ca=
se
> >> + for
> >> +                         this property is in AMP systems where multip=
le
> >> +                         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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 1/2] powerpc: document the MPIC device tree binding
       [not found] ` <AANLkTi=QX4BfLvPfQDMOgmh90TtX4MQqio6AOZR8JKas@mail.gmail.com>
  2011-01-18 20:21   ` Yoder Stuart-B08248
@ 2011-01-19 22:14   ` Yoder Stuart-B08248
  2011-01-20  0:08     ` Meador Inge
  1 sibling, 1 reply; 9+ messages in thread
From: Yoder Stuart-B08248 @ 2011-01-19 22:14 UTC (permalink / raw)
  To: Meador Inge
  Cc: Blanchard, Hollis, Wood Scott-B07421, devicetree-discuss, linuxppc-dev


> +** 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.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] powerpc: document the MPIC device tree binding
  2011-01-19 22:14   ` Yoder Stuart-B08248
@ 2011-01-20  0:08     ` Meador Inge
  2011-01-20 15:50       ` Yoder Stuart-B08248
  0 siblings, 1 reply; 9+ messages in thread
From: Meador Inge @ 2011-01-20  0:08 UTC (permalink / raw)
  To: Yoder Stuart-B08248
  Cc: Blanchard, Hollis, Wood Scott-B07421, devicetree-discuss, linuxppc-dev

On 01/19/2011 04:14 PM, Yoder Stuart-B08248 wrote:
>
>> +** 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.
>
> 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.


-- 
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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 1/2] powerpc: document the MPIC device tree binding
  2011-01-20  0:08     ` Meador Inge
@ 2011-01-20 15:50       ` Yoder Stuart-B08248
  2011-01-27 23:50         ` Meador Inge
  0 siblings, 1 reply; 9+ messages in thread
From: Yoder Stuart-B08248 @ 2011-01-20 15:50 UTC (permalink / raw)
  To: Meador Inge
  Cc: Blanchard, Hollis, Wood Scott-B07421, devicetree-discuss, linuxppc-dev



> -----Original Message-----
> From: Meador Inge [mailto:meador_inge@mentor.com]
> Sent: Wednesday, January 19, 2011 6:08 PM
> To: Yoder Stuart-B08248
> Cc: Wood Scott-B07421; 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/19/2011 04:14 PM, Yoder Stuart-B08248 wrote:
> >
> >> +** 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 multip=
le
> >> +                         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.
>=20
> This was discussed a little bit already [1, 2].  The MPIC driver currentl=
y
> 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 source=
s
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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] powerpc: document the MPIC device tree binding
  2011-01-20 15:50       ` Yoder Stuart-B08248
@ 2011-01-27 23:50         ` Meador Inge
  0 siblings, 0 replies; 9+ messages in thread
From: Meador Inge @ 2011-01-27 23:50 UTC (permalink / raw)
  To: Yoder Stuart-B08248
  Cc: Blanchard, Hollis, Wood Scott-B07421, devicetree-discuss, linuxppc-dev

On 01/20/2011 09:50 AM, Yoder Stuart-B08248 wrote:
>
>
>> -----Original Message-----
>> From: Meador Inge [mailto:meador_inge@mentor.com]
>> Sent: Wednesday, January 19, 2011 6:08 PM
>> To: Yoder Stuart-B08248
>> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; devicetree-
>> discuss@lists.ozlabs.org; Blanchard, Hollis
>> Subject: Re: [PATCH 1/2] powerpc: document the MPIC device tree binding
>>
>> On 01/19/2011 04:14 PM, Yoder Stuart-B08248 wrote:
>>>
>>>> +** 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.
>>>
>>> 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.
>
> 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.
>

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.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-01-27 23:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-18  0:52 [PATCH 1/2] powerpc: document the MPIC device tree binding Meador Inge
2011-01-18 20:31 ` Scott Wood
     [not found] ` <AANLkTi=QX4BfLvPfQDMOgmh90TtX4MQqio6AOZR8JKas@mail.gmail.com>
2011-01-18 20:21   ` Yoder Stuart-B08248
2011-01-19 20:24     ` Meador Inge
2011-01-19 20:38       ` Yoder Stuart-B08248
2011-01-19 22:14   ` Yoder Stuart-B08248
2011-01-20  0:08     ` Meador Inge
2011-01-20 15:50       ` Yoder Stuart-B08248
2011-01-27 23:50         ` Meador Inge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).