Re: [U-Boot] [PATCH v5 01/20] sandbox: Add architecture header files

2011-10-09 Thread Wolfgang Denk
Dear Simon Glass,

In message 1318031631-13643-2-git-send-email-...@chromium.org you wrote:
 This adds required header files for the sandbox architecture, and a basic
 description of what sandbox is (README.sandbox).
 
 This commit generates a list of 44 checkpatch warnings:

This should go to the comment section.   I don't want to see this as
part of the commit message.

 0 errors, 44 warnings for 0001-sandbox-Add-architecture-header-files.patch:
 warning: arch/sandbox/include/asm/bitops.h,30: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,32: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,34: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,36: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,44: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,54: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,66: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,76: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,88: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,90: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 
 (These are the bitops and seem to have volatile in all the other archs also.)

Existence of bad code examples is no excuse for submitting new bad
code.

...
 +/*
 + * Function prototypes to keep gcc -Wall happy.
 + */
 +extern void set_bit(int nr, volatile void *addr);
 +
 +extern void clear_bit(int nr, volatile void *addr);
 +
 +extern void change_bit(int nr, volatile void *addr);

I see no reason to accept these voplatiles here.

 +static inline void __change_bit(int nr, volatile void *addr)
 +{
 + unsigned long mask = BIT_MASK(nr);
 + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
 +
 + *p ^= mask;

Please note that you actually even drop the volatile property in your
implementation.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
There is no statute of limitations on stupidity.
- Randomly produced by a computer program called Markov3.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v5 01/20] sandbox: Add architecture header files

2011-10-09 Thread Simon Glass
Hi Wolfgang,

On Sun, Oct 9, 2011 at 12:28 PM, Wolfgang Denk w...@denx.de wrote:
 Dear Simon Glass,

 In message 1318031631-13643-2-git-send-email-...@chromium.org you wrote:
 This adds required header files for the sandbox architecture, and a basic
 description of what sandbox is (README.sandbox).

 This commit generates a list of 44 checkpatch warnings:

 This should go to the comment section.   I don't want to see this as
 part of the commit message.

Neither do I, but I guessed another patch version was coming. I will move it.


 0 errors, 44 warnings for 0001-sandbox-Add-architecture-header-files.patch:
 warning: arch/sandbox/include/asm/bitops.h,30: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,32: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,34: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,36: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,44: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,54: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,66: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,76: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,88: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt
 warning: arch/sandbox/include/asm/bitops.h,90: Use of volatile is usually 
 wrong: see Documentation/volatile-considered-harmful.txt

 (These are the bitops and seem to have volatile in all the other archs also.)

 Existence of bad code examples is no excuse for submitting new bad
 code.

OK. I will drop volatile from the patch. Are you OK with the other
checkpatch problems?

Regards,
Simon


 ...
 +/*
 + * Function prototypes to keep gcc -Wall happy.
 + */
 +extern void set_bit(int nr, volatile void *addr);
 +
 +extern void clear_bit(int nr, volatile void *addr);
 +
 +extern void change_bit(int nr, volatile void *addr);

 I see no reason to accept these voplatiles here.

 +static inline void __change_bit(int nr, volatile void *addr)
 +{
 +     unsigned long mask = BIT_MASK(nr);
 +     unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
 +
 +     *p ^= mask;

 Please note that you actually even drop the volatile property in your
 implementation.


 Best regards,

 Wolfgang Denk

 --
 DENX Software Engineering GmbH,     MD: Wolfgang Denk  Detlev Zundel
 HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
 Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
 There is no statute of limitations on stupidity.
 - Randomly produced by a computer program called Markov3.

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


Re: [U-Boot] [PATCH v5 01/20] sandbox: Add architecture header files

2011-10-09 Thread Mike Frysinger
On Sunday 09 October 2011 15:28:16 Wolfgang Denk wrote:
 Simon Glass wrote:
  0 errors, 44 warnings for
  0001-sandbox-Add-architecture-header-files.patch: warning:
  arch/sandbox/include/asm/bitops.h,30: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,32: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,34: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,36: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,44: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,54: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,66: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,76: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,88: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,90: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt
  
  (These are the bitops and seem to have volatile in all the other archs
  also.)
 
 Existence of bad code examples is no excuse for submitting new bad
 code.

maybe we should take this up with LKML rather than forcing our code to 
randomly fork ...
-mike


signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v5 01/20] sandbox: Add architecture header files

2011-10-09 Thread Simon Glass
Hi,

On Sun, Oct 9, 2011 at 6:03 PM, Mike Frysinger vap...@gentoo.org wrote:
 On Sunday 09 October 2011 15:28:16 Wolfgang Denk wrote:
 Simon Glass wrote:
  0 errors, 44 warnings for
  0001-sandbox-Add-architecture-header-files.patch: warning:
  arch/sandbox/include/asm/bitops.h,30: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,32: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,34: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,36: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,44: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,54: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,66: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,76: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,88: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt warning:
  arch/sandbox/include/asm/bitops.h,90: Use of volatile is usually wrong:
  see Documentation/volatile-considered-harmful.txt
 
  (These are the bitops and seem to have volatile in all the other archs
  also.)

 Existence of bad code examples is no excuse for submitting new bad
 code.

 maybe we should take this up with LKML rather than forcing our code to
 randomly fork ...
 -mike


Maybe, but I suspect they would take a patch, given that their own
checkpatch complains about it.

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


[U-Boot] [PATCH v5 01/20] sandbox: Add architecture header files

2011-10-07 Thread Simon Glass
This adds required header files for the sandbox architecture, and a basic
description of what sandbox is (README.sandbox).

This commit generates a list of 44 checkpatch warnings:

0 errors, 44 warnings for 0001-sandbox-Add-architecture-header-files.patch:
warning: arch/sandbox/include/asm/bitops.h,30: Use of volatile is usually 
wrong: see Documentation/volatile-considered-harmful.txt
warning: arch/sandbox/include/asm/bitops.h,32: Use of volatile is usually 
wrong: see Documentation/volatile-considered-harmful.txt
warning: arch/sandbox/include/asm/bitops.h,34: Use of volatile is usually 
wrong: see Documentation/volatile-considered-harmful.txt
warning: arch/sandbox/include/asm/bitops.h,36: Use of volatile is usually 
wrong: see Documentation/volatile-considered-harmful.txt
warning: arch/sandbox/include/asm/bitops.h,44: Use of volatile is usually 
wrong: see Documentation/volatile-considered-harmful.txt
warning: arch/sandbox/include/asm/bitops.h,54: Use of volatile is usually 
wrong: see Documentation/volatile-considered-harmful.txt
warning: arch/sandbox/include/asm/bitops.h,66: Use of volatile is usually 
wrong: see Documentation/volatile-considered-harmful.txt
warning: arch/sandbox/include/asm/bitops.h,76: Use of volatile is usually 
wrong: see Documentation/volatile-considered-harmful.txt
warning: arch/sandbox/include/asm/bitops.h,88: Use of volatile is usually 
wrong: see Documentation/volatile-considered-harmful.txt
warning: arch/sandbox/include/asm/bitops.h,90: Use of volatile is usually 
wrong: see Documentation/volatile-considered-harmful.txt

(These are the bitops and seem to have volatile in all the other archs also.)

warning: arch/sandbox/include/asm/global_data.h,38: do not add new typedefs

(This is the typedef for gd_t which I think is needed.)

warning: arch/sandbox/include/asm/global_data.h,64: storage class should be at 
the beginning of the declaration

(This line is:

which I need to keep, so that various files can declare that pointer.)

warning: arch/sandbox/include/asm/posix_types.h,22: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,23: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,24: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,25: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,26: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,27: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,28: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,29: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,30: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,32: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,33: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,34: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,36: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,37: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,38: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,40: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,41: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,42: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,43: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,44: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,45: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,46: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,47: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,48: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,50: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,51: do not add new typedefs
warning: arch/sandbox/include/asm/posix_types.h,54: do not add new typedefs

(These are things like:

typedef unsigned short  __kernel_dev_t;

and I believe they are needed.)

warning: arch/sandbox/include/asm/types.h,26: do not add new typedefs
warning: arch/sandbox/include/asm/types.h,66: do not add new typedefs
warning: arch/sandbox/include/asm/types.h,67: do not add new typedefs
warning: arch/sandbox/include/asm/types.h,68: do not add new typedefs
warning: arch/sandbox/include/asm/u-boot.h,39: do not add new typedefs

(These are similar, and common with other architectures, so I think they are 
needed.)

Signed-off-by: Simon Glass s...@chromium.org
---
Changes in v2:
- Removed unneeded clock.h
- Moved gpio.h to asm-generic, removed GPIO_COUNT
- Removed kernel cruft from posix_types.h
- Removed dummy field from struct pt_regs
- Remove contents of string.h; instead include linux/string.h
- Remove unneeded functions in u-boot-sandbox.h
- Remove contents of unaligned.h; instead include