From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
To: Kever Yang <kever.yang@rock-chips.com>,
Quentin Schulz <foss+uboot@0leil.net>
Cc: sjg@chromium.org, philipp.tomsich@vrull.eu,
klaus.goger@theobroma-systems.com, knaerzche@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
Date: Tue, 6 Sep 2022 11:22:46 +0200 [thread overview]
Message-ID: <4feb827e-3932-c8d6-120e-5465ad92eac2@theobroma-systems.com> (raw)
In-Reply-To: <873d5eaa-b0ee-ae55-143b-8ddb340b2fbf@rock-chips.com>
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.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
next prev parent reply other threads:[~2022-09-06 9:22 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-22 16:06 [PATCH 00/12] Puma RK3399 migration to TPL and numerous fixes Quentin Schulz
2022-07-22 16:06 ` [PATCH 01/12] rockchip: puma-rk3399: fix boot_targets swap depending on U-Boot proper load medium Quentin Schulz
2022-09-04 11:41 ` Kever Yang
2022-07-22 16:06 ` [PATCH 02/12] rockchip: puma-rk3399: use gpio-hog instead of fixed-regulator for enabling eMMC/SPI-NOR Quentin Schulz
2022-09-01 13:04 ` Kever Yang
2022-07-22 16:06 ` [PATCH 03/12] rockchip: puma-rk3399: allow non-SD-Card-loaded SPL to load U-Boot proper from SD-Card Quentin Schulz
2022-09-01 13:04 ` Kever Yang
2022-07-22 16:06 ` [PATCH 04/12] rockchip: puma-rk3399: remove unused default ENV_OFFSET for SPI flashes Quentin Schulz
2022-09-01 13:04 ` Kever Yang
2022-07-22 16:06 ` [PATCH 05/12] rockchip: puma-rk3399: load environment from same MMC as used for loading U-Boot proper Quentin Schulz
2022-09-01 13:05 ` Kever Yang
2022-07-22 16:06 ` [PATCH 06/12] rockchip: puma-rk3399: allow loading environment from SPI-NOR flash Quentin Schulz
2022-09-01 13:05 ` Kever Yang
2022-07-22 16:06 ` [PATCH 07/12] rockchip: puma-rk3399: load environment from same medium as one used to load U-Boot proper Quentin Schulz
2022-09-01 13:03 ` Kever Yang
2022-09-01 13:13 ` Quentin Schulz
2022-09-04 11:49 ` Kever Yang
2022-09-06 9:22 ` Quentin Schulz [this message]
2022-09-14 9:17 ` patrick.delaunay
2022-09-20 12:28 ` Kever Yang
2022-09-20 14:02 ` Quentin Schulz
2022-07-22 16:06 ` [PATCH 08/12] rockchip: puma-rk3399: remove useless CONFIG_SYS_SPI_U_BOOT_OFFS Quentin Schulz
2022-09-01 13:05 ` Kever Yang
2022-07-22 16:06 ` [PATCH 09/12] rockchip: puma-rk3399: migrate to TPL Quentin Schulz
2022-09-01 13:05 ` Kever Yang
2022-09-04 11:52 ` Kever Yang
2022-07-22 16:06 ` [PATCH 10/12] rockchip: puma-rk3399: migrate to u-boot-rockchip.bin Quentin Schulz
2022-09-01 13:06 ` Kever Yang
2022-07-22 16:06 ` [PATCH 11/12] rockchip: lion-rk3368: " Quentin Schulz
2022-07-22 16:06 ` [PATCH 12/12] rockchip: puma-rk3399: migrate to u-boot-rockchip-spi.bin Quentin Schulz
2022-09-01 13:06 ` Kever Yang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4feb827e-3932-c8d6-120e-5465ad92eac2@theobroma-systems.com \
--to=quentin.schulz@theobroma-systems.com \
--cc=foss+uboot@0leil.net \
--cc=kever.yang@rock-chips.com \
--cc=klaus.goger@theobroma-systems.com \
--cc=knaerzche@gmail.com \
--cc=philipp.tomsich@vrull.eu \
--cc=sjg@chromium.org \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).