Re: [PATCH v4 5/7] configs: am62x_evm_*: Enable USB and DFU support

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Sjoerd,

Thank you for the patch.

On ven., janv. 12, 2024 at 09:52, Sjoerd Simons <sjoerd@collabora.com> wrote:

> Enable USB host as well as USB gadget and DFU support for a53; For the
> r5 due to the smaller available size create a config fragment for DFU
> supports which disables support for persistent storage to free up space
> for USB support
>
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>

I know this will be reworked in v5 (to use defconfig fragments) as
discussed in [1] but i'd still like to give some feedback on my testing.

Applying this series seems to break eMMC booting for me on AM62x SK EVM:

U-Boot SPL 2024.04-rc1-00106-g7c3798e43744 (Feb 08 2024 - 10:41:17 +0100)
SYSFW ABI: 3.1 (firmware rev 0x0009 '9.1.8--v09.01.08 (Kool Koala)')
SPL initial stack usage: 13368 bytes
Trying to boot from MMC1
alloc space exhausted
spl_load_image_fat: error reading image tispl.bin, err - -22
SPL: failed to boot from all boot devices
### ERROR ### Please RESET the board ###

It seems that the size of tispl.bin gets too big because of this commit.

Before series:
~/work/ti/src/bootloaders-upstream/out/ $ du -s tispl.bin_unsigned
796     tispl.bin_unsigned

After series:
~/work/ti/src/bootloaders-upstream/out/ $ du -s tispl.bin_unsigned
876     tispl.bin_unsigned

Per my understanding, this change increases the size of the a53 U-Boot
SPL, which is why we can no longer boot from eMMC.

Here is are other depending project refs:
- optee_os: 439c5ecbb68b ("core: arm: fix integer overflow in generic_timer_{handler,start}()")
- ti-linux-firmware: 0350c2edecab ("ti-dm: Update firmware for J722S device")
- trustef-firmware-a: 17bef2248d45 ("Merge "feat(fvp): delegate FFH RAS handling to SP" into integration")

Reverting just this commit makes it boot to eMMC for me again.

I also tried this downstream change[2] to increase the alloc space in
the SPL. That did not fix the booting problem.

To fix it, I had to remove:
CONFIG_SPL_ENV_SUPPORT=y
CONFIG_SPL_DFU=y

Which bring down the tispl to:
~/work/ti/src/bootloaders-upstream/out/ $ du -s tispl.bin_unsigned
820     tispl.bin_unsigned

Of course, removing both config options is not an acceptable solution since
it defeats the purpose of this patch.

Did you test booting from eMMC when developping this series, or did you
just tested booting from DFU ?

[1] https://lore.kernel.org/all/b587fcaebed8b61285a901b468e3ccbddf7d06f2.camel@collabora.com/
[2] https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2023.04&id=28c1b1ec8ba89b7c7b2cd43483054a432552008f

