All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 0/3] introduce genimg_get_kernel_addr()
@ 2014-08-05  0:43 Bryan Wu
  2014-08-05  0:43 ` [U-Boot] [PATCH v2 1/3] image: " Bryan Wu
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Bryan Wu @ 2014-08-05  0:43 UTC (permalink / raw)
  To: u-boot

When trying to fix the error message issue in pxe sysboot, we found
it's need a real kernel address for further image format detection.

So I take out such code from boot_get_kernel() of bootm.c and create
the new API functin genimg_get_kernel_addr(). Then convert pxe/sysboot
and bootm.c to use it.

v2:
  - move function description to header file
  - fix coding style issue
  - use argc < 1 check when calling genimg_get_kernel_addr() in bootm.c

Bryan Wu (3):
  image: introduce genimg_get_kernel_addr()
  pxe: detect image format before calling bootm/bootz
  bootm: use genimg_get_kernel_addr()

 common/bootm.c   | 25 +++++--------------------
 common/cmd_pxe.c | 15 +++++++++++----
 common/image.c   | 32 ++++++++++++++++++++++++++++++++
 include/image.h  | 12 ++++++++++++
 4 files changed, 60 insertions(+), 24 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 1/3] image: introduce genimg_get_kernel_addr()
  2014-08-05  0:43 [U-Boot] [PATCH v2 0/3] introduce genimg_get_kernel_addr() Bryan Wu
@ 2014-08-05  0:43 ` Bryan Wu
  2014-08-05 12:07   ` Simon Glass
  2014-08-05  0:43 ` [U-Boot] [PATCH v2 2/3] pxe: detect image format before calling bootm/bootz Bryan Wu
  2014-08-05  0:43 ` [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr() Bryan Wu
  2 siblings, 1 reply; 13+ messages in thread
From: Bryan Wu @ 2014-08-05  0:43 UTC (permalink / raw)
  To: u-boot

Kernel address is normally stored as a string argument of bootm or bootz.
This function is taken out from boot_get_kernel() of bootm.c, which can be
reused by others.

Signed-off-by: Bryan Wu <pengw@nvidia.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
 common/image.c  | 32 ++++++++++++++++++++++++++++++++
 include/image.h | 12 ++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/common/image.c b/common/image.c
index 11b3cf5..36fe487 100644
--- a/common/image.c
+++ b/common/image.c
@@ -642,6 +642,38 @@ int genimg_get_comp_id(const char *name)
 }
 
 #ifndef USE_HOSTCC
+ulong genimg_get_kernel_addr(char * const img_addr)
+{
+#if defined(CONFIG_FIT)
+	const char *fit_uname_config = NULL;
+	const char *fit_uname_kernel = NULL;
+#endif
+	ulong kernel_addr;
+
+	/* find out kernel image address */
+	if (!img_addr) {
+		kernel_addr = load_addr;
+		debug("*  kernel: default image load address = 0x%08lx\n",
+		      load_addr);
+#if defined(CONFIG_FIT)
+	} else if (fit_parse_conf(img_addr, load_addr, &kernel_addr,
+				  fit_uname_config)) {
+		debug("*  kernel: config '%s' from image at 0x%08lx\n",
+		      *fit_uname_config, kernel_addr);
+	} else if (fit_parse_subimage(img_addr, load_addr, &kernel_addr,
+				     fit_uname_kernel)) {
+		debug("*  kernel: subimage '%s' from image at 0x%08lx\n",
+		      *fit_uname_kernel, kernel_addr);
+#endif
+	} else {
+		kernel_addr = simple_strtoul(img_addr, NULL, 16);
+		debug("*  kernel: cmdline image address = 0x%08lx\n",
+		      kernel_addr);
+	}
+
+	return kernel_addr;
+}
+
 /**
  * genimg_get_format - get image format type
  * @img_addr: image start address
diff --git a/include/image.h b/include/image.h
index 3e8f78d..049a400 100644
--- a/include/image.h
+++ b/include/image.h
@@ -424,6 +424,18 @@ enum fit_load_op {
 #define IMAGE_FORMAT_FIT	0x02	/* new, libfdt based format */
 #define IMAGE_FORMAT_ANDROID	0x03	/* Android boot image */
 
+/**
+ * genimg_get_kernel_addr - get the real kernel address
+ * @img_addr: a string might contain real image address. If img_addr is NULL,
+ *            return kernel_addr as load_addr.
+ *
+ * genimg_get_kernel_addr() get the real kernel start address from a string
+ * which is normally the first argv of bootm/bootz
+ *
+ * returns:
+ *     kernel start address
+ */
+ulong genimg_get_kernel_addr(char * const img_addr);
 int genimg_get_format(const void *img_addr);
 int genimg_has_config(bootm_headers_t *images);
 ulong genimg_get_image(ulong img_addr);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 2/3] pxe: detect image format before calling bootm/bootz
  2014-08-05  0:43 [U-Boot] [PATCH v2 0/3] introduce genimg_get_kernel_addr() Bryan Wu
  2014-08-05  0:43 ` [U-Boot] [PATCH v2 1/3] image: " Bryan Wu
