[RFC] MPIC Bindings and Bindings for AMP Systems

[RFC] MPIC Bindings and Bindings for AMP Systems

[Meador Inge]

Hi All,

I am currently doing some work on Linux PPC AMP systems (with Hollis,
CC'd).  We are using device trees to partition resources between the
different OSes.  To help with this effort, we would like to introduce
some new bindings to the MPIC.

Currently, there are no bindings for the MPIC under
'.../Documentation/powerpc/dts-bindings/'.  I think most folks are
following the IEEE 1275 bindings for Open PIC.

It would be nice to check-in a binding for the MPIC that is a superset
of the 1275 bindings as a place to document extensions.  The already in
use property 'protected-sources', for example, could be placed there.

I have included a draft proposal of what this would like.  It includes
some properties ('no-reset') and a node (for message registers) that we
are currently using for our AMP implementation and we would really
like to push upstream.  If the general idea seems sound, then I will
clean up the proposal and submit a patch for the bindings.

Thoughts?


============================

* 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 a superset of the binding defined for Open 
PIC in [1].

** Required properties:

     - All of the required properties mentioned in [1].

** Optional properties:

     - no-reset : the presence of this property indicates that the
                  interrupt controller should not be reset during OS
                  initialization.
     - protected-sources : a list of interrupt sources that are not
                           available for use.

** Example:

	mpic: pic@40000 {
		interrupt-controller;
		#address-cells = <0>;
		#interrupt-cells = <2>;
		reg = <0x40000 0x40000>;
		compatible = "chrp,open-pic";
		device_type = "open-pic";
		protected-sources = <0xb1 0xb2>;
		no-reset;
	};

* MPIC Message Registers

This binding specifies how the MPIC message registers implemented on
some Freescale platforms for interprocessor communication should be
represented.

** Required parent:

     - Message register nodes should always been nested under a MPIC
       node.

** Required properties:

     - compatible : compatible list, contains 2 entries, first is
                    "fsl,CHIP-msgr", where CHIP is the processor
                    (p2020, mpc8572, etc ...) and the second is
                    "fsl,mpic-msgr".
     - reg        : should contain the address and the length of the
                    message register block.
     - cell-index : the index of the message register block.
     - interrupts : each one of the interrupts here represents the
                    interrupt line for one message register.  These
                    interrupts are routed internally to the MPIC.

NOTE: The 'interrupt-parent' is implicit since message register nodes
        are always children of interrupt controller nodes.

** Example:

	mpic: pic@40000 {
		interrupt-controller;
		#address-cells = <0>;
		#interrupt-cells = <2>;
		reg = <0x40000 0x40000>;
		compatible = "chrp,open-pic";
		device_type = "open-pic";
		protected-sources = <0xb1>;

		msgr@1400 {
			compatible = "fsl,p2020-msgr", "fsl,mpic-msgr";
			reg = <0x1400 0x200>;
			cell-index = <0>;
			interrupts = <0xb0 0x2 0xb1 0x2
                                       0xb2 0x2 0xb3 0x2>;
		};

		msgr@2400 {
			compatible = "fsl,p2020-msgr", "fsl,mpic-msgr";
			reg = <0x2400 0x200>;
			cell-index = <1>;
			interrupts = <0xb4 0x2 0xb5 0x2
                                       0xb6 0x2 0xb7 0x2>;
	         };	
	};

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


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

[RFC] MPIC Bindings and Bindings for AMP Systems

[Meador Inge]

Hi All,

I am currently doing some work on Linux PPC AMP systems (with Hollis,
CC'd).  We are using device trees to partition resources between the
different OSes.  To help with this effort, we would like to introduce
some new bindings to the MPIC.

Currently, there are no bindings for the MPIC under
'.../Documentation/powerpc/dts-bindings/'.  I think most folks are
following the IEEE 1275 bindings for Open PIC.

It would be nice to check-in a binding for the MPIC that is a superset
of the 1275 bindings as a place to document extensions.  The already in
use property 'protected-sources', for example, could be placed there.

I have included a draft proposal of what this would like.  It includes
some properties ('no-reset') and a node (for message registers) that we
are currently using for our AMP implementation and we would really
like to push upstream.  If the general idea seems sound, then I will
clean up the proposal and submit a patch for the bindings.

Thoughts?

===============

* 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 a superset of the binding defined for Open 
PIC in [1].

** Required properties:

     - All of the required properties mentioned in [1].

** Optional properties:

     - no-reset : the presence of this property indicates that the
                  interrupt controller should not be reset during OS
                  initialization.
     - protected-sources : a list of interrupt sources that are not
                           available for use.

** Example:

	mpic: pic@40000 {
		interrupt-controller;
		#address-cells = <0>;
		#interrupt-cells = <2>;
		reg = <0x40000 0x40000>;
		compatible = "chrp,open-pic";
		device_type = "open-pic";
		protected-sources = <0xb1 0xb2>;
		no-reset;
	};

* MPIC Message Registers

This binding specifies how the MPIC message registers implemented on
some Freescale platforms for interprocessor communication should be
represented.

** Required parent:

     - Message register nodes should always been nested under a MPIC
       node.

** Required properties:

     - compatible : compatible list, contains 2 entries, first is
                    "fsl,CHIP-msgr", where CHIP is the processor
                    (p2020, mpc8572, etc ...) and the second is
                    "fsl,mpic-msgr".
     - reg        : should contain the address and the length of the
                    message register block.
     - cell-index : the index of the message register block.
     - interrupts : each one of the interrupts here represents the
                    interrupt line for one message register.  These
                    interrupts are routed internally to the MPIC.

NOTE: The 'interrupt-parent' is implicit since message register nodes
        are always children of interrupt controller nodes.

** Example:

	mpic: pic@40000 {
		interrupt-controller;
		#address-cells = <0>;
		#interrupt-cells = <2>;
		reg = <0x40000 0x40000>;
		compatible = "chrp,open-pic";
		device_type = "open-pic";
		protected-sources = <0xb1>;

		msgr@1400 {
			compatible = "fsl,p2020-msgr", "fsl,mpic-msgr";
			reg = <0x1400 0x200>;
			cell-index = <0>;
			interrupts = <0xb0 0x2 0xb1 0x2
                                       0xb2 0x2 0xb3 0x2>;
		};

		msgr@2400 {
			compatible = "fsl,p2020-msgr", "fsl,mpic-msgr";
			reg = <0x2400 0x200>;
			cell-index = <1>;
			interrupts = <0xb4 0x2 0xb5 0x2
                                       0xb6 0x2 0xb7 0x2>;
	         };	
	};

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


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

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Grant Likely]

On Thu, Dec 23, 2010 at 12:51:29AM -0600, Meador Inge wrote:
> Hi All,
> 
> I am currently doing some work on Linux PPC AMP systems (with Hollis,
> CC'd).  We are using device trees to partition resources between the
> different OSes.  To help with this effort, we would like to introduce
> some new bindings to the MPIC.
> 
> Currently, there are no bindings for the MPIC under
> '.../Documentation/powerpc/dts-bindings/'.  I think most folks are
> following the IEEE 1275 bindings for Open PIC.
> 
> It would be nice to check-in a binding for the MPIC that is a superset
> of the 1275 bindings as a place to document extensions.  The already in
> use property 'protected-sources', for example, could be placed there.
> 
> I have included a draft proposal of what this would like.  It includes
> some properties ('no-reset') and a node (for message registers) that we
> are currently using for our AMP implementation and we would really
> like to push upstream.  If the general idea seems sound, then I will
> clean up the proposal and submit a patch for the bindings.
> 
> Thoughts?

Hi Meador.  Comments below.

g.

> 
> ===============
> 
> * 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 a superset of the binding defined for
> Open PIC in [1].
> 
> ** Required properties:
> 
>     - All of the required properties mentioned in [1].

You should probably list them here anyway to aid the reader.

> 
> ** Optional properties:
> 
>     - no-reset : the presence of this property indicates that the
>                  interrupt controller should not be reset during OS
>                  initialization.
>     - protected-sources : a list of interrupt sources that are not
>                           available for use.

What is the use case for the protected-sources property?  Wouldn't the
irqs simply not be referenced by any device nodes?  Documenting the
reason for this property would be useful here.

> 
> ** Example:
> 
> 	mpic: pic@40000 {
> 		interrupt-controller;
> 		#address-cells = <0>;
> 		#interrupt-cells = <2>;
> 		reg = <0x40000 0x40000>;
> 		compatible = "chrp,open-pic";
> 		device_type = "open-pic";
> 		protected-sources = <0xb1 0xb2>;
> 		no-reset;
> 	};
> 
> * MPIC Message Registers
> 
> This binding specifies how the MPIC message registers implemented on
> some Freescale platforms for interprocessor communication should be
> represented.
> 
> ** Required parent:
> 
>     - Message register nodes should always been nested under a MPIC
>       node.
> 
> ** Required properties:
> 
>     - compatible : compatible list, contains 2 entries, first is
>                    "fsl,CHIP-msgr", where CHIP is the processor
>                    (p2020, mpc8572, etc ...) and the second is
>                    "fsl,mpic-msgr".

I'd rather see the 'generic' value of mpic-msgr omitted and instead
allow new parts to claim compatibility with an older chip.  Generic
or wildcard compatible values can be troublesome because the meaning
has a tendency to change over time.

>     - reg        : should contain the address and the length of the
>                    message register block.
>     - cell-index : the index of the message register block.

?  This needs some more explanation.  cell-index often gets abused as
a way to enumerate devices.  Typically, the address of the device
itself is sufficient to identify the device.

>     - interrupts : each one of the interrupts here represents the
>                    interrupt line for one message register.  These
>                    interrupts are routed internally to the MPIC.
> 
> NOTE: The 'interrupt-parent' is implicit since message register nodes
>        are always children of interrupt controller nodes.
> 
> ** Example:
> 
> 	mpic: pic@40000 {
> 		interrupt-controller;
> 		#address-cells = <0>;
> 		#interrupt-cells = <2>;
> 		reg = <0x40000 0x40000>;
> 		compatible = "chrp,open-pic";
> 		device_type = "open-pic";
> 		protected-sources = <0xb1>;
> 
> 		msgr@1400 {
> 			compatible = "fsl,p2020-msgr", "fsl,mpic-msgr";
> 			reg = <0x1400 0x200>;

Are these registers memory mapped?  If so, then the parent node
needs to have a 'ranges' property.

> 			cell-index = <0>;
> 			interrupts = <0xb0 0x2 0xb1 0x2
>                                       0xb2 0x2 0xb3 0x2>;
> 		};
> 
> 		msgr@2400 {
> 			compatible = "fsl,p2020-msgr", "fsl,mpic-msgr";
> 			reg = <0x2400 0x200>;
> 			cell-index = <1>;
> 			interrupts = <0xb4 0x2 0xb5 0x2
>                                       0xb6 0x2 0xb7 0x2>;
> 	         };	
> 	};
> 
> * 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)
> 
> 
> -- 
> Meador Inge     | meador_inge AT mentor.com
> Mentor Embedded | http://www.mentor.com/embedded-software
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Meador Inge]

