All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function"
@ 2018-02-15 12:14 Fabio Estevam
  2018-02-15 14:18 ` Stefano Babic
  2018-02-15 14:33 ` Lukasz Majewski
  0 siblings, 2 replies; 12+ messages in thread
From: Fabio Estevam @ 2018-02-15 12:14 UTC (permalink / raw)
  To: u-boot

This reverts commit d695d6627803dbb78a226e04b0436a01633a9936.

Commit d695d6627803 ("spl: eMMC/SD: Provide one __weak spl_boot_mode()
function") breaks the boot on several i.MX6 boards,
such as cuboxi and wandboard:

U-Boot SPL 2018.03-rc1-00212-g48914fc119 (Feb 10 2018 - 11:04:33 +1300)
Trying to boot from MMC1
Failed to mount ext2 filesystem...
spl_load_image_ext: ext4fs mount err - 0

Revert it so that we can boot U-Boot again.

Reported-by: Jonathan Gray <jsg@jsg.id.au>
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
Changes since v1:
- Fix typo in commit log: "breaks the boot" instead of "breaks the build"

 arch/arm/cpu/arm1136/mx35/generic.c       | 21 +++++++++++++++++++++
 arch/arm/cpu/armv7/ls102xa/spl.c          | 17 +++++++++++++++++
 arch/arm/cpu/armv8/fsl-layerscape/spl.c   | 17 +++++++++++++++++
 arch/arm/cpu/armv8/zynqmp/spl.c           | 14 ++++++++++++++
 arch/arm/mach-at91/spl.c                  | 15 +++++++++++++++
 arch/arm/mach-davinci/spl.c               |  5 +++++
 arch/arm/mach-imx/spl.c                   | 23 +++++++++++++++++++++++
 arch/arm/mach-mvebu/spl.c                 |  7 +++++++
 arch/arm/mach-rockchip/rk3188-board-spl.c |  5 +++++
 arch/arm/mach-rockchip/rk3288-board-spl.c |  5 +++++
 arch/arm/mach-rockchip/rk3368-board-spl.c |  5 +++++
 arch/arm/mach-rockchip/rk3399-board-spl.c |  5 +++++
 arch/arm/mach-socfpga/spl.c               | 11 +++++++++++
 arch/arm/mach-sunxi/board.c               |  6 ++++++
 arch/arm/mach-zynq/spl.c                  |  7 +++++++
 common/spl/spl_mmc.c                      | 11 -----------
 16 files changed, 163 insertions(+), 11 deletions(-)

diff --git a/arch/arm/cpu/arm1136/mx35/generic.c b/arch/arm/cpu/arm1136/mx35/generic.c
index 4dcfc72..5297d62 100644
--- a/arch/arm/cpu/arm1136/mx35/generic.c
+++ b/arch/arm/cpu/arm1136/mx35/generic.c
@@ -524,3 +524,24 @@ u32 spl_boot_device(void)
 
 	return BOOT_DEVICE_NONE;
 }
+
+#ifdef CONFIG_SPL_BUILD
+u32 spl_boot_mode(const u32 boot_device)
+{
+	switch (spl_boot_device()) {
+	case BOOT_DEVICE_MMC1:
+#ifdef CONFIG_SPL_FAT_SUPPORT
+		return MMCSD_MODE_FS;
+#else
+		return MMCSD_MODE_RAW;
+#endif
+		break;
+	case BOOT_DEVICE_NAND:
+		return 0;
+		break;
+	default:
+		puts("spl: ERROR:  unsupported device\n");
+		hang();
+	}
+}
+#endif
diff --git a/arch/arm/cpu/armv7/ls102xa/spl.c b/arch/arm/cpu/armv7/ls102xa/spl.c
index 1e4a164..1246eed 100644
--- a/arch/arm/cpu/armv7/ls102xa/spl.c
+++ b/arch/arm/cpu/armv7/ls102xa/spl.c
@@ -14,3 +14,20 @@ u32 spl_boot_device(void)
 #endif
 	return BOOT_DEVICE_NAND;
 }
+
+u32 spl_boot_mode(const u32 boot_device)
+{
+	switch (spl_boot_device()) {
+	case BOOT_DEVICE_MMC1:
+#ifdef CONFIG_SPL_FAT_SUPPORT
+		return MMCSD_MODE_FS;
+#else
+		return MMCSD_MODE_RAW;
+#endif
+	case BOOT_DEVICE_NAND:
+		return 0;
+	default:
+		puts("spl: error: unsupported device\n");
+		hang();
+	}
+}
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/spl.c b/arch/arm/cpu/armv8/fsl-layerscape/spl.c
index 3a74040..4093d15 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/spl.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/spl.c
@@ -26,6 +26,23 @@ u32 spl_boot_device(void)
 	return 0;
 }
 
+u32 spl_boot_mode(const u32 boot_device)
+{
+	switch (spl_boot_device()) {
+	case BOOT_DEVICE_MMC1:
+#ifdef CONFIG_SPL_FAT_SUPPORT
+		return MMCSD_MODE_FS;
+#else
+		return MMCSD_MODE_RAW;
+#endif
+	case BOOT_DEVICE_NAND:
+		return 0;
+	default:
+		puts("spl: error: unsupported device\n");
+		hang();
+	}
+}
+
 #ifdef CONFIG_SPL_BUILD
 
 void spl_board_init(void)
diff --git a/arch/arm/cpu/armv8/zynqmp/spl.c b/arch/arm/cpu/armv8/zynqmp/spl.c
index 0bfa5c1..bc7313a 100644
--- a/arch/arm/cpu/armv8/zynqmp/spl.c
+++ b/arch/arm/cpu/armv8/zynqmp/spl.c
@@ -115,6 +115,20 @@ u32 spl_boot_device(void)
 	return 0;
 }
 
+u32 spl_boot_mode(const u32 boot_device)
+{
+	switch (boot_device) {
+	case BOOT_DEVICE_RAM:
+		return 0;
+	case BOOT_DEVICE_MMC1:
+	case BOOT_DEVICE_MMC2:
+		return MMCSD_MODE_FS;
+	default:
+		puts("spl: error: unsupported device\n");
+		hang();
+	}
+}
+
 #ifdef CONFIG_SPL_OS_BOOT
 int spl_start_uboot(void)
 {
diff --git a/arch/arm/mach-at91/spl.c b/arch/arm/mach-at91/spl.c
index 91add92..7e7e24b 100644
--- a/arch/arm/mach-at91/spl.c
+++ b/arch/arm/mach-at91/spl.c
@@ -87,3 +87,18 @@ u32 spl_boot_device(void)
 	return BOOT_DEVICE_NONE;
 }
 #endif
+
+u32 spl_boot_mode(const u32 boot_device)
+{
+	switch (boot_device) {
+#if defined(CONFIG_SYS_USE_MMC) || defined(CONFIG_SD_BOOT)
+	case BOOT_DEVICE_MMC1:
+	case BOOT_DEVICE_MMC2:
+		return MMCSD_MODE_FS;
+		break;
+#endif
+	case BOOT_DEVICE_NONE:
+	default:
+		hang();
+	}
+}
diff --git a/arch/arm/mach-davinci/spl.c b/arch/arm/mach-davinci/spl.c
index 4c74db9..564c200 100644
--- a/arch/arm/mach-davinci/spl.c
+++ b/arch/arm/mach-davinci/spl.c
@@ -45,6 +45,11 @@ void spl_board_init(void)
 	preloader_console_init();
 }
 
+u32 spl_boot_mode(const u32 boot_device)
+{
+	return MMCSD_MODE_RAW;
+}
+
 u32 spl_boot_device(void)
 {
 	switch (davinci_syscfg_regs->bootcfg) {
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index a9079fc..b2521b2 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -132,6 +132,29 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name)
 }
 #endif
 
