diff mbox

[U-Boot] SPL: do not use fix value for u-boot size

Message ID 1345711214-11969-1-git-send-email-sbabic@denx.de
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

Stefano Babic Aug. 23, 2012, 8:40 a.m. UTC
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 <sbabic@denx.de>
CC: Tom Rini <trini@ti.com>
---

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(+)

Comments

Stefan Roese Aug. 23, 2012, 8:51 a.m. UTC | #1
On 08/23/2012 10:40 AM, Stefano Babic wrote:
> 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 <sbabic@denx.de>
> CC: Tom Rini <trini@ti.com>
> ---
> 
> 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(+)
> 
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 7d15460..827ff1c 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -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 Aug. 23, 2012, 9:23 a.m. UTC | #2
On 23/08/2012 10:51, Stefan Roese wrote:
> On 08/23/2012 10:40 AM, Stefano Babic wrote:
>> 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 <sbabic@denx.de>
>> CC: Tom Rini <trini@ti.com>
>> ---
>>

Hi Stefan,

>> 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(+)
>>
>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>> index 7d15460..827ff1c 100644
>> --- a/common/spl/spl.c
>> +++ b/common/spl/spl.c
>> @@ -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
Stefan Roese Aug. 23, 2012, 9:28 a.m. UTC | #3
Hi Stefano,

On 08/23/2012 11:23 AM, Stefano Babic wrote:
>>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>>> index 7d15460..827ff1c 100644
>>> --- a/common/spl/spl.c
>>> +++ b/common/spl/spl.c
>>> @@ -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.

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 Aug. 23, 2012, 9:38 a.m. UTC | #4
On 23/08/2012 11:28, Stefan Roese wrote:
> Hi Stefano,
> 
> On 08/23/2012 11:23 AM, Stefano Babic wrote:
>>>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>>>> index 7d15460..827ff1c 100644
>>>> --- a/common/spl/spl.c
>>>> +++ b/common/spl/spl.c
>>>> @@ -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.
> 
> 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.

Fine with me - I will fix in V2.

Regards,
Stefano
diff mbox

Patch

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 7d15460..827ff1c 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -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;