On 12/23/2010 12:56 PM, Grant Likely wrote:
> Hi Meador.  Comments below.
>
> g.

Thanks a lot for the feedback Grant.

> You should probably list them here anyway to aid the reader.

Will do.

> What is the use case for the protected-sources property?  Wouldn't the
> irqs simply not be referenced by any device nodes?  Documenting the
> reason for this property would be useful here.

One use case is the original [1]:

	Some HW platforms, such as the new cell blades, requires some
	MPIC sources to be left alone by the operating system. This
	implements support for a "protected-sources" property in the
	mpic controller node containing a list of source numbers to be
	protected against operating system interference.

Our use case is to reserve certain IRQs for certain OSes in an AMP 
system.  As an example, consider that OS 1 has (simplified for discussion):

     mpic: pic@40000 {
         ...
         protected-sources = <0xb1>;

         msgr@41400 {
             interrupts = <0xb0 2 0xb1 2 0xb2 2 0xb3 2>;
         };
     };

and OS 2 has:

     mpic: pic@40000 {
         ...
         protected-sources = <0xb0>;

         msgr@41400 {
             interrupts = <0xb0 2 0xb1 2 0xb2 2 0xb3 2>;
         };
     };

where OS 1 is sent messages through the message register tied to 0xb0 
and OS 2 is sent messages through the message register tied to 0xb1.

We can't just remove the IRQ of the _other_ OS from the 'interrupts' 
property in the message node because we need to know the IRQ in order to 
talk to the other OS.  So, we use protected sources to tell the OS that 
an IRQ is not available for its own use, while at the same time keeping 
complete information on all the IRQ mappings for the message registers.

I will update the documentation.

> I'd rather see the 'generic' value of mpic-msgr omitted and instead
> allow new parts to claim compatibility with an older chip.  Generic
> or wildcard compatible values can be troublesome because the meaning
> has a tendency to change over time.

I don't completely see the issue here.  Do you have a specific example 
of how generic values like this cause problems?  How do you see this 
working in terms of processing the data?  It seems like we are going to 
have to be aware of N values instead of 1, which seems worse.

> ?  This needs some more explanation.  cell-index often gets abused as
> a way to enumerate devices.  Typically, the address of the device
> itself is sufficient to identify the device.

The message registers typically come in blocks of four memory mapped 
registers and may not be in contiguous memory (example [3]).  The intent 
of 'cell-index' is to put an ordering on the blocks (so, yes, 
enumeration).  We could order them by address as well I suppose.  One 
less property to worry about :)

> Are these registers memory mapped?  If so, then the parent node
> needs to have a 'ranges' property.

Yes, they are.  I will look into adding that.


[1] commit 7fd7218610600b16f6f0af3f9d9353ba0265c09f, 
http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.36.y.git;a=commit;h=7fd7218610600b16f6f0af3f9d9353ba0265c09f

[2] Documentation/powerpc/dts-bindings/fsl/msi-pic.txt

[3] 
http://cache.freescale.com/files/32bit/doc/ref_manual/MPC8572ERM.pdf?fr=g

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

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Grant Likely]

On Thu, Dec 23, 2010 at 03:49:54PM -0600, Meador Inge wrote:
> On 12/23/2010 12:56 PM, Grant Likely wrote:
> >Hi Meador.  Comments below.
> >
> >g.
> 
> Thanks a lot for the feedback Grant.
> 
> >You should probably list them here anyway to aid the reader.
> 
> Will do.
> 
> >What is the use case for the protected-sources property?  Wouldn't the
> >irqs simply not be referenced by any device nodes?  Documenting the
> >reason for this property would be useful here.
> 
> One use case is the original [1]:
> 
> 	Some HW platforms, such as the new cell blades, requires some
> 	MPIC sources to be left alone by the operating system. This
> 	implements support for a "protected-sources" property in the
> 	mpic controller node containing a list of source numbers to be
> 	protected against operating system interference.
> 
> Our use case is to reserve certain IRQs for certain OSes in an AMP
> system.  As an example, consider that OS 1 has (simplified for
> discussion):
> 
>     mpic: pic@40000 {
>         ...
>         protected-sources = <0xb1>;
> 
>         msgr@41400 {
>             interrupts = <0xb0 2 0xb1 2 0xb2 2 0xb3 2>;
>         };
>     };
> 
> and OS 2 has:
> 
>     mpic: pic@40000 {
>         ...
>         protected-sources = <0xb0>;
> 
>         msgr@41400 {
>             interrupts = <0xb0 2 0xb1 2 0xb2 2 0xb3 2>;
>         };
>     };
> 
> where OS 1 is sent messages through the message register tied to
> 0xb0 and OS 2 is sent messages through the message register tied to
> 0xb1.
> 
> We can't just remove the IRQ of the _other_ OS from the 'interrupts'
> property in the message node because we need to know the IRQ in
> order to talk to the other OS.  So, we use protected sources to tell
> the OS that an IRQ is not available for its own use, while at the
> same time keeping complete information on all the IRQ mappings for
> the message registers.

If the OS cannot use the IRQ, then it better not be encoded into an
'interrupts' property.  Instead, they should be encoded into a
different property (which could use exactly the same encoding as
'interrupts') that tells the driver that it is the target processor's
irq.  Something like 'target-interrupts-cpu0 = <0xb1 2>' for OS#1 in
the example.

> 
> I will update the documentation.
> 
> >I'd rather see the 'generic' value of mpic-msgr omitted and instead
> >allow new parts to claim compatibility with an older chip.  Generic
> >or wildcard compatible values can be troublesome because the meaning
> >has a tendency to change over time.
> 
> I don't completely see the issue here.  Do you have a specific
> example of how generic values like this cause problems?

