[PATCH 12/17] rockchip: odroid-go2: Use env from same storage FIT was loaded from

[Jonas Karlman]

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 <jonas at 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.schulz at cherry.de>
> 
> Thanks!
> Quentin