* [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
@ 2019-09-03 21:43 Lukasz Majewski
2019-09-04 4:45 ` Heiko Schocher
0 siblings, 1 reply; 12+ messages in thread
From: Lukasz Majewski @ 2019-09-03 21:43 UTC (permalink / raw)
To: u-boot
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(+)
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
/* 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.
+
config SPL_MMC_TINY
bool "Tiny MMC framework in SPL"
depends on SPL_MMC_SUPPORT
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
2019-09-03 21:43 [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode Lukasz Majewski
@ 2019-09-04 4:45 ` Heiko Schocher
2019-09-04 8:46 ` Lukasz Majewski
0 siblings, 1 reply; 12+ messages in thread
From: Heiko Schocher @ 2019-09-04 4:45 UTC (permalink / raw)
To: u-boot
Hello Lukasz,
added Stefano to cc as he is the imx custodian.
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 ...
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) and boots U-Boot and system from sd card.
Than all software gets installed fully automated with the help of
swupdate ...
> 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?
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()
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 ?
> /* 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.
> +
> config SPL_MMC_TINY
> bool "Tiny MMC framework in SPL"
> depends on SPL_MMC_SUPPORT
>
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
2019-09-04 4:45 ` Heiko Schocher
@ 2019-09-04 8:46 ` Lukasz Majewski
2019-09-04 8:59 ` Stefano Babic
0 siblings, 1 reply; 12+ messages in thread
From: Lukasz Majewski @ 2019-09-04 8:46 UTC (permalink / raw)
To: u-boot
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.
>
> > /* 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190904/51782664/attachment.sig>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
2019-09-04 8:46 ` Lukasz Majewski
@ 2019-09-04 8:59 ` Stefano Babic
2019-09-04 9:54 ` Lukasz Majewski
0 siblings, 1 reply; 12+ messages in thread
From: Stefano Babic @ 2019-09-04 8:59 UTC (permalink / raw)
To: u-boot
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
=====================================================================
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
2019-09-04 8:59 ` Stefano Babic
@ 2019-09-04 9:54 ` Lukasz Majewski
2019-09-04 10:35 ` Lukasz Majewski
0 siblings, 1 reply; 12+ messages in thread
From: Lukasz Majewski @ 2019-09-04 9:54 UTC (permalink / raw)
To: u-boot
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@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.
>
> 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
>
>
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190904/02d5ca2b/attachment.sig>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
2019-09-04 9:54 ` Lukasz Majewski
@ 2019-09-04 10:35 ` Lukasz Majewski
2019-09-05 8:08 ` Stefano Babic
0 siblings, 1 reply; 12+ messages in thread
From: Lukasz Majewski @ 2019-09-04 10:35 UTC (permalink / raw)
To: u-boot
On Wed, 4 Sep 2019 11:54:40 +0200
Lukasz Majewski <lukma@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@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).
>
> >
> > 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
> >
> >
>
>
>
> 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
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190904/70f120a5/attachment.sig>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
2019-09-04 10:35 ` Lukasz Majewski
@ 2019-09-05 8:08 ` Stefano Babic
2019-09-05 9:42 ` Lukasz Majewski
2019-09-05 10:02 ` Heiko Schocher
0 siblings, 2 replies; 12+ messages in thread
From: Stefano Babic @ 2019-09-05 8:08 UTC (permalink / raw)
To: u-boot
On 04/09/19 12:35, Lukasz Majewski wrote:
> On Wed, 4 Sep 2019 11:54:40 +0200
> Lukasz Majewski <lukma@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@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 ?
Regards,
Stefano
>
>>
>>>
>>> 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
>>>
>>>
>>
>>
>>
>> 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
>
>
>
> 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
=====================================================================
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
2019-09-05 8:08 ` Stefano Babic
@ 2019-09-05 9:42 ` Lukasz Majewski
2019-09-05 10:18 ` Heiko Schocher
2019-09-05 10:02 ` Heiko Schocher
1 sibling, 1 reply; 12+ messages in thread
From: Lukasz Majewski @ 2019-09-05 9:42 UTC (permalink / raw)
To: u-boot
Hi Stefano,
> On 04/09/19 12:35, Lukasz Majewski wrote:
> > On Wed, 4 Sep 2019 11:54:40 +0200
> > Lukasz Majewski <lukma@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@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 ?
Making the spl_boot_device() __weak would be a fix only for a specific
appearance of spl_boot_device() in arch/arm/mach-imx/spl.c (L178):
------------------>8--------------------------
#if defined(CONFIG_SPL_MMC_SUPPORT)
/* 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()) {
/* for MMC return either RAW or FAT mode */
------------------8<--------------------------
The above code is only executed when SPL_MMC_SUPPORT is enabled and
IMHO the spl_boot_device() call assumes that one boots up from eMMC
(but not from SPI-NOR).
I'm (in a first place) curious if this code is correct - the boot_device
is passed as a parameter to this function, but then it is ignored.
> 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 ?
>
> Regards,
> Stefano
>
> >
> >>
> >>>
> >>> 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
> >>>
> >>>
> >>
> >>
> >>
> >> 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
> >
> >
> >
> > 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
>
>
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190905/c8916b4d/attachment.sig>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
2019-09-05 8:08 ` Stefano Babic
2019-09-05 9:42 ` Lukasz Majewski
@ 2019-09-05 10:02 ` Heiko Schocher
1 sibling, 0 replies; 12+ messages in thread
From: Heiko Schocher @ 2019-09-05 10:02 UTC (permalink / raw)
To: u-boot
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@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@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
2019-09-05 9:42 ` Lukasz Majewski
@ 2019-09-05 10:18 ` Heiko Schocher
2019-09-09 8:02 ` Lukasz Majewski
0 siblings, 1 reply; 12+ messages in thread
From: Heiko Schocher @ 2019-09-05 10:18 UTC (permalink / raw)
To: u-boot
Hello Lukasz,
Am 05.09.2019 um 11:42 schrieb Lukasz Majewski:
> Hi Stefano,
>
>> On 04/09/19 12:35, Lukasz Majewski wrote:
>>> On Wed, 4 Sep 2019 11:54:40 +0200
>>> Lukasz Majewski <lukma@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@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 ?
>
> Making the spl_boot_device() __weak would be a fix only for a specific
> appearance of spl_boot_device() in arch/arm/mach-imx/spl.c (L178):
>
> ------------------>8--------------------------
> #if defined(CONFIG_SPL_MMC_SUPPORT)
>
> /* 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()) {
> /* for MMC return either RAW or FAT mode */
> ------------------8<--------------------------
>
> The above code is only executed when SPL_MMC_SUPPORT is enabled and
> IMHO the spl_boot_device() call assumes that one boots up from eMMC
> (but not from SPI-NOR).
Indeed, it is not that easy ... so forget my previous commment.
> I'm (in a first place) curious if this code is correct - the boot_device
> is passed as a parameter to this function, but then it is ignored.
Yes, that was also my first thought, so I simply replaced
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index 4023f916ee..7fabec206b 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -178,7 +178,7 @@ 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()) {
+ switch (boot_device) {
/* for MMC return either RAW or FAT mode */
case BOOT_DEVICE_MMC1:
case BOOT_DEVICE_MMC2:
but I see Stefanos arguments:
https://patchwork.ozlabs.org/patch/796237/#1735852
So your current patch seems the best solution for me ...
bye,
Heiko
>
>
>> 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 ?
>>
>> Regards,
>> Stefano
>>
>>>
>>>>
>>>>>
>>>>> 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
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> 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
>>>
>>>
>>>
>>> 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
>>
>>
>
>
>
> 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-52 Fax: +49-8142-66989-80 Email: hs at denx.de
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
2019-09-05 10:18 ` Heiko Schocher
@ 2019-09-09 8:02 ` Lukasz Majewski
2019-09-09 11:27 ` Stefano Babic
0 siblings, 1 reply; 12+ messages in thread
From: Lukasz Majewski @ 2019-09-09 8:02 UTC (permalink / raw)
To: u-boot
Hi Heiko, Stefano
> Hello Lukasz,
>
> Am 05.09.2019 um 11:42 schrieb Lukasz Majewski:
> > Hi Stefano,
> >
> >> On 04/09/19 12:35, Lukasz Majewski wrote:
> >>> On Wed, 4 Sep 2019 11:54:40 +0200
> >>> Lukasz Majewski <lukma@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@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 ?
> >
> > Making the spl_boot_device() __weak would be a fix only for a
> > specific appearance of spl_boot_device() in arch/arm/mach-imx/spl.c
> > (L178):
> > ------------------>8--------------------------
> > #if defined(CONFIG_SPL_MMC_SUPPORT)
> >
> > /* 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()) {
> > /* for MMC return either RAW or FAT mode */
> > ------------------8<--------------------------
> >
> > The above code is only executed when SPL_MMC_SUPPORT is enabled and
> > IMHO the spl_boot_device() call assumes that one boots up from eMMC
> > (but not from SPI-NOR).
>
> Indeed, it is not that easy ... so forget my previous commment.
>
> > I'm (in a first place) curious if this code is correct - the
> > boot_device is passed as a parameter to this function, but then it
> > is ignored.
>
> Yes, that was also my first thought, so I simply replaced
>
> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
> index 4023f916ee..7fabec206b 100644
> --- a/arch/arm/mach-imx/spl.c
> +++ b/arch/arm/mach-imx/spl.c
> @@ -178,7 +178,7 @@ 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()) {
> + switch (boot_device) {
> /* for MMC return either RAW or FAT mode */
> case BOOT_DEVICE_MMC1:
> case BOOT_DEVICE_MMC2:
>
> but I see Stefanos arguments:
> https://patchwork.ozlabs.org/patch/796237/#1735852
>
> So your current patch seems the best solution for me ...
>
Can I assume that we do have a consensus here?
If yes - I will prepare v2 of this patch taking in the comments
regarding Heiko's production flashing comments.
> bye,
> Heiko
>
>
> >
> >
> >> 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 ?
> >>
> >> Regards,
> >> Stefano
> >>
> >>>
> >>>>
> >>>>>
> >>>>> 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
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>> 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
> >>>
> >>>
> >>>
> >>> 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
> >>
> >>
> >
> >
> >
> > 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
>
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190909/b6a952d9/attachment.sig>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
2019-09-09 8:02 ` Lukasz Majewski
@ 2019-09-09 11:27 ` Stefano Babic
0 siblings, 0 replies; 12+ messages in thread
From: Stefano Babic @ 2019-09-09 11:27 UTC (permalink / raw)
To: u-boot
On 09/09/19 10:02, Lukasz Majewski wrote:
> Hi Heiko, Stefano
>
>> Hello Lukasz,
>>
>> Am 05.09.2019 um 11:42 schrieb Lukasz Majewski:
>>> Hi Stefano,
>>>
>>>> On 04/09/19 12:35, Lukasz Majewski wrote:
>>>>> On Wed, 4 Sep 2019 11:54:40 +0200
>>>>> Lukasz Majewski <lukma@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@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 ?
>>>
>>> Making the spl_boot_device() __weak would be a fix only for a
>>> specific appearance of spl_boot_device() in arch/arm/mach-imx/spl.c
>>> (L178):
>>> ------------------>8--------------------------
>>> #if defined(CONFIG_SPL_MMC_SUPPORT)
>>>
>>> /* 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()) {
>>> /* for MMC return either RAW or FAT mode */
>>> ------------------8<--------------------------
>>>
>>> The above code is only executed when SPL_MMC_SUPPORT is enabled and
>>> IMHO the spl_boot_device() call assumes that one boots up from eMMC
>>> (but not from SPI-NOR).
>>
>> Indeed, it is not that easy ... so forget my previous commment.
>>
>>> I'm (in a first place) curious if this code is correct - the
>>> boot_device is passed as a parameter to this function, but then it
>>> is ignored.
>>
>> Yes, that was also my first thought, so I simply replaced
>>
>> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
>> index 4023f916ee..7fabec206b 100644
>> --- a/arch/arm/mach-imx/spl.c
>> +++ b/arch/arm/mach-imx/spl.c
>> @@ -178,7 +178,7 @@ 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()) {
>> + switch (boot_device) {
>> /* for MMC return either RAW or FAT mode */
>> case BOOT_DEVICE_MMC1:
>> case BOOT_DEVICE_MMC2:
>>
>> but I see Stefanos arguments:
>> https://patchwork.ozlabs.org/patch/796237/#1735852
>>
>> So your current patch seems the best solution for me ...
>>
>
> Can I assume that we do have a consensus here?
>
> If yes - I will prepare v2 of this patch taking in the comments
> regarding Heiko's production flashing comments.
ok, fine.
Regards,
Stefano
>
>> bye,
>> Heiko
>>
>>
>>>
>>>
>>>> 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 ?
>>>>
>>>> Regards,
>>>> Stefano
>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>> 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
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> 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
>>>>>
>>>>>
>>>>>
>>>>> 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
>>>>
>>>>
>>>
>>>
>>>
>>> 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
>>
>
>
>
> 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
=====================================================================
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-09-09 11:27 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-03 21:43 [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode Lukasz Majewski
2019-09-04 4:45 ` Heiko Schocher
2019-09-04 8:46 ` Lukasz Majewski
2019-09-04 8:59 ` Stefano Babic
2019-09-04 9:54 ` Lukasz Majewski
2019-09-04 10:35 ` Lukasz Majewski
2019-09-05 8:08 ` Stefano Babic
2019-09-05 9:42 ` Lukasz Majewski
2019-09-05 10:18 ` Heiko Schocher
2019-09-09 8:02 ` Lukasz Majewski
2019-09-09 11:27 ` Stefano Babic
2019-09-05 10:02 ` Heiko Schocher
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.