linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Albert Herranz <albert_herranz@yahoo.es>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: linuxppc-dev@lists.ozlabs.org
Subject: Re: [RFC PATCH 02/19] powerpc: gamecube: device tree
Date: Mon, 23 Nov 2009 20:44:03 +0100	[thread overview]
Message-ID: <4B0AE603.9050208@yahoo.es> (raw)
In-Reply-To: <fa686aa40911221502g2de254d2o4341d9abed0cdf41@mail.gmail.com>

Grant Likely wrote:
> On Sun, Nov 22, 2009 at 3:01 PM, Albert Herranz <albert_herranz@yahoo.es> wrote:
>> Add a device tree source file for the Nintendo GameCube video game console.
>>
>> Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
> 
> Mostly looks good.  A few comments below.  Biggest comment is you need
> to add a brief blurb for every new "compatible" property value that
> you've added to Documentation/powerpc/dts-bindings.  General agreement
> is that we don't merge drivers with new OF tree bindings unless the
> binding is also documented.  It doesn't need to be long, it just needs
> to state what the device is, and what properties are expected.  If you
> define new properties, then you need to state what they are used for.
> 
> Once the comments below are addressed...
> 
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
> 

Thanks. I'll add the documentation for the new compatible properties.

>> ---
>>  arch/powerpc/boot/dts/gamecube.dts |  135 ++++++++++++++++++++++++++++++++++++
>>  1 files changed, 135 insertions(+), 0 deletions(-)
>>  create mode 100644 arch/powerpc/boot/dts/gamecube.dts
>>
>> diff --git a/arch/powerpc/boot/dts/gamecube.dts b/arch/powerpc/boot/dts/gamecube.dts
>> new file mode 100644
>> index 0000000..941a2c4
>> --- /dev/null
>> +++ b/arch/powerpc/boot/dts/gamecube.dts
>> @@ -0,0 +1,135 @@
>> +/*
>> + * arch/powerpc/boot/dts/gamecube.dts
>> + *
>> + * Nintendo GameCube platform device tree source
>> + * Copyright (C) 2007-2009 The GameCube Linux Team
>> + * Copyright (C) 2007,2008,2009 Albert Herranz
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version 2
>> + * of the License, or (at your option) any later version.
>> + *
>> + */
>> +
>> +/dts-v1/;
>> +
>> +/ {
>> +       model = "NintendoGameCube";
>> +       compatible = "nintendo,gamecube";
> 
> To date, we've been using the same form for both the model and
> compatible properties.  Specifically the <vendor>,<model> form to
> maintain the namespace.
> 

Ok, I'll change model to "nintendo,gamecube".

>> +       #address-cells = <1>;
>> +       #size-cells = <1>;
>> +
>> +       chosen {
>> +               bootargs = "root=/dev/gcnsda2 rootwait udbg-immortal";
>> +               linux,stdout-path = &USBGECKO0;
>> +       };
>> +
>> +       aliases {
>> +               ugecon = &USBGECKO0;
>> +       };
>> +
>> +       memory {
>> +               device_type = "memory";
>> +               /* 24M minus framebuffer memory area (640*576*2*2) */
>> +               reg = <0x00000000 0x01698000>;
>> +       };
>> +
>> +       cpus {
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +
>> +               PowerPC,gekko@0 {
>> +                       device_type = "cpu";
>> +                       reg = <0>;
>> +                       clock-frequency = <486000000>; /* 486MHz */
>> +                       bus-frequency = <162000000>; /* 162MHz core-to-bus 3x */
>> +                       timebase-frequency = <40500000>; /* 162MHz / 4 */
>> +                       i-cache-line-size = <32>;
>> +                       d-cache-line-size = <32>;
>> +                       i-cache-size = <32768>;
>> +                       d-cache-size = <32768>;
>> +               };
>> +       };
>> +
>> +       /* devices contained int the flipper chipset */
>> +       soc {
> 
> It would be better to rename this as IMMR or the bus type.  This node
> doesn't actually describe the entire chip, but describes the internal
> memory mapped registers.
> 

Can you please elaborate more on this or point me to documentation?
The soc node here tries to represent the big multi-function chip that integrates most of the devices of the video game consoles ("Flipper" on the Nintendo GameCube and "Hollywood" on the Wii).

>> +               #address-cells = <1>;
>> +               #size-cells = <1>;
>> +               #interrupt-cells = <1>;
>> +               model = "flipper";
> 
> Drop the model property
> 

Ok, I'll fix that.

>> +               compatible = "nintendo,flipper";
>> +               clock-frequency = <162000000>; /* 162MHz */
>> +               ranges = <0x0c000000 0x0c000000 0x00010000>;
> 
> Since you're only doing 1:1 mappings; you could replace this with an
> empty "ranges;" property instead.
> 

Nice, didn't know about this. I'll do that.

>> +
>> +               video@0c002000 {
>> +                       compatible = "nintendo,flipper-video";
>> +                       reg = <0x0c002000 0x100>;
>> +                       interrupts = <8>;
>> +                       interrupt-parent = <&pic>;
> 
> Hint:  If you move the interrupt-parent property up to the root node,
> then you don't need to specify it in every single device node; it will
> just inherit from the parent.
> 

Got it. This makes a lot of sense here with a single interrupt controller.

>> +                       /* XFB is the eXternal FrameBuffer */
>> +                       xfb-start = <0x01698000>; /* end-of-ram - xfb-size */
>> +                       xfb-size = <0x168000>;
> 
> Can 'xfb' be made a second tuple to the 'reg' property so that all the
> address mapping code works on it?  ie:
> 
> reg = <0x0c002000 0x100
>        0x01698000 0x168000>;
> 
> At the very least, it is wise to adopt the same form as the reg
> property when describing address ranges instead of splitting it into
> multiple properties.
> 

I'll look into moving xfb-* into the reg property.

>> +               };
>> +
>> +               pic: pic@0c003000 {
>> +                       #interrupt-cells = <1>;
>> +                       compatible = "nintendo,flipper-pic";
>> +                       reg = <0x0c003000 0x8>;
>> +                       interrupt-controller;
>> +               };
>> +
>> +               resetswitch@0c003000 {
>> +                       compatible = "nintendo,flipper-resetswitch";
>> +                       reg = <0x0c003000 0x4>;
>> +                       interrupts = <1>;
>> +                       interrupt-parent = <&pic>;
>> +               };
>> +
>> +               auxram@0c005000 {
>> +                       compatible = "nintendo,flipper-auxram";
>> +                       reg = <0x0c005000 0x200>;       /* DSP */
>> +                       interrupts = <6>;
>> +                       interrupt-parent = <&pic>;
>> +               };
>> +
>> +               audio@0c005000 {
>> +                       compatible = "nintendo,flipper-audio";
>> +                       reg = <0x0c005000 0x200         /* DSP */
>> +                              0x0c006c00 0x20>;        /* AI */
>> +                       interrupts = <6>;
>> +                       interrupt-parent = <&pic>;
>> +               };
>> +
>> +               disk@0c006000 {
>> +                       compatible = "nintendo,flipper-disk";
>> +                       reg = <0x0c006000 0x40>;
>> +                       interrupts = <2>;
>> +                       interrupt-parent = <&pic>;
>> +               };
>> +
>> +               serial@0c006400 {
>> +                       compatible = "nintendo,flipper-serial";
>> +                       reg = <0x0c006400 0x100>;
>> +                       interrupts = <3>;
>> +                       interrupt-parent = <&pic>;
>> +               };
>> +
>> +               /* External Interface bus */
>> +               exi@0c006800 {
>> +                       #address-cells = <1>;
>> +                       #size-cells = <1>;
>> +                       compatible = "nintendo,flipper-exi";
>> +                       reg = <0x0c006800 0x40>;
>> +                       interrupts = <4>;
>> +                       interrupt-parent = <&pic>;
>> +
>> +                       USBGECKO0: usbgecko@0c006814 {
>> +                               compatible = "usbgecko,usbgecko";
>> +                               reg = <0x0c006814 0x14>;
>> +                               virtual-reg = <0xcc006814>;
>> +                       };
>> +               };
>> +        };
>> +};
>> +

