U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Lukasz Majewski <lukma@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2] Revert "spl: eMMC/SD: Provide one __weak spl_boot_mode() function"
Date: Thu, 15 Feb 2018 15:33:29 +0100	[thread overview]
Message-ID: <20180215153329.466691e3@jawa> (raw)
In-Reply-To: <1518696884-15642-1-git-send-email-fabio.estevam@nxp.com>

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>

  parent reply	other threads:[~2018-02-15 14:33 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180215153329.466691e3@jawa \
    --to=lukma@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox