[RFC,18/19] powerpc: wii: platform support

Add platform support for the Nintendo Wii video game console.

Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
---
 arch/powerpc/platforms/embedded6xx/Kconfig   |    8 +
 arch/powerpc/platforms/embedded6xx/Makefile  |    1 +
 arch/powerpc/platforms/embedded6xx/wii.c     |  270 ++++++++++++++++++++++++++
 arch/powerpc/platforms/embedded6xx/wii_dev.c |   47 +++++
 4 files changed, 326 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/platforms/embedded6xx/wii.c
 create mode 100644 arch/powerpc/platforms/embedded6xx/wii_dev.c

On Sun, Nov 22, 2009 at 3:01 PM, Albert Herranz <albert_herranz@yahoo.es> wrote:
> Add platform support for the Nintendo Wii video game console.
>
> Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
> ---
> +static int wii_setup_hw_resets(void)
> +{
> +       struct device_node *np;
> +       struct resource res;
> +       int error = -ENODEV;
> +
> +       np = of_find_compatible_node(NULL, NULL, HW_RESETS_OF_COMPATIBLE);
> +       if (!np) {
> +               pr_err("no compatible node found for %s\n",
> +                      HW_RESETS_OF_COMPATIBLE);
> +               goto out;
> +       }
> +       error = of_address_to_resource(np, 0, &res);
> +       if (error) {
> +               pr_err("no valid reg found for %s\n", np->name);
> +               goto out_put;
> +       }
> +
> +       hw_resets = ioremap(res.start, res.end - res.start + 1);

Or you could use of_iomap() to cut out some code.

> +       if (hw_resets) {
> +               pr_info("hw_resets at 0x%08x mapped to 0x%p\n",
> +                       res.start, hw_resets);
> +       }
> +
> +out_put:
> +       of_node_put(np);
> +out:
> +       return error;
> +}
> +
> +static int wii_setup_hw_gpio(void)
> +{
> +       struct device_node *np;
> +       struct resource res;
> +       const char *path;
> +       int error = -ENODEV;
> +
> +       np = of_find_node_by_name(NULL, "aliases");
> +       if (!np) {
> +               pr_err("unable to find node %s\n", "aliases");
> +               goto out;
> +       }
> +
> +       path = of_get_property(np, HW_GPIO_ALIAS, NULL);
> +       of_node_put(np);
> +       if (!path) {
> +               pr_err("alias %s unknown\n", HW_GPIO_ALIAS);
> +               goto out;
> +       }
> +
> +       np = of_find_node_by_path(path);
> +       if (!np) {
> +               pr_err("node for alias %s unknown\n", HW_GPIO_ALIAS);
> +               goto out;
> +       }
> +       error = of_address_to_resource(np, 0, &res);
> +       if (error) {
> +               pr_err("no valid reg found for %s\n", np->name);
> +               goto out_put;
> +       }
> +
> +       hw_gpio = ioremap(res.start, res.end - res.start + 1);
> +       if (hw_gpio) {
> +               pr_info("hw_gpio at 0x%08x mapped to 0x%p\n",
> +                       res.start, hw_gpio);
> +       }
> +
> +out_put:
> +       of_node_put(np);
> +out:
> +       return error;
> +}
> +
> +static void wii_setup(void)
> +{
> +       wii_setup_hw_resets();
> +       wii_setup_hw_gpio();
> +}
> +
> +static void wii_restart(char *cmd)
> +{
> +       local_irq_disable();
> +
> +       if (hw_resets) {
> +               /* clear the system reset pin to cause a reset */
> +               clear_bit(0, hw_resets);
> +       }
> +       wii_spin();
> +}
> +
> +static void wii_power_off(void)
> +{
> +       local_irq_disable();
> +
> +       if (hw_gpio) {
> +               /* make sure that the poweroff GPIO is configured as output */
> +               out_be32(hw_gpio + HW_GPIO_DIR,
> +                        in_be32(hw_gpio + HW_GPIO_DIR) | HW_GPIO_SHUTDOWN);
> +
> +               /* drive the poweroff GPIO high */
> +               out_be32(hw_gpio + HW_GPIO_OUT,
> +                        in_be32(hw_gpio + HW_GPIO_OUT) | HW_GPIO_SHUTDOWN);
> +       }
> +       wii_spin();
> +}
> +
> +#else
> +
> +static void wii_setup(void)
> +{
> +}
> +
> +static void wii_restart(char *cmd)
> +{
> +       wii_spin();
> +}
> +
> +static void wii_power_off(void)
> +{
> +       wii_spin();
> +}
> +
> +#endif /* CONFIG_STARLET_MINI */
> +
> +static void wii_halt(void)
> +{
> +       if (ppc_md.restart)
> +               ppc_md.restart(NULL);
> +       wii_spin();
> +}
> +
> +static void wii_show_cpuinfo(struct seq_file *m)
> +{
> +       seq_printf(m, "vendor\t\t: IBM\n");
> +       seq_printf(m, "machine\t\t: Nintendo Wii\n");
> +}

Drop show_cpuinfo() hook.

> +static int wii_discover_ipc_flavour(void)
> +{
> +       struct mipc_infohdr *hdrp;
> +       int error;
> +
> +       error = mipc_infohdr_get(&hdrp);
> +       if (!error) {
> +               mipc_infohdr_put(hdrp);
> +               starlet_ipc_flavour = STARLET_IPC_MINI;
> +               wii_setup();
> +               ppc_md.restart = wii_restart;
> +               ppc_md.power_off = wii_power_off;
> +       }
> +
> +       return 0;
> +}
> +
> +static void __init wii_setup_arch(void)
> +{
> +       ug_udbg_init();
> +       wii_discover_ipc_flavour();
> +}
> +
> +static void __init wii_init_early(void)
> +{
> +}
> +
> +static int __init wii_probe(void)
> +{
> +       unsigned long dt_root;
> +
> +       dt_root = of_get_flat_dt_root();
> +       if (!of_flat_dt_is_compatible(dt_root, "nintendo,wii"))
> +               return 0;
> +
> +       return 1;
> +}
> +
> +static void wii_shutdown(void)
> +{
> +       flipper_quiesce();
> +}
> +
> +#ifdef CONFIG_KEXEC
> +static int wii_machine_kexec_prepare(struct kimage *image)
> +{
> +       return 0;
> +}
> +
> +static void wii_machine_kexec(struct kimage *image)
> +{
> +       default_machine_kexec(image);
> +}

Drop unnecessary hooks.  If no kexec hook it offered, then
default_machine_kexec() gets called anyway.

> +#endif /* CONFIG_KEXEC */