For example, both the mpc83xx family and the mpc5xxx family of
processors implement the same i2c core (historically using compatible
"fsl,i2c"), but the behaviour of the two parts is subtly and
incompatibly different.  The actual implementation name has to be
matched to figure out how to drive the core correctly.  So, in that
case which implementation does "fsl,i2c" really describe?

> How do you
> see this working in terms of processing the data?  It seems like we
> are going to have to be aware of N values instead of 1, which seems
> worse.

This argument has been rehashed many times, but it basically comes
down to compatible values should ideally be anchored to a real
implemented device, not to a family of devices, or to an unversioned
specification.

In practise, the implementation doesn't actually look any different
except that the 'reference' version specifies a specific
implementation instead of a generic name.  To use a concrete example,
if there are two parts using this MPIC, like the freescale p2040 and
p4080, and say for argument that the p2040 was implemented first, then
the compatible values would look like:

for the p2040:   compatible = "fsl,p2040-msgr";
for the p4080:   compatible = "fsl,p4080-msgr", "fsl,p2040-msgr";

and the driver could simply bind on "fsl,p2040-msgr" to work with both
chips.  So, instead of an arbitrary "fsl,mpic-msgr" string,
"fsl,p2040-msgr" is used as the baseline value and there is no
ambiguity about which part it describes.

If the p4080 is actually subtly different from the p2040, then
it would not claim compatibility with the former and the driver can
match against either string; adapting its behaviour as necessary.

> 
> >?  This needs some more explanation.  cell-index often gets abused as
> >a way to enumerate devices.  Typically, the address of the device
> >itself is sufficient to identify the device.
> 
> The message registers typically come in blocks of four memory mapped
> registers and may not be in contiguous memory (example [3]).  The
> intent of 'cell-index' is to put an ordering on the blocks (so, yes,
> enumeration).  We could order them by address as well I suppose.
> One less property to worry about :)

:-)  I'd prefer to see a binding that doesn't need to enumerate nodes.
If the nodes *really* need to be enumerated, then I recommend looking
at using properties in the aliases node to assign globally enumerated
names, or at explicitly having an order list in the parent node.

> 
> >Are these registers memory mapped?  If so, then the parent node
> >needs to have a 'ranges' property.
> 
> Yes, they are.  I will look into adding that.

Thanks.
g.

> 
> 
> [1] commit 7fd7218610600b16f6f0af3f9d9353ba0265c09f, http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.36.y.git;a=commit;h=7fd7218610600b16f6f0af3f9d9353ba0265c09f
> 
> [2] Documentation/powerpc/dts-bindings/fsl/msi-pic.txt
> 
> [3] http://cache.freescale.com/files/32bit/doc/ref_manual/MPC8572ERM.pdf?fr=g
> 
> --
> Meador Inge     | meador_inge AT mentor.com
> Mentor Embedded | http://www.mentor.com/embedded-software

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Scott Wood]

On Thu, 23 Dec 2010 15:33:25 -0700
Grant Likely <grant.likely@secretlab.ca> wrote:

> On Thu, Dec 23, 2010 at 03:49:54PM -0600, Meador Inge wrote:
> > How do you
> > see this working in terms of processing the data?  It seems like we
> > are going to have to be aware of N values instead of 1, which seems
> > worse.
> 
> This argument has been rehashed many times, but it basically comes
> down to compatible values should ideally be anchored to a real
> implemented device, not to a family of devices, or to an unversioned
> specification.

Freescale MPICs do have version numbers (version registers, even).  We
should put that version (possibly along with a compatible version) in
the compatible, though, for blocks such as this which don't include the
main MPIC registers and thus the version registers.

> In practise, the implementation doesn't actually look any different
> except that the 'reference' version specifies a specific
> implementation instead of a generic name.  To use a concrete example,
> if there are two parts using this MPIC, like the freescale p2040 and
> p4080, and say for argument that the p2040 was implemented first, then
> the compatible values would look like:
> 
> for the p2040:   compatible = "fsl,p2040-msgr";
> for the p4080:   compatible = "fsl,p4080-msgr", "fsl,p2040-msgr";

While I don't think it affected the message unit, p4080 rev 1 has a
different version of the MPIC from p4080 rev 2 (4.0 versus 4.1, IIRC).

I don't think "mpic-" should be dropped, whether a specific chip is
added or not.  "msgr" just seems too generic, and "mpic-" tells the
reader where in the chip manual they can find information about it.

> > >?  This needs some more explanation.  cell-index often gets abused as
> > >a way to enumerate devices.  Typically, the address of the device
> > >itself is sufficient to identify the device.
> > 
> > The message registers typically come in blocks of four memory mapped
> > registers and may not be in contiguous memory (example [3]).  The
> > intent of 'cell-index' is to put an ordering on the blocks (so, yes,
> > enumeration).  We could order them by address as well I suppose.
> > One less property to worry about :)

But why do we care about ordering them?  What's important is just that
you use the same one on both the sending partition and the receiving
partition.

-Scott

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Scott Wood]

On Wed, 22 Dec 2010 23:58:09 -0600
Meador Inge <meador_inge@mentor.com> wrote:

> NOTE: The 'interrupt-parent' is implicit since message register nodes
>         are always children of interrupt controller nodes.
> 
> ** Example:
> 
> 	mpic: pic@40000 {
> 		interrupt-controller;
> 		#address-cells = <0>;
> 		#interrupt-cells = <2>;
> 		reg = <0x40000 0x40000>;
> 		compatible = "chrp,open-pic";
> 		device_type = "open-pic";
> 		protected-sources = <0xb1>;
> 
> 		msgr@1400 {
> 			compatible = "fsl,p2020-msgr", "fsl,mpic-msgr";
> 			reg = <0x1400 0x200>;
> 			cell-index = <0>;
> 			interrupts = <0xb0 0x2 0xb1 0x2
>                                        0xb2 0x2 0xb3 0x2>;
> 		};
> 
> 		msgr@2400 {
> 			compatible = "fsl,p2020-msgr", "fsl,mpic-msgr";
> 			reg = <0x2400 0x200>;
> 			cell-index = <1>;
> 			interrupts = <0xb4 0x2 0xb5 0x2
>                                        0xb6 0x2 0xb7 0x2>;
> 	         };	
> 	};

These nodes cannot go under the mpic node, because interrupt
controllers need #address-cells = <0>.

It would be nice if the binding provided some way of partitioning
up individual message interrupts within a block.

Interrupt generation could be exported as a "service", similar to
(inbound) interrupts and gpios.

Perhaps a something like this, with "doorbell" being a new standard
hw-independent service with its own binding:

msg1: mpic-msg@1400 {
	compatible = "fsl,mpic-v3.0-msg";
	reg = <0x1400 0x200>;
	interrupts <176 2 178 2>;

	// We have message registers 0 and 2 for sending,
	// and 1 and 3 for receiving.
	// If absent, we own all message registers in this block.
	fsl,mpic-msg-send-mask = <0x5>;
	fsl,mpic-msg-receive-mask = <0xa>;

	doorbell-controller;

	// split into #doorbell-send-cells and #doorbell-receive-cells?
	#doorbell-cells = <1>;
};

some-amp-protocol-thingy {
	send-doorbells = <&msg1 0>; // generate messages on MSGR0
	receive-doorbells = <&msg1 0>; // receive messages on MSGR1
};

some-other-amp-protocol-thingy {
	send-doorbells = <&msg1 1>; // generate messages on MSGR2
	receive-doorbells = <&msg1 1>; // receive messages on MSGR3
};

Doorbell capabilities such as passing a 32-bit message can be negotiated
between the drivers for the doorbell controller and the doorbell client.

-Scott

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Blanchard, Hollis]

On 12/23/2010 01:49 PM, Meador Inge wrote:
>
> We can't just remove the IRQ of the _other_ OS from the 'interrupts'=20
> property in the message node because we need to know the IRQ in order=20
> to talk to the other OS.  So, we use protected sources to tell the OS=20
> that an IRQ is not available for its own use, while at the same time=20
> keeping=20
> complete information on all the IRQ mappings for the message =
registers.
There's a simpler and more fundamental problem: the Linux MPIC driver=20
initializes the "VECPRI" register for all interrupt sources (which is=20
how you associate a source with a vector). We need the Linux driver not=20
to do this.

(The "no-reset" property solves a related problem: it tells the driver=20
not to use the hardware's special "reset all registers" bit.)

Hollis Blanchard
Mentor Graphics, Embedded Systems Division

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Meador Inge]

Thanks for the feedback Scott.

