Re: devicetree-discuss Digest, Vol 6, Issue 7

devicetree-discuss-bounces+christian.rund=de.ibm.com-mnsaURCQ41sdnm+yROfE0A@public.gmane.org wrote on 
17.12.2008 02:00:07:

Send devicetree-discuss mailing list submissions to
   devicetree-discuss-mnsaURCQ41sdnm+yROfE0A@public.gmane.org

To subscribe or unsubscribe via the World Wide Web, visit
   https://ozlabs.org/mailman/listinfo/devicetree-discuss
or, via email, send a message with subject or body 'help' to
   devicetree-discuss-request-mnsaURCQ41sdnm+yROfE0A@public.gmane.org

You can reach the person managing the list at
   devicetree-discuss-owner-mnsaURCQ41sdnm+yROfE0A@public.gmane.org

When replying, please edit your Subject line so it is more specific
than "Re: Contents of devicetree-discuss digest..."


Today's Topics:

   1. Re: Reminder 1: Device Tree documentation discussion for
      Cell/B.E.   binding DRAFT - see Digest Vol 6, Issue 1 (Grant 

Likely)

----------------------------------------------------------------------

Message: 1
Date: Tue, 16 Dec 2008 15:08:17 -0700
From: "Grant Likely" <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Subject: Re: Reminder 1: Device Tree documentation discussion for
   Cell/B.E.   binding DRAFT - see Digest Vol 6, Issue 1
To: "Christian Rund" <redacted>
Cc: devicetree-discuss-mnsaURCQ41sdnm+yROfE0A@public.gmane.org
Message-ID:
   [off-list ref]
Content-Type: text/plain; charset=ISO-8859-1

On Tue, Dec 16, 2008 at 7:03 AM, Christian Rund
[off-list ref] wrote:

quoted

this is a reminder for review. Feedback appreciated!
For convenience I removed the disclaimer etc.

On dec 5th I posted our (complete) draft version of the Cell/B.E. 

device

quoted

tree documentation.

Goal of these review should be to finally establish the document as
Power.org
PAPR Binding for the Cell/B.E. processor.

Please review and give us feedback.

First comments; This document is quite verbose.  Much time is spent
describing how standard properties work (ie; content of the 'reg' and
'ranges' properties).  Reviewing takes more effort when there is a
large amount of boilerplate to filter.

Second, very few nodes have a 'compatible' value defined.  Currently,
the 'compatible' list is the preferred method for device drivers to
find supported devices in the tree.  Each distinct device should have
a unique compatible value defined.

More comments below

quoted

1 "be" node

[...]

quoted

"device-type" property

            Standard property name which specifies the type of the 

node.

quoted

            Encoded as with encode-string.

            Default value is { "be" }.

Is a new OpenFirmware driver interface being defined for working with
'be' devices? 

No

If not then the device-type property should not be
defined.

The device-type property instances will be removed where not 
necessary or no OpenFirmware binding recommendations exist
respectively.

quoted

"model" property
            property name: Specifies model of node.
            Encoded as with encode-string.
            Default value is { "IBM,CBEA" }.

quoted

"ibm,dt-version" property

I'm concerned about the usage model of this property.  It is not
common for driver to go looking for an additional version property
when interpreting node data.  Are all drivers expected to test the
value of this property?  What should they do when the major or minor
values do not match?

This property is meant to be informational. Drivers could check whenever
they require a certain level.
 

quoted

1 ioc node


The Input/Output Controller (IOC) node contains among others the 

properties

quoted

specifying the address range of MMIO register space controlling the 

IOC.

quoted

"reg" property

Default property name: Property to specify the MMIO offset of the IOC,
which are two sets of registers each represented by an < offset, size 

quoted

pair.

prop-encoded-array: Encoded as with encode-phys for the offset values,
encode-int for the size values.

The array consists of four 32-bit values to represent two < offset, 

size >

quoted

pairs. Value one in this array corresponds to the first offset value 

within

quoted

the child address space, encoded as with encode-phys. Value two 

corresponds

quoted

to the size, encoded as with encode-int. Value three in this array
corresponds to the second offset value, value four to the second size.

Default value is { 0x00510000 0x00001000 0x00511000 0x00001000 }.

These spaces are contiguous; what is the reason for the two array 

entries?

