[U-Boot] [PATCH 00/32] Initial sparse fix series

[Jerry Van Baren <gvb.uboot@gmail.com>]

Hi David, Jon,

Kim Phillips created a series of patches to change variable declarations
that are big endian to be __be32/__be64.  Since the device tree is
defined to be big endian, he created a patch to mark the appropriate
libfdt entities as __be*.

On 10/16/2012 08:28 PM, Kim Phillips wrote:
> This 32-patch series only begins to address making u-boot source more
> 'sparseable,' or sparse-clean, ultimately to catch type, address space,
> and endianness mismatches and generally improve code quality. E.g., in this
> initial dose whose main purpose is to reduce the output volume to workable
> levels, a couple of endianness bugs are found and fixed in
> of_bus_default_translate() and fdt_get_base_address().  See [PATCH 14/32]
> common/fdt_support.c: sparse fixes.

Upside: This is very good for identifying endian errors early.
Downside: It could break/complicate non-linux uses of libfdt.

What are your thoughts on this quest?

[snip]

> Note that there are two libfdt dependencies:

[snipped #1, u-boot-fdt dependency, NBD]

> 2. potential upstream dtc change dependencies, due to having to attribute base
> device tree header types to __be32 in include/libfdt.  See patch 19/32
> "include/fdt.h: sparse fixes".  It is unknown whether such changes would
> be welcome to dtc (but there's a way to find out :).

[snip]

> Build-tested in both endians :).  Please help test.
> 
> Thanks,
> 
> Kim

Below is the actual patch for reference (it was in a separate email).
The impact in terms of changed lines is pretty small.  I'm not sure how
this impacts non-linux / non-gcc systems since the sparse checker comes
from a linux background and uses gcc extensions.

Possibly this could be handled a definition:

#ifndef _LINUX_TYPES_H
typedef uint32_t __be32
typedef uint64_t __be64
#endif

>  include/fdt.h         | 33 +++++++++++++++++----------------
>  include/fdt_support.h |  2 ++
>  include/libfdt.h      |  4 ++--
>  lib/libfdt/fdt.c      |  2 +-
>  lib/libfdt/fdt_ro.c   |  2 +-
>  lib/libfdt/fdt_rw.c   |  4 ++--
>  lib/libfdt/fdt_sw.c   |  4 ++--
>  lib/libfdt/fdt_wip.c  |  2 +-
>  8 files changed, 28 insertions(+), 25 deletions(-)
> 
> diff --git a/include/fdt.h b/include/fdt.h
> index c51212e..1b7f044 100644
> --- a/include/fdt.h
> +++ b/include/fdt.h
> @@ -2,40 +2,41 @@
>  #define _FDT_H
>  
>  #ifndef __ASSEMBLY__
> +#include <asm/byteorder.h>
>  
>  struct fdt_header {
> -	uint32_t magic;			 /* magic word FDT_MAGIC */
> -	uint32_t totalsize;		 /* total size of DT block */
> -	uint32_t off_dt_struct;		 /* offset to structure */
> -	uint32_t off_dt_strings;	 /* offset to strings */
> -	uint32_t off_mem_rsvmap;	 /* offset to memory reserve map */
> -	uint32_t version;		 /* format version */
> -	uint32_t last_comp_version;	 /* last compatible version */
> +	__be32 magic;			 /* magic word FDT_MAGIC */
> +	__be32 totalsize;		 /* total size of DT block */
> +	__be32 off_dt_struct;		 /* offset to structure */
> +	__be32 off_dt_strings;	 /* offset to strings */
> +	__be32 off_mem_rsvmap;	 /* offset to memory reserve map */
> +	__be32 version;		 /* format version */
> +	__be32 last_comp_version;	 /* last compatible version */
>  
>  	/* version 2 fields below */
> -	uint32_t boot_cpuid_phys;	 /* Which physical CPU id we're
> +	__be32 boot_cpuid_phys;	 /* Which physical CPU id we're
>  					    booting on */
>  	/* version 3 fields below */
> -	uint32_t size_dt_strings;	 /* size of the strings block */
> +	__be32 size_dt_strings;	 /* size of the strings block */
>  
>  	/* version 17 fields below */
> -	uint32_t size_dt_struct;	 /* size of the structure block */
> +	__be32 size_dt_struct;	 /* size of the structure block */
>  };
>  
>  struct fdt_reserve_entry {
> -	uint64_t address;
> -	uint64_t size;
> +	__be64 address;
> +	__be64 size;
>  };
>  
>  struct fdt_node_header {
> -	uint32_t tag;
> +	__be32 tag;
>  	char name[0];
>  };
>  
>  struct fdt_property {
> -	uint32_t tag;
> -	uint32_t len;
> -	uint32_t nameoff;
> +	__be32 tag;
> +	__be32 len;
> +	__be32 nameoff;
>  	char data[0];
>  };
>  
> diff --git a/include/fdt_support.h b/include/fdt_support.h
> index 2e18d82..399b73f 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -81,7 +81,9 @@ int fdt_pci_dma_ranges(void *blob, int phb_off, struct pci_controller *hose);
>  #ifdef CONFIG_OF_BOARD_SETUP
>  void ft_board_setup(void *blob, bd_t *bd);
>  void ft_cpu_setup(void *blob, bd_t *bd);
> +void ft_fixup_num_cores(void *blob);
>  void ft_pci_setup(void *blob, bd_t *bd);
> +void ft_srio_setup(void *blob);
>  #endif
>  
>  void set_working_fdt_addr(void *addr);
> diff --git a/include/libfdt.h b/include/libfdt.h
> index 4589c5f..ab98d23 100644
> --- a/include/libfdt.h
> +++ b/include/libfdt.h
> @@ -1153,8 +1153,8 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
>  static inline int fdt_setprop_u32(void *fdt, int nodeoffset, const char *name,
>  				  uint32_t val)
>  {
> -	val = cpu_to_fdt32(val);
> -	return fdt_setprop(fdt, nodeoffset, name, &val, sizeof(val));
> +	__be32 tmp = cpu_to_fdt32(val);
> +	return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
>  }
>  
>  /**
> diff --git a/lib/libfdt/fdt.c b/lib/libfdt/fdt.c
> index 4157b21..5a82c60 100644
> --- a/lib/libfdt/fdt.c
> +++ b/lib/libfdt/fdt.c
> @@ -96,7 +96,7 @@ const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
>  
>  uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
>  {
> -	const uint32_t *tagp, *lenp;
> +	const __be32 *tagp, *lenp;
>  	uint32_t tag;
>  	int offset = startoffset;
>  	const char *p;
> diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c
> index 1933010..4e3d87a 100644
> --- a/lib/libfdt/fdt_ro.c
> +++ b/lib/libfdt/fdt_ro.c
> @@ -326,7 +326,7 @@ const void *fdt_getprop(const void *fdt, int nodeoffset,
>  
>  uint32_t fdt_get_phandle(const void *fdt, int nodeoffset)
>  {
> -	const uint32_t *php;
> +	const __be32 *php;
>  	int len;
>  
>  	/* FIXME: This is a bit sub-optimal, since we potentially scan
> diff --git a/lib/libfdt/fdt_rw.c b/lib/libfdt/fdt_rw.c
> index 5ed23d6..688b817 100644
> --- a/lib/libfdt/fdt_rw.c
> +++ b/lib/libfdt/fdt_rw.c
> @@ -343,7 +343,7 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset,
>  	int nodelen;
>  	int err;
>  	uint32_t tag;
> -	uint32_t *endtag;
> +	__be32 *endtag;
>  
>  	FDT_RW_CHECK_HEADER(fdt);
>  
> @@ -370,7 +370,7 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset,
>  	nh->tag = cpu_to_fdt32(FDT_BEGIN_NODE);
>  	memset(nh->name, 0, FDT_TAGALIGN(namelen+1));
>  	memcpy(nh->name, name, namelen);
> -	endtag = (uint32_t *)((char *)nh + nodelen - FDT_TAGSIZE);
> +	endtag = (__be32 *)((char *)nh + nodelen - FDT_TAGSIZE);
>  	*endtag = cpu_to_fdt32(FDT_END_NODE);
>  
>  	return offset;
> diff --git a/lib/libfdt/fdt_sw.c b/lib/libfdt/fdt_sw.c
> index 55ebebf..5ce91ba 100644
> --- a/lib/libfdt/fdt_sw.c
> +++ b/lib/libfdt/fdt_sw.c
> @@ -153,7 +153,7 @@ int fdt_begin_node(void *fdt, const char *name)
>  
>  int fdt_end_node(void *fdt)
>  {
> -	uint32_t *en;
> +	__be32 *en;
>  
>  	FDT_SW_CHECK_HEADER(fdt);
>  
> @@ -213,7 +213,7 @@ int fdt_property(void *fdt, const char *name, const void *val, int len)
>  int fdt_finish(void *fdt)
>  {
>  	char *p = (char *)fdt;
> -	uint32_t *end;
> +	__be32 *end;
>  	int oldstroffset, newstroffset;
>  	uint32_t tag;
>  	int offset, nextoffset;
> diff --git a/lib/libfdt/fdt_wip.c b/lib/libfdt/fdt_wip.c
> index e373677..dc1e14c 100644
> --- a/lib/libfdt/fdt_wip.c
> +++ b/lib/libfdt/fdt_wip.c
> @@ -78,7 +78,7 @@ int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name,
>  
>  static void _fdt_nop_region(void *start, int len)
>  {
> -	uint32_t *p;
> +	__be32 *p;
>  
>  	for (p = start; (char *)p < ((char *)start + len); p++)
>  		*p = cpu_to_fdt32(FDT_NOP);