> +
> +define_machine(wii) {
> +       .name                   = "wii",
> +       .probe                  = wii_probe,
> +       .setup_arch             = wii_setup_arch,
> +       .init_early             = wii_init_early,
> +       .show_cpuinfo           = wii_show_cpuinfo,
> +       .halt                   = wii_halt,
> +       .init_IRQ               = flipper_pic_probe,
> +       .get_irq                = flipper_pic_get_irq,
> +       .calibrate_decr         = generic_calibrate_decr,
> +       .progress               = udbg_progress,
> +       .machine_shutdown       = wii_shutdown,
> +#ifdef CONFIG_KEXEC
> +       .machine_kexec_prepare  = wii_machine_kexec_prepare,
> +       .machine_kexec          = wii_machine_kexec,
> +#endif
> +};
> +
> diff --git a/arch/powerpc/platforms/embedded6xx/wii_dev.c b/arch/powerpc/platforms/embedded6xx/wii_dev.c
> new file mode 100644
> index 0000000..903063e
> --- /dev/null
> +++ b/arch/powerpc/platforms/embedded6xx/wii_dev.c
> @@ -0,0 +1,47 @@
> +/*
> + * arch/powerpc/platforms/embedded6xx/wii_dev.c
> + *
> + * Nintendo Wii platform device setup.
> + * Copyright (C) 2008-2009 The GameCube Linux Team
> + * Copyright (C) 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.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/of_platform.h>
> +
> +#include <asm/machdep.h>
> +
> +static struct of_device_id wii_of_bus[] = {
> +       { .compatible = "nintendo,hollywood", },
> +#ifdef CONFIG_STARLET_MINI
> +       { .compatible = "twiizers,starlet-mini-ipc", },
> +#endif
> +       { },
> +};
> +
> +static int __init wii_device_probe(void)
> +{
> +       struct device_node *np;
> +
> +       if (!machine_is(wii))
> +               return 0;
> +
> +       of_platform_bus_probe(NULL, wii_of_bus, NULL);
> +
> +       np = of_find_compatible_node(NULL, NULL, "nintendo,hollywood-mem2");
> +       if (np) {
> +               of_platform_device_create(np, NULL, NULL);
> +               of_node_put(np);
> +       }
> +
> +       return 0;
> +}
> +device_initcall(wii_device_probe);

Why is this split into a separate file?  (Same comment goes for the
Gamecube version).  Just roll all the platform support into a single
file since there is no shared code.

g.

Grant Likely wrote:
> On Sun, Nov 22, 2009 at 3:01 PM, Albert Herranz <albert_herranz@yahoo.es> wrote:
>> Add platform support for the Nintendo Wii video game console.
>>
>> Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
>> ---
>> +static int wii_setup_hw_resets(void)
>> +{
>> +       struct device_node *np;
>> +       struct resource res;
>> +       int error = -ENODEV;
>> +
>> +       np = of_find_compatible_node(NULL, NULL, HW_RESETS_OF_COMPATIBLE);
>> +       if (!np) {
>> +               pr_err("no compatible node found for %s\n",
>> +                      HW_RESETS_OF_COMPATIBLE);
>> +               goto out;
>> +       }
>> +       error = of_address_to_resource(np, 0, &res);
>> +       if (error) {
>> +               pr_err("no valid reg found for %s\n", np->name);
>> +               goto out_put;
>> +       }
>> +
>> +       hw_resets = ioremap(res.start, res.end - res.start + 1);
> 
> Or you could use of_iomap() to cut out some code.
> 

Thanks. I'll have a look at it.

>> +       if (hw_resets) {
>> +               pr_info("hw_resets at 0x%08x mapped to 0x%p\n",
>> +                       res.start, hw_resets);
>> +       }
>> +
>> +out_put:
>> +       of_node_put(np);
>> +out:
>> +       return error;
>> +}
>> +
>> +static int wii_setup_hw_gpio(void)
>> +{
>> +       struct device_node *np;
>> +       struct resource res;
>> +       const char *path;
>> +       int error = -ENODEV;
>> +
>> +       np = of_find_node_by_name(NULL, "aliases");
>> +       if (!np) {
>> +               pr_err("unable to find node %s\n", "aliases");
>> +               goto out;
>> +       }
>> +
>> +       path = of_get_property(np, HW_GPIO_ALIAS, NULL);
>> +       of_node_put(np);
>> +       if (!path) {
>> +               pr_err("alias %s unknown\n", HW_GPIO_ALIAS);
>> +               goto out;
>> +       }
>> +
>> +       np = of_find_node_by_path(path);
>> +       if (!np) {
>> +               pr_err("node for alias %s unknown\n", HW_GPIO_ALIAS);
>> +               goto out;
>> +       }
>> +       error = of_address_to_resource(np, 0, &res);
>> +       if (error) {
>> +               pr_err("no valid reg found for %s\n", np->name);
>> +               goto out_put;
>> +       }
>> +
>> +       hw_gpio = ioremap(res.start, res.end - res.start + 1);
>> +       if (hw_gpio) {
>> +               pr_info("hw_gpio at 0x%08x mapped to 0x%p\n",
>> +                       res.start, hw_gpio);
>> +       }
>> +
>> +out_put:
>> +       of_node_put(np);
>> +out:
>> +       return error;
>> +}
>> +
>> +static void wii_setup(void)
>> +{
>> +       wii_setup_hw_resets();
>> +       wii_setup_hw_gpio();
>> +}
>> +
>> +static void wii_restart(char *cmd)
>> +{
>> +       local_irq_disable();
>> +
>> +       if (hw_resets) {
>> +               /* clear the system reset pin to cause a reset */
>> +               clear_bit(0, hw_resets);
>> +       }
>> +       wii_spin();
>> +}
>> +
>> +static void wii_power_off(void)
>> +{
>> +       local_irq_disable();
>> +
>> +       if (hw_gpio) {
>> +               /* make sure that the poweroff GPIO is configured as output */
>> +               out_be32(hw_gpio + HW_GPIO_DIR,
>> +                        in_be32(hw_gpio + HW_GPIO_DIR) | HW_GPIO_SHUTDOWN);
>> +
>> +               /* drive the poweroff GPIO high */
>> +               out_be32(hw_gpio + HW_GPIO_OUT,
>> +                        in_be32(hw_gpio + HW_GPIO_OUT) | HW_GPIO_SHUTDOWN);
>> +       }
>> +       wii_spin();
>> +}
>> +
>> +#else
>> +
>> +static void wii_setup(void)
>> +{
>> +}
>> +
>> +static void wii_restart(char *cmd)
>> +{
>> +       wii_spin();
>> +}
>> +
>> +static void wii_power_off(void)
>> +{
>> +       wii_spin();
>> +}
>> +
>> +#endif /* CONFIG_STARLET_MINI */
>> +
>> +static void wii_halt(void)
>> +{
>> +       if (ppc_md.restart)
>> +               ppc_md.restart(NULL);
>> +       wii_spin();
>> +}
>> +
>> +static void wii_show_cpuinfo(struct seq_file *m)
>> +{
>> +       seq_printf(m, "vendor\t\t: IBM\n");
>> +       seq_printf(m, "machine\t\t: Nintendo Wii\n");
>> +}
> 
> Drop show_cpuinfo() hook.
> 

