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

* Re: devicetree-discuss Digest, Vol 6, Issue 7
       [not found] ` <mailman.11.1229475607.16627.devicetree-discuss-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
@ 2008-12-18 14:17   ` Christian Rund
       [not found]     ` <OF6B36395E.E0A4363C-ONC1257522.0035FA7C-C1257523.004E84FF-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Christian Rund @ 2008-12-18 14:17 UTC (permalink / raw)
  Cc: devicetree-discuss-bounces+christian.rund=de.ibm.com-mnsaURCQ41sdnm+yROfE0A,
	devicetree-discuss-mnsaURCQ41sdnm+yROfE0A


[-- Attachment #1.1: Type: text/plain, Size: 14623 bytes --]

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" <Christian.Rund-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> Cc: devicetree-discuss-mnsaURCQ41sdnm+yROfE0A@public.gmane.org
> Message-ID:
>    <fa686aa40812161408j3672ba5cs7d8526cd643b69de-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
> Content-Type: text/plain; charset=ISO-8859-1
> 
> On Tue, Dec 16, 2008 at 7:03 AM, Christian Rund
> <Christian.Rund-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org> wrote:
> > 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
> > 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
> 
> > 1 "be" node
> [...]
> > "device-type" property
> >
> >             Standard property name which specifies the type of the 
node.
> >
> >             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.

> > "model" property
> >             property name: Specifies model of node.
> >             Encoded as with encode-string.
> >             Default value is { "IBM,CBEA" }.
> 
> > "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.
 
> > 1 ioc node
> >
> >
> > The Input/Output Controller (IOC) node contains among others the 
properties
> > specifying the address range of MMIO register space controlling the 
IOC.
> >
> > "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 
>
> > 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 >
> > pairs. Value one in this array corresponds to the first offset value 
within
> > the child address space, encoded as with encode-phys. Value two 
corresponds
> > 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 }.

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

> > 2 "bic0" node
> [...]
> > "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

> 
> > 3 "bic1" node
> 
> This is identical to bic0
> 
> > 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.

> 
> > 5 "pervasive" node
> >
> >
> > The pervasive node node represents the  pervasive unit in the device 
tree.
> > 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.

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

> > "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.
> 
> > 7 "interrupt-controller" node
> >
> > The Cell Broadband Engine Architecture processor contains an Internal
> > Interrupt Controller (IIC), which is handling all the interrupts from 
the
> > PPE, the SPE and the connected IO.
> >
> > "reg" property
> >
> > Standard property name: Property to specify the MMIO offset of the 
IIC, one
> > range for each of the two threads contained in each PPE and one range 
for
> > the common MMIO.
> >
> > prop-encoded-array: Consisting of six 32-bit values. The values form 
three
> > < offset,length > pairs of the denoted space encoded as with 
encode-phys
> > 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.

> > 8 "spe" nodes
> >
> > The Cell Broadband Engine Architecture processor contains eight SPEs, 
each
> > consisting of an SPU, 256kB local store and a Memory Flow Controller 
(MFC).
> > 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
> > 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
> > 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.
> 
> > "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.

> > "interrupts" property
> >
> > Standard property name: Property to specify the interrupt numbers of 
the
> > interrupts issued by SPE.
> >
> > prop-encoded array: List of interrupt numbers issued by the SPE. Each 
value
> > 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
> > 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 <krafft-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>

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 <krafft-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>

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

[-- Attachment #1.2: Type: text/html, Size: 21913 bytes --]

[-- Attachment #2: Type: text/plain, Size: 194 bytes --]

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

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