diff mbox series

ARM: stm32: Fill in missing loadaddr

Message ID 20191218065855.12239-1-marex@denx.de
State Superseded
Delegated to: Patrick Delaunay
Headers show
Series ARM: stm32: Fill in missing loadaddr | expand

Commit Message

Marek Vasut Dec. 18, 2019, 6:58 a.m. UTC
Since CONFIG_LOADADDR is not set, the default value of $loadaddr
variable is not set in the environment either. Set the default
load address to 256 MiB from the start of DRAM.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Patrick Delaunay <patrick.delaunay@st.com>
Cc: Patrice Chotard <patrice.chotard@st.com>
---
 include/configs/stm32mp1.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Patrick Delaunay Dec. 18, 2019, 1:04 p.m. UTC | #1
Hi Marek,

> From: Marek Vasut <marex@denx.de>
> Sent: mercredi 18 d├ęcembre 2019 07:59
> 
> Since CONFIG_LOADADDR is not set, the default value of $loadaddr variable is
> not set in the environment either. Set the default load address to 256 MiB from the
> start of DRAM.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Patrick Delaunay <patrick.delaunay@st.com>
> Cc: Patrice Chotard <patrice.chotard@st.com>
> ---
>  include/configs/stm32mp1.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h index
> dfc397c63c..59a86aee79 100644
> --- a/include/configs/stm32mp1.h
> +++ b/include/configs/stm32mp1.h
> @@ -36,6 +36,7 @@
>   * Needed by "loadb"
>   */
>  #define CONFIG_SYS_LOAD_ADDR			STM32_DDR_BASE
> +#define CONFIG_LOADADDR				0xd0000000
> 
>  /* ATAGs */
>  #define CONFIG_CMDLINE_TAG
> --
> 2.24.1

Yes 

I never defined this CONFIG because not yet needed...
I alway use the load or boot command with address.

Just  to known, you need to have loadaddr for which use case ?

I check the usage in U-Boot code ...
And it is the only the default address when the value is not defined in CLI

./cmd/qfw.c:126:	env = env_get("loadaddr");
./cmd/load.c:432:	s = env_get("loadaddr");
./cmd/bootefi.c:369:		saddr = env_get("loadaddr");
./cmd/ini.c:240:		argc < 3 ? env_get("loadaddr") : argv[2], NULL, 16);
./cmd/net.c:190:	s = env_get("loadaddr");
./cmd/reiser.c:91:		addr_str = env_get("loadaddr");
./cmd/zfs.c:57:		addr_str = env_get("loadaddr");
./cmd/mvebu/bubt.c:101:	addr_str = env_get("loadaddr");
./fs/fs.c:691:		addr_str = env_get("loadaddr");
./common/board_r.c:467:	load_addr = env_get_ulong("loadaddr", 16, load_addr);
./common/update.c:269:	env_addr = env_get("loadaddr");
./common/image.c:563:static int on_loadaddr(const char *name, const char *value, enum env_op op,
./common/image.c:1079:		select = (argc == 0) ? env_get("loadaddr") : argv[0];

It is more or less the same usage than: 

./include/configs/stm32mp1.h:38:#define CONFIG_SYS_LOAD_ADDR			STM32_DDR_BASE

PS: I don't known why we have 2 differents CONFIG : CONFIG_SYS_LOAD_ADDR & CONFIG_LOADADDR

Normally with distro, the others variables for location of each binary are used:

	kernel_addr_r=0xc2000000
	fdt_addr_r=0xc4000000
	scriptaddr=0xc4100000
	pxefile_addr_r=0xc4200000
	splashimage=0xc4300000
	ramdisk_addr_r=0xc4400000


And I don't see reason to don't use the beginning of the DDR for load with address,
so I prefer:

- /*
-  * Needed by "loadb"
-  */
-  #define CONFIG_SYS_LOAD_ADDR			STM32_DDR_BASE

+ /*
+  * default load location
-+ */
+ #define CONFIG_LOADADDR			STM32_DDR_BASE
+ #define CONFIG_SYS_LOAD_ADDR			CONFIG_LOADADDR 


But perhaps I miss something...

Patrick
Marek Vasut Dec. 18, 2019, 2:08 p.m. UTC | #2
On 12/18/19 2:04 PM, Patrick DELAUNAY wrote:
> Hi Marek,

Hi,

>> From: Marek Vasut <marex@denx.de>
>> Sent: mercredi 18 d├ęcembre 2019 07:59
>>
>> Since CONFIG_LOADADDR is not set, the default value of $loadaddr variable is
>> not set in the environment either. Set the default load address to 256 MiB from the
>> start of DRAM.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Patrick Delaunay <patrick.delaunay@st.com>
>> Cc: Patrice Chotard <patrice.chotard@st.com>
>> ---
>>  include/configs/stm32mp1.h | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h index
>> dfc397c63c..59a86aee79 100644
>> --- a/include/configs/stm32mp1.h
>> +++ b/include/configs/stm32mp1.h
>> @@ -36,6 +36,7 @@
>>   * Needed by "loadb"
>>   */
>>  #define CONFIG_SYS_LOAD_ADDR			STM32_DDR_BASE
>> +#define CONFIG_LOADADDR				0xd0000000
>>
>>  /* ATAGs */
>>  #define CONFIG_CMDLINE_TAG
>> --
>> 2.24.1
> 
> Yes 
> 
> I never defined this CONFIG because not yet needed...

It is needed all right, to make the system behave sane.

> I alway use the load or boot command with address.

That's not how it was designed to be used. I always use it without the
address , because the $loadaddr should provide sane loading default. If
it's not defined, there's a problem.

> Just  to known, you need to have loadaddr for which use case ?

For the sane default use case ? :)

> I check the usage in U-Boot code ...
> And it is the only the default address when the value is not defined in CLI

Correct

> ./cmd/qfw.c:126:	env = env_get("loadaddr");
> ./cmd/load.c:432:	s = env_get("loadaddr");
> ./cmd/bootefi.c:369:		saddr = env_get("loadaddr");
> ./cmd/ini.c:240:		argc < 3 ? env_get("loadaddr") : argv[2], NULL, 16);
> ./cmd/net.c:190:	s = env_get("loadaddr");
> ./cmd/reiser.c:91:		addr_str = env_get("loadaddr");
> ./cmd/zfs.c:57:		addr_str = env_get("loadaddr");
> ./cmd/mvebu/bubt.c:101:	addr_str = env_get("loadaddr");
> ./fs/fs.c:691:		addr_str = env_get("loadaddr");
> ./common/board_r.c:467:	load_addr = env_get_ulong("loadaddr", 16, load_addr);
> ./common/update.c:269:	env_addr = env_get("loadaddr");
> ./common/image.c:563:static int on_loadaddr(const char *name, const char *value, enum env_op op,
> ./common/image.c:1079:		select = (argc == 0) ? env_get("loadaddr") : argv[0];
> 
> It is more or less the same usage than: 
> 
> ./include/configs/stm32mp1.h:38:#define CONFIG_SYS_LOAD_ADDR			STM32_DDR_BASE
> 
> PS: I don't known why we have 2 differents CONFIG : CONFIG_SYS_LOAD_ADDR & CONFIG_LOADADDR

That's something on my list of things to sort out. If you want to take
it up, have at it.

[...]
diff mbox series

Patch

diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
index dfc397c63c..59a86aee79 100644
--- a/include/configs/stm32mp1.h
+++ b/include/configs/stm32mp1.h
@@ -36,6 +36,7 @@ 
  * Needed by "loadb"
  */
 #define CONFIG_SYS_LOAD_ADDR			STM32_DDR_BASE
+#define CONFIG_LOADADDR				0xd0000000
 
 /* ATAGs */
 #define CONFIG_CMDLINE_TAG