Re: [PATCH v3 02/12] powerpc: wiiu: device tree

2022-06-29 Thread Segher Boessenkool
On Wed, Jun 29, 2022 at 08:13:13PM +0200, Krzysztof Kozlowski wrote:
> On 29/06/2022 18:13, Segher Boessenkool wrote:
> > On Wed, Jun 29, 2022 at 11:58:18AM +0200, Krzysztof Kozlowski wrote:
> >>> + /* TODO: Add SMP */
> >>> + PowerPC,espresso@0 {
> >>
> >> Node name should be generic, so "cpu". Unless something needs the
> >> specific node name?
> > 
> > This is how most other PowerPC firmwares do it.  The PowerPC processor
> > binding is older than the generic naming practice, so CPU nodes have
> > device_type "cpu" instead.  
> 
> ePAPR 1.0 from 2008 explicitly asks for generic node names. So 4 years
> before Nintento Wii U. Maybe earlier ePAPR-s were also asking for this,
> no clue, don't have them.

The majority of PowerPC 750 systems long predate that.  Many *current*
systems implement the PowerPC Processor Binding, too (not the epapr
thing, which is incompatible with the older standards!)

> > There is no added value in generic naming for CPU nodes anyway, since
> > you just find them as the children of the "/cpus" node :-)
> 
> There is because you might have there caches. It also makes code easier
> to read.

In the processor binding the cache nodes were subnodes of cpu nodes or
other cache nodes.  But in some server products you can have cache that
is enabled while the corresponding core is disabled; and also, not all
cache belongs to only one higher level anyway.  This was modelled pretty
uncleanly, yup (cleaner would have been to have a /caches node as well
as /cpus, for example).

But on 750 you just have "l2-cache" subnodes, and all nodes in /cpus are
CPUs :-)


Segher


Re: [PATCH v3 02/12] powerpc: wiiu: device tree

2022-06-29 Thread Krzysztof Kozlowski
On 29/06/2022 18:13, Segher Boessenkool wrote:
> On Wed, Jun 29, 2022 at 11:58:18AM +0200, Krzysztof Kozlowski wrote:
>> On 28/06/2022 15:31, Ash Logan wrote:
>>> +   model = "nintendo,wiiu";
>>
>> It's not compatible, but user-visible string, e.g. "Nintendo Wii U"
> 
> The "model" property in OF is documented as:
> 
> ---
> “model”S
> Standard property name to define a manufacturer’s model number.
> 
> prop-encoded-array:
>   Text string, encoded with encode-string.
> A manufacturer-dependent string that generally specifies the model name
> and number (including revision level) for this device. The format of the
> text string is arbitrary, although in conventional usage the string
> begins with the name of the device’s manufacturer as with the “name”
> property.
> Although there is no standard interpretation for the value of the
> “model” property, a specific device driver might use it to learn, for
> instance, the revision level of its particular device.
> 
> See also: property, model.
> 
> Used as: " XYZCO,1416-02" encode-string " model" property

Hm, surprising to duplicate the compatible, but OK.