The spaces are in fact contiguous, but belong to two different units.
0x00510xxx covers the IOC Address Translation MMIO Registers, whereras
0x00511xxx covers the I/O Command MMIO Registers.
Despite, Linux is mapping the two ranges in one single step, which means
the range can well be summarized to 
        Default value is { 0x00510000 0x00002000 }.

quoted

"device_type" property

Standard property name: Specify the type of this node

Encoded as with encode-string

Default value is { "ioc" }

Again, if an OpenFirmware driver interface is not being defined, then
don't use the device-type property.

see comment above.

quoted

2 "bic0" node

[...]

quoted

"device_type" property

Standard property name: Property to specify the type of this node.

Encoded as with encode-string.

Default value is { "bic0" }.

ditto on device_type

see comment above

quoted

3 "bic1" node

This is identical to bic0

quoted

4 "mic-tm" node
6 "ppe-mmio" node

mic-tm, ppe-mmio, and the bic* node documentation just duplicates the
definition of standard node properties.  In this case is should be
sufficient to describe what device the node describes an that it uses
the standard properties.

I will change the documentation to describe only one of a kind.

quoted

5 "pervasive" node


The pervasive node node represents the  pervasive unit in the device 

tree.

quoted

T>he main property value is the address range of MMIO register space
controlling the pervasive unit.

Umm, what is a "pervasive unit"?

