[PATCH v4 03/16] common: update: add a generic interface for FIT image

[Heinrich Schuchardt]

On 22.07.20 08:05, AKASHI Takahiro wrote:
> The main purpose of this patch is to separate a generic interface for
> updating firmware using DFU drivers from "auto-update" via tftp.
>
> This function will also be used in implementing UEFI capsule update
> in a later commit.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
>  common/Kconfig      | 14 +++++++++
>  common/Makefile     |  3 +-
>  common/update.c     | 71 +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/dfu/Kconfig |  1 +
>  include/image.h     | 12 ++++++++
>  5 files changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/common/Kconfig b/common/Kconfig
> index ca42ba37b726..86568dec2e25 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -1014,6 +1014,20 @@ endmenu
>
>  menu "Update support"
>
> +config UPDATE_COMMON
> +	bool
> +	default n
> +	select DFU_ALT

Why do we need separate symbols DFU_ALT and DFU_COMMON?

> +
> +config UPDATE_FIT
> +	bool "Firmware update using fitImage"
> +	depends on FIT
> +	depends on DFU
> +	select UPDATE_COMMON
> +	help
> +	  This option allows performing update of DFU-capable storage with
> +	  data in fitImage.
> +
>  config ANDROID_AB
>  	bool "Android A/B updates"
>  	default n
> diff --git a/common/Makefile b/common/Makefile
> index 2e7a090588d9..bcf352d01652 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -53,8 +53,7 @@ obj-$(CONFIG_LCD_ROTATION) += lcd_console_rotation.o
>  obj-$(CONFIG_LCD_DT_SIMPLEFB) += lcd_simplefb.o
>  obj-$(CONFIG_LYNXKDI) += lynxkdi.o
>  obj-$(CONFIG_MENU) += menu.o
> -obj-$(CONFIG_UPDATE_TFTP) += update.o
> -obj-$(CONFIG_DFU_TFTP) += update.o
> +obj-$(CONFIG_UPDATE_COMMON) += update.o
>  obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
>  obj-$(CONFIG_CMDLINE) += cli_readline.o cli_simple.o
>
> diff --git a/common/update.c b/common/update.c
> index f82d77cc0be9..2c75b37f19e6 100644
> --- a/common/update.c
> +++ b/common/update.c
> @@ -23,6 +23,7 @@
>  #include <dfu.h>
>  #include <errno.h>
>
> +#ifdef CONFIG_DFU_TFTP
>  /* env variable holding the location of the update file */
>  #define UPDATE_FILE_ENV		"updatefile"
>
> @@ -89,6 +90,7 @@ static int update_load(char *filename, ulong msec_max, int cnt_max, ulong addr)
>
>  	return rv;
>  }
> +#endif /* CONFIG_DFU_TFTP */
>
>  static int update_fit_getparams(const void *fit, int noffset, ulong *addr,
>  						ulong *fladdr, ulong *size)
> @@ -106,6 +108,7 @@ static int update_fit_getparams(const void *fit, int noffset, ulong *addr,
>  	return 0;
>  }
>
> +#ifdef CONFIG_DFU_TFTP
>  int update_tftp(ulong addr, char *interface, char *devstring)
>  {
>  	char *filename, *env_addr, *fit_image_name;
> @@ -194,3 +197,71 @@ next_node:
>
>  	return ret;
>  }
> +#endif /* CONFIG_DFU_UPDATE */

Why do we need all those #ifdef? The linker removes all unused functions.

We should move update_tftp() to drivers/dfu/dfu_tftp.c

Best regards

Heinrich

> +
> +#ifdef CONFIG_UPDATE_FIT
> +/**
> + * fit_update - update storage with FIT image
> + * @fit:	Pointer to FIT image
> + *
> + * Update firmware on storage using FIT image as input.
> + * The storage area to be update will be identified by the name
> + * in FIT and matching it to "dfu_alt_info" variable.
> + *
> + * Return:      0 - on success, non-zero - otherwise
> + */
> +int fit_update(const void *fit)
> +{
> +	char *fit_image_name;
> +	ulong update_addr, update_fladdr, update_size;
> +	int images_noffset, ndepth, noffset;
> +	int ret = 0;
> +
> +	if (!fit)
> +		return -EINVAL;
> +
> +	if (!fit_check_format((void *)fit)) {
> +		printf("Bad FIT format of the update file, aborting auto-update\n");
> +		return -EINVAL;
> +	}
> +
> +	/* process updates */
> +	images_noffset = fdt_path_offset(fit, FIT_IMAGES_PATH);
> +
> +	ndepth = 0;
> +	noffset = fdt_next_node(fit, images_noffset, &ndepth);
> +	while (noffset >= 0 && ndepth > 0) {
> +		if (ndepth != 1)
> +			goto next_node;
> +
> +		fit_image_name = (char *)fit_get_name(fit, noffset, NULL);
> +		printf("Processing update '%s' :", fit_image_name);
> +
> +		if (!fit_image_verify(fit, noffset)) {
> +			printf("Error: invalid update hash, aborting\n");
> +			ret = 1;
> +			goto next_node;
> +		}
> +
> +		printf("\n");
> +		if (update_fit_getparams(fit, noffset, &update_addr,
> +					 &update_fladdr, &update_size)) {
> +			printf("Error: can't get update parameters, aborting\n");
> +			ret = 1;
> +			goto next_node;
> +		}
> +
> +		if (fit_image_check_type(fit, noffset, IH_TYPE_FIRMWARE)) {
> +			ret = dfu_write_by_name(fit_image_name,
> +						(void *)update_addr,
> +						update_size, NULL, NULL);
> +			if (ret)
> +				return ret;
> +		}
> +next_node:
> +		noffset = fdt_next_node(fit, noffset, &ndepth);
> +	}
> +
> +	return ret;
> +}
> +#endif /* CONFIG_UPDATE_FIT */
> diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig
> index d680b28ecf51..df0585c4fc83 100644
> --- a/drivers/dfu/Kconfig
> +++ b/drivers/dfu/Kconfig
> @@ -22,6 +22,7 @@ config DFU_TFTP
>  	bool "DFU via TFTP"
>  	select DFU_ALT
>  	select DFU_OVER_TFTP
> +	select UPDATE_COMMON
>  	help
>  	  This option allows performing update of DFU-managed medium with data
>  	  sent via TFTP boot.
> diff --git a/include/image.h b/include/image.h
> index 9a5a87dbf870..dce2997f9a6a 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1592,4 +1592,16 @@ struct fit_loadable_tbl {
>  		.handler = _handler, \
>  	}
>
> +/**
> + * fit_update - update storage with FIT image
> + * @fit:        Pointer to FIT image
> + *
> + * Update firmware on storage using FIT image as input.
> + * The storage area to be update will be indentified by the name
> + * in FIT and matching it to "dfu_alt_info" variable.
> + *
> + * Return:      0 on success, non-zero otherwise
> + */
> +int fit_update(const void *fit);
> +
>  #endif	/* __IMAGE_H__ */
>