[U-Boot] [PATCH] imx-common: spl: Allow booting from eMMC when SPL is loaded from non-eMMC device
This patch tries to solve the problem described in following patch: https://patchwork.ozlabs.org/patch/796237/
The main argument against the above code was the potential lack of consistency if we boot SPL from the SD card (and then eMMC may load u-boot proper).
This patch preserves this consistency if spl_boot_device() detects boot from either SD card or eMMC.
It only will change boot device if boot from non-SD/eMMC device is detected - i.e SPI-NOR (as in this case).
Signed-off-by: Lukasz Majewski lukma@denx.de ---
arch/arm/mach-imx/spl.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c index 6c16872f59..735d9f6261 100644 --- a/arch/arm/mach-imx/spl.c +++ b/arch/arm/mach-imx/spl.c @@ -134,7 +134,12 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name) /* 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()) { + u32 spl_bd = spl_boot_device(); + + if (spl_bd != BOOT_DEVICE_MMC1) + spl_bd = boot_device; + + switch (spl_bd) { /* for MMC return either RAW or FAT mode */ case BOOT_DEVICE_MMC1: case BOOT_DEVICE_MMC2:
On 01/26/2018 03:57 PM, Lukasz Majewski wrote:
This patch tries to solve the problem described in following patch: https://patchwork.ozlabs.org/patch/796237/
You should explain what the problem is in the commit message. Random link to a random website which may go away at some point is useless. Having it below --- is fine, but in the commit message it's not.
The main argument against the above code was the potential lack of consistency if we boot SPL from the SD card (and then eMMC may load u-boot proper).
This patch preserves this consistency if spl_boot_device() detects boot from either SD card or eMMC.
It only will change boot device if boot from non-SD/eMMC device is detected - i.e SPI-NOR (as in this case).
And from this, I don't really understand what this patch is trying to solve. IMO it'd be better to solve this on a board-level.
Signed-off-by: Lukasz Majewski lukma@denx.de
arch/arm/mach-imx/spl.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c index 6c16872f59..735d9f6261 100644 --- a/arch/arm/mach-imx/spl.c +++ b/arch/arm/mach-imx/spl.c @@ -134,7 +134,12 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name) /* 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()) {
- u32 spl_bd = spl_boot_device();
- if (spl_bd != BOOT_DEVICE_MMC1)
spl_bd = boot_device;- switch (spl_bd) { /* for MMC return either RAW or FAT mode */ case BOOT_DEVICE_MMC1: case BOOT_DEVICE_MMC2:
On 26/01/2018 15:57, Lukasz Majewski wrote:
This patch tries to solve the problem described in following patch: https://patchwork.ozlabs.org/patch/796237/
The main argument against the above code was the potential lack of consistency if we boot SPL from the SD card (and then eMMC may load u-boot proper).
This patch preserves this consistency if spl_boot_device() detects boot from either SD card or eMMC.
It only will change boot device if boot from non-SD/eMMC device is detected - i.e SPI-NOR (as in this case).
Signed-off-by: Lukasz Majewski lukma@denx.de
arch/arm/mach-imx/spl.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c index 6c16872f59..735d9f6261 100644 --- a/arch/arm/mach-imx/spl.c +++ b/arch/arm/mach-imx/spl.c @@ -134,7 +134,12 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name) /* 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()) {
- u32 spl_bd = spl_boot_device();
- if (spl_bd != BOOT_DEVICE_MMC1)
spl_bd = boot_device;- switch (spl_bd) { /* for MMC return either RAW or FAT mode */ case BOOT_DEVICE_MMC1: case BOOT_DEVICE_MMC2:
But have we not board_boot_order() for such as cases ? It is a wek function and you can define it in your board code. Put in mach-imx, it is valid for all boards.
Best regards, Stefano
On 01/26/2018 05:05 PM, Stefano Babic wrote:
On 26/01/2018 15:57, Lukasz Majewski wrote:
This patch tries to solve the problem described in following patch: https://patchwork.ozlabs.org/patch/796237/
The main argument against the above code was the potential lack of consistency if we boot SPL from the SD card (and then eMMC may load u-boot proper).
This patch preserves this consistency if spl_boot_device() detects boot from either SD card or eMMC.
It only will change boot device if boot from non-SD/eMMC device is detected - i.e SPI-NOR (as in this case).
Signed-off-by: Lukasz Majewski lukma@denx.de
arch/arm/mach-imx/spl.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c index 6c16872f59..735d9f6261 100644 --- a/arch/arm/mach-imx/spl.c +++ b/arch/arm/mach-imx/spl.c @@ -134,7 +134,12 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name) /* 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()) {
- u32 spl_bd = spl_boot_device();
- if (spl_bd != BOOT_DEVICE_MMC1)
spl_bd = boot_device;- switch (spl_bd) { /* for MMC return either RAW or FAT mode */ case BOOT_DEVICE_MMC1: case BOOT_DEVICE_MMC2:
But have we not board_boot_order() for such as cases ? It is a wek function and you can define it in your board code. Put in mach-imx, it is valid for all boards.
After a bit of offline discussion with Lukasz, I think I understand what the problem is.
The SPL common code calls spl_boot_device() to determine which boot device to boot from next ; in his case, SPI or eMMC. In case eMMC is selected, the SPL MMC common code then calls spl_boot_mode() (different function than spl_boot_device()) to determine whether the content on SD/eMMC is RAW or on a FS.
The iMX implementation (and layerspace too) does extra stuff in the implementation of spl_boot_mode() by calling spl_boot_device() again, which is wrong.
spl_boot_mode() is only used by spl_mmc.c , so we are certain the system is booting from SD/MMC . By turning this around, if the board code did override the board_boot_order() to load the next payload from SD/MMC/eMMC AND started SPL from non-SD/MMC/eMMC device , calling spl_boot_device() in spl_boot_mode() implementation will return bogus result and the spl_boot_mode() will fail in the default: case.
Thus, the fix here is to do what ie. socfpga does, just discern whether the content on SD/MMC/eMMC is RAW or FS and do not do any extra checks.
On 01/26/2018 05:16 PM, Marek Vasut wrote:
On 01/26/2018 05:05 PM, Stefano Babic wrote:
On 26/01/2018 15:57, Lukasz Majewski wrote:
This patch tries to solve the problem described in following patch: https://patchwork.ozlabs.org/patch/796237/
The main argument against the above code was the potential lack of consistency if we boot SPL from the SD card (and then eMMC may load u-boot proper).
This patch preserves this consistency if spl_boot_device() detects boot from either SD card or eMMC.
It only will change boot device if boot from non-SD/eMMC device is detected - i.e SPI-NOR (as in this case).
Signed-off-by: Lukasz Majewski lukma@denx.de
arch/arm/mach-imx/spl.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c index 6c16872f59..735d9f6261 100644 --- a/arch/arm/mach-imx/spl.c +++ b/arch/arm/mach-imx/spl.c @@ -134,7 +134,12 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name) /* 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()) {
- u32 spl_bd = spl_boot_device();
- if (spl_bd != BOOT_DEVICE_MMC1)
spl_bd = boot_device;- switch (spl_bd) { /* for MMC return either RAW or FAT mode */ case BOOT_DEVICE_MMC1: case BOOT_DEVICE_MMC2:
But have we not board_boot_order() for such as cases ? It is a wek function and you can define it in your board code. Put in mach-imx, it is valid for all boards.
After a bit of offline discussion with Lukasz, I think I understand what the problem is.
The SPL common code calls spl_boot_device() to determine which boot device to boot from next ; in his case, SPI or eMMC. In case eMMC is selected, the SPL MMC common code then calls spl_boot_mode() (different function than spl_boot_device()) to determine whether the content on SD/eMMC is RAW or on a FS.
The iMX implementation (and layerspace too) does extra stuff in the implementation of spl_boot_mode() by calling spl_boot_device() again, which is wrong.
spl_boot_mode() is only used by spl_mmc.c , so we are certain the system is booting from SD/MMC . By turning this around, if the board code did override the board_boot_order() to load the next payload from SD/MMC/eMMC AND started SPL from non-SD/MMC/eMMC device , calling spl_boot_device() in spl_boot_mode() implementation will return bogus result and the spl_boot_mode() will fail in the default: case.
Thus, the fix here is to do what ie. socfpga does, just discern whether the content on SD/MMC/eMMC is RAW or FS and do not do any extra checks.
By doing so, we can probably even clean this up and have default __weak spl_boot_mode() implementation rather than having a custom copy for each SoC.
participants (3)
-
Lukasz Majewski -
Marek Vasut -
Stefano Babic