[U-Boot] [PATCH 2/6] arm64: mvebu: Add bubt command for flash image burn

Stefan Roese sr at denx.de
Thu Nov 24 09:58:33 CET 2016


Hi Kosta,

a general comment: Please use scripts/checkpatch before sending
the patches to the list to make sure that all (mostly style
related) issues are resolved.

Please find some more mostly nitpicking comments below.

On 20.11.2016 16:38, kostap at marvell.com wrote:
> From: Konstantin Porotchkin <kostap at marvell.com>
> 
> Add support for mvebu bubt command for flash image
> load, check and burn on boot device.

Could you please extent the patch (cmd) description here a bit?
Perhaps by adding an example on how this command can be used
to load and update image (loading via tftp, buring into SPI...).
 
> Change-Id: If2b971069ae44232761b601bbbcf0162567f5603
> Signed-off-by: Konstantin Porotchkin <kostap at marvell.com>
> Cc: Stefan Roese <sr at denx.de>
> Cc: Nadav Haklai <nadavh at marvell.com>
> Cc: Neta Zur Hershkovits <neta at marvell.com>
> Cc: Omri Itach <omrii at marvell.com>
> Cc: Igal Liberman <igall at marvell.com>
> Cc: Haim Boot <hayim at marvell.com>
> Cc: Hanna Hawa <hannah at marvell.com>
> ---
>  arch/arm/mach-mvebu/Kconfig |  30 ++
>  cmd/Kconfig                 |   3 +
>  cmd/Makefile                |   2 +
>  cmd/mvebu/Kconfig           |  10 +
>  cmd/mvebu/Makefile          |  19 ++
>  cmd/mvebu/bubt.c            | 699 ++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 763 insertions(+)
>  create mode 100644 cmd/mvebu/Kconfig
>  create mode 100644 cmd/mvebu/Makefile
>  create mode 100644 cmd/mvebu/bubt.c
> 
> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> index 7248fd7..6de85d5 100644
> --- a/arch/arm/mach-mvebu/Kconfig
> +++ b/arch/arm/mach-mvebu/Kconfig
> @@ -99,6 +99,36 @@ config TARGET_THEADORABLE
>  
>  endchoice
>  
> +choice
> +	prompt "Flash for image"
> +	default MVEBU_SPI_BOOT
> +	depends on (TARGET_MVEBU_DB_88F3720 || TARGET_MVEBU_ARMADA_8K)
> +
> +config MVEBU_NAND_BOOT
> +	bool "NAND flash boot"
> +	depends on NAND_PXA3XX
> +	help
> +	  Enable boot from NAND
> +
> +config MVEBU_SPI_BOOT
> +	bool "SPI flash boot"
> +	depends on SPI_FLASH
> +
> +config MVEBU_MMC_BOOT
> +	bool "eMMC flash boot"
> +	depends on MVEBU_MMC
> +	help
> +	  Enable boot from eMMC boot partition
> +
> +endchoice
> +
> +config MVEBU_UBOOT_DFLT_NAME
> +	string "Default image name for bubt command"
> +	default "flash-image.bin"
> +	help
> +	   This option should contain a default file name to be used with
> +	   MVEBU "bubt" command if the source file name is omitted
> +

I'm not sure if these Kconfig options really below in this file - please
see below...

>  config SYS_BOARD
>  	default "clearfog" if TARGET_CLEARFOG
>  	default "mvebu_db-88f3720" if TARGET_MVEBU_DB_88F3720
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index e339d86..39dd0a0 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -610,6 +610,9 @@ config CMD_QFW
>  	  This provides access to the QEMU firmware interface.  The main
>  	  feature is to allow easy loading of files passed to qemu-system
>  	  via -kernel / -initrd
> +
> +source "cmd/mvebu/Kconfig"
> +
>  endmenu
>  
>  config CMD_BOOTSTAGE
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 9c9a9d1..34bc544 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -163,3 +163,5 @@ obj-$(CONFIG_CMD_BLOB) += blob.o
>  
>  # core command
>  obj-y += nvedit.o
> +
> +obj-$(CONFIG_ARCH_MVEBU) += mvebu/

