[PATCH 1/2] imx8mn_ddr4_evk: Add USB Mass Storage support

From: Fabio Estevam festevam@denx.de
Add USB Mass Storage support, which is a convenient way to flash the eMMC card, for example.
Signed-off-by: Fabio Estevam festevam@denx.de --- configs/imx8mn_ddr4_evk_defconfig | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/configs/imx8mn_ddr4_evk_defconfig b/configs/imx8mn_ddr4_evk_defconfig index 917cdb5aa9d9..065f7392b706 100644 --- a/configs/imx8mn_ddr4_evk_defconfig +++ b/configs/imx8mn_ddr4_evk_defconfig @@ -42,6 +42,8 @@ CONFIG_CMD_FUSE=y CONFIG_CMD_GPIO=y CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y +CONFIG_CMD_USB=y +CONFIG_CMD_USB_MASS_STORAGE=y CONFIG_CMD_CACHE=y CONFIG_CMD_REGULATOR=y CONFIG_CMD_EXT4_WRITE=y @@ -82,4 +84,14 @@ CONFIG_SPL_SYSRESET=y CONFIG_SYSRESET_PSCI=y CONFIG_SYSRESET_WATCHDOG=y CONFIG_DM_THERMAL=y +CONFIG_USB=y +# CONFIG_SPL_DM_USB is not set +CONFIG_USB_EHCI_HCD=y +CONFIG_MXC_USB_OTG_HACTIVE=y +CONFIG_USB_GADGET=y +CONFIG_USB_GADGET_MANUFACTURER="FSL" +CONFIG_USB_GADGET_VENDOR_NUM=0x0525 +CONFIG_USB_GADGET_PRODUCT_NUM=0xa4a5 +CONFIG_CI_UDC=y +CONFIG_USB_GADGET_DOWNLOAD=y CONFIG_IMX_WATCHDOG=y

From: Fabio Estevam festevam@denx.de
Currently, on i.MX8MN/i.MX8MP (Bootrom version 2) it is not possible to load U-Boot via serial download mode, unless CONFIG_ENV_IS_NOWHERE is selected.
This was noticed before by Adam Ford and fixed on the imx8mn beacon board in commit 2c7ebf7778cf ("imx: imx8mn_beacon: Fix USB booting").
Such commit log states:
"The i.MX8M Nano can boot over USB using the boot ROM instead of adding extra code to SPL to support USB drivers, etc. However, when booting from USB, the environment doesnt' know where to load and causes a hang. Fix this hang by supporting CONFIG_ENV_IS_NOWHERE=y. It only falls back to this condition when booting from USB, so it does not impact MMC booting."
Select ENV_IS_NOWHERE globally when i.MX8MN or i.MX8MP are used, so that serial download mode can be used by default on these SoCs.
Signed-off-by: Fabio Estevam festevam@denx.de --- arch/arm/mach-imx/imx8m/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/arm/mach-imx/imx8m/Kconfig b/arch/arm/mach-imx/imx8m/Kconfig index 55db25062a9b..ba7328aec8a1 100644 --- a/arch/arm/mach-imx/imx8m/Kconfig +++ b/arch/arm/mach-imx/imx8m/Kconfig @@ -16,10 +16,12 @@ config IMX8MM config IMX8MN bool select IMX8M + select ENV_IS_NOWHERE
config IMX8MP bool select IMX8M + select ENV_IS_NOWHERE
config SYS_SOC default "imx8m"

