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.

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.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

Reply via email to