Cheers,
Albert

  reply	other threads:[~2009-11-23 19:44 UTC|newest]

Thread overview: 181+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-22 22:01 [RFC PATCH 00/19] powerpc: nintendo gamecube and wii support Albert Herranz
2009-11-22 22:01 ` [RFC PATCH 01/19] powerpc: gamecube/wii: usbgecko bootwrapper console support Albert Herranz
2009-11-22 22:01   ` [RFC PATCH 02/19] powerpc: gamecube: device tree Albert Herranz
2009-11-22 22:01     ` [RFC PATCH 03/19] powerpc: gamecube: bootwrapper bits Albert Herranz
2009-11-22 22:01       ` [RFC PATCH 04/19] powerpc: wii: device tree Albert Herranz
2009-11-22 22:01         ` [RFC PATCH 05/19] powerpc: wii: bootwrapper bits Albert Herranz
2009-11-22 22:01           ` [RFC PATCH 06/19] powerpc: gamecube/wii: introduce GAMECUBE_COMMON Albert Herranz
2009-11-22 22:01             ` [RFC PATCH 07/19] powerpc: gamecube/wii: declare as non-coherent platforms Albert Herranz
2009-11-22 22:01               ` [RFC PATCH 08/19] powerpc: gamecube/wii: do not include PCI support Albert Herranz
2009-11-22 22:01                 ` [RFC PATCH 09/19] powerpc: gamecube/wii: udbg support for usbgecko Albert Herranz
2009-11-22 22:01                   ` [RFC PATCH 10/19] powerpc: gamecube/wii: early debugging using usbgecko Albert Herranz
2009-11-22 22:01                     ` [RFC PATCH 11/19] powerpc: gamecube/wii: flipper interrupt controller support Albert Herranz
2009-11-22 22:01                       ` [RFC PATCH 12/19] powerpc: gamecube: platform support Albert Herranz
2009-11-22 22:01                         ` [RFC PATCH 13/19] powerpc: gamecube: default config Albert Herranz
2009-11-22 22:01                           ` [RFC PATCH 14/19] powerpc: allow ioremap within reserved fake ram regions Albert Herranz
2009-11-22 22:01                             ` [RFC PATCH 15/19] powerpc: broadway processor support Albert Herranz
2009-11-22 22:01                               ` [RFC PATCH 16/19] powerpc: wii: hollywood interrupt controller support Albert Herranz
2009-11-22 22:01                                 ` [RFC PATCH 17/19] powerpc: wii: bootmii starlet 'mini' firmware support Albert Herranz
2009-11-22 22:01                                   ` [RFC PATCH 18/19] powerpc: wii: platform support Albert Herranz
2009-11-22 22:01                                     ` [RFC PATCH 19/19] powerpc: wii: default config Albert Herranz
2009-11-22 23:45                                     ` [RFC PATCH 18/19] powerpc: wii: platform support Grant Likely
2009-11-23 20:21                                       ` Albert Herranz
2009-11-24 22:24                                     ` Segher Boessenkool
2009-11-25 17:58                                       ` Albert Herranz
2009-11-26 22:05                                         ` Segher Boessenkool
2009-11-22 22:48                                   ` [RFC PATCH 17/19] powerpc: wii: bootmii starlet 'mini' firmware support Arnd Bergmann
2009-11-23 19:21                                     ` Albert Herranz
2009-11-24 22:13                                   ` Segher Boessenkool
2009-11-25 17:47                                     ` Albert Herranz
2009-11-26 22:00                                       ` Segher Boessenkool
2009-11-22 23:40                                 ` [RFC PATCH 16/19] powerpc: wii: hollywood interrupt controller support Grant Likely
2009-11-23 20:18                                   ` Albert Herranz
2009-11-24 21:54                                 ` Segher Boessenkool
2009-11-25 17:27                                   ` Albert Herranz
2009-11-26  5:23                                 ` Benjamin Herrenschmidt
2009-11-26 15:42                                   ` Albert Herranz
2009-11-26 21:13                                     ` Benjamin Herrenschmidt
2009-11-26 23:03                                   ` Segher Boessenkool
2009-11-22 23:38                               ` [RFC PATCH 15/19] powerpc: broadway processor support Grant Likely
2009-11-23 20:16                                 ` Albert Herranz
2009-11-22 23:36                             ` [RFC PATCH 14/19] powerpc: allow ioremap within reserved fake ram regions Grant Likely
2009-11-23 20:16                               ` Albert Herranz
2009-11-23 20:41                                 ` Grant Likely
2009-11-23 23:45                                 ` Michael Ellerman
2009-11-24 17:09                                   ` Albert Herranz
2009-11-25  0:38                                     ` Michael Ellerman
2009-11-26  5:22                                     ` Benjamin Herrenschmidt
2009-11-26 15:35                                       ` Albert Herranz
2009-11-26 21:13                                         ` Benjamin Herrenschmidt
2009-11-22 23:34                         ` [RFC PATCH 12/19] powerpc: gamecube: platform support Grant Likely
2009-11-23 20:02                           ` Albert Herranz
2009-11-24 21:39                         ` Segher Boessenkool
2009-11-25 17:24                           ` Albert Herranz
2009-11-26 21:53                             ` Segher Boessenkool
2009-11-26 22:07                               ` Albert Herranz
2009-11-27  0:09                                 ` Segher Boessenkool
2009-11-27 21:35                                   ` Benjamin Herrenschmidt
2009-11-28  3:56                                     ` Grant Likely
2009-11-28  3:56                                       ` Grant Likely
2009-11-22 23:28                       ` [RFC PATCH 11/19] powerpc: gamecube/wii: flipper interrupt controller support Grant Likely
2009-11-23 19:59                         ` Albert Herranz
2009-11-26  5:18                         ` Benjamin Herrenschmidt
2009-11-26 15:33                           ` Albert Herranz
2009-11-26 21:12                             ` Benjamin Herrenschmidt
2009-11-26 23:00                           ` Segher Boessenkool
2009-11-26 23:38                             ` Benjamin Herrenschmidt
2009-11-24 21:30                       ` Segher Boessenkool
2009-11-25 17:13                         ` Albert Herranz
2009-11-26 21:52                           ` Segher Boessenkool
2009-11-26 22:05                             ` Albert Herranz
2009-11-27  0:06                               ` Segher Boessenkool
2009-11-27 12:17                                 ` Segher Boessenkool
2009-11-27 17:27                                   ` Albert Herranz
2009-11-27 21:34                                     ` Benjamin Herrenschmidt
2009-11-28  2:04                                       ` Segher Boessenkool
2009-11-28  2:26                                         ` Benjamin Herrenschmidt
2009-11-26  5:18                       ` Benjamin Herrenschmidt
2009-11-26 15:30                         ` Albert Herranz
2009-11-26 21:10                           ` Benjamin Herrenschmidt
2009-11-22 22:27                     ` [RFC PATCH 10/19] powerpc: gamecube/wii: early debugging using usbgecko Arnd Bergmann
2009-11-23 19:10                       ` Albert Herranz
2009-11-23 22:13                         ` Arnd Bergmann
2009-11-26  5:14                       ` Benjamin Herrenschmidt
2009-11-26 22:54                         ` Segher Boessenkool
2009-11-26 23:27                           ` Benjamin Herrenschmidt
2009-11-24  0:54                     ` Segher Boessenkool
2009-11-24 18:19                       ` Albert Herranz
2009-11-22 23:22                   ` [RFC PATCH 09/19] powerpc: gamecube/wii: udbg support for usbgecko Grant Likely
2009-11-24  0:49                   ` Segher Boessenkool
2009-11-24 18:08                     ` Albert Herranz
2009-11-24 21:19                       ` Segher Boessenkool
2009-11-25 17:07                         ` Albert Herranz
2009-11-26  5:12                   ` Benjamin Herrenschmidt
2009-11-26 15:28                     ` Albert Herranz
2009-11-26 21:02                       ` Benjamin Herrenschmidt
2009-11-26 23:12                       ` Segher Boessenkool
2009-11-22 23:21                 ` [RFC PATCH 08/19] powerpc: gamecube/wii: do not include PCI support Grant Likely
2009-11-26  5:10                   ` Benjamin Herrenschmidt
2009-11-22 22:20               ` [RFC PATCH 07/19] powerpc: gamecube/wii: declare as non-coherent platforms Arnd Bergmann
2009-11-23 19:06                 ` Albert Herranz
2009-11-26  5:09               ` Benjamin Herrenschmidt
2009-11-22 23:20             ` [RFC PATCH 06/19] powerpc: gamecube/wii: introduce GAMECUBE_COMMON Grant Likely
2009-11-23 19:56               ` Albert Herranz
2009-11-24  0:35             ` Segher Boessenkool
2009-11-24 18:03               ` Albert Herranz
2009-11-24 21:15                 ` Segher Boessenkool
2009-11-24 22:33                   ` Arnd Bergmann
2009-11-25 17:06                   ` Albert Herranz
2009-11-26  5:08                 ` Benjamin Herrenschmidt
2009-11-26 15:22                   ` Albert Herranz
2009-11-27 12:12                   ` Segher Boessenkool
2009-11-22 23:19           ` [RFC PATCH 05/19] powerpc: wii: bootwrapper bits Grant Likely
2009-11-24  0:33           ` Segher Boessenkool
2009-11-24 17:56             ` Albert Herranz
2009-11-24 21:13               ` Segher Boessenkool
2009-11-22 23:18         ` [RFC PATCH 04/19] powerpc: wii: device tree Grant Likely
2009-11-23 19:54           ` Albert Herranz
2009-11-23 20:36             ` Grant Likely
2009-11-23 21:55               ` Albert Herranz
2009-11-25 16:57             ` Segher Boessenkool
2009-11-25 18:09               ` Albert Herranz
2009-11-25 17:49         ` Segher Boessenkool
2009-11-25 18:34           ` Albert Herranz
2009-11-26  4:58             ` Benjamin Herrenschmidt
2009-11-26 15:19               ` Albert Herranz
2009-11-26  4:51           ` Benjamin Herrenschmidt
2009-11-26 15:18             ` Albert Herranz
2009-11-26 21:01               ` Benjamin Herrenschmidt
2009-11-26 21:38                 ` Albert Herranz
2009-11-26 22:37                   ` Benjamin Herrenschmidt
2009-11-27  0:16                     ` Segher Boessenkool
2009-11-27  0:15                       ` Benjamin Herrenschmidt
2009-11-26 23:25                 ` Segher Boessenkool
2009-11-26 22:50             ` Segher Boessenkool
2009-11-26 23:02               ` Albert Herranz
2009-11-26  4:45         ` Benjamin Herrenschmidt
2009-11-26 15:09           ` Albert Herranz
2009-11-26 20:48             ` Benjamin Herrenschmidt
2009-11-26 21:20               ` Albert Herranz
2009-11-26 23:17               ` Segher Boessenkool
2009-11-26 22:45           ` Segher Boessenkool
2009-11-22 23:11       ` [RFC PATCH 03/19] powerpc: gamecube: bootwrapper bits Grant Likely
2009-11-24  0:08       ` Segher Boessenkool
2009-11-24 17:38         ` Albert Herranz
2009-11-24 21:00           ` Segher Boessenkool
2009-11-24 23:45             ` Albert Herranz
2009-11-25 16:53               ` Segher Boessenkool
2009-11-26  4:36             ` Benjamin Herrenschmidt
2009-11-26  8:17               ` Gabriel Paubert
2009-11-26  8:46                 ` Benjamin Herrenschmidt
2009-11-26 23:06                 ` Segher Boessenkool
2009-11-26  4:35           ` Benjamin Herrenschmidt
2009-11-26 22:23             ` Segher Boessenkool
2009-11-22 23:02     ` [RFC PATCH 02/19] powerpc: gamecube: device tree Grant Likely
2009-11-23 19:44       ` Albert Herranz [this message]
2009-11-23 20:19         ` Grant Likely
2009-11-23 20:25           ` Albert Herranz
2009-11-24 22:53           ` Segher Boessenkool
2009-11-26  4:23           ` Benjamin Herrenschmidt
2009-11-27 12:09             ` Segher Boessenkool
2009-11-24 22:36       ` Segher Boessenkool
2009-11-26  4:23         ` Benjamin Herrenschmidt
2009-11-26 22:15           ` Segher Boessenkool
2009-11-26 22:38             ` Benjamin Herrenschmidt
2009-11-27  0:18               ` Segher Boessenkool
2009-11-26  4:21       ` Benjamin Herrenschmidt
2009-11-26  4:38         ` Grant Likely
2009-11-26  5:29           ` Benjamin Herrenschmidt
2009-11-26  5:51             ` Grant Likely
2009-11-26 22:30         ` Segher Boessenkool
2009-11-25 18:00     ` Segher Boessenkool
2009-11-25 18:43       ` Albert Herranz
2009-11-27 12:07         ` Segher Boessenkool
2009-11-26  4:27       ` Benjamin Herrenschmidt
2009-11-26 22:19         ` Segher Boessenkool
2009-11-22 22:50   ` [RFC PATCH 01/19] powerpc: gamecube/wii: usbgecko bootwrapper console support Grant Likely
2009-11-26  4:12   ` Benjamin Herrenschmidt
2009-11-22 22:48 ` [RFC PATCH 00/19] powerpc: nintendo gamecube and wii support Grant Likely
2009-11-22 22:49 ` Arnd Bergmann
2009-11-23 19:22   ` Albert Herranz
2009-11-23  5:13 ` Wolfram Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B0AE603.9050208@yahoo.es \
    --to=albert_herranz@yahoo.es \
    --cc=grant.likely@secretlab.ca \
    --cc=linuxppc-dev@lists.ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).