On 01/03/2011 02:22 PM, Scott Wood wrote:
> On Wed, 22 Dec 2010 23:58:09 -0600
> These nodes cannot go under the mpic node, because interrupt
> controllers need #address-cells =<0>.

Good point.  Do they actually need it or is that just the way it 
currently is?  [1] mandates it, I didn't see anything in [2] and I can't 
access [3].

However, AFAIK '#address-cells' is taken directly from the parent node 
and is not inherited from ancestors higher in the tree.  So another 
option would be to do something like:

     mpic: pic@40000 {
        ...
        message-registers@0 {
           #address-cells = <1>;
           #size-cells = <1>;

           msgr@1400 {
              compatible = "fsl,mpic-v3.0-msgr";
              reg = <0x1400 0x200>;
              interrupts = <0xb0 0x2 0xb1 0x2 0xb2 0x2 0xb3 0x2>;
           };

           msgr@2400 {
              compatible = "fsl,mpic-v3.0-msgr";
              reg = <0x2400 0x200>;
              interrupts = <0xb4 0x2 0xb5 0x2 0xb6 0x2 0xb7 0x2>;
           };
        };
     };

I like the nesting as it models the physical relationship closer and 
creates a clean namespace.

> It would be nice if the binding provided some way of partitioning
> up individual message interrupts within a block.
>
> Interrupt generation could be exported as a "service", similar to
> (inbound) interrupts and gpios.
>
> Perhaps a something like this, with "doorbell" being a new standard
> hw-independent service with its own binding:

I need to think about this proposal more, but our original intent was to 
just have a simple description of the message registers in the device 
tree and the policy for how those message registers are used is in 
software (not necessarily an exact API use case, but you get the point):

    /* Core 0 */
    mpic_msgr_reserve(0);
    mpic_msgr_reserve(1);

    /* Send message to Core 1 */
    mpic_msgr_write(3, 13);

    /* Read a value */
    u32 value;
    mpic_msgr_read(0, &value);

    /* Free the register */
    mpic_msgr_release(0);
    ...

    /* Core 1 */
    mpic_msgr_reserve(3);
    mpic_msgr_reserve(4);

    /* Send message to Core 0 */
    mpic_msgr_write(0, 1);

Note that a "reservation" is still isolated to a particular core, e.g. 
'mpic_msgr_reserve(0)' on core 0 will not cause 'mpic_msgr_reserve(0)' 
to fail on another core.  Where as two invocations of 
'mpic_msgr_reserve(0)' on the same core without an interleaved 
'mpic_msgr_release(0)' would, of course, fail.

> msg1: mpic-msg@1400 {
> 	compatible = "fsl,mpic-v3.0-msg";
> 	reg =<0x1400 0x200>;
> 	interrupts<176 2 178 2>;
>
> 	// We have message registers 0 and 2 for sending,
> 	// and 1 and 3 for receiving.
> 	// If absent, we own all message registers in this block.
> 	fsl,mpic-msg-send-mask =<0x5>;
> 	fsl,mpic-msg-receive-mask =<0xa>;
>
> 	doorbell-controller;
>
> 	// split into #doorbell-send-cells and #doorbell-receive-cells?
> 	#doorbell-cells =<1>;
> };
>
> some-amp-protocol-thingy {
> 	send-doorbells =<&msg1 0>; // generate messages on MSGR0
> 	receive-doorbells =<&msg1 0>; // receive messages on MSGR1
> };
>
> some-other-amp-protocol-thingy {
> 	send-doorbells =<&msg1 1>; // generate messages on MSGR2
> 	receive-doorbells =<&msg1 1>; // receive messages on MSGR3
> };
>
> Doorbell capabilities such as passing a 32-bit message can be negotiated
> between the drivers for the doorbell controller and the doorbell client.


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

[1] Power.org™ Standard for Embedded Power Architecture™ Platform 
Requirements (ePAPR) Version 1.0
[2] PowerPC Microprocessor Common Hardware Reference Platform (CHRP) 
Binding, Version 1.8, 1998. Published by the Open Firmware Working Group.
[3] The Open Programmable Interrupt Controller (PIC) Register Interface 
Specification Revision 1.2

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Scott Wood]

On Tue, 4 Jan 2011 17:52:38 -0600
Meador Inge <meador_inge@mentor.com> wrote:

> Thanks for the feedback Scott.
> 
> On 01/03/2011 02:22 PM, Scott Wood wrote:
> > On Wed, 22 Dec 2010 23:58:09 -0600
> > These nodes cannot go under the mpic node, because interrupt
> > controllers need #address-cells =<0>.
> 
> Good point.  Do they actually need it or is that just the way it 
> currently is?  [1] mandates it, I didn't see anything in [2] and I can't 
> access [3].

It's because of the way interrupt maps work -- a full interrupt
specifier includes the node's reg address as well as the values in the
interrupts property.  This is useful for things like PCI controllers,
but normal interrupt controllers won't have any use for it.

In theory, I suppose you could have a non-zero #address-cells in an
interrupt controller, but then you'd have to pad the parent interrupt
specifiers in any interrupt map entries that point at the MPIC node with
that many don't-care cells.

Better to just avoid the issue.

> However, AFAIK '#address-cells' is taken directly from the parent node 
> and is not inherited from ancestors higher in the tree.  So another 
> option would be to do something like:
> 
>      mpic: pic@40000 {
>         ...
>         message-registers@0 {
>            #address-cells = <1>;
>            #size-cells = <1>;
> 
>            msgr@1400 {
>               compatible = "fsl,mpic-v3.0-msgr";
>               reg = <0x1400 0x200>;
>               interrupts = <0xb0 0x2 0xb1 0x2 0xb2 0x2 0xb3 0x2>;
>            };
> 
>            msgr@2400 {
>               compatible = "fsl,mpic-v3.0-msgr";
>               reg = <0x2400 0x200>;
>               interrupts = <0xb4 0x2 0xb5 0x2 0xb6 0x2 0xb7 0x2>;
>            };
>         };
>      };

Won't work, the reg addresses need to be translatable through ranges all
the way up to the root.

> I like the nesting as it models the physical relationship closer and 
> creates a clean namespace.

Sure, if it weren't for the #address-cells issue I'd agree.

> > It would be nice if the binding provided some way of partitioning
> > up individual message interrupts within a block.
> >
> > Interrupt generation could be exported as a "service", similar to
> > (inbound) interrupts and gpios.
> >
> > Perhaps a something like this, with "doorbell" being a new standard
> > hw-independent service with its own binding:
> 
> I need to think about this proposal more, but our original intent was to 
> just have a simple description of the message registers in the device 
> tree and the policy for how those message registers are used is in 
> software (not necessarily an exact API use case, but you get the point):

That may be fine for your use case (just as some people may be happy
to hardcode device knowledge into their kernel), but in the general case
inter-partition protocols are effectively a non-probeable part of the
"hardware" the partition runs on.  It's good to have a standard way of
labelling what goes where. The details of the protocol would still be
in software, referenced by the compatible string on the protocol node.

>     /* Core 0 */
>     mpic_msgr_reserve(0);
>     mpic_msgr_reserve(1);
> 
>     /* Send message to Core 1 */
>     mpic_msgr_write(3, 13);
> 
>     /* Read a value */
>     u32 value;
>     mpic_msgr_read(0, &value);
> 
>     /* Free the register */
>     mpic_msgr_release(0);
>     ...
> 
>     /* Core 1 */
>     mpic_msgr_reserve(3);
>     mpic_msgr_reserve(4);
> 
>     /* Send message to Core 0 */
>     mpic_msgr_write(0, 1);

You're hardcoding into software that core 0 gets msg0 and msg1, and
core 1 gets msg3 and msg4, etc.  Not much different than hardcoding that
core 0 gets enet0 and core 1 gets enet1 and enet2.

> Note that a "reservation" is still isolated to a particular core, e.g. 
> 'mpic_msgr_reserve(0)' on core 0 will not cause 'mpic_msgr_reserve(0)' 
> to fail on another core.  Where as two invocations of 
> 'mpic_msgr_reserve(0)' on the same core without an interleaved 
> 'mpic_msgr_release(0)' would, of course, fail.

So basically, you're assigning the resource to both partitions, and
relying on dynamic cooperation.  That's fine, in that case both
partitions would see the resource in their device tree, hopefully along
with something to indicate what the situation is.  But dedicated
ownership is common enough, and similar enough to the question of
whether the hardware exists at all, that it's nice to be able to express
it in the device tree.  It also makes it easier to deal with situations
where you later want to plug in different hardware (or possibly a
virtualized interface) underneath the protocol.

I'm not sure how much practical benefit there is to dynamically
allocating message registers across partitions, though -- you'd have
to have some way of communicating to the other end the result of the
allocation, so it knows which one to send a message on.  And if you
only have 2 cores, and unhypervised AMP, you'll be able to dedicate 4
message registers to each partition...

-Scott

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Meador Inge]

On 01/04/2011 06:13 PM, Scott Wood wrote:
> On Tue, 4 Jan 2011 17:52:38 -0600
> Meador Inge<meador_inge@mentor.com>  wrote:
>
>> Thanks for the feedback Scott.
>>
>> On 01/03/2011 02:22 PM, Scott Wood wrote:
>>> On Wed, 22 Dec 2010 23:58:09 -0600
>>> These nodes cannot go under the mpic node, because interrupt
>>> controllers need #address-cells =<0>.
>>
>> Good point.  Do they actually need it or is that just the way it
>> currently is?  [1] mandates it, I didn't see anything in [2] and I can't
>> access [3].
>
> It's because of the way interrupt maps work -- a full interrupt
> specifier includes the node's reg address as well as the values in the
> interrupts property.  This is useful for things like PCI controllers,
> but normal interrupt controllers won't have any use for it.
>
> In theory, I suppose you could have a non-zero #address-cells in an
> interrupt controller, but then you'd have to pad the parent interrupt
> specifiers in any interrupt map entries that point at the MPIC node with
> that many don't-care cells.
>
> Better to just avoid the issue.

Agreed, it would be nasty to have padding for this.

>> However, AFAIK '#address-cells' is taken directly from the parent node
>> and is not inherited from ancestors higher in the tree.  So another
>> option would be to do something like:
>>
>>       mpic: pic@40000 {
>>          ...
>>          message-registers@0 {
>>             #address-cells =<1>;
>>             #size-cells =<1>;
>>
>>             msgr@1400 {
>>                compatible = "fsl,mpic-v3.0-msgr";
>>                reg =<0x1400 0x200>;
>>                interrupts =<0xb0 0x2 0xb1 0x2 0xb2 0x2 0xb3 0x2>;
>>             };
>>
>>             msgr@2400 {
>>                compatible = "fsl,mpic-v3.0-msgr";
>>                reg =<0x2400 0x200>;
>>                interrupts =<0xb4 0x2 0xb5 0x2 0xb6 0x2 0xb7 0x2>;
>>             };
>>          };
>>       };
>
> Won't work, the reg addresses need to be translatable through ranges all
> the way up to the root.

Ah, I see.  This is because we would have to add 'ranges' on 
'message-registers' and 'pic', but 'pic' needs to have '#address-cells' 
 > 0 to interpret 'ranges'.  Fun :)  I will hoist the message registers 
nodes out of the MPIC node.

>> I like the nesting as it models the physical relationship closer and
>> creates a clean namespace.
>
> Sure, if it weren't for the #address-cells issue I'd agree.
>
>>> It would be nice if the binding provided some way of partitioning
>>> up individual message interrupts within a block.
>>>
>>> Interrupt generation could be exported as a "service", similar to
>>> (inbound) interrupts and gpios.
>>>
>>> Perhaps a something like this, with "doorbell" being a new standard
>>> hw-independent service with its own binding:
>>
>> I need to think about this proposal more, but our original intent was to
>> just have a simple description of the message registers in the device
>> tree and the policy for how those message registers are used is in
>> software (not necessarily an exact API use case, but you get the point):
>
> That may be fine for your use case (just as some people may be happy
> to hardcode device knowledge into their kernel), but in the general case
> inter-partition protocols are effectively a non-probeable part of the
> "hardware" the partition runs on.  It's good to have a standard way of
> labelling what goes where. The details of the protocol would still be
> in software, referenced by the compatible string on the protocol node.
>
>>      /* Core 0 */
>>      mpic_msgr_reserve(0);
>>      mpic_msgr_reserve(1);
>>
>>      /* Send message to Core 1 */
>>      mpic_msgr_write(3, 13);
>>
>>      /* Read a value */
>>      u32 value;
>>      mpic_msgr_read(0,&value);
>>
>>      /* Free the register */
>>      mpic_msgr_release(0);
>>      ...
>>
>>      /* Core 1 */
>>      mpic_msgr_reserve(3);
>>      mpic_msgr_reserve(4);
>>
>>      /* Send message to Core 0 */
>>      mpic_msgr_write(0, 1);
>
> You're hardcoding into software that core 0 gets msg0 and msg1, and
> core 1 gets msg3 and msg4, etc.  Not much different than hardcoding that
> core 0 gets enet0 and core 1 gets enet1 and enet2.
>
>> Note that a "reservation" is still isolated to a particular core, e.g.
>> 'mpic_msgr_reserve(0)' on core 0 will not cause 'mpic_msgr_reserve(0)'
>> to fail on another core.  Where as two invocations of
>> 'mpic_msgr_reserve(0)' on the same core without an interleaved
>> 'mpic_msgr_release(0)' would, of course, fail.
>
> So basically, you're assigning the resource to both partitions, and
> relying on dynamic cooperation.  That's fine, in that case both
> partitions would see the resource in their device tree, hopefully along
> with something to indicate what the situation is.  But dedicated
> ownership is common enough, and similar enough to the question of
> whether the hardware exists at all, that it's nice to be able to express
> it in the device tree.  It also makes it easier to deal with situations
> where you later want to plug in different hardware (or possibly a
> virtualized interface) underneath the protocol.

I agree that it would be nice to have some elements of the protocol 
represented in the device tree.  I would just like to do this in small 
steps.  We are going to need the data in the device tree for the message 
registers no matter what.  We are also going to need a simple API for 
interfacing with the registers.  The bottom layer so to speak.  These 
building blocks can be utilized later by a higher-level protocol if need 
be.  For now I would just like to propose the MPIC bindings, the initial 
message register bindings, and the message registers API.  Sound good?

> I'm not sure how much practical benefit there is to dynamically
> allocating message registers across partitions, though -- you'd have
> to have some way of communicating to the other end the result of the
> allocation, so it knows which one to send a message on.  And if you
> only have 2 cores, and unhypervised AMP, you'll be able to dedicate 4
> message registers to each partition...

Hollis and I discussed that a bit already.  The problem with our use 
case (and presumably others) is that we are using the message registers 
to aid in communicating over shared memory.  If we go into dynamic 
allocation of message registers across partitions, then we need to 
coordinate between the partitions across shared memory.  So you need 
shared memory communication to setup shared memory communication.  Also, 
if we just get the basic pieces in place with the message register API 
('reserve', 'release', 'read', 'write', etc...) we can perhaps figure 
out a way to build a dynamically allocator on top of those services 
later, if need be.

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

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Meador Inge]

On 01/03/2011 01:51 PM, Scott Wood wrote:
> On Thu, 23 Dec 2010 15:33:25 -0700
> Grant Likely<grant.likely@secretlab.ca>  wrote:
>
>> On Thu, Dec 23, 2010 at 03:49:54PM -0600, Meador Inge wrote:
>>> How do you
>>> see this working in terms of processing the data?  It seems like we
>>> are going to have to be aware of N values instead of 1, which seems
>>> worse.
>>
>> This argument has been rehashed many times, but it basically comes
>> down to compatible values should ideally be anchored to a real
>> implemented device, not to a family of devices, or to an unversioned
>> specification.
>
> Freescale MPICs do have version numbers (version registers, even).  We
> should put that version (possibly along with a compatible version) in
> the compatible, though, for blocks such as this which don't include the
> main MPIC registers and thus the version registers.

I like that better than claiming compatibility with other chips.  I will 
incorporate that idea.  Thanks.


>> In practise, the implementation doesn't actually look any different
>> except that the 'reference' version specifies a specific
>> implementation instead of a generic name.  To use a concrete example,
>> if there are two parts using this MPIC, like the freescale p2040 and
>> p4080, and say for argument that the p2040 was implemented first, then
>> the compatible values would look like:
>>
>> for the p2040:   compatible = "fsl,p2040-msgr";
>> for the p4080:   compatible = "fsl,p4080-msgr", "fsl,p2040-msgr";
>
> While I don't think it affected the message unit, p4080 rev 1 has a
> different version of the MPIC from p4080 rev 2 (4.0 versus 4.1, IIRC).
>
> I don't think "mpic-" should be dropped, whether a specific chip is
> added or not.  "msgr" just seems too generic, and "mpic-" tells the
> reader where in the chip manual they can find information about it.

