On 04/09/19 10:46, Lukasz Majewski wrote: > Hi Heiko, > >> Hello Lukasz, >> >> added Stefano to cc as he is the imx custodian. > > I've relied on patman ... Thanks for adding Stefano to CC. > >> >> Am 03.09.2019 um 23:43 schrieb Lukasz Majewski: >>> This change tries to fix the following problem: >>> >>> - The board boots (to be more precise - ROM loads SPL) from a slow >>> SPI-NOR memory. >>> As a result the spl_boot_device() will return SPI-NOR as a boot >>> device (which is correct). >>> >>> - The problem is that in 'falcon boot' the eMMC is used as a boot >>> medium to load kernel from its partition. >>> Calling spl_boot_device() will break things as it returns >>> SPI-NOR device. >>> >>> To fix this issue the new CONFIG_SPL_FORCE_MMC_BOOT Kconfig flag is >>> introduced to handle this special use case. By default it is not >>> defined, so there is no change in the legacy code flow. >>> >>> Signed-off-by: Lukasz Majewski <lu...@denx.de> >>> >>> >>> --- >>> >>> This patch is a follow up of previous discussion/fix: >>> https://patchwork.ozlabs.org/patch/796237/ >>> >>> Travis-CI: >>> https://travis-ci.org/lmajewski/u-boot-dfu/builds/580272414 >>> --- >>> arch/arm/mach-imx/spl.c | 11 +++++++++++ >>> common/spl/Kconfig | 7 +++++++ >>> 2 files changed, 18 insertions(+) >> >> I have just similiar change for an imx6ull based board ... > > Also one more of my boards uses this trick (i.MX28 based one). > >> just antoher usecase: In factory empty board boots into USB SDP mode, >> and SPL gets loaded with usb_loader ... SPL detects a sd card (there >> is no sd card slot mounted, mmc boot is disabled! They attach only one >> for installing software) > > So there is no dedicated SD card slot (also the mmc is disabled on > that point). > Instead, in the factory the sd card is attached to pads - just for this > time. > >> and boots U-Boot and system from sd card. >> Than all software gets installed fully automated with the help of >> swupdate ... > > Ok. > >> >>> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c >>> index 1f230aca3397..daa3d8f7ed94 100644 >>> --- a/arch/arm/mach-imx/spl.c >>> +++ b/arch/arm/mach-imx/spl.c >>> @@ -178,7 +178,18 @@ 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) { >>> +/* >>> + * When CONFIG_SPL_FORCE_MMC_BOOT is defined the 'boot_device' is >>> used >>> + * unconditionally to decide about device to use for booting. >>> + * This is crucial for falcon boot mode, when board boots up (i.e. >>> ROM >>> + * loads SPL) from slow SPI-NOR memory and afterwards the SPL's >>> 'falcon' boot >>> + * mode is used to load Linux OS from eMMC partition. >>> + */ >>> +#ifdef CONFIG_SPL_FORCE_MMC_BOOT >>> + switch (boot_device) { >>> +#else >>> switch (spl_boot_device()) { >>> +#endif >> >> Just dummy question .. couldn;t we switch to use always boot_device? > > Stefano has provided some rationale for the need of spl_boot_device() > in the previous version of this fix [1]: > > https://patchwork.ozlabs.org/patch/796237/ > > >> >> In my case I had a board specific: >> >> void board_boot_order(u32 *spl_boot_list) >> { >> spl_boot_list[0] = BOOT_DEVICE_MMC1; >> spl_boot_list[1] = BOOT_DEVICE_SPI; >> spl_boot_list[2] = BOOT_DEVICE_BOARD; >> spl_boot_list[3] = BOOT_DEVICE_NONE; >> } >> >> which leads that BOOT_DEVICE_MMC1 is passed to spl_boot_mode() > > Is your board normally booting from MMC or SPI-NOR (from where SoC ROM > loads SPL) ? > >> >> If MMC1 is not found, it tries to boot U-Boot from SPI, and if this is >> empty, as fallback board go into USB SDP mode. >> >> The weak function of board_boot_order is: >> >> __weak void board_boot_order(u32 *spl_boot_list) >> { >> spl_boot_list[0] = spl_boot_device(); >> } >> >> which is exactly what we pass to spl_boot_mode() ... so instead >> calling spl_boot_device() twice we can use the passed boot_device ... >> and save the ifdef and new Kconfig option. >> >> But may I oversee something ? > > Please read the Stefano's reply from [1] - the spl_boot_device() has a > valid use cases as well.
Nevertheless, why do we need to add a new compiler switch if this can be check into board code ? You just need that spl_boot_device() returns the device you want (MMC for falcon boot). Why do you need to force it in this way ? Regards, Stefano > >> >>> /* for MMC return either RAW or FAT mode */ >>> case BOOT_DEVICE_MMC1: >>> case BOOT_DEVICE_MMC2: >>> diff --git a/common/spl/Kconfig b/common/spl/Kconfig >>> index f467eca2be72..3460beb62d59 100644 >>> --- a/common/spl/Kconfig >>> +++ b/common/spl/Kconfig >>> @@ -607,6 +607,13 @@ config SPL_MMC_SUPPORT >>> this option to build the drivers in drivers/mmc as part >>> of an SPL build. >>> >>> +config SPL_FORCE_MMC_BOOT >>> + bool "Force SPL's falcon boot from MMC" >>> + depends on SPL_MMC_SUPPORT >>> + default n >>> + help >>> + Force SPL's falcon boot to use MMC device for Linux >>> kernel booting. >> >> Hmm... falcon boot is just one use case. I would drop "falcon boot" >> It just to force boot from MMC. > > Ok. I will rewrite the help message. > >> >>> + >>> config SPL_MMC_TINY >>> bool "Tiny MMC framework in SPL" >>> depends on SPL_MMC_SUPPORT >>> >> >> bye, >> Heiko > > > > 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-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de > -- ===================================================================== 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: sba...@denx.de ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot