diff mbox

[U-Boot,v2,01/11] arm: Optionally use existing atags in bootm.c

Message ID 1335634011-9104-2-git-send-email-pali.rohar@gmail.com
State Changes Requested
Headers show

Commit Message

Pali Rohár April 28, 2012, 5:26 p.m. UTC
This patch adapts the bootm command so that it can use an existing atags command
set up by a previous bootloader. If the environment variable "atagaddr" is unset,
bootm behaves as normal. If "atagaddr" is set, bootm will use atags address from
environment variable and also append new boot args (if specified in u-boot). For
example, if a previous boot loader already set up the atags struct at 0x80000100:

setenv atagaddr 0x80000100; bootm 0x80008000

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
Changes since v1:
   - Rebased on u-boot master

Changes since original version:
   - Added info to README file
   - Added local define CONFIG_SETUP_ANY_TAG
   - Fixed compile warning
   - Fixed commit message
   - Check if atagaddr is not NULL

 README               |    2 ++
 arch/arm/lib/bootm.c |   60 ++++++++++++++++++++++++++++++++------------------
 2 files changed, 40 insertions(+), 22 deletions(-)

Comments

Wolfgang Denk April 28, 2012, 9:20 p.m. UTC | #1
Dear =?UTF-8?q?Pali=20Roh=C3=A1r?=,

In message <1335634011-9104-2-git-send-email-pali.rohar@gmail.com> you wrote:
> VGhpcyBwYXRjaCBhZGFwdHMgdGhlIGJvb3RtIGNvbW1hbmQgc28gdGhhdCBpdCBjYW4gdXNlIGFu
> IGV4aXN0aW5nIGF0YWdzIGNvbW1hbmQKc2V0IHVwIGJ5IGEgcHJldmlvdXMgYm9vdGxvYWRlci4g
> SWYgdGhlIGVudmlyb25tZW50IHZhcmlhYmxlICJhdGFnYWRkciIgaXMgdW5zZXQsCmJvb3RtIGJl
> aGF2ZXMgYXMgbm9ybWFsLiBJZiAiYXRhZ2FkZHIiIGlzIHNldCwgYm9vdG0gd2lsbCB1c2UgYXRh
> Z3MgYWRkcmVzcyBmcm9tCmVudmlyb25tZW50IHZhcmlhYmxlIGFuZCBhbHNvIGFwcGVuZCBuZXcg
...

Please stop posting your patches with base64 Content-transfer-encoding;
thanks.

Please also run all your patches through checkpatch _before_ posting,
and fix warnings and errors.

For example, this patch has a number of too long lines which cause
WARNING: line over 80 characters

Please fix all such issues, and resubmit.

Thanks.

Best regards,

Wolfgang Denk
Marek Vasut April 28, 2012, 10:15 p.m. UTC | #2
Dear Pali Rohár,

> This patch adapts the bootm command so that it can use an existing atags
> command set up by a previous bootloader. If the environment variable
> "atagaddr" is unset, bootm behaves as normal. If "atagaddr" is set, bootm
> will use atags address from environment variable and also append new boot
> args (if specified in u-boot). For example, if a previous boot loader
> already set up the atags struct at 0x80000100:

Won't it be easier to create a preprocessing function that'd fill gd properly, 
so uboot can generate the atags through standard means then?