Do you plan to add move mvebu specific commands?

> diff --git a/cmd/mvebu/Kconfig b/cmd/mvebu/Kconfig
> new file mode 100644
> index 0000000..e7fbd20
> --- /dev/null
> +++ b/cmd/mvebu/Kconfig
> @@ -0,0 +1,10 @@
> +menu "MVEBU commands"
> +depends on ARCH_MVEBU
> +
> +config CMD_MVEBU_BUBT
> +	bool "bubt"
> +	default n
> +	help
> +	  bubt - Burn a u-boot image to flash
> +
> +endmenu

Here you introduce a new Kconfig file. Wouldn't it be better, to
move all Kconfig option into this file instead of having most
of them in the mach-mvebu file?

> diff --git a/cmd/mvebu/Makefile b/cmd/mvebu/Makefile
> new file mode 100644
> index 0000000..b0817e3
> --- /dev/null
> +++ b/cmd/mvebu/Makefile
> @@ -0,0 +1,19 @@
> +#
> +# ***************************************************************************
> +# Copyright (C) 2015 Marvell International Ltd.
> +# ***************************************************************************
> +# This program is free software: you can redistribute it and/or modify it
> +# under the terms of the GNU General Public License as published by the Free
> +# Software Foundation, either version 2 of the License, or any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +# ***************************************************************************
> +#

As already mentioned by Simon, please move to using SPDX
license headers. You can and should of course keep your copyright
notice. Please fix this globally. For SPDX in U-Boot I suggest
to take a look at this page:

http://www.denx.de/wiki/U-Boot/Licensing

> +
> +obj-$(CONFIG_CMD_MVEBU_BUBT) += bubt.o
> diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
> new file mode 100644
> index 0000000..6539063
> --- /dev/null
> +++ b/cmd/mvebu/bubt.c
> @@ -0,0 +1,699 @@
> +/*
> + * ***************************************************************************
> + * Copyright (C) 2015 Marvell International Ltd.
> + * ***************************************************************************
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation, either version 2 of the License, or any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + * ***************************************************************************
> + */
> +
> +#include <config.h>
> +#include <common.h>
> +#include <command.h>
> +#include <vsprintf.h>
> +#include <errno.h>
> +#include <dm.h>
> +
> +#include <spi_flash.h>
> +#include <spi.h>
> +#include <nand.h>
> +#include <usb.h>
> +#include <fs.h>
> +#include <mmc.h>
> +#include <u-boot/sha1.h>
> +#include <u-boot/sha256.h>
> +
> +#if defined(CONFIG_TARGET_MVEBU_ARMADA_8K)
> +#define MAIN_HDR_MAGIC		0xB105B002

As already mentioned by Simon, please move to using lower
case characters for hex numbers.

