Dear Simon,
Am 26.08.2011 11:57, schrieb Simon Schwarz:
Dear Andreas,
On 08/25/2011 11:40 AM, Andreas Bießmann wrote:
Dear Simon,
<snip>
void arch_lmb_reserve(struct lmb *lmb) @@ -98,63 +101,67 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) bd_t *bd = gd->bd; char *s; int machid = bd->bi_arch_number;
- void (*kernel_entry)(int zero, int arch, uint params);
- void (*kernel_entry)(int zero, int arch, uint params) = NULL;
This should not be necessary, kernel_entry would be on bss which should be initialized to zero by start.S.
I added this because of compiler warnings that it could be used uninitialized.
I see, you changed the flow of this function elementary. Sorry, did not realize that.
#ifdef CONFIG_CMDLINE_TAG char *commandline = getenv ("bootargs"); #endif
- if ((flag != 0)&& (flag != BOOTM_STATE_OS_GO))
return 1;- s = getenv ("machid");
- if (s) {
machid = simple_strtoul (s, NULL, 16);printf ("Using machid 0x%x from environment\n", machid);- }
- show_boot_progress (15);
- if ((flag != 0)&& (!(flag& BOOTM_STATE_OS_GO ||
flag& BOOTM_STATE_OS_PREP)))switch'n'case would be much cleaner here. And seperating the functionality into functions would be nice too.
Hehe. Somehow I did know that this topic will come up. I intended to change the code as little as possible.
Well, switch'n'case is not correct here, sorry for misleading comment.
So essentially this would mean to rewrite this to reflect the structure of the ppc version. Will do if there are no objections.
+1 for rewrite, Albert how do you think about?
If there are no objections I also would like to separate this patch from this series. This has some advantages:
- Support for the prep subcommand is essential for saving the boot
parameters. (if prep is in saving can also be done manually)
- I think that there won't be much discussion about the usefulness of
implementing this - just some about the how.
I think it would be good idea to add the 'bootm prep' command to arm implementation of bootm in a separate step.
<snip>
- if (flag == 0 || flag& BOOTM_STATE_OS_GO) {
flag == 0? Shouldn't that be flag == BOOTM_STATE_OS_GO?
flag = 0 means that no subcommand was issued -> execute everything.
again, sorry for the noise.
<snip>
best regards
Andreas Bießmann