> ---
> 
>>> +   cpus {
>>> +   #address-cells = <1>;
>>> +   #size-cells = <0>;
>>> +
>>> +   /* TODO: Add SMP */
>>> +   PowerPC,espresso@0 {
>>
>> Node name should be generic, so "cpu". Unless something needs the
>> specific node name?
> 
> This is how most other PowerPC firmwares do it.  The PowerPC processor
> binding is older than the generic naming practice, so CPU nodes have
> device_type "cpu" instead.  

ePAPR 1.0 from 2008 explicitly asks for generic node names. So 4 years
before Nintento Wii U. Maybe earlier ePAPR-s were also asking for this,
no clue, don't have them.

> This is a required property btw, with that
> value.  (There is no requirement on the names of the CPU nodes).

That's fine, I am not talking about property.

> There is no added value in generic naming for CPU nodes anyway, since
> you just find them as the children of the "/cpus" node :-)

There is because you might have there caches. It also makes code easier
to read.

Best regards,
Krzysztof


Re: [PATCH v3 02/12] powerpc: wiiu: device tree

2022-06-29 Thread Segher Boessenkool
On Wed, Jun 29, 2022 at 11:58:18AM +0200, Krzysztof Kozlowski wrote:
> On 28/06/2022 15:31, Ash Logan wrote:
> > +   model = "nintendo,wiiu";
> 
> It's not compatible, but user-visible string, e.g. "Nintendo Wii U"

The "model" property in OF is documented as:

---
“model”S
Standard property name to define a manufacturer’s model number.

prop-encoded-array:
  Text string, encoded with encode-string.
A manufacturer-dependent string that generally specifies the model name
and number (including revision level) for this device. The format of the
text string is arbitrary, although in conventional usage the string
begins with the name of the device’s manufacturer as with the “name”
property.
Although there is no standard interpretation for the value of the
“model” property, a specific device driver might use it to learn, for
instance, the revision level of its particular device.

See also: property, model.

Used as: " XYZCO,1416-02" encode-string " model" property
---

> > +   cpus {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   /* TODO: Add SMP */
> > +   PowerPC,espresso@0 {
> 
> Node name should be generic, so "cpu". Unless something needs the
> specific node name?

This is how most other PowerPC firmwares do it.  The PowerPC processor
binding is older than the generic naming practice, so CPU nodes have
device_type "cpu" instead.  This is a required property btw, with that
value.  (There is no requirement on the names of the CPU nodes).

There is no added value in generic naming for CPU nodes anyway, since
you just find them as the children of the "/cpus" node :-)


Segher


Re: [PATCH v3 02/12] powerpc: wiiu: device tree

2022-06-29 Thread Krzysztof Kozlowski
On 28/06/2022 15:31, Ash Logan wrote:
> Add a device tree source file for the Nintendo Wii U video game console.
> 
> Signed-off-by: Ash Logan 
> Co-developed-by: Roberto Van Eeden 
> Signed-off-by: Roberto Van Eeden 
> Co-developed-by: Emmanuel Gil Peyrot 
> Signed-off-by: Emmanuel Gil Peyrot 
> ---
> v1->v2: Style and formatting changes suggested by Rob Herring.
>  License remains GPL-2.0 as the other powerpc dtses are the same, happy
>  to change if there is a different preferred default.
> v2->v3: Re-added address-cells accidentally removed in v2.
>  Marked latte as a simple-bus, since it is.

Thank you for your patch. There is something to discuss/improve.

> 
>  arch/powerpc/boot/dts/wiiu.dts | 326 +
>  1 file changed, 326 insertions(+)
>  create mode 100644 arch/powerpc/boot/dts/wiiu.dts
> 
> diff --git a/arch/powerpc/boot/dts/wiiu.dts b/arch/powerpc/boot/dts/wiiu.dts
> new file mode 100644
> index ..44a5a1469095
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/wiiu.dts
> @@ -0,0 +1,326 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Nintendo Wii U Device Tree Source
> + *
> + * Copyright (C) 2022 The linux-wiiu Team
> + */
> +
> +/dts-v1/;
> +#include 
> +#include 
> +
> +/ {
> + model = "nintendo,wiiu";

It's not compatible, but user-visible string, e.g. "Nintendo Wii U"

> + compatible = "nintendo,wiiu";
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + chosen {
> + bootargs = "root=/dev/sda1 rootwait";

This does not belong to shared DTS. No bootargs.

> + };
> +
> + memory {
> + device_type = "memory";
> + reg = <0x 0x0200/* MEM1 - 32MiB */
> +0x0800 0x0030/* MEM0 - 3MiB  */
> +0x1000 0x8000>;  /* MEM2 - 2GiB  */
> + };
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /* TODO: Add SMP */
> + PowerPC,espresso@0 {

Node name should be generic, so "cpu". Unless something needs the
specific node name?

> + device_type = "cpu";
> + reg = <0>;
> + clock-frequency = <1243125000>; /* 1.243125GHz 
> */
> + bus-frequency = <248625000>;/* 248.625MHz 
> core-to-bus 5x */
> + timebase-frequency = <62156250>;/* 1/4 of the 
> bus clock */
> + i-cache-size = <32768>; /* 32K icache */
> + i-cache-line-size = <32>;
> + i-cache-block-size = <32>;
> + i-cache-sets = <128>;
> + d-cache-size = <32768>; /* 32K dcache */
> + d-cache-line-size = <32>;
> + d-cache-block-size = <32>;
> + d-cache-sets = <128>;
> + next-level-cache = <_0>;
> + L2_0:l2-cache {
> + compatible = "cache";
> + cache-level = <2>;
> + cache-unified;
> + cache-size = <0x8>; /* 512KB L2 */
> + cache-line-size = <64>;
> + cache-block-size = <32>;
> + cache-sets = <2048>;
> + };
> + };
> + };
> +
> + latte {

Generic node names.

> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "nintendo,latte", "simple-bus";
> + ranges = <0x0c00 0x0c00 0x0040  /* 
> Espresso-only registers */
> +   0x0d00 0x0d00 0x0020  /* Latte AHB 
> deivces */
> +   0x0d80 0x0d80 0x0080>;/* Latte SoC 
> registers */
> +
> + latte_gpu: gpu@c20 {
> + compatible = "nintendo,latte-gpu7";
> + reg = <0x0c20 0x8>;
> + interrupts = <2>;
> + interrupt-parent = <_pic>;
> + };
> +
> + espresso_pic: pic@c78 {

interrupt-controller

> + #interrupt-cells = <1>;
> + interrupt-controller;
> +
> + compatible = "nintendo,espresso-pic";
> + reg = <0x0c78 0x18>;
> + };
> +
> + latte_dsp: dsp@c005000 {
> + compatible = "nintendo,latte-dsp";
> + reg = <0x0c005000 0x200>;
> + };
> +
> + ehci_0: usb@d04 {
> + compatible = "nintendo,latte-usb-ehci", "usb-ehci";
> + reg = <0x0d04 0x100>;
> + interrupts = <4>;
> + interrupt-parent = <_pic>;
> + big-endian-regs;
> + };
> +
> + ohci_0_0: usb@d05 {

[PATCH v3 02/12] powerpc: wiiu: device tree

2022-06-28 Thread Ash Logan
Add a device tree source file for the Nintendo Wii U video game console.

Signed-off-by: Ash Logan 
Co-developed-by: Roberto Van Eeden 
Signed-off-by: Roberto Van Eeden 
Co-developed-by: Emmanuel Gil Peyrot 
Signed-off-by: Emmanuel Gil Peyrot 
---
v1->v2: Style and formatting changes suggested by Rob Herring.
 License remains GPL-2.0 as the other powerpc dtses are the same, happy
 to change if there is a different preferred default.
v2->v3: Re-added address-cells accidentally removed in v2.
 Marked latte as a simple-bus, since it is.

 arch/powerpc/boot/dts/wiiu.dts | 326 +
 1 file changed, 326 insertions(+)
 create mode 100644 arch/powerpc/boot/dts/wiiu.dts

diff --git a/arch/powerpc/boot/dts/wiiu.dts b/arch/powerpc/boot/dts/wiiu.dts
new file mode 100644
index ..44a5a1469095
--- /dev/null
+++ b/arch/powerpc/boot/dts/wiiu.dts
@@ -0,0 +1,326 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Nintendo Wii U Device Tree Source
+ *
+ * Copyright (C) 2022 The linux-wiiu Team
+ */
+
+/dts-v1/;
+#include 
+#include 
+
+/ {
+   model = "nintendo,wiiu";
+   compatible = "nintendo,wiiu";
+
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   chosen {
+   bootargs = "root=/dev/sda1 rootwait";
+   };
+
+   memory {
+   device_type = "memory";
+   reg = <0x 0x0200/* MEM1 - 32MiB */
+  0x0800 0x0030/* MEM0 - 3MiB  */
+  0x1000 0x8000>;  /* MEM2 - 2GiB  */
+   };
+
+   cpus {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   /* TODO: Add SMP */
+   PowerPC,espresso@0 {
+   device_type = "cpu";
+   reg = <0>;
+   clock-frequency = <1243125000>; /* 1.243125GHz 
*/
+   bus-frequency = <248625000>;/* 248.625MHz 
core-to-bus 5x */
+   timebase-frequency = <62156250>;/* 1/4 of the 
bus clock */
+   i-cache-size = <32768>; /* 32K icache */
+   i-cache-line-size = <32>;
+   i-cache-block-size = <32>;
+   i-cache-sets = <128>;
+   d-cache-size = <32768>; /* 32K dcache */
+   d-cache-line-size = <32>;
+   d-cache-block-size = <32>;
+   d-cache-sets = <128>;
+   next-level-cache = <_0>;
+   L2_0:l2-cache {
+   compatible = "cache";
+   cache-level = <2>;
+   cache-unified;
+   cache-size = <0x8>; /* 512KB L2 */
+   cache-line-size = <64>;
+   cache-block-size = <32>;
+   cache-sets = <2048>;
+   };
+   };
+   };
+
+   latte {
+   #address-cells = <1>;
+   #size-cells = <1>;
+   compatible = "nintendo,latte", "simple-bus";
+   ranges = <0x0c00 0x0c00 0x0040  /* 
Espresso-only registers */
+ 0x0d00 0x0d00 0x0020  /* Latte AHB 
deivces */
+ 0x0d80 0x0d80 0x0080>;/* Latte SoC 
registers */
+
+   latte_gpu: gpu@c20 {
+   compatible = "nintendo,latte-gpu7";
+   reg = <0x0c20 0x8>;
+   interrupts = <2>;
+   interrupt-parent = <_pic>;
+   };
+
+   espresso_pic: pic@c78 {
+   #interrupt-cells = <1>;
+   interrupt-controller;
+
+   compatible = "nintendo,espresso-pic";
+   reg = <0x0c78 0x18>;
+   };
+
+   latte_dsp: dsp@c005000 {
+   compatible = "nintendo,latte-dsp";
+   reg = <0x0c005000 0x200>;
+   };
+
+   ehci_0: usb@d04 {
+   compatible = "nintendo,latte-usb-ehci", "usb-ehci";
+   reg = <0x0d04 0x100>;
+   interrupts = <4>;
+   interrupt-parent = <_pic>;
+   big-endian-regs;
+   };
+
+   ohci_0_0: usb@d05 {
+   compatible = "nintendo,latte-usb-ohci";
+   reg = <0x0d05 0x100>;
+   interrupts = <5>;
+   interrupt-parent = <_pic>;
+
+   big-endian-regs;
+   };
+
+   ohci_0_1: usb@d06 {
+   compatible = "nintendo,latte-usb-ohci";
+   reg = <0x0d06 0x100>;
+   interrupts = <6>;