@ 2014-08-05  0:43 ` Bryan Wu
  2014-08-05  0:43 ` [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr() Bryan Wu
  2 siblings, 0 replies; 13+ messages in thread
From: Bryan Wu @ 2014-08-05  0:43 UTC (permalink / raw)
  To: u-boot

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.

This patch needs new function genimg_get_kernel_addr().

Signed-off-by: Bryan Wu <pengw@nvidia.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
 common/cmd_pxe.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
index ba48692..59b3598 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+
  */
@@ -609,6 +610,8 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)
 	char *bootargs;
 	int bootm_argc = 3;
 	int len = 0;
+	ulong kernel_addr;
+	void *buf;
 
 	label_print(label);
 
@@ -771,11 +774,15 @@ 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);
-
+	kernel_addr = genimg_get_kernel_addr(bootm_argv[1]);
+	buf = map_sysmem(kernel_addr, 0);
+	/* Try bootm for legacy and FIT format image */
+	if (genimg_get_format(buf) != 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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
  2014-08-05  0:43 [U-Boot] [PATCH v2 0/3] introduce genimg_get_kernel_addr() Bryan Wu
  2014-08-05  0:43 ` [U-Boot] [PATCH v2 1/3] image: " Bryan Wu
  2014-08-05  0:43 ` [U-Boot] [PATCH v2 2/3] pxe: detect image format before calling bootm/bootz Bryan Wu
@ 2014-08-05  0:43 ` Bryan Wu
  2014-08-05 12:08   ` Simon Glass
  2014-08-15  1:00   ` York Sun
  2 siblings, 2 replies; 13+ messages in thread
From: Bryan Wu @ 2014-08-05  0:43 UTC (permalink / raw)
  To: u-boot

Use the new API which is originally taken out from boot_get_kernel
of bootm.c

Signed-off-by: Bryan Wu <pengw@nvidia.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
 common/bootm.c | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/common/bootm.c b/common/bootm.c
index 7ec2ed8..621bff2 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -731,26 +731,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
 	int		os_noffset;
 #endif
 
-	/* find out kernel image address */
-	if (argc < 1) {
-		img_addr = load_addr;
-		debug("*  kernel: default image load address = 0x%08lx\n",
-		      load_addr);
-#if defined(CONFIG_FIT)
-	} else if (fit_parse_conf(argv[0], load_addr, &img_addr,
-				  &fit_uname_config)) {
-		debug("*  kernel: config '%s' from image at 0x%08lx\n",
-		      fit_uname_config, img_addr);
-	} else if (fit_parse_subimage(argv[0], load_addr, &img_addr,
-				     &fit_uname_kernel)) {
-		debug("*  kernel: subimage '%s' from image@0x%08lx\n",
-		      fit_uname_kernel, img_addr);
-#endif
-	} else {
-		img_addr = simple_strtoul(argv[0], NULL, 16);
-		debug("*  kernel: cmdline image address = 0x%08lx\n",
-		      img_addr);
-	}
+	img_addr = genimg_get_kernel_addr(argc < 1 ? NULL : argv[0]);
 
 	bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
 
@@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
 #endif
 #if defined(CONFIG_FIT)
 	case IMAGE_FORMAT_FIT:
+		if (!fit_parse_conf(argv[0], load_addr, &img_addr,
+					fit_uname_config))
+			fit_parse_subimage(argv[0], load_addr, &img_addr,
+					fit_uname_kernel);
 		os_noffset = fit_image_load(images, img_addr,
 				&fit_uname_kernel, &fit_uname_config,
 				IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 1/3] image: introduce genimg_get_kernel_addr()
  2014-08-05  0:43 ` [U-Boot] [PATCH v2 1/3] image: " Bryan Wu
@ 2014-08-05 12:07   ` Simon Glass
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2014-08-05 12:07 UTC (permalink / raw)
  To: u-boot

On 4 August 2014 18:43, Bryan Wu <cooloney@gmail.com> wrote:
> Kernel address is normally stored as a string argument of bootm or bootz.
> This function is taken out from boot_get_kernel() of bootm.c, which can be
> reused by others.
>
> Signed-off-by: Bryan Wu <pengw@nvidia.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
  2014-08-05  0:43 ` [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr() Bryan Wu
@ 2014-08-05 12:08   ` Simon Glass
  2014-08-15  1:00   ` York Sun
  1 sibling, 0 replies; 13+ messages in thread
From: Simon Glass @ 2014-08-05 12:08 UTC (permalink / raw)
  To: u-boot

On 4 August 2014 18:43, Bryan Wu <cooloney@gmail.com> wrote:
> Use the new API which is originally taken out from boot_get_kernel
> of bootm.c
>
> Signed-off-by: Bryan Wu <pengw@nvidia.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
  2014-08-05  0:43 ` [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr() Bryan Wu
  2014-08-05 12:08   ` Simon Glass
@ 2014-08-15  1:00   ` York Sun
  2014-08-15  1:05     ` Bryan Wu
  2014-08-15  1:38     ` Tom Rini
  1 sibling, 2 replies; 13+ messages in thread
From: York Sun @ 2014-08-15  1:00 UTC (permalink / raw)
  To: u-boot

On 08/04/2014 05:43 PM, Bryan Wu wrote:
> Use the new API which is originally taken out from boot_get_kernel
> of bootm.c
> 
> Signed-off-by: Bryan Wu <pengw@nvidia.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
> ---
>  common/bootm.c | 25 +++++--------------------
>  1 file changed, 5 insertions(+), 20 deletions(-)
> 
> diff --git a/common/bootm.c b/common/bootm.c
> index 7ec2ed8..621bff2 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c

<snip>

>  	bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
>  
> @@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
>  #endif
>  #if defined(CONFIG_FIT)
>  	case IMAGE_FORMAT_FIT:
> +		if (!fit_parse_conf(argv[0], load_addr, &img_addr,
> +					fit_uname_config))
> +			fit_parse_subimage(argv[0], load_addr, &img_addr,
> +					fit_uname_kernel);
>  		os_noffset = fit_image_load(images, img_addr,
>  				&fit_uname_kernel, &fit_uname_config,
>  				IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
> 

Did anyone test this addition? It breaks my booting. The variable img_addr get
changed for calling fit_parse_conf(). If I don't use load_addr variable and run
"bootm <addr>" with a FIT image, this will fail. It took me hours to figure out.

Why is this change needed? What does it fix?

York

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
  2014-08-15  1:00   ` York Sun
@ 2014-08-15  1:05     ` Bryan Wu
  2014-08-15  1:15       ` York Sun
  2014-08-15  1:38     ` Tom Rini
  1 sibling, 1 reply; 13+ messages in thread
From: Bryan Wu @ 2014-08-15  1:05 UTC (permalink / raw)
  To: u-boot

On Thu, Aug 14, 2014 at 6:00 PM, York Sun <yorksun@freescale.com> wrote:
> On 08/04/2014 05:43 PM, Bryan Wu wrote:
>> Use the new API which is originally taken out from boot_get_kernel
>> of bootm.c
>>
>> Signed-off-by: Bryan Wu <pengw@nvidia.com>
>> Tested-by: Stephen Warren <swarren@nvidia.com>
>> Reviewed-by: Stephen Warren <swarren@nvidia.com>
>> ---
>>  common/bootm.c | 25 +++++--------------------
>>  1 file changed, 5 insertions(+), 20 deletions(-)
>>
>> diff --git a/common/bootm.c b/common/bootm.c
>> index 7ec2ed8..621bff2 100644
>> --- a/common/bootm.c
>> +++ b/common/bootm.c
>
> <snip>
>
>>       bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
>>
>> @@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
>>  #endif
>>  #if defined(CONFIG_FIT)
>>       case IMAGE_FORMAT_FIT:
>> +             if (!fit_parse_conf(argv[0], load_addr, &img_addr,
>> +                                     fit_uname_config))
>> +                     fit_parse_subimage(argv[0], load_addr, &img_addr,
>> +                                     fit_uname_kernel);
>>               os_noffset = fit_image_load(images, img_addr,
>>                               &fit_uname_kernel, &fit_uname_config,
>>                               IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
>>
>
> Did anyone test this addition? It breaks my booting. The variable img_addr get
> changed for calling fit_parse_conf(). If I don't use load_addr variable and run
> "bootm <addr>" with a FIT image, this will fail. It took me hours to figure out.
>
> Why is this change needed? What does it fix?
>
> York
>

I think Tom merged one with the fixing:

----
+             if (!fit_parse_conf(argv[0], load_addr, &img_addr,
+                                     &fit_uname_config))
+                     fit_parse_subimage(argv[0], load_addr, &img_addr,
+                                     &fit_uname_kernel);
----

Can you try that?
-Bryan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
  2014-08-15  1:05     ` Bryan Wu
@ 2014-08-15  1:15       ` York Sun
  0 siblings, 0 replies; 13+ messages in thread
From: York Sun @ 2014-08-15  1:15 UTC (permalink / raw)
  To: u-boot

Sorry for top posting from my phone.

I tested with the latest code merged, not your original patch.

York

________________________________
From: Bryan Wu
Sent: Thu, 14/08/2014 18:05
To: Sun York-R58495 <yorksun@freescale.com>
CC: Tom Rini <trini@ti.com>; Stephen Warren <swarren@wwwdotorg.org>; u-boot at lists.denx.de; simon Glass <sjg@chromium.org>; Basu Arnab-B45036 <arnab.basu@freescale.com>
Subject: Re: [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()


On Thu, Aug 14, 2014 at 6:00 PM, York Sun <yorksun@freescale.com> wrote:
> On 08/04/2014 05:43 PM, Bryan Wu wrote:
>> Use the new API which is originally taken out from boot_get_kernel
>> of bootm.c
>>
>> Signed-off-by: Bryan Wu <pengw@nvidia.com>
>> Tested-by: Stephen Warren <swarren@nvidia.com>
>> Reviewed-by: Stephen Warren <swarren@nvidia.com>
>> ---
>>  common/bootm.c | 25 +++++--------------------
>>  1 file changed, 5 insertions(+), 20 deletions(-)
>>
>> diff --git a/common/bootm.c b/common/bootm.c
>> index 7ec2ed8..621bff2 100644
>> --- a/common/bootm.c
>> +++ b/common/bootm.c
>
> <snip>
>
>>       bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
>>
>> @@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
>>  #endif
>>  #if defined(CONFIG_FIT)
>>       case IMAGE_FORMAT_FIT:
>> +             if (!fit_parse_conf(argv[0], load_addr, &img_addr,
>> +                                     fit_uname_config))
>> +                     fit_parse_subimage(argv[0], load_addr, &img_addr,
>> +                                     fit_uname_kernel);
>>               os_noffset = fit_image_load(images, img_addr,
>>                               &fit_uname_kernel, &fit_uname_config,
>>                               IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
>>
>
> Did anyone test this addition? It breaks my booting. The variable img_addr get
> changed for calling fit_parse_conf(). If I don't use load_addr variable and run
> "bootm <addr>" with a FIT image, this will fail. It took me hours to figure out.
>
> Why is this change needed? What does it fix?
>
> York
>

I think Tom merged one with the fixing:

----
+             if (!fit_parse_conf(argv[0], load_addr, &img_addr,
+                                     &fit_uname_config))
+                     fit_parse_subimage(argv[0], load_addr, &img_addr,
+                                     &fit_uname_kernel);
----

Can you try that?
-Bryan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
  2014-08-15  1:00   ` York Sun
  2014-08-15  1:05     ` Bryan Wu
@ 2014-08-15  1:38     ` Tom Rini
  2014-08-15  1:42       ` York Sun
  1 sibling, 1 reply; 13+ messages in thread
From: Tom Rini @ 2014-08-15  1:38 UTC (permalink / raw)
  To: u-boot

On Thu, Aug 14, 2014 at 06:00:31PM -0700, York Sun wrote:
> On 08/04/2014 05:43 PM, Bryan Wu wrote:
> > Use the new API which is originally taken out from boot_get_kernel
> > of bootm.c
> > 
> > Signed-off-by: Bryan Wu <pengw@nvidia.com>
> > Tested-by: Stephen Warren <swarren@nvidia.com>
> > Reviewed-by: Stephen Warren <swarren@nvidia.com>
> > ---
> >  common/bootm.c | 25 +++++--------------------
> >  1 file changed, 5 insertions(+), 20 deletions(-)
> > 
> > diff --git a/common/bootm.c b/common/bootm.c
> > index 7ec2ed8..621bff2 100644
> > --- a/common/bootm.c
> > +++ b/common/bootm.c
> 
> <snip>
> 
> >  	bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
> >  
> > @@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
> >  #endif
> >  #if defined(CONFIG_FIT)
> >  	case IMAGE_FORMAT_FIT:
> > +		if (!fit_parse_conf(argv[0], load_addr, &img_addr,
> > +					fit_uname_config))
> > +			fit_parse_subimage(argv[0], load_addr, &img_addr,
> > +					fit_uname_kernel);
> >  		os_noffset = fit_image_load(images, img_addr,
> >  				&fit_uname_kernel, &fit_uname_config,
> >  				IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
> > 
> 
> Did anyone test this addition? It breaks my booting. The variable img_addr get
> changed for calling fit_parse_conf(). If I don't use load_addr variable and run
> "bootm <addr>" with a FIT image, this will fail. It took me hours to figure out.

How exactly are you booting?  I tested a fit with bootm addr#conf at 1 or
so.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140814/b7f54ff8/attachment.pgp>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
  2014-08-15  1:38     ` Tom Rini
@ 2014-08-15  1:42       ` York Sun
  2014-08-15  3:48         ` Simon Glass
  0 siblings, 1 reply; 13+ messages in thread
From: York Sun @ 2014-08-15  1:42 UTC (permalink / raw)
  To: u-boot

Tom,

I tested with "bootm 806f0000". My FIT image is loaded there. I don't have load_addr variable. My default load_addr (from CONFIG macro) is different from this address.

York

________________________________
From: Tom Rini
Sent: Thu, 14/08/2014 18:38
To: Sun York-R58495 <yorksun@freescale.com>
CC: Bryan Wu <cooloney@gmail.com>; sjg at chromium.org; swarren at wwwdotorg.org; u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()


On Thu, Aug 14, 2014 at 06:00:31PM -0700, York Sun wrote:
> On 08/04/2014 05:43 PM, Bryan Wu wrote:
> > Use the new API which is originally taken out from boot_get_kernel
> > of bootm.c
> >
> > Signed-off-by: Bryan Wu <pengw@nvidia.com>
> > Tested-by: Stephen Warren <swarren@nvidia.com>
> > Reviewed-by: Stephen Warren <swarren@nvidia.com>
> > ---
> >  common/bootm.c | 25 +++++--------------------
> >  1 file changed, 5 insertions(+), 20 deletions(-)
> >
> > diff --git a/common/bootm.c b/common/bootm.c
> > index 7ec2ed8..621bff2 100644
> > --- a/common/bootm.c
> > +++ b/common/bootm.c
>
> <snip>
>
> >      bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
> >
> > @@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
> >  #endif
> >  #if defined(CONFIG_FIT)
> >      case IMAGE_FORMAT_FIT:
> > +           if (!fit_parse_conf(argv[0], load_addr, &img_addr,
> > +                                   fit_uname_config))
> > +                   fit_parse_subimage(argv[0], load_addr, &img_addr,
> > +                                   fit_uname_kernel);
> >              os_noffset = fit_image_load(images, img_addr,
> >                              &fit_uname_kernel, &fit_uname_config,
> >                              IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
> >
>
> Did anyone test this addition? It breaks my booting. The variable img_addr get
> changed for calling fit_parse_conf(). If I don't use load_addr variable and run
> "bootm <addr>" with a FIT image, this will fail. It took me hours to figure out.

How exactly are you booting?  I tested a fit with bootm addr#conf at 1 or
so.

--
Tom

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
  2014-08-15  1:42       ` York Sun
@ 2014-08-15  3:48         ` Simon Glass
  2014-08-15  4:02           ` Bryan Wu
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2014-08-15  3:48 UTC (permalink / raw)
  To: u-boot

Hi York,

On 14 August 2014 19:42, York Sun <yorksun@freescale.com> wrote:
> Tom,
>
> I tested with "bootm 806f0000". My FIT image is loaded there. I don't have
> load_addr variable. My default load_addr (from CONFIG macro) is different
> from this address.

Yes unfortunately this is completely wrong. I did not catch this in my
review unfortunately, perhaps because the & was removed in the code I
reviewed.

fit_uname_config and fit_uname_kernel must be passed back from
genimg_get_kernel_addr() - that function will need two new parameters.

Regards,
Simon

>
> York
>
> ________________________________
> From: Tom Rini
> Sent: Thu, 14/08/2014 18:38
> To: Sun York-R58495 <yorksun@freescale.com>
> CC: Bryan Wu <cooloney@gmail.com>; sjg at chromium.org; swarren at wwwdotorg.org;
> u-boot at lists.denx.de
>
> Subject: Re: [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
>
> On Thu, Aug 14, 2014 at 06:00:31PM -0700, York Sun wrote:
>> On 08/04/2014 05:43 PM, Bryan Wu wrote:
>> > Use the new API which is originally taken out from boot_get_kernel
>> > of bootm.c
>> >
>> > Signed-off-by: Bryan Wu <pengw@nvidia.com>
>> > Tested-by: Stephen Warren <swarren@nvidia.com>
>> > Reviewed-by: Stephen Warren <swarren@nvidia.com>
>> > ---
>> >  common/bootm.c | 25 +++++--------------------
>> >  1 file changed, 5 insertions(+), 20 deletions(-)
>> >
>> > diff --git a/common/bootm.c b/common/bootm.c
>> > index 7ec2ed8..621bff2 100644
>> > --- a/common/bootm.c
>> > +++ b/common/bootm.c
>>
>> <snip>
>>
>> >      bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
>> >
>> > @@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t
>> > *cmdtp, int flag, int argc,
>> >  #endif
>> >  #if defined(CONFIG_FIT)
>> >      case IMAGE_FORMAT_FIT:
>> > +           if (!fit_parse_conf(argv[0], load_addr, &img_addr,
>> > +                                   fit_uname_config))
>> > +                   fit_parse_subimage(argv[0], load_addr, &img_addr,
>> > +                                   fit_uname_kernel);
>> >              os_noffset = fit_image_load(images, img_addr,
>> >                              &fit_uname_kernel, &fit_uname_config,
>> >                              IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
>> >
>>
>> Did anyone test this addition? It breaks my booting. The variable img_addr
>> get
>> changed for calling fit_parse_conf(). If I don't use load_addr variable
>> and run
>> "bootm <addr>" with a FIT image, this will fail. It took me hours to
>> figure out.
>
> How exactly are you booting?  I tested a fit with bootm addr#conf at 1 or
> so.
>
> --
> Tom

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
  2014-08-15  3:48         ` Simon Glass
@ 2014-08-15  4:02           ` Bryan Wu
  0 siblings, 0 replies; 13+ messages in thread
From: Bryan Wu @ 2014-08-15  4:02 UTC (permalink / raw)
  To: u-boot

On Thu, Aug 14, 2014 at 8:48 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi York,
>
> On 14 August 2014 19:42, York Sun <yorksun@freescale.com> wrote:
>> Tom,
>>
>> I tested with "bootm 806f0000". My FIT image is loaded there. I don't have
>> load_addr variable. My default load_addr (from CONFIG macro) is different
>> from this address.
>
> Yes unfortunately this is completely wrong. I did not catch this in my
> review unfortunately, perhaps because the & was removed in the code I
> reviewed.
>
> fit_uname_config and fit_uname_kernel must be passed back from
> genimg_get_kernel_addr() - that function will need two new parameters.
>

I'm working on a fix to add this 2 new parameters.

-Bryan

>
>>
>> York
>>
>> ________________________________
>> From: Tom Rini
>> Sent: Thu, 14/08/2014 18:38
>> To: Sun York-R58495 <yorksun@freescale.com>
>> CC: Bryan Wu <cooloney@gmail.com>; sjg at chromium.org; swarren at wwwdotorg.org;
>> u-boot at lists.denx.de
>>
>> Subject: Re: [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
>>
>> On Thu, Aug 14, 2014 at 06:00:31PM -0700, York Sun wrote:
>>> On 08/04/2014 05:43 PM, Bryan Wu wrote:
>>> > Use the new API which is originally taken out from boot_get_kernel
>>> > of bootm.c
>>> >
>>> > Signed-off-by: Bryan Wu <pengw@nvidia.com>
>>> > Tested-by: Stephen Warren <swarren@nvidia.com>
>>> > Reviewed-by: Stephen Warren <swarren@nvidia.com>
>>> > ---
>>> >  common/bootm.c | 25 +++++--------------------
>>> >  1 file changed, 5 insertions(+), 20 deletions(-)
>>> >
>>> > diff --git a/common/bootm.c b/common/bootm.c
>>> > index 7ec2ed8..621bff2 100644
>>> > --- a/common/bootm.c
>>> > +++ b/common/bootm.c
>>>
>>> <snip>
>>>
>>> >      bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
>>> >
>>> > @@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t
>>> > *cmdtp, int flag, int argc,
>>> >  #endif
>>> >  #if defined(CONFIG_FIT)
>>> >      case IMAGE_FORMAT_FIT:
>>> > +           if (!fit_parse_conf(argv[0], load_addr, &img_addr,
>>> > +                                   fit_uname_config))
>>> > +                   fit_parse_subimage(argv[0], load_addr, &img_addr,
>>> > +                                   fit_uname_kernel);
>>> >              os_noffset = fit_image_load(images, img_addr,
>>> >                              &fit_uname_kernel, &fit_uname_config,
>>> >                              IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
>>> >
>>>
>>> Did anyone test this addition? It breaks my booting. The variable img_addr
>>> get
>>> changed for calling fit_parse_conf(). If I don't use load_addr variable
>>> and run
>>> "bootm <addr>" with a FIT image, this will fail. It took me hours to
>>> figure out.
>>
>> How exactly are you booting?  I tested a fit with bootm addr#conf at 1 or
>> so.
>>
>> --
>> Tom

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-08-15  4:02 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-05  0:43 [U-Boot] [PATCH v2 0/3] introduce genimg_get_kernel_addr() Bryan Wu
2014-08-05  0:43 ` [U-Boot] [PATCH v2 1/3] image: " Bryan Wu
2014-08-05 12:07   ` Simon Glass
2014-08-05  0:43 ` [U-Boot] [PATCH v2 2/3] pxe: detect image format before calling bootm/bootz Bryan Wu
2014-08-05  0:43 ` [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr() Bryan Wu
2014-08-05 12:08   ` Simon Glass
2014-08-15  1:00   ` York Sun
2014-08-15  1:05     ` Bryan Wu
2014-08-15  1:15       ` York Sun
2014-08-15  1:38     ` Tom Rini
2014-08-15  1:42       ` York Sun
2014-08-15  3:48         ` Simon Glass
2014-08-15  4:02           ` Bryan Wu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.