[PATCH 2/2] rockchip: ringneck-px30: enforce ENV_IS_NOWHERE with Kconfig select

[Quentin Schulz]

Hi Troy,

On 3/9/23 20:34, Troy Kisky wrote:
> On Wed, Mar 1, 2023 at 9:39 AM Quentin Schulz <foss+uboot at 0leil.net> wrote:
> 
>> From: Quentin Schulz <quentin.schulz at theobroma-systems.com>
>>
>> Instead of letting the compiler error out if CONFIG_ENV_IS_NOWHERE is
>> not selected by the user, let's just enforce it when the user builds for
>> Ringneck PX30 so that no check needs to be performed by the compiler and
>> the configuration is always valid.
>>
>> Suggested-by: Tom Rini <trini at konsulko.com>
>> Cc: Quentin Schulz <foss+uboot at 0leil.net>
>> Signed-off-by: Quentin Schulz <quentin.schulz at theobroma-systems.com>
>> ---
>>   board/theobroma-systems/ringneck_px30/Kconfig         | 1 +
>>   board/theobroma-systems/ringneck_px30/ringneck-px30.c | 4 ----
>>   configs/ringneck-px30_defconfig                       | 1 -
>>   3 files changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/board/theobroma-systems/ringneck_px30/Kconfig
>> b/board/theobroma-systems/ringneck_px30/Kconfig
>> index 24d94807db8..c33253bdad8 100644
>> --- a/board/theobroma-systems/ringneck_px30/Kconfig
>> +++ b/board/theobroma-systems/ringneck_px30/Kconfig
>> @@ -11,6 +11,7 @@ config SYS_CONFIG_NAME
>>
>>   config BOARD_SPECIFIC_OPTIONS # dummy
>>          def_bool y
>> +       select ENV_IS_NOWHERE
>>
>>   config ENV_SIZE
>>          default 0x3000
>> diff --git a/board/theobroma-systems/ringneck_px30/ringneck-px30.c
>> b/board/theobroma-systems/ringneck_px30/ringneck-px30.c
>> index 47d1a40ef7c..bb1bb4acf5c 100644
>> --- a/board/theobroma-systems/ringneck_px30/ringneck-px30.c
>> +++ b/board/theobroma-systems/ringneck_px30/ringneck-px30.c
>> @@ -118,10 +118,6 @@ int mmc_get_env_dev(void)
>>          return CONFIG_SYS_MMC_ENV_DEV;
>>   }
>>
>> -#if !IS_ENABLED(CONFIG_ENV_IS_NOWHERE)
>> -#error Please enable CONFIG_ENV_IS_NOWHERE
>> -#endif
>> -
>>   enum env_location arch_env_get_location(enum env_operation op, int prio)
>>   {
>>          const char *boot_device =
>> diff --git a/configs/ringneck-px30_defconfig
>> b/configs/ringneck-px30_defconfig
>> index 91706d8def2..e9234efc2a0 100644
>> --- a/configs/ringneck-px30_defconfig
>> +++ b/configs/ringneck-px30_defconfig
>> @@ -68,7 +68,6 @@ CONFIG_SPL_OF_CONTROL=y
>>   CONFIG_OF_LIVE=y
>>   CONFIG_OF_SPL_REMOVE_PROPS="interrupt-parent assigned-clocks
>> assigned-clock-rates assigned-clock-parents"
>>   CONFIG_ENV_OVERWRITE=y
>> -CONFIG_ENV_IS_NOWHERE=y
>>   CONFIG_ENV_IS_IN_MMC=y
>>
> 
> 
> Hi Quentin,
> 
> Should you delete the line
>   CONFIG_ENV_IS_IN_MMC=y
> 
> as well ?
> 

We want to allow the user to pick either SPI flash or eMMC flash or both 
storage media (or none) as possible storage location for the U-Boot 
environment.

Since there's no need to enforce support for both flash storage medium 
kinds, I don't believe we should do the same as ENV_IS_NOWHERE for 
ENV_IS_IN_MMC or ENV_IS_IN_SPI_FLASH especially since this is handled in 
the board C file, c.f.: 
https://source.denx.de/u-boot/u-boot/-/blob/master/board/theobroma-systems/ringneck_px30/ringneck-px30.c#L141-L144 
and 
https://source.denx.de/u-boot/u-boot/-/blob/master/board/theobroma-systems/puma_rk3399/puma-rk3399.c#L159-L166.

While we aren't too much interested in no storage medium for U-Boot 
environment, this is unfortunately a requirement in how U-Boot handles 
missing U-Boot environment in supported storage media for the 
environment, so here we are :)

Does that answer your question?

BR,
Quentin