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

2023-10-05 Thread Fabio Estevam
On Thu, Oct 5, 2023 at 6:40 PM  wrote:
>
> From: Benjamin Szőke 
>
> Take over codes from Techenxion to support mmc autodetect boot for pico-imx7d.
>
> Signed-off-by: Benjamin Szőke 

Still not good:

WARNING: Possible unwrapped commit description (prefer a maximum 75
chars per line)
#57:
Take over codes from Techenxion to support mmc autodetect boot for pico-imx7d.

WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef'
where possible
#90: FILE: board/technexion/pico-imx7d/pico-imx7d.c:134:
+#if CONFIG_IS_ENABLED(FSL_ESDHC_IMX)

CHECK: Unnecessary parentheses around 'autodetect_str != NULL'
#95: FILE: board/technexion/pico-imx7d/pico-imx7d.c:139:
+ if ((autodetect_str != NULL) &&
+ (strcmp(autodetect_str, "yes") == 0)) {

CHECK: Comparison to NULL could be written "autodetect_str"
#95: FILE: board/technexion/pico-imx7d/pico-imx7d.c:139:
+ if ((autodetect_str != NULL) &&

CHECK: Alignment should match open parenthesis
#96: FILE: board/technexion/pico-imx7d/pico-imx7d.c:140:
+ if ((autodetect_str != NULL) &&
+ (strcmp(autodetect_str, "yes") == 0)) {

ERROR: switch and case should be at the same indent
#111: FILE: board/technexion/pico-imx7d/pico-imx7d.c:155:
+ switch (get_boot_device()) {
+ case SD3_BOOT:
+ case MMC3_BOOT:
[...]
+ case SD1_BOOT:
[...]
+ default:

ERROR: trailing whitespace
#127: FILE: board/technexion/pico-imx7d/pico-imx7d.c:171:
+^I$

WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef'
where possible
#140: FILE: board/technexion/pico-imx7d/pico-imx7d.c:224:
+#if CONFIG_IS_ENABLED(FSL_ESDHC_IMX)

WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef'
where possible
#141: FILE: board/technexion/pico-imx7d/pico-imx7d.c:225:
+#if defined(CONFIG_ENV_IS_IN_MMC) || defined(CONFIG_ENV_IS_NOWHERE)

WARNING: Move const after static - use 'static const iomux_v3_cfg_t'
#178: FILE: board/technexion/pico-imx7d/spl.c:165:
+static iomux_v3_cfg_t const usdhc1_pads[] = {

WARNING: Move const after static - use 'static const iomux_v3_cfg_t'
#189: FILE: board/technexion/pico-imx7d/spl.c:176:
+static iomux_v3_cfg_t const usdhc3_emmc_pads[] = {

CHECK: Lines should not end with a '('
#238: FILE: board/technexion/pico-imx7d/spl.c:225:
+ imx_iomux_v3_setup_multiple_pads(

CHECK: Lines should not end with a '('
#242: FILE: board/technexion/pico-imx7d/spl.c:229:
+ imx_iomux_v3_setup_multiple_pads(

ERROR: switch and case should be at the same indent
#246: FILE: board/technexion/pico-imx7d/spl.c:233:
+ switch (get_boot_device()) {
+ case SD1_BOOT:
[...]
+ case MMC3_BOOT:
[...]
+ case SD3_BOOT:
+ default:

total: 3 errors, 6 warnings, 5 checks, 220 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
  mechanically convert to the typical style using --fix or --fix-inplace.

NOTE: Whitespace errors detected.
  You may wish to use scripts/cleanpatch or scripts/cleanfile


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

2023-10-05 Thread egyszeregy
From: Benjamin Szőke 

Take over codes from Techenxion to support mmc autodetect boot for pico-imx7d.

Signed-off-by: Benjamin Szőke 
---
 board/technexion/pico-imx7d/pico-imx7d.c | 57 +++
 board/technexion/pico-imx7d/spl.c| 91 ++--
 include/configs/pico-imx7d.h |  4 +-
 3 files changed, 144 insertions(+), 8 deletions(-)

diff --git a/board/technexion/pico-imx7d/pico-imx7d.c 
b/board/technexion/pico-imx7d/pico-imx7d.c
index 6e98b85b28..a3fa915b49 100644
--- a/board/technexion/pico-imx7d/pico-imx7d.c
+++ b/board/technexion/pico-imx7d/pico-imx7d.c
@@ -5,6 +5,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -13,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -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 != NULL) &&
+   (strcmp(autodetect_str, "yes") == 0)) {
+   return 1;
+   }
+
+   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;
+   default:
+   printf("Wrong boot device!");
+   }
+
+   /* Set mmcdev env */
+   env_set_ulong("mmcdev", dev_no);
+   
+   sprintf(cmd, "mmc dev %d", dev_no);
+   run_command(cmd, 0);
+}
+#endif
+
 static void setup_iomux_uart(void)
 {
imx_iomux_v3_setup_multiple_pads(uart5_pads, ARRAY_SIZE(uart5_pads));
@@ -176,6 +221,12 @@ int board_late_init(void)
 
set_wdog_reset(wdog);
 
+#if CONFIG_IS_ENABLED(FSL_ESDHC_IMX)
+#if defined(CONFIG_ENV_IS_IN_MMC) || defined(CONFIG_ENV_IS_NOWHERE)
+   board_late_mmc_init();
+#endif /* CONFIG_ENV_IS_IN_MMC or CONFIG_ENV_IS_NOWHERE */
+#endif
+
/*
 * Do not assert internal WDOG_RESET_B_DEB(controlled by bit 4),
 * since we use PMIC_PWRON to reset the board.
@@ -210,3 +261,9 @@ int board_ehci_hcd_init(int port)
}
return 0;
 }
+
+/* This should be defined for each board */
+__weak int mmc_map_to_kernel_blk(int dev_no)
+{
+   return dev_no;
+}
diff --git a/board/technexion/pico-imx7d/spl.c 
b/board/technexion/pico-imx7d/spl.c
index c6b21aaa42..2fe76145be 100644
--- a/board/technexion/pico-imx7d/spl.c
+++ b/board/technexion/pico-imx7d/spl.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -159,7 +160,20 @@ void reset_cpu(void)
 #define USDHC_PAD_CTRL (PAD_CTL_DSE_3P3V_32OHM | PAD_CTL_SRE_SLOW | \
PAD_CTL_HYS | PAD_CTL_PUE | PAD_CTL_PUS_PU47KOHM)
 
-static iomux_v3_cfg_t const usdhc3_pads[] = {
+#define USDHC1_CD_GPIO IMX_GPIO_NR(5, 0)
+/* EMMC/SD */
+static iomux_v3_cfg_t const usdhc1_pads[] = {
+   MX7D_PAD_SD1_CLK__SD1_CLK | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+   MX7D_PAD_SD1_CMD__SD1_CMD | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+   MX7D_PAD_SD1_DATA0__SD1_DATA0 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+   MX7D_PAD_SD1_DATA1__SD1_DATA1 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+   MX7D_PAD_SD1_DATA2__SD1_DATA2 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+   MX7D_PAD_SD1_DATA3__SD1_DATA3 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+   MX7D_PAD_SD1_CD_B__GPIO5_IO0  | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+};
+
+#define USDHC3_CD_GPIO IMX_GPIO_NR(1, 14)
+static iomux_v3_cfg_t const usdhc3_emmc_pads[] = {
MX7D_PAD_SD3_CLK__SD3_CLK | MUX_PAD_CTRL(USDHC_PAD_CTRL),
MX7D_PAD_SD3_CMD__SD3_CMD | MUX_PAD_CTRL(USDHC_PAD_CTRL),
MX7D_PAD_SD3_DATA0__SD3_DATA0 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
@@ -173,20 +187,83 @@ static iomux_v3_cfg_t const usdhc3_pads[] = {
MX7D_PAD_GPIO1_IO14__GPIO1_IO14 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
 };
 
-static struct fsl_esdhc_cfg usdhc_cfg[1] = {
+static struct fsl_esdhc_cfg usdhc_cfg[2] = {
{USDHC3_BASE_ADDR},
+   {USDHC1_BASE_ADDR},
 };
 
 int board_mmc_getcd(struct mmc *mmc)
 {
-   /* Assume uSDHC3 emmc is always present */
-   return 1;
+   struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
+   int ret = 0;
+
+   switch (cfg->esdhc_base) {
+   case USDHC1_BASE_ADDR:
+   ret = !gpio_get_value(USDHC1_CD_GPIO); /* Assume uSDHC1 sd is 
always present */
+   break;
+   case USDHC3_BASE_ADDR:
+   ret = !gpio_get_value(USDHC3_CD_GPIO); /* Assume uSDHC3 emmc is 
always present */
+   break;
+   }
+
+   return ret;
 }
 
 int board_mmc_init(struct bd_info