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

2012-10-25 Thread Tom Rini
On Tue, Oct 16, 2012 at 07:28:16PM -0500, 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.

I want to repeat myself that I want to see sparse fixes come in.  That
said, I expect that for v2 you will have done MAKEALL -a $arch for every
arch there's an ELDK 5.2.x toolchain for, at least, and there's no new
problems popping up.  If you like I can pastebin my MAKEALL wrapper that
lets you tell it things like --arch arm --eldk-521 and it sets up the
environment for the toolchain.

I also needed to not apply patch 01/32 and so I can really only take 27
and 32 right now (I'll reply separately once I really do).

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2012-10-25 Thread Kim Phillips
On Thu, 25 Oct 2012 10:46:54 -0700
Tom Rini tr...@ti.com wrote:

 On Tue, Oct 16, 2012 at 07:28:16PM -0500, 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.
 
 I want to repeat myself that I want to see sparse fixes come in.  That
 said, I expect that for v2 you will have done MAKEALL -a $arch for every
 arch there's an ELDK 5.2.x toolchain for, at least, and there's no new

so power, arm, and mips.

 problems popping up.  If you like I can pastebin my MAKEALL wrapper that
 lets you tell it things like --arch arm --eldk-521 and it sets up the
 environment for the toolchain.

please do.

 I also needed to not apply patch 01/32 and so I can really only take 27
 and 32 right now (I'll reply separately once I really do).

that's fine, thanks.

Kim

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2012-10-25 Thread Tom Rini
On Thu, Oct 25, 2012 at 01:59:45PM -0500, Kim Phillips wrote:
 On Thu, 25 Oct 2012 10:46:54 -0700
 Tom Rini tr...@ti.com wrote:
 
  On Tue, Oct 16, 2012 at 07:28:16PM -0500, 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.
  
  I want to repeat myself that I want to see sparse fixes come in.  That
  said, I expect that for v2 you will have done MAKEALL -a $arch for every
  arch there's an ELDK 5.2.x toolchain for, at least, and there's no new
 
 so power, arm, and mips.

Correct.

  problems popping up.  If you like I can pastebin my MAKEALL wrapper that
  lets you tell it things like --arch arm --eldk-521 and it sets up the
  environment for the toolchain.
 
 please do.

http://pastebin.com/rkRSL2d3 has it now.  It's a little clumsy at times,
for example for not-arm (biased, sorry) you need to do uboot-build.sh
--arch powerpc --eldk-521 --cpu mpc85xx so that it sets up for arch/powerpc,
and then the toolchain and then re-sets itself for cpu/mpc85xx.
arch/cpu/soc default to NCPUS=1 NBUILDS=`grep -c processor
/proc/cpuinfo` and single board targets default to letting MAKEALL guess
right.  I'm pretty sure that even on the bigger boxes this is still
right.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2012-10-24 Thread Tom Rini
On Thu, Oct 18, 2012 at 09:53:36AM -0700, Tom Rini wrote:
 On Tue, Oct 16, 2012 at 07:28:16PM -0500, 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.
  
  Patch 1 adds endianness attributes to byteorder.h helpers, e.g.,
  cpu_to_be32().  This is required for correct operation and
  prevents sparse from emitting false-positives.
  
  Patches 2-6 fix issues where u-boot had imported linux header code
  and the importer simply force-#defined sparse-specific attributes to
  nothing, to allow u-boot to build.
  
  Patches 7-10 are general sparse fixes to common header areas.
  
  Patch 11 is too, which also changes the long-standing u-boot image header
  types to __be32, as per u-boot image definition.
  
  Patches 12-14 address further misc. sparse issues in common/.
  
  Patches 15-16 do the same for the net subsystem.
  
  Patches 17-18 do the same for lib/.
  
  Patch 19 for include/fdt.h.
  
  Patches 20-23 for Power Arch's mpc8xxx, 83xx, and 85xx subsystems.
 
 I've assigned in patchwork some of these patches to the area custodians,
 once reviewed please hand them back to me.  For the rest of the series,
 I'm giving things a read and review.

To this end I'm preparing to merge some of these patches today.
However, even cutting out a number of patches with changes requested
already I'm seeing various failures on MAKEALL --arch arm.  I'll let you
know at what point I stop taking the patches in and what the error is.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2012-10-24 Thread Kim Phillips
On Wed, 24 Oct 2012 14:21:20 -0700
Tom Rini tr...@ti.com wrote:

 On Thu, Oct 18, 2012 at 09:53:36AM -0700, Tom Rini wrote:
  On Tue, Oct 16, 2012 at 07:28:16PM -0500, 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.
   
   Patch 1 adds endianness attributes to byteorder.h helpers, e.g.,
   cpu_to_be32().  This is required for correct operation and
   prevents sparse from emitting false-positives.
   
   Patches 2-6 fix issues where u-boot had imported linux header code
   and the importer simply force-#defined sparse-specific attributes to
   nothing, to allow u-boot to build.
   
   Patches 7-10 are general sparse fixes to common header areas.
   
   Patch 11 is too, which also changes the long-standing u-boot image header
   types to __be32, as per u-boot image definition.
   
   Patches 12-14 address further misc. sparse issues in common/.
   
   Patches 15-16 do the same for the net subsystem.
   
   Patches 17-18 do the same for lib/.
   
   Patch 19 for include/fdt.h.
   
   Patches 20-23 for Power Arch's mpc8xxx, 83xx, and 85xx subsystems.
  
  I've assigned in patchwork some of these patches to the area custodians,
  once reviewed please hand them back to me.  For the rest of the series,
  I'm giving things a read and review.
 
 To this end I'm preparing to merge some of these patches today.
 However, even cutting out a number of patches with changes requested
 already I'm seeing various failures on MAKEALL --arch arm.  I'll let you
 know at what point I stop taking the patches in and what the error is.

I was going to resubmit the whole series after I'd re-done the
libfdt ones to suit the dtc and the kernel libfdt copies, but
patches 1 through 6 are the most important.  Whether I tangled
things up in my rebases, sure, there might be issues; I just wanted
to partition by importance and subsystem, so the subsystem
maintainers can ack/apply freely (as one did).

I will definitely re-submit 14 and 19, and run checkpatch over any
others you don't end up applying.  I'm just caught up doing other
stuff right now...

Kim

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2012-10-18 Thread Tom Rini
On Tue, Oct 16, 2012 at 07:28:16PM -0500, 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.
 
 Patch 1 adds endianness attributes to byteorder.h helpers, e.g.,
 cpu_to_be32().  This is required for correct operation and
 prevents sparse from emitting false-positives.
 
 Patches 2-6 fix issues where u-boot had imported linux header code
 and the importer simply force-#defined sparse-specific attributes to
 nothing, to allow u-boot to build.
 
 Patches 7-10 are general sparse fixes to common header areas.
 
 Patch 11 is too, which also changes the long-standing u-boot image header
 types to __be32, as per u-boot image definition.
 
 Patches 12-14 address further misc. sparse issues in common/.
 
 Patches 15-16 do the same for the net subsystem.
 
 Patches 17-18 do the same for lib/.
 
 Patch 19 for include/fdt.h.
 
 Patches 20-23 for Power Arch's mpc8xxx, 83xx, and 85xx subsystems.

I've assigned in patchwork some of these patches to the area custodians,
once reviewed please hand them back to me.  For the rest of the series,
I'm giving things a read and review.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2012-10-18 Thread David Gibson
On Wed, Oct 17, 2012 at 08:19:23PM -0400, Jerry Van Baren wrote:
 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*.

So, in general I approve the idea of having endian annotations.
Obviously we do need to make sure it doesn't break things.

 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.

This shouldn't be an inherent problem - we can always have the default
behaviour be that be32 etc. are #defined to be uint32_t, and we only
turn on real annotations when we have the right setup.

It does complicate things a bit, but I think it should be manageable.

I'd much prefer to see this done against the upstream dtc/libfdt, of
course, rather than the u-boot copy.

 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

Yes, something along those lines would be appropriate, although I
think that condition isn't right.

   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

This change, however, is not acceptable.  libfdt is supposed to
compile in a wide range of environments (such as bootloaders and
firmwares) which may be very different from a typical Unix userland
environment.  As such all the headers are built to have minimal
dependencies on external libraries.  The byteorder headers are, alas,
horribly non-portable even amongst otherwise similar Unix systems, so
right out for libfdt.

In fact, the way this is supposed to work is that the *only* external
header directly included by the fdt headers is libfdt_env.h.  It is
supplied by the surrounding build environment and is responsible for
providing the minimal things that libfdt does require.  Roughly
speaking that's: stdint like types, some byteswap functions and a some
string.h prototypes - exactly what's required should be better
documented than it currently is.  libfdt_env.h can do this either by
including appropriate pre-existing headers from the environment, or by
directly defining the required things, whichever makes sense.  The
libfdt_env.h which is shipped with libfdt is essentially just an
example, for use in the environment of POSIX like userspace.

Anyway, I think the right way to handle this is to decide on a name
like __FDT_ANNOTATIONS__ or something.  In fdt.h we have

#ifndef __FDT_ANNOTATIONS__
typedef uint32_t fdt32_t;
/* etc */
#endif

And libfdt is reworked to use these fdt32_t and so forth types.  Then,
by default everything should compile fine, but with no extra checking.
Environments which have sparse or a similar checker available can
#define __FDT_ANNOTATIONS__ from their version of libfdt_env.h in
which case they would also be required to define the annotated integer
types as necessary for their checker.

For convenience on Linux systems we can have the default
libfdt_env.h shipped with libfdt go either way depending on SPARSE,
LINUX_TYPES or whatever suitable pre-existing preprocessor 

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

2012-10-18 Thread Kim Phillips
On Thu, 18 Oct 2012 23:11:12 +1100
David Gibson da...@gibson.dropbear.id.au wrote:

 On Wed, Oct 17, 2012 at 08:19:23PM -0400, Jerry Van Baren wrote:
  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*.
 
 So, in general I approve the idea of having endian annotations.
 Obviously we do need to make sure it doesn't break things.

cool.

  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.
 
 This shouldn't be an inherent problem - we can always have the default
 behaviour be that be32 etc. are #defined to be uint32_t, and we only
 turn on real annotations when we have the right setup.
 
 It does complicate things a bit, but I think it should be manageable.
 
 I'd much prefer to see this done against the upstream dtc/libfdt, of
 course, rather than the u-boot copy.

understood.

  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
 
 Yes, something along those lines would be appropriate, although I
 think that condition isn't right.

right, we don't want all uint32_t's to be one endian, we just want
fdt32 types to have big endian annotation when being checked by a
checker such as sparse.

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
 
 This change, however, is not acceptable.  libfdt is supposed to
 compile in a wide range of environments (such as bootloaders and
 firmwares) which may be very different from a typical Unix userland
 environment.  As such all the headers are built to have minimal
 dependencies on external libraries.  The byteorder headers are, alas,
 horribly non-portable even amongst otherwise similar Unix systems, so
 right out for libfdt.
 
 In fact, the way this is supposed to work is that the *only* external
 header directly included by the fdt headers is libfdt_env.h.  It is
 supplied by the surrounding build environment and is responsible for
 providing the minimal things that libfdt does require.  Roughly
 speaking that's: stdint like types, some byteswap functions and a some
 string.h prototypes - exactly what's required should be better
 documented than it currently is.  libfdt_env.h can do this either by
 including appropriate pre-existing headers from the environment, or by
 directly defining the required things, whichever makes sense.  The
 libfdt_env.h which is shipped with libfdt is essentially just an
 example, for use in the environment of POSIX like userspace.
 
 Anyway, I think the right way to handle this is to decide on a name
 like __FDT_ANNOTATIONS__ or something.

this is named __CHECKER__ in linux and u-boot.  Ifdef __CHECKER__,
then the code is being parsed by a checker.

  In fdt.h we have
 
 #ifndef __FDT_ANNOTATIONS__
 typedef uint32_t fdt32_t;
 /* etc */
 #endif
 
 And libfdt is reworked to use these fdt32_t and so forth types.  Then,
 by 

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

2012-10-18 Thread David Gibson
On Thu, Oct 18, 2012 at 05:30:22PM -0500, Kim Phillips wrote:
 On Thu, 18 Oct 2012 23:11:12 +1100
 David Gibson da...@gibson.dropbear.id.au wrote:
 
  On Wed, Oct 17, 2012 at 08:19:23PM -0400, Jerry Van Baren wrote:
   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*.
  
  So, in general I approve the idea of having endian annotations.
  Obviously we do need to make sure it doesn't break things.
 
 cool.
 
   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.
  
  This shouldn't be an inherent problem - we can always have the default
  behaviour be that be32 etc. are #defined to be uint32_t, and we only
  turn on real annotations when we have the right setup.
  
  It does complicate things a bit, but I think it should be manageable.
  
  I'd much prefer to see this done against the upstream dtc/libfdt, of
  course, rather than the u-boot copy.
 
 understood.
 
   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
  
  Yes, something along those lines would be appropriate, although I
  think that condition isn't right.
 
 right, we don't want all uint32_t's to be one endian, we just want
 fdt32 types to have big endian annotation when being checked by a
 checker such as sparse.

Right.

 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
  
  This change, however, is not acceptable.  libfdt is supposed to
  compile in a wide range of environments (such as bootloaders and
  firmwares) which may be very different from a typical Unix userland
  environment.  As such all the headers are built to have minimal
  dependencies on external libraries.  The byteorder headers are, alas,
  horribly non-portable even amongst otherwise similar Unix systems, so
  right out for libfdt.
  
  In fact, the way this is supposed to work is that the *only* external
  header directly included by the fdt headers is libfdt_env.h.  It is
  supplied by the surrounding build environment and is responsible for
  providing the minimal things that libfdt does require.  Roughly
  speaking that's: stdint like types, some byteswap functions and a some
  string.h prototypes - exactly what's required should be better
  documented than it currently is.  libfdt_env.h can do this either by
  including appropriate pre-existing headers from the environment, or by
  directly defining the required things, whichever makes sense.  The
  libfdt_env.h which is shipped with libfdt is essentially just an
  example, for use in the environment of POSIX like userspace.
  
  Anyway, I think the right way to handle this is to decide on a name
  like __FDT_ANNOTATIONS__ or something.
 
 this is named __CHECKER__ in linux and u-boot.  Ifdef __CHECKER__,
 then the code 

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

2012-10-17 Thread Jerry Van Baren
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 

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

2012-10-16 Thread Kim Phillips
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.

Patch 1 adds endianness attributes to byteorder.h helpers, e.g.,
cpu_to_be32().  This is required for correct operation and
prevents sparse from emitting false-positives.

Patches 2-6 fix issues where u-boot had imported linux header code
and the importer simply force-#defined sparse-specific attributes to
nothing, to allow u-boot to build.

Patches 7-10 are general sparse fixes to common header areas.

Patch 11 is too, which also changes the long-standing u-boot image header
types to __be32, as per u-boot image definition.

Patches 12-14 address further misc. sparse issues in common/.

Patches 15-16 do the same for the net subsystem.

Patches 17-18 do the same for lib/.

Patch 19 for include/fdt.h.

Patches 20-23 for Power Arch's mpc8xxx, 83xx, and 85xx subsystems.

Last but not least, patches 24-32 attempt to clean up various drivers.

Note that there are two libfdt dependencies:

1. commits aa28b89b libfdt: Add helpers for 64-bit integer properties and
commit 786ed04 libfdt: Add support for appending the values to a existing
property, currently sitting in u-boot-fdt/next.  This series is based
on today's mainline u-boot/next with those two patches pre-applied.

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 :).

For authors of new patches not willing to sift through the still quite
large sparse output for their board/platform, though interested in what
sparse has to say about their new code, it is recommended to build
with make C=1 after changes such that sparse output will only include
newly built code.

For your convenience, I've pushed the series onto the u-boot-mpc83xx
tree, 'sparsefixes' branch [1].  The branch includes libfdt dependency
#1 mentioned above.

Build-tested in both endians :).  Please help test.

Thanks,

Kim

[1] see 
http://git.denx.de/u-boot.git/?p=u-boot/u-boot-mpc83xx.git;a=shortlog;h=refs/heads/sparsefixes

Kim Phillips (32):
  include/linux/byteorder: import latest endian definitions from linux
  include/linux/compat.h: fix warning: preprocessor token __iomem
redefined
  include/linux/unaligned/generic.h: fix warning: preprocessor token
__force redefined
  include/linux/stddef.h: avoid 'warning: preprocessor token offsetof
redefined'
  arch/powerpc/include/asm/io.h: fix warning: preprocessor token
__iomem redefined
  arch/powerpc/lib/bootm.c: fix noinline attribute
  arch/powerpc/lib/extable.c: sparse fix
  arch/powerpc/lib/board.c,*traps.c: sparse fixes
  include/common.h: sparse fix
  include/command.h: sparse fix
  include/image.h: sparse fixes
  common/cmd_*.c: sparse fixes
  common/misc: sparse fixes
  common/fdt_support.c: sparse fixes
  net/: sparse fixes
  drivers/net/: sparse fixes
  lib/zlib: sparse fixes
  lib/vsprintf.c: sparse fixes
  include/fdt.h: sparse fixes
  arch/powerpc/cpu/mpc8xxx/: sparse fixes
  arch/powerpc/cpu/mpc85xx/fdt.c: sparse fixes
  powerpc/mpc85xx: sparse fixes
  powerpc/mpc83xx: sparse fixes
  drivers/block/: sparse fixes
  drivers/gpio/mpc83xx_gpio.c: sparse fixes
  drivers/input/input.c: sparse fix
  drivers/i2c/fsl_i2c.c: sparse fix
  drivers/mmc/mmc.c: sparse fixes
  drivers/mmc/fsl_esdhc.c: sparse fixes
  drivers/mtd/cfi_flash.c: sparse fixes
  drivers/mtd/nand: sparse fixes
  drivers/serial/serial_ns16550.c: sparse fixes

 arch/powerpc/cpu/74xx_7xx/traps.c  |   6 +-
 arch/powerpc/cpu/mpc512x/traps.c   |   6 +-
 arch/powerpc/cpu/mpc5xx/traps.c|   6 +-
 arch/powerpc/cpu/mpc5xxx/traps.c   |   6 +-
 arch/powerpc/cpu/mpc8220/traps.c   |   6 +-
 arch/powerpc/cpu/mpc824x/traps.c   |   6 +-
 arch/powerpc/cpu/mpc8260/traps.c   |   6 +-
 arch/powerpc/cpu/mpc83xx/fdt.c |   4 +-
 arch/powerpc/cpu/mpc83xx/speed.c   |   4 +-
 arch/powerpc/cpu/mpc83xx/traps.c   |   6 +-
 arch/powerpc/cpu/mpc85xx/cpu_init.c|   2 +-
 arch/powerpc/cpu/mpc85xx/fdt.c |  57 +
 arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c  |   2 +-
 arch/powerpc/cpu/mpc85xx/traps.c   |   6 +-
 arch/powerpc/cpu/mpc86xx/traps.c   |   6 +-
 arch/powerpc/cpu/mpc8xx/traps.c|   6 +-
 arch/powerpc/cpu/mpc8xxx/cpu.c |   8 +-