Re: [RFC PATCH 18/19] powerpc: wii: platform support
+#ifdef CONFIG_STARLET_MINI
+
+#define HW_RESETS_OF_COMPATIBLEnintendo,hollywood-resets
+#define HW_GPIO_ALIAShw_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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 18/19] powerpc: wii: platform support
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_COMPATIBLEnintendo,hollywood-resets
+#define HW_GPIO_ALIAShw_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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 18/19] powerpc: wii: platform support
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_COMPATIBLEnintendo,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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 18/19] powerpc: wii: platform support
Grant Likely wrote:
On Sun, Nov 22, 2009 at 3:01 PM, Albert Herranz albert_herr...@yahoo.es
wrote:
Add platform support for the Nintendo Wii video game console.
Signed-off-by: Albert Herranz albert_herr...@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
Re: [RFC PATCH 18/19] powerpc: wii: platform support
On Sun, Nov 22, 2009 at 3:01 PM, Albert Herranz albert_herr...@yahoo.es wrote:
Add platform support for the Nintendo Wii video game console.
Signed-off-by: Albert Herranz albert_herr...@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) {
+