> +struct mvebu_image_header {
> +	uint32_t	magic;			/*  0-3  */
> +	uint32_t	prolog_size;		/*  4-7  */
> +	uint32_t	prolog_checksum;	/*  8-11 */
> +	uint32_t	boot_image_size;	/* 12-15 */
> +	uint32_t	boot_image_checksum;	/* 16-19 */
> +	uint32_t	rsrvd0;			/* 20-23 */
> +	uint32_t	load_addr;		/* 24-27 */
> +	uint32_t	exec_addr;		/* 28-31 */
> +	uint8_t		uart_cfg;		/*  32   */
> +	uint8_t		baudrate;		/*  33   */
> +	uint8_t		ext_count;		/*  34   */
> +	uint8_t		aux_flags;		/*  35   */
> +	uint32_t	io_arg_0;		/* 36-39 */
> +	uint32_t	io_arg_1;		/* 40-43 */
> +	uint32_t	io_arg_2;		/* 43-47 */
> +	uint32_t	io_arg_3;		/* 48-51 */
> +	uint32_t	rsrvd1;			/* 52-55 */
> +	uint32_t	rsrvd2;			/* 56-59 */
> +	uint32_t	rsrvd3;			/* 60-63 */
> +};
> +#elif defined(CONFIG_TARGET_MVEBU_DB_88F3720)	/* A3700 */
> +#define HASH_SUM_LEN		16
> +#define IMAGE_VERSION_3_6_0	0x030600
> +#define IMAGE_VERSION_3_5_0	0x030500
> +
> +struct common_tim_data {
> +	u32	version;
> +	u32	identifier;
> +	u32	trusted;
> +	u32	issue_date;
> +	u32	oem_unique_id;
> +	u32	reserved[5];		/* Reserve 20 bytes */
> +	u32	boot_flash_sign;
> +	u32	num_images;
> +	u32	num_keys;
> +	u32	size_of_reserved;
> +};
> +
> +struct mvebu_image_info {
> +	u32	image_id;
> +	u32	next_image_id;
> +	u32	flash_entry_addr;
> +	u32	load_addr;
> +	u32	image_size;
> +	u32	image_size_to_hash;
> +	u32	hash_algorithm_id;
> +	u32	hash[HASH_SUM_LEN];		/* Reserve 512 bits for the hash */
> +	u32	partition_number;
> +	u32	enc_algorithm_id;
> +	u32	encrypt_start_offset;
> +	u32	encrypt_size;
> +};
> +#endif
> +
> +struct bubt_dev {
> +	char name[8];
> +	size_t (*read)(const char *file_name);
> +	int (*write)(size_t image_size);
> +	int (*active)(void);
> +};
> +
> +static ulong get_load_addr(void)
> +{
> +	const char *addr_str;
> +	unsigned long addr;
> +
> +	addr_str = getenv("loadaddr");
> +	if (addr_str != NULL)
> +		addr = simple_strtoul(addr_str, NULL, 16);
> +	else
> +		addr = CONFIG_SYS_LOAD_ADDR;
> +
> +	return addr;
> +}
> +
> +/********************************************************************
> + *     eMMC services
> + ********************************************************************/
> +#ifdef CONFIG_DM_MMC
> +static int mmc_burn_image(size_t image_size)
> +{
> +	struct mmc	*mmc;
> +	lbaint_t	start_lba;
> +	lbaint_t	blk_count;
> +	ulong		blk_written;
> +#ifdef CONFIG_SYS_MMC_ENV_DEV
> +	const u8	mmc_dev_num = CONFIG_SYS_MMC_ENV_DEV;
> +#else
> +	const u8	mmc_dev_num = 0;
> +#endif

I suggest to move this one up in this file to make the code look a bit
"cleaner", like this:

#ifndef CONFIG_SYS_MMC_ENV_DEV
#define CONFIG_SYS_MMC_ENV_DEV	0
#endif

Then you can remove the #ifdef from the code here.

> +
> +	mmc = find_mmc_device(mmc_dev_num);
> +	if (!mmc) {
> +		printf("No SD/MMC/eMMC card found\n");
> +		return 1;
> +	}

Does this bubt command only support eMMC or SD/MMC as well? If so,
please mention this in the Kconfig options too, as they only list
eMMC (IIRC).

> +
> +	if (mmc_init(mmc)) {
> +		printf("%s(%d) init failed\n", IS_SD(mmc) ? "SD" : "MMC", mmc_dev_num);
> +		return 1;
> +	}
> +
> +#ifdef CONFIG_SYS_MMC_ENV_PART
> +	if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num) {
> +		if (mmc_switch_part(mmc_dev_num, CONFIG_SYS_MMC_ENV_PART)) {
> +			printf("MMC partition switch failed\n");
> +			return 1;
> +		}
> +	}
> +#endif
> +
> +	/* SD reserves LBA-0 for MBR and boots from LBA-1, MMC/eMMC boots from LBA-0 */
> +	start_lba = IS_SD(mmc) ? 1 : 0;
> +	blk_count = image_size / mmc->block_dev.blksz;
> +	if (image_size % mmc->block_dev.blksz)
> +		blk_count += 1;
> +
> +	blk_written = mmc->block_dev.block_write(mmc_dev_num,
> +						start_lba, blk_count, (void *)get_load_addr());
> +	if (blk_written != blk_count) {
> +		printf("Error - written %#lx blocks\n", blk_written);
> +		return 1;
> +	} else {
> +		printf("Done!\n");
> +	}
> +
> +#ifdef CONFIG_SYS_MMC_ENV_PART
> +	if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num)
> +		mmc_switch_part(mmc_dev_num, mmc->part_num);
> +#endif
> +
> +	return 0;
> +}
> +
> +static size_t mmc_read_file(const char *file_name)
> +{
> +	loff_t act_read = 0;
> +	int rc;
> +	struct mmc	*mmc;
> +#ifdef CONFIG_SYS_MMC_ENV_DEV
> +	const u8	mmc_dev_num = CONFIG_SYS_MMC_ENV_DEV;
> +#else
> +	const u8	mmc_dev_num = 0;
> +#endif

Again, please use the construct suggested above.

> +	mmc = find_mmc_device(mmc_dev_num);
> +	if (!mmc) {
> +		printf("No SD/MMC/eMMC card found\n");
> +		return 0;
> +	}
> +
> +	if (mmc_init(mmc)) {
> +		printf("%s(%d) init failed\n", IS_SD(mmc) ? "SD" : "MMC", mmc_dev_num);
> +		return 0;
> +	}
> +
> +	/* Load from data partition (0) */
> +	if (fs_set_blk_dev("mmc", "0", FS_TYPE_ANY)) {
> +		printf("Error: MMC 0 not found\n");
> +		return 0;
> +	}
> +
> +	/* Perfrom file read */
> +	rc = fs_read(file_name, get_load_addr(), 0, 0, &act_read);
> +	if (rc)
> +		return 0;
> +
> +	return act_read;
> +}
> +
> +int is_mmc_active(void)
> +{
> +	return 1;
> +}
> +#else /* CONFIG_DM_MMC */
> +#define mmc_burn_image 0
> +#define mmc_read_file 0
> +#define is_mmc_active 0

