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

2011-03-21 Thread Moffett, Kyle D 
Wolfgang,

Thanks for your detailed reviews!

Once I get these last few style issues resolved, what more do I need to do to 
get this merged?  I don't really want to spam the list with more nearly 
identical copies of these patches unless I'm sure that all the necessary review 
items have been taken care of.


On Mar 15, 2011, at 15:36, Wolfgang Denk wrote:
 In message 1300208664-18339-5-git-send-email-kyle.d.moff...@boeing.com you 
 wrote:
 The eXMeritus HWW-1U-1A unit is a DO-160-certified 13lb 1U chassis
 with 3 independent TEMPEST zones.  Two independent P2020 computers may
 be found inside each zone.  Complete hardware support is included.
 
 Please run checkpatch on your submissions!

I did run it on this patch, although I forgot to run it on the resurrected 
reset patch (which is also now fixed).  It's a common gripe on the LKML that 
the tool is overzealous about certain warnings in cases where the fix makes 
the code less readable.

Specifically:

 ...
 +/* Ok, now go ahead and program all of those in one go */
 +mpc85xx_gpio_set(   gpio_high|gpio_low|gpio_in,
 +gpio_high|gpio_low,
 +gpio_high);
 
 ERROR: space prohibited after that open parenthesis '('
 #427: FILE: board/exmeritus/hww1u1a/hww1u1a.c:100:
 +   mpc85xx_gpio_set(   gpio_high|gpio_low|gpio_in,

I could fix this code to read:

mpc85xx_gpio_set(gpio_high|gpio_low|gpio_in,
gpio_high|gpio_low, gpio_high);

And it would be much harder to visually compare the three bitmask arguments 
against each other.


 +/*
 + * If things have been taken out of reset early (for example, by one
 + * of the BDI3000 debuggers), then we need to put them back in reset
 + * and delay a while before we continue.
 + */
 +#define GPIO_RESETS (GPIO_DIMM_RESET|GPIO_USB_RESET|GPIO_GETH0_RESET)
 +if (mpc85xx_gpio_get(GPIO_RESETS)) {
 
 Please don;t add #defines right in the middle of the code.

Fixed, thanks!


 +/*
 + * This little shell function just returns whether or not it's CPU A.
 + * It can be used to select the right device-tree when booting, etc.
 + */
 +int do_hww1u1a_test_cpu_a(cmd_tbl_t *cmdtp, int flag, int argc, char * 
 const argv[])
 
 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-character 
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.  This thread discusses it further:
  http://lkml.org/lkml/2009/12/15/490

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


 +/* Now the serial# part of the hostname */
 +for (j = 0; serialnr[j]; j++)
 +if (isalnum(serialnr[j]))
 +hww1u1a_prompt[i++] = tolower(serialnr[j]);
 
 Braces needed for multiline statements.

Fixed, thanks!


 +/* Turn on the HRESET_REQ pin (hard-reset request) */
 +printf(\nRESET: Hardware reset triggered, waiting...\n);
 +out_be32(gur-rstcr, 0x2);
 +while (1)
 +udelay(1);
 +}
 
 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.


 +/* Enable the U-Boot memory test */
 +#define CONFIG_SYS_MEMTEST_START 0x
 +#define CONFIG_SYS_MEMTEST_END   0x7fff
 
 I think this has not been tested, right?

I'm pretty sure it's been tested, but not very recently; the memory on all the 
shipped units is ECC anyways.

Cheers,
Kyle Moffett
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-03-21 Thread Wolfgang Denk 
Dear Moffett, Kyle D,

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 * c=
 onst 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.

 ter line than split it across 2 different lines.  Even still, this warning =
 is only issued if you pass --subjective to checkpatch, which is documente=
 d to enable more subjective tests.  This thread discusses it further:

No, I get this warning without such flags.  The command I run is just
checkpatch.pl --no-tree

BTW - could you please restrict your line length to some 70 characters
or so?  Thanks.

  +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 th=
 e 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.

  +  /* Turn on the HRESET_REQ pin (hard-reset request) */
  +  printf(\nRESET: Hardware reset triggered, waiting...\n);
  +  out_be32(gur-rstcr, 0x2);
  +  while (1)
  +  udelay(1);
  +  }
 =20
  Should that not be an infinite wait here?
 
 At this point if the board does not reset due to hardware failure it's bett=
 er off hanging than silently falling through.

