| Message ID | 1324006361-25456-1-git-send-email-Kyle.D.Moffett@boeing.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Thursday 15 December 2011 22:32:41 Kyle Moffett wrote: > This new #define is set in config_cmd_defaults.h (which is automatically > included on every board by "mkconfig"), but this allows boards to elect > to omit the "reset" command if necessary with "#undef CONFIG_CMD_RESET". NAK: doesn't seem to address the feedback i posted last time ... -mike
On Dec 16, 2011, at 00:05, Mike Frysinger wrote: > On Thursday 15 December 2011 22:32:41 Kyle Moffett wrote: >> This new #define is set in config_cmd_defaults.h (which is automatically >> included on every board by "mkconfig"), but this allows boards to elect >> to omit the "reset" command if necessary with "#undef CONFIG_CMD_RESET". > > NAK: doesn't seem to address the feedback i posted last time ... Hmm, I thought I addressed these: >> --- a/include/config_cmd_defaults.h >> +++ b/include/config_cmd_defaults.h > > updating these files is not sufficient and will break boards. drop the hunks to > these files and update include/config_defaults.h instead. The "mkconfig" program automatically includes both config_defaults.h and config_cmd_defaults.h, so this is sufficient to not break boards. This is exactly how CONFIG_CMD_GO works. Since it's always on, there's no need to add it to config_cmd_all.h either (which is why I removed that hunk). > would be a good time to split this into a dedicated common/cmd_reset.c, > we can also then fix cmd_boot.c to only build when CONFIG_CMD_GO is enabled The only references to CONFIG_CMD_GO are in config_cmd_defaults.h, README, and common/cmd_boot.c, there is nothing that ever turns it off, so perhaps the config symbol should just be removed? Cheers, Kyle Moffett
On Friday 16 December 2011 13:49:15 Moffett, Kyle D wrote: > On Dec 16, 2011, at 00:05, Mike Frysinger wrote: > > On Thursday 15 December 2011 22:32:41 Kyle Moffett wrote: > >> This new #define is set in config_cmd_defaults.h (which is automatically > >> included on every board by "mkconfig"), but this allows boards to elect > >> to omit the "reset" command if necessary with "#undef CONFIG_CMD_RESET". > > > > NAK: doesn't seem to address the feedback i posted last time ... > > Hmm, I thought I addressed these: > >> --- a/include/config_cmd_defaults.h > >> +++ b/include/config_cmd_defaults.h > > > > updating these files is not sufficient and will break boards. drop the > > hunks to these files and update include/config_defaults.h instead. > > The "mkconfig" program automatically includes both config_defaults.h and > config_cmd_defaults.h, so this is sufficient to not break boards. ah, you're right. sorry about that. i was thinking of "config_cmd_default.h" which is not the same as "config_cmd_defaults.h" ;). > > would be a good time to split this into a dedicated common/cmd_reset.c, > > we can also then fix cmd_boot.c to only build when CONFIG_CMD_GO is > > enabled > > The only references to CONFIG_CMD_GO are in config_cmd_defaults.h, README, > and common/cmd_boot.c, there is nothing that ever turns it off, so perhaps > the config symbol should just be removed? i have some boards that turn it off -mike
On Dec 16, 2011, at 14:30, Mike Frysinger wrote: > On Friday 16 December 2011 13:49:15 Moffett, Kyle D wrote: >> On Dec 16, 2011, at 00:05, Mike Frysinger wrote: >>> On Thursday 15 December 2011 22:32:41 Kyle Moffett wrote: >>>> This new #define is set in config_cmd_defaults.h (which is automatically >>>> included on every board by "mkconfig"), but this allows boards to elect >>>> to omit the "reset" command if necessary with "#undef CONFIG_CMD_RESET". >>> >>> would be a good time to split this into a dedicated common/cmd_reset.c, >>> we can also then fix cmd_boot.c to only build when CONFIG_CMD_GO is >>> enabled >> >> The only references to CONFIG_CMD_GO are in config_cmd_defaults.h, README, >> and common/cmd_boot.c, there is nothing that ever turns it off, so perhaps >> the config symbol should just be removed? > > i have some boards that turn it off Ok, I really don't see the point in moving these 5 lines to their own file, especially since they are in is already just 78 lines long: U_BOOT_CMD( reset, 1, 0, do_reset, "Perform RESET of the CPU", "" ); If you really think that's what I should do, though, I'll go ahead and make that change. Cheers, Kyle Moffett -- Curious about my work on the Debian powerpcspe port? I'm keeping a blog here: http://pureperl.blogspot.com/
On Friday 16 December 2011 15:42:26 Moffett, Kyle D wrote: > On Dec 16, 2011, at 14:30, Mike Frysinger wrote: > > On Friday 16 December 2011 13:49:15 Moffett, Kyle D wrote: > >> On Dec 16, 2011, at 00:05, Mike Frysinger wrote: > >>> On Thursday 15 December 2011 22:32:41 Kyle Moffett wrote: > >>>> This new #define is set in config_cmd_defaults.h (which is > >>>> automatically included on every board by "mkconfig"), but this allows > >>>> boards to elect to omit the "reset" command if necessary with "#undef > >>>> CONFIG_CMD_RESET". > >>> > >>> would be a good time to split this into a dedicated common/cmd_reset.c, > >>> we can also then fix cmd_boot.c to only build when CONFIG_CMD_GO is > >>> enabled > >> > >> The only references to CONFIG_CMD_GO are in config_cmd_defaults.h, > >> README, and common/cmd_boot.c, there is nothing that ever turns it off, > >> so perhaps the config symbol should just be removed? > > > > i have some boards that turn it off > > Ok, > > I really don't see the point in moving these 5 lines to their own file, > especially since they are in is already just 78 lines long: > U_BOOT_CMD( > reset, 1, 0, do_reset, > "Perform RESET of the CPU", > "" > ); > > If you really think that's what I should do, though, I'll go ahead and > make that change. i think so, that way we can change cmd_boot.o in the Makefile to depend on CONFIG_CMD_GO and drop the ifdef logic from cmd_boot.c -mike
diff --git a/README b/README index ff72e47..90989db 100644 --- a/README +++ b/README @@ -793,6 +793,7 @@ The following options need to be configured: host CONFIG_CMD_PORTIO * Port I/O CONFIG_CMD_REGINFO * Register dump + CONFIG_CMD_RESET Reset the CPU CONFIG_CMD_RUN run command in env variable CONFIG_CMD_SAVES * save S record dump CONFIG_CMD_SCSI * SCSI Support diff --git a/common/cmd_boot.c b/common/cmd_boot.c index 0afd939..f5779e9 100644 --- a/common/cmd_boot.c +++ b/common/cmd_boot.c @@ -71,8 +71,10 @@ U_BOOT_CMD( #endif +#ifdef CONFIG_CMD_RESET U_BOOT_CMD( reset, 1, 0, do_reset, "Perform RESET of the CPU", "" ); +#endif diff --git a/include/config_cmd_defaults.h b/include/config_cmd_defaults.h index a55b268..b65a945 100644 --- a/include/config_cmd_defaults.h +++ b/include/config_cmd_defaults.h @@ -14,5 +14,6 @@ #define CONFIG_CMD_EXPORTENV 1 #define CONFIG_CMD_GO 1 #define CONFIG_CMD_IMPORTENV 1 +#define CONFIG_CMD_RESET 1 #endif
This new #define is set in config_cmd_defaults.h (which is automatically included on every board by "mkconfig"), but this allows boards to elect to omit the "reset" command if necessary with "#undef CONFIG_CMD_RESET". Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com> Cc: Wolfgang Denk <wd@denx.de> Cc: Mike Frysinger <vapier@gentoo.org> --- README | 1 + common/cmd_boot.c | 2 ++ include/config_cmd_defaults.h | 1 + 3 files changed, 4 insertions(+), 0 deletions(-)