[v2,2/3] powerpc: document the Open PIC 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
Open PIC should be reset during runtime initialization.

The general formatting and interrupt specifier definition is based off of
Stuart Yoder's FSL MPIC binding.

Signed-off-by: Meador Inge <meador_inge@mentor.com>
CC: Hollis Blanchard <hollis_blanchard@mentor.com>
CC: Stuart Yoder <stuart.yoder@freescale.com>
---
 Documentation/powerpc/dts-bindings/open-pic.txt |  115 +++++++++++++++++++++++
 1 files changed, 115 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/powerpc/dts-bindings/open-pic.txt

On Wed, Feb 2, 2011 at 6:51 PM, Meador Inge <meador_inge@mentor.com> wrote:
> 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
> Open PIC should be reset during runtime initialization.
>
> The general formatting and interrupt specifier definition is based off of
> Stuart Yoder's FSL MPIC binding.
>
> Signed-off-by: Meador Inge <meador_inge@mentor.com>
> CC: Hollis Blanchard <hollis_blanchard@mentor.com>
> CC: Stuart Yoder <stuart.yoder@freescale.com>
> ---
>  Documentation/powerpc/dts-bindings/open-pic.txt |  115 +++++++++++++++++++++++
>  1 files changed, 115 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/powerpc/dts-bindings/open-pic.txt
>
> diff --git a/Documentation/powerpc/dts-bindings/open-pic.txt b/Documentation/powerpc/dts-bindings/open-pic.txt
> new file mode 100644
> index 0000000..447ef65
> --- /dev/null
> +++ b/Documentation/powerpc/dts-bindings/open-pic.txt
> @@ -0,0 +1,115 @@
> +* Open PIC Binding
> +
> +This binding specifies what properties must be available in the device tree
> +representation of an Open PIC compliant interrupt controller.  This binding is
> +based on the binding defined for Open PIC in [1] and is a superset of that
> +binding.
> +
> +PROPERTIES
> +
> +  NOTE: Many of these descriptions were paraphrased here from [1] to aid
> +        readability.
> +
> +  - compatible
> +      Usage: required
> +      Value type: <string>
> +      Definition: Specifies the compatibility list for the PIC.  The
> +          property value shall include "open-pic".
> +
> +  - reg
> +      Usage: required
> +      Value type: <prop-encoded-array>
> +      Definition: Specifies the base physical address(s) and size(s) of this
> +          PIC's addressable register space.
> +
> +  - interrupt-controller
> +      Usage: required
> +      Value type: <empty>
> +      Definition: The presence of this property identifies the node
> +          as an Open PIC.  No property value should be defined.
> +
> +  - #interrupt-cells
> +      Usage: required
> +      Value type: <u32>
> +      Definition: Specifies the number of cells needed to encode an
> +          interrupt source.  Shall be 2.
> +
> +  - #address-cells
> +      Usage: required
> +      Value type: <u32>
> +      Definition: Specifies the number of cells needed to encode an
> +          address.  The value of this property shall always be 0.
> +          As such, 'interrupt-map' nodes do not have to specify a
> +          parent unit address.
> +
> +  - no-reset
> +      Usage: optional
> +      Value type: <empty>
> +      Definition: The presence of this property indicates that the PIC
> +          should not be reset during runtime initialization.  The presence of
> +          this property also mandates that any initialization related to
> +          interrupt sources shall be limited to sources explicitly referenced
> +          in the device tree.

Please follow the lead set by the other binding documentation which is
more concise and tends to be of the form:

    Required properties:
        - reg : <description>
        - interrupt-controller : <description>

    Optional Properties:
        - no-reset : blah

I'm considering formalizing the binding format so that fully specified
and cross-referenced documentation can be generated from the bindings
directory.

Also, to avoid the potential of a future namespace collision, it would
not be a bad idea to name this openpic-no-reset or something that
makes it clear that this is a binding specific property.  "no-reset"
sounds generic enough to give me pause.

