* Re: [PATCH v2 3/3]CPU DLPAR handling
From: Paul Mackerras @ 2009-11-26 2:59 UTC (permalink / raw)
To: Nathan Fontenot; +Cc: linuxppc-dev, gregkh, linux-kernel
In-Reply-To: <4B0CD91C.1090704@austin.ibm.com>
Nathan Fontenot writes:
> This patch adds the specific routines to probe and release (add and remove)
> cpu resource for the powerpc pseries platform and registers these handlers
> with the ppc_md callout structure.
>
> Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>
Acked-by: Paul Mackerras <paulus@samba.org>
^ permalink raw reply
* Re: [PATCH v3 2/3] sysfs cpu probe/release files
From: Nathan Fontenot @ 2009-11-26 3:23 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel; +Cc: gregkh, paul Mackerras
In-Reply-To: <4B0CD8D5.8050803@austin.ibm.com>
Version 3 of this patch is updated with documentation added to
Documentation/ABI. There are no changes to any of the C code from v2
of the patch.
In order to support kernel DLPAR of CPU resources we need to provide an
interface to add (probe) and remove (release) the resource from the system.
This patch Creates new generic probe and release sysfs files to facilitate
cpu probe/release. The probe/release interface provides for allowing each
arch to supply their own routines for implementing the backend of adding
and removing cpus to/from the system.
This also creates the powerpc specific stubs to handle the arch callouts
from writes to the sysfs files.
The creation and use of these files is regulated by the
CONFIG_ARCH_CPU_PROBE_RELEASE option so that only architectures that need the
capability will have the files created.
Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>
---
Documentation/ABI/testing/sysfs-devices-system-cpu | 15 +++++++++
arch/powerpc/Kconfig | 4 ++
arch/powerpc/include/asm/machdep.h | 5 +++
arch/powerpc/kernel/sysfs.c | 19 ++++++++++++
drivers/base/cpu.c | 32 +++++++++++++++++++++
include/linux/cpu.h | 2 +
6 files changed, 77 insertions(+)
Index: powerpc/drivers/base/cpu.c
===================================================================
--- powerpc.orig/drivers/base/cpu.c 2009-11-25 04:52:37.000000000 -0600
+++ powerpc/drivers/base/cpu.c 2009-11-25 04:54:25.000000000 -0600
@@ -72,6 +72,38 @@
per_cpu(cpu_sys_devices, logical_cpu) = NULL;
return;
}
+
+#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
+static ssize_t cpu_probe_store(struct class *class, const char *buf,
+ size_t count)
+{
+ return arch_cpu_probe(buf, count);
+}
+
+static ssize_t cpu_release_store(struct class *class, const char *buf,
+ size_t count)
+{
+ return arch_cpu_release(buf, count);
+}
+
+static CLASS_ATTR(probe, S_IWUSR, NULL, cpu_probe_store);
+static CLASS_ATTR(release, S_IWUSR, NULL, cpu_release_store);
+
+int __init cpu_probe_release_init(void)
+{
+ int rc;
+
+ rc = sysfs_create_file(&cpu_sysdev_class.kset.kobj,
+ &class_attr_probe.attr);
+ if (!rc)
+ rc = sysfs_create_file(&cpu_sysdev_class.kset.kobj,
+ &class_attr_release.attr);
+
+ return rc;
+}
+device_initcall(cpu_probe_release_init);
+#endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
+
#else /* ... !CONFIG_HOTPLUG_CPU */
static inline void register_cpu_control(struct cpu *cpu)
{
Index: powerpc/arch/powerpc/include/asm/machdep.h
===================================================================
--- powerpc.orig/arch/powerpc/include/asm/machdep.h 2009-11-25 04:52:37.000000000 -0600
+++ powerpc/arch/powerpc/include/asm/machdep.h 2009-11-25 04:54:25.000000000 -0600
@@ -266,6 +266,11 @@
void (*suspend_disable_irqs)(void);
void (*suspend_enable_irqs)(void);
#endif
+
+#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
+ ssize_t (*cpu_probe)(const char *, size_t);
+ ssize_t (*cpu_release)(const char *, size_t);
+#endif
};
extern void e500_idle(void);
Index: powerpc/arch/powerpc/kernel/sysfs.c
===================================================================
--- powerpc.orig/arch/powerpc/kernel/sysfs.c 2009-11-25 04:52:37.000000000 -0600
+++ powerpc/arch/powerpc/kernel/sysfs.c 2009-11-25 04:54:25.000000000 -0600
@@ -461,6 +461,25 @@
cacheinfo_cpu_offline(cpu);
}
+
+#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
+ssize_t arch_cpu_probe(const char *buf, size_t count)
+{
+ if (ppc_md.cpu_probe)
+ return ppc_md.cpu_probe(buf, count);
+
+ return -EINVAL;
+}
+
+ssize_t arch_cpu_release(const char *buf, size_t count)
+{
+ if (ppc_md.cpu_release)
+ return ppc_md.cpu_release(buf, count);
+
+ return -EINVAL;
+}
+#endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
+
#endif /* CONFIG_HOTPLUG_CPU */
static int __cpuinit sysfs_cpu_notify(struct notifier_block *self,
Index: powerpc/arch/powerpc/Kconfig
===================================================================
--- powerpc.orig/arch/powerpc/Kconfig 2009-11-25 04:52:37.000000000 -0600
+++ powerpc/arch/powerpc/Kconfig 2009-11-25 04:54:25.000000000 -0600
@@ -320,6 +320,10 @@
Say N if you are unsure.
+config ARCH_CPU_PROBE_RELEASE
+ def_bool y
+ depends on HOTPLUG_CPU
+
config ARCH_ENABLE_MEMORY_HOTPLUG
def_bool y
Index: powerpc/include/linux/cpu.h
===================================================================
--- powerpc.orig/include/linux/cpu.h 2009-11-25 04:52:37.000000000 -0600
+++ powerpc/include/linux/cpu.h 2009-11-25 04:54:25.000000000 -0600
@@ -43,6 +43,8 @@
#ifdef CONFIG_HOTPLUG_CPU
extern void unregister_cpu(struct cpu *cpu);
+extern ssize_t arch_cpu_probe(const char *, size_t);
+extern ssize_t arch_cpu_release(const char *, size_t);
#endif
struct notifier_block;
Index: powerpc/Documentation/ABI/testing/sysfs-devices-system-cpu
===================================================================
--- powerpc.orig/Documentation/ABI/testing/sysfs-devices-system-cpu 2009-11-20 17:53:51.000000000 -0600
+++ powerpc/Documentation/ABI/testing/sysfs-devices-system-cpu 2009-11-26 01:29:25.000000000 -0600
@@ -62,6 +62,21 @@
See Documentation/cputopology.txt for more information.
+What: /sys/devices/system/cpu/probe
+ /sys/devices/system/cpu/release
+Date: November 2009
+Contact: Linux kernel mailing list <linux-kernel@vger.kernel.org>
+Description: Dynamic addition and removal of CPU's. This is not hotplug
+ removal, this is meant complete removal/addition of the CPU
+ from the system.
+
+ probe: writes to this file will dynamically add a CPU to the
+ system. Information written to the file to add CPU's is
+ architecture specific.
+
+ release: writes to this file dynamically remove a CPU from
+ the system. Information writtento the file to remove CPU's
+ is architecture specific.
What: /sys/devices/system/cpu/cpu#/node
Date: October 2009
^ permalink raw reply
* Re: hypervisor call tracepoints hcall_stats touchup.
From: Anton Blanchard @ 2009-11-26 3:28 UTC (permalink / raw)
To: Will Schmidt; +Cc: linuxppc-dev, Frederic Weisbecker, mingo, Steven Rostedt
In-Reply-To: <1259165529.16349.191.camel@lexx>
Hi Will,
> The tb_total and purr_total values reported via the hcall_stats code
> should be cumulative, rather than being replaced by the latest delta tb
> or purr value.
>
> Tested-by: Will Schmidt <will_schmidt@vnet.ibm.com>
> Signed-off-by: Will Schmidt <will_schmidt@vnet.ibm.com>
Ouch! Nice catch.
Acked-by: Anton Blanchard <anton@samba.org>
Anton
> [ This is a touch-up to the "[3/6] powerpc: tracing: Add hypervisor call
> tracepoints" patch submitted by Anton a few weeks back, so I've copied
> folks Anton had on CC for his original patch, this fix is rather ppc
> specific, so can probably go in via the ppc tree, but I've no real
> preference. ]
>
> diff --git a/arch/powerpc/platforms/pseries/hvCall_inst.c b/arch/powerpc/platforms/pseries/hvCall_inst.c
> index 2f58c71..1fefae7 100644
> --- a/arch/powerpc/platforms/pseries/hvCall_inst.c
> +++ b/arch/powerpc/platforms/pseries/hvCall_inst.c
> @@ -124,8 +124,8 @@ static void probe_hcall_exit(unsigned long opcode, unsigned long retval,
>
> h = &__get_cpu_var(hcall_stats)[opcode / 4];
> h->num_calls++;
> - h->tb_total = mftb() - h->tb_start;
> - h->purr_total = mfspr(SPRN_PURR) - h->purr_start;
> + h->tb_total += mftb() - h->tb_start;
> + h->purr_total += mfspr(SPRN_PURR) - h->purr_start;
>
> put_cpu_var(hcall_stats);
> }
>
^ permalink raw reply
* Re: [PATCH 01/11] of/flattree: Merge early_init_dt_check_for_initrd()
From: Benjamin Herrenschmidt @ 2009-11-26 3:51 UTC (permalink / raw)
To: Grant Likely
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, davem
In-Reply-To: <20091124081747.6216.88376.stgit@angua>
On Tue, 2009-11-24 at 01:17 -0700, Grant Likely wrote:
> Merge common code between PowerPC and Microblaze
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
> Tested-by: Michal Simek <monstr@monstr.eu>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Remind me how you want to merge that ? via my tree ?
Cheers,
Ben.
> ---
>
> arch/microblaze/kernel/prom.c | 32 --------------------------------
> arch/powerpc/kernel/prom.c | 30 ------------------------------
> drivers/of/fdt.c | 37 +++++++++++++++++++++++++++++++++++++
> include/linux/of_fdt.h | 1 +
> 4 files changed, 38 insertions(+), 62 deletions(-)
>
> diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
> index a38e373..7959495 100644
> --- a/arch/microblaze/kernel/prom.c
> +++ b/arch/microblaze/kernel/prom.c
> @@ -113,38 +113,6 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
> return 0;
> }
>
> -#ifdef CONFIG_BLK_DEV_INITRD
> -static void __init early_init_dt_check_for_initrd(unsigned long node)
> -{
> - unsigned long l;
> - u32 *prop;
> -
> - pr_debug("Looking for initrd properties... ");
> -
> - prop = of_get_flat_dt_prop(node, "linux,initrd-start", &l);
> - if (prop) {
> - initrd_start = (unsigned long)
> - __va((u32)of_read_ulong(prop, l/4));
> -
> - prop = of_get_flat_dt_prop(node, "linux,initrd-end", &l);
> - if (prop) {
> - initrd_end = (unsigned long)
> - __va((u32)of_read_ulong(prop, 1/4));
> - initrd_below_start_ok = 1;
> - } else {
> - initrd_start = 0;
> - }
> - }
> -
> - pr_debug("initrd_start=0x%lx initrd_end=0x%lx\n",
> - initrd_start, initrd_end);
> -}
> -#else
> -static inline void early_init_dt_check_for_initrd(unsigned long node)
> -{
> -}
> -#endif /* CONFIG_BLK_DEV_INITRD */
> -
> static int __init early_init_dt_scan_chosen(unsigned long node,
> const char *uname, int depth, void *data)
> {
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 7f88566..1ecd6c6 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -373,36 +373,6 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
> return 0;
> }
>
> -#ifdef CONFIG_BLK_DEV_INITRD
> -static void __init early_init_dt_check_for_initrd(unsigned long node)
> -{
> - unsigned long l;
> - u32 *prop;
> -
> - DBG("Looking for initrd properties... ");
> -
> - prop = of_get_flat_dt_prop(node, "linux,initrd-start", &l);
> - if (prop) {
> - initrd_start = (unsigned long)__va(of_read_ulong(prop, l/4));
> -
> - prop = of_get_flat_dt_prop(node, "linux,initrd-end", &l);
> - if (prop) {
> - initrd_end = (unsigned long)
> - __va(of_read_ulong(prop, l/4));
> - initrd_below_start_ok = 1;
> - } else {
> - initrd_start = 0;
> - }
> - }
> -
> - DBG("initrd_start=0x%lx initrd_end=0x%lx\n", initrd_start, initrd_end);
> -}
> -#else
> -static inline void early_init_dt_check_for_initrd(unsigned long node)
> -{
> -}
> -#endif /* CONFIG_BLK_DEV_INITRD */
> -
> static int __init early_init_dt_scan_chosen(unsigned long node,
> const char *uname, int depth, void *data)
> {
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 43d236c..6ad98e8 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -11,6 +11,7 @@
>
> #include <linux/kernel.h>
> #include <linux/lmb.h>
> +#include <linux/initrd.h>
> #include <linux/of.h>
> #include <linux/of_fdt.h>
>
> @@ -369,6 +370,42 @@ unsigned long __init unflatten_dt_node(unsigned long mem,
> return mem;
> }
>
> +#ifdef CONFIG_BLK_DEV_INITRD
> +/**
> + * early_init_dt_check_for_initrd - Decode initrd location from flat tree
> + * @node: reference to node containing initrd location ('chosen')
> + */
> +void __init early_init_dt_check_for_initrd(unsigned long node)
> +{
> + unsigned long len;
> + u32 *prop;
> +
> + pr_debug("Looking for initrd properties... ");
> +
> + prop = of_get_flat_dt_prop(node, "linux,initrd-start", &len);
> + if (prop) {
> + initrd_start = (unsigned long)
> + __va(of_read_ulong(prop, len/4));
> +
> + prop = of_get_flat_dt_prop(node, "linux,initrd-end", &len);
> + if (prop) {
> + initrd_end = (unsigned long)
> + __va(of_read_ulong(prop, len/4));
> + initrd_below_start_ok = 1;
> + } else {
> + initrd_start = 0;
> + }
> + }
> +
> + pr_debug("initrd_start=0x%lx initrd_end=0x%lx\n",
> + initrd_start, initrd_end);
> +}
> +#else
> +inline void early_init_dt_check_for_initrd(unsigned long node)
> +{
> +}
> +#endif /* CONFIG_BLK_DEV_INITRD */
> +
> /**
> * unflatten_device_tree - create tree of device_nodes from flat blob
> *
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 81231e0..ec2db82 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -69,6 +69,7 @@ extern void *of_get_flat_dt_prop(unsigned long node, const char *name,
> unsigned long *size);
> extern int of_flat_dt_is_compatible(unsigned long node, const char *name);
> extern unsigned long of_get_flat_dt_root(void);
> +extern void early_init_dt_check_for_initrd(unsigned long node);
>
> /* Other Prototypes */
> extern void finish_device_tree(void);
^ permalink raw reply
* Re: [PATCH 02/11] of/flattree: Merge earlyinit_dt_scan_root()
From: Benjamin Herrenschmidt @ 2009-11-26 3:54 UTC (permalink / raw)
To: Grant Likely
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, davem
In-Reply-To: <20091124081800.6216.27311.stgit@angua>
On Tue, 2009-11-24 at 01:18 -0700, Grant Likely wrote:
> Merge common code between PowerPC and Microblaze
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
Ok with that, like the precendent, however you are making global some
symbols that were previously static which sucks a bit... But then they
can be made static again I suppose once more has been merged.
Ben.
> arch/microblaze/kernel/prom.c | 23 -----------------------
> arch/powerpc/kernel/prom.c | 24 ------------------------
> drivers/of/fdt.c | 26 ++++++++++++++++++++++++++
> include/linux/of_fdt.h | 6 ++++++
> 4 files changed, 32 insertions(+), 47 deletions(-)
>
> diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
> index 7959495..189179a 100644
> --- a/arch/microblaze/kernel/prom.c
> +++ b/arch/microblaze/kernel/prom.c
> @@ -42,9 +42,6 @@
> #include <asm/sections.h>
> #include <asm/pci-bridge.h>
>
> -static int __initdata dt_root_addr_cells;
> -static int __initdata dt_root_size_cells;
> -
> typedef u32 cell_t;
>
> /* export that to outside world */
> @@ -158,26 +155,6 @@ static int __init early_init_dt_scan_chosen(unsigned long node,
> return 1;
> }
>
> -static int __init early_init_dt_scan_root(unsigned long node,
> - const char *uname, int depth, void *data)
> -{
> - u32 *prop;
> -
> - if (depth != 0)
> - return 0;
> -
> - prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> - dt_root_size_cells = (prop == NULL) ? 1 : *prop;
> - pr_debug("dt_root_size_cells = %x\n", dt_root_size_cells);
> -
> - prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> - dt_root_addr_cells = (prop == NULL) ? 2 : *prop;
> - pr_debug("dt_root_addr_cells = %x\n", dt_root_addr_cells);
> -
> - /* break now */
> - return 1;
> -}
> -
> static u64 __init dt_mem_next_cell(int s, cell_t **cellp)
> {
> cell_t *p = *cellp;
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 1ecd6c6..78f65a4 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -61,10 +61,6 @@
> #define DBG(fmt...)
> #endif
>
> -
> -static int __initdata dt_root_addr_cells;
> -static int __initdata dt_root_size_cells;
> -
> #ifdef CONFIG_PPC64
> int __initdata iommu_is_off;
> int __initdata iommu_force_on;
> @@ -436,26 +432,6 @@ static int __init early_init_dt_scan_chosen(unsigned long node,
> return 1;
> }
>
> -static int __init early_init_dt_scan_root(unsigned long node,
> - const char *uname, int depth, void *data)
> -{
> - u32 *prop;
> -
> - if (depth != 0)
> - return 0;
> -
> - prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> - dt_root_size_cells = (prop == NULL) ? 1 : *prop;
> - DBG("dt_root_size_cells = %x\n", dt_root_size_cells);
> -
> - prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> - dt_root_addr_cells = (prop == NULL) ? 2 : *prop;
> - DBG("dt_root_addr_cells = %x\n", dt_root_addr_cells);
> -
> - /* break now */
> - return 1;
> -}
> -
> static u64 __init dt_mem_next_cell(int s, cell_t **cellp)
> {
> cell_t *p = *cellp;
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 6ad98e8..be200be 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -15,6 +15,9 @@
> #include <linux/of.h>
> #include <linux/of_fdt.h>
>
> +int __initdata dt_root_addr_cells;
> +int __initdata dt_root_size_cells;
> +
> struct boot_param_header *initial_boot_params;
>
> char *find_flat_dt_string(u32 offset)
> @@ -407,6 +410,29 @@ inline void early_init_dt_check_for_initrd(unsigned long node)
> #endif /* CONFIG_BLK_DEV_INITRD */
>
> /**
> + * early_init_dt_scan_root - fetch the top level address and size cells
> + */
> +int __init early_init_dt_scan_root(unsigned long node, const char *uname,
> + int depth, void *data)
> +{
> + u32 *prop;
> +
> + if (depth != 0)
> + return 0;
> +
> + prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> + dt_root_size_cells = (prop == NULL) ? 1 : *prop;
> + pr_debug("dt_root_size_cells = %x\n", dt_root_size_cells);
> +
> + prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> + dt_root_addr_cells = (prop == NULL) ? 2 : *prop;
> + pr_debug("dt_root_addr_cells = %x\n", dt_root_addr_cells);
> +
> + /* break now */
> + return 1;
> +}
> +
> +/**
> * unflatten_device_tree - create tree of device_nodes from flat blob
> *
> * unflattens the device-tree passed by the firmware, creating the
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index ec2db82..828c3cd 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -58,6 +58,8 @@ struct boot_param_header {
> };
>
> /* TBD: Temporary export of fdt globals - remove when code fully merged */
> +extern int __initdata dt_root_addr_cells;
> +extern int __initdata dt_root_size_cells;
> extern struct boot_param_header *initial_boot_params;
>
> /* For scanning the flat device-tree at boot time */
> @@ -71,6 +73,10 @@ extern int of_flat_dt_is_compatible(unsigned long node, const char *name);
> extern unsigned long of_get_flat_dt_root(void);
> extern void early_init_dt_check_for_initrd(unsigned long node);
>
> +/* Early flat tree scan hooks */
> +extern int early_init_dt_scan_root(unsigned long node, const char *uname,
> + int depth, void *data);
> +
> /* Other Prototypes */
> extern void finish_device_tree(void);
> extern void unflatten_device_tree(void);
^ permalink raw reply
* Re: [PATCH 03/11] of/flattree: merge dt_mem_next_cell
From: Benjamin Herrenschmidt @ 2009-11-26 3:55 UTC (permalink / raw)
To: Grant Likely
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, davem
In-Reply-To: <20091124081814.6216.97169.stgit@angua>
On Tue, 2009-11-24 at 01:18 -0700, Grant Likely wrote:
> Merge common code between PowerPC and Microblaze
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>
> arch/microblaze/kernel/prom.c | 8 --------
> arch/powerpc/kernel/prom.c | 8 --------
> drivers/of/fdt.c | 8 ++++++++
> include/linux/of_fdt.h | 1 +
> 4 files changed, 9 insertions(+), 16 deletions(-)
Same comment here, something goes from static to global... do you plan
to make things back to static in the end ?
Ben.
> diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
> index 189179a..e0f4c34 100644
> --- a/arch/microblaze/kernel/prom.c
> +++ b/arch/microblaze/kernel/prom.c
> @@ -155,14 +155,6 @@ static int __init early_init_dt_scan_chosen(unsigned long node,
> return 1;
> }
>
> -static u64 __init dt_mem_next_cell(int s, cell_t **cellp)
> -{
> - cell_t *p = *cellp;
> -
> - *cellp = p + s;
> - return of_read_number(p, s);
> -}
> -
> static int __init early_init_dt_scan_memory(unsigned long node,
> const char *uname, int depth, void *data)
> {
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 78f65a4..048e3a3 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -432,14 +432,6 @@ static int __init early_init_dt_scan_chosen(unsigned long node,
> return 1;
> }
>
> -static u64 __init dt_mem_next_cell(int s, cell_t **cellp)
> -{
> - cell_t *p = *cellp;
> -
> - *cellp = p + s;
> - return of_read_number(p, s);
> -}
> -
> #ifdef CONFIG_PPC_PSERIES
> /*
> * Interpret the ibm,dynamic-memory property in the
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index be200be..ebce509 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -432,6 +432,14 @@ int __init early_init_dt_scan_root(unsigned long node, const char *uname,
> return 1;
> }
>
> +u64 __init dt_mem_next_cell(int s, u32 **cellp)
> +{
> + u32 *p = *cellp;
> +
> + *cellp = p + s;
> + return of_read_number(p, s);
> +}
> +
> /**
> * unflatten_device_tree - create tree of device_nodes from flat blob
> *
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 828c3cd..d1a37e5 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -72,6 +72,7 @@ extern void *of_get_flat_dt_prop(unsigned long node, const char *name,
> extern int of_flat_dt_is_compatible(unsigned long node, const char *name);
> extern unsigned long of_get_flat_dt_root(void);
> extern void early_init_dt_check_for_initrd(unsigned long node);
> +extern u64 dt_mem_next_cell(int s, u32 **cellp);
>
> /* Early flat tree scan hooks */
> extern int early_init_dt_scan_root(unsigned long node, const char *uname,
^ permalink raw reply
* Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef
From: Benjamin Herrenschmidt @ 2009-11-26 3:59 UTC (permalink / raw)
To: Grant Likely
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, davem
In-Reply-To: <20091124081827.6216.1896.stgit@angua>
On Tue, 2009-11-24 at 01:18 -0700, Grant Likely wrote:
> A cell is firmly established as a u32. No need to do an ugly typedef
> to redefine it to cell_t. Eliminate the unnecessary typedef so that
> it doesn't have to be added to the of_fdt header file
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
I'm not sure about that one. Yes, we do use u32 a lot and cell_t rarely,
so it would seem logical to switch.... On the other hand, we have that
pesky endianness issue we have never fully solved. So we need accessors
to sort that out, which means directly tapping things as u32 * is not a
good idea if we're going to enforce the use of such accessors.
I believe we should probably just enforce that properties are big endian
for flat device-trees. In which case we could just use __be32 or on of
thoes sparse-friendly types. I know x86 people won't like that much and
to be honest I don't know what 1295 specifies for real OFs but there
aren't enough real OFs around on LE machines for us to care much about
it, is there ?
The reason I prefer a fixed endianness is that allowing "LE" trees
becomes really nasty when a number is expressed using multiple cells.
That brings the question as to whether the two cells need to be flipped
as well or only the bytes within each cell. And that's the easy bit
(probably flip the whole thing). What about something like a PCI "reg"
property which is made of 3 cells, two of them forming a 64-bit address
and one containing additional data & attributes ? What is flipped and
where ?
So yes, cell_t might not be the right approach and by far to generic a
name, but u32 isn't the answer neither.
Cheers,
Ben.
> arch/microblaze/kernel/prom.c | 10 ++++------
> arch/powerpc/kernel/prom.c | 14 ++++++--------
> 2 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
> index e0f4c34..7760186 100644
> --- a/arch/microblaze/kernel/prom.c
> +++ b/arch/microblaze/kernel/prom.c
> @@ -42,8 +42,6 @@
> #include <asm/sections.h>
> #include <asm/pci-bridge.h>
>
> -typedef u32 cell_t;
> -
> /* export that to outside world */
> struct device_node *of_chosen;
>
> @@ -159,7 +157,7 @@ static int __init early_init_dt_scan_memory(unsigned long node,
> const char *uname, int depth, void *data)
> {
> char *type = of_get_flat_dt_prop(node, "device_type", NULL);
> - cell_t *reg, *endp;
> + u32 *reg, *endp;
> unsigned long l;
>
> /* Look for the ibm,dynamic-reconfiguration-memory node */
> @@ -178,13 +176,13 @@ static int __init early_init_dt_scan_memory(unsigned long node,
> } else if (strcmp(type, "memory") != 0)
> return 0;
>
> - reg = (cell_t *)of_get_flat_dt_prop(node, "linux,usable-memory", &l);
> + reg = (u32 *)of_get_flat_dt_prop(node, "linux,usable-memory", &l);
> if (reg == NULL)
> - reg = (cell_t *)of_get_flat_dt_prop(node, "reg", &l);
> + reg = (u32 *)of_get_flat_dt_prop(node, "reg", &l);
> if (reg == NULL)
> return 0;
>
> - endp = reg + (l / sizeof(cell_t));
> + endp = reg + (l / sizeof(u32));
>
> pr_debug("memory scan node %s, reg size %ld, data: %x %x %x %x,\n",
> uname, l, reg[0], reg[1], reg[2], reg[3]);
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 048e3a3..43cdba2 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -67,8 +67,6 @@ int __initdata iommu_force_on;
> unsigned long tce_alloc_start, tce_alloc_end;
> #endif
>
> -typedef u32 cell_t;
> -
> extern rwlock_t devtree_lock; /* temporary while merging */
>
> /* export that to outside world */
> @@ -441,22 +439,22 @@ static int __init early_init_dt_scan_chosen(unsigned long node,
> */
> static int __init early_init_dt_scan_drconf_memory(unsigned long node)
> {
> - cell_t *dm, *ls, *usm;
> + u32 *dm, *ls, *usm;
> unsigned long l, n, flags;
> u64 base, size, lmb_size;
> unsigned int is_kexec_kdump = 0, rngs;
>
> ls = of_get_flat_dt_prop(node, "ibm,lmb-size", &l);
> - if (ls == NULL || l < dt_root_size_cells * sizeof(cell_t))
> + if (ls == NULL || l < dt_root_size_cells * sizeof(u32))
> return 0;
> lmb_size = dt_mem_next_cell(dt_root_size_cells, &ls);
>
> dm = of_get_flat_dt_prop(node, "ibm,dynamic-memory", &l);
> - if (dm == NULL || l < sizeof(cell_t))
> + if (dm == NULL || l < sizeof(u32))
> return 0;
>
> n = *dm++; /* number of entries */
> - if (l < (n * (dt_root_addr_cells + 4) + 1) * sizeof(cell_t))
> + if (l < (n * (dt_root_addr_cells + 4) + 1) * sizeof(u32))
> return 0;
>
> /* check if this is a kexec/kdump kernel. */
> @@ -515,7 +513,7 @@ static int __init early_init_dt_scan_memory(unsigned long node,
> const char *uname, int depth, void *data)
> {
> char *type = of_get_flat_dt_prop(node, "device_type", NULL);
> - cell_t *reg, *endp;
> + u32 *reg, *endp;
> unsigned long l;
>
> /* Look for the ibm,dynamic-reconfiguration-memory node */
> @@ -540,7 +538,7 @@ static int __init early_init_dt_scan_memory(unsigned long node,
> if (reg == NULL)
> return 0;
>
> - endp = reg + (l / sizeof(cell_t));
> + endp = reg + (l / sizeof(u32));
>
> DBG("memory scan node %s, reg size %ld, data: %x %x %x %x,\n",
> uname, l, reg[0], reg[1], reg[2], reg[3]);
^ permalink raw reply
* Re: [PATCH 01/11] of/flattree: Merge early_init_dt_check_for_initrd()
From: Grant Likely @ 2009-11-26 4:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, davem
In-Reply-To: <1259207478.16367.218.camel@pasglop>
On Wed, Nov 25, 2009 at 8:51 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2009-11-24 at 01:17 -0700, Grant Likely wrote:
>> Merge common code between PowerPC and Microblaze
>>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
>> Tested-by: Michal Simek <monstr@monstr.eu>
>
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
> Remind me how you want to merge that ? via my tree ?
Since it is cross-arch, I'll ask Linus to pull directly. We'll see if
that takes.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 02/11] of/flattree: Merge earlyinit_dt_scan_root()
From: Grant Likely @ 2009-11-26 4:03 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, davem
In-Reply-To: <1259207655.16367.220.camel@pasglop>
On Wed, Nov 25, 2009 at 8:54 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2009-11-24 at 01:18 -0700, Grant Likely wrote:
>> Merge common code between PowerPC and Microblaze
>>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> ---
>
> Ok with that, like the precendent, however you are making global some
> symbols that were previously static which sucks a bit... But then they
> can be made static again I suppose once more has been merged.
exactly. My plan is: 1) merge. 2) refactor (and make things static
again) 3) port to ARM 4) ???? 5) profit.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 06/11] of/flattree: merge early_init_devtree() and early_init_move_devtree()
From: Benjamin Herrenschmidt @ 2009-11-26 4:04 UTC (permalink / raw)
To: Grant Likely
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, davem
In-Reply-To: <20091124081853.6216.68105.stgit@angua>
On Tue, 2009-11-24 at 01:19 -0700, Grant Likely wrote:
>
> -static int __init early_init_dt_scan_cpus(unsigned long node,
> - const char *uname, int depth,
> - void *data)
> +int __init early_init_dt_scan_cpus(unsigned long node, const char *uname,
> + int depth, void *data)
> {
So now you make this one non-static as well with little hope of making
it static ever again
> static int logical_cpuid;
> char *type = of_get_flat_dt_prop(node, "device_type", NULL);
> @@ -113,8 +112,8 @@ void __init early_init_dt_scan_chosen_arch(unsigned long node)
> /* No Microblaze specific code here */
> }
>
> -static int __init early_init_dt_scan_memory(unsigned long node,
> - const char *uname, int depth, void *data)
> +int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
> + int depth, void *data)
> {
And this one
> char *type = of_get_flat_dt_prop(node, "device_type", NULL);
> u32 *reg, *endp;
> @@ -201,7 +200,7 @@ static inline unsigned long phyp_dump_calculate_reserve_size(void)
> * without reserving anything. The memory in case of dump being
> * active is freed when the dump is collected (by userland tools).
> */
> -static void __init phyp_dump_reserve_mem(void)
> +void __init phyp_dump_reserve_mem(void)
> {
And this one...
> /**
> + * early_init_move_devtree - move tree to an unused area, if needed.
> + *
> + * The device tree may be allocated beyond our memory limit, or inside the
> + * crash kernel region for kdump. If so, move it out of the way.
> + */
> +#if defined(CONFIG_PPC)
> +static void __init early_init_move_devtree(void)
And you still end up with an ifdef mess in the common code ...
Would it be possible instead to have one common early_init_devtree()
that calls into the "common" ones (which you can then make static again,
inside the common code) and then calls one arch_early_init_devtree()
which regroups the arch specific ones ?
Or there's too many ordering issues ?
Another option then is to call from that early_init_devtree() something
like:
arch_early_init_dt_mem()
arch_early_init_dt_cpu()
arch_early_init_move_devtree()
etc... in the right spots in the common code and have the archs who
don't do anything there just have them as empty inlines.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 07/11] of: merge machine_is_compatible()
From: Benjamin Herrenschmidt @ 2009-11-26 4:05 UTC (permalink / raw)
To: Grant Likely
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, davem
In-Reply-To: <20091124081906.6216.67035.stgit@angua>
On Tue, 2009-11-24 at 01:19 -0700, Grant Likely wrote:
> Merge common code between PowerPC and Microblaze
I don't like moving this one to common code without the of_ prefix. I
think you should move it with the of_ prefix, and then add a alias in
powerpc and microblaze without of_ in a header until we fix all call
sites (which you can put on your to-do list :-)
Cheers,
Ben.
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>
> arch/microblaze/kernel/prom.c | 18 ------------------
> arch/powerpc/kernel/prom.c | 18 ------------------
> drivers/of/base.c | 18 ++++++++++++++++++
> 3 files changed, 18 insertions(+), 36 deletions(-)
>
> diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
> index 543465a..c97192d 100644
> --- a/arch/microblaze/kernel/prom.c
> +++ b/arch/microblaze/kernel/prom.c
> @@ -281,24 +281,6 @@ void __init early_init_devtree_arch(void)
> /* No Microblaze specific code here */
> }
>
> -/**
> - * Indicates whether the root node has a given value in its
> - * compatible property.
> - */
> -int machine_is_compatible(const char *compat)
> -{
> - struct device_node *root;
> - int rc = 0;
> -
> - root = of_find_node_by_path("/");
> - if (root) {
> - rc = of_device_is_compatible(root, compat);
> - of_node_put(root);
> - }
> - return rc;
> -}
> -EXPORT_SYMBOL(machine_is_compatible);
> -
> /*******
> *
> * New implementation of the OF "find" APIs, return a refcounted
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index a5b3b9d..65de093 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -677,24 +677,6 @@ void __init early_init_devtree_arch(void)
> lmb_enforce_memory_limit(limit);
> }
>
> -/**
> - * Indicates whether the root node has a given value in its
> - * compatible property.
> - */
> -int machine_is_compatible(const char *compat)
> -{
> - struct device_node *root;
> - int rc = 0;
> -
> - root = of_find_node_by_path("/");
> - if (root) {
> - rc = of_device_is_compatible(root, compat);
> - of_node_put(root);
> - }
> - return rc;
> -}
> -EXPORT_SYMBOL(machine_is_compatible);
> -
> /*******
> *
> * New implementation of the OF "find" APIs, return a refcounted
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index ec56739..e81558f 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -144,6 +144,24 @@ int of_device_is_compatible(const struct device_node *device,
> EXPORT_SYMBOL(of_device_is_compatible);
>
> /**
> + * Indicates whether the root node has a given value in its
> + * compatible property.
> + */
> +int machine_is_compatible(const char *compat)
> +{
> + struct device_node *root;
> + int rc = 0;
> +
> + root = of_find_node_by_path("/");
> + if (root) {
> + rc = of_device_is_compatible(root, compat);
> + of_node_put(root);
> + }
> + return rc;
> +}
> +EXPORT_SYMBOL(machine_is_compatible);
> +
> +/**
> * of_device_is_available - check if a device is available for use
> *
> * @device: Node to check for availability
^ permalink raw reply
* Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef
From: Grant Likely @ 2009-11-26 4:05 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, davem
In-Reply-To: <1259207974.16367.226.camel@pasglop>
On Wed, Nov 25, 2009 at 8:59 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2009-11-24 at 01:18 -0700, Grant Likely wrote:
>> A cell is firmly established as a u32. =A0No need to do an ugly typedef
>> to redefine it to cell_t. =A0Eliminate the unnecessary typedef so that
>> it doesn't have to be added to the of_fdt header file
>>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> ---
>
> I'm not sure about that one. Yes, we do use u32 a lot and cell_t rarely,
> so it would seem logical to switch.... On the other hand, we have that
> pesky endianness issue we have never fully solved. So we need accessors
> to sort that out, which means directly tapping things as u32 * is not a
> good idea if we're going to enforce the use of such accessors.
>
> I believe we should probably just enforce that properties are big endian
> for flat device-trees. In which case we could just use __be32 or on of
> thoes sparse-friendly types. I know x86 people won't like that much and
> to be honest I don't know what 1295 specifies for real OFs but there
> aren't enough real OFs around on LE machines for us to care much about
> it, is there ?
Word from Mitch is the device tree is network byte order. period.
> The reason I prefer a fixed endianness is that allowing "LE" trees
> becomes really nasty when a number is expressed using multiple cells.
> That brings the question as to whether the two cells need to be flipped
> as well or only the bytes within each cell. And that's the easy bit
> (probably flip the whole thing). What about something like a PCI "reg"
> property which is made of 3 cells, two of them forming a 64-bit address
> and one containing additional data & attributes ? What is flipped and
> where ?
exactly.
> So yes, cell_t might not be the right approach and by far to generic a
> name, but u32 isn't the answer neither.
You're right, it's not, but makes merging less complex, and then I can
refactor properly.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 08/11] of: Merge of_node_get() and of_node_put()
From: Benjamin Herrenschmidt @ 2009-11-26 4:06 UTC (permalink / raw)
To: Grant Likely
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, davem
In-Reply-To: <20091124081918.6216.77775.stgit@angua>
On Tue, 2009-11-24 at 01:19 -0700, Grant Likely wrote:
>
> +#if !defined(CONFIG_SPARC) /* SPARC doesn't do ref counting (yet) */
> +/**
Make this a Kconfig symbol, something like CONFIG_OF_DYNAMIC. You need
refcounting when you can add/remove nodes dynamically. Some embedded
archs might want the option to not enable that and save space.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 09/11] of: merge of_attach_node() & of_detach_node()
From: Benjamin Herrenschmidt @ 2009-11-26 4:07 UTC (permalink / raw)
To: Grant Likely
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, davem
In-Reply-To: <20091124081931.6216.27102.stgit@angua>
On Tue, 2009-11-24 at 01:19 -0700, Grant Likely wrote:
> Merge common code between PowerPC and Microblaze
Some of those guys might wnat to be in of_dynamic (see previous email)
Remember: We want to keep the footprint low for embedded archs that
don't want to do dynamic stuff. Really low.
Cheers,
Ben.
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>
> arch/microblaze/include/asm/prom.h | 4 --
> arch/microblaze/kernel/prom.c | 59 -----------------------------------
> arch/powerpc/include/asm/prom.h | 4 --
> arch/powerpc/kernel/prom.c | 59 -----------------------------------
> drivers/of/base.c | 60 ++++++++++++++++++++++++++++++++++++
> include/linux/of.h | 4 ++
> 6 files changed, 64 insertions(+), 126 deletions(-)
>
> diff --git a/arch/microblaze/include/asm/prom.h b/arch/microblaze/include/asm/prom.h
> index 07d1063..6c6b386 100644
> --- a/arch/microblaze/include/asm/prom.h
> +++ b/arch/microblaze/include/asm/prom.h
> @@ -39,10 +39,6 @@ extern struct device_node *of_chosen;
>
> extern rwlock_t devtree_lock; /* temporary while merging */
>
> -/* For updating the device tree at runtime */
> -extern void of_attach_node(struct device_node *);
> -extern void of_detach_node(struct device_node *);
> -
> /* Other Prototypes */
> extern int early_uartlite_console(void);
>
> diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
> index c0d53b7..8c00457 100644
> --- a/arch/microblaze/kernel/prom.c
> +++ b/arch/microblaze/kernel/prom.c
> @@ -313,65 +313,6 @@ struct device_node *of_find_node_by_phandle(phandle handle)
> }
> EXPORT_SYMBOL(of_find_node_by_phandle);
>
> -/*
> - * Plug a device node into the tree and global list.
> - */
> -void of_attach_node(struct device_node *np)
> -{
> - unsigned long flags;
> -
> - write_lock_irqsave(&devtree_lock, flags);
> - np->sibling = np->parent->child;
> - np->allnext = allnodes;
> - np->parent->child = np;
> - allnodes = np;
> - write_unlock_irqrestore(&devtree_lock, flags);
> -}
> -
> -/*
> - * "Unplug" a node from the device tree. The caller must hold
> - * a reference to the node. The memory associated with the node
> - * is not freed until its refcount goes to zero.
> - */
> -void of_detach_node(struct device_node *np)
> -{
> - struct device_node *parent;
> - unsigned long flags;
> -
> - write_lock_irqsave(&devtree_lock, flags);
> -
> - parent = np->parent;
> - if (!parent)
> - goto out_unlock;
> -
> - if (allnodes == np)
> - allnodes = np->allnext;
> - else {
> - struct device_node *prev;
> - for (prev = allnodes;
> - prev->allnext != np;
> - prev = prev->allnext)
> - ;
> - prev->allnext = np->allnext;
> - }
> -
> - if (parent->child == np)
> - parent->child = np->sibling;
> - else {
> - struct device_node *prevsib;
> - for (prevsib = np->parent->child;
> - prevsib->sibling != np;
> - prevsib = prevsib->sibling)
> - ;
> - prevsib->sibling = np->sibling;
> - }
> -
> - of_node_set_flag(np, OF_DETACHED);
> -
> -out_unlock:
> - write_unlock_irqrestore(&devtree_lock, flags);
> -}
> -
> #if defined(CONFIG_DEBUG_FS) && defined(DEBUG)
> static struct debugfs_blob_wrapper flat_dt_blob;
>
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index 2ab9cbd..f384db8 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -34,10 +34,6 @@ extern struct device_node *of_chosen;
>
> #define HAVE_ARCH_DEVTREE_FIXUPS
>
> -/* For updating the device tree at runtime */
> -extern void of_attach_node(struct device_node *);
> -extern void of_detach_node(struct device_node *);
> -
> #ifdef CONFIG_PPC32
> /*
> * PCI <-> OF matching functions
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 6873db9..7d0beeb 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -740,65 +740,6 @@ struct device_node *of_find_next_cache_node(struct device_node *np)
> return NULL;
> }
>
> -/*
> - * Plug a device node into the tree and global list.
> - */
> -void of_attach_node(struct device_node *np)
> -{
> - unsigned long flags;
> -
> - write_lock_irqsave(&devtree_lock, flags);
> - np->sibling = np->parent->child;
> - np->allnext = allnodes;
> - np->parent->child = np;
> - allnodes = np;
> - write_unlock_irqrestore(&devtree_lock, flags);
> -}
> -
> -/*
> - * "Unplug" a node from the device tree. The caller must hold
> - * a reference to the node. The memory associated with the node
> - * is not freed until its refcount goes to zero.
> - */
> -void of_detach_node(struct device_node *np)
> -{
> - struct device_node *parent;
> - unsigned long flags;
> -
> - write_lock_irqsave(&devtree_lock, flags);
> -
> - parent = np->parent;
> - if (!parent)
> - goto out_unlock;
> -
> - if (allnodes == np)
> - allnodes = np->allnext;
> - else {
> - struct device_node *prev;
> - for (prev = allnodes;
> - prev->allnext != np;
> - prev = prev->allnext)
> - ;
> - prev->allnext = np->allnext;
> - }
> -
> - if (parent->child == np)
> - parent->child = np->sibling;
> - else {
> - struct device_node *prevsib;
> - for (prevsib = np->parent->child;
> - prevsib->sibling != np;
> - prevsib = prevsib->sibling)
> - ;
> - prevsib->sibling = np->sibling;
> - }
> -
> - of_node_set_flag(np, OF_DETACHED);
> -
> -out_unlock:
> - write_unlock_irqrestore(&devtree_lock, flags);
> -}
> -
> #ifdef CONFIG_PPC_PSERIES
> /*
> * Fix up the uninitialized fields in a new device node:
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 4b10c89..9212b87 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -60,6 +60,66 @@ int of_n_size_cells(struct device_node *np)
> }
> EXPORT_SYMBOL(of_n_size_cells);
>
> +/**
> + * of_attach_node - Plug a device node into the tree and global list.
> + */
> +void of_attach_node(struct device_node *np)
> +{
> + unsigned long flags;
> +
> + write_lock_irqsave(&devtree_lock, flags);
> + np->sibling = np->parent->child;
> + np->allnext = allnodes;
> + np->parent->child = np;
> + allnodes = np;
> + write_unlock_irqrestore(&devtree_lock, flags);
> +}
> +
> +/**
> + * of_detach_node - "Unplug" a node from the device tree.
> + *
> + * The caller must hold a reference to the node. The memory associated with
> + * the node is not freed until its refcount goes to zero.
> + */
> +void of_detach_node(struct device_node *np)
> +{
> + struct device_node *parent;
> + unsigned long flags;
> +
> + write_lock_irqsave(&devtree_lock, flags);
> +
> + parent = np->parent;
> + if (!parent)
> + goto out_unlock;
> +
> + if (allnodes == np)
> + allnodes = np->allnext;
> + else {
> + struct device_node *prev;
> + for (prev = allnodes;
> + prev->allnext != np;
> + prev = prev->allnext)
> + ;
> + prev->allnext = np->allnext;
> + }
> +
> + if (parent->child == np)
> + parent->child = np->sibling;
> + else {
> + struct device_node *prevsib;
> + for (prevsib = np->parent->child;
> + prevsib->sibling != np;
> + prevsib = prevsib->sibling)
> + ;
> + prevsib->sibling = np->sibling;
> + }
> +
> + of_node_set_flag(np, OF_DETACHED);
> +
> +out_unlock:
> + write_unlock_irqrestore(&devtree_lock, flags);
> +}
> +
> #if !defined(CONFIG_SPARC) /* SPARC doesn't do ref counting (yet) */
> /**
> * of_node_get - Increment refcount of a node
> diff --git a/include/linux/of.h b/include/linux/of.h
> index d4c014a..0a51742 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -130,6 +130,10 @@ static inline unsigned long of_read_ulong(const u32 *cell, int size)
>
> #define OF_BAD_ADDR ((u64)-1)
>
> +/* For updating the device tree at runtime */
> +extern void of_attach_node(struct device_node *);
> +extern void of_detach_node(struct device_node *);
> +
> extern struct device_node *of_find_node_by_name(struct device_node *from,
> const char *name);
> #define for_each_node_by_name(dn, name) \
^ permalink raw reply
* Re: [RFC PATCH 01/19] powerpc: gamecube/wii: usbgecko bootwrapper console support
From: Benjamin Herrenschmidt @ 2009-11-26 4:12 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <1258927311-4340-2-git-send-email-albert_herranz@yahoo.es>
On Sun, 2009-11-22 at 23:01 +0100, Albert Herranz wrote:
> Add support for using the USB Gecko adapter as a bootwrapper console on
> the Nintendo GameCube and Wii video game consoles.
> The USB Gecko is a 3rd party memory card interface adapter that provides
> a EXI (External Interface) to USB serial converter.
Looks good. Not sure yet when I'll merge these, I might wait a bit for
the dust to settle but I may also just stick some of the simple/obvious
patches in early like this one to make things easier.
Cheers,
Ben.
> Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
> ---
> arch/powerpc/boot/Makefile | 2 +-
> arch/powerpc/boot/ugecon.c | 128 ++++++++++++++++++++++++++++++++++++++++++++
> arch/powerpc/boot/ugecon.h | 25 +++++++++
> 3 files changed, 154 insertions(+), 1 deletions(-)
> create mode 100644 arch/powerpc/boot/ugecon.c
> create mode 100644 arch/powerpc/boot/ugecon.h
>
> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> index 7bfc8ad..44bce21 100644
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -66,7 +66,7 @@ src-wlib := string.S crt0.S crtsavres.S stdio.c main.c \
> gunzip_util.c elf_util.c $(zlib) devtree.c oflib.c ofconsole.c \
> 4xx.c ebony.c mv64x60.c mpsc.c mv64x60_i2c.c cuboot.c bamboo.c \
> cpm-serial.c stdlib.c mpc52xx-psc.c planetcore.c uartlite.c \
> - fsl-soc.c mpc8xx.c pq2.c
> + fsl-soc.c mpc8xx.c pq2.c ugecon.c
> src-plat := of.c cuboot-52xx.c cuboot-824x.c cuboot-83xx.c cuboot-85xx.c holly.c \
> cuboot-ebony.c cuboot-hotfoot.c treeboot-ebony.c prpmc2800.c \
> ps3-head.S ps3-hvcall.S ps3.c treeboot-bamboo.c cuboot-8xx.c \
> diff --git a/arch/powerpc/boot/ugecon.c b/arch/powerpc/boot/ugecon.c
> new file mode 100644
> index 0000000..704f374
> --- /dev/null
> +++ b/arch/powerpc/boot/ugecon.c
> @@ -0,0 +1,128 @@
> +/*
> + * arch/powerpc/boot/ugecon.c
> + *
> + * USB Gecko bootwrapper console.
> + * 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 <stddef.h>
> +#include "stdio.h"
> +#include "types.h"
> +#include "io.h"
> +#include "ops.h"
> +
> +
> +#define EXI_CLK_32MHZ 5
> +
> +#define EXI_CSR 0x00
> +#define EXI_CSR_CLKMASK (0x7<<4)
> +#define EXI_CSR_CLK_32MHZ (EXI_CLK_32MHZ<<4)
> +#define EXI_CSR_CSMASK (0x7<<7)
> +#define EXI_CSR_CS_0 (0x1<<7) /* Chip Select 001 */
> +
> +#define EXI_CR 0x0c
> +#define EXI_CR_TSTART (1<<0)
> +#define EXI_CR_WRITE (1<<2)
> +#define EXI_CR_READ_WRITE (2<<2)
> +#define EXI_CR_TLEN(len) (((len)-1)<<4)
> +
> +#define EXI_DATA 0x10
> +
> +
> +/* virtual address base for input/output, retrieved from device tree */
> +static void *ug_io_base;
> +
> +
> +static u32 ug_io_transaction(u32 in)
> +{
> + u32 *csr_reg = ug_io_base + EXI_CSR;
> + u32 *data_reg = ug_io_base + EXI_DATA;
> + u32 *cr_reg = ug_io_base + EXI_CR;
> + u32 csr, data, cr;
> +
> + /* select */
> + csr = EXI_CSR_CLK_32MHZ | EXI_CSR_CS_0;
> + out_be32(csr_reg, csr);
> +
> + /* read/write */
> + data = in;
> + out_be32(data_reg, data);
> + cr = EXI_CR_TLEN(2) | EXI_CR_READ_WRITE | EXI_CR_TSTART;
> + out_be32(cr_reg, cr);
> +
> + while (in_be32(cr_reg) & EXI_CR_TSTART)
> + barrier();
> +
> + /* deselect */
> + out_be32(csr_reg, 0);
> +
> + data = in_be32(data_reg);
> + return data;
> +}
> +
> +static int ug_is_txfifo_ready(void)
> +{
> + return ug_io_transaction(0xc0000000) & 0x04000000;
> +}
> +
> +static void ug_raw_putc(char ch)
> +{
> + ug_io_transaction(0xb0000000 | (ch << 20));
> +}
> +
> +static void ug_putc(char ch)
> +{
> + int count = 16;
> +
> + if (!ug_io_base)
> + return;
> +
> + while (!ug_is_txfifo_ready() && count--)
> + barrier();
> + if (count)
> + ug_raw_putc(ch);
> +}
> +
> +void ug_console_write(const char *buf, int len)
> +{
> + char *b = (char *)buf;
> +
> + while (len--) {
> + if (*b == '\n')
> + ug_putc('\r');
> + ug_putc(*b++);
> + }
> +}
> +
> +int ug_is_adapter_present(void)
> +{
> + if (!ug_io_base)
> + return 0;
> +
> + return ug_io_transaction(0x90000000) == 0x04700000;
> +}
> +
> +int ug_grab_io_base(void)
> +{
> + u32 v;
> + void *devp;
> +
> + devp = find_node_by_alias("ugecon");
> + if (devp == NULL)
> + goto err_out;
> + if (getprop(devp, "virtual-reg", &v, sizeof(v)) != sizeof(v))
> + goto err_out;
> +
> + ug_io_base = (u8 *)v;
> + return 0;
> +
> +err_out:
> + return -1;
> +}
> diff --git a/arch/powerpc/boot/ugecon.h b/arch/powerpc/boot/ugecon.h
> new file mode 100644
> index 0000000..1fdb590
> --- /dev/null
> +++ b/arch/powerpc/boot/ugecon.h
> @@ -0,0 +1,25 @@
> +/*
> + * arch/powerpc/boot/ugecon.h
> + *
> + * USB Gecko early bootwrapper console.
> + * 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.
> + *
> + */
> +
> +#ifndef __UGECON_H
> +#define __UGECON_H
> +
> +extern int ug_grab_io_base(void);
> +extern int ug_is_adapter_present(void);
> +
> +extern void ug_putc(char ch);
> +extern void ug_console_write(const char *buf, int len);
> +
> +#endif /* __UGECON_H */
> +
^ permalink raw reply
* Re: [RFC PATCH 02/19] powerpc: gamecube: device tree
From: Benjamin Herrenschmidt @ 2009-11-26 4:21 UTC (permalink / raw)
To: Grant Likely; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <fa686aa40911221502g2de254d2o4341d9abed0cdf41@mail.gmail.com>
On Sun, 2009-11-22 at 16:02 -0700, Grant Likely wrote:
> > + /* 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.
I would really just call it "flipper" :-)
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + #interrupt-cells = <1>;
> > + model = "flipper";
>
> Drop the model property
>
> > + 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.
On the other hand it is a useful "documentation" to specify the exact
range decoded when you know it :-) For non documented HW I prefer when
the DT contains as precise information as possible. It also allows, if
so wished, to create proper hierarchical struct resource in the kernel
that represent the bus hierarchy in a nicer way. So I vote for keeping
that in.
> > +
> > + 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.
Note that this is a linux-ism no ? (aka ePAPRism). If they aim toward
having a real OF which I think they do they may wish to pass on this
trick.
> > + /* 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 agree with using the same form as reg. I'm not sure about using "reg",
it depends. Albert, is that xfb location something that is configurable
or is it fixed ? If it's configurable, it could remain a separate
property I suppose, since it overlaps main memory, it's a bit fishy to
have it in "reg"... you do seem to strip off the fb from the memory
"reg" property though... Maybe the right thing to do is to leave the
memory "reg" property to be the whole RAM and just reserve the area
covered by the fb ?
> > + /* 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>;
> > + };
> > + };
> > + };
> > +};
> > +
Shouldn't the above be dynamically detected ?
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH 02/19] powerpc: gamecube: device tree
From: Benjamin Herrenschmidt @ 2009-11-26 4:23 UTC (permalink / raw)
To: Grant Likely; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <fa686aa40911231219k3a52b9fdn66b8cffb735e5c2f@mail.gmail.com>
On Mon, 2009-11-23 at 13:19 -0700, Grant Likely wrote:
> so the node really is
> describing the internal bus, not the entire SoC. On some chips it is
> documented as the "internally memory mapped registers", or IMMR. So,
> it is better to name this node in a way that reflects what it is (an
> internal bus) instead of as the whole chip.
It's not even the internal bus. Flipper and Hollywood are separate chips
from the PPC afaik. They are integrated northbridge + gfx + IOs
basically.
> Similarly, it is better to use a compatible value of something like:
> compatible = "nintendo,flipper-immr"; (instead of "nintendo,flipper")
> because your describing just the internal bus, not the entire chip.
I would just call the nodes "flipper" and "hollywood".
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH 02/19] powerpc: gamecube: device tree
From: Benjamin Herrenschmidt @ 2009-11-26 4:23 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <0AE26D40-D051-4CDE-878C-64CC8EF33E07@kernel.crashing.org>
On Tue, 2009-11-24 at 23:36 +0100, Segher Boessenkool wrote:
> If you have only one interrupt controller, like here, you don't
> need to refer to it _at all_ :-)
I think Linux requires that you do though. It might be a mistake on our
part but heh ...
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH 02/19] powerpc: gamecube: device tree
From: Benjamin Herrenschmidt @ 2009-11-26 4:27 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <49443.84.105.60.153.1259172058.squirrel@gate.crashing.org>
On Wed, 2009-11-25 at 19:00 +0100, Segher Boessenkool wrote:
> > + memory {
> > + device_type = "memory";
> > + /* 24M minus framebuffer memory area (640*576*2*2) */
> > + reg = <0x00000000 0x01698000>;
>
> Put the whole 24MB here, probe the framebuffer address and size
> in the platform code?
Agreed. That's what I was proposing. Though you need to be careful about
early boot code that will try to allocate the hash table etc... or even
the DT itself. So you need to probe and reserve the fb really early,
for example in the platform probe() routine itself. Or you can stick it
in the reserve map too I suppose.
> > + video@0c002000 {
> > + compatible = "nintendo,flipper-video";
> > + reg = <0x0c002000 0x100>;
> > + interrupts = <8>;
> > + interrupt-parent = <&pic>;
> > + /* XFB is the eXternal FrameBuffer */
> > + xfb-start = <0x01698000>; /* end-of-ram - xfb-size */
> > + xfb-size = <0x168000>;
>
> XFB address isn't fixed on the hardware, and the kernel might
> want to move it, and you can easily probe for it anyway. Remove
> these last two properties please.
Ok but you need to know what it was setup to by the FW no ? To avoid
having a temporary display "glitch" while booting... Also on a 24M
config,it might get tough for the driver to allocate 2M contiguous like
that if it's done late in the boot process.
>
> > + 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>;
> > + };
>
> These two have the same address, not good. Just remove the
> auxram node?
Or make it a child of audio ? :-)
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH 03/19] powerpc: gamecube: bootwrapper bits
From: Benjamin Herrenschmidt @ 2009-11-26 4:35 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0C1A25.8030401@yahoo.es>
On Tue, 2009-11-24 at 18:38 +0100, Albert Herranz wrote:
> Segher Boessenkool wrote:
> > Hi Albert,
> >
> >> +asm ("\n\
> >
> >
> > A file scope asm?! Please don't.
> >
>
> So what's your proposal then? Placing it within a fake func?
Just do a .S file :-)
> That asm snippet is the entry point. I took as an example how prpmc2800.c
> deals with that, providing an own version of the (weak) _zImage_start.
Right but I agree with Segher here, it would be nicer as a .S file.
> >> + * We enter with the cache enabled, the MMU enabled and some known
> >> legacy
> >> + * memory mappings active. xBAT3 is unused
> >
> > It would be good if you could depend as little as possible on these things;
> > that makes writing another bootloader a lot easier.
> >
>
> Ok. I'll do a similar approach as done on the wii bootwrapper.
>
> >> + /* IBAT3,DBAT3 for first 16Mbytes */\n\
> >> + li 8, 0x01ff /* 16MB */\n\
> >> + li 9, 0x0002 /* rw */\n\
> >> + mtspr 0x216, 8 /* IBAT3U */\n\
> >> + mtspr 0x217, 9 /* IBAT3L */\n\
> >> + mtspr 0x21e, 8 /* DBAT3U */\n\
> >> + mtspr 0x21f, 9 /* DBAT3L */\n\
> >
> > WIMG=0000, are you sure? Not M=1?
> >
>
> To be honest, I don't recall the details now.
> But it was tested in the very early days, the result was not the expected one and,
> in the end, manual cache coherency management was still needed.
Ouch. I wouldn't be surprised if those guys don't do cache coherency
in the bridge anyways.
> So everything is designed and working assuming M=0.
> This can be re-checked again later if needed.
Agreed.
> >> + bcl- 20,4*cr7+so,1f\n\
> >
> > Just write bcl 20,31,1f .
>
> Ok, I used two variants for this and I know which one you like now ;).
Cheers,
Ben.
> >
> >
> > Segher
> >
> >
>
> Thanks,
> Albert
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [RFC PATCH 03/19] powerpc: gamecube: bootwrapper bits
From: Benjamin Herrenschmidt @ 2009-11-26 4:36 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <CC1D5EB1-27C2-47A1-96D8-D007D86C4881@kernel.crashing.org>
On Tue, 2009-11-24 at 22:00 +0100, Segher Boessenkool wrote:
>
> Sure, the memory controllers don't do coherency. I'm slightly worried
> about two things:
> 1) Will the generic code use M=0 as well? Is it a problem if it
> doesn't?
We can make it not do it.
> 2) Do lwarx. etc. work in M=0?
They should hopefully... as long as you don't rely on the reservation
blowing as a result of a DMA write.
Cheers,
Ben
^ permalink raw reply
* Re: [RFC PATCH 02/19] powerpc: gamecube: device tree
From: Grant Likely @ 2009-11-26 4:38 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <1259209283.16367.241.camel@pasglop>
On Wed, Nov 25, 2009 at 9:21 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Sun, 2009-11-22 at 16:02 -0700, Grant Likely wrote:
>> > +
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 video@0c002000 {
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "nintendo=
,flipper-video";
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D <0x0c002000 0x10=
0>;
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 interrupts =3D <8>;
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 interrupt-parent =3D <&p=
ic>;
>>
>> Hint: =A0If 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.
>
> Note that this is a linux-ism no ? (aka ePAPRism). If they aim toward
> having a real OF which I think they do they may wish to pass on this
> trick.
But this isn't real OF. Real OF can generate its own valid tree.
This is a flat tree, and it is valid according to all users of the
flat tree. Besides, the last time we talked about this, you told me
that moving it to the root was a good idea, and I happily changed all
the 5200 .dts files. :-)
http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-January/067046.html
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [RFC PATCH 04/19] powerpc: wii: device tree
From: Benjamin Herrenschmidt @ 2009-11-26 4:45 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <1258927311-4340-5-git-send-email-albert_herranz@yahoo.es>
On Sun, 2009-11-22 at 23:01 +0100, Albert Herranz wrote:
> +/memreserve/ 0x01800000 0xe800000; /* memory hole (includes I/O area) */
> +/memreserve/ 0x10000000 0x0004000; /* DSP RAM */
Weird layout... nothing you can do about I suppose.
Out of curiosity, what is that DSP RAM ? Some actual DSP core somewhere
in the IO chip setup to use memory from up there ?
I'll skip on some things that I'm sure others will have commented
about :-)
> + /*
> + * The Nintendo Wii has two discontiguous RAM memory areas called
> + * MEM1 and MEM2.
> + * MEM1 starts at 0x00000000 and contains 24MB of 1T-SRAM.
> + * MEM2 starts at 0x10000000 and contains 64MB of DDR2 RAM.
> + * Between both memory address ranges there is an address space
> + * where memory-mapped I/O registers are found.
> + *
> + * Currently, Linux 32-bit PowerPC does not support RAM in
> + * discontiguous memory address spaces. Thus, in order to use
> + * both RAM areas, we declare as RAM the range from the start of
> + * MEM1 to the end of useable MEM2 and exclude the needed parts
> + * with /memreserve/ statements, at the expense of wasting a bit
> + * of memory.
> + */
> + memory {
> + device_type = "memory";
> + /* MEM1 + memory hole + MEM2 - firmware/buffers area */
> + reg = <0x00000000 0x133e0000>;
> + };
So we do have a nasty hole here in the middle. How bad it is if you try
to just have two ranges in there (ie as if it was discontiguous) ? We
shouldn't be far from being able to do discontig mem actually, should be
easy enough to fix. Tho I don't have (yet) the HW :-) (I'm tempted...)
Same comment as other discussions about the framebuffer here.
> + cpus {
> + #cpus = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + PowerPC,broadway@0 {
> + device_type = "cpu";
> + reg = <0>;
> + clock-frequency = <729000000>; /* 729MHz */
> + bus-frequency = <243000000>; /* 243MHz core-to-bus 3x */
> + timebase-frequency = <60750000>; /* 243MHz / 4 */
> + i-cache-line-size = <32>;
> + d-cache-line-size = <32>;
> + i-cache-size = <32768>;
> + d-cache-size = <32768>;
> + };
> + };
> +
> + /* devices contained in the hollywood chipset */
> + soc {
Call it "hollywood"
> + #address-cells = <1>;
> + #size-cells = <1>;
> + #interrupt-cells = <1>;
> + model = "hollywood";
> + compatible = "nintendo,hollywood";
> + clock-frequency = <243000000>; /* 243MHz */
> + ranges = <0x0c000000 0x0c000000 0x00010000
> + 0x0d000000 0x0d000000 0x00010000
> + 0x0d040000 0x0d040000 0x00050000
> + 0x0d800000 0x0d800000 0x00001000
> + 0x133e0000 0x133e0000 0x00c20000>;
> +
> + video@0c002000 {
> + compatible = "nintendo,hollywood-video";
> + reg = <0x0c002000 0x100>;
> + interrupts = <8>;
> + interrupt-parent = <&PIC0>;
> + };
> +
> + PIC0: pic0@0c003000 {
> + #interrupt-cells = <1>;
> + compatible = "nintendo,flipper-pic";
> + reg = <0x0c003000 0x8>;
> + interrupt-controller;
> + };
> +
> + resetswitch@0c003000 {
> + compatible = "nintendo,hollywood-resetswitch";
> + reg = <0x0c003000 0x4>;
> + interrupts = <1>;
> + interrupt-parent = <&PIC0>;
> + };
> +
> + audio@0c005000 {
> + compatible = "nintendo,hollywood-audio";
> + reg = <0x0c005000 0x200 /* DSP */
> + 0x0d006c00 0x20>; /* AI */
> + interrupts = <6>;
> + interrupt-parent = <&PIC0>;
> + };
> +
> + /* Team Twiizers' 'mini' firmware IPC */
Out of curiosity, what are these ? :-)
> + mini@0d000000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + #interrupt-cells = <1>;
> + compatible = "twiizers,starlet-mini-ipc";
> + reg = <0x0d000000 0x40 /* IPC */
> + 0x13fffffc 0x4>; /* mini header pointer */
> + };
> +
> + serial@0d006400 {
> + compatible = "nintendo,hollywood-serial";
> + reg = <0x0d006400 0x100>;
> + interrupts = <3>;
> + interrupt-parent = <&PIC0>;
> + };
> +
> + /* External Interface bus */
> + exi@0d006800 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "nintendo,hollywood-exi";
> + reg = <0x0d006800 0x40>;
> + interrupts = <4>;
> + interrupt-parent = <&PIC0>;
> +
> + USBGECKO0: usbgecko@0d006814 {
> + compatible = "usbgecko,usbgecko";
> + reg = <0x0d006814 0x14>;
> + virtual-reg = <0xcd006814>;
> + };
> + };
Similar comment as before, could the above be dynamically probed ? If
you are not a hacker you may not need that at all to use some linux
based piece of SW on the wii right ?
> + ehci@0d040000 {
> + compatible = "nintendo,hollywood-ehci";
> + reg = <0x0d040000 0x100
> + 0x133e0000 0x80000>; /* 512 KB */
> + interrupts = <4>;
> + interrupt-parent = <&PIC1>;
> + };
> +
> + ohci0@0d050000 {
> + compatible = "nintendo,hollywood-ohci";
> + reg = <0x0d050000 0x100
> + 0x13460000 0x80000>; /* 512 KB */
> + interrupts = <5>;
> + interrupt-parent = <&PIC1>;
> + };
> +
> + ohci1@0d060000 {
Why the "1" ?
> + compatible = "nintendo,hollywood-ohci";
> + reg = <0x0d060000 0x100
> + 0x134e0000 0x80000>; /* 512 KB */
> + interrupts = <6>;
> + interrupt-parent = <&PIC1>;
> + };
Are the above OHCI and EHCI "special" ? If not, there's an existing
binding for that sort of thing, which btw requires properties to
indicate the endianness of the registers and in-memory data structures
etc...
> + sdhc0@0d070000 {
> + compatible = "nintendo,hollywood-sdhci";
> + reg = <0x0d070000 0x200>;
> + interrupts = <7>;
> + interrupt-parent = <&PIC1>;
> + };
> +
> + sdhc1@0d080000 {
> + compatible = "nintendo,hollywood-sdhci";
> + reg = <0x0d080000 0x200>;
> + interrupts = <8>;
> + interrupt-parent = <&PIC1>;
> + };
Again, no need for a "1" in there. Names don't have to be unique if the
unit address is different.
> + PIC1: pic1@0d800030 {
> + #interrupt-cells = <1>;
> + compatible = "nintendo,hollywood-pic";
> + reg = <0x0d800030 0x8>;
> + interrupt-controller;
> + interrupts = <14>;
> + interrupt-parent = <&PIC0>;
> + };
Ah, a cascaded PIC, fun fun fun
> + hollywood-ahbprot@0d800064 {
> + compatible = "nintendo,hollywood-ahbprot";
> + reg = <0x0d800064 0x4>;
> + };
What is this ?
> + gpio0: hollywood-gpio@0d8000c0 {
> + compatible = "nintendo,hollywood-gpio";
> + reg = <0x0d8000c0 0x20>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
> + gpio1: hollywood-gpio@0d8000e0 {
> + compatible = "nintendo,hollywood-gpio";
> + reg = <0x0d8000e0 0x20>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
Same comment about the "1"
> + hollywood-resets@0d800194 {
> + compatible = "nintendo,hollywood-resets";
> + reg = <0x0d800194 0x4>;
> + };
> +
> + i2c-video {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "virtual,i2c-gpio";
> +
> + gpios = <&gpio0 16 0 /* 31-15 */
> + &gpio0 17 0 /* 31-14 */
> + >;
> + sda-is-open-drain = <1>;
> + sda-enforce-dir = <1>;
> + scl-is-open-drain = <1>;
> + scl-is-output-only = <1>;
> + udelay = <2>;
> +
> + audio-video-encoder {
> + compatible = "nintendo,wii-ave-rvl";
> + reg = <0x70>;
> + };
> + };
> + };
> +};
> +
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH 04/19] powerpc: wii: device tree
From: Benjamin Herrenschmidt @ 2009-11-26 4:51 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <49436.84.105.60.153.1259171377.squirrel@gate.crashing.org>
On Wed, 2009-11-25 at 18:49 +0100, Segher Boessenkool wrote:
> > +/memreserve/ 0x01800000 0xe800000; /* memory hole (includes I/O area) */
>
> Like others have said already, don't do this. If you need
> a workaround, put it in the platform code.
>
> > +/memreserve/ 0x10000000 0x0004000; /* DSP RAM */
>
> This address is fixed in the DSP hardware, right? If not,
> you shouldn't do a reserve here.
>
> > + chosen {
> > + /* root filesystem on 2nd partition of SD card */
> > + bootargs = "nobats root=/dev/mmcblk0p2 rootwait udbg-immortal";
>
> Question: why do you need/want nobats?
Good point. I can't even guarantee that the kernel will work reliably
with nobats :-) At least you really want the kernel .text to be fully
covered by an IBAT.
> What does this clock freq mean? Hollywood has lots of
> clocks, many of them configurable. Bus frequency is in
> the cpu node already. The binding doesn't say what this
> is either, since you didn't write a binding :-)
>
> Just remove it?
BTW. If we want to play with clocks, maybe you should look at
my proposed binding for clocks and implementing the clk API :-)
> > + ranges = <0x0c000000 0x0c000000 0x00010000
> > + 0x0d000000 0x0d000000 0x00010000
> > + 0x0d040000 0x0d040000 0x00050000
> > + 0x0d800000 0x0d800000 0x00001000
>
> All of 0cXXXXXX and 0dXXXXXX actually.
>
> > + 0x133e0000 0x133e0000 0x00c20000>;
>
> This is just part of MEM2, don't treat it special here.
>
> > + video@0c002000 {
> > + compatible = "nintendo,hollywood-video";
> > + reg = <0x0c002000 0x100>;
> > + interrupts = <8>;
> > + interrupt-parent = <&PIC0>;
>
> If you say interrupt-parent = <..> in the root node, you can
> leave it out in most of the rest of the tree. Using the Flipper
> PIC for this sounds like a good plan.
Well, for the rest of the tree except stuff whose interrupt parent
is PIC1. With two PICs I prefer being explicit.
> > + PIC0: pic0@0c003000 {
> > + #interrupt-cells = <1>;
> > + compatible = "nintendo,flipper-pic";
> > + reg = <0x0c003000 0x8>;
>
> This register block is bigger actually. It's not only PIC,
> but some other bus stuff as well, dunno what to do here.
Add nodes for the other things ?
> > + interrupt-controller;
> > + };
> > +
> > + resetswitch@0c003000 {
> > + compatible = "nintendo,hollywood-resetswitch";
> > + reg = <0x0c003000 0x4>;
>
> That's the same address. Don't do that.
>
> Create a flipper-processor-interface node for the whole 3000
> block (size 100)? You can hang the PIC as a subnode under it,
> without a "reg".
Yeah.
> > + /* Team Twiizers' 'mini' firmware IPC */
> > + mini@0d000000 {
>
> Although mini is awesome, it isn't hardware, and doesn't
> belong in the device tree.
So what is at d0000000 ?
> > + serial@0d006400 {
> > + compatible = "nintendo,hollywood-serial";
>
> You might want to pick a more descriptive name for this,
> "serial" is usually understaood to mean "RS232". Which
> this isn't.
What is it then ? You definitely want some other name if it's not
classic rs232 style serial.
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH 04/19] powerpc: wii: device tree
From: Benjamin Herrenschmidt @ 2009-11-26 4:58 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0D78B0.502@yahoo.es>
On Wed, 2009-11-25 at 19:34 +0100, Albert Herranz wrote:
>
> We need nobats (or a hack in the mmu_mapin_ram() code) because of the
> discontiguous ram problem.
I would vote for a hack in mmu_mapin_ram() for now.
Cheers,
Ben.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox