[PATCH 1/1] [u-boot][master][PATCH v5] pico-imx7d: add baseboard SD card boot detect

[Fabio Estevam]

On Sat, Oct 21, 2023 at 9:40 AM <egyszeregy at freemail.hu> wrote:
>
> From: Benjamin Szőke <egyszeregy at freemail.hu>
>
> Technexion PICO-IMX7 SoM is supporting USDHC3 (eMMC or micro SD on SoM)
> and USDHC1 (SD on carrier board) to use on any carrier board like
> PICO-NYMPH. This pacth is intend to take over codes from Technexion
> to support mmc autodetect boot for pico-imx7d to able to boot from
> selected USDHC1 or USDHC3 boot devices.
>
> Signed-off-by: Benjamin Szőke <egyszeregy at freemail.hu>
> ---

Please always provide a changelog.

>  board/technexion/pico-imx7d/pico-imx7d.c | 59 ++++++++++++++-
>  board/technexion/pico-imx7d/spl.c        | 91 ++++++++++++++++++++++--
>  include/configs/pico-imx7d.h             |  4 +-
>  3 files changed, 145 insertions(+), 9 deletions(-)
>
> diff --git a/board/technexion/pico-imx7d/pico-imx7d.c b/board/technexion/pico-imx7d/pico-imx7d.c
> index 6e98b85b28..01a9520d32 100644
> --- a/board/technexion/pico-imx7d/pico-imx7d.c
> +++ b/board/technexion/pico-imx7d/pico-imx7d.c
> @@ -5,6 +5,7 @@
>
>  #include <init.h>
>  #include <net.h>
> +#include <command.h>
>  #include <asm/arch/clock.h>
>  #include <asm/arch/crm_regs.h>
>  #include <asm/arch/imx-regs.h>
> @@ -13,6 +14,7 @@
>  #include <asm/global_data.h>
>  #include <asm/gpio.h>
>  #include <asm/mach-imx/iomux-v3.h>
> +#include <asm/mach-imx/boot_mode.h>
>  #include <asm/io.h>
>  #include <common.h>
>  #include <miiphy.h>
> @@ -106,7 +108,7 @@ int board_phy_config(struct phy_device *phydev)
>  {
>         unsigned short val;
>
> -       /* To enable AR8035 ouput a 125MHz clk from CLK_25M */
> +       /* To enable AR8035 output a 125MHz clk from CLK_25M */

This is an unrelated change.

>         phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x7);
>         phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016);
>         phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007);
> @@ -129,6 +131,49 @@ int board_phy_config(struct phy_device *phydev)
>  }
>  #endif
>
> +#if CONFIG_IS_ENABLED(FSL_ESDHC_IMX)
> +int check_mmc_autodetect(void)
> +{
> +       char *autodetect_str = env_get("mmcautodetect");
> +
> +       if ((autodetect_str) &&
> +           (strcmp(autodetect_str, "yes") == 0)) {
> +               return 1;
> +       }

No need for { to enclose a single line statement. Please remove it.


> +
> +       return 0;
> +}
> +
> +void board_late_mmc_init(void)
> +{
> +       int dev_no = 0;
> +       char cmd[32];
> +
> +       if (!check_mmc_autodetect())
> +               return;
> +
> +       switch (get_boot_device()) {
> +       case SD3_BOOT:
> +       case MMC3_BOOT:
> +               env_set("bootdev", "MMC3");
> +               dev_no = 2;
> +               break;
> +       case SD1_BOOT:
> +               env_set("bootdev", "SD1");
> +               dev_no = 0;
> +               break;

These dev_no assignments are not correct.

imx7d-pico-pi-u-boot.dtsi defines aliases for the eMMC:

    aliases {
        mmc0 = &usdhc3;

So to avoid errors, I had to apply the following change on top o your patch:

--- a/board/technexion/pico-imx7d/pico-imx7d.c
+++ b/board/technexion/pico-imx7d/pico-imx7d.c
@@ -156,11 +156,11 @@ void board_late_mmc_init(void)
        case SD3_BOOT:
        case MMC3_BOOT:
                env_set("bootdev", "MMC3");
-               dev_no = 2;
+               dev_no = 0;
                break;
        case SD1_BOOT:
                env_set("bootdev", "SD1");
-               dev_no = 0;
+               dev_no = 1;
                break;
        default:
                printf("Wrong boot device!");

> @@ -13,7 +13,7 @@
>  #define CFG_MXC_UART_BASE              UART5_IPS_BASE_ADDR
>
>  /* MMC Config */
> -#define CFG_SYS_FSL_ESDHC_ADDR 0
> +#define CFG_SYS_FSL_ESDHC_ADDR USDHC3_BASE_ADDR

Is this change needed?

>  #define CFG_DFU_ENV_SETTINGS \
>         "dfu_alt_info=" \
> @@ -79,9 +79,11 @@
>                 "name=rootfs,size=0,uuid=${uuid_gpt_rootfs}\0" \
>         "fastboot_partition_alias_system=rootfs\0" \
>         "setup_emmc=mmc dev 0; gpt write mmc 0 $partitions; reset;\0" \
> +       "mmcautodetect=yes\0" \
>         PICO_BOOT_ENV
>
>  #define BOOT_TARGET_DEVICES(func) \
> +       func(MMC, mmc, 2) \

This should be func(MMC, mmc, 1)

Please define aliases if needed.