Hi Quentin,
On 7/15/2025 11:46 AM, Quentin Schulz wrote:
> Hi Jonas,
>
> On 7/14/25 1:34 AM, Jonas Karlman wrote:
>> Change to dynamically select what storage media to use for the U-Boot
>> environment depending on from what storage media the FIT images was
>> loaded from, fall back to use env from nowhere.
>>
>> U-Boot SPL 2025.07 (Jul 13 2025 - 10:07:16 +0000)
>> Trying to boot from MMC1
>> ...
>> Loading Environment from MMC... Reading from MMC(0)...
>>
>> or
>>
>> U-Boot SPL 2025.07 (Jul 13 2025 - 10:07:16 +0000)
>> Trying to boot from SPI
>> ...
>> Loading Environment from SPIFlash...
>>
>> Signed-off-by: Jonas Karlman <jo...@kwiboo.se>
>> ---
>> board/hardkernel/odroid_go2/Kconfig | 1 +
>> board/hardkernel/odroid_go2/go2.c | 34 +++++++++++++++++++++++++++++
>> configs/odroid-go2_defconfig | 2 ++
>> 3 files changed, 37 insertions(+)
>>
>> diff --git a/board/hardkernel/odroid_go2/Kconfig
>> b/board/hardkernel/odroid_go2/Kconfig
>> index dd6e366282e3..6487335972b8 100644
>> --- a/board/hardkernel/odroid_go2/Kconfig
>> +++ b/board/hardkernel/odroid_go2/Kconfig
>> @@ -13,6 +13,7 @@ config BOARD_SPECIFIC_OPTIONS # dummy
>> def_bool y
>> select ADC
>> select BOARD_TYPES
>> + select ENV_IS_NOWHERE
>> select SPL_ADC
>>
>> endif
>> diff --git a/board/hardkernel/odroid_go2/go2.c
>> b/board/hardkernel/odroid_go2/go2.c
>> index 9d9f3cee36a5..ae32ea87af88 100644
>> --- a/board/hardkernel/odroid_go2/go2.c
>> +++ b/board/hardkernel/odroid_go2/go2.c
>> @@ -7,7 +7,9 @@
>> #include <adc.h>
>> #include <asm/io.h>
>> #include <dm.h>
>> +#include <dm/uclass-internal.h>
>> #include <env.h>
>> +#include <env_internal.h>
>> #include <stdlib.h>
>>
>> DECLARE_GLOBAL_DATA_PTR;
>> @@ -129,3 +131,35 @@ int board_fit_config_name_match(const char *name)
>>
>> return -EINVAL;
>> }
>> +
>> +enum env_location env_get_location(enum env_operation op, int prio)
>> +{
>> + const char *boot_device;
>> + struct udevice *dev;
>> + ofnode node;
>> +
>> + if (prio)
>> + return ENVL_UNKNOWN;
>> +
>> + boot_device = ofnode_read_chosen_string("u-boot,spl-boot-device");
>> + 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);
>> +
>> + node = ofnode_path(boot_device);
>> + if (!ofnode_valid(node))
>> + return ENVL_NOWHERE;
>> +
>> + if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH) &&
>> + !uclass_find_device_by_ofnode(UCLASS_SPI_FLASH, node, &dev))
>> + return ENVL_SPI_FLASH;
>> +
>> + if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC) &&
>> + !uclass_find_device_by_ofnode(UCLASS_MMC, node, &dev))
>> + return ENVL_MMC;
>> +
>> + return ENVL_NOWHERE;
>> +}
>
> This is virtually identical to the implementation in
> board/theobroma-systems/common/common.c and I think it's quite a
> reasonable approach (though I'm the one who implemented it so I may be
> slightly biased :) ).
Hehe, your arch_env_get_location() and the common mmc_get_env_dev() has
been a huge inspiration to this ;-)
And I fully agree this to be a reasonable approach.
>
> Should we somehow make it available in a header so it's easy/easier to
> implement the same for other boards? E.g. env_get_location() could
> simply call env_get_location_same_as_proper()? What do you think?
Fully agree to something like that, and/or adding a special Kconfig
option that enable a similar shared arch_env_get_location().
Something for a future series :-)
Regards,
Jonas
>
> Not necessary for this patch to be merged though, so
>
> Reviewed-by: Quentin Schulz <quentin.sch...@cherry.de>
>
> Thanks!
> Quentin
