Re: [RFC PATCH 18/19] powerpc: wii: platform support

[Segher Boessenkool <segher@kernel.crashing.org>]

> 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