subject:"\[U\-Boot\] \[PATCH V3 1\/8\] arm\: Add Prep subcommand support to bootm"

Re: [U-Boot] [PATCH V3 1/8] arm: Add Prep subcommand support to bootm

2011-08-26 Thread Simon Schwarz

Dear Andreas,

On 08/25/2011 11:40 AM, Andreas Bießmann wrote:
 Dear Simon,

 Am 25.08.2011 10:33, schrieb Simon Schwarz:
 Adds prep subcommand to bootm implementation of ARM. When bootm is called 
 with
 the subcommand prep the function stops right after ATAGS creation and before
 announce_and_cleanup.

 This is used in savebp command

 Signed-off-by: Simon Schwarzsimonschwarz...@gmail.com
 

 V2 changes:
 nothing

 V3 changes:
 nothing
 ---
   arch/arm/lib/bootm.c |  116 
 +++--
   common/cmd_bootm.c   |2 +-
   2 files changed, 65 insertions(+), 53 deletions(-)

 diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
 index 802e833..d3152ae 100644
 --- a/arch/arm/lib/bootm.c
 +++ b/arch/arm/lib/bootm.c
 @@ -1,4 +1,7 @@
 -/*
 +/* Copyright (C) 2011
 + * Corscience GmbH  Co. KG - Simon Schwarzschw...@corscience.de
 + *  - Added prep subcommand support
 + *
* (C) Copyright 2002
* Sysgo Real-Time Solutions, GmbHwww.elinos.com
* Marius Groegermgroe...@sysgo.de
 @@ -55,7 +58,7 @@ static struct tag *params;

   static ulong get_sp(void);
   #if defined(CONFIG_OF_LIBFDT)
 -static int bootm_linux_fdt(int machid, bootm_headers_t *images);
 +static int bootm_linux_fdt(int machid, bootm_headers_t *images, int flag);
   #endif

   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.


   #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.

So essentially this would mean to rewrite this to reflect the structure 
of the ppc version. Will do if there are no objections.

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.


 +return 1; /* subcommand not implemented */
 +else if ((flag == 0) || flag  BOOTM_STATE_OS_PREP) {
 +s = getenv(machid);
 +if (s) {
 +strict_strtoul(s, 16, (long unsigned int *)machid);
 +printf(Using machid 0x%x from environment\n, machid);
 +}
 +
 +show_boot_progress(15);

   #ifdef CONFIG_OF_LIBFDT
 -if (images-ft_len)
 -return bootm_linux_fdt(machid, images);
 +if (images-ft_len)
 +return bootm_linux_fdt(machid, images, flag);
   #endif

 -kernel_entry = (void (*)(int, int, uint))images-ep;
 +kernel_entry = (void (*)(int, int, uint))images-ep;

 -debug (## Transferring control to Linux (at address %08lx) ...\n,
 -   (ulong) kernel_entry);
 +debug(## Transferring control to Linux (at address %08lx) \
 +...\n, (ulong) kernel_entry);

   #if defined (CONFIG_SETUP_MEMORY_TAGS) || \
   defined (CONFIG_CMDLINE_TAG) || \
   defined (CONFIG_INITRD_TAG) || \
   defined (CONFIG_SERIAL_TAG) || \
   defined (CONFIG_REVISION_TAG)
 -setup_start_tag (bd);
 +setup_start_tag(bd);
   #ifdef CONFIG_SERIAL_TAG
 -setup_serial_tag (params);
 +setup_serial_tag(params);
   #endif
   #ifdef CONFIG_REVISION_TAG
 -setup_revision_tag (params);
 +setup_revision_tag(params);
   #endif
   #ifdef CONFIG_SETUP_MEMORY_TAGS
 -setup_memory_tags (bd);
 +setup_memory_tags(bd);
   #endif
   #ifdef CONFIG_CMDLINE_TAG
 -setup_commandline_tag (bd, commandline);
 +setup_commandline_tag(bd, commandline);
   #endif
   #ifdef CONFIG_INITRD_TAG
 -if (images-rd_start  images-rd_end)
 -setup_initrd_tag (bd, images-rd_start, images-rd_end);
 +if (images-rd_start  images-rd_end)
 +setup_initrd_tag(bd, images-rd_start,

Re: [U-Boot] [PATCH V3 1/8] arm: Add Prep subcommand support to bootm

2011-08-26 Thread Andreas Bießmann

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;
   intmachid = 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


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH V3 1/8] arm: Add Prep subcommand support to bootm

2011-08-25 Thread Simon Schwarz

Adds prep subcommand to bootm implementation of ARM. When bootm is called with
the subcommand prep the function stops right after ATAGS creation and before
announce_and_cleanup.

This is used in savebp command

Signed-off-by: Simon Schwarz simonschwarz...@gmail.com


V2 changes:
nothing

V3 changes:
nothing
---
 arch/arm/lib/bootm.c |  116 +++--
 common/cmd_bootm.c   |2 +-
 2 files changed, 65 insertions(+), 53 deletions(-)

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 802e833..d3152ae 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -1,4 +1,7 @@
-/*
+/* Copyright (C) 2011
+ * Corscience GmbH  Co. KG - Simon Schwarz schw...@corscience.de
+ *  - Added prep subcommand support
+ *
  * (C) Copyright 2002
  * Sysgo Real-Time Solutions, GmbH www.elinos.com
  * Marius Groeger mgroe...@sysgo.de
@@ -55,7 +58,7 @@ static struct tag *params;
 
 static ulong get_sp(void);
 #if defined(CONFIG_OF_LIBFDT)
-static int bootm_linux_fdt(int machid, bootm_headers_t *images);
+static int bootm_linux_fdt(int machid, bootm_headers_t *images, int flag);
 #endif
 
 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;
 
 #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)))
+   return 1; /* subcommand not implemented */
+   else if ((flag == 0) || flag  BOOTM_STATE_OS_PREP) {
+   s = getenv(machid);
+   if (s) {
+   strict_strtoul(s, 16, (long unsigned int *) machid);
+   printf(Using machid 0x%x from environment\n, machid);
+   }
+
+   show_boot_progress(15);
 
 #ifdef CONFIG_OF_LIBFDT
-   if (images-ft_len)
-   return bootm_linux_fdt(machid, images);
+   if (images-ft_len)
+   return bootm_linux_fdt(machid, images, flag);
 #endif
 
-   kernel_entry = (void (*)(int, int, uint))images-ep;
+   kernel_entry = (void (*)(int, int, uint))images-ep;
 
-   debug (## Transferring control to Linux (at address %08lx) ...\n,
-  (ulong) kernel_entry);
+   debug(## Transferring control to Linux (at address %08lx) \
+   ...\n, (ulong) kernel_entry);
 
 #if defined (CONFIG_SETUP_MEMORY_TAGS) || \
 defined (CONFIG_CMDLINE_TAG) || \
 defined (CONFIG_INITRD_TAG) || \
 defined (CONFIG_SERIAL_TAG) || \
 defined (CONFIG_REVISION_TAG)
-   setup_start_tag (bd);
+   setup_start_tag(bd);
 #ifdef CONFIG_SERIAL_TAG
-   setup_serial_tag (params);
+   setup_serial_tag(params);
 #endif
 #ifdef CONFIG_REVISION_TAG
-   setup_revision_tag (params);
+   setup_revision_tag(params);
 #endif
 #ifdef CONFIG_SETUP_MEMORY_TAGS
-   setup_memory_tags (bd);
+   setup_memory_tags(bd);
 #endif
 #ifdef CONFIG_CMDLINE_TAG
-   setup_commandline_tag (bd, commandline);
+   setup_commandline_tag(bd, commandline);
 #endif
 #ifdef CONFIG_INITRD_TAG
-   if (images-rd_start  images-rd_end)
-   setup_initrd_tag (bd, images-rd_start, images-rd_end);
+   if (images-rd_start  images-rd_end)
+   setup_initrd_tag(bd, images-rd_start, images-rd_end);
 #endif
-   setup_end_tag(bd);
+   setup_end_tag(bd);
 #endif
+   if (flag  BOOTM_STATE_OS_PREP)
+   return 0;
+   }
 
-   announce_and_cleanup();
-
-   kernel_entry(0, machid, bd-bi_boot_params);
-   /* does not return */
+   if (flag == 0 || flag  BOOTM_STATE_OS_GO) {
+   announce_and_cleanup();
 
+   kernel_entry(0, machid, bd-bi_boot_params);
+   /* does not return */
+   }
return 1;
 }
 