Why don't you simply call hang() then?

  +/* Enable the U-Boot memory test */
  +#define CONFIG_SYS_MEMTEST_START 0x
  +#define CONFIG_SYS_MEMTEST_END   0x7fff
 =20
  I think this has not been tested, right?
 
 I'm pretty sure it's been tested, but not very recently; the memory on all =
 the shipped units is ECC anyways.

I guess testing it would reveal that it crashes your system because you
are overwriting the exception vectors.

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: w...@denx.de
Madness has no purpose.  Or reason.  But it may have a goal.
-- Spock, The Alternative Factor, stardate 3088.7
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-03-21 Thread Moffett, Kyle D 
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 21
$ 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(1);
 +  }
 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

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

2011-03-21 Thread Wolfgang Denk 
Dear Moffett, Kyle D,

In message ac0c0781-9e5f-42f7-9db6-eecf6a5be...@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 21
 $ 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: w...@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.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-03-21 Thread Moffett, Kyle D 
On Mar 21, 2011, at 17:34, Wolfgang Denk wrote:
 In message ac0c0781-9e5f-42f7-9db6-eecf6a5be...@boeing.com you wrote:
 
 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 21
 $ 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.

If those were patches from two years ago and your style policies had
significantly changed since then I would understand.

But those are patches from *LAST MONTH* which you were perfectly happy to
merge from dozens of different developers, but had *I* submitted identical
patches they would have been rejected without even a second glance.

That is inconsistent at best, and in my humble opinion downright rude.


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

I give up.

I apparently cannot rely on the U-Boot *CODE* to understand what the
U-Boot *CODING* style is.

The time investment to get reasonable board support merged into U-Boot is
proving to be *greater* than the time investment to just maintain our
board ports out of tree.

If anyone would like to use our code as a reference or try to get it merged
themselves, I will continue to maintain our GPLed out-of-tree patchset here:
  http://opensource.exmeritus.com/git/

But otherwise I see no valid reason I should waste any more of my time
submitting patches which get torn apart out of hand over issues which are
completely ignored for patches which come in from other maintainers.

In the future we will have to weigh other boot-loader alternatives due to
the unfriendly attitude here.

Good luck.

Cheers,
Kyle Moffett
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-03-21 Thread Wolfgang Denk 
Dear Moffett, Kyle D,

In message ddd31591-33cd-49e4-b303-3477e0093...@boeing.com you wrote:

 I apparently cannot rely on the U-Boot *CODE* to understand what the
 U-Boot *CODING* style is.

You don't have to rely on the code.  It's clearly documented.

The README says:

Coding Standards:
-

All contributions to U-Boot should conform to the Linux kernel
coding style; see the file Documentation/CodingStyle and the script
scripts/Lindent in your Linux kernel source directory...

http://www.denx.de/wiki/U-Boot/CodingStyle  says the same.

And the referred document says:

Chapter 2: Breaking long lines and strings
 
Coding style is all about readability and maintainability using
commonly
available tools.
 
The limit on the length of lines is 80 columns and this is a strongly
preferred limit.
 
Statements longer than 80 columns will be broken into sensible chunks.

Now what exactly is unclear here?


And no, you are not the only one who is asked to fix his code beause
of Line too long errors. Here just a small collection from the last
4 months or so:

11/11 To:Xiangfu Liu Re: [U-Boot] [PATCH 1/5] those files are jz4740 base 
files
  http://article.gmane.org/gmane.comp.boot-loaders.u-boot/88655
12/04 To:Luigi 'Comio'   Re: [U-Boot] [PATCH 5/6 v2] Enable bootstrap support 
for MIPS architecture.
  http://article.gmane.org/gmane.comp.boot-loaders.u-boot/90325
