Re: [PATCH 3/5] FWU: mtd: Add helper functions for accessing FWU metadata

On 7/22/22 19:43, jassisinghbrar@gmail.com wrote:

From: Sughosh Ganu sughosh.ganu@linaro.org

Add helper functions needed for accessing the FWU metadata which contains information on the updatable images.

Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Signed-off-by: Jassi Brar jaswinder.singh@linaro.org

---

lib/fwu_updates/Makefile | 1 + lib/fwu_updates/fwu_mtd.c | 173 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 lib/fwu_updates/fwu_mtd.c

diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile index 1993088e5b..c9e3c06b48 100644 --- a/lib/fwu_updates/Makefile +++ b/lib/fwu_updates/Makefile @@ -5,3 +5,4 @@

obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o obj-$(CONFIG_FWU_MDATA_GPT_BLK) += fwu_gpt.o +obj-$(CONFIG_FWU_MDATA_MTD) += fwu_mtd.o diff --git a/lib/fwu_updates/fwu_mtd.c b/lib/fwu_updates/fwu_mtd.c new file mode 100644 index 0000000000..f13cb149a2 --- /dev/null +++ b/lib/fwu_updates/fwu_mtd.c @@ -0,0 +1,173 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*

• • Copyright (c) 2022, Linaro Limited
• */

+#include <dfu.h> +#include <fwu.h> +#include <fwu_mdata.h> +#include <log.h> +#include <malloc.h> +#include <mtd.h> +#include <uuid.h> +#include <vsprintf.h>

+#include <dm/ofnode.h>

Please, document functions.

You missed adding the function to an include in the series. The include changes should be in the same patch as the library functions.

+int mtd_plat_get_alt_num(efi_guid_t *image_id, int *alt_num,

• 	const char *mtd_dev, bool guid)
  

+{

• int i, nalt;
• int ret = -1;
• struct mtd_info *mtd;
• struct dfu_entity *dfu;
• ofnode node, parts_node;
• fdt_addr_t offset, size;
• char uuidbuf[UUID_STR_LEN + 1];
• mtd_probe_devices();
• mtd = get_mtd_device_nm(mtd_dev);
• /* Find partition node under given MTD device. */
• parts_node = ofnode_by_compatible(mtd_get_ofnode(mtd),

• 			  "fixed-partitions");
  

• uuid_bin_to_str(image_id->b, uuidbuf,

• 	guid ? UUID_STR_FORMAT_GUID : UUID_STR_FORMAT_STD);
  

• node = ofnode_by_prop_value(parts_node, "uuid", uuidbuf,

• 		    sizeof(uuidbuf));
  

• if (!ofnode_valid(node)) {

• log_warning("Warning: Failed to find partition by image UUID\n");
  

• return -ENOENT;
  

• }

Isn't finding an mtd partition by UUID something that should be a function in the mtd uclass because it may find usages outside FWU?

• offset = ofnode_get_addr_size_index_notrans(node, 0, &size);
• if (offset == FDT_ADDR_T_NONE || !size)

• return -ENOENT;
  

• dfu_init_env_entities(NULL, NULL);
• nalt = 0;
• list_for_each_entry(dfu, &dfu_list, list) {

• nalt++;
  

• }
• if (!nalt) {

• log_warning("No entities in dfu_alt_info\n");
  

• dfu_free_entities();
  

• return -ENOENT;
  

• }
• for (i = 0; i < nalt; i++) {

• dfu = dfu_get_entity(i);
  

• if (!dfu)
  

• 	continue;
  

• if (dfu->dev_type != DFU_DEV_MTD)
  

• 	continue;
  

• if (dfu->layout == DFU_RAW_ADDR &&
  

•     dfu->data.mtd.start == offset &&
  

•     dfu->data.mtd.size == size) {
  

• 	*alt_num = dfu->alt;
  

• 	ret = 0;
  

• 	break;
  

• }
  

• }
• dfu_free_entities();
• return ret;

+}

+int gen_image_alt_info(char *buf, size_t len, int sidx,

•        struct fwu_image_entry *img, struct mtd_info *mtd)
  

+{

• int i;
• const char *suuid;
• ofnode node, parts_node;
• char uuidbuf[UUID_STR_LEN + 1];
• char *p = buf, *end = buf + len;
• /* Find partition node under given MTD device. */
• parts_node = ofnode_by_compatible(mtd_get_ofnode(mtd),

• 			  "fixed-partitions");
  

• if (!ofnode_valid(parts_node))

• return -ENOENT;
  

• /* Check the media UUID if exist. */
• suuid = ofnode_read_string(parts_node, "uuid");
• if (suuid) {

• log_debug("Get location uuid %s\n", suuid);
  

• uuid_bin_to_str(img->location_uuid.b, uuidbuf,
  

• 		UUID_STR_FORMAT_STD);
  

• if (strcasecmp(suuid, uuidbuf))
  

• 	log_warning("Warning: Location UUID does not match!\n");
  

• }
• p += snprintf(p, end - p, "mtd %s", mtd->name);
• if (end < p)

err_log() needed?

• return -E2BIG;
  

• /*

• * List up the image banks in the FWU mdata and search the corresponding
  

%s/up //

• * partition based on partition's uuid.
  

• */
  

• for (i = 0; i < CONFIG_FWU_NUM_BANKS; i++) {

• struct fwu_image_bank_info *bank;
  

• fdt_addr_t offset, size;
  

• /* Query a partition by image UUID */
  

• bank = &img->img_bank_info[i];
  

• uuid_bin_to_str(bank->image_uuid.b, uuidbuf,
  

• 		UUID_STR_FORMAT_STD);
  

• node = ofnode_by_prop_value(parts_node, "uuid", uuidbuf,
  

• 			    sizeof(uuidbuf));
  

• if (!ofnode_valid(node)) {
  

• 	log_warning("Warning: Failed to find partition by image UUID\n");
  

• 	break;
  

• }
  

• offset = ofnode_get_addr_size_index_notrans(node, 0, &size);
  

• if (offset == FDT_ADDR_T_NONE || !size)
  

• 	break;
  

• p += snprintf(p, end - p, "%sbank%d raw %lx %lx",
  

• 	      i == 0 ? "=" : ";", i, (unsigned long)offset,
  

• 	      (unsigned long)size);
  

• if (end < p)
  

• 	return -E2BIG;
  

• }
• return i != CONFIG_FWU_NUM_BANKS ? -ENOENT : 0;

This is easier to read:

if (i != CONFIG_FWU_NUM_BANKS) return -ENOENT; return 0;

Best regards

Heinrich

+}

+int fwu_gen_alt_info_from_mtd(char *buf, size_t len, struct mtd_info *mtd) +{

• struct fwu_mdata *mdata = NULL;
• int i, l, ret;
• ret = fwu_get_mdata(&mdata);
• if (ret < 0) {

• log_debug("Failed to get the FWU mdata.\n");
  

• return ret;
  

• }
• for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) {

• ret = gen_image_alt_info(buf, len, i * CONFIG_FWU_NUM_BANKS,
  

• 			 &mdata->img_entry[i], mtd);
  

• if (ret)
  

• 	break;
  

• l = strlen(buf);
  

• /* Replace the last ';' with '&' if there is another image. */
  

• if (i != CONFIG_FWU_NUM_IMAGES_PER_BANK - 1 && l)
  

• 	buf[l - 1] = '&';
  

• len -= l;
  

• buf += l;
  

• }
• free(mdata);
• return ret;

+}