Hi,
ST Restricted > -----Original Message----- > From: U-Boot <u-boot-boun...@lists.denx.de> On Behalf Of Quentin Schulz > Sent: mardi 6 septembre 2022 11:23 > To: Kever Yang <kever.y...@rock-chips.com>; Quentin Schulz > <foss+ub...@0leil.net> > Cc: s...@chromium.org; philipp.toms...@vrull.eu; klaus.goger@theobroma- > systems.com; knaerz...@gmail.com; u-boot@lists.denx.de > Subject: Re: [PATCH 07/12] rockchip: puma-rk3399: load environment from same > medium as one used to load U-Boot proper > > Hi Kever, > > On 9/4/22 13:49, Kever Yang wrote: > > 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. > > > > It's *default* to yes if none is defined. It's not a requirement. > > > 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. > > > > This can be nowhere in case the proper config option is not selected. > E.g. we're booting from SPI-NOR but the ENV_IS_IN_SPI_FLASH is not set. > How does one handle this scenario? Since we don't want fallbacks, it needs to > be > "nowhere". What you're suggesting is to let the user run a broken build and > have it > cryptically crash with the following error: > > initcall sequence 00000000002866c0 failed at call 0000000000256b34 (err=-19) > > Instead, I suggest to more gracefully error out at runtime by letting the > user know > that the option is not compiled in but U-Boot is still usable. > > If you have another way to forbid fallbacks but not crash U-Boot in case the > option > is not selected, let me know. > For information : It is exactly that I do to stm32mp15 platform See board/st/stm32mp1/stm32mp1.c::env_get_location() ENV_IS_NOWHERE = use the default environment, embedded in U-Boot and env It is the fallback if environment not activated ENV_IS_IN_* for the boot device, even it is not the case by default in stm32mp15_defconfig. And it is why , I change Kconfig to allow activation of any ENV_IS_IN_* and ENV_IS_NOWHERE Patrick > Cheers, > Quentin > > > > 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.sch...@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+ub...@0leil.net> > >>>> Signed-off-by: Quentin Schulz > >>>> <quentin.sch...@theobroma-systems.com> > >>>> --- > >>>> > >>>> Depends on > >>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.or > >>>> g_u-2Dboot_20220715151552.953654-2D1-2Dfoss-2Buboot-400leil.net_&d= > >>>> > DwIDaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSq > HGq8 > >>>> yBP6m6qZZ4njZguQhZhkI_- > 172IIy1t&m=TZndtGz1ePTd2Il6YcEjqzo9oXv73RCWH > >>>> > IRVSiFVsnp2OzyCJEDzZ2KPz56AcWdn&s=wgEMbr3EjeCtvcWU_UoXqNOwQula > VN-0Q > >>>> b2yL2ysaOs&e= > >>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.or > >>>> g_u-2Dboot_20220715151552.953654-2D2-2Dfoss-2Buboot-400leil.net_&d= > >>>> > DwIDaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSq > HGq8 > >>>> yBP6m6qZZ4njZguQhZhkI_- > 172IIy1t&m=TZndtGz1ePTd2Il6YcEjqzo9oXv73RCWH > >>>> > IRVSiFVsnp2OzyCJEDzZ2KPz56AcWdn&s=PKwYBMB7r8ekIPV1ZG7xkj7vF60YN > FlYX > >>>> QRrvaVgJR8&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