+#if defined(CONFIG_SPL_MMC_SUPPORT)
+/* called from spl_mmc to see type of boot mode for storage (RAW or FAT) */
+u32 spl_boot_mode(const u32 boot_device)
+{
+	switch (spl_boot_device()) {
+	/* for MMC return either RAW or FAT mode */
+	case BOOT_DEVICE_MMC1:
+	case BOOT_DEVICE_MMC2:
+#if defined(CONFIG_SPL_FAT_SUPPORT)
+		return MMCSD_MODE_FS;
+#elif defined(CONFIG_SUPPORT_EMMC_BOOT)
+		return MMCSD_MODE_EMMCBOOT;
+#else
+		return MMCSD_MODE_RAW;
+#endif
+		break;
+	default:
+		puts("spl: ERROR:  unsupported device\n");
+		hang();
+	}
+}
+#endif
+
 #if defined(CONFIG_SECURE_BOOT)
 
 /*
diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index a5086f1..d16a62d 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -70,6 +70,13 @@ u32 spl_boot_device(void)
 	return get_boot_device();
 }
 
+#ifdef CONFIG_SPL_MMC_SUPPORT
+u32 spl_boot_mode(const u32 boot_device)
+{
+	return MMCSD_MODE_RAW;
+}
+#endif
+
 void board_init_f(ulong dummy)
 {
 	int ret;
diff --git a/arch/arm/mach-rockchip/rk3188-board-spl.c b/arch/arm/mach-rockchip/rk3188-board-spl.c
index 74771d3..8e3b8ae 100644
--- a/arch/arm/mach-rockchip/rk3188-board-spl.c
+++ b/arch/arm/mach-rockchip/rk3188-board-spl.c
@@ -72,6 +72,11 @@ fallback:
 	return BOOT_DEVICE_MMC1;
 }
 
+u32 spl_boot_mode(const u32 boot_device)
+{
+	return MMCSD_MODE_RAW;
+}
+
 static int setup_arm_clock(void)
 {
 	struct udevice *dev;
diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c b/arch/arm/mach-rockchip/rk3288-board-spl.c
index f3ea624..f64a548 100644
--- a/arch/arm/mach-rockchip/rk3288-board-spl.c
+++ b/arch/arm/mach-rockchip/rk3288-board-spl.c
@@ -78,6 +78,11 @@ fallback:
 	return BOOT_DEVICE_MMC1;
 }
 
+u32 spl_boot_mode(const u32 boot_device)
+{
+	return MMCSD_MODE_RAW;
+}
+
 #ifdef CONFIG_SPL_MMC_SUPPORT
 static int configure_emmc(struct udevice *pinctrl)
 {
diff --git a/arch/arm/mach-rockchip/rk3368-board-spl.c b/arch/arm/mach-rockchip/rk3368-board-spl.c
index 8055ae5..72d2c97 100644
--- a/arch/arm/mach-rockchip/rk3368-board-spl.c
+++ b/arch/arm/mach-rockchip/rk3368-board-spl.c
@@ -57,6 +57,11 @@ void board_init_f(ulong dummy)
 	}
 }
 
+u32 spl_boot_mode(const u32 boot_device)
+{
+	return MMCSD_MODE_RAW;
+}
+
 u32 spl_boot_device(void)
 {
 	return BOOT_DEVICE_MMC1;
diff --git a/arch/arm/mach-rockchip/rk3399-board-spl.c b/arch/arm/mach-rockchip/rk3399-board-spl.c
index d35990e..b96903e 100644
--- a/arch/arm/mach-rockchip/rk3399-board-spl.c
+++ b/arch/arm/mach-rockchip/rk3399-board-spl.c
@@ -60,6 +60,11 @@ u32 spl_boot_device(void)
 	return boot_device;
 }
 
+u32 spl_boot_mode(const u32 boot_device)
+{
+	return MMCSD_MODE_RAW;
+}
+
 #define TIMER_CHN10_BASE	0xff8680a0
 #define TIMER_END_COUNT_L	0x00
 #define TIMER_END_COUNT_H	0x04
diff --git a/arch/arm/mach-socfpga/spl.c b/arch/arm/mach-socfpga/spl.c
index 9bf3b9a..71bae82 100644
--- a/arch/arm/mach-socfpga/spl.c
+++ b/arch/arm/mach-socfpga/spl.c
@@ -66,6 +66,17 @@ u32 spl_boot_device(void)
 	}
 }
 
+#ifdef CONFIG_SPL_MMC_SUPPORT
+u32 spl_boot_mode(const u32 boot_device)
+{
+#if defined(CONFIG_SPL_FAT_SUPPORT) || defined(CONFIG_SPL_EXT_SUPPORT)
+	return MMCSD_MODE_FS;
+#else
+	return MMCSD_MODE_RAW;
+#endif
+}
+#endif
+
 #if defined(CONFIG_TARGET_SOCFPGA_GEN5)
 static void socfpga_nic301_slave_ns(void)
 {
diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
index 1753fae..0c60ee0 100644
--- a/arch/arm/mach-sunxi/board.c
+++ b/arch/arm/mach-sunxi/board.c
@@ -260,6 +260,12 @@ u32 spl_boot_device(void)
 	return sunxi_get_boot_device();
 }
 
+/* No confirmation data available in SPL yet. Hardcode bootmode */
+u32 spl_boot_mode(const u32 boot_device)
+{
+	return MMCSD_MODE_RAW;
+}
+
 void board_init_f(ulong dummy)
 {
 	spl_init();
diff --git a/arch/arm/mach-zynq/spl.c b/arch/arm/mach-zynq/spl.c
index 0a303f4..b7e6d98 100644
--- a/arch/arm/mach-zynq/spl.c
+++ b/arch/arm/mach-zynq/spl.c
@@ -69,6 +69,13 @@ u32 spl_boot_device(void)
 	return mode;
 }
 
+#ifdef CONFIG_SPL_MMC_SUPPORT
+u32 spl_boot_mode(const u32 boot_device)
+{
+	return MMCSD_MODE_FS;
+}
+#endif
+
 #ifdef CONFIG_SPL_OS_BOOT
 int spl_start_uboot(void)
 {
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index 351f4ed..b57e0b0 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -281,17 +281,6 @@ static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image, struct mmc *mmc)
 }
 #endif
 
