[U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
Heiko Schocher
hs at denx.de
Thu Sep 5 10:02:48 UTC 2019
Hello Stefano, Lukasz,
Am 05.09.2019 um 10:08 schrieb Stefano Babic:
> On 04/09/19 12:35, Lukasz Majewski wrote:
>> On Wed, 4 Sep 2019 11:54:40 +0200
>> Lukasz Majewski <lukma at denx.de> wrote:
>>
>>> Hi Stefano, Heiko,
>>>
>>>> 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 at 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).
>>>
>>> Current status:
>>>
>>> git grep -n "spl_boot_mode" | grep weak
>>> common/spl/spl_mmc.c:287:u32 __weak spl_boot_mode(const u32
>>> boot_device)
>>>
>>> git grep -n "board_boot_order" | grep weak
>>> common/spl/spl.c:491:__weak void board_boot_order(u32 *spl_boot_list)
>>>
>>>
>>>
>>>
>>> The spl_boot_device() is defined in arch/arm/mach-imx/spl.c [*] as a
>>> NON __weak function:
>>> git grep -n "spl_boot_device" | grep weak
>>>
>>>
>>>
>>> Shall I make this function [*] as __weak and provide override for it
>>> in the board file?
>>>
>>> The problem is in the call of spl_boot_mode() (which is overridden
>>> already in arch/arm/mach-imx/spl.c) at common/spl/spl_mmc.c
>>>
>>> Which then calls spl_boot_device() [**], which may return (correctly)
>>> SPI-NOR for eMMC.
>>>
>>>
>>> As is now spl_boot_device() is not a __weak function.
>>>
>>>> Why do you need to
>>>> force it in this way ?
>>>
>>> The option is to use the proposed patch to make spl_boot_device as a
>>> weak function.
>>
>> The above text is a bit misleading - better version below.
>>
>> The solution would be:
>>
>> 1. Use the proposed patch (with Kconfig option)
>>
>> 2. Define spl_boot_device as weak and override it in board file.
>> However, it is not the preferred solution as spl_boot_device() on my
>> setup correctly returns SPI-NOR (as SOC ROM loads SPL from SPI-NOR,
>> not eMMC).
>
> Ok, but is a wek not thought for this purpose ? If it is weak, you can
> change it and decide to return the device you want. You could also check
> if Falco boot is enabled, at compile time with CONFIG_SPL_OS_BOOT or at
> runtime with spl_start_uboot(). Should it not be enough ?
I am fine with a weak function only.
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de
More information about the U-Boot
mailing list