| Message ID | 20191218065855.12239-1-marex@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Patrick Delaunay |
| Headers | show |
| Series | ARM: stm32: Fill in missing loadaddr | expand |
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
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 --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
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(+)