diff mbox

[u-boot,v4,2/3] bootm: relocate ramdisk if CONFIG_SYS_BOOT_RAMDISK_HIGH set

Message ID 1481133996-29973-3-git-send-email-raltherr@google.com
State Accepted, archived
Headers show

Commit Message

Rick Altherr Dec. 7, 2016, 6:06 p.m. UTC 
In 35fc84f, bootm was refactored so plain 'bootm' and
'bootm <subcommand>' shared a common implementation.
The 'bootm ramdisk' command implementation is now part of the common
implementation but not invoke by plain 'bootm' since the original
implementation never did ramdisk relocation.  Instead, ramdisk
relocation happened in image_setup_linux() which is typically called
during the OS portion of 'bootm'.

On ARM, parameters to the Linux kernel can either be passed by FDT or
ATAGS. When using FDT, image_setup_linux() is called which also triggers
ramdisk relocation.  When using ATAGS, image_setup_linux() is _not_
called because it mostly does FDT setup.

Instead of calling image_setup_linux() in both FDT and ATAGS cases,
include BOOTM_STATE_RAMDISK in the requested states during a plain
'bootm' if CONFIG_SYS_BOOT_RAMDISK_HIGH is set and remove the ramdisk
relocation from image_setup_linux().  This causes ramdisk relocation to
happen on any system where CONFIG_SYS_BOOT_RAMDISK_HIGH regardless of
the OS being booted. Also remove IMAGE_ENABLE_RAMDISK_HIGH as it was
only used by the now-removed code from image_setup_linux().

Signed-off-by: Rick Altherr <raltherr@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v4:
- Remove unused variables from image_setup_linux()

Changes in v3:
- Remove unused #define IMAGE_ENABLE_RAMDISK_HIGH

Changes in v2:
- Fix compiler warning caused by ramdisk relocation hack

 cmd/bootm.c     |  3 +++
 common/image.c  | 10 ----------
 include/image.h |  6 ------
 3 files changed, 3 insertions(+), 16 deletions(-)

Comments

Cédric Le Goater Dec. 7, 2016, 6:24 p.m. UTC | #1 
On 12/07/2016 07:06 PM, Rick Altherr wrote:
> In 35fc84f, bootm was refactored so plain 'bootm' and
> 'bootm <subcommand>' shared a common implementation.
> The 'bootm ramdisk' command implementation is now part of the common
> implementation but not invoke by plain 'bootm' since the original
> implementation never did ramdisk relocation.  Instead, ramdisk
> relocation happened in image_setup_linux() which is typically called
> during the OS portion of 'bootm'.
> 
> On ARM, parameters to the Linux kernel can either be passed by FDT or
> ATAGS. When using FDT, image_setup_linux() is called which also triggers
> ramdisk relocation.  When using ATAGS, image_setup_linux() is _not_
> called because it mostly does FDT setup.
> 
> Instead of calling image_setup_linux() in both FDT and ATAGS cases,
> include BOOTM_STATE_RAMDISK in the requested states during a plain
> 'bootm' if CONFIG_SYS_BOOT_RAMDISK_HIGH is set and remove the ramdisk
> relocation from image_setup_linux().  This causes ramdisk relocation to
> happen on any system where CONFIG_SYS_BOOT_RAMDISK_HIGH regardless of
> the OS being booted. Also remove IMAGE_ENABLE_RAMDISK_HIGH as it was
> only used by the now-removed code from image_setup_linux().
> 
> Signed-off-by: Rick Altherr <raltherr@google.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

looks good.

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C. 