Agreed.

>>>> ?  This needs some more explanation.  cell-index often gets abused as
>>>> a way to enumerate devices.  Typically, the address of the device
>>>> itself is sufficient to identify the device.
>>>
>>> The message registers typically come in blocks of four memory mapped
>>> registers and may not be in contiguous memory (example [3]).  The
>>> intent of 'cell-index' is to put an ordering on the blocks (so, yes,
>>> enumeration).  We could order them by address as well I suppose.
>>> One less property to worry about :)
>
> But why do we care about ordering them?  What's important is just that
> you use the same one on both the sending partition and the receiving
> partition.

We need some sort of mapping between a message register and a message 
register number so that the message registers can be referenced through 
some sort of API (e.g. 'mpic_msgr_read(0)').  One way to do that would 
be by putting an order on the registers.  Maybe there is a better way, 
though ...

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

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Scott Wood]

On Wed, 5 Jan 2011 15:58:55 -0600
Meador Inge <meador_inge@mentor.com> wrote:

> We need some sort of mapping between a message register and a message 
> register number so that the message registers can be referenced through 
> some sort of API (e.g. 'mpic_msgr_read(0)').  One way to do that would 
> be by putting an order on the registers.  Maybe there is a better way, 
> though ...

A message register is uniquely identified by a reference to the device
tree node, plus a 0-3 index into that node's message registers.

-Scott

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Meador Inge]

On 12/23/2010 04:33 PM, Grant Likely wrote:
> This argument has been rehashed many times, but it basically comes
> down to compatible values should ideally be anchored to a real
> implemented device, not to a family of devices, or to an unversioned
> specification.
>
> In practise, the implementation doesn't actually look any different
> except that the 'reference' version specifies a specific
> implementation instead of a generic name.  To use a concrete example,
> if there are two parts using this MPIC, like the freescale p2040 and
> p4080, and say for argument that the p2040 was implemented first, then
> the compatible values would look like:
>
> for the p2040:   compatible = "fsl,p2040-msgr";
> for the p4080:   compatible = "fsl,p4080-msgr", "fsl,p2040-msgr";
>
> and the driver could simply bind on "fsl,p2040-msgr" to work with both
> chips.  So, instead of an arbitrary "fsl,mpic-msgr" string,
> "fsl,p2040-msgr" is used as the baseline value and there is no
> ambiguity about which part it describes.
>
> If the p4080 is actually subtly different from the p2040, then
> it would not claim compatibility with the former and the driver can
> match against either string; adapting its behaviour as necessary.

Thanks for the explanation.  I see now that there is a warning note in 
'http://www.devicetree.org/Device_Tree_Usage' about this case.  Perhaps 
a "best practices" guide for writing bindings might be useful as well. 
I would be happy to contribute to that, but I am still learning the best 
practices :)

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

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Blanchard, Hollis]

On 01/05/2011 02:09 PM, Scott Wood wrote:
> On Wed, 5 Jan 2011 15:58:55 -0600
> Meador Inge<meador_inge@mentor.com>  wrote:
>
>> We need some sort of mapping between a message register and a message
>> register number so that the message registers can be referenced =
through
>> some sort of API (e.g. 'mpic_msgr_read(0)').  One way to do that =
would
>> be by putting an order on the registers.  Maybe there is a better =
way,
>> though ...
> A message register is uniquely identified by a reference to the device
> tree node, plus a 0-3 index into that node's message registers.
Really what we're talking about is software configuration, not hardware=20
description. We've gone back and forth on representing this information=20
in the device tree, and most recently decided against it. Outside the=20
kernel, a device node reference isn't really practical.

Hollis Blanchard
Mentor Graphics, Embedded Systems Division

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Scott Wood]

On Wed, 5 Jan 2011 14:49:40 -0800
"Blanchard, Hollis" <Hollis_Blanchard@mentor.com> wrote:

> On 01/05/2011 02:09 PM, Scott Wood wrote:
> > On Wed, 5 Jan 2011 15:58:55 -0600
> > Meador Inge<meador_inge@mentor.com>  wrote:
> >
> >> We need some sort of mapping between a message register and a message
> >> register number so that the message registers can be referenced through
> >> some sort of API (e.g. 'mpic_msgr_read(0)').  One way to do that would
> >> be by putting an order on the registers.  Maybe there is a better way,
> >> though ...
> > A message register is uniquely identified by a reference to the device
> > tree node, plus a 0-3 index into that node's message registers.
> Really what we're talking about is software configuration, not hardware 
> description.

Part of that software configuration involves identifying the hardware
being referenced.

> We've gone back and forth on representing this information 
> in the device tree, and most recently decided against it. Outside the 
> kernel, a device node reference isn't really practical.

Global enumeration isn't much fun either.  For something like this
where it's very unlikely that additional MPIC message units will be
added to the system dynamically, it's managable, but it's not a good
habit to get into.  Look at the pain that's been caused by such
assumptions in the i2c subsystem, in kernel interrupt management, etc.

A reference to a node is just a pointer to a software message driver
object, which can be obtained from looking up an alias.  It's a little
less simple than just using a number, but it's not impractical. It also
provides a natural place to put a layer of indirection in the code that
isolates the upper-layer protocol from the details of what sort of
message transport it is using.

Now, if you don't care about this, and want to just use numbers in your
application, go ahead.  But I don't think that such an assumption
should go into the device tree binding.  Have the software number the
message register banks in increasing physical address order, or based
on numbered aliases similar to how U-Boot enumerates ethernet nodes, or
something similar.

-Scott

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Meador Inge]

On 01/03/2011 02:22 PM, Scott Wood wrote:
> On Wed, 22 Dec 2010 23:58:09 -0600
> Perhaps a something like this, with "doorbell" being a new standard
> hw-independent service with its own binding:
>
> msg1: mpic-msg@1400 {
> 	compatible = "fsl,mpic-v3.0-msg";
> 	reg =<0x1400 0x200>;
> 	interrupts<176 2 178 2>;
>
> 	// We have message registers 0 and 2 for sending,
> 	// and 1 and 3 for receiving.
> 	// If absent, we own all message registers in this block.
> 	fsl,mpic-msg-send-mask =<0x5>;
> 	fsl,mpic-msg-receive-mask =<0xa>;
>
> 	doorbell-controller;
>
> 	// split into #doorbell-send-cells and #doorbell-receive-cells?
> 	#doorbell-cells =<1>;
> };
>
> some-amp-protocol-thingy {
> 	send-doorbells =<&msg1 0>; // generate messages on MSGR0
> 	receive-doorbells =<&msg1 0>; // receive messages on MSGR1
> };
>
> some-other-amp-protocol-thingy {
> 	send-doorbells =<&msg1 1>; // generate messages on MSGR2
> 	receive-doorbells =<&msg1 1>; // receive messages on MSGR3
> };
>
> Doorbell capabilities such as passing a 32-bit message can be negotiated
> between the drivers for the doorbell controller and the doorbell client.

After thinking about it a little more, I like the idea of having a 
'receive-mask' to further partition the message register blocks.  This 
would also allow us to remove IRQs from the 'interrupts' property that 
are not being used on a given node.  As for the 'send-mask', why would 
we want to block sending messages?  It seems to me that it would be 
reasonable to allow a node to send a message to any other node.

As an example, consider a four core system.  Then we might have 
something like (only relevant DTS bits shown):

Core 0:
    mpic-msgr-block@1400 {
       // Receives messages on registers 1 and 3.
       interrupts = <0xb1 2 0xb3 2>;
       receive-mask = <0xa>;
    };
Core 1:
    mpic-msgr-block@1400 {
       // Receives messages on register 2.
       interrupts = <0xb2 2>;
       receive-mask = <0x4>;
    };
Core 2:
    mpic-msgr-block@1400 {
       // Receives messages on register 0.
       interrupts = <0xb0 2>;
       receive-mask = <0x1>;
    };
Core 3:
    mpic-msgr-block@1400 {
       // Receives no messages.
       interrupts = <>;
    };

Then the API usage, for say core 0, might look something like:

    /* Core 0 */
    mpic_msgr *reg0 = mpic_get(0);
    mpic_msgr *reg1 = mpic_get(1);
    assert(mpic_msgr_get(100) == NULL);
    u32 value;

    /* Send a message on register 0. */
    assert(mpic_msgr_write(reg0, 12) == 0);
    /* Send a message on register 1. */
    assert(mpic_msgr_write(reg1, 12) == 0);

    /* Attempt to read a message on register 0, but can't
       since it is not owned. */
    assert(mpic_msgr_read(reg0, &value) == -ENODEV);

    /* Successfully read a message on register 1. */
    assert(mpic_msgr_read(reg1, &value) == 0);