> 
> setenv atagaddr 0x80000100; bootm 0x80008000
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> Changes since v1:
>    - Rebased on u-boot master
> 
> Changes since original version:
>    - Added info to README file
>    - Added local define CONFIG_SETUP_ANY_TAG
>    - Fixed compile warning
>    - Fixed commit message
>    - Check if atagaddr is not NULL
> 
>  README               |    2 ++
>  arch/arm/lib/bootm.c |   60
> ++++++++++++++++++++++++++++++++------------------ 2 files changed, 40
> insertions(+), 22 deletions(-)
> 
> diff --git a/README b/README
> index 43074cf..60ad9c2 100644
> --- a/README
> +++ b/README
> @@ -3687,6 +3687,8 @@ Some configuration options can be set using
> Environment Variables.
> 
>  List of environment variables (most likely not complete):
> 
> +  atagaddr	- bootm will use ATAGs struct from specified address (arm only)
> +
>    baudrate	- see CONFIG_BAUDRATE
> 
>    bootdelay	- see CONFIG_BOOTDELAY
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 599547d..0f3c97b 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -42,6 +42,10 @@ DECLARE_GLOBAL_DATA_PTR;
>  	defined(CONFIG_INITRD_TAG) || \
>  	defined(CONFIG_SERIAL_TAG) || \
>  	defined(CONFIG_REVISION_TAG)
> +		#define CONFIG_SETUP_ANY_TAG
> +#endif
> +
> +#ifdef CONFIG_SETUP_ANY_TAG
>  static struct tag *params;
>  #endif
> 
> @@ -106,11 +110,7 @@ static void announce_and_cleanup(void)
>  	cleanup_before_linux();
>  }
> 
> -#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
> -	defined(CONFIG_CMDLINE_TAG) || \
> -	defined(CONFIG_INITRD_TAG) || \
> -	defined(CONFIG_SERIAL_TAG) || \
> -	defined(CONFIG_REVISION_TAG)
> +#ifdef CONFIG_SETUP_ANY_TAG
>  static void setup_start_tag (bd_t *bd)
>  {
>  	params = (struct tag *)bd->bi_boot_params;
> @@ -217,11 +217,7 @@ void setup_revision_tag(struct tag **in_params)
>  }
>  #endif
> 
> -#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
> -	defined(CONFIG_CMDLINE_TAG) || \
> -	defined(CONFIG_INITRD_TAG) || \
> -	defined(CONFIG_SERIAL_TAG) || \
> -	defined(CONFIG_REVISION_TAG)
> +#ifdef CONFIG_SETUP_ANY_TAG
>  static void setup_end_tag(bd_t *bd)
>  {
>  	params->hdr.tag = ATAG_NONE;
> @@ -280,13 +276,23 @@ static void boot_prep_linux(bootm_headers_t *images)
>  	} else
>  #endif
>  	{
> -#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
> -	defined(CONFIG_CMDLINE_TAG) || \
> -	defined(CONFIG_INITRD_TAG) || \
> -	defined(CONFIG_SERIAL_TAG) || \
> -	defined(CONFIG_REVISION_TAG)
> +		char *atagaddr = getenv("atagaddr");
>  		debug("using: ATAGS\n");
> -		setup_start_tag(gd->bd);
> +
> +		if (atagaddr)
> +			gd->bd->bi_boot_params = simple_strtoul(atagaddr, NULL, 
16);
> +
> +		if (gd->bd->bi_boot_params) {
> +			printf("Using existing atags at %#lx\n", gd->bd-
>bi_boot_params);
> +
> +			params = (struct tag *) gd->bd->bi_boot_params;
> +			while (params->hdr.size > 0)
> +				params = tag_next(params);
> +		} else {
> +#ifdef CONFIG_SETUP_ANY_TAG
> +			setup_start_tag(gd->bd);
> +#endif
> +		}
>  #ifdef CONFIG_SERIAL_TAG
>  		setup_serial_tag(&params);
>  #endif
> @@ -297,18 +303,28 @@ static void boot_prep_linux(bootm_headers_t *images)
>  		setup_revision_tag(&params);
>  #endif
>  #ifdef CONFIG_SETUP_MEMORY_TAGS
> -		setup_memory_tags(gd->bd);
> +		if (!atagaddr)
> +			setup_memory_tags(gd->bd);
>  #endif
>  #ifdef CONFIG_INITRD_TAG
>  		if (images->rd_start && images->rd_end)
>  			setup_initrd_tag(gd->bd, images->rd_start,
>  			images->rd_end);
>  #endif
> -		setup_end_tag(gd->bd);
> -#else /* all tags */
> -		printf("FDT and ATAGS support not compiled in - hanging\n");
> -		hang();
> -#endif /* all tags */
> +		if (atagaddr) {
> +			if (params->hdr.size > 0)
> +				setup_end_tag(gd->bd);
> +		} else {
> +#ifdef CONFIG_SETUP_ANY_TAG
> +			setup_end_tag(gd->bd);
> +#endif
> +		}
> +#ifndef CONFIG_SETUP_ANY_TAG
> +		if (!atagaddr) {
> +			printf("FDT and ATAGS support not compiled in - 
hanging\n");
> +			hang();
> +		}
> +#endif
>  	}
>  }
Mike Frysinger April 28, 2012, 11:22 p.m. UTC | #3
On Saturday 28 April 2012 13:26:41 Pali Rohár wrote:
>  	defined(CONFIG_INITRD_TAG) || \
>  	defined(CONFIG_SERIAL_TAG) || \
>  	defined(CONFIG_REVISION_TAG)
> +		#define CONFIG_SETUP_ANY_TAG