> +
> +INTERRUPT SPECIFIER DEFINITION
> +
> +  Interrupt specifiers consists of 2 cells encoded as
> +  follows:
> +
> +   <1st-cell>   interrupt-number
> +
> +                Identifies the interrupt source.
> +
> +   <2nd-cell>   level-sense information, encoded as follows:
> +                    0 = low-to-high edge triggered
> +                    1 = active low level-sensitive
> +                    2 = active high level-sensitive
> +                    3 = high-to-low edge triggered
> +
> +EXAMPLE 1
> +
> +    /*
> +     * An Open PIC interrupt controller
> +     */
> +       mpic: pic@40000 {
> +        // This is an interrupt controller node.
> +               interrupt-controller;
> +
> +        // No address cells so that 'interrupt-map' nodes which reference
> +        // this Open PIC node do not need a parent address specifier.
> +               #address-cells = <0>;
> +
> +        // Two cells to encode interrupt sources.
> +               #interrupt-cells = <2>;
> +
> +        // Offset address of 0x40000 and size of 0x40000.
> +               reg = <0x40000 0x40000>;
> +
> +        // Compatible with Open PIC.
> +               compatible = "open-pic";
> +
> +        // The PIC should not be reset.
> +               no-reset;
> +       };
> +
> +EXAMPLE 2
> +
> +    /*
> +     * An interrupt generating device that is wired to an Open PIC.
> +     */
> +    serial0: serial@4500 {
> +        // Interrupt source '42' that is active high level-sensitive.
> +        // Note that there are only two cells as specified in the interrupt
> +        // parent's '#interrupt-cells' property.
> +        interrupts = <42 2>;
> +
> +        // The interrupt controller that this device is wired to.
> +        interrupt-parent = <&mpic>;
> +    };
> +
> +REFERENCES
> +
> +[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)
> +
> --
> 1.6.3.3
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>

On 02/03/2011 09:56 AM, Grant Likely wrote:
> On Wed, Feb 2, 2011 at 6:51 PM, Meador Inge<meador_inge@mentor.com>  wrote:
>> 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
>> Open PIC should be reset during runtime initialization.
>>
>> The general formatting and interrupt specifier definition is based off of
>> Stuart Yoder's FSL MPIC binding.
>>
>> Signed-off-by: Meador Inge<meador_inge@mentor.com>
>> CC: Hollis Blanchard<hollis_blanchard@mentor.com>
>> CC: Stuart Yoder<stuart.yoder@freescale.com>
>> ---
>>   Documentation/powerpc/dts-bindings/open-pic.txt |  115 +++++++++++++++++++++++
>>   1 files changed, 115 insertions(+), 0 deletions(-)
>>   create mode 100644 Documentation/powerpc/dts-bindings/open-pic.txt
>>
>> diff --git a/Documentation/powerpc/dts-bindings/open-pic.txt b/Documentation/powerpc/dts-bindings/open-pic.txt
>> new file mode 100644
>> index 0000000..447ef65
>> --- /dev/null
>> +++ b/Documentation/powerpc/dts-bindings/open-pic.txt
>> @@ -0,0 +1,115 @@
>> +* Open PIC Binding
>> +
>> +This binding specifies what properties must be available in the device tree
>> +representation of an Open PIC compliant interrupt controller.  This binding is
>> +based on the binding defined for Open PIC in [1] and is a superset of that
>> +binding.
>> +
>> +PROPERTIES
>> +
>> +  NOTE: Many of these descriptions were paraphrased here from [1] to aid
>> +        readability.
>> +
>> +  - compatible
>> +      Usage: required
>> +      Value type:<string>
>> +      Definition: Specifies the compatibility list for the PIC.  The
>> +          property value shall include "open-pic".
>> +
>> +  - reg
>> +      Usage: required
>> +      Value type:<prop-encoded-array>
>> +      Definition: Specifies the base physical address(s) and size(s) of this
>> +          PIC's addressable register space.
>> +
>> +  - interrupt-controller
>> +      Usage: required
>> +      Value type:<empty>
>> +      Definition: The presence of this property identifies the node
>> +          as an Open PIC.  No property value should be defined.
>> +
>> +  - #interrupt-cells
>> +      Usage: required
>> +      Value type:<u32>
>> +      Definition: Specifies the number of cells needed to encode an
>> +          interrupt source.  Shall be 2.
>> +
>> +  - #address-cells
>> +      Usage: required
>> +      Value type:<u32>
>> +      Definition: Specifies the number of cells needed to encode an
>> +          address.  The value of this property shall always be 0.
>> +          As such, 'interrupt-map' nodes do not have to specify a
>> +          parent unit address.
>> +
>> +  - no-reset
>> +      Usage: optional
>> +      Value type:<empty>
>> +      Definition: The presence of this property indicates that the PIC
>> +          should not be reset during runtime initialization.  The presence of
>> +          this property also mandates that any initialization related to
>> +          interrupt sources shall be limited to sources explicitly referenced
>> +          in the device tree.
>
> Please follow the lead set by the other binding documentation which is
> more concise and tends to be of the form:
>
>      Required properties:
>          - reg :<description>
>          - interrupt-controller :<description>
>
>      Optional Properties:
>          - no-reset : blah

