* [U-Boot] [PATCH] imx-common: spl: Allow booting from eMMC when SPL is loaded from non-eMMC device @ 2018-01-26 14:57 Lukasz Majewski 2018-01-26 15:19 ` Marek Vasut 2018-01-26 16:05 ` Stefano Babic 0 siblings, 2 replies; 5+ messages in thread From: Lukasz Majewski @ 2018-01-26 14:57 UTC (permalink / raw) To: u-boot 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: -- 2.11.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] imx-common: spl: Allow booting from eMMC when SPL is loaded from non-eMMC device 2018-01-26 14:57 [U-Boot] [PATCH] imx-common: spl: Allow booting from eMMC when SPL is loaded from non-eMMC device Lukasz Majewski @ 2018-01-26 15:19 ` Marek Vasut 2018-01-26 16:05 ` Stefano Babic 1 sibling, 0 replies; 5+ messages in thread From: Marek Vasut @ 2018-01-26 15:19 UTC (permalink / raw) To: u-boot 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: > -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] imx-common: spl: Allow booting from eMMC when SPL is loaded from non-eMMC device 2018-01-26 14:57 [U-Boot] [PATCH] imx-common: spl: Allow booting from eMMC when SPL is loaded from non-eMMC device Lukasz Majewski 2018-01-26 15:19 ` Marek Vasut @ 2018-01-26 16:05 ` Stefano Babic 2018-01-26 16:16 ` Marek Vasut 1 sibling, 1 reply; 5+ messages in thread From: Stefano Babic @ 2018-01-26 16:05 UTC (permalink / raw) To: u-boot 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 -- ===================================================================== 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] 5+ messages in thread
* [U-Boot] [PATCH] imx-common: spl: Allow booting from eMMC when SPL is loaded from non-eMMC device 2018-01-26 16:05 ` Stefano Babic @ 2018-01-26 16:16 ` Marek Vasut 2018-01-26 16:19 ` Marek Vasut 0 siblings, 1 reply; 5+ messages in thread From: Marek Vasut @ 2018-01-26 16:16 UTC (permalink / raw) To: u-boot 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. -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] imx-common: spl: Allow booting from eMMC when SPL is loaded from non-eMMC device 2018-01-26 16:16 ` Marek Vasut @ 2018-01-26 16:19 ` Marek Vasut 0 siblings, 0 replies; 5+ messages in thread From: Marek Vasut @ 2018-01-26 16:19 UTC (permalink / raw) To: u-boot 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. -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-01-26 16:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-01-26 14:57 [U-Boot] [PATCH] imx-common: spl: Allow booting from eMMC when SPL is loaded from non-eMMC device Lukasz Majewski 2018-01-26 15:19 ` Marek Vasut 2018-01-26 16:05 ` Stefano Babic 2018-01-26 16:16 ` Marek Vasut 2018-01-26 16:19 ` Marek Vasut
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.