On Mar 21, 2011, at 16:30, Wolfgang Denk wrote: > In message <5b9d9c87-c278-4af3-b20c-26ecff6c0...@boeing.com> you wrote: >> >>> WARNING: line over 80 characters >>> #463: FILE: board/exmeritus/hww1u1a/hww1u1a.c:136: >>> +int do_hww1u1a_test_cpu_a(cmd_tbl_t *cmdtp, int flag, int argc, char * >>> const argv[]) >> >> This one is only a warning, and it's much more readable to have 1 84-charac > > In U-Boot, it is considered to be an ERROR.
Just looking at the last ~200 commits (actually 187, because it ignores merges): $ git format-patch -o recent-patches -200 origin/master $ ./checkpatch.pl --no-tree --strict recent-patches/* >checkpatch.log 2>&1 $ grep 'over 80 char' checkpatch.log | wc -l 130 That's 130 lines in the last 200 patches which are over 80 characters?!?! How are those patches any different from mine? $ grep '^ERROR:' checkpatch.log | wc -l 113 And that's 113 HARD ERRORS from checkpatch!?!?! Of those, 32 are "Missing Signed-off-by: line(s)", 20 are "macros with complex values should be enclosed in parenthesis", 19 are inconsistent or missing whitespace issues, 4 are "(foo*) instead of "(foo *)", 3 are invalid UTF-8 errors, 4 are "return is not a function" errors, etc, etc. Look, I'm really trying to comply with U-Boot coding standards, but I'm really of pissed off about the inconsistent requirements you are applying to my patches versus a lot of other things that YOU ARE MERGING on a regular basis. So why are you picking on my board-specific code so hard here? This is extremely frustrating for me and a strong deterrent against us *ever* contributing to U-Boot again in the future. >> ter line than split it across 2 different lines. Even still, this >> warning is only issued if you pass "--subjective" to checkpatch, which >> is documented to "enable more subjective tests". > > No, I get this warning without such flags. The command I run is just > "checkpatch.pl --no-tree" What version of checkpatch are you running? I copied version 0.31 out of my latest Linux kernel tree, which identical to the latest version from here: http://www.kernel.org/pub/linux/kernel/people/apw/checkpatch/ If U-Boot policy is to run checkpatch then you'd better either specify a particular version and command-line options or be willing to accept the default output of the latest version. > BTW - could you please restrict your line length to some 70 characters > or so? Thanks. Sorry about that, sending email through an Exchange server is no fun :-(. Hopefully I've got it fixed. >>>> +U_BOOT_CMD( >>>> + hww1u1a_test_cpu_a, 1, 0, do_hww1u1a_test_cpu_a, >>>> + "Test if this is CPU A (versus B) on the eXMeritus HWW-1U-1A board", >>>> + /* */" && <command-if-true>\n" >>>> + "hww1u1a_test_cpu_a || <command-if-false>\n" >>> =20 >>> What is this empty comment needed for? >> >> Just a mental placeholder for the fact that the U_BOOT_CMD macro inserts >> the name of the command in that spot. Will remove. > > We don't provide usage examples in the help text. This should be > fixed in the first place. This *IS* the help text, and not a sample usage. This is visually and effectively no different from this text in common/cmd_mp.c: U_BOOT_CMD( cpu, CONFIG_SYS_MAXARGS, 1, cpu_cmd, "Multiprocessor CPU boot manipulation and release", "<num> reset - Reset cpu <num>\n" "cpu <num> status - Status of cpu <num>\n" "cpu <num> disable - Disable cpu <num>\n" "cpu <num> release <addr> [args] - Release cpu <num> at <addr> with [args]" #ifdef CPU_ARCH_HELP "\n" CPU_ARCH_HELP #endif ); >>>> + /* Turn on the "HRESET_REQ" pin (hard-reset request) */ >>>> + printf("\nRESET: Hardware reset triggered, waiting...\n"); >>>> + out_be32(&gur->rstcr, 0x2); >>>> + while (1) >>>> + udelay(10000); >>>> + } >>> Should that not be an infinite wait here? >> >> At this point if the board does not reset due to hardware failure it's >> better off hanging than silently falling through. > > Why don't you simply call "hang()" then? Didn't know it existed at the time the code was written. Will fix. Cheers, Kyle Moffett _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
