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

[Kever Yang]

Hi Quentin,

The Kconfig from env/Kconfig

config ENV_IS_NOWHERE
         bool "Environment is not stored"
         default y if !ENV_IS_IN_EEPROM && !ENV_IS_IN_EXT4 && \
                      !ENV_IS_IN_FAT && !ENV_IS_IN_FLASH && \
                      !ENV_IS_IN_MMC && !ENV_IS_IN_NAND && \
                      !ENV_IS_IN_NVRAM && !ENV_IS_IN_ONENAND && \
                      !ENV_IS_IN_REMOTE && !ENV_IS_IN_SPI_FLASH && \
                      !ENV_IS_IN_UBI

I think the logic is the env parameter is stored on some kind of 
storage, or NOWHERE.

And what you want to do is to load from the same medium as SPL boot 
device(location of U-Boot proper),

this could not be NOWHERE.


Thanks,

- Kever

On 2022/9/1 21:13, Quentin Schulz wrote:
> 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 at 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 at 0leil.net>
>>> Signed-off-by: Quentin Schulz <quentin.schulz at 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 at ff1d0000/flash at 0"))
>>> +        return ENVL_SPI_FLASH;
>>> +
>>> +    if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC) &&
>>> +        (!strcmp(boot_device, "/mmc at fe320000") ||
>>> +         !strcmp(boot_device, "/mmc at 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