@@ -174,10 +181,10 @@ static int fixup_memory_node(void *blob)
return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
 }
 
-static int bootm_linux_fdt(int machid, bootm_headers_t *images)
+static int bootm_linux_fdt(int machid, bootm_headers_t *images, int flag)
 {
ulong rd_len;
-   void (*kernel_entry)(int zero, int dt_machid, void *dtblob);
+   void (*kernel_entry)(int zero, int dt_machid, void *dtblob) = NULL;
ulong of_size =

Re: [U-Boot] [PATCH V3 1/8] arm: Add Prep subcommand support to bootm

2011-08-25 Thread Andreas Bießmann

Dear Simon,

Am 25.08.2011 10:33, schrieb Simon Schwarz:
 Adds prep subcommand to bootm implementation of ARM. When bootm is called with
 the subcommand prep the function stops right after ATAGS creation and before
 announce_and_cleanup.
 
 This is used in savebp command
 
 Signed-off-by: Simon Schwarz simonschwarz...@gmail.com
 
 
 V2 changes:
 nothing
 
 V3 changes:
 nothing
 ---
  arch/arm/lib/bootm.c |  116 +++--
  common/cmd_bootm.c   |2 +-
  2 files changed, 65 insertions(+), 53 deletions(-)
 
 diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
 index 802e833..d3152ae 100644
 --- a/arch/arm/lib/bootm.c
 +++ b/arch/arm/lib/bootm.c
 @@ -1,4 +1,7 @@
 -/*
 +/* Copyright (C) 2011
 + * Corscience GmbH  Co. KG - Simon Schwarz schw...@corscience.de
 + *  - Added prep subcommand support
 + *
   * (C) Copyright 2002
   * Sysgo Real-Time Solutions, GmbH www.elinos.com
   * Marius Groeger mgroe...@sysgo.de
 @@ -55,7 +58,7 @@ static struct tag *params;
  
  static ulong get_sp(void);
  #if defined(CONFIG_OF_LIBFDT)
 -static int bootm_linux_fdt(int machid, bootm_headers_t *images);
 +static int bootm_linux_fdt(int machid, bootm_headers_t *images, int flag);
  #endif
  
  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.
  
  #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.

 + return 1; /* subcommand not implemented */
 + else if ((flag == 0) || flag  BOOTM_STATE_OS_PREP) {
 + s = getenv(machid);
 + if (s) {
 + strict_strtoul(s, 16, (long unsigned int *) machid);
 + printf(Using machid 0x%x from environment\n, machid);
 + }
 +
 + show_boot_progress(15);
  
  #ifdef CONFIG_OF_LIBFDT
 - if (images-ft_len)
 - return bootm_linux_fdt(machid, images);
 + if (images-ft_len)
 + return bootm_linux_fdt(machid, images, flag);
  #endif
  
 - kernel_entry = (void (*)(int, int, uint))images-ep;
 + kernel_entry = (void (*)(int, int, uint))images-ep;
  
 - debug (## Transferring control to Linux (at address %08lx) ...\n,
 -(ulong) kernel_entry);
 + debug(## Transferring control to Linux (at address %08lx) \
 + ...\n, (ulong) kernel_entry);
  
  #if defined (CONFIG_SETUP_MEMORY_TAGS) || \
  defined (CONFIG_CMDLINE_TAG) || \
  defined (CONFIG_INITRD_TAG) || \
  defined (CONFIG_SERIAL_TAG) || \
  defined (CONFIG_REVISION_TAG)
 - setup_start_tag (bd);
 + setup_start_tag(bd);
  #ifdef CONFIG_SERIAL_TAG
 - setup_serial_tag (params);
 + setup_serial_tag(params);
  #endif
  #ifdef CONFIG_REVISION_TAG
 - setup_revision_tag (params);
 + setup_revision_tag(params);
  #endif
  #ifdef CONFIG_SETUP_MEMORY_TAGS
 - setup_memory_tags (bd);
 + setup_memory_tags(bd);
  #endif
  #ifdef CONFIG_CMDLINE_TAG
 - setup_commandline_tag (bd, commandline);
 + setup_commandline_tag(bd, commandline);
  #endif
  #ifdef CONFIG_INITRD_TAG
 - if (images-rd_start  images-rd_end)
 - setup_initrd_tag (bd, images-rd_start, images-rd_end);
 + if (images-rd_start  images-rd_end)
 + setup_initrd_tag(bd, images-rd_start, images-rd_end);
  #endif
 - setup_end_tag(bd);
 + setup_end_tag(bd);
  #endif
 + if (flag  BOOTM_STATE_OS_PREP)
 + return 0;
 + }
  
 - announce_and_cleanup();
 -
 - kernel_entry(0, machid, bd-bi_boot_params);
 - /* does not return */
 + if (flag == 0 || flag  BOOTM_STATE_OS_GO) {

flag == 0? Shouldn't that be flag == BOOTM_STATE_OS_GO?

 + announce_and_cleanup();
  
 + kernel_entry(0, machid, bd-bi_boot_params);
 + /* does not return */
 + }
   return 1;
  }
  
 @@ -174,10 +181,10 @@ static int fixup_memory_node(void *blob)
   return fdt_fixup_memory_banks(blob, start, size,

Re: [U-Boot] [PATCH V3 1/8] arm: Add Prep subcommand support to bootm

Re: [U-Boot] [PATCH V3 1/8] arm: Add Prep subcommand support to bootm

[U-Boot] [PATCH V3 1/8] arm: Add Prep subcommand support to bootm

Re: [U-Boot] [PATCH V3 1/8] arm: Add Prep subcommand support to bootm

4 matches

Site Navigation

Mail list logo

Footer information