[U-Boot] [PATCH v6 4/4] mpc85xx: Add board support for the eXMeritus HWW-1U-1A devices

[Wolfgang Denk]

Dear "Moffett, Kyle D",

In message <AC0C0781-9E5F-42F7-9DB6-EECF6A5BE840 at boeing.com> you wrote:
>
> Just looking at the last ~200 commits (actually 187, because it ignores mer=
> ges):
> 
> $ 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?

The difference is: They were not detected.

Patches welcome.

> Look, I'm really trying to comply with U-Boot coding standards, but I'm rea=
> lly
> 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 basi=
> s.

The requirements are NOT inconsistent.  It's just that nobody is
perfect, and nobody ever claimed that we manage to get 100% of review
coverage.

> So why are you picking on my board-specific code so hard here?  This is

It's just that the problems got noticed.

> > "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:

-> checkpatch.pl --version
Usage: checkpatch.pl [OPTION]... [FILE]...
Version: 0.31
...

> 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.

I don't see any version issue here, nor do I use any non-standard
options.

> >>>> +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:

The synopsis of a command gives the command name and possible options,
and explains what the command does and what the options do.

"name && <command-if-true>" does NOT fall into that pattern.

Look, alternatively I can claim your help message is highly incomplete
as it fails to cover use cases like

	name || <command-if-false>

etc.

Not to mention that the usage message is plain wrong for all boards
that don't have the hush shell enabled.

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
Ninety-Ninety Rule of Project Schedules:
        The first ninety percent of the task takes ninety percent of
the time, and the last ten percent takes the other ninety percent.