>
> ---
>
> Changes in v4:
> - Move R5 dfu config to a config fragment rather then a full defconfig
> - Don't enable XHCI for the R5 SPL, unneeded
>
> Changes in v3:
> - Run savedefconfig to adjust to more recent u-boot
>
> Changes in v2:
> - Create a seperate defconfig for R5
>
>  configs/am62x_evm_a53_defconfig | 30 ++++++++++++++++++++++++++++++
>  configs/am62x_r5_usbdfu.config  | 28 ++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
>  create mode 100644 configs/am62x_r5_usbdfu.config
>
> diff --git a/configs/am62x_evm_a53_defconfig b/configs/am62x_evm_a53_defconfig
> index aa96c1b3125..f335eb11e63 100644
> --- a/configs/am62x_evm_a53_defconfig
> +++ b/configs/am62x_evm_a53_defconfig
> @@ -1,5 +1,6 @@
>  CONFIG_ARM=y
>  CONFIG_ARCH_K3=y
> +CONFIG_SYS_MALLOC_LEN=0x2000000
>  CONFIG_SYS_MALLOC_F_LEN=0x8000
>  CONFIG_SPL_LIBCOMMON_SUPPORT=y
>  CONFIG_SPL_LIBGENERIC_SUPPORT=y
> @@ -41,16 +42,23 @@ CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>  CONFIG_SPL_STACK_R=y
>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
> +CONFIG_SPL_ENV_SUPPORT=y
>  CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
>  CONFIG_SPL_DM_MAILBOX=y
>  CONFIG_SPL_DM_SPI_FLASH=y
>  CONFIG_SPL_POWER_DOMAIN=y
> +CONFIG_SPL_RAM_SUPPORT=y
> +CONFIG_SPL_RAM_DEVICE=y
>  # CONFIG_SPL_SPI_FLASH_TINY is not set
>  CONFIG_SPL_SPI_FLASH_SFDP_SUPPORT=y
>  CONFIG_SPL_SPI_LOAD=y
>  CONFIG_SYS_SPI_U_BOOT_OFFS=0x280000
> +CONFIG_SPL_USB_GADGET=y
> +CONFIG_SPL_DFU=y
>  CONFIG_SPL_YMODEM_SUPPORT=y
> +CONFIG_CMD_DFU=y
>  CONFIG_CMD_MMC=y
> +CONFIG_CMD_USB=y
>  CONFIG_OF_CONTROL=y
>  CONFIG_SPL_OF_CONTROL=y
>  CONFIG_MULTI_DTB_FIT=y
> @@ -61,10 +69,17 @@ CONFIG_SPL_DM=y
>  CONFIG_SPL_DM_SEQ_ALIAS=y
>  CONFIG_REGMAP=y
>  CONFIG_SPL_REGMAP=y
> +CONFIG_SYSCON=y
> +CONFIG_SPL_SYSCON=y
>  CONFIG_SPL_OF_TRANSLATE=y
>  CONFIG_CLK=y
>  CONFIG_SPL_CLK=y
>  CONFIG_CLK_TI_SCI=y
> +CONFIG_DFU_MMC=y
> +CONFIG_DFU_RAM=y
> +CONFIG_DFU_SF=y
> +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> +CONFIG_SYS_DFU_MAX_FILE_SIZE=0x800000
>  CONFIG_DMA_CHANNELS=y
>  CONFIG_TI_K3_NAVSS_UDMA=y
>  CONFIG_TI_SCI_PROTOCOL=y
> @@ -103,4 +118,19 @@ CONFIG_CADENCE_QSPI=y
>  CONFIG_SYSRESET=y
>  CONFIG_SPL_SYSRESET=y
>  CONFIG_SYSRESET_TI_SCI=y
> +CONFIG_USB=y
> +CONFIG_DM_USB_GADGET=y
> +CONFIG_SPL_DM_USB_GADGET=y
> +CONFIG_USB_XHCI_HCD=y
> +CONFIG_USB_XHCI_DWC3=y
> +CONFIG_USB_DWC3=y
> +CONFIG_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_AM62=y
> +CONFIG_USB_DWC3_AM62=y
> +CONFIG_USB_GADGET=y
> +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> +CONFIG_USB_GADGET_DOWNLOAD=y
>  CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
> diff --git a/configs/am62x_r5_usbdfu.config b/configs/am62x_r5_usbdfu.config
> new file mode 100644
> index 00000000000..772bb2ab935
> --- /dev/null
> +++ b/configs/am62x_r5_usbdfu.config
> @@ -0,0 +1,28 @@
> +CONFIG_SPL_ENV_SUPPORT=y
> +CONFIG_SYSCON=y
> +CONFIG_SPL_SYSCON=y
> +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> +CONFIG_MISC=y
> +CONFIG_USB=y
> +CONFIG_DM_USB_GADGET=y
> +CONFIG_SPL_DM_USB_GADGET=y
> +CONFIG_USB_DWC3=y
> +CONFIG_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_AM62=y
> +CONFIG_USB_GADGET=y
> +CONFIG_SPL_USB_GADGET=y
> +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> +CONFIG_USB_GADGET_DOWNLOAD=y
> +CONFIG_SPL_DFU=y
> +# CONFIG_SPL_MMC is not set
> +# CONFIG_SPL_FS_FAT is not set
> +# CONFIG_SPL_LIBDISK_SUPPORT is not set
> +# CONFIG_SPL_SPI is not set
> +# CONFIG_SPL_SYS_MALLOC is not set
> +# CONFIG_CMD_GPT is not set
> +# CONFIG_CMD_MMC is not set
> +# CONFIG_CMD_FAT is not set
> +# CONFIG_MMC_SDHCI is not set
> -- 
> 2.43.0