don't indent the "#"
-mike
Pali Rohár April 29, 2012, 7:14 a.m. UTC | #4
On Sunday 29 April 2012 00:15:23 Marek Vasut wrote:
> Won't it be easier to create a preprocessing function that'd
> fill gd properly, so uboot can generate the atags through
> standard means then?

Do you mean to generate/copy other atags in board code? This will 
not work because u-boot (in bootm.c) always passing ATAG_CORE in 
function setup_start_tag. And I do not want to pass ATAG_CORE two 
times to kernel (once which I copy from other bootloader and once 
which generate u-boot).
Pali Rohár April 29, 2012, 7:20 a.m. UTC | #5
On Saturday 28 April 2012 23:20:04 Wolfgang Denk wrote:
> Please stop posting your patches with base64
> Content-transfer-encoding; thanks.

How can I tell this to git format-patch and git send-email? Also 
I looked in my mailbox and all emails have no base64 header, but 
Content-Transfer-Encoding: 8bit

> 
> Please also run all your patches through checkpatch _before_
> posting, and fix warnings and errors.

Ok, I forgot to run checkpatch on this patch. But I ran 
checkpatch on all other patches and checkpatch did not show any 
other warnings or errors.

> 
> For example, this patch has a number of too long lines which
> cause WARNING: line over 80 characters
> 
> Please fix all such issues, and resubmit.
> 
> Thanks.
> 
> Best regards,
> 
> Wolfgang Denk
Pali Rohár April 29, 2012, 7:57 a.m. UTC | #6
On Saturday 28 April 2012 19:22:38 Mike Frysinger wrote:
> On Saturday 28 April 2012 13:26:41 Pali Rohár wrote:
> >  	defined(CONFIG_INITRD_TAG) || \
> >  	defined(CONFIG_SERIAL_TAG) || \
> >  	defined(CONFIG_REVISION_TAG)
> > 
> > +		#define CONFIG_SETUP_ANY_TAG
> 
> don't indent the "#"
> -mike

Ok.
Marek Vasut April 29, 2012, 9:10 a.m. UTC | #7
Dear Pali Rohár,

> On Sunday 29 April 2012 00:15:23 Marek Vasut wrote:
> > Won't it be easier to create a preprocessing function that'd
> > fill gd properly, so uboot can generate the atags through
> > standard means then?
> 
> Do you mean to generate/copy other atags in board code? This will
> not work because u-boot (in bootm.c) always passing ATAG_CORE in
> function setup_start_tag. And I do not want to pass ATAG_CORE two
> times to kernel (once which I copy from other bootloader and once
> which generate u-boot).