12/12 To:Macpaul Lin Re: [U-Boot] [PATCH] ftgmac100: support of gigabit eth 
ftgmac100
  http://article.gmane.org/gmane.comp.boot-loaders.u-boot/90740
12/12 To:Macpaul Lin Re: [U-Boot] [PATCH] ftgmac100: support of gigabit eth 
ftgmac100
  http://article.gmane.org/gmane.comp.boot-loaders.u-boot/90740
03/13 To:Heiko Schocher  Re: [U-Boot] [PATCH 01/20] keymile: rework headerfiles 
for keymile boards
  http://article.gmane.org/gmane.comp.boot-loaders.u-boot/95728


 But otherwise I see no valid reason I should waste any more of my time
 submitting patches which get torn apart out of hand over issues which are
 completely ignored for patches which come in from other maintainers.

This is simply not true. But I'm not sure if you are still listening
to any rational arguments. I can only ask you to calm down, and
eventually reconsider.  Sorry.

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: w...@denx.de
Quote from a recent meeting:   We are going to continue having these
meetings everyday until I find out why no work is getting done.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-03-21 Thread Moffett, Kyle D 
On Mar 21, 2011, at 18:24, Wolfgang Denk wrote:
 In message ddd31591-33cd-49e4-b303-3477e0093...@boeing.com you wrote:
 
 I apparently cannot rely on the U-Boot *CODE* to understand what the
 U-Boot *CODING* style is.
 
 You don't have to rely on the code.  It's clearly documented.
 
 The README says:
 
   Coding Standards:
   -
 
   All contributions to U-Boot should conform to the Linux kernel
   coding style; see the file Documentation/CodingStyle and the script
   scripts/Lindent in your Linux kernel source directory...
 
 http://www.denx.de/wiki/U-Boot/CodingStyle  says the same.

So here it is claimed that the U-Boot coding style is the same as the Linux
Kernel coding style.

But previously you said:
 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-character 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.
 
 In U-Boot, it is considered to be an ERROR.


So which is it?

If U-Boot uses the official Linux Kernel CodingStyle, then a few 80-char
lines are OK if it increases readability, for example by not having to wrap
a simple U_BOOT_CMD function declaration that goes just a whole 4 characters
over the limit.

If U-Boot does NOT use the Linux Kernel CodingStyle and wants to refuse all
patches with over 80 characters then you should copy checkpatch.pl and the
CodingStyle document and change that wording from strongly preferred to
hard requirement, and change the WARNING to ERROR.


 And the referred document says:
   Chapter 2: Breaking long lines and strings

   Coding style is all about readability and maintainability using
   commonly
   available tools.

   The limit on the length of lines is 80 columns and this is a strongly
   preferred limit.

   Statements longer than 80 columns will be broken into sensible chunks.
 
 Now what exactly is unclear here?

You removed the very next paragraph in the Linux CodingStyle file,
which contains:

 The only exception to this is where exceeding 80 columns significantly 
 increases

 readability and does not hide information.


Furthermore, Linus Torvalds himself said in an email:
 We fixed that to allow checkpatch to skip those warnings, but the fact is, 
 the fundamnetal problem has always been the 80 character part.

 
 I don't think any kernel developers use a vt100 any more. And even if they 
 do, I bet they curse the 24 lines more than they curse the occasional 
 80+ character lines.
 
 I'd be ok with changing the warning to 132 characters, which is another 
 perfectly fine historical limit. Or we can split the difference, and say 
 ok, 106 characters is too much. I don't care. But 80 characters is 
 causing too many idiotic changes.
 
 There are way worse problems in many patches than long lines. Too complex 
 expressions. Too deep indentation. Pure crap code. People seem to get way 
 too hung up on .. but at least it passes checkpatch. 

