Re: [U-Boot] [RFC PATCH 1/5] spl: dfu: add dfu support in SPL

Hi Ravi,

Thanks for adding this functionality to DFU.

Please find below organizational tips:

- Please use 'patman' (./tools/patman/patman) tool when sending patches (it adds me automatically to CC).

- Would it be hard to add SPL-DFU support also to BeagleBone Black(am335x) (BBB) board? I'm asking since BBB is far more pervasive than dra7x (and I might be able to cover SPL-DFU easily with our new ptest (DFU) test suite).

Traditionally the DFU support is available only as part 2nd stage boot loader(u-boot) and DFU is not supported in SPL.

The SPL-DFU feature is useful for boards which has only USB inteface and do not have external interface like ethernet or MMC/SD to boot the board.

This patch add DFU support in SPL to flash boot inital binary images to factory or bare-metal boards to memory devices like SPI, eMMC, MMC/SD card using USB interface.

This SPL-DFU support can be enabled through Menuconfig->Boot Images->Enable SPL-DFU support

Signed-off-by: Ravi Babu ravibabu@ti.com

Kconfig | 40 ++++++++++++++++++++++++++++++++++++++++ cmd/Makefile | 11 ++++++++++- common/Makefile | 28 ++++++++++++++++++++-------- common/command.c | 2 +- drivers/mmc/Makefile | 3 +-- drivers/mmc/mmc_private.h | 2 +- scripts/Makefile.spl | 13 +++++++++++++ 7 files changed, 86 insertions(+), 13 deletions(-)

diff --git a/Kconfig b/Kconfig index f53759a..8c033d0 100644 --- a/Kconfig +++ b/Kconfig @@ -285,6 +285,46 @@ config SPL_LOAD_FIT particular it can handle selecting from multiple device tree and passing the correct one to U-Boot.

+config SPL_DFU

• bool "Enable SPL with DFU to load binaries to bootdevices

using USB"

• depends on USB && CMD_DFU
• help

•  Normally with the SPL only image does not have capability
  

to

•  load the binaries or boot images to boot devices like
  

eMMC,SPI,etc.

•  This feature enables the DFU (Device Firmware Upgarde) in
  

SPL with

•  eMMC device as default bootdevice. The ROM code will load
  

and execute

•  the SPL/MLO dfu image. The user can flash the binaries to
  

selected

•  dfu device partition from host-pc using dfu-utils.
  

• This feature will be useful to flash the binaries to
  

factory

•  or bare-metal boards using USB interface.
  

+choice

• bool "DFU device selection"
• depends on CMD_DFU && SPL_DFU

+config SPL_DFU_EMMC

• bool "eMMC device"
• depends on CMD_DFU && SPL_DFU
• help

• select eMMC memory device for flashing binary images to
  

• the selection partition using DFU.
  

+config SPL_DFU_MMC

• bool "MMC/SD device"
• depends on CMD_DFU && SPL_DFU
• help

• select MMC/SD memory device for flashing binary images to
  

• the selection partition using DFU.
  

+config SPL_DFU_SF

• bool "SPI device"
• depends on CMD_DFU && SPL_DFU
• help

• select SPI flash memory device for flashing binary images to
  

• the selection partition using DFU.
  

+endchoice

config SYS_CLK_FREQ depends on ARC || ARCH_SUNXI int "CPU clock frequency" diff --git a/cmd/Makefile b/cmd/Makefile index f95759e..139189e 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -5,6 +5,13 @@ # SPDX-License-Identifier: GPL-2.0+ #

+CONFIG_INC_COMMON=y

I think that we should not introduce any extra "common" defines.

I'd be more than happy if you would try to implement DFU support according to SPL framework (./doc/README.spl).

As an example we could use ./common/spl/spl_{mmc|usb}.c files.

+ifdef CONFIG_SPL_BUILD +ifndef CONFIG_SPL_DFU +CONFIG_INC_COMMON=n +endif +endif

ifndef CONFIG_SPL_BUILD # core command obj-y += boot.o @@ -146,7 +153,6 @@ obj-$(CONFIG_CMD_SPL) += spl.o obj-$(CONFIG_CMD_ZIP) += zip.o obj-$(CONFIG_CMD_ZFS) += zfs.o

-obj-$(CONFIG_CMD_DFU) += dfu.o obj-$(CONFIG_CMD_GPT) += gpt.o obj-$(CONFIG_CMD_ETHSW) += ethsw.o

@@ -161,6 +167,9 @@ obj-$(CONFIG_CMD_SCSI) += scsi.o endif endif # CONFIG_SPL_BUILD

+ifeq ($(CONFIG_INC_COMMON),y) +obj-$(CONFIG_CMD_DFU) += dfu.o

It seems like other subsystems used enabled in SPL do not need to add whole commands to SPL.

It IMHO seems like an overkill to support all, full blown DFU command interface in SPL. For other IP blocks (like USB, MMC) only small subset of commands is provided.

