Subject: Re: [PATCH 1/6] Start split out of common open firmware code

From: Segher Boessenkool <[email-masked]>
Date: Tue, 24 Apr 2007 15:27:42 +0200

> +int of_n_addr_cells(struct device_node *np)
> +{
> +	const int *ip;
> +
> +	do {
> +		if (np->parent)
> +			np = np->parent;
> +		ip = of_get_property(np, "#address-cells", NULL);
> +		if (ip)
> +			return *ip;
> +	} while (np->parent);
> +	/* No #address-cells property for the root node */
> +	return OF_ROOT_NODE_ADDR_CELLS_DEFAULT;
> +}
> +EXPORT_SYMBOL(of_n_addr_cells);

While I realise your patch is just moving stuff around,
can this be fixed please?  A missing "#address-cells"
property means "two cells", not "inherit from parent".
The few platforms with a broken tree should just be
fixed (patch the tree in the bootwrapper, or perhaps
_do_ use this workaround, but only on the affected
platforms).

> +int of_n_size_cells(struct device_node *np)

Same here (the default is one cell though).

> --- a/include/asm-powerpc/prom.h
> +++ b/include/asm-powerpc/prom.h
> @@ -21,6 +21,9 @@
>  #include <asm/irq.h>
>  #include <asm/atomic.h>
>
> +#define OF_ROOT_NODE_ADDR_CELLS_DEFAULT	1
> +#define OF_ROOT_NODE_SIZE_CELLS_DEFAULT	1

Even worse!

> --- a/include/asm-sparc/prom.h
> +++ b/include/asm-sparc/prom.h
> @@ -20,6 +20,9 @@
>  #include <linux/proc_fs.h>
>  #include <asm/atomic.h>
>
> +#define OF_ROOT_NODE_ADDR_CELLS_DEFAULT	2
> +#define OF_ROOT_NODE_SIZE_CELLS_DEFAULT	1

Those are the defaults for all nodes, not just the
root node.

Segher