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

Lukasz Majewski l.majewski at samsung.com
Mon May 30 13:54:00 CEST 2016


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 at 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/



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group


More information about the U-Boot mailing list