The line you were complaining about is a static U_BOOT_CMD function declaration
for goodness sakes!  It's about as common as dirt in the U-Boot code and the
only reason it doesn't fit on one line anymore is because I made the name of the
function *NAME* about 8 characters longer in this version of the patch than it
was in a previous patch.

It's not any more complex than it was before, nor is it any harder to read.

I'm tired and fed up with this whole mess.  If you think it's likely to be
accepted I'll go ahead and submit one more final respin tomorrow, assuming
I feel up to it.  Otherwise, I wish you the best of luck with U-Boot.

Good night.

Cheers,
Kyle Moffett
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-03-15 Thread Kyle Moffett 
The eXMeritus HWW-1U-1A unit is a DO-160-certified 13lb 1U chassis
with 3 independent TEMPEST zones.  Two independent P2020 computers may
be found inside each zone.  Complete hardware support is included.

High-level hardware overview:
  * DO-160 certified for passenger aircraft (noncritical)
  * TEMPEST ceritified for RED/BLACK separation
  * 3 zones per chassis, 2 computers per zone (total of 6)
  * Dual-core 1.066GHz P2020 per computer
  * One 2GB DDR2 SO-RDIMM module per computer (upgradable to 4GB)
  * Removable 80GB or 160GB Intel X18-M SSD per computer
  * Front-accessible dual-port E1000E per computer
  * Front-accessible serial console per computer
  * Front-accessible USB port per computer
  * Internal Gigabit crossover within each TEMPEST zone
  * Internal unidirectional fiber links across TEMPEST zones
  * Battery-backed DS1339 I2C RTC on each CPU.

Combined, each 13lb 1U chassis contains 12GB RAM, 12 cores @ 1.066GHz,
12 front-accessible Gigabit Ethernet ports and 960GB of solid-state
storage with a total power consumption of ~200W.

Additional notes:
  * SPD detection is only known to work with the DO-160-certified DIMMs

  * A U-Boot built with 36-bit address-space seems to work, but I don't
yet have a usable 36-bit kernel or DTB, so it's mostly untested.

  * CPU reset is a little quirky due to hardware misfeature, see the
extensive comments in the board_reset() function in hww1u1a.c

Signed-off-by: Kyle Moffett kyle.d.moff...@boeing.com
Cc: Andy Fleming aflem...@gmail.com
Cc: Kumar Gala kumar.g...@freescale.com
---
 MAINTAINERS   |4 +
 board/exmeritus/hww1u1a/Makefile  |   54 
 board/exmeritus/hww1u1a/ddr.c |   34 +++
 board/exmeritus/hww1u1a/gpios.h   |   67 +
 board/exmeritus/hww1u1a/hww1u1a.c |  543 +
 board/exmeritus/hww1u1a/law.c |   34 +++
 board/exmeritus/hww1u1a/tlb.c |  106 +++
 boards.cfg|2 +
 include/configs/HWW1U1A.h |  474 
 9 files changed, 1318 insertions(+), 0 deletions(-)
 create mode 100644 board/exmeritus/hww1u1a/Makefile
 create mode 100644 board/exmeritus/hww1u1a/ddr.c
 create mode 100644 board/exmeritus/hww1u1a/gpios.h
 create mode 100644 board/exmeritus/hww1u1a/hww1u1a.c
 create mode 100644 board/exmeritus/hww1u1a/law.c
 create mode 100644 board/exmeritus/hww1u1a/tlb.c
 create mode 100644 include/configs/HWW1U1A.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 4756f14..6644baf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -320,6 +320,10 @@ Reinhard Meyer reinhard.me...@emk-elektronik.de
TOP5200 MPC5200
TOP9000 ARM926EJS (AT91SAM9xxx SoC)
 
+Kyle Moffett kyle.d.moff...@boeing.com
+
+   HWW1U1A P2020
+
 Tolunay Orkun tor...@nextio.com
 
