On Sun, Oct 9, 2011 at 2:22 PM, Wolfgang Denk <[email protected]> wrote: > Dear Joe Hershberger, > > In message > <CANr=Z=y+q6aoj-+gkc+ydeapgclsaqkpczamhjcrnwts9jx...@mail.gmail.com> you > wrote: >> >> I'm attempting to make the files I touched in several recent >> patch-series chechkpatch.pl compliant. >> >> I've hit several cases which fail and probably shouldn't. For each of >> these cases, should the warning / error just be ignored or reported to >> checkpatch maintainers or altered some other way? > > Please see this message / thread: > > http://thread.gmane.org/gmane.linux.kernel/1130494/focus=1172475 > > It would be best if we > 1) copied the current checkpatch.pl from Linux to tools/checkpatch.pl > (which would also make it easier for all to use the same version) > and > 2) provide a customized U-Boot specific .config file that takes care > about things like the ones you list.
Done. >> ERROR: need consistent spacing around '/' (ctx:WxV) >> +#define CONFIG_ROOTPATH /nfs/root/path > > Actually this is IMO wrong. Should it not be "/nfs/root/path" > instead? Potentially it should be a defined as string literal in the future. It is currently expected to not be a string in common/env_embedded.c, common/env_common.c, and tools/env/fw_env.c where it uses MK_STR. Potentially a future cleanup could change all of these references, however it would break any external definitions of CONFIG_ROOTPATH that may be passed in from the environment (expecting the MK_STR). > >> ERROR: Macros with complex values should be enclosed in parenthesis >> +#define CONFIG_UBOOTPATH u-boot.bin > > Ditto here. This seems to be local to these files and can be made into a string literal. >> >> ERROR: Macros with complex values should be enclosed in parenthesis >> #218: FILE: include/configs/MPC8313ERDB.h:274: >> +#define CONFIG_SYS_NAND_BR_PRELIM (CONFIG_SYS_NAND_BASE \ >> | (2<<BR_DECC_SHIFT) /* Use HW ECC */ \ >> - | BR_PS_8 /* Port Size = >> 8 bit */ \ >> + | BR_PS_8 /* 8 bit port */ \ >> | BR_MS_FCM /* MSEL = FCM */ \ >> - | BR_V ) /* valid */ >> -#define CONFIG_SYS_NAND_OR_PRELIM ( 0xFFFF8000 /* >> length 32K */ \ >> + | BR_V) /* valid */ > > I think this one should actually be reported. Done. -Joe _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