On 4/20/22 23:07, Fabio Estevam wrote:
From: Fabio Estevam festevam@denx.de
Currently, on i.MX8MN/i.MX8MP (Bootrom version 2) it is not possible to load U-Boot via serial download mode, unless CONFIG_ENV_IS_NOWHERE is selected.
This was noticed before by Adam Ford and fixed on the imx8mn beacon board in commit 2c7ebf7778cf ("imx: imx8mn_beacon: Fix USB booting").
Such commit log states:
"The i.MX8M Nano can boot over USB using the boot ROM instead of adding extra code to SPL to support USB drivers, etc. However, when booting from USB, the environment doesnt' know where to load and causes a hang. Fix this hang by supporting CONFIG_ENV_IS_NOWHERE=y. It only falls back to this condition when booting from USB, so it does not impact MMC booting."
I suspect this happens because
arch/arm/mach-imx/imx8m/soc.c env_get_location()
contains " ... default: return ENVL_NOWHERE; "
right ?
I wonder what would happen if you were to add:
case USB_BOOT: return ENVL_UNKNOWN;
Maybe that would even work without CONFIG_ENV_IS_NOWHERE=y ?

Hi Marek,
On Wed, Apr 20, 2022 at 6:42 PM Marek Vasut marex@denx.de wrote:
I suspect this happens because
arch/arm/mach-imx/imx8m/soc.c env_get_location()
contains " ... default: return ENVL_NOWHERE; "
right ?
I wonder what would happen if you were to add:
case USB_BOOT: return ENVL_UNKNOWN;
Maybe that would even work without CONFIG_ENV_IS_NOWHERE=y ?
Unfortunately, with this change the boot via SDP does not complete:
U-Boot SPL 2022.04-00857-g0f0a48bd2df4-dirty (Apr 20 2022 - 18:57:00 -0300) SEC0: RNG instantiated Normal Boot WDT: Started watchdog@30280000 with servicing (60s timeout) Trying to boot from BOOTROM Find img info 0x&48020a00, size 872 Need continue download 1024 Download 762080, Total size 763616 NOTICE: BL31: v2.2(release):rel_imx_5.4.47_2.2.0-0-gc949a888e909 NOTICE: BL31: Built : 16:07:45, Apr 20 2022 (it hangs here)

On 4/21/22 00:03, Fabio Estevam wrote:
Hi Marek,
Hi,
On Wed, Apr 20, 2022 at 6:42 PM Marek Vasut marex@denx.de wrote:
I suspect this happens because
arch/arm/mach-imx/imx8m/soc.c env_get_location()
contains " ... default: return ENVL_NOWHERE; "
right ?
I wonder what would happen if you were to add:
case USB_BOOT: return ENVL_UNKNOWN;
Maybe that would even work without CONFIG_ENV_IS_NOWHERE=y ?
Unfortunately, with this change the boot via SDP does not complete:
Did you actually hit the USB_BOOT case or did you fall into the default: case ?

