[PATCH] drivers/of: use __be32 types for big-endian device tree data

[PATCH] drivers/of: use __be32 types for big-endian device tree data

[Jeremy Kerr]

Use the sparse annotations so we can keep track of endianness.

Signed-off-by: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

---
 drivers/of/address.c |    2 +-
 drivers/of/base.c    |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index fcadb72..3a1c7e7 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -163,7 +163,7 @@ static int of_bus_pci_translate(u32 *addr, u64 offset, int na)
 const u32 *of_get_pci_address(struct device_node *dev, int bar_no, u64 *size,
 			unsigned int *flags)
 {
-	const u32 *prop;
+	const __be32 *prop;
 	unsigned int psize;
 	struct device_node *parent;
 	struct of_bus *bus;
diff --git a/drivers/of/base.c b/drivers/of/base.c
index aa80525..710b53b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -33,7 +33,7 @@ DEFINE_RWLOCK(devtree_lock);
 
 int of_n_addr_cells(struct device_node *np)
 {
-	const int *ip;
+	const __be32 *ip;
 
 	do {
 		if (np->parent)
@@ -49,7 +49,7 @@ EXPORT_SYMBOL(of_n_addr_cells);
 
 int of_n_size_cells(struct device_node *np)
 {
-	const int *ip;
+	const __be32 *ip;
 
 	do {
 		if (np->parent)

Re: [PATCH] drivers/of: use __be32 types for big-endian device tree data

[Timur Tabi]

On Mon, Sep 13, 2010 at 10:13 PM, Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
> Use the sparse annotations so we can keep track of endianness.
>
> Signed-off-by: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

Does this mean that I should be using be32_to_cpu() whenever I
dereference a pointer to a u32 in the device tree?  Currently, I do
this:

const u32 *iprop;
const u32 num;
iprop = of_get_property(np, ...)
num = *iprop;

Should I be doing this instead?

const __be32 *iprop;
const u32 num;
iprop = of_get_property(np, ...)
num = be32_to_cpu(*iprop);

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] drivers/of: use __be32 types for big-endian device tree data

[David Gibson]

On Mon, Sep 13, 2010 at 10:20:11PM -0500, Timur Tabi wrote:
> On Mon, Sep 13, 2010 at 10:13 PM, Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
> > Use the sparse annotations so we can keep track of endianness.
> >
> > Signed-off-by: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> 
> Does this mean that I should be using be32_to_cpu() whenever I
> dereference a pointer to a u32 in the device tree?  Currently, I do
> this:
> 
> const u32 *iprop;
> const u32 num;
> iprop = of_get_property(np, ...)
> num = *iprop;
> 
> Should I be doing this instead?
> 
> const __be32 *iprop;
> const u32 num;
> iprop = of_get_property(np, ...)
> num = be32_to_cpu(*iprop);

Yes.

Although, we should probably add a get_intprop() or similar helper
that will do this for you.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH] drivers/of: use __be32 types for big-endian device tree data

[Jeremy Kerr]

Hi Timur,

> Does this mean that I should be using be32_to_cpu() whenever I
> dereference a pointer to a u32 in the device tree?  Currently, I do
> this:
> 
> const u32 *iprop;
> const u32 num;
> iprop = of_get_property(np, ...)
> num = *iprop;
> 
> Should I be doing this instead?
> 
> const __be32 *iprop;
> const u32 num;
> iprop = of_get_property(np, ...)
> num = be32_to_cpu(*iprop);

 
If your code will be run on little-endian platforms, you'll need to do
this, yes. It may also be prudent to do this on BE platforms too, as
we'll undoubtedly have someone copy code from one to another. But I
don't think anyone is suggesting that we modify existing BE-only code.

Also, in the example you have given, I think be32_to_cpup(iprop) is
preferred.

Cheers,


Jeremy

Re: [PATCH] drivers/of: use __be32 types for big-endian device tree data

[Grant Likely]

On Tue, Sep 14, 2010 at 11:13:51AM +0800, Jeremy Kerr wrote:
> Use the sparse annotations so we can keep track of endianness.
> 
> Signed-off-by: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

Added to my test-devicetree branch

g.

> 
> ---
>  drivers/of/address.c |    2 +-
>  drivers/of/base.c    |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index fcadb72..3a1c7e7 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -163,7 +163,7 @@ static int of_bus_pci_translate(u32 *addr, u64 offset, int na)
>  const u32 *of_get_pci_address(struct device_node *dev, int bar_no, u64 *size,
>  			unsigned int *flags)
>  {
> -	const u32 *prop;
> +	const __be32 *prop;
>  	unsigned int psize;
>  	struct device_node *parent;
>  	struct of_bus *bus;
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index aa80525..710b53b 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -33,7 +33,7 @@ DEFINE_RWLOCK(devtree_lock);
>  
>  int of_n_addr_cells(struct device_node *np)
>  {
> -	const int *ip;
> +	const __be32 *ip;
>  
>  	do {
>  		if (np->parent)
> @@ -49,7 +49,7 @@ EXPORT_SYMBOL(of_n_addr_cells);
>  
>  int of_n_size_cells(struct device_node *np)
>  {
> -	const int *ip;
> +	const __be32 *ip;
>  
>  	do {
>  		if (np->parent)