| Message ID | 1315666667-6270-1-git-send-email-amurray@theiet.org |
|---|---|
| State | Rejected |
| Headers | show |
Dear Andrew Murray, In message <1315666667-6270-1-git-send-email-amurray@theiet.org> you wrote: > It's common for the bootm command to move a provided image in memory > prior to it's execution - this move can contribute to increased boot > times. This move is often seen in poorly configured devices which > boot from NAND. > > This patch improves the output of the bootm command such that when > a move occurs it is made clear to the user. Sorry, but I don't want to have this output. It would be always be printed for all systems booting from NOR flash, where the copy operation is absolutely normal. Also, on all PowerPC systems it is absolutely normal that you must load the image to a different address in RAM, because otherwise you would overwrite the exception vectors and thus crash U-Boot. What you are trying to "warn" about is an absolutely normal use case, and there is no reason for additional output. Yes, there may be systems that could work more efficiently, but there are many areas where you can optimize (starting with the Linux image building). Sorry, but NAK. Best regards, Wolfgang Denk
Hi, On 10 September 2011 16:06, Wolfgang Denk <wd@denx.de> wrote: > Sorry, but I don't want to have this output. It would be always be > printed for all systems booting from NOR flash, where the copy > operation is absolutely normal. > > Also, on all PowerPC systems it is absolutely normal that you must > load the image to a different address in RAM, because otherwise you > would overwrite the exception vectors and thus crash U-Boot. > > What you are trying to "warn" about is an absolutely normal use case, > and there is no reason for additional output. > > Yes, there may be systems that could work more efficiently, but there > are many areas where you can optimize (starting with the Linux image > building). > Yes I understand your point but perhaps my description was not very useful. The patch doesn't add any additional messages or output or anything which indicates something is not ideal. Instead it renames the exiting 'Loading' text to a arguably more descriptive 'Moving' text - and removes the message all together when the move operation doesn't occur. Thanks, Andrew Murray
On Saturday, September 10, 2011 10:57:47 Andrew Murray wrote: > --- a/common/cmd_bootm.c > +++ b/common/cmd_bootm.c > > if (load == blob_start || load == image_start) { > .......... > - } else { > + } else if (load != image_start) { sorry, but why does this new if() make any sense ? the only way this else branch could execute is if load != image_start since load == image_start was explicitly handled in the first if check. -mike
On 11 September 2011 20:22, Mike Frysinger <vapier@gentoo.org> wrote: > On Saturday, September 10, 2011 10:57:47 Andrew Murray wrote: > > --- a/common/cmd_bootm.c > > +++ b/common/cmd_bootm.c > > > > if (load == blob_start || load == image_start) { > > .......... > > - } else { > > + } else if (load != image_start) { > > sorry, but why does this new if() make any sense ? the only way this else > branch could execute is if load != image_start since load == image_start > was > explicitly handled in the first if check. > -mike > Yes that's correct. The move is executed and a print statement displayed - only when the load address differs from the image start address. In other words the patch prevents unnecessary/confusing output and a call to a function that doesn't do anything when load == image_start. Andrew Murray
On Sunday, September 11, 2011 16:13:26 Andrew Murray wrote: > On 11 September 2011 20:22, Mike Frysinger <vapier@gentoo.org> wrote: > > On Saturday, September 10, 2011 10:57:47 Andrew Murray wrote: > > > --- a/common/cmd_bootm.c > > > +++ b/common/cmd_bootm.c > > > > > > if (load == blob_start || load == image_start) { > > > .......... > > > - } else { > > > + } else if (load != image_start) { > > > > sorry, but why does this new if() make any sense ? the only way this > > else branch could execute is if load != image_start since load == > > image_start was > > explicitly handled in the first if check. > > Yes that's correct. The move is executed and a print statement displayed - > only when the load address differs from the image start address. In other > words the patch prevents unnecessary/confusing output and a call to a > function that doesn't do anything when load == image_start. i think you missed my point. your proposed change to the "else" branch makes no difference to the existing code. current code: if (load == image_start) { ... } else { ... } your new code: if (load == image_start) { ... } else if (load != image_start) { ... } your change to the if statement is pointless ? -mike
Hi Mike, On 11 September 2011 21:17, Mike Frysinger <vapier@gentoo.org> wrote: > > your change to the if statement is pointless ? You are right - I thought I was going crazy then realized I made this patch on an earlier version of the code which didn't have the load == image_start check in the first line (see http://git.denx.de/?p=u-boot.git;a=commitdiff;h=02cf345973a7fe9986626448a089ed54f1a26d13 from January). I must have blindly moved this patch forward. I will get my tools in order. Many thanks for you patience. Andrew Murray
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 147e8de..b5f28b0 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -346,8 +346,8 @@ static int bootm_load_os(image_info_t os, ulong *load_end, int boot_progress) case IH_COMP_NONE: if (load == blob_start || load == image_start) { printf (" XIP %s ... ", type_name); - } else { - printf (" Loading %s ... ", type_name); + } else if (load != image_start) { + printf (" Moving %s ... ", type_name); memmove_wd ((void *)load, (void *)image_start, image_len, CHUNKSZ); }
It's common for the bootm command to move a provided image in memory prior to it's execution - this move can contribute to increased boot times. This move is often seen in poorly configured devices which boot from NAND. This patch improves the output of the bootm command such that when a move occurs it is made clear to the user. Signed-off-by: Andrew Murray <amurray@theiet.org> --- common/cmd_bootm.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)