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/1/22 15:03, Kever Yang wrote:
> Hi Quentin,
> 
> On 2022/7/23 00:06, Quentin Schulz wrote:
>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>
>> Chances are when one boots U-Boot proper from a given storage medium,
>> they want the same medium to be used to load and store the environment.
>>
>> This basically allows to have completely separate U-Boot (TPL/SPL/U-Boot
>> proper/environment) per storage medium which is convenient when working
>> with recovery from SD-Card as one would just need to insert a properly
>> configured SD-Card into the device to have access to their whole debug
>> setup.
>>
>> No fallback mechanism is provided as to not dirty other storage medium
>> environment by mistake. However, since arch_env_get_location() is called
>> by env_init() which is part of the pre-relocation process, a valid,
>> non-ENVL_UNKNOWN, value shall be returned otherwise the relocation fails
>> with the following message:
>> initcall sequence 00000000002866c0 failed at call 0000000000256b34 
>> (err=-19)
>>
>> This valid, non-ENVL_UNKNOWN, value is ENVL_NOWHERE which requires to
>> always select CONFIG_ENV_IS_NOWHERE otherwise this work-around does not
>> work.
>>
>> Cc: Quentin Schulz <foss+uboot@0leil.net>
>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>> ---
>>
>> Depends on
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_u-2Dboot_20220715151552.953654-2D1-2Dfoss-2Buboot-400leil.net_&d=DwIDaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=TZndtGz1ePTd2Il6YcEjqzo9oXv73RCWHIRVSiFVsnp2OzyCJEDzZ2KPz56AcWdn&s=wgEMbr3EjeCtvcWU_UoXqNOwQulaVN-0Qb2yL2ysaOs&e= https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_u-2Dboot_20220715151552.953654-2D2-2Dfoss-2Buboot-400leil.net_&d=DwIDaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=TZndtGz1ePTd2Il6YcEjqzo9oXv73RCWHIRVSiFVsnp2OzyCJEDzZ2KPz56AcWdn&s=PKwYBMB7r8ekIPV1ZG7xkj7vF60YNFlYXQRrvaVgJR8&e=
>>   .../puma_rk3399/puma-rk3399.c                 | 37 +++++++++++++++++++
>>   configs/puma-rk3399_defconfig                 |  1 +
>>   2 files changed, 38 insertions(+)
>>
>> diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c 
>> b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
>> index 5e5e58c88e..7ef4bac24b 100644
>> --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
>> +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
>> @@ -6,6 +6,7 @@
>>   #include <common.h>
>>   #include <dm.h>
>>   #include <env.h>
>> +#include <env_internal.h>
>>   #include <init.h>
>>   #include <log.h>
>>   #include <misc.h>
>> @@ -135,6 +136,42 @@ 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 =
>> +        ofnode_read_chosen_string("u-boot,spl-boot-device");
>> +
>> +    if (prio > 0)
>> +        return ENVL_UNKNOWN;
>> +
>> +    if (!boot_device) {
>> +        debug("%s: /chosen/u-boot,spl-boot-device not set\n",
>> +              __func__);
>> +        return ENVL_NOWHERE;
>> +    }
>> +
>> +    debug("%s: booted from %s\n", __func__, boot_device);
>> +
>> +    if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH) &&
>> +        !strcmp(boot_device, "/spi@ff1d0000/flash@0"))
>> +        return ENVL_SPI_FLASH;
>> +
>> +    if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC) &&
>> +        (!strcmp(boot_device, "/mmc@fe320000") ||
>> +         !strcmp(boot_device, "/mmc@fe330000")))
>> +        return ENVL_MMC;
>> +
>> +    printf("%s: No environment available: booted from %s but U-Boot "
>> +           "config does not allow loading environment from it.",
>> +           __func__, boot_device);
>> +
>> +    return ENVL_NOWHERE;
>> +}
>> +
>>   int misc_init_r(void)
>>   {
>>       const u32 cpuid_offset = 0x7;
>> diff --git a/configs/puma-rk3399_defconfig 
>> b/configs/puma-rk3399_defconfig
>> index 87d7e4f57c..e218532d70 100644
>> --- a/configs/puma-rk3399_defconfig
>> +++ b/configs/puma-rk3399_defconfig
>> @@ -44,6 +44,7 @@ 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
> 
> This option is conflict with CONFIG_ENV_IS_IN_MMC,  please check again 
> where should be this board get the env.
> 

I created the defconfig with make savedefconfig, so if you're talking 
about KConfig conflict, that is incorrect, there is no conflict.

If you're talking about something else, please clarify because I don't 
see the issue right now.

Cheers,
Quentin