[PATCH v2] rockchip: load env from boot MMC device

[Quentin Schulz]

Hi Ben,

On 3/8/24 04:00, Ben Wolsieffer wrote:
> [Some people who received this message don't often get email from benwolsieffer at 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 <benwolsieffer at gmail.com>

Minor (optional I believe) comments below but for the "logic" part:

Reviewed-by: Quentin Schulz <quentin.schulz at 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