[U-Boot] [PATCH] powerpc/85xx: Add basic support for P1023RDS board

Wolfgang Denk wd at denx.de
Wed Jun 8 11:34:46 CEST 2011


Dear Zang Roy-R61911,


[excessive Cc: list trimmed]

In message <2239AC579C7D3646A720227A37E0268115768A at 039-SN1MPN1-004.039d.mgd.msft.net> you wrote:
> 
> > Please fix the checkpatch warnings (7 lines over 80 characters)
> 
> I can fix it. In fact, I use checkpatch every time before I send out patch.

Then I do not understand why you submitted the patch at all before
cleaning it up.

> If you check the board header file, most of them has this problem.

Cleanup patches are welcome.

> Do you think 
> + /* physical addr of CCSRBAR */
> +#define CONFIG_SYS_CCSRBAR_PHYS        CONFIG_SYS_CCSRBAR
> is better than
> +#define CONFIG_SYS_CCSRBAR_PHYS        CONFIG_SYS_CCSRBAR      /* physical
>  addr of CCSRBAR */
> ???
> Please clarify.

At least the first version does not violate coding style requirements.

Regarding the general topic of comments - there are such and such
comments.  Things like

	i = i + 1;	/* Increment variable i by one */

are gratuitous and of no use at all.

In your example above, I ask myself why we need
CONFIG_SYS_CCSRBAR_PHYS at all when whe already have
CONFIG_SYS_CCSRBAR (or vice versa).

And why we need a comment that explains this in grat detail - separate
for each single macro.

Please re-read "Chapter 8: Commenting" in the coding-style document
for more details.


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: wd at denx.de
The philosophy exam was a piece of cake  -  which  was  a  bit  of  a
surprise, actually, because I was expecting some questions on a sheet
of paper.


More information about the U-Boot mailing list