Yup.

>> +static int wii_discover_ipc_flavour(void)
>> +{
>> +       struct mipc_infohdr *hdrp;
>> +       int error;
>> +
>> +       error = mipc_infohdr_get(&hdrp);
>> +       if (!error) {
>> +               mipc_infohdr_put(hdrp);
>> +               starlet_ipc_flavour = STARLET_IPC_MINI;
>> +               wii_setup();
>> +               ppc_md.restart = wii_restart;
>> +               ppc_md.power_off = wii_power_off;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static void __init wii_setup_arch(void)
>> +{
>> +       ug_udbg_init();
>> +       wii_discover_ipc_flavour();
>> +}
>> +
>> +static void __init wii_init_early(void)
>> +{
>> +}
>> +
>> +static int __init wii_probe(void)
>> +{
>> +       unsigned long dt_root;
>> +
>> +       dt_root = of_get_flat_dt_root();
>> +       if (!of_flat_dt_is_compatible(dt_root, "nintendo,wii"))
>> +               return 0;
>> +
>> +       return 1;
>> +}
>> +
>> +static void wii_shutdown(void)
>> +{
>> +       flipper_quiesce();
>> +}
>> +
>> +#ifdef CONFIG_KEXEC
>> +static int wii_machine_kexec_prepare(struct kimage *image)
>> +{
>> +       return 0;
>> +}
>> +
>> +static void wii_machine_kexec(struct kimage *image)
>> +{
>> +       default_machine_kexec(image);
>> +}
> 
> Drop unnecessary hooks.  If no kexec hook it offered, then
> default_machine_kexec() gets called anyway.
> 

Yes, at this stage I can get rid of those functions and add them later when they get actual code.
Thanks.

>> +#endif /* CONFIG_KEXEC */
> 
>> +
>> +define_machine(wii) {
>> +       .name                   = "wii",
>> +       .probe                  = wii_probe,
>> +       .setup_arch             = wii_setup_arch,
>> +       .init_early             = wii_init_early,
>> +       .show_cpuinfo           = wii_show_cpuinfo,
>> +       .halt                   = wii_halt,
>> +       .init_IRQ               = flipper_pic_probe,
>> +       .get_irq                = flipper_pic_get_irq,
>> +       .calibrate_decr         = generic_calibrate_decr,
>> +       .progress               = udbg_progress,
>> +       .machine_shutdown       = wii_shutdown,
>> +#ifdef CONFIG_KEXEC
>> +       .machine_kexec_prepare  = wii_machine_kexec_prepare,
>> +       .machine_kexec          = wii_machine_kexec,
>> +#endif
>> +};
>> +
>> diff --git a/arch/powerpc/platforms/embedded6xx/wii_dev.c b/arch/powerpc/platforms/embedded6xx/wii_dev.c
>> new file mode 100644
>> index 0000000..903063e
>> --- /dev/null
>> +++ b/arch/powerpc/platforms/embedded6xx/wii_dev.c
>> @@ -0,0 +1,47 @@
>> +/*
>> + * arch/powerpc/platforms/embedded6xx/wii_dev.c
>> + *
>> + * Nintendo Wii platform device setup.
>> + * Copyright (C) 2008-2009 The GameCube Linux Team
>> + * Copyright (C) 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.
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/init.h>
>> +#include <linux/of_platform.h>
>> +
>> +#include <asm/machdep.h>
>> +
>> +static struct of_device_id wii_of_bus[] = {
>> +       { .compatible = "nintendo,hollywood", },
>> +#ifdef CONFIG_STARLET_MINI
>> +       { .compatible = "twiizers,starlet-mini-ipc", },
>> +#endif
>> +       { },
>> +};
>> +
>> +static int __init wii_device_probe(void)
>> +{
>> +       struct device_node *np;
>> +
>> +       if (!machine_is(wii))
>> +               return 0;
>> +
>> +       of_platform_bus_probe(NULL, wii_of_bus, NULL);
>> +
>> +       np = of_find_compatible_node(NULL, NULL, "nintendo,hollywood-mem2");
>> +       if (np) {
>> +               of_platform_device_create(np, NULL, NULL);
>> +               of_node_put(np);
>> +       }
>> +
>> +       return 0;
>> +}
>> +device_initcall(wii_device_probe);
> 
> Why is this split into a separate file?  (Same comment goes for the
> Gamecube version).  Just roll all the platform support into a single
> file since there is no shared code.
> 

I'll do that too. Thanks.

Cheers,
Albert

> diff --git a/arch/powerpc/platforms/embedded6xx/wii.c b/arch/ 
> powerpc/platforms/embedded6xx/wii.c

> +#define DRV_MODULE_NAME "rvl"

Should this be "wii"?

> +static enum starlet_ipc_flavour starlet_ipc_flavour;
> +
> +enum starlet_ipc_flavour starlet_get_ipc_flavour(void)
> +{
> +	return starlet_ipc_flavour;
> +}

This can go I think, unless you plan on supporting something else  
than mini?

> +#ifdef CONFIG_STARLET_MINI
> +
> +#define HW_RESETS_OF_COMPATIBLE	"nintendo,hollywood-resets"
> +#define HW_GPIO_ALIAS		"hw_gpio

This should be unconditional now I think?  You access the hardware  
directly.

> +	np = of_find_node_by_name(NULL, "aliases");
> +	if (!np) {
> +		pr_err("unable to find node %s\n", "aliases");
> +		goto out;
> +	}
> +
> +	path = of_get_property(np, HW_GPIO_ALIAS, NULL);
> +	of_node_put(np);
> +	if (!path) {
> +		pr_err("alias %s unknown\n", HW_GPIO_ALIAS);
> +		goto out;
> +	}

Don't use an alias here, search for e.g. a matching "compatible"  
instead.

> +static struct of_device_id wii_of_bus[] = {
> +	{ .compatible = "nintendo,hollywood", },

Like with Flipper, why a platform bus?

> +#ifdef CONFIG_STARLET_MINI
> +	{ .compatible = "twiizers,starlet-mini-ipc", },
> +#endif

This one should go completely, I will have more to say about it when  
I get
to review the device trees (saving those for last :-) ).


Segher

Segher Boessenkool wrote:
>> diff --git a/arch/powerpc/platforms/embedded6xx/wii.c
>> b/arch/powerpc/platforms/embedded6xx/wii.c
> 
>> +#define DRV_MODULE_NAME "rvl"
> 
> Should this be "wii"?
> 

Ok. I'm fine with both.

>> +static enum starlet_ipc_flavour starlet_ipc_flavour;
>> +
>> +enum starlet_ipc_flavour starlet_get_ipc_flavour(void)
>> +{
>> +    return starlet_ipc_flavour;
>> +}
> 
> This can go I think, unless you plan on supporting something else than
> mini?
> 

See comment in previous message.
Irrespective of supporting other firmwares, we need to know the one we are running under.

>> +#ifdef CONFIG_STARLET_MINI
>> +
>> +#define HW_RESETS_OF_COMPATIBLE    "nintendo,hollywood-resets"
>> +#define HW_GPIO_ALIAS        "hw_gpio
> 
> This should be unconditional now I think?  You access the hardware
> directly.
> 

Yes, at this stage direct hardware should be possible, but only if 'mini' support is compiled-in (which will be the default option at this stage).
We can either leave the conditionals as is now, or remove them and add them later if we support more than one firmware flavour.

I'm fine with both options.

>> +    np = of_find_node_by_name(NULL, "aliases");
>> +    if (!np) {
>> +        pr_err("unable to find node %s\n", "aliases");
>> +        goto out;
>> +    }
>> +
>> +    path = of_get_property(np, HW_GPIO_ALIAS, NULL);
>> +    of_node_put(np);
>> +    if (!path) {
>> +        pr_err("alias %s unknown\n", HW_GPIO_ALIAS);
>> +        goto out;
>> +    }
> 
> Don't use an alias here, search for e.g. a matching "compatible" instead.
> 

I used an alias because I wanted explicitly the second GPIO word.

Is there another way to select a specific instance of repeated identical hardware?
We have two instances of gpios here, matching the same "compatible".

>> +static struct of_device_id wii_of_bus[] = {
>> +    { .compatible = "nintendo,hollywood", },
> 
> Like with Flipper, why a platform bus?
> 

See previous message.

>> +#ifdef CONFIG_STARLET_MINI
>> +    { .compatible = "twiizers,starlet-mini-ipc", },
>> +#endif
> 
> This one should go completely, I will have more to say about it when I get
> to review the device trees (saving those for last :-) ).
> 

Yes, this is not needed. I'll get rid of it.

Thanks a lot!
Albert

>>> +#ifdef CONFIG_STARLET_MINI
>>> +
>>> +#define HW_RESETS_OF_COMPATIBLE    "nintendo,hollywood-resets"
>>> +#define HW_GPIO_ALIAS        "hw_gpio
>>
>> This should be unconditional now I think?  You access the hardware
>> directly.
>
> Yes, at this stage direct hardware should be possible, but only if  
> 'mini' support is compiled-in (which will be the default option at  
> this stage).
> We can either leave the conditionals as is now, or remove them and  
> add them later if we support more than one firmware flavour.
>
> I'm fine with both options.

Removing all superfluous code is best in my opinion.

>>> +    np = of_find_node_by_name(NULL, "aliases");
>>> +    if (!np) {
>>> +        pr_err("unable to find node %s\n", "aliases");
>>> +        goto out;
>>> +    }
>>> +
>>> +    path = of_get_property(np, HW_GPIO_ALIAS, NULL);
>>> +    of_node_put(np);
>>> +    if (!path) {
>>> +        pr_err("alias %s unknown\n", HW_GPIO_ALIAS);
>>> +        goto out;
>>> +    }
>>
>> Don't use an alias here, search for e.g. a matching "compatible"  
>> instead.
>
> I used an alias because I wanted explicitly the second GPIO word.
>
> Is there another way to select a specific instance of repeated  
> identical hardware?

Yes, probe for it with its address.  There is no guarantee in which  
order
you will get them.

> We have two instances of gpios here, matching the same "compatible".

It's a pain in the behind that the GPIO binding will not allow you to
show the two GPIO regs as one controller, which it is.  Or can it
actually do that?


Segher