-u32 __weak spl_boot_mode(const u32 boot_device)
-{
-#if defined(CONFIG_SPL_FAT_SUPPORT) || defined(CONFIG_SPL_EXT_SUPPORT)
-	return MMCSD_MODE_FS;
-#elif defined(CONFIG_SUPPORT_EMMC_BOOT)
-	return MMCSD_MODE_EMMCBOOT;
-#else
-	return MMCSD_MODE_RAW;
-#endif
-}
-
 int spl_mmc_load_image(struct spl_image_info *spl_image,
 		       struct spl_boot_device *bootdev)
 {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function"
  2018-02-15 12:14 [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function" Fabio Estevam
@ 2018-02-15 14:18 ` Stefano Babic
  2018-02-15 14:41   ` Lukasz Majewski
  2018-02-15 14:33 ` Lukasz Majewski
  1 sibling, 1 reply; 12+ messages in thread
From: Stefano Babic @ 2018-02-15 14:18 UTC (permalink / raw)
  To: u-boot

Hi Heiko,

On 15/02/2018 13:14, Fabio Estevam wrote:
> This reverts commit d695d6627803dbb78a226e04b0436a01633a9936.
> 
> Commit d695d6627803 ("spl: eMMC/SD: Provide one __weak spl_boot_mode()
> function") breaks the boot on several i.MX6 boards,
> such as cuboxi and wandboard:
> 
> U-Boot SPL 2018.03-rc1-00212-g48914fc119 (Feb 10 2018 - 11:04:33 +1300)
> Trying to boot from MMC1
> Failed to mount ext2 filesystem...
> spl_load_image_ext: ext4fs mount err - 0
> 
> Revert it so that we can boot U-Boot again.
> 

Can this the cause of the issue on the wandboard you told me yesterday ?
Wasn't it due to CONFIG_SPL_EXT_SUPPORT ?

Just to collect and understand myself if issues are connected...

Best regards,
Stefano

> Reported-by: Jonathan Gray <jsg@jsg.id.au>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> Changes since v1:
> - Fix typo in commit log: "breaks the boot" instead of "breaks the build"
> 
>  arch/arm/cpu/arm1136/mx35/generic.c       | 21 +++++++++++++++++++++
>  arch/arm/cpu/armv7/ls102xa/spl.c          | 17 +++++++++++++++++
>  arch/arm/cpu/armv8/fsl-layerscape/spl.c   | 17 +++++++++++++++++
>  arch/arm/cpu/armv8/zynqmp/spl.c           | 14 ++++++++++++++
>  arch/arm/mach-at91/spl.c                  | 15 +++++++++++++++
>  arch/arm/mach-davinci/spl.c               |  5 +++++
>  arch/arm/mach-imx/spl.c                   | 23 +++++++++++++++++++++++
>  arch/arm/mach-mvebu/spl.c                 |  7 +++++++
>  arch/arm/mach-rockchip/rk3188-board-spl.c |  5 +++++
>  arch/arm/mach-rockchip/rk3288-board-spl.c |  5 +++++
>  arch/arm/mach-rockchip/rk3368-board-spl.c |  5 +++++
>  arch/arm/mach-rockchip/rk3399-board-spl.c |  5 +++++
>  arch/arm/mach-socfpga/spl.c               | 11 +++++++++++
>  arch/arm/mach-sunxi/board.c               |  6 ++++++
>  arch/arm/mach-zynq/spl.c                  |  7 +++++++
>  common/spl/spl_mmc.c                      | 11 -----------
>  16 files changed, 163 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/cpu/arm1136/mx35/generic.c b/arch/arm/cpu/arm1136/mx35/generic.c
> index 4dcfc72..5297d62 100644
> --- a/arch/arm/cpu/arm1136/mx35/generic.c
> +++ b/arch/arm/cpu/arm1136/mx35/generic.c
> @@ -524,3 +524,24 @@ u32 spl_boot_device(void)
>  
>  	return BOOT_DEVICE_NONE;
>  }
> +
> +#ifdef CONFIG_SPL_BUILD
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	switch (spl_boot_device()) {
> +	case BOOT_DEVICE_MMC1:
> +#ifdef CONFIG_SPL_FAT_SUPPORT
> +		return MMCSD_MODE_FS;
> +#else
> +		return MMCSD_MODE_RAW;
> +#endif
> +		break;
> +	case BOOT_DEVICE_NAND:
> +		return 0;
> +		break;
> +	default:
> +		puts("spl: ERROR:  unsupported device\n");
> +		hang();
> +	}
> +}
> +#endif
> diff --git a/arch/arm/cpu/armv7/ls102xa/spl.c b/arch/arm/cpu/armv7/ls102xa/spl.c
> index 1e4a164..1246eed 100644
> --- a/arch/arm/cpu/armv7/ls102xa/spl.c
> +++ b/arch/arm/cpu/armv7/ls102xa/spl.c
> @@ -14,3 +14,20 @@ u32 spl_boot_device(void)
>  #endif
>  	return BOOT_DEVICE_NAND;
>  }
> +
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	switch (spl_boot_device()) {
> +	case BOOT_DEVICE_MMC1:
> +#ifdef CONFIG_SPL_FAT_SUPPORT
> +		return MMCSD_MODE_FS;
> +#else
> +		return MMCSD_MODE_RAW;
> +#endif
> +	case BOOT_DEVICE_NAND:
> +		return 0;
> +	default:
> +		puts("spl: error: unsupported device\n");
> +		hang();
> +	}
> +}
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/spl.c b/arch/arm/cpu/armv8/fsl-layerscape/spl.c
> index 3a74040..4093d15 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/spl.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/spl.c
> @@ -26,6 +26,23 @@ u32 spl_boot_device(void)
>  	return 0;
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	switch (spl_boot_device()) {
> +	case BOOT_DEVICE_MMC1:
> +#ifdef CONFIG_SPL_FAT_SUPPORT
> +		return MMCSD_MODE_FS;
> +#else
> +		return MMCSD_MODE_RAW;
> +#endif
> +	case BOOT_DEVICE_NAND:
> +		return 0;
> +	default:
> +		puts("spl: error: unsupported device\n");
> +		hang();
> +	}
> +}
> +
>  #ifdef CONFIG_SPL_BUILD
>  
>  void spl_board_init(void)
> diff --git a/arch/arm/cpu/armv8/zynqmp/spl.c b/arch/arm/cpu/armv8/zynqmp/spl.c
> index 0bfa5c1..bc7313a 100644
> --- a/arch/arm/cpu/armv8/zynqmp/spl.c
> +++ b/arch/arm/cpu/armv8/zynqmp/spl.c
> @@ -115,6 +115,20 @@ u32 spl_boot_device(void)
>  	return 0;
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	switch (boot_device) {
> +	case BOOT_DEVICE_RAM:
> +		return 0;
> +	case BOOT_DEVICE_MMC1:
> +	case BOOT_DEVICE_MMC2:
> +		return MMCSD_MODE_FS;
> +	default:
> +		puts("spl: error: unsupported device\n");
> +		hang();
> +	}
> +}
> +
>  #ifdef CONFIG_SPL_OS_BOOT
>  int spl_start_uboot(void)
>  {
> diff --git a/arch/arm/mach-at91/spl.c b/arch/arm/mach-at91/spl.c
> index 91add92..7e7e24b 100644
> --- a/arch/arm/mach-at91/spl.c
> +++ b/arch/arm/mach-at91/spl.c
> @@ -87,3 +87,18 @@ u32 spl_boot_device(void)
>  	return BOOT_DEVICE_NONE;
>  }
>  #endif
> +
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	switch (boot_device) {
> +#if defined(CONFIG_SYS_USE_MMC) || defined(CONFIG_SD_BOOT)
> +	case BOOT_DEVICE_MMC1:
> +	case BOOT_DEVICE_MMC2:
> +		return MMCSD_MODE_FS;
> +		break;
> +#endif
> +	case BOOT_DEVICE_NONE:
> +	default:
> +		hang();
> +	}
> +}
> diff --git a/arch/arm/mach-davinci/spl.c b/arch/arm/mach-davinci/spl.c
> index 4c74db9..564c200 100644
> --- a/arch/arm/mach-davinci/spl.c
> +++ b/arch/arm/mach-davinci/spl.c
> @@ -45,6 +45,11 @@ void spl_board_init(void)
>  	preloader_console_init();
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +
>  u32 spl_boot_device(void)
>  {
>  	switch (davinci_syscfg_regs->bootcfg) {
> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
> index a9079fc..b2521b2 100644
> --- a/arch/arm/mach-imx/spl.c
> +++ b/arch/arm/mach-imx/spl.c
> @@ -132,6 +132,29 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name)
>  }
>  #endif
>  
> +#if defined(CONFIG_SPL_MMC_SUPPORT)
> +/* called from spl_mmc to see type of boot mode for storage (RAW or FAT) */
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	switch (spl_boot_device()) {
> +	/* for MMC return either RAW or FAT mode */
> +	case BOOT_DEVICE_MMC1:
> +	case BOOT_DEVICE_MMC2:
> +#if defined(CONFIG_SPL_FAT_SUPPORT)
> +		return MMCSD_MODE_FS;
> +#elif defined(CONFIG_SUPPORT_EMMC_BOOT)
> +		return MMCSD_MODE_EMMCBOOT;
> +#else
> +		return MMCSD_MODE_RAW;
> +#endif
> +		break;
> +	default:
> +		puts("spl: ERROR:  unsupported device\n");
> +		hang();
> +	}
> +}
> +#endif
> +
>  #if defined(CONFIG_SECURE_BOOT)
>  
>  /*
> diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
> index a5086f1..d16a62d 100644
> --- a/arch/arm/mach-mvebu/spl.c
> +++ b/arch/arm/mach-mvebu/spl.c
> @@ -70,6 +70,13 @@ u32 spl_boot_device(void)
>  	return get_boot_device();
>  }
>  
> +#ifdef CONFIG_SPL_MMC_SUPPORT
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +#endif
> +
>  void board_init_f(ulong dummy)
>  {
>  	int ret;
> diff --git a/arch/arm/mach-rockchip/rk3188-board-spl.c b/arch/arm/mach-rockchip/rk3188-board-spl.c
> index 74771d3..8e3b8ae 100644
> --- a/arch/arm/mach-rockchip/rk3188-board-spl.c
> +++ b/arch/arm/mach-rockchip/rk3188-board-spl.c
> @@ -72,6 +72,11 @@ fallback:
>  	return BOOT_DEVICE_MMC1;
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +
>  static int setup_arm_clock(void)
>  {
>  	struct udevice *dev;
> diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c b/arch/arm/mach-rockchip/rk3288-board-spl.c
> index f3ea624..f64a548 100644
> --- a/arch/arm/mach-rockchip/rk3288-board-spl.c
> +++ b/arch/arm/mach-rockchip/rk3288-board-spl.c
> @@ -78,6 +78,11 @@ fallback:
>  	return BOOT_DEVICE_MMC1;
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +
>  #ifdef CONFIG_SPL_MMC_SUPPORT
>  static int configure_emmc(struct udevice *pinctrl)
>  {
> diff --git a/arch/arm/mach-rockchip/rk3368-board-spl.c b/arch/arm/mach-rockchip/rk3368-board-spl.c
> index 8055ae5..72d2c97 100644
> --- a/arch/arm/mach-rockchip/rk3368-board-spl.c
> +++ b/arch/arm/mach-rockchip/rk3368-board-spl.c
> @@ -57,6 +57,11 @@ void board_init_f(ulong dummy)
>  	}
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +
>  u32 spl_boot_device(void)
>  {
>  	return BOOT_DEVICE_MMC1;
> diff --git a/arch/arm/mach-rockchip/rk3399-board-spl.c b/arch/arm/mach-rockchip/rk3399-board-spl.c
> index d35990e..b96903e 100644
> --- a/arch/arm/mach-rockchip/rk3399-board-spl.c
> +++ b/arch/arm/mach-rockchip/rk3399-board-spl.c
> @@ -60,6 +60,11 @@ u32 spl_boot_device(void)
>  	return boot_device;
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +
>  #define TIMER_CHN10_BASE	0xff8680a0
>  #define TIMER_END_COUNT_L	0x00
>  #define TIMER_END_COUNT_H	0x04
> diff --git a/arch/arm/mach-socfpga/spl.c b/arch/arm/mach-socfpga/spl.c
> index 9bf3b9a..71bae82 100644
> --- a/arch/arm/mach-socfpga/spl.c
> +++ b/arch/arm/mach-socfpga/spl.c
> @@ -66,6 +66,17 @@ u32 spl_boot_device(void)
>  	}
>  }
>  
> +#ifdef CONFIG_SPL_MMC_SUPPORT
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +#if defined(CONFIG_SPL_FAT_SUPPORT) || defined(CONFIG_SPL_EXT_SUPPORT)
> +	return MMCSD_MODE_FS;
> +#else
> +	return MMCSD_MODE_RAW;
> +#endif
> +}
> +#endif
> +
>  #if defined(CONFIG_TARGET_SOCFPGA_GEN5)
>  static void socfpga_nic301_slave_ns(void)
>  {
> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> index 1753fae..0c60ee0 100644
> --- a/arch/arm/mach-sunxi/board.c
> +++ b/arch/arm/mach-sunxi/board.c
> @@ -260,6 +260,12 @@ u32 spl_boot_device(void)
>  	return sunxi_get_boot_device();
>  }
>  
> +/* No confirmation data available in SPL yet. Hardcode bootmode */
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +
>  void board_init_f(ulong dummy)
>  {
>  	spl_init();
> diff --git a/arch/arm/mach-zynq/spl.c b/arch/arm/mach-zynq/spl.c
> index 0a303f4..b7e6d98 100644
> --- a/arch/arm/mach-zynq/spl.c
> +++ b/arch/arm/mach-zynq/spl.c
> @@ -69,6 +69,13 @@ u32 spl_boot_device(void)
>  	return mode;
>  }
>  
> +#ifdef CONFIG_SPL_MMC_SUPPORT
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_FS;
> +}
> +#endif
> +
>  #ifdef CONFIG_SPL_OS_BOOT
>  int spl_start_uboot(void)
>  {
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index 351f4ed..b57e0b0 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -281,17 +281,6 @@ static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image, struct mmc *mmc)
>  }
>  #endif
>  
> -u32 __weak spl_boot_mode(const u32 boot_device)
> -{
> -#if defined(CONFIG_SPL_FAT_SUPPORT) || defined(CONFIG_SPL_EXT_SUPPORT)
> -	return MMCSD_MODE_FS;
> -#elif defined(CONFIG_SUPPORT_EMMC_BOOT)
> -	return MMCSD_MODE_EMMCBOOT;
> -#else
> -	return MMCSD_MODE_RAW;
> -#endif
> -}
> -
>  int spl_mmc_load_image(struct spl_image_info *spl_image,
>  		       struct spl_boot_device *bootdev)
>  {
> 


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function"
  2018-02-15 12:14 [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function" Fabio Estevam
  2018-02-15 14:18 ` Stefano Babic
@ 2018-02-15 14:33 ` Lukasz Majewski
  2018-02-15 14:43   ` Fabio Estevam
  2018-02-15 14:44   ` Tom Rini
  1 sibling, 2 replies; 12+ messages in thread
From: Lukasz Majewski @ 2018-02-15 14:33 UTC (permalink / raw)
  To: u-boot

Hi Fabio,

> This reverts commit d695d6627803dbb78a226e04b0436a01633a9936.
> 
> Commit d695d6627803 ("spl: eMMC/SD: Provide one __weak spl_boot_mode()
> function") breaks the boot on several i.MX6 boards,
> such as cuboxi and wandboard:
> 
> U-Boot SPL 2018.03-rc1-00212-g48914fc119 (Feb 10 2018 - 11:04:33
> +1300) Trying to boot from MMC1
> Failed to mount ext2 filesystem...
> spl_load_image_ext: ext4fs mount err - 0
> 
> Revert it so that we can boot U-Boot again.

This is IMHO throwing the baby with the batch....

The problem is with legacy iMX6 boards, which use MMCSD_MODE_RAW to try
booting, even when they use CONFIG_SPL_EXT_SUPPORT for the boot.

On those boards we do have following boot order:

if CONFIG_SPL_FAT_SUPPORT {
	return MMCSD_MODE_FS
} else {
	return MODE_RAW
}

And thn in the spl_load_image() they first try MODE_RAW and if failed,
then MMCSD_MODE_EXT


So the (implicit) boot order is as follows:

1. SPL_FAT

2. RAW

3. If RAW fails, use EXT.


What I've think of:

1. Either implement the proper boot order (or just stick to the correct
SPL_EXT_SUPPORT).

2. If you agree - I can prepare the code to put imx code there they
were before this patch (to override the __weak function).
In that way we would got the cleanup for other archs in.



> 
> Reported-by: Jonathan Gray <jsg@jsg.id.au>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> Changes since v1:
> - Fix typo in commit log: "breaks the boot" instead of "breaks the
> build"
> 
>  arch/arm/cpu/arm1136/mx35/generic.c       | 21 +++++++++++++++++++++
>  arch/arm/cpu/armv7/ls102xa/spl.c          | 17 +++++++++++++++++
>  arch/arm/cpu/armv8/fsl-layerscape/spl.c   | 17 +++++++++++++++++
>  arch/arm/cpu/armv8/zynqmp/spl.c           | 14 ++++++++++++++
>  arch/arm/mach-at91/spl.c                  | 15 +++++++++++++++
>  arch/arm/mach-davinci/spl.c               |  5 +++++
>  arch/arm/mach-imx/spl.c                   | 23
> +++++++++++++++++++++++ arch/arm/mach-mvebu/spl.c                 |
> 7 +++++++ arch/arm/mach-rockchip/rk3188-board-spl.c |  5 +++++
>  arch/arm/mach-rockchip/rk3288-board-spl.c |  5 +++++
>  arch/arm/mach-rockchip/rk3368-board-spl.c |  5 +++++
>  arch/arm/mach-rockchip/rk3399-board-spl.c |  5 +++++
>  arch/arm/mach-socfpga/spl.c               | 11 +++++++++++
>  arch/arm/mach-sunxi/board.c               |  6 ++++++
>  arch/arm/mach-zynq/spl.c                  |  7 +++++++
>  common/spl/spl_mmc.c                      | 11 -----------
>  16 files changed, 163 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/cpu/arm1136/mx35/generic.c
> b/arch/arm/cpu/arm1136/mx35/generic.c index 4dcfc72..5297d62 100644
> --- a/arch/arm/cpu/arm1136/mx35/generic.c
> +++ b/arch/arm/cpu/arm1136/mx35/generic.c
> @@ -524,3 +524,24 @@ u32 spl_boot_device(void)
>  
>  	return BOOT_DEVICE_NONE;
>  }
> +
> +#ifdef CONFIG_SPL_BUILD
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	switch (spl_boot_device()) {
> +	case BOOT_DEVICE_MMC1:
> +#ifdef CONFIG_SPL_FAT_SUPPORT
> +		return MMCSD_MODE_FS;
> +#else
> +		return MMCSD_MODE_RAW;
> +#endif
> +		break;
> +	case BOOT_DEVICE_NAND:
> +		return 0;
> +		break;
> +	default:
> +		puts("spl: ERROR:  unsupported device\n");
> +		hang();
> +	}
> +}
> +#endif
> diff --git a/arch/arm/cpu/armv7/ls102xa/spl.c
> b/arch/arm/cpu/armv7/ls102xa/spl.c index 1e4a164..1246eed 100644
> --- a/arch/arm/cpu/armv7/ls102xa/spl.c
> +++ b/arch/arm/cpu/armv7/ls102xa/spl.c
> @@ -14,3 +14,20 @@ u32 spl_boot_device(void)
>  #endif
>  	return BOOT_DEVICE_NAND;
>  }
> +
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	switch (spl_boot_device()) {
> +	case BOOT_DEVICE_MMC1:
> +#ifdef CONFIG_SPL_FAT_SUPPORT
> +		return MMCSD_MODE_FS;
> +#else
> +		return MMCSD_MODE_RAW;
> +#endif
> +	case BOOT_DEVICE_NAND:
> +		return 0;
> +	default:
> +		puts("spl: error: unsupported device\n");
> +		hang();
> +	}
> +}
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/spl.c
> b/arch/arm/cpu/armv8/fsl-layerscape/spl.c index 3a74040..4093d15
> 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/spl.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/spl.c
> @@ -26,6 +26,23 @@ u32 spl_boot_device(void)
>  	return 0;
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	switch (spl_boot_device()) {
> +	case BOOT_DEVICE_MMC1:
> +#ifdef CONFIG_SPL_FAT_SUPPORT
> +		return MMCSD_MODE_FS;
> +#else
> +		return MMCSD_MODE_RAW;
> +#endif
> +	case BOOT_DEVICE_NAND:
> +		return 0;
> +	default:
> +		puts("spl: error: unsupported device\n");
> +		hang();
> +	}
> +}
> +
>  #ifdef CONFIG_SPL_BUILD
>  
>  void spl_board_init(void)
> diff --git a/arch/arm/cpu/armv8/zynqmp/spl.c
> b/arch/arm/cpu/armv8/zynqmp/spl.c index 0bfa5c1..bc7313a 100644
> --- a/arch/arm/cpu/armv8/zynqmp/spl.c
> +++ b/arch/arm/cpu/armv8/zynqmp/spl.c
> @@ -115,6 +115,20 @@ u32 spl_boot_device(void)
>  	return 0;
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	switch (boot_device) {
> +	case BOOT_DEVICE_RAM:
> +		return 0;
> +	case BOOT_DEVICE_MMC1:
> +	case BOOT_DEVICE_MMC2:
> +		return MMCSD_MODE_FS;
> +	default:
> +		puts("spl: error: unsupported device\n");
> +		hang();
> +	}
> +}
> +
>  #ifdef CONFIG_SPL_OS_BOOT
>  int spl_start_uboot(void)
>  {
> diff --git a/arch/arm/mach-at91/spl.c b/arch/arm/mach-at91/spl.c
> index 91add92..7e7e24b 100644
> --- a/arch/arm/mach-at91/spl.c
> +++ b/arch/arm/mach-at91/spl.c
> @@ -87,3 +87,18 @@ u32 spl_boot_device(void)
>  	return BOOT_DEVICE_NONE;
>  }
>  #endif
> +
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	switch (boot_device) {
> +#if defined(CONFIG_SYS_USE_MMC) || defined(CONFIG_SD_BOOT)
> +	case BOOT_DEVICE_MMC1:
> +	case BOOT_DEVICE_MMC2:
> +		return MMCSD_MODE_FS;
> +		break;
> +#endif
> +	case BOOT_DEVICE_NONE:
> +	default:
> +		hang();
> +	}
> +}
> diff --git a/arch/arm/mach-davinci/spl.c b/arch/arm/mach-davinci/spl.c
> index 4c74db9..564c200 100644
> --- a/arch/arm/mach-davinci/spl.c
> +++ b/arch/arm/mach-davinci/spl.c
> @@ -45,6 +45,11 @@ void spl_board_init(void)
>  	preloader_console_init();
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +
>  u32 spl_boot_device(void)
>  {
>  	switch (davinci_syscfg_regs->bootcfg) {
> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
> index a9079fc..b2521b2 100644
> --- a/arch/arm/mach-imx/spl.c
> +++ b/arch/arm/mach-imx/spl.c
> @@ -132,6 +132,29 @@ int g_dnl_bind_fixup(struct
> usb_device_descriptor *dev, const char *name) }
>  #endif
>  
> +#if defined(CONFIG_SPL_MMC_SUPPORT)
> +/* called from spl_mmc to see type of boot mode for storage (RAW or
> FAT) */ +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	switch (spl_boot_device()) {
> +	/* for MMC return either RAW or FAT mode */
> +	case BOOT_DEVICE_MMC1:
> +	case BOOT_DEVICE_MMC2:
> +#if defined(CONFIG_SPL_FAT_SUPPORT)
> +		return MMCSD_MODE_FS;
> +#elif defined(CONFIG_SUPPORT_EMMC_BOOT)
> +		return MMCSD_MODE_EMMCBOOT;
> +#else
> +		return MMCSD_MODE_RAW;
> +#endif
> +		break;
> +	default:
> +		puts("spl: ERROR:  unsupported device\n");
> +		hang();
> +	}
> +}
> +#endif
> +
>  #if defined(CONFIG_SECURE_BOOT)
>  
>  /*
> diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
> index a5086f1..d16a62d 100644
> --- a/arch/arm/mach-mvebu/spl.c
> +++ b/arch/arm/mach-mvebu/spl.c
> @@ -70,6 +70,13 @@ u32 spl_boot_device(void)
>  	return get_boot_device();
>  }
>  
> +#ifdef CONFIG_SPL_MMC_SUPPORT
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +#endif
> +
>  void board_init_f(ulong dummy)
>  {
>  	int ret;
> diff --git a/arch/arm/mach-rockchip/rk3188-board-spl.c
> b/arch/arm/mach-rockchip/rk3188-board-spl.c index 74771d3..8e3b8ae
> 100644 --- a/arch/arm/mach-rockchip/rk3188-board-spl.c
> +++ b/arch/arm/mach-rockchip/rk3188-board-spl.c
> @@ -72,6 +72,11 @@ fallback:
>  	return BOOT_DEVICE_MMC1;
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +
>  static int setup_arm_clock(void)
>  {
>  	struct udevice *dev;
> diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c
> b/arch/arm/mach-rockchip/rk3288-board-spl.c index f3ea624..f64a548
> 100644 --- a/arch/arm/mach-rockchip/rk3288-board-spl.c
> +++ b/arch/arm/mach-rockchip/rk3288-board-spl.c
> @@ -78,6 +78,11 @@ fallback:
>  	return BOOT_DEVICE_MMC1;
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +
>  #ifdef CONFIG_SPL_MMC_SUPPORT
>  static int configure_emmc(struct udevice *pinctrl)
>  {
> diff --git a/arch/arm/mach-rockchip/rk3368-board-spl.c
> b/arch/arm/mach-rockchip/rk3368-board-spl.c index 8055ae5..72d2c97
> 100644 --- a/arch/arm/mach-rockchip/rk3368-board-spl.c
> +++ b/arch/arm/mach-rockchip/rk3368-board-spl.c
> @@ -57,6 +57,11 @@ void board_init_f(ulong dummy)
>  	}
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +
>  u32 spl_boot_device(void)
>  {
>  	return BOOT_DEVICE_MMC1;
> diff --git a/arch/arm/mach-rockchip/rk3399-board-spl.c
> b/arch/arm/mach-rockchip/rk3399-board-spl.c index d35990e..b96903e
> 100644 --- a/arch/arm/mach-rockchip/rk3399-board-spl.c
> +++ b/arch/arm/mach-rockchip/rk3399-board-spl.c
> @@ -60,6 +60,11 @@ u32 spl_boot_device(void)
>  	return boot_device;
>  }
>  
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +
>  #define TIMER_CHN10_BASE	0xff8680a0
>  #define TIMER_END_COUNT_L	0x00
>  #define TIMER_END_COUNT_H	0x04
> diff --git a/arch/arm/mach-socfpga/spl.c b/arch/arm/mach-socfpga/spl.c
> index 9bf3b9a..71bae82 100644
> --- a/arch/arm/mach-socfpga/spl.c
> +++ b/arch/arm/mach-socfpga/spl.c
> @@ -66,6 +66,17 @@ u32 spl_boot_device(void)
>  	}
>  }
>  
> +#ifdef CONFIG_SPL_MMC_SUPPORT
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +#if defined(CONFIG_SPL_FAT_SUPPORT) ||
> defined(CONFIG_SPL_EXT_SUPPORT)
> +	return MMCSD_MODE_FS;
> +#else
> +	return MMCSD_MODE_RAW;
> +#endif
> +}
> +#endif
> +
>  #if defined(CONFIG_TARGET_SOCFPGA_GEN5)
>  static void socfpga_nic301_slave_ns(void)
>  {
> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> index 1753fae..0c60ee0 100644
> --- a/arch/arm/mach-sunxi/board.c
> +++ b/arch/arm/mach-sunxi/board.c
> @@ -260,6 +260,12 @@ u32 spl_boot_device(void)
>  	return sunxi_get_boot_device();
>  }
>  
> +/* No confirmation data available in SPL yet. Hardcode bootmode */
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_RAW;
> +}
> +
>  void board_init_f(ulong dummy)
>  {
>  	spl_init();
> diff --git a/arch/arm/mach-zynq/spl.c b/arch/arm/mach-zynq/spl.c
> index 0a303f4..b7e6d98 100644
> --- a/arch/arm/mach-zynq/spl.c
> +++ b/arch/arm/mach-zynq/spl.c
> @@ -69,6 +69,13 @@ u32 spl_boot_device(void)
>  	return mode;
>  }
>  
> +#ifdef CONFIG_SPL_MMC_SUPPORT
> +u32 spl_boot_mode(const u32 boot_device)
> +{
> +	return MMCSD_MODE_FS;
> +}
> +#endif
> +
>  #ifdef CONFIG_SPL_OS_BOOT
>  int spl_start_uboot(void)
>  {
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index 351f4ed..b57e0b0 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -281,17 +281,6 @@ static int spl_mmc_do_fs_boot(struct
> spl_image_info *spl_image, struct mmc *mmc) }
>  #endif
>  
> -u32 __weak spl_boot_mode(const u32 boot_device)
> -{
> -#if defined(CONFIG_SPL_FAT_SUPPORT) ||
> defined(CONFIG_SPL_EXT_SUPPORT)
> -	return MMCSD_MODE_FS;
> -#elif defined(CONFIG_SUPPORT_EMMC_BOOT)
> -	return MMCSD_MODE_EMMCBOOT;
> -#else
> -	return MMCSD_MODE_RAW;
> -#endif
> -}
> -
>  int spl_mmc_load_image(struct spl_image_info *spl_image,
>  		       struct spl_boot_device *bootdev)
>  {




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180215/85187a28/attachment.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function"
  2018-02-15 14:18 ` Stefano Babic
@ 2018-02-15 14:41   ` Lukasz Majewski
  0 siblings, 0 replies; 12+ messages in thread
From: Lukasz Majewski @ 2018-02-15 14:41 UTC (permalink / raw)
  To: u-boot

Hi Stefano,

> Hi Heiko,
> 
> On 15/02/2018 13:14, Fabio Estevam wrote:
> > This reverts commit d695d6627803dbb78a226e04b0436a01633a9936.
> > 
> > Commit d695d6627803 ("spl: eMMC/SD: Provide one __weak
> > spl_boot_mode() function") breaks the boot on several i.MX6 boards,
> > such as cuboxi and wandboard:
> > 
> > U-Boot SPL 2018.03-rc1-00212-g48914fc119 (Feb 10 2018 - 11:04:33
> > +1300) Trying to boot from MMC1
> > Failed to mount ext2 filesystem...
> > spl_load_image_ext: ext4fs mount err - 0
> > 
> > Revert it so that we can boot U-Boot again.
> >   
> 
> Can this the cause of the issue on the wandboard you told me
> yesterday ? Wasn't it due to CONFIG_SPL_EXT_SUPPORT ?

Yes, exactly - those boards have CONFIG_SPL_EXT_SUPPORT defined, but
want to first use RAW booting (normally as default) and then boot
from EXT.

This is somewhat priority inversion.

I've explained it in detail in the other reply to this issue and in 
http://patchwork.ozlabs.org/patch/868854/


> 
> Just to collect and understand myself if issues are connected...
> 
> Best regards,
> Stefano
> 
> > Reported-by: Jonathan Gray <jsg@jsg.id.au>
> > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> > ---
> > Changes since v1:
> > - Fix typo in commit log: "breaks the boot" instead of "breaks the
> > build"
> > 
> >  arch/arm/cpu/arm1136/mx35/generic.c       | 21
> > +++++++++++++++++++++ arch/arm/cpu/armv7/ls102xa/spl.c          |
> > 17 +++++++++++++++++ arch/arm/cpu/armv8/fsl-layerscape/spl.c   | 17
> > +++++++++++++++++ arch/arm/cpu/armv8/zynqmp/spl.c           | 14
> > ++++++++++++++ arch/arm/mach-at91/spl.c                  | 15
> > +++++++++++++++ arch/arm/mach-davinci/spl.c               |  5 +++++
> >  arch/arm/mach-imx/spl.c                   | 23
> > +++++++++++++++++++++++ arch/arm/mach-mvebu/spl.c
> > |  7 +++++++ arch/arm/mach-rockchip/rk3188-board-spl.c |  5 +++++
> >  arch/arm/mach-rockchip/rk3288-board-spl.c |  5 +++++
> >  arch/arm/mach-rockchip/rk3368-board-spl.c |  5 +++++
> >  arch/arm/mach-rockchip/rk3399-board-spl.c |  5 +++++
> >  arch/arm/mach-socfpga/spl.c               | 11 +++++++++++
> >  arch/arm/mach-sunxi/board.c               |  6 ++++++
> >  arch/arm/mach-zynq/spl.c                  |  7 +++++++
> >  common/spl/spl_mmc.c                      | 11 -----------
> >  16 files changed, 163 insertions(+), 11 deletions(-)
> > 
> > diff --git a/arch/arm/cpu/arm1136/mx35/generic.c
> > b/arch/arm/cpu/arm1136/mx35/generic.c index 4dcfc72..5297d62 100644
> > --- a/arch/arm/cpu/arm1136/mx35/generic.c
> > +++ b/arch/arm/cpu/arm1136/mx35/generic.c
> > @@ -524,3 +524,24 @@ u32 spl_boot_device(void)
> >  
> >  	return BOOT_DEVICE_NONE;
> >  }
> > +
> > +#ifdef CONFIG_SPL_BUILD
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	switch (spl_boot_device()) {
> > +	case BOOT_DEVICE_MMC1:
> > +#ifdef CONFIG_SPL_FAT_SUPPORT
> > +		return MMCSD_MODE_FS;
> > +#else
> > +		return MMCSD_MODE_RAW;
> > +#endif
> > +		break;
> > +	case BOOT_DEVICE_NAND:
> > +		return 0;
> > +		break;
> > +	default:
> > +		puts("spl: ERROR:  unsupported device\n");
> > +		hang();
> > +	}
> > +}
> > +#endif
> > diff --git a/arch/arm/cpu/armv7/ls102xa/spl.c
> > b/arch/arm/cpu/armv7/ls102xa/spl.c index 1e4a164..1246eed 100644
> > --- a/arch/arm/cpu/armv7/ls102xa/spl.c
> > +++ b/arch/arm/cpu/armv7/ls102xa/spl.c
> > @@ -14,3 +14,20 @@ u32 spl_boot_device(void)
> >  #endif
> >  	return BOOT_DEVICE_NAND;
> >  }
> > +
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	switch (spl_boot_device()) {
> > +	case BOOT_DEVICE_MMC1:
> > +#ifdef CONFIG_SPL_FAT_SUPPORT
> > +		return MMCSD_MODE_FS;
> > +#else
> > +		return MMCSD_MODE_RAW;
> > +#endif
> > +	case BOOT_DEVICE_NAND:
> > +		return 0;
> > +	default:
> > +		puts("spl: error: unsupported device\n");
> > +		hang();
> > +	}
> > +}
> > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/spl.c
> > b/arch/arm/cpu/armv8/fsl-layerscape/spl.c index 3a74040..4093d15
> > 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/spl.c
> > +++ b/arch/arm/cpu/armv8/fsl-layerscape/spl.c
> > @@ -26,6 +26,23 @@ u32 spl_boot_device(void)
> >  	return 0;
> >  }
> >  
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	switch (spl_boot_device()) {
> > +	case BOOT_DEVICE_MMC1:
> > +#ifdef CONFIG_SPL_FAT_SUPPORT
> > +		return MMCSD_MODE_FS;
> > +#else
> > +		return MMCSD_MODE_RAW;
> > +#endif
> > +	case BOOT_DEVICE_NAND:
> > +		return 0;
> > +	default:
> > +		puts("spl: error: unsupported device\n");
> > +		hang();
> > +	}
> > +}
> > +
> >  #ifdef CONFIG_SPL_BUILD
> >  
> >  void spl_board_init(void)
> > diff --git a/arch/arm/cpu/armv8/zynqmp/spl.c
> > b/arch/arm/cpu/armv8/zynqmp/spl.c index 0bfa5c1..bc7313a 100644
> > --- a/arch/arm/cpu/armv8/zynqmp/spl.c
> > +++ b/arch/arm/cpu/armv8/zynqmp/spl.c
> > @@ -115,6 +115,20 @@ u32 spl_boot_device(void)
> >  	return 0;
> >  }
> >  
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	switch (boot_device) {
> > +	case BOOT_DEVICE_RAM:
> > +		return 0;
> > +	case BOOT_DEVICE_MMC1:
> > +	case BOOT_DEVICE_MMC2:
> > +		return MMCSD_MODE_FS;
> > +	default:
> > +		puts("spl: error: unsupported device\n");
> > +		hang();
> > +	}
> > +}
> > +
> >  #ifdef CONFIG_SPL_OS_BOOT
> >  int spl_start_uboot(void)
> >  {
> > diff --git a/arch/arm/mach-at91/spl.c b/arch/arm/mach-at91/spl.c
> > index 91add92..7e7e24b 100644
> > --- a/arch/arm/mach-at91/spl.c
> > +++ b/arch/arm/mach-at91/spl.c
> > @@ -87,3 +87,18 @@ u32 spl_boot_device(void)
> >  	return BOOT_DEVICE_NONE;
> >  }
> >  #endif
> > +
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	switch (boot_device) {
> > +#if defined(CONFIG_SYS_USE_MMC) || defined(CONFIG_SD_BOOT)
> > +	case BOOT_DEVICE_MMC1:
> > +	case BOOT_DEVICE_MMC2:
> > +		return MMCSD_MODE_FS;
> > +		break;
> > +#endif
> > +	case BOOT_DEVICE_NONE:
> > +	default:
> > +		hang();
> > +	}
> > +}
> > diff --git a/arch/arm/mach-davinci/spl.c
> > b/arch/arm/mach-davinci/spl.c index 4c74db9..564c200 100644
> > --- a/arch/arm/mach-davinci/spl.c
> > +++ b/arch/arm/mach-davinci/spl.c
> > @@ -45,6 +45,11 @@ void spl_board_init(void)
> >  	preloader_console_init();
> >  }
> >  
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	return MMCSD_MODE_RAW;
> > +}
> > +
> >  u32 spl_boot_device(void)
> >  {
> >  	switch (davinci_syscfg_regs->bootcfg) {
> > diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
> > index a9079fc..b2521b2 100644
> > --- a/arch/arm/mach-imx/spl.c
> > +++ b/arch/arm/mach-imx/spl.c
> > @@ -132,6 +132,29 @@ int g_dnl_bind_fixup(struct
> > usb_device_descriptor *dev, const char *name) }
> >  #endif
> >  
> > +#if defined(CONFIG_SPL_MMC_SUPPORT)
> > +/* called from spl_mmc to see type of boot mode for storage (RAW
> > or FAT) */ +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	switch (spl_boot_device()) {
> > +	/* for MMC return either RAW or FAT mode */
> > +	case BOOT_DEVICE_MMC1:
> > +	case BOOT_DEVICE_MMC2:
> > +#if defined(CONFIG_SPL_FAT_SUPPORT)
> > +		return MMCSD_MODE_FS;
> > +#elif defined(CONFIG_SUPPORT_EMMC_BOOT)
> > +		return MMCSD_MODE_EMMCBOOT;
> > +#else
> > +		return MMCSD_MODE_RAW;
> > +#endif
> > +		break;
> > +	default:
> > +		puts("spl: ERROR:  unsupported device\n");
> > +		hang();
> > +	}
> > +}
> > +#endif
> > +
> >  #if defined(CONFIG_SECURE_BOOT)
> >  
> >  /*
> > diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
> > index a5086f1..d16a62d 100644
> > --- a/arch/arm/mach-mvebu/spl.c
> > +++ b/arch/arm/mach-mvebu/spl.c
> > @@ -70,6 +70,13 @@ u32 spl_boot_device(void)
> >  	return get_boot_device();
> >  }
> >  
> > +#ifdef CONFIG_SPL_MMC_SUPPORT
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	return MMCSD_MODE_RAW;
> > +}
> > +#endif
> > +
> >  void board_init_f(ulong dummy)
> >  {
> >  	int ret;
> > diff --git a/arch/arm/mach-rockchip/rk3188-board-spl.c
> > b/arch/arm/mach-rockchip/rk3188-board-spl.c index 74771d3..8e3b8ae
> > 100644 --- a/arch/arm/mach-rockchip/rk3188-board-spl.c
> > +++ b/arch/arm/mach-rockchip/rk3188-board-spl.c
> > @@ -72,6 +72,11 @@ fallback:
> >  	return BOOT_DEVICE_MMC1;
> >  }
> >  
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	return MMCSD_MODE_RAW;
> > +}
> > +
> >  static int setup_arm_clock(void)
> >  {
> >  	struct udevice *dev;
> > diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c
> > b/arch/arm/mach-rockchip/rk3288-board-spl.c index f3ea624..f64a548
> > 100644 --- a/arch/arm/mach-rockchip/rk3288-board-spl.c
> > +++ b/arch/arm/mach-rockchip/rk3288-board-spl.c
> > @@ -78,6 +78,11 @@ fallback:
> >  	return BOOT_DEVICE_MMC1;
> >  }
> >  
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	return MMCSD_MODE_RAW;
> > +}
> > +
> >  #ifdef CONFIG_SPL_MMC_SUPPORT
> >  static int configure_emmc(struct udevice *pinctrl)
> >  {
> > diff --git a/arch/arm/mach-rockchip/rk3368-board-spl.c
> > b/arch/arm/mach-rockchip/rk3368-board-spl.c index 8055ae5..72d2c97
> > 100644 --- a/arch/arm/mach-rockchip/rk3368-board-spl.c
> > +++ b/arch/arm/mach-rockchip/rk3368-board-spl.c
> > @@ -57,6 +57,11 @@ void board_init_f(ulong dummy)
> >  	}
> >  }
> >  
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	return MMCSD_MODE_RAW;
> > +}
> > +
> >  u32 spl_boot_device(void)
> >  {
> >  	return BOOT_DEVICE_MMC1;
> > diff --git a/arch/arm/mach-rockchip/rk3399-board-spl.c
> > b/arch/arm/mach-rockchip/rk3399-board-spl.c index d35990e..b96903e
> > 100644 --- a/arch/arm/mach-rockchip/rk3399-board-spl.c
> > +++ b/arch/arm/mach-rockchip/rk3399-board-spl.c
> > @@ -60,6 +60,11 @@ u32 spl_boot_device(void)
> >  	return boot_device;
> >  }
> >  
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	return MMCSD_MODE_RAW;
> > +}
> > +
> >  #define TIMER_CHN10_BASE	0xff8680a0
> >  #define TIMER_END_COUNT_L	0x00
> >  #define TIMER_END_COUNT_H	0x04
> > diff --git a/arch/arm/mach-socfpga/spl.c
> > b/arch/arm/mach-socfpga/spl.c index 9bf3b9a..71bae82 100644
> > --- a/arch/arm/mach-socfpga/spl.c
> > +++ b/arch/arm/mach-socfpga/spl.c
> > @@ -66,6 +66,17 @@ u32 spl_boot_device(void)
> >  	}
> >  }
> >  
> > +#ifdef CONFIG_SPL_MMC_SUPPORT
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +#if defined(CONFIG_SPL_FAT_SUPPORT) ||
> > defined(CONFIG_SPL_EXT_SUPPORT)
> > +	return MMCSD_MODE_FS;
> > +#else
> > +	return MMCSD_MODE_RAW;
> > +#endif
> > +}
> > +#endif
> > +
> >  #if defined(CONFIG_TARGET_SOCFPGA_GEN5)
> >  static void socfpga_nic301_slave_ns(void)
> >  {
> > diff --git a/arch/arm/mach-sunxi/board.c
> > b/arch/arm/mach-sunxi/board.c index 1753fae..0c60ee0 100644
> > --- a/arch/arm/mach-sunxi/board.c
> > +++ b/arch/arm/mach-sunxi/board.c
> > @@ -260,6 +260,12 @@ u32 spl_boot_device(void)
> >  	return sunxi_get_boot_device();
> >  }
> >  
> > +/* No confirmation data available in SPL yet. Hardcode bootmode */
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	return MMCSD_MODE_RAW;
> > +}
> > +
> >  void board_init_f(ulong dummy)
> >  {
> >  	spl_init();
> > diff --git a/arch/arm/mach-zynq/spl.c b/arch/arm/mach-zynq/spl.c
> > index 0a303f4..b7e6d98 100644
> > --- a/arch/arm/mach-zynq/spl.c
> > +++ b/arch/arm/mach-zynq/spl.c
> > @@ -69,6 +69,13 @@ u32 spl_boot_device(void)
> >  	return mode;
> >  }
> >  
> > +#ifdef CONFIG_SPL_MMC_SUPPORT
> > +u32 spl_boot_mode(const u32 boot_device)
> > +{
> > +	return MMCSD_MODE_FS;
> > +}
> > +#endif
> > +
> >  #ifdef CONFIG_SPL_OS_BOOT
> >  int spl_start_uboot(void)
> >  {
> > diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> > index 351f4ed..b57e0b0 100644
> > --- a/common/spl/spl_mmc.c
> > +++ b/common/spl/spl_mmc.c
> > @@ -281,17 +281,6 @@ static int spl_mmc_do_fs_boot(struct
> > spl_image_info *spl_image, struct mmc *mmc) }
> >  #endif
> >  
> > -u32 __weak spl_boot_mode(const u32 boot_device)
> > -{
> > -#if defined(CONFIG_SPL_FAT_SUPPORT) ||
> > defined(CONFIG_SPL_EXT_SUPPORT)
> > -	return MMCSD_MODE_FS;
> > -#elif defined(CONFIG_SUPPORT_EMMC_BOOT)
> > -	return MMCSD_MODE_EMMCBOOT;
> > -#else
> > -	return MMCSD_MODE_RAW;
> > -#endif
> > -}
> > -
> >  int spl_mmc_load_image(struct spl_image_info *spl_image,
> >  		       struct spl_boot_device *bootdev)
> >  {
> >   
> 
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180215/43b137a8/attachment.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function"
  2018-02-15 14:33 ` Lukasz Majewski
@ 2018-02-15 14:43   ` Fabio Estevam
  2018-02-15 14:44   ` Tom Rini
  1 sibling, 0 replies; 12+ messages in thread
From: Fabio Estevam @ 2018-02-15 14:43 UTC (permalink / raw)
  To: u-boot

Hi Lukasz,

On Thu, Feb 15, 2018 at 12:33 PM, Lukasz Majewski <lukma@denx.de> wrote:

> 2. If you agree - I can prepare the code to put imx code there they
> were before this patch (to override the __weak function).
> In that way we would got the cleanup for other archs in.

As we are in rc2 I prefer if we go with the imx partial revert.

Just sent a v3 as suggested.

Thanks

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function"
  2018-02-15 14:33 ` Lukasz Majewski
  2018-02-15 14:43   ` Fabio Estevam
@ 2018-02-15 14:44   ` Tom Rini
  2018-02-15 14:50     ` Marek Vasut
                       ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Tom Rini @ 2018-02-15 14:44 UTC (permalink / raw)
  To: u-boot

On Thu, Feb 15, 2018 at 03:33:29PM +0100, Lukasz Majewski wrote:
> Hi Fabio,
> 
> > This reverts commit d695d6627803dbb78a226e04b0436a01633a9936.
> > 
> > Commit d695d6627803 ("spl: eMMC/SD: Provide one __weak spl_boot_mode()
> > function") breaks the boot on several i.MX6 boards,
> > such as cuboxi and wandboard:
> > 
> > U-Boot SPL 2018.03-rc1-00212-g48914fc119 (Feb 10 2018 - 11:04:33
> > +1300) Trying to boot from MMC1
> > Failed to mount ext2 filesystem...
> > spl_load_image_ext: ext4fs mount err - 0
> > 
> > Revert it so that we can boot U-Boot again.
> 
> This is IMHO throwing the baby with the batch....

I kind of feel we need to revert this too, sorry.  I'm also worried that
we've broken some of the other platforms that also have funky if-else
expected logic, and we'll be at the point soon where we have enough
platforms that need to override the spl_boot_mode func with what they
had before that it's not a huge win.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180215/17284e0a/attachment.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function"
  2018-02-15 14:44   ` Tom Rini
@ 2018-02-15 14:50     ` Marek Vasut
  2018-02-15 14:58     ` Lukasz Majewski
  2018-02-20 22:48     ` Fabio Estevam
  2 siblings, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2018-02-15 14:50 UTC (permalink / raw)
  To: u-boot

On 02/15/2018 03:44 PM, Tom Rini wrote:
> On Thu, Feb 15, 2018 at 03:33:29PM +0100, Lukasz Majewski wrote:
>> Hi Fabio,
>>
>>> This reverts commit d695d6627803dbb78a226e04b0436a01633a9936.
>>>
>>> Commit d695d6627803 ("spl: eMMC/SD: Provide one __weak spl_boot_mode()
>>> function") breaks the boot on several i.MX6 boards,
>>> such as cuboxi and wandboard:
>>>
>>> U-Boot SPL 2018.03-rc1-00212-g48914fc119 (Feb 10 2018 - 11:04:33
>>> +1300) Trying to boot from MMC1
>>> Failed to mount ext2 filesystem...
>>> spl_load_image_ext: ext4fs mount err - 0
>>>
>>> Revert it so that we can boot U-Boot again.
>>
>> This is IMHO throwing the baby with the batch....
> 
> I kind of feel we need to revert this too, sorry.  I'm also worried that
> we've broken some of the other platforms that also have funky if-else
> expected logic, and we'll be at the point soon where we have enough
> platforms that need to override the spl_boot_mode func with what they
> had before that it's not a huge win.

Maybe we should go with the ordering option instead ? But that'd
probably be something for 2018.05 .

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function"
  2018-02-15 14:44   ` Tom Rini
  2018-02-15 14:50     ` Marek Vasut
@ 2018-02-15 14:58     ` Lukasz Majewski
  2018-02-15 15:01       ` Fabio Estevam
  2018-02-15 17:09       ` Stefano Babic
  2018-02-20 22:48     ` Fabio Estevam
  2 siblings, 2 replies; 12+ messages in thread
From: Lukasz Majewski @ 2018-02-15 14:58 UTC (permalink / raw)
  To: u-boot

Hi Tom,

> On Thu, Feb 15, 2018 at 03:33:29PM +0100, Lukasz Majewski wrote:
> > Hi Fabio,
> >   
> > > This reverts commit d695d6627803dbb78a226e04b0436a01633a9936.
> > > 
> > > Commit d695d6627803 ("spl: eMMC/SD: Provide one __weak
> > > spl_boot_mode() function") breaks the boot on several i.MX6
> > > boards, such as cuboxi and wandboard:
> > > 
> > > U-Boot SPL 2018.03-rc1-00212-g48914fc119 (Feb 10 2018 - 11:04:33
> > > +1300) Trying to boot from MMC1
> > > Failed to mount ext2 filesystem...
> > > spl_load_image_ext: ext4fs mount err - 0
> > > 
> > > Revert it so that we can boot U-Boot again.  
> > 
> > This is IMHO throwing the baby with the batch....  
> 
> I kind of feel we need to revert this too, sorry.  I'm also worried
> that we've broken some of the other platforms that also have funky
> if-else expected logic, and we'll be at the point soon where we have
> enough platforms that need to override the spl_boot_mode func with
> what they had before that it's not a huge win.
> 

Just my 2 cents.

1. IMHO 2 months release cycle is too short, regarding the peace of
development; we move Kconfig, perform DM conversion, and deal with
legacy code

2. As we removed the obsolete / not maitained archs. We also need to
scrutinize the "core" u-boot code (which is often 10+ years old).


As this patch showed - spotting some "implicit" errors - which cannot be
found with travis CI build tests consumes some time.
What is even more strange - this code from the very beginnig was
developmed on iMX6 board ....


We JUST need MORE time to stabilize thing......


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180215/d68db4a3/attachment.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function"
  2018-02-15 14:58     ` Lukasz Majewski
@ 2018-02-15 15:01       ` Fabio Estevam
  2018-02-15 17:09       ` Stefano Babic
  1 sibling, 0 replies; 12+ messages in thread
From: Fabio Estevam @ 2018-02-15 15:01 UTC (permalink / raw)
  To: u-boot

Hi Lukasz,

On Thu, Feb 15, 2018 at 12:58 PM, Lukasz Majewski <lukma@denx.de> wrote:

> As this patch showed - spotting some "implicit" errors - which cannot be
> found with travis CI build tests consumes some time.

Travis CI build test is great, but we also need boot tests to catch
issues like this.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function"
  2018-02-15 14:58     ` Lukasz Majewski
  2018-02-15 15:01       ` Fabio Estevam
@ 2018-02-15 17:09       ` Stefano Babic
  2018-02-15 17:43         ` Marek Vasut
  1 sibling, 1 reply; 12+ messages in thread
From: Stefano Babic @ 2018-02-15 17:09 UTC (permalink / raw)
  To: u-boot

Hi Lukasz,

On 15/02/2018 15:58, Lukasz Majewski wrote:
> Hi Tom,
> 
>> On Thu, Feb 15, 2018 at 03:33:29PM +0100, Lukasz Majewski wrote:
>>> Hi Fabio,
>>>   
>>>> This reverts commit d695d6627803dbb78a226e04b0436a01633a9936.
>>>>
>>>> Commit d695d6627803 ("spl: eMMC/SD: Provide one __weak
>>>> spl_boot_mode() function") breaks the boot on several i.MX6
>>>> boards, such as cuboxi and wandboard:
>>>>
>>>> U-Boot SPL 2018.03-rc1-00212-g48914fc119 (Feb 10 2018 - 11:04:33
>>>> +1300) Trying to boot from MMC1
>>>> Failed to mount ext2 filesystem...
>>>> spl_load_image_ext: ext4fs mount err - 0
>>>>
>>>> Revert it so that we can boot U-Boot again.  
>>>
>>> This is IMHO throwing the baby with the batch....  
>>
>> I kind of feel we need to revert this too, sorry.  I'm also worried
>> that we've broken some of the other platforms that also have funky
>> if-else expected logic, and we'll be at the point soon where we have
>> enough platforms that need to override the spl_boot_mode func with
>> what they had before that it's not a huge win.
>>
> 
> Just my 2 cents.
> 

I add my two cents, too.


> 1. IMHO 2 months release cycle is too short, regarding the peace of
> development; we move Kconfig, perform DM conversion, and deal with
> legacy code

+1

I have always the feeling to be late and, if shortening the release
cycle has advantages, the side-effect is that each release must take
care of some new improvement without breaking past because, yes, it is a
release.

I understand that two months matches the Linux release cycle, but I
agree with Lukasz that we need more time to stabilize a release, taking
into account the goals we have (DM,...). And most of use are not working
full time at the bootloader, that makes thing worst.

Even if distros release cycle is independent from U-Boot / Kernel, I do
not know how many people take advantage of fast releasing. The most used
embedded buildsystems (Buildroot / Linux) have a most relaxed release
cycle and usually a Yocto release have just a U-Boot version.

> 
> 2. As we removed the obsolete / not maitained archs. We also need to
> scrutinize the "core" u-boot code (which is often 10+ years old).
> 
> 
> As this patch showed - spotting some "implicit" errors - which cannot be
> found with travis CI build tests consumes some time.
> What is even more strange - this code from the very beginnig was
> developmed on iMX6 board ....
> 
> 
> We JUST need MORE time to stabilize thing......
> 

+1

Best regards,
Stefano


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function"
  2018-02-15 17:09       ` Stefano Babic
@ 2018-02-15 17:43         ` Marek Vasut
  0 siblings, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2018-02-15 17:43 UTC (permalink / raw)
  To: u-boot

On 02/15/2018 06:09 PM, Stefano Babic wrote:
> Hi Lukasz,
> 
> On 15/02/2018 15:58, Lukasz Majewski wrote:
>> Hi Tom,
>>
>>> On Thu, Feb 15, 2018 at 03:33:29PM +0100, Lukasz Majewski wrote:
>>>> Hi Fabio,
>>>>   
>>>>> This reverts commit d695d6627803dbb78a226e04b0436a01633a9936.
>>>>>
>>>>> Commit d695d6627803 ("spl: eMMC/SD: Provide one __weak
>>>>> spl_boot_mode() function") breaks the boot on several i.MX6
>>>>> boards, such as cuboxi and wandboard:
>>>>>
>>>>> U-Boot SPL 2018.03-rc1-00212-g48914fc119 (Feb 10 2018 - 11:04:33
>>>>> +1300) Trying to boot from MMC1
>>>>> Failed to mount ext2 filesystem...
>>>>> spl_load_image_ext: ext4fs mount err - 0
>>>>>
>>>>> Revert it so that we can boot U-Boot again.  
>>>>
>>>> This is IMHO throwing the baby with the batch....  
>>>
>>> I kind of feel we need to revert this too, sorry.  I'm also worried
>>> that we've broken some of the other platforms that also have funky
>>> if-else expected logic, and we'll be at the point soon where we have
>>> enough platforms that need to override the spl_boot_mode func with
>>> what they had before that it's not a huge win.
>>>
>>
>> Just my 2 cents.
>>
> 
> I add my two cents, too.
> 
> 
>> 1. IMHO 2 months release cycle is too short, regarding the peace of
>> development; we move Kconfig, perform DM conversion, and deal with
>> legacy code
> 
> +1
> 
> I have always the feeling to be late and, if shortening the release
> cycle has advantages, the side-effect is that each release must take
> care of some new improvement without breaking past because, yes, it is a
> release.
> 
> I understand that two months matches the Linux release cycle

It does not, Linux has 70 days per cycle, 2.5 months. But is has
significantly more contributors and maintainers.

, but I
> agree with Lukasz that we need more time to stabilize a release, taking
> into account the goals we have (DM,...). And most of use are not working
> full time at the bootloader, that makes thing worst.
> 
> Even if distros release cycle is independent from U-Boot / Kernel, I do
> not know how many people take advantage of fast releasing. The most used
> embedded buildsystems (Buildroot / Linux) have a most relaxed release
> cycle and usually a Yocto

... project reference release ...

> release have just a U-Boot version.

Right.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function"
  2018-02-15 14:44   ` Tom Rini
  2018-02-15 14:50     ` Marek Vasut
  2018-02-15 14:58     ` Lukasz Majewski
@ 2018-02-20 22:48     ` Fabio Estevam
  2 siblings, 0 replies; 12+ messages in thread
From: Fabio Estevam @ 2018-02-20 22:48 UTC (permalink / raw)
  To: u-boot

Hi Tom/Stefano,

On Thu, Feb 15, 2018 at 12:44 PM, Tom Rini <trini@konsulko.com> wrote:
> On Thu, Feb 15, 2018 at 03:33:29PM +0100, Lukasz Majewski wrote:
>> Hi Fabio,
>>
>> > This reverts commit d695d6627803dbb78a226e04b0436a01633a9936.
>> >
>> > Commit d695d6627803 ("spl: eMMC/SD: Provide one __weak spl_boot_mode()
>> > function") breaks the boot on several i.MX6 boards,
>> > such as cuboxi and wandboard:
>> >
>> > U-Boot SPL 2018.03-rc1-00212-g48914fc119 (Feb 10 2018 - 11:04:33
>> > +1300) Trying to boot from MMC1
>> > Failed to mount ext2 filesystem...
>> > spl_load_image_ext: ext4fs mount err - 0
>> >
>> > Revert it so that we can boot U-Boot again.
>>
>> This is IMHO throwing the baby with the batch....
>
> I kind of feel we need to revert this too, sorry.  I'm also worried that
> we've broken some of the other platforms that also have funky if-else
> expected logic, and we'll be at the point soon where we have enough
> platforms that need to override the spl_boot_mode func with what they
> had before that it's not a huge win.

What is the resolution here? I also sent a partial revert on v3.

Please pick the one you prefer :-)

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-02-20 22:48 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-15 12:14 [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function" Fabio Estevam
2018-02-15 14:18 ` Stefano Babic
2018-02-15 14:41   ` Lukasz Majewski
2018-02-15 14:33 ` Lukasz Majewski
2018-02-15 14:43   ` Fabio Estevam
2018-02-15 14:44   ` Tom Rini
2018-02-15 14:50     ` Marek Vasut
2018-02-15 14:58     ` Lukasz Majewski
2018-02-15 15:01       ` Fabio Estevam
2018-02-15 17:09       ` Stefano Babic
2018-02-15 17:43         ` Marek Vasut
2018-02-20 22:48     ` Fabio Estevam

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.