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

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@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__ */