| Message ID | 20110707143109.GC5438@harvey-pc.matrox.com |
|---|---|
| State | Accepted |
| Delegated to: | Albert ARIBAUD |
| Headers | show |
Hi Christopher, Le 07/07/2011 16:31, Christopher Harvey a écrit : > Signed-off-by: Christopher Harvey<charvey@matrox.com> > --- > > V2: > I didn't receive comments on this one. By not compiling this by > default it removes a couple of #defines people need to think > about. Also, I'm guessing the memory test isn't used very much, so > this makes u-boot a bit smaller. > > common/cmd_mem.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/common/cmd_mem.c b/common/cmd_mem.c > index a5576aa..833af66 100644 > --- a/common/cmd_mem.c > +++ b/common/cmd_mem.c > @@ -610,6 +610,8 @@ int do_mem_loopw (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > } > #endif /* CONFIG_LOOPW */ > > +#ifdef CONFIG_CMD_MTEST > + > /* > * Perform a memory test. A more complete alternative test can be > * configured using CONFIG_SYS_ALT_MEMTEST. The complete test loops until > @@ -965,6 +967,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > return 0; /* not reached */ > } > > +#endif /* CONFIG_CMD_MTEST */ > > /* Modify memory. > * > @@ -1245,11 +1248,13 @@ U_BOOT_CMD( > ); > #endif /* CONFIG_LOOPW */ > > +#ifdef CONFIG_CMD_MTEST > U_BOOT_CMD( > mtest, 5, 1, do_mem_mtest, > "simple RAM read/write test", > "[start [end [pattern [iterations]]]]" > ); > +#endif /* CONFIG_CMD_MTEST */ > > #ifdef CONFIG_MX_CYCLIC > U_BOOT_CMD( Applied to u-boot-arm/master, thanks! Amicalement,
Dear Albert ARIBAUD, In message <4E3A3CB8.2030703@aribaud.net> you wrote: > > > I didn't receive comments on this one. By not compiling this by > > default it removes a couple of #defines people need to think > > about. Also, I'm guessing the memory test isn't used very much, so > > this makes u-boot a bit smaller. ... > > common/cmd_mem.c | 5 +++++ > > 1 files changed, 5 insertions(+), 0 deletions(-) ... > Applied to u-boot-arm/master, thanks! Argh.. This is NOT an ARM specific patch. Even worse, it changes global behaviour for most baords. This cannot be dones easily. Sorry, but NAK. Best regards, Wolfgang Denk
Dear Christopher Harvey, In message <20110707143109.GC5438@harvey-pc.matrox.com> you wrote: > > I didn't receive comments on this one. By not compiling this by > default it removes a couple of #defines people need to think > about. Also, I'm guessing the memory test isn't used very much, so > this makes u-boot a bit smaller. > > common/cmd_mem.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/common/cmd_mem.c b/common/cmd_mem.c > index a5576aa..833af66 100644 > --- a/common/cmd_mem.c > +++ b/common/cmd_mem.c > @@ -610,6 +610,8 @@ int do_mem_loopw (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > } > #endif /* CONFIG_LOOPW */ > > +#ifdef CONFIG_CMD_MTEST Formal complaint: ifh you introduce new CONFIG_ options, you MUST document these in the README. What is worse is that your commit changes global behaviour of U-Boot for all architectures, nearly all boards. Even if you guess that some feature might not be used we cannot simply remove it lightly. If you want to make this configurable, then as an opt-out, but not mandatory for everybody. Best regards, Wolfgang Denk
On 04/08/2011 12:14, Wolfgang Denk wrote: > Dear Albert ARIBAUD, > > In message<4E3A3CB8.2030703@aribaud.net> you wrote: >> >>> I didn't receive comments on this one. By not compiling this by >>> default it removes a couple of #defines people need to think >>> about. Also, I'm guessing the memory test isn't used very much, so >>> this makes u-boot a bit smaller. > ... >>> common/cmd_mem.c | 5 +++++ >>> 1 files changed, 5 insertions(+), 0 deletions(-) > ... >> Applied to u-boot-arm/master, thanks! > > Argh.. > > This is NOT an ARM specific patch. > > Even worse, it changes global behaviour for most baords. This cannot > be dones easily. > > Sorry, but NAK. Quite understandable, as I hurriedly applied the entire four-patch set when I should only have pulled the last two, ARM-related, patches. > Best regards, > > Wolfgang Denk Amicalement,
diff --git a/common/cmd_mem.c b/common/cmd_mem.c index a5576aa..833af66 100644 --- a/common/cmd_mem.c +++ b/common/cmd_mem.c @@ -610,6 +610,8 @@ int do_mem_loopw (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } #endif /* CONFIG_LOOPW */ +#ifdef CONFIG_CMD_MTEST + /* * Perform a memory test. A more complete alternative test can be * configured using CONFIG_SYS_ALT_MEMTEST. The complete test loops until @@ -965,6 +967,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return 0; /* not reached */ } +#endif /* CONFIG_CMD_MTEST */ /* Modify memory. * @@ -1245,11 +1248,13 @@ U_BOOT_CMD( ); #endif /* CONFIG_LOOPW */ +#ifdef CONFIG_CMD_MTEST U_BOOT_CMD( mtest, 5, 1, do_mem_mtest, "simple RAM read/write test", "[start [end [pattern [iterations]]]]" ); +#endif /* CONFIG_CMD_MTEST */ #ifdef CONFIG_MX_CYCLIC U_BOOT_CMD(
Signed-off-by: Christopher Harvey <charvey@matrox.com> --- V2: I didn't receive comments on this one. By not compiling this by default it removes a couple of #defines people need to think about. Also, I'm guessing the memory test isn't used very much, so this makes u-boot a bit smaller. common/cmd_mem.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)