No, I mean parse the old ATAGS from nolo and fill u-boot's internal structures 
with that. Then let uboot generate the ATAGS from it's internal structures as 
usual.

Best regards,
Marek Vasut
Pali Rohár April 29, 2012, 9:15 a.m. UTC | #8
On Sunday 29 April 2012 11:10:42 Marek Vasut wrote:
> Dear Pali Rohár,
> 
> > On Sunday 29 April 2012 00:15:23 Marek Vasut wrote:
> > > Won't it be easier to create a preprocessing function
> > > that'd
> > > fill gd properly, so uboot can generate the atags through
> > > standard means then?
> > 
> > Do you mean to generate/copy other atags in board code? This
> > will not work because u-boot (in bootm.c) always passing
> > ATAG_CORE in function setup_start_tag. And I do not want to
> > pass ATAG_CORE two times to kernel (once which I copy from
> > other bootloader and once which generate u-boot).
> 
> No, I mean parse the old ATAGS from nolo and fill u-boot's
> internal structures with that. Then let uboot generate the
> ATAGS from it's internal structures as usual.
> 
> Best regards,
> Marek Vasut

Ok, but what to do with non-standard omap/maemo atags which is 
used only for maemo (atag for bootreason, atag for bootmode ...)? 
U-Boot does not have internal structures for these non-standrad 
atags and also does not support passing it.
Wolfgang Denk April 29, 2012, 12:16 p.m. UTC | #9
Dear Pali =?ISO-8859-1?Q?Roh=E1r?=,

In message <2099459.BqkWJMp2Uq@pali> you wrote:
> 
> On Saturday 28 April 2012 23:20:04 Wolfgang Denk wrote:
> > Please stop posting your patches with base64
> > Content-transfer-encoding; thanks.
> 
> How can I tell this to git format-patch and git send-email? Also 
> I looked in my mailbox and all emails have no base64 header, but 
> Content-Transfer-Encoding: 8bit

Do not use funny characters in the commit message.

Try sending a patch to yourself first and verify that git-send-email
does not need to base64 encode it.

Such patches are also all undigestable by patchwork.

Best regards,

Wolfgang Denk
Marek Vasut April 29, 2012, 1:08 p.m. UTC | #10
Dear Pali Rohár,

> On Sunday 29 April 2012 11:10:42 Marek Vasut wrote:
> > Dear Pali Rohár,
> > 
> > > On Sunday 29 April 2012 00:15:23 Marek Vasut wrote:
> > > > Won't it be easier to create a preprocessing function
> > > > that'd
> > > > fill gd properly, so uboot can generate the atags through
> > > > standard means then?
> > > 
> > > Do you mean to generate/copy other atags in board code? This
> > > will not work because u-boot (in bootm.c) always passing
> > > ATAG_CORE in function setup_start_tag. And I do not want to
> > > pass ATAG_CORE two times to kernel (once which I copy from
> > > other bootloader and once which generate u-boot).
> > 
> > No, I mean parse the old ATAGS from nolo and fill u-boot's
> > internal structures with that. Then let uboot generate the
> > ATAGS from it's internal structures as usual.
> > 
> > Best regards,
> > Marek Vasut
> 
> Ok, but what to do with non-standard omap/maemo atags which is
> used only for maemo (atag for bootreason, atag for bootmode ...)?
> U-Boot does not have internal structures for these non-standrad
> atags and also does not support passing it.

Implement support for passing ad-hoc additional non-standard atags then?

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/README b/README
index 43074cf..60ad9c2 100644
--- a/README
+++ b/README
@@ -3687,6 +3687,8 @@  Some configuration options can be set using Environment Variables.
 
 List of environment variables (most likely not complete):
 
+  atagaddr	- bootm will use ATAGs struct from specified address (arm only)
+
   baudrate	- see CONFIG_BAUDRATE
 
   bootdelay	- see CONFIG_BOOTDELAY
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 599547d..0f3c97b 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -42,6 +42,10 @@  DECLARE_GLOBAL_DATA_PTR;
 	defined(CONFIG_INITRD_TAG) || \
 	defined(CONFIG_SERIAL_TAG) || \
 	defined(CONFIG_REVISION_TAG)
+		#define CONFIG_SETUP_ANY_TAG
+#endif
+
+#ifdef CONFIG_SETUP_ANY_TAG
 static struct tag *params;
 #endif
 
@@ -106,11 +110,7 @@  static void announce_and_cleanup(void)
 	cleanup_before_linux();
 }
 
-#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
-	defined(CONFIG_CMDLINE_TAG) || \
-	defined(CONFIG_INITRD_TAG) || \
-	defined(CONFIG_SERIAL_TAG) || \
-	defined(CONFIG_REVISION_TAG)
+#ifdef CONFIG_SETUP_ANY_TAG
 static void setup_start_tag (bd_t *bd)
 {
 	params = (struct tag *)bd->bi_boot_params;
@@ -217,11 +217,7 @@  void setup_revision_tag(struct tag **in_params)
 }
 #endif
 
-#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
-	defined(CONFIG_CMDLINE_TAG) || \
-	defined(CONFIG_INITRD_TAG) || \
-	defined(CONFIG_SERIAL_TAG) || \
-	defined(CONFIG_REVISION_TAG)
+#ifdef CONFIG_SETUP_ANY_TAG
 static void setup_end_tag(bd_t *bd)
 {
 	params->hdr.tag = ATAG_NONE;
@@ -280,13 +276,23 @@  static void boot_prep_linux(bootm_headers_t *images)
 	} else
 #endif
 	{
-#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
-	defined(CONFIG_CMDLINE_TAG) || \
-	defined(CONFIG_INITRD_TAG) || \
-	defined(CONFIG_SERIAL_TAG) || \
-	defined(CONFIG_REVISION_TAG)
+		char *atagaddr = getenv("atagaddr");
 		debug("using: ATAGS\n");
-		setup_start_tag(gd->bd);
+
+		if (atagaddr)
+			gd->bd->bi_boot_params = simple_strtoul(atagaddr, NULL, 16);
+
+		if (gd->bd->bi_boot_params) {
+			printf("Using existing atags at %#lx\n", gd->bd->bi_boot_params);
+
+			params = (struct tag *) gd->bd->bi_boot_params;
+			while (params->hdr.size > 0)
+				params = tag_next(params);
+		} else {
+#ifdef CONFIG_SETUP_ANY_TAG
+			setup_start_tag(gd->bd);
+#endif
+		}
 #ifdef CONFIG_SERIAL_TAG
 		setup_serial_tag(&params);
 #endif
@@ -297,18 +303,28 @@  static void boot_prep_linux(bootm_headers_t *images)
 		setup_revision_tag(&params);
 #endif
 #ifdef CONFIG_SETUP_MEMORY_TAGS
-		setup_memory_tags(gd->bd);
+		if (!atagaddr)
+			setup_memory_tags(gd->bd);
 #endif
 #ifdef CONFIG_INITRD_TAG
 		if (images->rd_start && images->rd_end)
 			setup_initrd_tag(gd->bd, images->rd_start,
 			images->rd_end);
 #endif
-		setup_end_tag(gd->bd);
-#else /* all tags */
-		printf("FDT and ATAGS support not compiled in - hanging\n");
-		hang();
-#endif /* all tags */
+		if (atagaddr) {
+			if (params->hdr.size > 0)
+				setup_end_tag(gd->bd);
+		} else {
+#ifdef CONFIG_SETUP_ANY_TAG
+			setup_end_tag(gd->bd);
+#endif
+		}
+#ifndef CONFIG_SETUP_ANY_TAG
+		if (!atagaddr) {
+			printf("FDT and ATAGS support not compiled in - hanging\n");
+			hang();
+		}
+#endif
 	}
 }