+endif obj-$(CONFIG_CMD_BLOB) += blob.o

# core command diff --git a/common/Makefile b/common/Makefile index b23f312..3576fac 100644 --- a/common/Makefile +++ b/common/Makefile @@ -6,15 +6,31 @@ #

# core -ifndef CONFIG_SPL_BUILD -obj-y += init/ -obj-y += main.o -obj-y += exports.o

+CONFIG_INC_COMMON=y +ifdef CONFIG_SPL_BUILD +ifndef CONFIG_SPL_DFU +CONFIG_INC_COMMON=n +endif +endif

+ifeq ($(CONFIG_INC_COMMON),y) obj-y += hash.o ifdef CONFIG_SYS_HUSH_PARSER obj-y += cli_hush.o endif

+obj-y += env_attr.o +obj-y += env_callback.o +obj-y += env_flags.o +obj-y += cli.o +endif

+ifndef CONFIG_SPL_BUILD +obj-y += init/ +obj-y += main.o +obj-y += exports.o

# This option is not just y/n - it can have a numeric value ifdef CONFIG_BOOTDELAY obj-y += autoboot.o @@ -34,9 +50,6 @@ obj-$(CONFIG_DISPLAY_BOARDINFO_LATE) += board_info.o obj-$(CONFIG_CMD_BOOTM) += bootm.o bootm_os.o

# environment -obj-y += env_attr.o -obj-y += env_callback.o -obj-y += env_flags.o obj-$(CONFIG_ENV_IS_IN_DATAFLASH) += env_dataflash.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += env_eeprom.o extra-$(CONFIG_ENV_IS_EMBEDDED) += env_embedded.o @@ -153,7 +166,6 @@ endif # We always have this since drivers/ddr/fs/interactive.c needs it obj-$(CONFIG_CMDLINE) += cli_simple.o

-obj-y += cli.o obj-$(CONFIG_CMDLINE) += cli_readline.o obj-y += command.o obj-y += s_record.o diff --git a/common/command.c b/common/command.c index e5d9b9c..d1c049c 100644 --- a/common/command.c +++ b/common/command.c @@ -520,7 +520,7 @@ enum command_ret_t cmd_process(int flag, int argc, char * const argv[], if (argc > cmdtp->maxargs) rc = CMD_RET_USAGE;

-#if defined(CONFIG_CMD_BOOTD) +#if defined(CONFIG_CMD_BOOTD) && !defined(CONFIG_SPL_BUILD) /* avoid "bootd" recursion */ else if (cmdtp->cmd == do_bootd) { if (flag & CMD_FLAG_BOOTD) { diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile index 585aaf3..7abac59 100644 --- a/drivers/mmc/Makefile +++ b/drivers/mmc/Makefile @@ -43,11 +43,10 @@ obj-$(CONFIG_SPEAR_SDHCI) += spear_sdhci.o obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o obj-$(CONFIG_MMC_UNIPHIER) += uniphier-sd.o obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o +obj-$(CONFIG_GENERIC_MMC) += mmc_write.o

I'm wondering if this could be added/extended at ./common/spl/ code?

ifdef CONFIG_SPL_BUILD obj-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o -else -obj-$(CONFIG_GENERIC_MMC) += mmc_write.o endif obj-$(CONFIG_PIC32_SDHCI) += pic32_sdhci.o obj-$(CONFIG_MSM_SDHCI) += msm_sdhci.o diff --git a/drivers/mmc/mmc_private.h b/drivers/mmc/mmc_private.h index d3f6bfe..d221362 100644 --- a/drivers/mmc/mmc_private.h +++ b/drivers/mmc/mmc_private.h @@ -20,7 +20,7 @@ extern int mmc_set_blocklen(struct mmc *mmc, int len); void mmc_adapter_card_type_ident(void); #endif

-#ifndef CONFIG_SPL_BUILD +#ifdef CONFIG_GENERIC_MMC

unsigned long mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt); diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index ec8d8f1..6e9a589 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -35,6 +35,13 @@ else SPL_BIN := u-boot-spl endif

+CONFIG_INC_COMMON=y +ifdef CONFIG_SPL_BUILD +ifndef CONFIG_SPL_DFU +CONFIG_INC_COMMON=n +endif +endif

include $(srctree)/config.mk include $(srctree)/arch/$(ARCH)/Makefile

@@ -56,6 +63,12 @@ libs-y += common/init/ libs-$(CONFIG_SPL_LIBCOMMON_SUPPORT) += common/ cmd/ libs-$(CONFIG_SPL_LIBDISK_SUPPORT) += disk/ libs-y += drivers/ +ifeq ($(CONFIG_INC_COMMON),y) +libs-y += drivers/dfu/ +libs-y += drivers/usb/gadget/ +libs-y += drivers/usb/gadget/udc/ +libs-y += drivers/usb/dwc3/ +endif libs-y += dts/ libs-y += fs/ libs-$(CONFIG_SPL_LIBGENERIC_SUPPORT) += lib/