csb272  PPC405GP
diff --git a/board/exmeritus/hww1u1a/Makefile b/board/exmeritus/hww1u1a/Makefile
new file mode 100644
index 000..b927f59
--- /dev/null
+++ b/board/exmeritus/hww1u1a/Makefile
@@ -0,0 +1,54 @@
+#
+# Copyright 2007-2009 Freescale Semiconductor, Inc.
+# (C) Copyright 2001-2006
+# Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB= $(obj)lib$(BOARD).o
+
+COBJS-y+= $(BOARD).o
+COBJS-y+= law.o
+COBJS-y+= tlb.o
+COBJS-$(CONFIG_DDR_SPD) += ddr.o
+
+SRCS   := $(SOBJS:.o=.S) $(COBJS-y:.o=.c)
+OBJS   := $(addprefix $(obj),$(COBJS-y))
+SOBJS  := $(addprefix $(obj),$(SOBJS))
+
+$(LIB):$(obj).depend $(OBJS) $(SOBJS)
+   $(call cmd_link_o_target, $(OBJS))
+
+clean:
+   rm -f $(OBJS) $(SOBJS)
+
+distclean: clean
+   rm -f $(LIB) core *.bak .depend
+
+#
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#
diff --git a/board/exmeritus/hww1u1a/ddr.c b/board/exmeritus/hww1u1a/ddr.c
new file mode 100644
index 000..36d02ad
--- /dev/null
+++ b/board/exmeritus/hww1u1a/ddr.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2009-2010 eXMeritus, A Boeing Company
+ * Copyright

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

2011-03-15 Thread Wolfgang Denk 
Dear Kyle Moffett,

In message 1300208664-18339-5-git-send-email-kyle.d.moff...@boeing.com you 
wrote:
 The eXMeritus HWW-1U-1A unit is a DO-160-certified 13lb 1U chassis
 with 3 independent TEMPEST zones.  Two independent P2020 computers may
 be found inside each zone.  Complete hardware support is included.

Please run checkpatch on your submissions!

...
 + /* Ok, now go ahead and program all of those in one go */
 + mpc85xx_gpio_set(   gpio_high|gpio_low|gpio_in,
 + gpio_high|gpio_low,
 + gpio_high);

ERROR: space prohibited after that open parenthesis '('
#427: FILE: board/exmeritus/hww1u1a/hww1u1a.c:100:
+   mpc85xx_gpio_set(   gpio_high|gpio_low|gpio_in,

 + /*
 +  * If things have been taken out of reset early (for example, by one
 +  * of the BDI3000 debuggers), then we need to put them back in reset
 +  * and delay a while before we continue.
 +  */
 +#define GPIO_RESETS (GPIO_DIMM_RESET|GPIO_USB_RESET|GPIO_GETH0_RESET)
 + if (mpc85xx_gpio_get(GPIO_RESETS)) {

Please don;t add #defines right in the middle of the code.

 +/*
 + * This little shell function just returns whether or not it's CPU A.
 + * It can be used to select the right device-tree when booting, etc.
 + */
 +int do_hww1u1a_test_cpu_a(cmd_tbl_t *cmdtp, int flag, int argc, char * const 
 argv[])

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[])

 +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

What is this empty comment needed for?

 + /* Now the serial# part of the hostname */
 + for (j = 0; serialnr[j]; j++)
 + if (isalnum(serialnr[j]))
 + hww1u1a_prompt[i++] = tolower(serialnr[j]);

Braces needed for multiline statements.

...
 + /* Turn on the HRESET_REQ pin (hard-reset request) */
 + printf(\nRESET: Hardware reset triggered, waiting...\n);
 + out_be32(gur-rstcr, 0x2);
 + while (1)
 + udelay(1);
 + }

Should that not be an infinite wait here?

...
 +/* Enable the U-Boot memory test */
 +#define CONFIG_SYS_MEMTEST_START 0x
 +#define CONFIG_SYS_MEMTEST_END   0x7fff

I think this has not been tested, right?


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: w...@denx.de
News is what a chap who doesn't care much  about  anything  wants  to
read. And it's only news until he's read it. After that it's dead.
   - Evelyn Waugh _Scoop_ (1938) bk. 1, ch. 5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot