[U-Boot] checkpatch compliance

[Joe Hershberger]

On Sun, Oct 9, 2011 at 2:22 PM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Joe Hershberger,
>
> In message <CANr=Z=Y+Q6AoJ-+gkc+YDeAPGcLsaQkPCzAmhJcRNwts9jXJzw at 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