Hi Marek,
On Wed, Apr 20, 2022 at 7:15 PM Marek Vasut marex@denx.de wrote:
Did you actually hit the USB_BOOT case or did you fall into the default: case ?
It does hit the USB_BOOT case.
I also tested forcing to return ENVL_UNKNOWN:
--- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -1531,6 +1531,7 @@ void do_error(struct pt_regs *pt_regs) enum env_location env_get_location(enum env_operation op, int prio) { enum boot_device dev = get_boot_device(); + return ENVL_UNKNOWN;
if (prio) return ENVL_UNKNOWN;
but still, the boot does not complete.

Hi Fabio
On Thu, Apr 21, 2022 at 4:17 PM Fabio Estevam festevam@gmail.com wrote:
Hi Marek,
On Wed, Apr 20, 2022 at 7:15 PM Marek Vasut marex@denx.de wrote:
Did you actually hit the USB_BOOT case or did you fall into the default: case ?
It does hit the USB_BOOT case.
I also tested forcing to return ENVL_UNKNOWN:
--- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -1531,6 +1531,7 @@ void do_error(struct pt_regs *pt_regs) enum env_location env_get_location(enum env_operation op, int prio) { enum boot_device dev = get_boot_device();
return ENVL_UNKNOWN; if (prio) return ENVL_UNKNOWN;
but still, the boot does not complete.
You need only add USB_ case in MMC and NAND #ifdef CONFIG_ENV_IS_IN_NAND /* add */ case USB_BOOT: case NAND_BOOT: env_loc = ENVL_NAND; break; #endif #ifdef CONFIG_ENV_IS_IN_MMC /* add */ case USB_BOOT: case SD1_BOOT: case SD2_BOOT: case SD3_BOOT: case MMC1_BOOT: case MMC2_BOOT: case MMC3_BOOT: env_loc = ENVL_MMC; break; #endif
Michael

Hi Michael,
On 21/04/2022 11:34, Michael Nazzareno Trimarchi wrote:
You need only add USB_ case in MMC and NAND #ifdef CONFIG_ENV_IS_IN_NAND /* add */ case USB_BOOT: case NAND_BOOT: env_loc = ENVL_NAND; break; #endif #ifdef CONFIG_ENV_IS_IN_MMC /* add */ case USB_BOOT: case SD1_BOOT: case SD2_BOOT: case SD3_BOOT: case MMC1_BOOT: case MMC2_BOOT: case MMC3_BOOT: env_loc = ENVL_MMC; break; #endif
Your suggestion works, thanks.
I will prepare v2.
Thanks,
Fabio Estevam

Hi Fabio
On Thu, Apr 21, 2022 at 6:19 PM Fabio Estevam festevam@denx.de wrote:
Hi Michael,
On 21/04/2022 11:34, Michael Nazzareno Trimarchi wrote:
You need only add USB_ case in MMC and NAND #ifdef CONFIG_ENV_IS_IN_NAND /* add */ case USB_BOOT: case NAND_BOOT: env_loc = ENVL_NAND; break; #endif #ifdef CONFIG_ENV_IS_IN_MMC /* add */ case USB_BOOT: case SD1_BOOT: case SD2_BOOT: case SD3_BOOT: case MMC1_BOOT: case MMC2_BOOT: case MMC3_BOOT: env_loc = ENVL_MMC; break; #endif
Your suggestion works, thanks.
I will prepare v2.
That function should drop. There is not other architecture that does it. What about:
register_enviroment_hook() deregister_enviroment_hook()
This can replace the one default with board/arch etc
Michael
Thanks,
Fabio Estevam
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-60 Fax: (+49)-8142-66989-80 Email: festevam@denx.de

Hi Michael,
On 21/04/2022 13:34, Michael Nazzareno Trimarchi wrote:
That function should drop. There is not other architecture that does it. What about:
I implemented your suggestion like this:
--- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -1536,6 +1536,14 @@ enum env_location arch_env_get_location(enum env_operation op, int prio) return ENVL_UNKNOWN;
switch (dev) { + case USB_BOOT: + if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)) + return ENVL_SPI_FLASH; + if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND)) + return ENVL_NAND; + if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) + return ENVL_MMC; + return ENVL_NOWHERE; case QSPI_BOOT: case SPI_NOR_BOOT: if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH))
This still allows me to save the env into the eMMC when U-Boot is loaded from USB.
register_enviroment_hook() deregister_enviroment_hook()
This can replace the one default with board/arch etc
Marek submitted these patches:
https://source.denx.de/u-boot/u-boot/-/commit/de70e8879bb253f4d2a9ba9149cd41...
https://source.denx.de/u-boot/u-boot/-/commit/e4dc2d0620851d6e0e784d4ef0a50f...
Is this the mechanism that you are looking for?

On Thu, Apr 21, 2022 at 11:47 AM Fabio Estevam festevam@denx.de wrote:
Hi Michael,
On 21/04/2022 13:34, Michael Nazzareno Trimarchi wrote:
That function should drop. There is not other architecture that does it. What about:
I implemented your suggestion like this:
--- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -1536,6 +1536,14 @@ enum env_location arch_env_get_location(enum env_operation op, int prio) return ENVL_UNKNOWN;
switch (dev) {
case USB_BOOT:
if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH))
return ENVL_SPI_FLASH;
if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND))
return ENVL_NAND;
if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC))
return ENVL_MMC;
return ENVL_NOWHERE; case QSPI_BOOT: case SPI_NOR_BOOT: if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH))
This still allows me to save the env into the eMMC when U-Boot is loaded from USB.
register_enviroment_hook() deregister_enviroment_hook()
This can replace the one default with board/arch etc
Marek submitted these patches:
https://source.denx.de/u-boot/u-boot/-/commit/de70e8879bb253f4d2a9ba9149cd41...
https://source.denx.de/u-boot/u-boot/-/commit/e4dc2d0620851d6e0e784d4ef0a50f...
I believe those were applied today. I haven't verified it yet, but I think I saw the e-mail go by.
Is this the mechanism that you are looking for?

