[U-Boot] [PATCH] pxe: detect image format before calling bootm/bootz

[U-Boot] [PATCH] pxe: detect image format before calling bootm/bootz

[Bryan Wu]

Trying bootm for zImage will print out several error message which
is not necessary for this case. So detect image format firstly, only
try bootm for legacy and FIT format image then try bootz for others.

Signed-off-by: Bryan Wu <pengw@nvidia.com>
---
 common/cmd_pxe.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
index ba48692..a9cf576 100644
--- a/common/cmd_pxe.c
+++ b/common/cmd_pxe.c
@@ -1,5 +1,6 @@
 /*
  * Copyright 2010-2011 Calxeda, Inc.
+ * Copyright (c) 2014, NVIDIA CORPORATION.  All rights reserved.
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */
@@ -771,11 +772,14 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)
 	if (bootm_argv[3])
 		bootm_argc = 4;
 
-	do_bootm(cmdtp, 0, bootm_argc, bootm_argv);
 
+	/* Try bootm for legacy and FIT format image */
+	if (genimg_get_format(bootm_argv[1]) != IMAGE_FORMAT_INVALID)
+		do_bootm(cmdtp, 0, bootm_argc, bootm_argv);
 #ifdef CONFIG_CMD_BOOTZ
-	/* Try booting a zImage if do_bootm returns */
-	do_bootz(cmdtp, 0, bootm_argc, bootm_argv);
+	/* Try booting a zImage */
+	else
+		do_bootz(cmdtp, 0, bootm_argc, bootm_argv);
 #endif
 	return 1;
 }
-- 
1.9.1

[U-Boot] [PATCH] pxe: detect image format before calling bootm/bootz

[Stephen Warren]

On 07/30/2014 04:54 PM, Bryan Wu wrote:
> Trying bootm for zImage will print out several error message which
> is not necessary for this case. So detect image format firstly, only
> try bootm for legacy and FIT format image then try bootz for others.

> diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c

> @@ -771,11 +772,14 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)
>   	if (bootm_argv[3])
>   		bootm_argc = 4;
>
> -	do_bootm(cmdtp, 0, bootm_argc, bootm_argv);
>
> +	/* Try bootm for legacy and FIT format image */
> +	if (genimg_get_format(bootm_argv[1]) != IMAGE_FORMAT_INVALID)
> +		do_bootm(cmdtp, 0, bootm_argc, bootm_argv);

This breaks the ability to boot a uImage, since the call to 
genimg_get_format() is incorrect.

genimg_get_format() takes the address of an image itself, not the 
address of a string containing a textual representation of the address.

Also, bootm_argv[1] might not be supplied at all, in which case, 
boot_get_kernel() falls back to using load_addr as the address.

This is further complicated by the fact that for FIT images, the string 
in bootm_argv[1] won't just be the address of the image, but perhaps 
have concatenated information indicating which sub-image to select 
(since FIT images pack multiple files into a single image).

You probably need to pull the start of boot_get_kernel() (in 
common/bootm.c) into a utility function and call that to distinguish 
between zImage/not, so as not to duplicate any of the logic.

(Yes, I failed to notice this when I reviewed this downstream, but I 
just actually tested this patch, using a uImage for pxe booting...)

For the zImage case, this patch certainly does remove the annoying error 
message generated when do_bootm() can't identify the image type. So, I 
think it's worth continuing to work on a solution.

P.S. you didn't CC anyone who might apply this patch.

[U-Boot] [PATCH] pxe: detect image format before calling bootm/bootz

[Bryan Wu]

On Thu, Jul 31, 2014 at 10:36 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 07/30/2014 04:54 PM, Bryan Wu wrote:
>>
>> Trying bootm for zImage will print out several error message which
>> is not necessary for this case. So detect image format firstly, only
>> try bootm for legacy and FIT format image then try bootz for others.
>
>
>> diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
>
>
>> @@ -771,11 +772,14 @@ static int label_boot(cmd_tbl_t *cmdtp, struct
>> pxe_label *label)
>>         if (bootm_argv[3])
>>                 bootm_argc = 4;
>>
>> -       do_bootm(cmdtp, 0, bootm_argc, bootm_argv);
>>
>> +       /* Try bootm for legacy and FIT format image */
>> +       if (genimg_get_format(bootm_argv[1]) != IMAGE_FORMAT_INVALID)
>> +               do_bootm(cmdtp, 0, bootm_argc, bootm_argv);
>
>
> This breaks the ability to boot a uImage, since the call to
> genimg_get_format() is incorrect.
>
> genimg_get_format() takes the address of an image itself, not the address of
> a string containing a textual representation of the address.
>
> Also, bootm_argv[1] might not be supplied at all, in which case,
> boot_get_kernel() falls back to using load_addr as the address.
>
> This is further complicated by the fact that for FIT images, the string in
> bootm_argv[1] won't just be the address of the image, but perhaps have
> concatenated information indicating which sub-image to select (since FIT
> images pack multiple files into a single image).
>
> You probably need to pull the start of boot_get_kernel() (in common/bootm.c)
> into a utility function and call that to distinguish between zImage/not, so
> as not to duplicate any of the logic.
>

My bad. I will fix that soon.

> (Yes, I failed to notice this when I reviewed this downstream, but I just
> actually tested this patch, using a uImage for pxe booting...)
>

Yeah, I also missed to test uImage.

> For the zImage case, this patch certainly does remove the annoying error
> message generated when do_bootm() can't identify the image type. So, I think
> it's worth continuing to work on a solution.
>
> P.S. you didn't CC anyone who might apply this patch.

So weird, I sent email to u-boot at list.denx.de, but it didn't show up
in the mail list. I think I've already subscribed the u-boot mail list

Thanks,
-Bryan