The API usage for other cores would look similar.  As mentioned in 
another thread, this will provide us with the low-level building blocks 
and we can layer other protocols, such as the doorbell protocol, on top 
later (if needed).

Hollis, how do you feel about this?

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

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Scott Wood]

On Wed, 5 Jan 2011 20:58:36 -0600
Meador Inge <meador_inge@mentor.com> wrote:

> On 01/03/2011 02:22 PM, Scott Wood wrote:
> > On Wed, 22 Dec 2010 23:58:09 -0600
> > Perhaps a something like this, with "doorbell" being a new standard
> > hw-independent service with its own binding:
> >
> > msg1: mpic-msg@1400 {
> > 	compatible = "fsl,mpic-v3.0-msg";
> > 	reg =<0x1400 0x200>;
> > 	interrupts<176 2 178 2>;
> >
> > 	// We have message registers 0 and 2 for sending,
> > 	// and 1 and 3 for receiving.
> > 	// If absent, we own all message registers in this block.
> > 	fsl,mpic-msg-send-mask =<0x5>;
> > 	fsl,mpic-msg-receive-mask =<0xa>;

Alternatively, we could describe available ranges similarly to the
existing MSI binding.

> After thinking about it a little more, I like the idea of having a 
> 'receive-mask' to further partition the message register blocks.  This 
> would also allow us to remove IRQs from the 'interrupts' property that 
> are not being used on a given node.  As for the 'send-mask', why would 
> we want to block sending messages?  It seems to me that it would be 
> reasonable to allow a node to send a message to any other node.

The send-mask, like the receive-mask, is optional.  If for your
application it makes sense to give all partitions send access to all
doorbells, then you can do that.

You might want to provide a send-mask if you are not specifying to the
partition how to use the message registers, just splitting them up for
their own internal use (especially if you have a larger multicore system
where each partition has multiple CPUs).

Or, you may want to add a layer of robustness against one partition
interfering with a message register that is only supposed to be used by
another partition, even if there are instructions elsewhere about which
message register to use for what purpose.

Plus, it provides symmetry between send and receive -- otherwise, you'd
have a numberspace that is limited to your own resources on receive,
but a global numberspace on send.

> As an example, consider a four core system.  Then we might have 
> something like (only relevant DTS bits shown):
> 
> Core 0:
>     mpic-msgr-block@1400 {
>        // Receives messages on registers 1 and 3.
>        interrupts = <0xb1 2 0xb3 2>;
>        receive-mask = <0xa>;
>     };
> Core 1:
>     mpic-msgr-block@1400 {
>        // Receives messages on register 2.
>        interrupts = <0xb2 2>;
>        receive-mask = <0x4>;
>     };
> Core 2:
>     mpic-msgr-block@1400 {
>        // Receives messages on register 0.
>        interrupts = <0xb0 2>;
>        receive-mask = <0x1>;
>     };
> Core 3:
>     mpic-msgr-block@1400 {
>        // Receives no messages.
>        interrupts = <>;
>     };

For core 3 I'd just leave out the node entirely.  Note that the absence
of a receive-mask indicates that all the registers are available (i.e.
normal unpartitioned case), not none.

-Scott

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Blanchard, Hollis]

On 01/05/2011 03:07 PM, Scott Wood wrote:
> On Wed, 5 Jan 2011 14:49:40 -0800
> "Blanchard, Hollis"<Hollis_Blanchard@mentor.com>  wrote:
>
>> On 01/05/2011 02:09 PM, Scott Wood wrote:
>>> On Wed, 5 Jan 2011 15:58:55 -0600
>>> Meador Inge<meador_inge@mentor.com>   wrote:
>>>
>>>> We need some sort of mapping between a message register and a =
message
>>>> register number so that the message registers can be referenced =
through
>>>> some sort of API (e.g. 'mpic_msgr_read(0)').  One way to do that =
would
>>>> be by putting an order on the registers.  Maybe there is a better =
way,
>>>> though ...
>>> A message register is uniquely identified by a reference to the =
device
>>> tree node, plus a 0-3 index into that node's message registers.
>> Really what we're talking about is software configuration, not =
hardware
>> description.
> Part of that software configuration involves identifying the hardware
> being referenced.
>
>> We've gone back and forth on representing this information
>> in the device tree, and most recently decided against it. Outside the
>> kernel, a device node reference isn't really practical.
> Global enumeration isn't much fun either.  For something like this
> where it's very unlikely that additional MPIC message units will be
> added to the system dynamically, it's managable, but it's not a good
> habit to get into.  Look at the pain that's been caused by such
> assumptions in the i2c subsystem, in kernel interrupt management, etc.
>
> A reference to a node is just a pointer to a software message driver
> object, which can be obtained from looking up an alias.  It's a little
> less simple than just using a number, but it's not impractical. It =
also
> provides a natural place to put a layer of indirection in the code =
that
> isolates the upper-layer protocol from the details of what sort of
> message transport it is using.
>
> Now, if you don't care about this, and want to just use numbers in =
your
> application, go ahead.  But I don't think that such an assumption
> should go into the device tree binding.  Have the software number the
> message register banks in increasing physical address order, or based
> on numbered aliases similar to how U-Boot enumerates ethernet nodes, =
or
> something similar.
Using physical addresses doesn't solve the enumeration problem either,=20
but I think it's beside the point: userspace must refer to the device.=20
There is a rich history of userspace *not* walking /proc/device-tree in=20
order to refer to a physical device. Are you suggesting this case is=20
special?

Hollis Blanchard
Mentor Graphics, Embedded Systems Division

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Grant Likely]

On Thu, Jan 6, 2011 at 2:52 PM, Blanchard, Hollis
<Hollis_Blanchard@mentor.com> wrote:
> On 01/05/2011 03:07 PM, Scott Wood wrote:
>> On Wed, 5 Jan 2011 14:49:40 -0800
>> "Blanchard, Hollis"<Hollis_Blanchard@mentor.com> =A0wrote:
>>
>>> On 01/05/2011 02:09 PM, Scott Wood wrote:
>>>> On Wed, 5 Jan 2011 15:58:55 -0600
>>>> Meador Inge<meador_inge@mentor.com> =A0 wrote:
>>>>
>>>>> We need some sort of mapping between a message register and a message
>>>>> register number so that the message registers can be referenced throu=
gh
>>>>> some sort of API (e.g. 'mpic_msgr_read(0)'). =A0One way to do that wo=
uld
>>>>> be by putting an order on the registers. =A0Maybe there is a better w=
ay,
>>>>> though ...
>>>> A message register is uniquely identified by a reference to the device
>>>> tree node, plus a 0-3 index into that node's message registers.
>>> Really what we're talking about is software configuration, not hardware
>>> description.
>> Part of that software configuration involves identifying the hardware
>> being referenced.
>>
>>> We've gone back and forth on representing this information
>>> in the device tree, and most recently decided against it. Outside the
>>> kernel, a device node reference isn't really practical.
>> Global enumeration isn't much fun either. =A0For something like this
>> where it's very unlikely that additional MPIC message units will be
>> added to the system dynamically, it's managable, but it's not a good
>> habit to get into. =A0Look at the pain that's been caused by such
>> assumptions in the i2c subsystem, in kernel interrupt management, etc.
>>
>> A reference to a node is just a pointer to a software message driver
>> object, which can be obtained from looking up an alias. =A0It's a little
>> less simple than just using a number, but it's not impractical. It also
>> provides a natural place to put a layer of indirection in the code that
>> isolates the upper-layer protocol from the details of what sort of
>> message transport it is using.
>>
>> Now, if you don't care about this, and want to just use numbers in your
>> application, go ahead. =A0But I don't think that such an assumption
>> should go into the device tree binding. =A0Have the software number the
>> message register banks in increasing physical address order, or based
>> on numbered aliases similar to how U-Boot enumerates ethernet nodes, or
>> something similar.
> Using physical addresses doesn't solve the enumeration problem either,
> but I think it's beside the point: userspace must refer to the device.
> There is a rich history of userspace *not* walking /proc/device-tree in
> order to refer to a physical device. Are you suggesting this case is
> special?

Actually, for a while now the kernel has been moving towards userspace
being responsible for device identification.  That's what udev is for.
 The kernel udev looks at the available information when a device is
