Re: [PATCH 07/12] rockchip: puma-rk3399: load environment from same medium as one used to load U-Boot proper

[Quentin Schulz <quentin.schulz@theobroma-systems.com>]

Hi Kever,

On 9/20/22 14:28, Kever Yang wrote:
> Hi Patrick, Quentin,
> 
> 
> Here is the definition about the ENV_IS_NOWHERE:
> 
> config ENV_IS_NOWHERE
>          bool "Environment is not stored"
> help
>            Define this if you don't want to or can't have an environment 
> stored
>            on a storage medium. In this case the environemnt will still 
> exist
>            while U-Boot is running, but once U-Boot exits it will not be
>            stored. U-Boot will therefore always start up with a default
>            environment.
> 
> 
> Which means ENV_IS_NOWHERE is ALWAYS use default environment,
> 
> but not stored on a storage medium.
> 
> I think what you want is a new ENV_IS_ANYWHERE which not able to
> 
> decide when the firmware is build but must be some where when the boot
> 
> device is decided.
> 

I do not share the same understanding. For me, ENV_IS_NOWHERE means the 
environment is stored in RAM, once you exit U-Boot or reset the board, 
it's gone. That's my understanding of the code, I can concede that the 
help message of the Kconfig option is confusing. It's just another 
"kind" of environment to me.

If the point was to ALWAYS use the default environment, one wouldn't be 
able to enable the option while other ENV_IS_IN_* are enabled. It is 
however possible.

ENV_IS_ANYWHERE is not a correct name for what I want, because I 
specifically do NOT want to load from anywhere. I want to load from a 
specific medium, and if not possible have a fallback to avoid U-Boot 
cryptically crashing.

Maybe we should rename ENV_IS_NOWHERE to ENV_IS_IN_RAM, maybe we could 
also stop crashing if there's no medium to load the environment from 
that is available, maybe we could rephrase the help text of the Kconfig 
option, but this is unrelated to this patch series.

Finally, STM32, some i.MX and a couple of Xilinx based boards actually 
have ENV_IS_NOWHERE enabled at the same time as other ENV_IS_IN_* 
options, so I'm clearly not the first one to use it this way. Also, see 
arch/arm/mach-imx/imx8m/soc.c for an implementation of 
arch_env_get_location that requires ENV_IS_NOWHERE to work and has 
almost the same logic as I'm trying to implement.

I'm trying to fix a non-booting board. This patch series is also only 
impacting the board I'm maintaining and nothing else. If merging the v2 
of this patch series is really asking you something unimaginable, just 
drop this patch from the series, merge the rest and we'll continue 
arguing on a resend.

Quentin