The pervasive node represents the 'Pervasive MMIO Registers' (i.e.
Pervasive Monitor, Power Management and Thermal Management described
in the Cell/B.E. public register spec.

quoted

"device_type" property

Standard property name: Property to specify the type of this node.

Encoded as with encode-string.

Default value is { "pervasive" }.

ditto on device_type

See comment above.

quoted

"ppe-throttle-temp" property
"ppe-end-throttle-temp" property
"ppe-full-throttle-temp" property
"spe-throttle-temp" property
"spe-end-throttle-temp" property
"spe-full-throttle-temp" property

These properties look good.

quoted

7 "interrupt-controller" node

The Cell Broadband Engine Architecture processor contains an Internal
Interrupt Controller (IIC), which is handling all the interrupts from 

the

quoted

PPE, the SPE and the connected IO.

"reg" property

Standard property name: Property to specify the MMIO offset of the 

IIC, one

quoted

range for each of the two threads contained in each PPE and one range 

for

quoted

the common MMIO.

prop-encoded-array: Consisting of six 32-bit values. The values form 

three

quoted

< offset,length > pairs of the denoted space encoded as with 

encode-phys

quoted

for the offsets and encode-int for the sizes

  1. for MMIO space of thread one

  2. for MMIO space of thread two

  3. for MMIO space of the common PPE MMIO space.

Default value is
{ 0x00508400 0x00000020 0x00508420 0x00000020 0x00508000 0x00001000 }.



"device_type" property

Standard property name: Property to specify the type of this node.

Encoded as with encode-string.

Default value is { "CBEA-Internal-Interrupt-Controller" }.

ditto on device_type.  In fact, you should not be inventing new values
for device_type at all.  Either use an already established
device_type, or don't assign the property at all.  And, if you do use
device_type, you are claiming that OpenFirmware has a driver interface
for the device.

See comment above.

quoted

8 "spe" nodes

The Cell Broadband Engine Architecture processor contains eight SPEs, 

each

quoted

consisting of an SPU, 256kB local store and a Memory Flow Controller 

(MFC).

quoted

The SPEs are connected to the EIB (Element Interconnect Bus) ring. The
access to the internal devices is done via MMIO reads, with a fixed 

offset

quoted

to the Cell Broadband Engine processor base address.

"reg" property

Standard property name: Specifies the MMIO offset and size of the SPEs
Local Storage, Problem-State, Privilege 2 Area and Privilege 1 Area.

prop-encoded array: Encoded as four < offset, length > pairs per SPE
encoded as with encode-phys for the offsets, encode-int for the size. 

The

quoted

pairs define the following SPE units:

  1. Local Store (LS)

  2. Problem State MMIO Registers

  3. Privilege State 2 MMIO Registers

  4. Privilege State 1 MMIO Registers

The property exists once in each spe node.

I like the above description.  It provides good information about how
data is organized in the reg property.

quoted

"device_type" property

Standard property name: Specifies the type of this node.

Encoded as with encode-string.

Default value is  {"spe" }.

ditto on device_type

See comment above.

quoted

"interrupts" property

Standard property name: Property to specify the interrupt numbers of 

the

quoted

interrupts issued by SPE.

prop-encoded array: List of interrupt numbers issued by the SPE. Each 

value

quoted

in the list is encoded as with encode-int.

The property exists once in each spe node.

Default values for SPEs are



|#          |Property Value                                |
|spe@0      |{ 0x4, 0x104, 0x204 }                         |
|spe@80000  |{ 0x7, 0x107, 0x207 }                         |
|spe@100000 |{ 0x3, 0x103, 0x203 }                         |
|spe@180000 |{ 0x8, 0x108, 0x208 }                         |
|spe@200000 |{ 0x2, 0x102, 0x202 }                         |
|spe@280000 |{ 0x9, 0x109, 0x209 }                         |
|spe@300000 |{0x1, 0x101, 0x201 }                          |
|spe@380000 |{0xa, 0x10a, 0x20a }                          |


"vicinity" property

property name: Specifies the direct neighbouring componentes on the 

EIB

quoted

ring related to each SPE.

prop-encoded array: Pairs of phandles ( < phandle, phandle >) of the
neighbouring nodes, each phandle is encoded as with encode-int.

The property exists once in each spe node.

Default values for SPEs



|#          |Property Value                                |
|spe@0      |{ phandle(mic-tm, SPE 3) }                    |
|spe@80000  |{ phandle(mic-tm, SPE 2) }                    |
|spe@100000 |{ phandle(SPE 0, SPE 4) }                     |
|spe@180000 |{ phandle(SPE 1, SPE 5) }                     |
|spe@200000 |{ phandle(SPE 2, SPE 6) }                     |
|spe@280000 |{ phandle(SPE 3, SPE 7) }                     |
|spe@300000 |{ phandle(SPE 4, BIC0) }                      |
|spe@380000 |{ phandle(SPE 5, BIC0) }                      |


"physical-id" property

property name: Property to specify the physical id of an SPE.

Default values for the physical id is encoded as with encode-int.

Unless there is some shared register set that needs the SPE physical
id to operate then I encourage you not to define this property.  If it
is just a logical number, the I think it is better to rely on the node
name instead of an arbitrarily defined logical number.  However, if
there is a register set shared between the SPEs that needs the SPE
number, then you should follow the lead of other existing bindings and
use the property name "cell-index" instead of physical-id.

The "physical-id" property is used by Linux (see information on ppc-patch 
below).

linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A@public.gmane.org
Subject: spu_manage: fix spu_unit_number for celleb device tree

From: Christian Krafft <redacted>

New device trees provide "physical-id".
Celleb device tree provide the "unit-id".
Legacy device tree used the reg property for the physical id of an spe.
This patch fixes find_spu_unit_number to look for the spu id in that 
order.
The length is checked to avoid misinterpretation in case the attributes
unit-id or reg do not contain the id.

Signed-off-by: Christian Krafft <redacted>

Index: linux/arch/powerpc/platforms/cell/spu_manage.c
===================================================================

--- linux.orig/arch/powerpc/platforms/cell/spu_manage.c
+++ linux/arch/powerpc/platforms/cell/spu_manage.c

@@ -48,10 +48,18 @@ static u64 __init find_spu_unit_number(s
 {
                 const unsigned int *prop;
                 int proplen;
+
+                /* new device trees should provide the physical-id 

attribute */
                 prop = of_get_property(spe, "physical-id", &proplen);
                 if (proplen == 4)
                                 return (u64)*prop;
 
+                /* celleb device tree provides the unit-id */
+                prop = of_get_property(spe, "unit-id", &proplen);
+                if (proplen == 4)
+                                return (u64)*prop;
+
+                /* legacy device trees provide the id in the reg 
attribute */
                 prop = of_get_property(spe, "reg", &proplen);
                 if (proplen == 4)
                                 return (u64)*prop;

Cheers,
g.

Thanks
Chris

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


------------------------------

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-mnsaURCQ41sdnm+yROfE0A@public.gmane.org
https://ozlabs.org/mailman/listinfo/devicetree-discuss


End of devicetree-discuss Digest, Vol 6, Issue 7
************************************************