[U-Boot-Users] PATCH] Add 8641reset commmand for mpc8641hpcn board
jdl at freescale.com
Fri Mar 23 15:52:47 CET 2007
On Thu, 2007-03-22 at 18:17, Wolfgang Denk wrote:
> Dear Jon,
> in message <1174588314.32390.41.camel at ld0161-tx32> you wrote:
> > Does this look better/acceptable to you this way?
> The implementation still lacks a lot.
But the overall approach? Is it better than the
previous version that hit common code?
> > - cmd = argv;
> > + cmd = argv;
> Instead of switching on single characters, ignoring all the rest of
> the strings, please use a string compare like done everywhere else in
> How many different usage messages are there for this command? Please
> be consistent, and implement this just once.
[ ... ]
> > + puts("For example: 8641reset cf 40 2.5 10\n");
> > + puts("See MPC8641HPCN Design Workbook for valid values of command line parameters.\n");
> Please don't provide examples in the usage message. These should go
> into the documentation, not the code.
> > +U_BOOT_CMD(
> > + 8641reset, CFG_MAXARGS, 1, mpc8641_reset_board,
> > + "8641reset - Reset the CPU to current bank or alternative bank\n",
> > + NULL
> > +);
> Now *this* is the real usage message. Please use this, and drop the
> "my_usage" code.
Got it. Will do.
> Also, I object against the command name. It should follow standard
> name rules (enven though these are not strictly enforced by the
> current command line parser), i. e. only consist of alphanumeric
> characters and underscores, and begin with an underscore or an
> alphabetic character (and I hereby discourage to use an underscore as
> the first character).
OK. Will "reset8641" be acceptable?
> > - mpc8641_reset_board(cmdtp, flag, argc, argv);
> > + out8(PIXIS_BASE + PIXIS_RST, 0);
> Some comment seems like a good idea to me...
> > +5. 8641reset command
> > +--------------------
> > +A new command "8641reset" is introduced for reset mpc8641hpcn board to
> > +current bank or alternative bank, with/without wathdog timer enabled,
> Typo: s/th/tch/
> Best regards,
> Wolfgang Denk
Thanks for your review!
I'll respin the patch an check again before asking for a pull!
More information about the U-Boot