Re: [U-Boot] [PATCH v5 01/20] sandbox: Add architecture header files
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
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
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
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
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