Hi
On Thu, Apr 21, 2022 at 6:51 PM Adam Ford aford173@gmail.com wrote:
On Thu, Apr 21, 2022 at 11:47 AM Fabio Estevam festevam@denx.de wrote:
Hi Michael,
On 21/04/2022 13:34, Michael Nazzareno Trimarchi wrote:
That function should drop. There is not other architecture that does it. What about:
I implemented your suggestion like this:
--- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -1536,6 +1536,14 @@ enum env_location arch_env_get_location(enum env_operation op, int prio) return ENVL_UNKNOWN;
switch (dev) {
case USB_BOOT:
if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH))
return ENVL_SPI_FLASH;
if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND))
return ENVL_NAND;
if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC))
return ENVL_MMC;
return ENVL_NOWHERE; case QSPI_BOOT: case SPI_NOR_BOOT: if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH))
This still allows me to save the env into the eMMC when U-Boot is loaded from USB.
register_enviroment_hook() deregister_enviroment_hook()
This can replace the one default with board/arch etc
Marek submitted these patches:
https://source.denx.de/u-boot/u-boot/-/commit/de70e8879bb253f4d2a9ba9149cd41...
https://source.denx.de/u-boot/u-boot/-/commit/e4dc2d0620851d6e0e784d4ef0a50f...
I believe those were applied today. I haven't verified it yet, but I think I saw the e-mail go by.
Is this the mechanism that you are looking for?
I have seen those patches already but the cost of a function pointer and a call is more readable. If they get applied, I will be fine with them ;)
Michael

On 21/04/2022 14:01, Michael Nazzareno Trimarchi wrote:
I have seen those patches already but the cost of a function pointer and a call is more readable. If they get applied, I will be fine with them ;)
They are already in master.
How do we proceed to fix the U-Boot load via USB?

Hi
On Thu, Apr 21, 2022 at 7:03 PM Fabio Estevam festevam@denx.de wrote:
On 21/04/2022 14:01, Michael Nazzareno Trimarchi wrote:
I have seen those patches already but the cost of a function pointer and a call is more readable. If they get applied, I will be fine with them ;)
They are already in master.
How do we proceed to fix the U-Boot load via USB?
The uboot has no problem, the problem is that the function is wrong ;). When you boot from USB you should inform where to pick the environment. You can force ENV_EVERYWHERE for those architectures, you can decide to change the switch as I said, or you can add in your board and override at board level the function ;)
Michael

On 21/04/2022 14:10, Michael Nazzareno Trimarchi wrote:
The uboot has no problem, the problem is that the function is wrong ;). When you boot from USB you should inform where to pick the environment. You can force ENV_EVERYWHERE for those architectures, you can decide to change the switch as I said, or you can add in your board and override
I will send a patch based on your suggestion, thanks.

On 4/21/22 16:17, Fabio Estevam wrote:
Hi Marek,
On Wed, Apr 20, 2022 at 7:15 PM Marek Vasut marex@denx.de wrote:
Did you actually hit the USB_BOOT case or did you fall into the default: case ?
It does hit the USB_BOOT case.
I also tested forcing to return ENVL_UNKNOWN:
Hum, sigh.
Can you check where exactly this hangs ? I speculate env_select() in env/env.c returns -ENODEV and then whatever calls env_select() ends up in hang(), no ?
participants (5)
-
Adam Ford
-
Fabio Estevam
-
Fabio Estevam
-
Marek Vasut
-
Michael Nazzareno Trimarchi