SPL: do not use fix value for u-boot size

PATCH New Stefano Babic 3 participants 8 messages Aug 23, 2012

Share & tools

Stefano Babic

author

2012-08-23 08:40 UTC #0 lore raw

PATCH 1/1 SPL: do not use fix value for u-boot size

If an u-boot image is not found, SPL thinks to load a bare u-boot.bin image with a maximum size of 200KB. Use CONFIG_SYS_MONITOR_LEN instead.

Signed-off-by Stefano Babic <[email protected]>

CC Tom Rini <[email protected]>

---

Note: this is based on Tom's series
"ARM: SPL: Make more generic, merge DaVinci and OMAP"

Tested with V2 version on a MX35.

 common/spl/spl.c |    4 ++++
 1 file changed, 4 insertions(+)

common/spl/spl.c +4 -0

index 7d15460..827ff1c 100644
@@ -100,7 +100,11 @@ void spl_parse_image_header(const struct image_header *header)
 		debug("mkimage signature not found - ih_magic = %x\n",
 			header->ih_magic);
 		/* Let's assume U-Boot will not be more than 200 KB */
+#ifdef CONFIG_SYS_MONITOR_LEN
+		spl_image.size = CONFIG_SYS_MONITOR_LEN;
+#else
 		spl_image.size = 200 * 1024;
+#endif
 		spl_image.entry_point = CONFIG_SYS_TEXT_BASE;
 		spl_image.load_addr = CONFIG_SYS_TEXT_BASE;
 		spl_image.os = IH_OS_U_BOOT;

Signature · 1 line

1.7.9.5

Stefan Roese

2012-08-23 08:51 UTC #1 lore raw

PATCH 1/1 SPL: do not use fix value for u-boot size

Quoted 21 lines · Stefano Babic

If an u-boot image is not found, SPL thinks to load a bare u-boot.bin image with a maximum size of 200KB. Use CONFIG_SYS_MONITOR_LEN instead.
Signed-off-by Stefano Babic <[email protected]>
CC Tom Rini <[email protected]>
---
Note: this is based on Tom's series
"ARM: SPL: Make more generic, merge DaVinci and OMAP"
Tested with V2 version on a MX35.
 common/spl/spl.c |    4 ++++
 1 file changed, 4 insertions(+)
common/spl/spl.c +0 -0
index 7d15460..827ff1c 100644

common/spl/spl.c void spl_parse_image_header(const struct image_header *header) · line 107

Quoted 8 lines

common/spl/spl.c +4 -0

@@ -100,7 +100,11 @@ void spl_parse_image_header(const struct image_header *header)
 		debug("mkimage signature not found - ih_magic = %x\n",
 			header->ih_magic);
 		/* Let's assume U-Boot will not be more than 200 KB */
+#ifdef CONFIG_SYS_MONITOR_LEN
+		spl_image.size = CONFIG_SYS_MONITOR_LEN;
+#else
 		spl_image.size = 200 * 1024;
+#endif

Yes, I noticed this 200 KiB setting as well. But it seems that this ".size" variable is not referenced at all. Or am I missing something? If this is correct, then we should probably remove setting it completely.

What do you think?

Thanks,
Stefan

Stefano Babic

author

2012-08-23 09:23 UTC #2 lore raw

PATCH 1/1 SPL: do not use fix value for u-boot size

Quoted 9 lines · Stefan Roese

Quoted 8 lines · Stefano Babic
If an u-boot image is not found, SPL thinks to load a bare u-boot.bin image with a maximum size of 200KB. Use CONFIG_SYS_MONITOR_LEN instead.
Signed-off-by Stefano Babic <[email protected]>
CC Tom Rini <[email protected]>
---

Hi Stefan,

Quoted 8 lines

Quoted 8 lines
Note: this is based on Tom's series
"ARM: SPL: Make more generic, merge DaVinci and OMAP"
Tested with V2 version on a MX35.
 common/spl/spl.c |    4 ++++
 1 file changed, 4 insertions(+)

Quoted 16 lines

common/spl/spl.c void spl_parse_image_header(const struct image_header *header) · line 107
Quoted 13 lines
common/spl/spl.c +4 -0
index 7d15460..827ff1c 100644
@@ -100,7 +100,11 @@ void spl_parse_image_header(const struct image_header *header)
 		debug("mkimage signature not found - ih_magic = %x\n",
 			header->ih_magic);
 		/* Let's assume U-Boot will not be more than 200 KB */
+#ifdef CONFIG_SYS_MONITOR_LEN
+		spl_image.size = CONFIG_SYS_MONITOR_LEN;
+#else
 		spl_image.size = 200 * 1024;
+#endif
Yes, I noticed this 200 KiB setting as well. But it seems that this ".size" variable is not referenced at all. Or am I missing something?

Your are missing something:

drivers/mmc/spl_mmc.c:

55 /* convert size to sectors - round up */
56 image_size_sectors = (spl_image.size + MMCSD_SECTOR_SIZE - 1) /
57 MMCSD_SECTOR_SIZE;

And I have an example where only a part of u-boot is copied.

Cheers,
Stefano

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

Stefan Roese

2012-08-23 09:28 UTC #3 lore raw

PATCH 1/1 SPL: do not use fix value for u-boot size

Hi Stefano,

Quoted 19 lines · Stefano Babic

Quoted 8 lines
Quoted 8 lines
common/spl/spl.c +0 -0
index 7d15460..827ff1c 100644
@@ -100,7 +100,11 @@ void spl_parse_image_header(const struct image_header *header)
 		debug("mkimage signature not found - ih_magic = %x\n",
 			header->ih_magic);
 		/* Let's assume U-Boot will not be more than 200 KB */
Quoted 8 lines
common/spl/spl.c void spl_parse_image_header(const struct image_header *header) · line 107
Quoted 5 lines
common/spl/spl.c +4 -0
+#ifdef CONFIG_SYS_MONITOR_LEN
+		spl_image.size = CONFIG_SYS_MONITOR_LEN;
+#else
 		spl_image.size = 200 * 1024;
+#endif
Yes, I noticed this 200 KiB setting as well. But it seems that this ".size" variable is not referenced at all. Or am I missing something?
Your are missing something:

Quoted 7 lines

drivers/mmc/spl_mmc.c:
55 /* convert size to sectors - round up */
56 image_size_sectors = (spl_image.size + MMCSD_SECTOR_SIZE - 1) /
57 MMCSD_SECTOR_SIZE;
And I have an example where only a part of u-boot is copied.

Ahh, I see. Thanks.

I would prefer the following approach then, to move the #ifdef out of the C code:

#ifndef CONFIG_SYS_MONITOR_LEN
#define CONFIG_SYS_MONITOR_LEN (200 * 1024)
#endif

and then use CONFIG_SYS_MONITOR_LEN unconditionally.

Thanks,
Stefan

Stefano Babic

author

2012-08-23 09:38 UTC #4 lore raw

PATCH 1/1 SPL: do not use fix value for u-boot size

Quoted 32 lines · Stefan Roese

Hi Stefano,
Quoted 19 lines · Stefano Babic
Quoted 8 lines
Quoted 8 lines
common/spl/spl.c +0 -0
index 7d15460..827ff1c 100644
@@ -100,7 +100,11 @@ void spl_parse_image_header(const struct image_header *header)
 		debug("mkimage signature not found - ih_magic = %x\n",
 			header->ih_magic);
 		/* Let's assume U-Boot will not be more than 200 KB */
Quoted 8 lines
common/spl/spl.c void spl_parse_image_header(const struct image_header *header) · line 107
Quoted 5 lines
common/spl/spl.c +4 -0
+#ifdef CONFIG_SYS_MONITOR_LEN
+		spl_image.size = CONFIG_SYS_MONITOR_LEN;
+#else
 		spl_image.size = 200 * 1024;
+#endif
Yes, I noticed this 200 KiB setting as well. But it seems that this ".size" variable is not referenced at all. Or am I missing something?
Your are missing something:
Quoted 7 lines
drivers/mmc/spl_mmc.c:
55 /* convert size to sectors - round up */
56 image_size_sectors = (spl_image.size + MMCSD_SECTOR_SIZE - 1) /
57 MMCSD_SECTOR_SIZE;
And I have an example where only a part of u-boot is copied.
Ahh, I see. Thanks.

Quoted 8 lines

I would prefer the following approach then, to move the #ifdef out of the C code:
#ifndef CONFIG_SYS_MONITOR_LEN
#define CONFIG_SYS_MONITOR_LEN (200 * 1024)
#endif
and then use CONFIG_SYS_MONITOR_LEN unconditionally.

Fine with me - I will fix in V2.

Regards,
Stefano

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

Stefano Babic

author

2012-08-23 10:46 UTC #5 lore raw

PATCH v2 1/1 SPL: do not use fix value for u-boot size

In reply to Stefano Babic

If an u-boot image is not found, SPL thinks to load a bare u-boot.bin image with a maximum size of 200KB. Use CONFIG_SYS_MONITOR_LEN instead.

Signed-off-by Stefano Babic <[email protected]>

CC Tom Rini <[email protected]>

CC Stefan Roese <[email protected]>

---

Changes:

- move the #ifdef out of the C code (Stefan Roese)

 common/spl/spl.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

common/spl/spl.c +5 -1

index 7d15460..d9e4b62 100644
@@ -36,6 +36,10 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#ifndef CONFIG_SYS_MONITOR_LEN
+#define CONFIG_SYS_MONITOR_LEN	(200 * 1024)
+#endif
+
 u32* boot_params_ptr = NULL;
 struct spl_image_info spl_image;

@@ -100,7 +104,7 @@ void spl_parse_image_header(const struct image_header *header)
 		debug("mkimage signature not found - ih_magic = %x\n",
 			header->ih_magic);
 		/* Let's assume U-Boot will not be more than 200 KB */
-		spl_image.size = 200 * 1024;
+		spl_image.size = CONFIG_SYS_MONITOR_LEN;
 		spl_image.entry_point = CONFIG_SYS_TEXT_BASE;
 		spl_image.load_addr = CONFIG_SYS_TEXT_BASE;
 		spl_image.os = IH_OS_U_BOOT;

Signature · 1 line

1.7.9.5

Stefan Roese

2012-08-23 11:19 UTC #6 lore raw

PATCH v2 1/1 SPL: do not use fix value for u-boot size

Quoted 7 lines · Stefano Babic

If an u-boot image is not found, SPL thinks to load a bare u-boot.bin image with a maximum size of 200KB. Use CONFIG_SYS_MONITOR_LEN instead.
Signed-off-by Stefano Babic <[email protected]>
CC Tom Rini <[email protected]>
CC Stefan Roese <[email protected]>

Acked-by Stefan Roese <[email protected]>

Thanks,
Stefan

Tom Rini

2012-08-23 22:03 UTC #7 lore raw

PATCH v2 1/1 SPL: do not use fix value for u-boot size

In reply to Stefano Babic

Quoted 7 lines · Stefano Babic

If an u-boot image is not found, SPL thinks to load a bare u-boot.bin image with a maximum size of 200KB. Use CONFIG_SYS_MONITOR_LEN instead.
Signed-off-by Stefano Babic <[email protected]>
CC Tom Rini <[email protected]>
CC Stefan Roese <[email protected]>

Looks good, I'll take this for my v4.

Signature · 1 line

Tom