* [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime
@ 2020-12-23 11:21 Pali Rohár
2020-12-23 11:21 ` [PATCH 1/3] env: Allow to set default_environment[] from board code via compile option DEFAULT_ENV_IS_RW Pali Rohár
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Pali Rohár @ 2020-12-23 11:21 UTC (permalink / raw)
To: u-boot
This patch series set default env values of $fdtfile and $ethNaddr for
Espressobin board at runtime.
It fixes two main issues on Espressobin board that 'env default -a'
completely erases permanent board MAC addresses and also erase $fdtfile
variable which is needed for booting Linux kernel via distro boot.
Lot of people were complaining about erasing permanent MAC addresses by
U-boot on this board and due to this issue some linux distributions
started using static hardcoded MAC addresses for all Espressobin boards
to workaround this issue. Apparently erase of MAC addresses or usage of
static hardcoded value caused more issues on network (e.g. inability to
connect two of these boards to the same network).
Pali Roh?r (3):
env: Allow to set default_environment[] from board code via compile
option DEFAULT_ENV_IS_RW
arm: mvebu: Espressobin: Set default value for $fdtfile env variable
arm: mvebu: Espressobin: Set default value for $ethNaddr env variable
board/Marvell/mvebu_armada-37xx/board.c | 41 ++++++++++++++++++++-----
include/configs/mvebu_armada-37xx.h | 17 +++++++++-
include/env_default.h | 2 ++
include/env_internal.h | 4 +++
4 files changed, 56 insertions(+), 8 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] env: Allow to set default_environment[] from board code via compile option DEFAULT_ENV_IS_RW
2020-12-23 11:21 [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime Pali Rohár
@ 2020-12-23 11:21 ` Pali Rohár
2020-12-23 11:21 ` [PATCH 2/3] arm: mvebu: Espressobin: Set default value for $fdtfile env variable Pali Rohár
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Pali Rohár @ 2020-12-23 11:21 UTC (permalink / raw)
To: u-boot
This change allows board code to modify default_environment[] array when
compile option DEFAULT_ENV_IS_RW is specified in board config file.
Some board default variables depend on runtime configuration which is not
known at compile time. Therefore allow to set default_environment[] array
as non-const and allow board code to modify it when it is needed.
Signed-off-by: Pali Roh?r <pali@kernel.org>
---
include/env_default.h | 2 ++
include/env_internal.h | 4 ++++
2 files changed, 6 insertions(+)
diff --git a/include/env_default.h b/include/env_default.h
index 8a0c3057f0..ea31a8eddf 100644
--- a/include/env_default.h
+++ b/include/env_default.h
@@ -19,6 +19,8 @@ env_t embedded_environment __UBOOT_ENV_SECTION__(environment) = {
{
#elif defined(DEFAULT_ENV_INSTANCE_STATIC)
static char default_environment[] = {
+#elif defined(DEFAULT_ENV_IS_RW)
+uchar default_environment[] = {
#else
const uchar default_environment[] = {
#endif
diff --git a/include/env_internal.h b/include/env_internal.h
index b26dc6239c..708c833a55 100644
--- a/include/env_internal.h
+++ b/include/env_internal.h
@@ -111,7 +111,11 @@ typedef struct environment_s {
extern env_t embedded_environment;
#endif /* ENV_IS_EMBEDDED */
+#ifdef DEFAULT_ENV_IS_RW
+extern unsigned char default_environment[];
+#else
extern const unsigned char default_environment[];
+#endif
#ifndef DO_DEPS_ONLY
--
2.20.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] arm: mvebu: Espressobin: Set default value for $fdtfile env variable
2020-12-23 11:21 [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime Pali Rohár
2020-12-23 11:21 ` [PATCH 1/3] env: Allow to set default_environment[] from board code via compile option DEFAULT_ENV_IS_RW Pali Rohár
@ 2020-12-23 11:21 ` Pali Rohár
2020-12-23 11:21 ` [PATCH 3/3] arm: mvebu: Espressobin: Set default value for $ethNaddr " Pali Rohár
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Pali Rohár @ 2020-12-23 11:21 UTC (permalink / raw)
To: u-boot
On Espressobin board value for $fdtfile cannot be determined at compile
time and is calculated at board runtime code. This change uses a new option
DEFAULT_ENV_IS_RW to allow modifying default_environment[] array at runtime
and set into it correct value.
This change also ensure that 'env default -a' set correct value to $fdtfile.
Signed-off-by: Pali Roh?r <pali@kernel.org>
---
board/Marvell/mvebu_armada-37xx/board.c | 22 +++++++++++++++-------
include/configs/mvebu_armada-37xx.h | 13 ++++++++++++-
2 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
index f67b04b78c..9297ea0f90 100644
--- a/board/Marvell/mvebu_armada-37xx/board.c
+++ b/board/Marvell/mvebu_armada-37xx/board.c
@@ -6,6 +6,7 @@
#include <common.h>
#include <dm.h>
#include <env.h>
+#include <env_internal.h>
#include <i2c.h>
#include <init.h>
#include <mmc.h>
@@ -84,15 +85,17 @@ int board_init(void)
#ifdef CONFIG_BOARD_LATE_INIT
int board_late_init(void)
{
+ char *ptr = (char *)&default_environment[0];
struct mmc *mmc_dev;
bool ddr4, emmc;
- if (env_get("fdtfile"))
- return 0;
-
if (!of_machine_is_compatible("globalscale,espressobin"))
return 0;
+ /* Find free buffer in default_environment[] for new variables */
+ while (*ptr != '\0' && *(ptr+1) != '\0') ptr++;
+ ptr += 2;
+
/* If the memory controller has been configured for DDR4, we're running on v7 */
ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS)
& A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4;
@@ -101,14 +104,19 @@ int board_late_init(void)
mmc_dev = find_mmc_device(1);
emmc = (mmc_dev && mmc_init(mmc_dev) == 0);
+ /* Ensure that 'env default -a' set correct value to $fdtfile */
if (ddr4 && emmc)
- env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb");
+ strcpy(ptr, "fdtfile=marvell/armada-3720-espressobin-v7-emmc.dtb");
else if (ddr4)
- env_set("fdtfile", "marvell/armada-3720-espressobin-v7.dtb");
+ strcpy(ptr, "fdtfile=marvell/armada-3720-espressobin-v7.dtb");
else if (emmc)
- env_set("fdtfile", "marvell/armada-3720-espressobin-emmc.dtb");
+ strcpy(ptr, "fdtfile=marvell/armada-3720-espressobin-emmc.dtb");
else
- env_set("fdtfile", "marvell/armada-3720-espressobin.dtb");
+ strcpy(ptr, "fdtfile=marvell/armada-3720-espressobin.dtb");
+
+ /* If $fdtfile was not set explicitly by user then set default value */
+ if (!env_get("fdtfile"))
+ env_set("fdtfile", ptr + sizeof("fdtfile="));
return 0;
}
diff --git a/include/configs/mvebu_armada-37xx.h b/include/configs/mvebu_armada-37xx.h
index 0d585606a7..6df702367c 100644
--- a/include/configs/mvebu_armada-37xx.h
+++ b/include/configs/mvebu_armada-37xx.h
@@ -57,6 +57,11 @@
*/
#define CONFIG_MTD_PARTITIONS /* required for UBI partition support */
+/*
+ * Environment
+ */
+#define DEFAULT_ENV_IS_RW /* required for configuring default fdtfile= */
+
/*
* Ethernet Driver configuration
*/
@@ -87,6 +92,11 @@
#include <config_distro_bootcmd.h>
+/* filler for default values filled by board_early_init_f() */
+#define ENV_RW_FILLER \
+ "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0" /* for fdtfile= */ \
+ ""
+
/* fdt_addr and kernel_addr are needed for existing distribution boot scripts */
#define CONFIG_EXTRA_ENV_SETTINGS \
"scriptaddr=0x6d00000\0" \
@@ -96,6 +106,7 @@
"kernel_addr=0x7000000\0" \
"kernel_addr_r=0x7000000\0" \
"ramdisk_addr_r=0xa000000\0" \
- BOOTENV
+ BOOTENV \
+ ENV_RW_FILLER
#endif /* _CONFIG_MVEBU_ARMADA_37XX_H */
--
2.20.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] arm: mvebu: Espressobin: Set default value for $ethNaddr env variable
2020-12-23 11:21 [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime Pali Rohár
2020-12-23 11:21 ` [PATCH 1/3] env: Allow to set default_environment[] from board code via compile option DEFAULT_ENV_IS_RW Pali Rohár
2020-12-23 11:21 ` [PATCH 2/3] arm: mvebu: Espressobin: Set default value for $fdtfile env variable Pali Rohár
@ 2020-12-23 11:21 ` Pali Rohár
2021-01-11 10:51 ` [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime Pali Rohár
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Pali Rohár @ 2020-12-23 11:21 UTC (permalink / raw)
To: u-boot
On Espressobin board are MAC addresses stored in U-Boot env area. Therefore
they are not present in default_environment[] array constructed at compile
time.
This change puts permanent MAC addresses into default_environment[] array
at board runtime. Espressobin board has enabled DEFAULT_ENV_IS_RW option
and therefore can modify this array.
This change ensure that 'env default -a' does not delete permanent MAC
addresses from Espressobin env storage area.
Signed-off-by: Pali Roh?r <pali@kernel.org>
---
board/Marvell/mvebu_armada-37xx/board.c | 19 +++++++++++++++++++
include/configs/mvebu_armada-37xx.h | 4 ++++
2 files changed, 23 insertions(+)
diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
index 9297ea0f90..dabd4eefd6 100644
--- a/board/Marvell/mvebu_armada-37xx/board.c
+++ b/board/Marvell/mvebu_armada-37xx/board.c
@@ -88,6 +88,9 @@ int board_late_init(void)
char *ptr = (char *)&default_environment[0];
struct mmc *mmc_dev;
bool ddr4, emmc;
+ const char *mac;
+ char eth[10];
+ int i;
if (!of_machine_is_compatible("globalscale,espressobin"))
return 0;
@@ -96,6 +99,22 @@ int board_late_init(void)
while (*ptr != '\0' && *(ptr+1) != '\0') ptr++;
ptr += 2;
+ /*
+ * Ensure that 'env default -a' does not erase permanent MAC addresses
+ * stored in env variables: $ethaddr, $eth1addr, $eth2addr and $eth3addr
+ */
+
+ mac = env_get("ethaddr");
+ if (mac && strlen(mac) <= 17)
+ ptr += sprintf(ptr, "ethaddr=%s", mac) + 1;
+
+ for (i = 1; i <= 3; i++) {
+ sprintf(eth, "eth%daddr", i);
+ mac = env_get(eth);
+ if (mac && strlen(mac) <= 17)
+ ptr += sprintf(ptr, "%s=%s", eth, mac) + 1;
+ }
+
/* If the memory controller has been configured for DDR4, we're running on v7 */
ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS)
& A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4;
diff --git a/include/configs/mvebu_armada-37xx.h b/include/configs/mvebu_armada-37xx.h
index 6df702367c..2ad4325eaf 100644
--- a/include/configs/mvebu_armada-37xx.h
+++ b/include/configs/mvebu_armada-37xx.h
@@ -94,6 +94,10 @@
/* filler for default values filled by board_early_init_f() */
#define ENV_RW_FILLER \
+ "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0" /* for ethaddr= */ \
+ "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0" /* for eth1addr= */ \
+ "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0" /* for eth2addr= */ \
+ "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0" /* for eth3addr= */ \
"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0" /* for fdtfile= */ \
""
--
2.20.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime
2020-12-23 11:21 [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime Pali Rohár
` (2 preceding siblings ...)
2020-12-23 11:21 ` [PATCH 3/3] arm: mvebu: Espressobin: Set default value for $ethNaddr " Pali Rohár
@ 2021-01-11 10:51 ` Pali Rohár
2021-01-12 8:18 ` Andre Heider
2021-02-01 15:24 ` Pali Rohár
2021-02-08 11:33 ` Stefan Roese
5 siblings, 1 reply; 15+ messages in thread
From: Pali Rohár @ 2021-01-11 10:51 UTC (permalink / raw)
To: u-boot
Hello Stefan and Andre!
Could you please look at this patch series and tell me what do you think
about it? If it is fine or needs to take different approach?
On Wednesday 23 December 2020 12:21:27 Pali Roh?r wrote:
> This patch series set default env values of $fdtfile and $ethNaddr for
> Espressobin board at runtime.
>
> It fixes two main issues on Espressobin board that 'env default -a'
> completely erases permanent board MAC addresses and also erase $fdtfile
> variable which is needed for booting Linux kernel via distro boot.
>
> Lot of people were complaining about erasing permanent MAC addresses by
> U-boot on this board and due to this issue some linux distributions
> started using static hardcoded MAC addresses for all Espressobin boards
> to workaround this issue. Apparently erase of MAC addresses or usage of
> static hardcoded value caused more issues on network (e.g. inability to
> connect two of these boards to the same network).
>
> Pali Roh?r (3):
> env: Allow to set default_environment[] from board code via compile
> option DEFAULT_ENV_IS_RW
> arm: mvebu: Espressobin: Set default value for $fdtfile env variable
> arm: mvebu: Espressobin: Set default value for $ethNaddr env variable
>
> board/Marvell/mvebu_armada-37xx/board.c | 41 ++++++++++++++++++++-----
> include/configs/mvebu_armada-37xx.h | 17 +++++++++-
> include/env_default.h | 2 ++
> include/env_internal.h | 4 +++
> 4 files changed, 56 insertions(+), 8 deletions(-)
>
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime
2021-01-11 10:51 ` [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime Pali Rohár
@ 2021-01-12 8:18 ` Andre Heider
2021-01-12 8:42 ` Andre Heider
2021-01-12 9:24 ` Pali Rohár
0 siblings, 2 replies; 15+ messages in thread
From: Andre Heider @ 2021-01-12 8:18 UTC (permalink / raw)
To: u-boot
Hi Pali,
On 11/01/2021 11:51, Pali Roh?r wrote:
> Hello Stefan and Andre!
>
> Could you please look at this patch series and tell me what do you think
> about it? If it is fine or needs to take different approach?
I like the idea very much, and I bet there're quite some boards which
could make good use of "immutable envvars".
The obvious review point is the filler thing and its dependency on
DEFAULT_ENV_IS_RW, which probably won't win a beauty contest :) Maybe a
nicer integration would help in getting it merged?
I don't think it would take too much effort, first thing that comes to mind:
- board provides list of immutable vars
- env_set_default() backs up these vars
- env_set_default() imports default_environment
- env_set_default() imports backup on top
The last step should be easy, see env_set_default_vars().
Maybe the first step can be solved with ENV_FLAGS_VAR, a new immutable
flag, and boards just making use of CONFIG_ENV_FLAGS_LIST_DEFAULT to
declare those. But I fail to find an example in-tree.
Thanks,
Andre
>
> On Wednesday 23 December 2020 12:21:27 Pali Roh?r wrote:
>> This patch series set default env values of $fdtfile and $ethNaddr for
>> Espressobin board at runtime.
>>
>> It fixes two main issues on Espressobin board that 'env default -a'
>> completely erases permanent board MAC addresses and also erase $fdtfile
>> variable which is needed for booting Linux kernel via distro boot.
>>
>> Lot of people were complaining about erasing permanent MAC addresses by
>> U-boot on this board and due to this issue some linux distributions
>> started using static hardcoded MAC addresses for all Espressobin boards
>> to workaround this issue. Apparently erase of MAC addresses or usage of
>> static hardcoded value caused more issues on network (e.g. inability to
>> connect two of these boards to the same network).
>>
>> Pali Roh?r (3):
>> env: Allow to set default_environment[] from board code via compile
>> option DEFAULT_ENV_IS_RW
>> arm: mvebu: Espressobin: Set default value for $fdtfile env variable
>> arm: mvebu: Espressobin: Set default value for $ethNaddr env variable
>>
>> board/Marvell/mvebu_armada-37xx/board.c | 41 ++++++++++++++++++++-----
>> include/configs/mvebu_armada-37xx.h | 17 +++++++++-
>> include/env_default.h | 2 ++
>> include/env_internal.h | 4 +++
>> 4 files changed, 56 insertions(+), 8 deletions(-)
>>
>> --
>> 2.20.1
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime
2021-01-12 8:18 ` Andre Heider
@ 2021-01-12 8:42 ` Andre Heider
2021-01-12 9:24 ` Pali Rohár
1 sibling, 0 replies; 15+ messages in thread
From: Andre Heider @ 2021-01-12 8:42 UTC (permalink / raw)
To: u-boot
On 12/01/2021 09:18, Andre Heider wrote:
...
> Maybe the first step can be solved with ENV_FLAGS_VAR, a new immutable
> flag, and boards just making use of CONFIG_ENV_FLAGS_LIST_DEFAULT to
> declare those. But I fail to find an example in-tree.
Found one, see ENV_FLAGS_LIST_STATIC. That even has ETHADDR_WILDCARD, so
when enabling CONFIG_REGEX it could look like this for espressobin:
CONFIG_ENV_FLAGS_LIST_DEFAULT="eth\d*addr:mX,fdtfile:X"
where "X" is the immutable flag.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime
2021-01-12 8:18 ` Andre Heider
2021-01-12 8:42 ` Andre Heider
@ 2021-01-12 9:24 ` Pali Rohár
2021-02-02 15:09 ` Stefan Roese
1 sibling, 1 reply; 15+ messages in thread
From: Pali Rohár @ 2021-01-12 9:24 UTC (permalink / raw)
To: u-boot
Hello!
On Tuesday 12 January 2021 09:18:44 Andre Heider wrote:
> Hi Pali,
>
> On 11/01/2021 11:51, Pali Roh?r wrote:
> > Hello Stefan and Andre!
> >
> > Could you please look at this patch series and tell me what do you think
> > about it? If it is fine or needs to take different approach?
>
> I like the idea very much, and I bet there're quite some boards which could
> make good use of "immutable envvars".
>
> The obvious review point is the filler thing and its dependency on
> DEFAULT_ENV_IS_RW, which probably won't win a beauty contest :) Maybe a
> nicer integration would help in getting it merged?
>
> I don't think it would take too much effort, first thing that comes to mind:
> - board provides list of immutable vars
> - env_set_default() backs up these vars
> - env_set_default() imports default_environment
> - env_set_default() imports backup on top
>
> The last step should be easy, see env_set_default_vars().
This could probably work for $ethNaddr variables.
But there is still an issue how to handle $fdtfile. There is basically
default value for this variable, but value itself cannot be determined
at compile time, only at runtime. And for it variable flags do not help,
we just need an mechanism how to set default variable values not only at
compile time but also runtime.
That is why I chosen for now solution with modifying
default_environment[] array as it solve issue for both $fdtfile and
$ethNaddr variables.
> Maybe the first step can be solved with ENV_FLAGS_VAR, a new immutable flag,
> and boards just making use of CONFIG_ENV_FLAGS_LIST_DEFAULT to declare
> those. But I fail to find an example in-tree.
>
> Thanks,
> Andre
>
> >
> > On Wednesday 23 December 2020 12:21:27 Pali Roh?r wrote:
> > > This patch series set default env values of $fdtfile and $ethNaddr for
> > > Espressobin board at runtime.
> > >
> > > It fixes two main issues on Espressobin board that 'env default -a'
> > > completely erases permanent board MAC addresses and also erase $fdtfile
> > > variable which is needed for booting Linux kernel via distro boot.
> > >
> > > Lot of people were complaining about erasing permanent MAC addresses by
> > > U-boot on this board and due to this issue some linux distributions
> > > started using static hardcoded MAC addresses for all Espressobin boards
> > > to workaround this issue. Apparently erase of MAC addresses or usage of
> > > static hardcoded value caused more issues on network (e.g. inability to
> > > connect two of these boards to the same network).
> > >
> > > Pali Roh?r (3):
> > > env: Allow to set default_environment[] from board code via compile
> > > option DEFAULT_ENV_IS_RW
> > > arm: mvebu: Espressobin: Set default value for $fdtfile env variable
> > > arm: mvebu: Espressobin: Set default value for $ethNaddr env variable
> > >
> > > board/Marvell/mvebu_armada-37xx/board.c | 41 ++++++++++++++++++++-----
> > > include/configs/mvebu_armada-37xx.h | 17 +++++++++-
> > > include/env_default.h | 2 ++
> > > include/env_internal.h | 4 +++
> > > 4 files changed, 56 insertions(+), 8 deletions(-)
> > >
> > > --
> > > 2.20.1
> > >
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime
2020-12-23 11:21 [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime Pali Rohár
` (3 preceding siblings ...)
2021-01-11 10:51 ` [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime Pali Rohár
@ 2021-02-01 15:24 ` Pali Rohár
2021-02-08 11:33 ` Stefan Roese
5 siblings, 0 replies; 15+ messages in thread
From: Pali Rohár @ 2021-02-01 15:24 UTC (permalink / raw)
To: u-boot
Stefan, could you please look at this patch series, what do you think about it?
On Wednesday 23 December 2020 12:21:27 Pali Roh?r wrote:
> This patch series set default env values of $fdtfile and $ethNaddr for
> Espressobin board at runtime.
>
> It fixes two main issues on Espressobin board that 'env default -a'
> completely erases permanent board MAC addresses and also erase $fdtfile
> variable which is needed for booting Linux kernel via distro boot.
>
> Lot of people were complaining about erasing permanent MAC addresses by
> U-boot on this board and due to this issue some linux distributions
> started using static hardcoded MAC addresses for all Espressobin boards
> to workaround this issue. Apparently erase of MAC addresses or usage of
> static hardcoded value caused more issues on network (e.g. inability to
> connect two of these boards to the same network).
>
> Pali Roh?r (3):
> env: Allow to set default_environment[] from board code via compile
> option DEFAULT_ENV_IS_RW
> arm: mvebu: Espressobin: Set default value for $fdtfile env variable
> arm: mvebu: Espressobin: Set default value for $ethNaddr env variable
>
> board/Marvell/mvebu_armada-37xx/board.c | 41 ++++++++++++++++++++-----
> include/configs/mvebu_armada-37xx.h | 17 +++++++++-
> include/env_default.h | 2 ++
> include/env_internal.h | 4 +++
> 4 files changed, 56 insertions(+), 8 deletions(-)
>
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime
2021-01-12 9:24 ` Pali Rohár
@ 2021-02-02 15:09 ` Stefan Roese
2021-02-02 15:19 ` Pali Rohár
2021-02-02 16:13 ` Andre Heider
0 siblings, 2 replies; 15+ messages in thread
From: Stefan Roese @ 2021-02-02 15:09 UTC (permalink / raw)
To: u-boot
Hi Pali,
Hi Andre,
On 12.01.21 10:24, Pali Roh?r wrote:
> Hello!
>
> On Tuesday 12 January 2021 09:18:44 Andre Heider wrote:
>> Hi Pali,
>>
>> On 11/01/2021 11:51, Pali Roh?r wrote:
>>> Hello Stefan and Andre!
>>>
>>> Could you please look at this patch series and tell me what do you think
>>> about it? If it is fine or needs to take different approach?
>>
>> I like the idea very much, and I bet there're quite some boards which could
>> make good use of "immutable envvars".
>>
>> The obvious review point is the filler thing and its dependency on
>> DEFAULT_ENV_IS_RW, which probably won't win a beauty contest :) Maybe a
>> nicer integration would help in getting it merged?
>>
>> I don't think it would take too much effort, first thing that comes to mind:
>> - board provides list of immutable vars
>> - env_set_default() backs up these vars
>> - env_set_default() imports default_environment
>> - env_set_default() imports backup on top
>>
>> The last step should be easy, see env_set_default_vars().
>
> This could probably work for $ethNaddr variables.
>
> But there is still an issue how to handle $fdtfile. There is basically
> default value for this variable, but value itself cannot be determined
> at compile time, only at runtime. And for it variable flags do not help,
> we just need an mechanism how to set default variable values not only at
> compile time but also runtime.
>
> That is why I chosen for now solution with modifying
> default_environment[] array as it solve issue for both $fdtfile and
> $ethNaddr variables.
So what is the outcome of this discussion? Andre, do you see any
hindering points in this patch series, apart from it not winning a
"beauty contest"? ;)
I tend to pull it in shortly, if nobody objects.
Thanks,
Stefan
>> Maybe the first step can be solved with ENV_FLAGS_VAR, a new immutable flag,
>> and boards just making use of CONFIG_ENV_FLAGS_LIST_DEFAULT to declare
>> those. But I fail to find an example in-tree.
>>
>> Thanks,
>> Andre
>>
>>>
>>> On Wednesday 23 December 2020 12:21:27 Pali Roh?r wrote:
>>>> This patch series set default env values of $fdtfile and $ethNaddr for
>>>> Espressobin board at runtime.
>>>>
>>>> It fixes two main issues on Espressobin board that 'env default -a'
>>>> completely erases permanent board MAC addresses and also erase $fdtfile
>>>> variable which is needed for booting Linux kernel via distro boot.
>>>>
>>>> Lot of people were complaining about erasing permanent MAC addresses by
>>>> U-boot on this board and due to this issue some linux distributions
>>>> started using static hardcoded MAC addresses for all Espressobin boards
>>>> to workaround this issue. Apparently erase of MAC addresses or usage of
>>>> static hardcoded value caused more issues on network (e.g. inability to
>>>> connect two of these boards to the same network).
>>>>
>>>> Pali Roh?r (3):
>>>> env: Allow to set default_environment[] from board code via compile
>>>> option DEFAULT_ENV_IS_RW
>>>> arm: mvebu: Espressobin: Set default value for $fdtfile env variable
>>>> arm: mvebu: Espressobin: Set default value for $ethNaddr env variable
>>>>
>>>> board/Marvell/mvebu_armada-37xx/board.c | 41 ++++++++++++++++++++-----
>>>> include/configs/mvebu_armada-37xx.h | 17 +++++++++-
>>>> include/env_default.h | 2 ++
>>>> include/env_internal.h | 4 +++
>>>> 4 files changed, 56 insertions(+), 8 deletions(-)
>>>>
>>>> --
>>>> 2.20.1
>>>>
>>
Viele Gr??e,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime
2021-02-02 15:09 ` Stefan Roese
@ 2021-02-02 15:19 ` Pali Rohár
2021-02-02 16:13 ` Andre Heider
1 sibling, 0 replies; 15+ messages in thread
From: Pali Rohár @ 2021-02-02 15:19 UTC (permalink / raw)
To: u-boot
On Tuesday 02 February 2021 16:09:03 Stefan Roese wrote:
> Hi Pali,
> Hi Andre,
>
> On 12.01.21 10:24, Pali Roh?r wrote:
> > Hello!
> >
> > On Tuesday 12 January 2021 09:18:44 Andre Heider wrote:
> > > Hi Pali,
> > >
> > > On 11/01/2021 11:51, Pali Roh?r wrote:
> > > > Hello Stefan and Andre!
> > > >
> > > > Could you please look at this patch series and tell me what do you think
> > > > about it? If it is fine or needs to take different approach?
> > >
> > > I like the idea very much, and I bet there're quite some boards which could
> > > make good use of "immutable envvars".
> > >
> > > The obvious review point is the filler thing and its dependency on
> > > DEFAULT_ENV_IS_RW, which probably won't win a beauty contest :) Maybe a
> > > nicer integration would help in getting it merged?
> > >
> > > I don't think it would take too much effort, first thing that comes to mind:
> > > - board provides list of immutable vars
> > > - env_set_default() backs up these vars
> > > - env_set_default() imports default_environment
> > > - env_set_default() imports backup on top
> > >
> > > The last step should be easy, see env_set_default_vars().
> >
> > This could probably work for $ethNaddr variables.
> >
> > But there is still an issue how to handle $fdtfile. There is basically
> > default value for this variable, but value itself cannot be determined
> > at compile time, only at runtime. And for it variable flags do not help,
> > we just need an mechanism how to set default variable values not only at
> > compile time but also runtime.
> >
> > That is why I chosen for now solution with modifying
> > default_environment[] array as it solve issue for both $fdtfile and
> > $ethNaddr variables.
>
> So what is the outcome of this discussion? Andre, do you see any
> hindering points in this patch series, apart from it not winning a
> "beauty contest"? ;)
Hello! I have looked at it again and I can say that implementing a new
"immutable" bit for ethNaddr env variables would be better / cleaner
solution. But as I wrote for fdtfile env variable that immutable bit
does not help and we need some option how to set default value of this
variable at the runtime.
> I tend to pull it in shortly, if nobody objects.
Andre thought that other u-boot developers would not like this approach.
But I have not received any response, so I do not know if just nobody
looked at this patch or more people looked at it and did not have
objections.
Anyway, Andre are you going to look & implement for ethNaddr env
variables that approach via immutable bits?
I think current implementation can be changed at anytime in future.
> Thanks,
> Stefan
>
> > > Maybe the first step can be solved with ENV_FLAGS_VAR, a new immutable flag,
> > > and boards just making use of CONFIG_ENV_FLAGS_LIST_DEFAULT to declare
> > > those. But I fail to find an example in-tree.
> > >
> > > Thanks,
> > > Andre
> > >
> > > >
> > > > On Wednesday 23 December 2020 12:21:27 Pali Roh?r wrote:
> > > > > This patch series set default env values of $fdtfile and $ethNaddr for
> > > > > Espressobin board at runtime.
> > > > >
> > > > > It fixes two main issues on Espressobin board that 'env default -a'
> > > > > completely erases permanent board MAC addresses and also erase $fdtfile
> > > > > variable which is needed for booting Linux kernel via distro boot.
> > > > >
> > > > > Lot of people were complaining about erasing permanent MAC addresses by
> > > > > U-boot on this board and due to this issue some linux distributions
> > > > > started using static hardcoded MAC addresses for all Espressobin boards
> > > > > to workaround this issue. Apparently erase of MAC addresses or usage of
> > > > > static hardcoded value caused more issues on network (e.g. inability to
> > > > > connect two of these boards to the same network).
> > > > >
> > > > > Pali Roh?r (3):
> > > > > env: Allow to set default_environment[] from board code via compile
> > > > > option DEFAULT_ENV_IS_RW
> > > > > arm: mvebu: Espressobin: Set default value for $fdtfile env variable
> > > > > arm: mvebu: Espressobin: Set default value for $ethNaddr env variable
> > > > >
> > > > > board/Marvell/mvebu_armada-37xx/board.c | 41 ++++++++++++++++++++-----
> > > > > include/configs/mvebu_armada-37xx.h | 17 +++++++++-
> > > > > include/env_default.h | 2 ++
> > > > > include/env_internal.h | 4 +++
> > > > > 4 files changed, 56 insertions(+), 8 deletions(-)
> > > > >
> > > > > --
> > > > > 2.20.1
> > > > >
> > >
>
>
> Viele Gr??e,
> Stefan
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime
2021-02-02 15:09 ` Stefan Roese
2021-02-02 15:19 ` Pali Rohár
@ 2021-02-02 16:13 ` Andre Heider
2021-02-02 16:32 ` Stefan Roese
1 sibling, 1 reply; 15+ messages in thread
From: Andre Heider @ 2021-02-02 16:13 UTC (permalink / raw)
To: u-boot
On 02/02/2021 16:09, Stefan Roese wrote:
> Hi Pali,
> Hi Andre,
>
> On 12.01.21 10:24, Pali Roh?r wrote:
>> Hello!
>>
>> On Tuesday 12 January 2021 09:18:44 Andre Heider wrote:
>>> Hi Pali,
>>>
>>> On 11/01/2021 11:51, Pali Roh?r wrote:
>>>> Hello Stefan and Andre!
>>>>
>>>> Could you please look at this patch series and tell me what do you
>>>> think
>>>> about it? If it is fine or needs to take different approach?
>>>
>>> I like the idea very much, and I bet there're quite some boards which
>>> could
>>> make good use of "immutable envvars".
>>>
>>> The obvious review point is the filler thing and its dependency on
>>> DEFAULT_ENV_IS_RW, which probably won't win a beauty contest :) Maybe a
>>> nicer integration would help in getting it merged?
>>>
>>> I don't think it would take too much effort, first thing that comes
>>> to mind:
>>> - board provides list of immutable vars
>>> - env_set_default() backs up these vars
>>> - env_set_default() imports default_environment
>>> - env_set_default() imports backup on top
>>>
>>> The last step should be easy, see env_set_default_vars().
>>
>> This could probably work for $ethNaddr variables.
>>
>> But there is still an issue how to handle $fdtfile. There is basically
>> default value for this variable, but value itself cannot be determined
>> at compile time, only at runtime. And for it variable flags do not help,
>> we just need an mechanism how to set default variable values not only at
>> compile time but also runtime.
>>
>> That is why I chosen for now solution with modifying
>> default_environment[] array as it solve issue for both $fdtfile and
>> $ethNaddr variables.
>
> So what is the outcome of this discussion? Andre, do you see any
> hindering points in this patch series, apart from it not winning a
> "beauty contest"? ;)
Hehe, nope, only aesthetic concerns, no hinderung points to block this
going in.
Thanks,
Andre
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime
2021-02-02 16:13 ` Andre Heider
@ 2021-02-02 16:32 ` Stefan Roese
2021-02-02 17:24 ` Andre Heider
0 siblings, 1 reply; 15+ messages in thread
From: Stefan Roese @ 2021-02-02 16:32 UTC (permalink / raw)
To: u-boot
On 02.02.21 17:13, Andre Heider wrote:
> On 02/02/2021 16:09, Stefan Roese wrote:
>> Hi Pali,
>> Hi Andre,
>>
>> On 12.01.21 10:24, Pali Roh?r wrote:
>>> Hello!
>>>
>>> On Tuesday 12 January 2021 09:18:44 Andre Heider wrote:
>>>> Hi Pali,
>>>>
>>>> On 11/01/2021 11:51, Pali Roh?r wrote:
>>>>> Hello Stefan and Andre!
>>>>>
>>>>> Could you please look at this patch series and tell me what do you
>>>>> think
>>>>> about it? If it is fine or needs to take different approach?
>>>>
>>>> I like the idea very much, and I bet there're quite some boards
>>>> which could
>>>> make good use of "immutable envvars".
>>>>
>>>> The obvious review point is the filler thing and its dependency on
>>>> DEFAULT_ENV_IS_RW, which probably won't win a beauty contest :) Maybe a
>>>> nicer integration would help in getting it merged?
>>>>
>>>> I don't think it would take too much effort, first thing that comes
>>>> to mind:
>>>> - board provides list of immutable vars
>>>> - env_set_default() backs up these vars
>>>> - env_set_default() imports default_environment
>>>> - env_set_default() imports backup on top
>>>>
>>>> The last step should be easy, see env_set_default_vars().
>>>
>>> This could probably work for $ethNaddr variables.
>>>
>>> But there is still an issue how to handle $fdtfile. There is basically
>>> default value for this variable, but value itself cannot be determined
>>> at compile time, only at runtime. And for it variable flags do not help,
>>> we just need an mechanism how to set default variable values not only at
>>> compile time but also runtime.
>>>
>>> That is why I chosen for now solution with modifying
>>> default_environment[] array as it solve issue for both $fdtfile and
>>> $ethNaddr variables.
>>
>> So what is the outcome of this discussion? Andre, do you see any
>> hindering points in this patch series, apart from it not winning a
>> "beauty contest"? ;)
>
> Hehe, nope, only aesthetic concerns, no hinderung points to block this
> going in.
I see. Then if appropriate, please send any matching tag(s) to these
patches.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime
2021-02-02 16:32 ` Stefan Roese
@ 2021-02-02 17:24 ` Andre Heider
0 siblings, 0 replies; 15+ messages in thread
From: Andre Heider @ 2021-02-02 17:24 UTC (permalink / raw)
To: u-boot
On 02/02/2021 17:32, Stefan Roese wrote:
> On 02.02.21 17:13, Andre Heider wrote:
>> On 02/02/2021 16:09, Stefan Roese wrote:
>>> Hi Pali,
>>> Hi Andre,
>>>
>>> On 12.01.21 10:24, Pali Roh?r wrote:
>>>> Hello!
>>>>
>>>> On Tuesday 12 January 2021 09:18:44 Andre Heider wrote:
>>>>> Hi Pali,
>>>>>
>>>>> On 11/01/2021 11:51, Pali Roh?r wrote:
>>>>>> Hello Stefan and Andre!
>>>>>>
>>>>>> Could you please look at this patch series and tell me what do you
>>>>>> think
>>>>>> about it? If it is fine or needs to take different approach?
>>>>>
>>>>> I like the idea very much, and I bet there're quite some boards
>>>>> which could
>>>>> make good use of "immutable envvars".
>>>>>
>>>>> The obvious review point is the filler thing and its dependency on
>>>>> DEFAULT_ENV_IS_RW, which probably won't win a beauty contest :)
>>>>> Maybe a
>>>>> nicer integration would help in getting it merged?
>>>>>
>>>>> I don't think it would take too much effort, first thing that comes
>>>>> to mind:
>>>>> - board provides list of immutable vars
>>>>> - env_set_default() backs up these vars
>>>>> - env_set_default() imports default_environment
>>>>> - env_set_default() imports backup on top
>>>>>
>>>>> The last step should be easy, see env_set_default_vars().
>>>>
>>>> This could probably work for $ethNaddr variables.
>>>>
>>>> But there is still an issue how to handle $fdtfile. There is basically
>>>> default value for this variable, but value itself cannot be determined
>>>> at compile time, only at runtime. And for it variable flags do not
>>>> help,
>>>> we just need an mechanism how to set default variable values not
>>>> only at
>>>> compile time but also runtime.
>>>>
>>>> That is why I chosen for now solution with modifying
>>>> default_environment[] array as it solve issue for both $fdtfile and
>>>> $ethNaddr variables.
>>>
>>> So what is the outcome of this discussion? Andre, do you see any
>>> hindering points in this patch series, apart from it not winning a
>>> "beauty contest"? ;)
>>
>> Hehe, nope, only aesthetic concerns, no hinderung points to block this
>> going in.
>
> I see. Then if appropriate, please send any matching tag(s) to these
> patches.
I didn't test it, so this set is at least:
Acked-by: Andre Heider <a.heider@gmail.com>
Thanks,
Andre
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime
2020-12-23 11:21 [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime Pali Rohár
` (4 preceding siblings ...)
2021-02-01 15:24 ` Pali Rohár
@ 2021-02-08 11:33 ` Stefan Roese
5 siblings, 0 replies; 15+ messages in thread
From: Stefan Roese @ 2021-02-08 11:33 UTC (permalink / raw)
To: u-boot
On 23.12.20 12:21, Pali Roh?r wrote:
> This patch series set default env values of $fdtfile and $ethNaddr for
> Espressobin board at runtime.
>
> It fixes two main issues on Espressobin board that 'env default -a'
> completely erases permanent board MAC addresses and also erase $fdtfile
> variable which is needed for booting Linux kernel via distro boot.
>
> Lot of people were complaining about erasing permanent MAC addresses by
> U-boot on this board and due to this issue some linux distributions
> started using static hardcoded MAC addresses for all Espressobin boards
> to workaround this issue. Apparently erase of MAC addresses or usage of
> static hardcoded value caused more issues on network (e.g. inability to
> connect two of these boards to the same network).
>
> Pali Roh?r (3):
> env: Allow to set default_environment[] from board code via compile
> option DEFAULT_ENV_IS_RW
> arm: mvebu: Espressobin: Set default value for $fdtfile env variable
> arm: mvebu: Espressobin: Set default value for $ethNaddr env variable
>
> board/Marvell/mvebu_armada-37xx/board.c | 41 ++++++++++++++++++++-----
> include/configs/mvebu_armada-37xx.h | 17 +++++++++-
> include/env_default.h | 2 ++
> include/env_internal.h | 4 +++
> 4 files changed, 56 insertions(+), 8 deletions(-)
>
Applied to u-boot-marvell/master
Thanks,
Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-02-08 11:33 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-23 11:21 [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime Pali Rohár
2020-12-23 11:21 ` [PATCH 1/3] env: Allow to set default_environment[] from board code via compile option DEFAULT_ENV_IS_RW Pali Rohár
2020-12-23 11:21 ` [PATCH 2/3] arm: mvebu: Espressobin: Set default value for $fdtfile env variable Pali Rohár
2020-12-23 11:21 ` [PATCH 3/3] arm: mvebu: Espressobin: Set default value for $ethNaddr " Pali Rohár
2021-01-11 10:51 ` [PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime Pali Rohár
2021-01-12 8:18 ` Andre Heider
2021-01-12 8:42 ` Andre Heider
2021-01-12 9:24 ` Pali Rohár
2021-02-02 15:09 ` Stefan Roese
2021-02-02 15:19 ` Pali Rohár
2021-02-02 16:13 ` Andre Heider
2021-02-02 16:32 ` Stefan Roese
2021-02-02 17:24 ` Andre Heider
2021-02-01 15:24 ` Pali Rohár
2021-02-08 11:33 ` Stefan Roese
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.