registered/bound, and it creates useful symlinks to the dynamically
assigned major/minor devices.  The rest of userspace doesn't need to
know about it; it can simply use the symlinks in /dev, but it is
appropriate to let udev figure out the correct naming.

Also, if you want to do global enumeration of devices, the accepted
way to do so is to use properties in the /aliases node in the form:
'<type><number> =3D "/path/to/device/node";'

g.

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Blanchard, Hollis]

On 01/07/2011 07:48 AM, Grant Likely wrote:
> Actually, for a while now the kernel has been moving towards userspace
> being responsible for device identification.  That's what udev is for.
>   The kernel udev looks at the available information when a device is
> registered/bound, and it creates useful symlinks to the dynamically
> assigned major/minor devices.  The rest of userspace doesn't need to
> know about it; it can simply use the symlinks in /dev, but it is
> appropriate to let udev figure out the correct naming.
Can you point out an example of how this is done for Open Firmware=20
devices currently? In particular, how are the udev rules supposed to=20
operate? They get a device path, use that to find additional information =

in /proc/device-tree (hope that's mounted), and then what? Do we=20
hardcode the block addresses in the rules, for example @1400 and @2400?

Hollis Blanchard
Mentor Graphics, Embedded Systems Division

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Grant Likely]

On Fri, Jan 7, 2011 at 9:00 AM, Blanchard, Hollis
<Hollis_Blanchard@mentor.com> wrote:
> On 01/07/2011 07:48 AM, Grant Likely wrote:
>> Actually, for a while now the kernel has been moving towards userspace
>> being responsible for device identification. =A0That's what udev is for.
>> =A0 The kernel udev looks at the available information when a device is
>> registered/bound, and it creates useful symlinks to the dynamically
>> assigned major/minor devices. =A0The rest of userspace doesn't need to
>> know about it; it can simply use the symlinks in /dev, but it is
>> appropriate to let udev figure out the correct naming.
> Can you point out an example of how this is done for Open Firmware
> devices currently?

Nope, because while it has been a theoretical concern, in practice the
issue hasn't come up on any of the hardware I've worked on or I've
seen patches for.  Mostly this is because the device tree
representations are internally self consistent;
dependencies/connections are explicitly expressed with phandles or
full paths which eliminates any need for enumeration in all the cases
I've had to deal with.

What I can point at is that almost all Linux character, block, and
network devices are assigned major/minor numbers in a first-come
first-served basis.  This of course causes problems for userspace
applications who need a stable handle to a specific device.  Device
renaming turned out to be horribly complex, which is why for character
and block devices the whole problem was punted out into userspace for
udev to handle with symlinks.  Network device unfortunately don't
appear in /dev, so they are stuck with device renaming until some form
of network device name aliasing is created.

As Scott also says, I know from experience that trying to maintain
enumeration in the device tree nodes themselves, and getting device
drivers to request specific numbers is painful, complex and fragile.
If I could go back in time I would never have attempted it on the
mpc5200 psc (serial) devices.

> In particular, how are the udev rules supposed to
> operate? They get a device path, use that to find additional information
> in /proc/device-tree (hope that's mounted), and then what? Do we
> hardcode the block addresses in the rules, for example @1400 and @2400?

You can do that; but from the little bit I know about your use case,
user-space visible references would be better explicitly expressed in
the aliases node.  It isn't currently, but matching aliases can easily
be added to the device's UEVENT property in /sys.  That is probably
the approach I would investigate for getting instance information out
to udev so it can create useful symlinks to the /dev device files.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Blanchard, Hollis]

On 01/07/2011 08:44 AM, Grant Likely wrote:
> On Fri, Jan 7, 2011 at 9:00 AM, Blanchard, Hollis
> <Hollis_Blanchard@mentor.com>  wrote:
>> On 01/07/2011 07:48 AM, Grant Likely wrote:
>>> Actually, for a while now the kernel has been moving towards =
userspace
>>> being responsible for device identification.  That's what udev is =
for.
>>>    The kernel udev looks at the available information when a device =
is
>>> registered/bound, and it creates useful symlinks to the dynamically
>>> assigned major/minor devices.  The rest of userspace doesn't need to
>>> know about it; it can simply use the symlinks in /dev, but it is
>>> appropriate to let udev figure out the correct naming.
>> Can you point out an example of how this is done for Open Firmware
>> devices currently?
> Nope, because while it has been a theoretical concern, in practice the
> issue hasn't come up on any of the hardware I've worked on or I've
> seen patches for.
That is still this case here: it's a theoretical concern. Don't forget:=20
we're talking about eight registers in the MPIC. There is no bus, no=20
hotplug, nothing. Really, the only requirement is that somehow we can=20
match the names used in the hardware documentation.
> Mostly this is because the device tree
> representations are internally self consistent;
> dependencies/connections are explicitly expressed with phandles or
> full paths which eliminates any need for enumeration in all the cases
> I've had to deal with.
The device tree on every platform contains devices which must be=20
referenced by userspace. That is the problem we're trying to solve --=20
let's not get distracted by theoretical principles of enumeration.

The closest analogy might be serial devices, which unlike ethernet=20
devices don't have other distinguishing information for udev to=20
interpret. To my surprise, when I reverse the order of the serial nodes=20
in the device tree, the ttyS0/S1 ordering reverses too. This is exactly=20
the problem we were trying to avoid... but it seems nobody has solved it =

anywhere. :(

Hollis Blanchard
Mentor Graphics, Embedded Systems Division

Re: [RFC] MPIC Bindings and Bindings for AMP Systems

[Scott Wood]

On Fri, 7 Jan 2011 12:30:52 -0800
"Blanchard, Hollis" <Hollis_Blanchard@mentor.com> wrote:

> On 01/07/2011 08:44 AM, Grant Likely wrote:
> > On Fri, Jan 7, 2011 at 9:00 AM, Blanchard, Hollis
> > <Hollis_Blanchard@mentor.com>  wrote:
> >> On 01/07/2011 07:48 AM, Grant Likely wrote:
> >>> Actually, for a while now the kernel has been moving towards userspace
> >>> being responsible for device identification.  That's what udev is for.
> >>>    The kernel udev looks at the available information when a device is
> >>> registered/bound, and it creates useful symlinks to the dynamically
> >>> assigned major/minor devices.  The rest of userspace doesn't need to
> >>> know about it; it can simply use the symlinks in /dev, but it is
> >>> appropriate to let udev figure out the correct naming.
> >> Can you point out an example of how this is done for Open Firmware
> >> devices currently?
> > Nope, because while it has been a theoretical concern, in practice the
> > issue hasn't come up on any of the hardware I've worked on or I've
> > seen patches for.
> That is still this case here: it's a theoretical concern. Don't forget: 
> we're talking about eight registers in the MPIC. There is no bus, no 
> hotplug, nothing. Really, the only requirement is that somehow we can 
> match the names used in the hardware documentation.

It's a practical concern on other devices, and we need to solve that.
Once that happens, the same infrastructure should be usable to deal
with this as well -- though really, it's a separate issue from what
goes in the device tree.

The established mechanism for "match the names used in the hardware
documentation" is aliases.

In this particular case, if you want userspace to just see timers 0-7,
have the kernel driver look for the standard aliases to determine which
block is which, and expose it to userspace in the manner of your
choosing.

> > Mostly this is because the device tree
> > representations are internally self consistent;
> > dependencies/connections are explicitly expressed with phandles or
> > full paths which eliminates any need for enumeration in all the cases
> > I've had to deal with.
> The device tree on every platform contains devices which must be 
> referenced by userspace. That is the problem we're trying to solve -- 
> let's not get distracted by theoretical principles of enumeration.
> 
> The closest analogy might be serial devices, which unlike ethernet 
> devices don't have other distinguishing information for udev to 
> interpret. To my surprise, when I reverse the order of the serial nodes 
> in the device tree, the ttyS0/S1 ordering reverses too. This is exactly 
> the problem we were trying to avoid... but it seems nobody has solved it 
> anywhere. :(

What if you plug in a PCI serial card?  Or USB?  What if you're under a
hypervisor and you have a variety of virtual serial/console/etc.
objects that don't have any meaningful enumeration order?

As Grant suggested, udev should be made aware of device-tree aliases --
which would be the distinguishing information udev needs.

In the absence of aliases or other good information, perhaps the udev
rule could fall back on heuristics such as bus type and address to at
least reduce unpredictability.

-Scott