[U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode

Stefano Babic
2019-09-04T08:59:22Z

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 <lukma@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: lukma at 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: sbabic at denx.de
=====================================================================