Re: [PATCH v2 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable
On Sunday 28 February 2021 22:26:25 Luka Kovacic wrote: > Hello Pali, > > On Sat, Feb 27, 2021 at 1:38 AM Pali Rohár wrote: > > > > On Monday 15 February 2021 20:59:33 Luka Kovacic wrote: > > > Add the loadaddr U-Boot environment variable, as this is available in > > > the stock Marvell U-Boot by default on Marvell Armada A37XX platforms. > > > > Hello Luka! Why is this change needed? mvebu_armada-37xx.h already > > defines CONFIG_SYS_LOAD_ADDR macro with its default value 0x0600. > > So defining loadaddr variable should not be needed at all. And it is > > suspicious for me why definition is to the same default value. > > I've added this to the environment, because it was not present in > U-Boot as a variable. > There's a need for this value in some scripts, are you aware of some > other way to retrieve it easily? Hello Luka! It is really needed? Because if you do not specify loadaddr then default value from CONFIG_SYS_LOAD_ADDR is used. It applies also for 'loadb' and 'loady' commands. I guess in past it was needed in scripts because default CONFIG_SYS_LOAD_ADDR was wrong. But I have fixed it in commit: 996ecfd3ec4e86be9dbee17b2223061e16627f71 It is possible that old Marvell's U-Boot fork has this problem too, but in recent version of upstream U-Boot it is fixed by above commit. > Some other boards also do this similarly. > > Kind regards, > Luka > > > > > Signed-off-by: Luka Kovacic > > > Cc: Luka Perkov > > > Cc: Robert Marko > > > --- > > > include/configs/mvebu_armada-37xx.h | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/include/configs/mvebu_armada-37xx.h > > > b/include/configs/mvebu_armada-37xx.h > > > index 2ad4325eaf..1041df8d91 100644 > > > --- a/include/configs/mvebu_armada-37xx.h > > > +++ b/include/configs/mvebu_armada-37xx.h > > > @@ -103,6 +103,7 @@ > > > > > > /* fdt_addr and kernel_addr are needed for existing distribution boot > > > scripts */ > > > #define CONFIG_EXTRA_ENV_SETTINGS\ > > > + "loadaddr=0x600\0" \ > > > "scriptaddr=0x6d0\0"\ > > > "pxefile_addr_r=0x6e0\0"\ > > > "fdt_addr=0x6f0\0" \
Re: [PATCH v2 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable
Hello Pali, On Sat, Feb 27, 2021 at 1:38 AM Pali Rohár wrote: > > On Monday 15 February 2021 20:59:33 Luka Kovacic wrote: > > Add the loadaddr U-Boot environment variable, as this is available in > > the stock Marvell U-Boot by default on Marvell Armada A37XX platforms. > > Hello Luka! Why is this change needed? mvebu_armada-37xx.h already > defines CONFIG_SYS_LOAD_ADDR macro with its default value 0x0600. > So defining loadaddr variable should not be needed at all. And it is > suspicious for me why definition is to the same default value. I've added this to the environment, because it was not present in U-Boot as a variable. There's a need for this value in some scripts, are you aware of some other way to retrieve it easily? Some other boards also do this similarly. Kind regards, Luka > > > Signed-off-by: Luka Kovacic > > Cc: Luka Perkov > > Cc: Robert Marko > > --- > > include/configs/mvebu_armada-37xx.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/include/configs/mvebu_armada-37xx.h > > b/include/configs/mvebu_armada-37xx.h > > index 2ad4325eaf..1041df8d91 100644 > > --- a/include/configs/mvebu_armada-37xx.h > > +++ b/include/configs/mvebu_armada-37xx.h > > @@ -103,6 +103,7 @@ > > > > /* fdt_addr and kernel_addr are needed for existing distribution boot > > scripts */ > > #define CONFIG_EXTRA_ENV_SETTINGS\ > > + "loadaddr=0x600\0" \ > > "scriptaddr=0x6d0\0"\ > > "pxefile_addr_r=0x6e0\0"\ > > "fdt_addr=0x6f0\0" \
Re: [PATCH v2 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable
On Monday 15 February 2021 20:59:33 Luka Kovacic wrote: > Add the loadaddr U-Boot environment variable, as this is available in > the stock Marvell U-Boot by default on Marvell Armada A37XX platforms. Hello Luka! Why is this change needed? mvebu_armada-37xx.h already defines CONFIG_SYS_LOAD_ADDR macro with its default value 0x0600. So defining loadaddr variable should not be needed at all. And it is suspicious for me why definition is to the same default value. > Signed-off-by: Luka Kovacic > Cc: Luka Perkov > Cc: Robert Marko > --- > include/configs/mvebu_armada-37xx.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/configs/mvebu_armada-37xx.h > b/include/configs/mvebu_armada-37xx.h > index 2ad4325eaf..1041df8d91 100644 > --- a/include/configs/mvebu_armada-37xx.h > +++ b/include/configs/mvebu_armada-37xx.h > @@ -103,6 +103,7 @@ > > /* fdt_addr and kernel_addr are needed for existing distribution boot > scripts */ > #define CONFIG_EXTRA_ENV_SETTINGS\ > + "loadaddr=0x600\0" \ > "scriptaddr=0x6d0\0"\ > "pxefile_addr_r=0x6e0\0"\ > "fdt_addr=0x6f0\0" \
[PATCH v2 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable
Add the loadaddr U-Boot environment variable, as this is available in the stock Marvell U-Boot by default on Marvell Armada A37XX platforms. Signed-off-by: Luka Kovacic Cc: Luka Perkov Cc: Robert Marko --- include/configs/mvebu_armada-37xx.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/configs/mvebu_armada-37xx.h b/include/configs/mvebu_armada-37xx.h index 2ad4325eaf..1041df8d91 100644 --- a/include/configs/mvebu_armada-37xx.h +++ b/include/configs/mvebu_armada-37xx.h @@ -103,6 +103,7 @@ /* fdt_addr and kernel_addr are needed for existing distribution boot scripts */ #define CONFIG_EXTRA_ENV_SETTINGS \ + "loadaddr=0x600\0" \ "scriptaddr=0x6d0\0"\ "pxefile_addr_r=0x6e0\0"\ "fdt_addr=0x6f0\0" \ -- 2.20.1