> ---
> 
> Changes in v4:
> - Remove unused variables from image_setup_linux()
> 
> Changes in v3:
> - Remove unused #define IMAGE_ENABLE_RAMDISK_HIGH
> 
> Changes in v2:
> - Fix compiler warning caused by ramdisk relocation hack
> 
>  cmd/bootm.c     |  3 +++
>  common/image.c  | 10 ----------
>  include/image.h |  6 ------
>  3 files changed, 3 insertions(+), 16 deletions(-)
> 
> diff --git a/cmd/bootm.c b/cmd/bootm.c
> index 16fdea5..8da750e 100644
> --- a/cmd/bootm.c
> +++ b/cmd/bootm.c
> @@ -131,6 +131,9 @@ int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  	return do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
>  		BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER |
>  		BOOTM_STATE_LOADOS |
> +#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
> +		BOOTM_STATE_RAMDISK |
> +#endif
>  #if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
>  		BOOTM_STATE_OS_CMDLINE |
>  #endif
> diff --git a/common/image.c b/common/image.c
> index 0be09e5..f9ee9d5 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -1453,10 +1453,7 @@ int image_setup_linux(bootm_headers_t *images)
>  {
>  	ulong of_size = images->ft_len;
>  	char **of_flat_tree = &images->ft_addr;
> -	ulong *initrd_start = &images->initrd_start;
> -	ulong *initrd_end = &images->initrd_end;
>  	struct lmb *lmb = &images->lmb;
> -	ulong rd_len;
>  	int ret;
>  
>  	if (IMAGE_ENABLE_OF_LIBFDT)
> @@ -1470,13 +1467,6 @@ int image_setup_linux(bootm_headers_t *images)
>  			return ret;
>  		}
>  	}
> -	if (IMAGE_ENABLE_RAMDISK_HIGH) {
> -		rd_len = images->rd_end - images->rd_start;
> -		ret = boot_ramdisk_high(lmb, images->rd_start, rd_len,
> -				initrd_start, initrd_end);
> -		if (ret)
> -			return ret;
> -	}
>  
>  	if (IMAGE_ENABLE_OF_LIBFDT) {
>  		ret = boot_relocate_fdt(lmb, of_flat_tree, &of_size);
> diff --git a/include/image.h b/include/image.h
> index d788c26..a16187f 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -99,12 +99,6 @@ struct lmb;
>  
>  #endif /* IMAGE_ENABLE_FIT */
>  
> -#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
> -# define IMAGE_ENABLE_RAMDISK_HIGH	1
> -#else
> -# define IMAGE_ENABLE_RAMDISK_HIGH	0
> -#endif
> -
>  #ifdef CONFIG_SYS_BOOT_GET_CMDLINE
>  # define IMAGE_BOOT_GET_CMDLINE		1
>  #else
>
Joel Stanley Dec. 7, 2016, 11:24 p.m. UTC | #2 
On Thu, Dec 8, 2016 at 4:54 AM, Cédric Le Goater <clg@kaod.org> wrote:
> On 12/07/2016 07:06 PM, Rick Altherr wrote:
>> In 35fc84f, bootm was refactored so plain 'bootm' and
>> 'bootm <subcommand>' shared a common implementation.
>> The 'bootm ramdisk' command implementation is now part of the common
>> implementation but not invoke by plain 'bootm' since the original
>> implementation never did ramdisk relocation.  Instead, ramdisk
>> relocation happened in image_setup_linux() which is typically called
>> during the OS portion of 'bootm'.
>>
>> On ARM, parameters to the Linux kernel can either be passed by FDT or
>> ATAGS. When using FDT, image_setup_linux() is called which also triggers
>> ramdisk relocation.  When using ATAGS, image_setup_linux() is _not_
>> called because it mostly does FDT setup.
>>
>> Instead of calling image_setup_linux() in both FDT and ATAGS cases,
>> include BOOTM_STATE_RAMDISK in the requested states during a plain
>> 'bootm' if CONFIG_SYS_BOOT_RAMDISK_HIGH is set and remove the ramdisk
>> relocation from image_setup_linux().  This causes ramdisk relocation to
>> happen on any system where CONFIG_SYS_BOOT_RAMDISK_HIGH regardless of
>> the OS being booted. Also remove IMAGE_ENABLE_RAMDISK_HIGH as it was
>> only used by the now-removed code from image_setup_linux().
>>
>> Signed-off-by: Rick Altherr <raltherr@google.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> looks good.
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Joel Stanley <joel@jms.id.au>

Rick, can you please submit this one upstream too?

Please keep all of our reviewed-by tags but drop the version when you do so.

Cheers,

Joel
diff mbox

Patch

diff --git a/cmd/bootm.c b/cmd/bootm.c
index 16fdea5..8da750e 100644
--- a/cmd/bootm.c
+++ b/cmd/bootm.c
@@ -131,6 +131,9 @@  int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
 		BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER |
 		BOOTM_STATE_LOADOS |
+#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
+		BOOTM_STATE_RAMDISK |
+#endif
 #if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
 		BOOTM_STATE_OS_CMDLINE |
 #endif
diff --git a/common/image.c b/common/image.c
index 0be09e5..f9ee9d5 100644
--- a/common/image.c
+++ b/common/image.c
@@ -1453,10 +1453,7 @@  int image_setup_linux(bootm_headers_t *images)
 {
 	ulong of_size = images->ft_len;
 	char **of_flat_tree = &images->ft_addr;
-	ulong *initrd_start = &images->initrd_start;
-	ulong *initrd_end = &images->initrd_end;
 	struct lmb *lmb = &images->lmb;
-	ulong rd_len;
 	int ret;
 
 	if (IMAGE_ENABLE_OF_LIBFDT)
@@ -1470,13 +1467,6 @@  int image_setup_linux(bootm_headers_t *images)
 			return ret;
 		}
 	}
-	if (IMAGE_ENABLE_RAMDISK_HIGH) {
-		rd_len = images->rd_end - images->rd_start;
-		ret = boot_ramdisk_high(lmb, images->rd_start, rd_len,
-				initrd_start, initrd_end);
-		if (ret)
-			return ret;
-	}
 
 	if (IMAGE_ENABLE_OF_LIBFDT) {
 		ret = boot_relocate_fdt(lmb, of_flat_tree, &of_size);
diff --git a/include/image.h b/include/image.h
index d788c26..a16187f 100644
--- a/include/image.h
+++ b/include/image.h
@@ -99,12 +99,6 @@  struct lmb;
 
 #endif /* IMAGE_ENABLE_FIT */
 
-#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
-# define IMAGE_ENABLE_RAMDISK_HIGH	1
-#else
-# define IMAGE_ENABLE_RAMDISK_HIGH	0
-#endif
-
 #ifdef CONFIG_SYS_BOOT_GET_CMDLINE
 # define IMAGE_BOOT_GET_CMDLINE		1
 #else