Please use empty functions instead.

> +#endif /* CONFIG_DM_MMC */
> +
> +
> +/********************************************************************
> + *     SPI services
> + ********************************************************************/
> +#ifdef CONFIG_SPI_FLASH
> +static int spi_burn_image(size_t image_size)
> +{
> +	int ret;
> +	struct spi_flash *flash;
> +	uint32_t erase_bytes;
> +
> +	/* Probe the SPI bus to get the flash device */
> +	flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
> +				CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE);
> +	if (!flash) {
> +		printf("Failed to probe SPI Flash\n");
> +		return 1;
> +	}
> +
> +#ifdef CONFIG_SPI_FLASH_PROTECTION
> +	spi_flash_protect(flash, 0);
> +#endif
> +	erase_bytes = image_size + (flash->erase_size - image_size % flash->erase_size);
> +	printf("Erasing %d bytes (%d blocks) at offset 0 ...", erase_bytes, erase_bytes/flash->erase_size);
> +	ret = spi_flash_erase(flash, 0, erase_bytes);
> +	if (ret)
> +		printf("Error!\n");
> +	else
> +		printf("Done!\n");
> +
> +	printf("Writing %d bytes from 0x%lx to offset 0 ...", (int)image_size, get_load_addr());
> +	ret = spi_flash_write(flash, 0, image_size, (void *)get_load_addr());
> +	if (ret)
> +		printf("Error!\n");
> +	else
> +		printf("Done!\n");
> +
> +#ifdef CONFIG_SPI_FLASH_PROTECTION
> +	spi_flash_protect(flash, 1);
> +#endif
> +
> +	return ret;
> +}
> +
> +int is_spi_active(void)
> +{
> +	return 1;
> +}
> +#else /* CONFIG_SPI_FLASH */
> +#define spi_burn_image 0
> +#define is_spi_active 0

Empty functions please.

> +#endif /* CONFIG_SPI_FLASH */
> +
> +/********************************************************************
> + *     NAND services
> + ********************************************************************/
> +#ifdef CONFIG_CMD_NAND
> +static int nand_burn_image(size_t image_size)
> +{
> +	int ret, block_size;
> +	nand_info_t *nand;
> +	int dev = nand_curr_device;
> +
> +	if ((dev < 0) || (dev >= CONFIG_SYS_MAX_NAND_DEVICE) ||
> +	    (!nand_info[dev].name)) {
> +		puts("\nno devices available\n");
> +		return 1;
> +	}
> +	nand = &nand_info[dev];
> +	block_size = nand->erasesize;
> +
> +	/* Align U-Boot size to currently used blocksize */
> +	image_size = ((image_size + (block_size - 1)) & (~(block_size-1)));

checkpatch will most likely complain about the missing space
before and after the "-" above.

> +
> +	/* Erase the U-BOOT image space */
> +	printf("Erasing 0x%x - 0x%x:...", 0, (int)image_size);
> +	ret = nand_erase(nand, 0, image_size);
> +	if (ret) {
> +		printf("Error!\n");
> +		goto error;
> +	}
> +	printf("Done!\n");
> +
> +	/* Write the image to flash */
> +	printf("Writing image:...");
> +	printf("&image_size = 0x%p\n", (void*)&image_size);
> +	ret = nand_write(nand, 0, &image_size, (void *)get_load_addr());
> +	if (ret)
> +		printf("Error!\n");
> +	else
> +		printf("Done!\n");
> +
> +error:
> +	return ret;
> +}
> +
> +int is_nand_active(void)
> +{
> +	return 1;
> +}
> +#else /* CONFIG_CMD_NAND */
> +#define nand_burn_image 0
> +#define is_nand_active 0

Empty functions please.

