From: Sam Ravnborg on
On Mon, Jul 26, 2010 at 04:04:55PM -0600, Grant Likely wrote:
> of_node_to_nid() is only relevant in a few architectures. Don't force
> everyone to implement it anyway. This patch also adds asm-generic/of.h
> which will be used to contain other overrideable symbols.
>
> Signed-off-by: Grant Likely <grant.likely(a)secretlab.ca>
> ---
>
> Changes in v3: don't use asm-generic, just keep macros in of.h
> Changes in v2: address comments from sfr, add asm-generic/of.h

The use of asm-generic makes perfect sense.
This is how we usually deal with arch specific stuff.

With v3 of your patch we have a different result depending
on if we do:
#include <linux/of.h>

or we do:
#include <asm/prom.h>

This is undesireable.

I suggest to go back to v2 of your patch where you use asm-generic/of.h.

linux/of.h shall include asm/of.h
Then all archs shall have a of.h that may
include the asm-generic variant.


One patch to introduce of.h all over.
And a second patch to do the of_node_to_nid stuff would be appropriate.


> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index da7dd63..dca25a5 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -103,6 +103,11 @@ struct device_node *of_find_next_cache_node(struct device_node *np);
> /* Get the MAC address */
> extern const void *of_get_mac_address(struct device_node *np);
>

This shall go in asm/of.h
> +#ifdef CONFIG_NUMA
> +extern int of_node_to_nid(struct device_node *device);
> +#define of_node_to_nid of_node_to_nid

This define is used to tell asm-generic/of.h that the arch has
a local definition - OK.

> +#endif
> +
> /**
> * of_irq_map_pci - Resolve the interrupt for a PCI device
> * @pdev: the device whose interrupt is to be resolved

> diff --git a/include/linux/of.h b/include/linux/of.h
> index b0756f3..cc936ca 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -146,6 +146,11 @@ static inline unsigned long of_read_ulong(const __be32 *cell, int size)
>
> #define OF_BAD_ADDR ((u64)-1)
>
> +#ifndef of_node_to_nid
> +static inline int of_node_to_nid(struct device_node *np) { return 0; }
> +#define of_node_to_nid of_node_to_nid

But I fail to see the purpose of this define.

Sam
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Sam Ravnborg on
On Tue, Jul 27, 2010 at 03:34:01PM +0200, Arnd Bergmann wrote:
> On Tuesday 27 July 2010, Grant Likely wrote:
> > > I suggest to go back to v2 of your patch where you use asm-generic/of.h.
> >
> > Stephen suggested dropping asm-generic/of.h. I'm happy to do it either way.
>
> I don't mind adding stuff to asm-generic, but I think in this case it would
> be easier to keep this in linux/of.h because there is nothing wrong with
> all architectures including it.
>
> Most files in asm-generic are there only for historical reasons, where some
> architectures use them but others don't. IMHO we should use the include/linux
> headers preferred for new stuff though.

Hi Arnd.

Thanks for this explanation. Obviously my previous post
about asm-generic was wrong (as Grant already indicated).

Sam
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/