[PATCH 0/3] arm: mvebu: Espressobin: Set default env values at runtime

[Pali Rohár]

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