Hi Ben,
On 3/8/24 04:00, Ben Wolsieffer wrote:
[Some people who received this message don't often get email from
benwolsief...@gmail.com. Learn why this is important at
https://aka.ms/LearnAboutSenderIdentification ]
Currently, if the environment is stored on an MMC device, the device
number is hardcoded by CONFIG_SYS_MMC_ENV_DEV. This is problematic
because many boards can choose between booting from an SD card or a
removable eMMC. For example, the Rock64 defconfig sets
CONFIG_SYS_MMC_ENV_DEV=1, which corresponds to the SD card. If an eMMC
is used as the boot device and no SD card is installed, it is impossible
to save the environment.
To avoid this problem, we can choose the environment MMC device based on
the boot device. The theobroma-systems boards already contain code to do
this, so this commit simply moves it to the common Rockchip board file,
with some refactoring. I also removed another implementation of
mmc_get_env_dev() from tinker_rk3288 that performed MMC boot device
detection by reading a bootrom register.
This has been tested on a Rock64v2.
Signed-off-by: Ben Wolsieffer <benwolsief...@gmail.com>
Minor (optional I believe) comments below but for the "logic" part:
Reviewed-by: Quentin Schulz <quentin.sch...@theobroma-systems.com>
---
v2:
* Use #ifdef rather than if(CONFIG_IS_ENABLED(...))
* Add a debug message if boot device is not found
arch/arm/mach-rockchip/board.c | 31 ++++++++++++++++++++
board/rockchip/tinker_rk3288/tinker-rk3288.c | 12 --------
board/theobroma-systems/common/common.c | 30 -------------------
3 files changed, 31 insertions(+), 42 deletions(-)
diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
index 2620530e03..8aa09f44b3 100644
--- a/arch/arm/mach-rockchip/board.c
+++ b/arch/arm/mach-rockchip/board.c
@@ -6,6 +6,7 @@
#include <clk.h>
#include <cpu_func.h>
#include <dm.h>
+#include <dm/uclass-internal.h>
c.f.
https://docs.u-boot.org/en/latest/develop/codingstyle.html#include-files
"""
In all cases, they should be listed in alphabetical order. First comes
headers which are located directly in our top-level include diretory.
This excludes the common.h header file which is to be removed. Second
are headers within subdirectories, Finally directory-local includes
should be listed.
"""
So it should be after
asm/arch-rockchip/misc.h
and before
power/regulator.h
#include <efi_loader.h>
#include <fastboot.h>
#include <init.h>
@@ -349,3 +350,33 @@ __weak int board_rng_seed(struct abuf *buf)
return 0;
}
#endif
+
+int mmc_get_env_dev(void)
+{
+ int devnum;
+ const char *boot_device;
+ struct udevice *dev;
+
This rule isn't written in the coding style I think, but some kernel
people like the reverse Christmas tree order.
So, here it'd be:
"""
+ const char *boot_device;
+ struct udevice *dev;
+ int devnum;
"""
Cheers,
Quentin