OK, will do.  The one thing that I like about the other format, though, 
is that it specifies the value type.  That is a useful addition.

> I'm considering formalizing the binding format so that fully specified
> and cross-referenced documentation can be generated from the bindings
> directory.

Formalizing the binding format would be great.  Perhaps we should add a 
HOWTO write a new binding document to the "Documentation" directory? 
The would be a great place to capture some of the common pitfalls that 
have been coming up on the list lately (versioned compatibility tags, 
for example).

> Also, to avoid the potential of a future namespace collision, it would
> not be a bad idea to name this openpic-no-reset or something that
> makes it clear that this is a binding specific property.  "no-reset"
> sounds generic enough to give me pause.

Isn't that a little redundant, though (e.g. 
"/soc/pic/openpic-no-reset")?  It is already scoped to the PIC node:

    mpic: pic@40000 {
       compatible = "open-pic";
       no-reset;
    };

Or are you worried that someone will find the wrong "no-reset" property 
when searching from a location higher in the tree than the PIC node?

I don't have a serious objection to the idea, but it seems slightly odd 
to partially flatten the hierarchy back into the property names.  On the 
other hand, I do see the practical consideration of having a more unique 
property which might prevent programming confusion/errors.

On Thu, Feb 3, 2011 at 9:29 AM, Meador Inge <meador_inge@mentor.com> wrote:
> On 02/03/2011 09:56 AM, Grant Likely wrote:
>>
>> On Wed, Feb 2, 2011 at 6:51 PM, Meador Inge<meador_inge@mentor.com>
>>  wrote:
>>>
>>> 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
>>> Open PIC should be reset during runtime initialization.
>>>
>>> The general formatting and interrupt specifier definition is based off of
>>> Stuart Yoder's FSL MPIC binding.
>>>
>>> Signed-off-by: Meador Inge<meador_inge@mentor.com>
>>> CC: Hollis Blanchard<hollis_blanchard@mentor.com>
>>> CC: Stuart Yoder<stuart.yoder@freescale.com>
>>> ---
>>>  Documentation/powerpc/dts-bindings/open-pic.txt |  115
>>> +++++++++++++++++++++++
>>>  1 files changed, 115 insertions(+), 0 deletions(-)
>>>  create mode 100644 Documentation/powerpc/dts-bindings/open-pic.txt
>>>
>>> diff --git a/Documentation/powerpc/dts-bindings/open-pic.txt
>>> b/Documentation/powerpc/dts-bindings/open-pic.txt
>>> new file mode 100644
>>> index 0000000..447ef65
>>> --- /dev/null
>>> +++ b/Documentation/powerpc/dts-bindings/open-pic.txt
>>> @@ -0,0 +1,115 @@
>>> +* Open PIC Binding
>>> +
>>> +This binding specifies what properties must be available in the device
>>> tree
>>> +representation of an Open PIC compliant interrupt controller.  This
>>> binding is
>>> +based on the binding defined for Open PIC in [1] and is a superset of
>>> that
>>> +binding.
>>> +
>>> +PROPERTIES
>>> +
>>> +  NOTE: Many of these descriptions were paraphrased here from [1] to aid
>>> +        readability.
>>> +
>>> +  - compatible
>>> +      Usage: required
>>> +      Value type:<string>
>>> +      Definition: Specifies the compatibility list for the PIC.  The
>>> +          property value shall include "open-pic".
>>> +
>>> +  - reg
>>> +      Usage: required
>>> +      Value type:<prop-encoded-array>
>>> +      Definition: Specifies the base physical address(s) and size(s) of
>>> this
>>> +          PIC's addressable register space.
>>> +
>>> +  - interrupt-controller
>>> +      Usage: required
>>> +      Value type:<empty>
>>> +      Definition: The presence of this property identifies the node
>>> +          as an Open PIC.  No property value should be defined.
>>> +
>>> +  - #interrupt-cells
>>> +      Usage: required
>>> +      Value type:<u32>
>>> +      Definition: Specifies the number of cells needed to encode an
>>> +          interrupt source.  Shall be 2.
>>> +
>>> +  - #address-cells
>>> +      Usage: required
>>> +      Value type:<u32>
>>> +      Definition: Specifies the number of cells needed to encode an
>>> +          address.  The value of this property shall always be 0.
>>> +          As such, 'interrupt-map' nodes do not have to specify a
>>> +          parent unit address.
>>> +
>>> +  - no-reset
>>> +      Usage: optional
>>> +      Value type:<empty>
>>> +      Definition: The presence of this property indicates that the PIC
>>> +          should not be reset during runtime initialization.  The
>>> presence of
>>> +          this property also mandates that any initialization related to
>>> +          interrupt sources shall be limited to sources explicitly
>>> referenced
>>> +          in the device tree.
>>
>> Please follow the lead set by the other binding documentation which is
>> more concise and tends to be of the form:
>>
>>     Required properties:
>>         - reg :<description>
>>         - interrupt-controller :<description>
>>
>>     Optional Properties:
>>         - no-reset : blah
>
> OK, will do.  The one thing that I like about the other format, though, is
> that it specifies the value type.  That is a useful addition.
>
>> I'm considering formalizing the binding format so that fully specified
>> and cross-referenced documentation can be generated from the bindings
>> directory.
>
> Formalizing the binding format would be great.  Perhaps we should add a
> HOWTO write a new binding document to the "Documentation" directory? The
> would be a great place to capture some of the common pitfalls that have been
> coming up on the list lately (versioned compatibility tags, for example).
>
>> Also, to avoid the potential of a future namespace collision, it would
>> not be a bad idea to name this openpic-no-reset or something that
>> makes it clear that this is a binding specific property.  "no-reset"
>> sounds generic enough to give me pause.
>
> Isn't that a little redundant, though (e.g. "/soc/pic/openpic-no-reset")?
>  It is already scoped to the PIC node:
>
>   mpic: pic@40000 {
>      compatible = "open-pic";
>      no-reset;
>   };
>
> Or are you worried that someone will find the wrong "no-reset" property when
> searching from a location higher in the tree than the PIC node?
>
> I don't have a serious objection to the idea, but it seems slightly odd to
> partially flatten the hierarchy back into the property names.  On the other
> hand, I do see the practical consideration of having a more unique property
> which might prevent programming confusion/errors.