> +#endif /* CONFIG_CMD_NAND */
> +
> +/********************************************************************
> + *     USB services
> + ********************************************************************/
> +#if defined(CONFIG_USB_STORAGE) && defined (CONFIG_BLK)
> +static size_t usb_read_file(const char *file_name)
> +{
> +	loff_t act_read = 0;
> +	struct udevice *dev;
> +	int rc;
> +
> +	usb_stop();
> +
> +	if (usb_init() < 0) {
> +		printf("Error: usb_init failed\n");
> +		return 0;
> +	}
> +
> +	/* Try to recognize storage devices immediately */
> +	blk_first_device(IF_TYPE_USB, &dev);
> +	if (dev == NULL) {
> +		printf("Error: USB storage device not found\n");
> +		return 0;
> +	}
> +
> +	/* Always load from usb 0 */
> +	if (fs_set_blk_dev("usb", "0", FS_TYPE_ANY)) {
> +		printf("Error: USB 0 not found\n");
> +		return 0;
> +	}
> +
> +	/* Perfrom file read */
> +	rc = fs_read(file_name, get_load_addr(), 0, 0, &act_read);
> +	if (rc)
> +		return 0;
> +
> +	return act_read;
> +}
> +
> +int is_usb_active(void)
> +{
> +	return 1;
> +}
> +#else /* defined(CONFIG_USB_STORAGE) && defined (CONFIG_BLK) */
> +#define usb_read_file 0
> +#define is_usb_active 0
> +#endif /* defined(CONFIG_USB_STORAGE) && defined (CONFIG_BLK) */
> +
> +/********************************************************************
> + *     Network services
> + ********************************************************************/
> +#ifdef CONFIG_CMD_NET
> +static size_t tftp_read_file(const char *file_name)
> +{
> +	/* update global variable load_addr before tftp file from network */
> +	load_addr = get_load_addr();
> +	return net_loop(TFTPGET);
> +}
> +
> +int is_tftp_active(void)
> +{
> +	return 1;
> +}
> +#else
> +#define tftp_read_file 0
> +#define is_tftp_active 0
> +#endif /* CONFIG_CMD_NET */
> +
> +
> +enum bubt_devices {
> +	BUBT_DEV_NET = 0,
> +	BUBT_DEV_USB,
> +	BUBT_DEV_MMC,
> +	BUBT_DEV_SPI,
> +	BUBT_DEV_NAND,
> +
> +	BUBT_MAX_DEV
> +};
> +
> +struct bubt_dev bubt_devs[BUBT_MAX_DEV] = {
> +	{"tftp", tftp_read_file, NULL, is_tftp_active},
> +	{"usb",  usb_read_file,  NULL, is_usb_active},
> +	{"mmc",  mmc_read_file,  mmc_burn_image, is_mmc_active},
> +	{"spi",  NULL, spi_burn_image,  is_spi_active},
> +	{"nand", NULL, nand_burn_image, is_nand_active},
> +};
> +
> +static int bubt_write_file(struct bubt_dev *dst, size_t image_size)
> +{
> +	if (!dst->write) {
> +		printf("Error: Write not supported on device %s\n", dst->name);
> +		return 1;
> +	}
> +
> +	return dst->write(image_size);
> +}
> +
> +#if defined(CONFIG_TARGET_MVEBU_ARMADA_8K)
> +uint32_t do_checksum32(uint32_t *start, int32_t len)
> +{
> +	uint32_t sum = 0;
> +	uint32_t *startp = start;
> +
> +	do {
> +		sum += *startp;
> +		startp++;
> +		len -= 4;
> +	} while (len > 0);
> +
> +	return sum;
> +}
> +
> +static int check_image_header(void)
> +{
> +	struct mvebu_image_header *hdr = (struct mvebu_image_header *)get_load_addr();
> +	uint32_t header_len = hdr->prolog_size;
> +	uint32_t checksum;
> +	uint32_t checksum_ref = hdr->prolog_checksum;
> +
> +	/*
> +	 * For now compare checksum, and magic. Later we can
> +	 * verify more stuff on the header like interface type, etc
> +	 */
> +	if (hdr->magic != MAIN_HDR_MAGIC) {
> +		printf("ERROR: Bad MAGIC 0x%08x != 0x%08x\n", hdr->magic, MAIN_HDR_MAGIC);
> +		return -ENOEXEC;
> +	}
> +
> +	/* The checksum value is discarded from checksum calculation */
> +	hdr->prolog_checksum = 0;
> +
> +	checksum = do_checksum32((uint32_t *)hdr, header_len);
> +	if (checksum != checksum_ref) {
> +		printf("Error: Bad Image checksum. 0x%x != 0x%x\n", checksum, checksum_ref);
> +		return -ENOEXEC;
> +	}
> +
> +	/* Restore the checksum before writing */
> +	hdr->prolog_checksum = checksum_ref;
> +	printf("Image checksum...OK!\n");
> +
> +	return 0;
> +}
> +#elif defined(CONFIG_TARGET_MVEBU_DB_88F3720) /* Armada 3700 */
> +static int check_image_header(void)
> +{
> +	struct common_tim_data *hdr = (struct common_tim_data *)get_load_addr();
> +	int image_num;
> +	u8 hash_160_output[SHA1_SUM_LEN];
> +	u8 hash_256_output[SHA256_SUM_LEN];
> +	sha1_context hash1_text;
> +	sha256_context hash256_text;
> +	u8 *hash_output;
> +	u32 hash_algorithm_id;
> +	u32 image_size_to_hash;
> +	u32 flash_entry_addr;
> +	u32 *hash_value;
> +	u32 internal_hash[HASH_SUM_LEN];
> +	const uint8_t *buff;
> +	u32 num_of_image = hdr->num_images;
> +	u32 version = hdr->version;
> +	u32 trusted = hdr->trusted;
> +
> +	/* bubt checksum validation only supports nontrusted images */
> +	if (trusted == 1) {
> +		printf("bypass image validation, only untrusted image is supported now\n");
> +		return 0;
> +	}
> +	/* only supports image version 3.5 and 3.6 */
> +	if (version != IMAGE_VERSION_3_5_0 && version != IMAGE_VERSION_3_6_0) {
> +		printf("Error: Unsupported Image version = 0x%08x\n", version);
> +		return -ENOEXEC;
> +	}
> +	/* validate images hash value */
> +	for (image_num = 0; image_num < num_of_image; image_num++) {
> +		struct mvebu_image_info *info = (struct mvebu_image_info *)(get_load_addr()
> +			     + sizeof(struct common_tim_data) + image_num * sizeof(struct mvebu_image_info));
> +		hash_algorithm_id = info->hash_algorithm_id;
> +		image_size_to_hash = info->image_size_to_hash;
> +		flash_entry_addr = info->flash_entry_addr;
> +		hash_value = info->hash;
> +		buff = (const uint8_t *)(get_load_addr() + flash_entry_addr);
> +
> +		if (image_num == 0) {
> +			/* The first image includes hash values in itself content. For hash calculation, we need
> +			 * to save original hash values to local variable that will be copied back for comparsion
> +			 * and set all zeros to replace orignal hash values to calculate its new hash value.
> +			 * First image original format : x...x (datum1) x...x(original hash values) x...x(datum2)
> +			 * Replaced first image format : x...x (datum1) 0...0(hash values) x...x(datum2)
> +			 */

Multicomment style is:

/*
 *
 */

And please note the 80 colums limit.

> +			memcpy(internal_hash, hash_value, sizeof(internal_hash));
> +			memset(hash_value, 0, sizeof(internal_hash));
> +		}
> +		if (image_size_to_hash == 0) {
> +			printf("Warning: Image_%d hash checksum is disable, skip the image validation.\n", image_num);
> +			continue;
> +		}
> +		switch (hash_algorithm_id) {
> +		case SHA1_SUM_LEN:
> +			sha1_starts(&hash1_text);
> +			sha1_update(&hash1_text, buff, image_size_to_hash);
> +			sha1_finish(&hash1_text, hash_160_output);
> +			hash_output = hash_160_output;
> +			break;
> +		case SHA256_SUM_LEN:
> +			sha256_starts(&hash256_text);
> +			sha256_update(&hash256_text, buff, image_size_to_hash);
> +			sha256_finish(&hash256_text, hash_256_output);
> +			hash_output = hash_256_output;
> +			break;
> +		default:
> +			printf("Error: Unsupported hash_algorithm_id = %d\n", hash_algorithm_id);
> +			return -ENOEXEC;
> +		}
> +		if (image_num == 0)
> +			memcpy(hash_value, internal_hash, sizeof(internal_hash));
> +		if (memcmp(hash_value, hash_output, hash_algorithm_id) != 0) {
> +			printf("Error: Image_%d checksum is not correct\n", image_num);
> +			return -ENOEXEC;
> +		}
> +	}
> +	printf("Image checksum...OK!\n");
> +	return 0;
> +}
> +#else
> +static int check_image_header(void)
> +{
> +	printf("bubt cmd does not support this SoC device or family!\n");
> +	return -ENOEXEC;
> +}
> +#endif
> +
> +static int bubt_verify(size_t image_size)
> +{
> +
> +	/* Check a correct image header exists */
> +	if (check_image_header()) {
> +		printf("Error: Image header verification failed\n");
> +		return 1;
> +	}
> +	return 0;
> +}
> +
> +
> +static int bubt_read_file(struct bubt_dev *src)
> +{
> +	size_t image_size;
> +
> +	if (!src->read) {
> +		printf("Error: Read not supported on device \"%s\"\n", src->name);
> +		return 0;
> +	}
> +
> +	image_size = src->read(net_boot_file_name);
> +	if (image_size <= 0) {
> +		printf("Error: Failed to read file %s from %s\n", net_boot_file_name, src->name);
> +		return 0;
> +	}
> +
> +	return image_size;
> +}
> +
> +static int bubt_is_dev_active(struct bubt_dev *dev)
> +{
> +	if (!dev->active) {
> +		printf("Device \"%s\" not supported by U-BOOT image\n", dev->name);
> +		return 0;
> +	}
> +
> +	if (!dev->active()) {
> +		printf("Device \"%s\" is inactive\n", dev->name);
> +		return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +struct bubt_dev *find_bubt_dev(char *dev_name)
> +{
> +	int dev;
> +
> +	for (dev = 0; dev < BUBT_MAX_DEV; dev++) {
> +		if (strcmp(bubt_devs[dev].name, dev_name) == 0)
> +			return &bubt_devs[dev];
> +	}
> +
> +	return 0;
> +}
> +
> +#define DEFAULT_BUBT_SRC "tftp"
> +
> +#ifndef DEFAULT_BUBT_DST
> +#ifdef CONFIG_MVEBU_SPI_BOOT
> +#define DEFAULT_BUBT_DST "spi"
> +#elif defined(CONFIG_MVEBU_NAND_BOOT)
> +#define DEFAULT_BUBT_DST "nand"
> +#elif defined(CONFIG_MVEBU_MMC_BOOT)
> +#define DEFAULT_BUBT_DST "mmc"
> +else
> +#define DEFAULT_BUBT_DST "error"
> +#endif
> +#endif /* DEFAULT_BUBT_DST */
> +
> +int do_bubt_cmd(cmd_tbl_t *cmdtp, int flag, int argc,
> +			char * const argv[])
> +{
> +	struct bubt_dev *src, *dst;
> +	size_t image_size;
> +	char src_dev_name[8];
> +	char dst_dev_name[8];
> +	char *name;
> +
> +	if (argc < 2)
> +		copy_filename(net_boot_file_name, CONFIG_MVEBU_UBOOT_DFLT_NAME, sizeof(net_boot_file_name));
> +	else
> +		copy_filename(net_boot_file_name, argv[1], sizeof(net_boot_file_name));
> +
> +	if (argc >= 3) {
> +		strncpy(dst_dev_name, argv[2], 8);
> +	} else {
> +		name = DEFAULT_BUBT_DST;
> +		strncpy(dst_dev_name, name, 8);
> +	}
> +
> +	if (argc >= 4)
> +		strncpy(src_dev_name, argv[3], 8);
> +	else
> +		strncpy(src_dev_name, DEFAULT_BUBT_SRC, 8);
> +
> +	/* Figure out the destination device */
> +	dst = find_bubt_dev(dst_dev_name);
> +	if (!dst) {
> +		printf("Error: Unknown destination \"%s\"\n", dst_dev_name);
> +		return 1;
> +	}
> +
> +	if (!bubt_is_dev_active(dst))
> +		return 1;
> +
> +	/* Figure out the source device */
> +	src = find_bubt_dev(src_dev_name);
> +	if (!src) {
> +		printf("Error: Unknown source \"%s\"\n", src_dev_name);
> +		return 1;
> +	}
> +
> +	if (!bubt_is_dev_active(src))
> +		return 1;
> +
> +	printf("Burning U-BOOT image \"%s\" from \"%s\" to \"%s\"\n", net_boot_file_name, src->name, dst->name);
> +
> +	image_size = bubt_read_file(src);
> +	if (!image_size)
> +		return 1;
> +
> +	if (bubt_verify(image_size))
> +		return 1;
> +
> +	if (bubt_write_file(dst, image_size))
> +		return 1;
> +
> +	return 0;
> +}
> +
> +U_BOOT_CMD(
> +	bubt, 4, 0, do_bubt_cmd,
> +	"Burn a u-boot image to flash",
> +	"[file-name] [destination [source]]\n"
> +	"\t-file-name     The image file name to burn. Default = u-boot.bin\n"
> +	"\t-destination   Flash to burn to [spi, nand, mmc]. Default = active boot device\n"
> +	"\t-source        The source to load image from [tftp, usb, mmc]. Default = tftp\n"
> +	"Examples:\n"
> +	"\tbubt - Burn flash-image.bin from tftp to active boot device\n"
> +	"\tbubt flash-image-new.bin nand - Burn flash-image-new.bin from tftp to NAND flash\n"
> +	"\tbubt backup-flash-image.bin mmc usb - Burn backup-flash-image.bin from usb to MMC\n"

Ah, here you have some nice examples. Please add at least one best
with a log from running on a board to the commit text as
mentioned above.

Thanks,
Stefan


More information about the U-Boot mailing list