It's the sort of thing where properties with really generic names,
like no-reset, I could potentially see as gaining a meaning across the
whole tree.  For instance, in the not so distant past the 'status'
property was defined for all nodes to indicate whether or not the
device is usable.  If any binding defined status for its own purposes,
then it would now be broken.  It is worth a little bit of
consideration to avoid collisions with names that might gain a meaning
in the global domain.  I don't care much about what the specific name
is, and openpic-no-reset may indeed be a little long, so feel free to
suggest something that you like better.

g.

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

> > +  - no-reset
> > +      Usage: optional
> > +      Value type: <empty>
> > +      Definition: The presence of this property indicates that the
> > + PIC
> > +          should not be reset during runtime initialization.  The
> > + presence of
> > +          this property also mandates that any initialization related
> > + to
> > +          interrupt sources shall be limited to sources explicitly
> > + referenced
> > +          in the device tree.
> 
> Please follow the lead set by the other binding documentation which is more
> concise and tends to be of the form:
> 
>     Required properties:
>         - reg : <description>
>         - interrupt-controller : <description>
> 
>     Optional Properties:
>         - no-reset : blah
> 
> I'm considering formalizing the binding format so that fully specified and
> cross-referenced documentation can be generated from the bindings
> directory.

Regarding the format-- The definition should also to specify the value
type.  I don't see this being consistently done in existing bindings.  
They are not completely unclear, but using consistent terms might help.

The ePAPR uses this convention:

    <empty>      # no value, a Boolean
     <u32>       # A 32-bit integer in big-endian format
     <u64>       # A 64-bit integer in big-endian format
   <string>      # null terminated
 <prop-encoded-array>    # format specific to the property
   <phandle>    # A <u32> value, referecnes another node
  <stringlist>    # A list of <string> values concatenated together.

The identifier prop-encoded-array came from precedence in other
of binding and ieee1275.   prop-encoded-arrays should be
be specifically defined in terms of # of cells and the 
meaning of each cell.

If you use the above types identifiers, there is no ambiguity.

Also, there are properties that don't necessarily fall in 'required'
and 'optional', but may be required depending on the context.  Thus
the 'Usage' identifier which Meador derived from my mpic binding
posted.   Usage could be:
               Required
               Optional
               See Definition

Stuart