On Fri, 14 Oct 2022 at 11:46, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > On Thu, Oct 06, 2022 at 02:36:17PM +0530, Sughosh Ganu wrote: > > In the FWU Multi Bank Update feature, the information about the > > updatable images is stored as part of the metadata, on a separate > > partition. Add a driver for reading from and writing to the metadata > > when the updatable images and the metadata are stored on a block > > device which is formatted with GPT based partition scheme. > > > > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org> > > Reviewed-by: Patrick Delaunay <patrick.delau...@foss.st.com> > > Reviewed-by: Etienne Carriere <etienne.carri...@linaro.org> > > --- > > Changes since V12: None > > > > drivers/fwu-mdata/Kconfig | 16 ++ > > drivers/fwu-mdata/Makefile | 8 + > > drivers/fwu-mdata/gpt_blk.c | 357 ++++++++++++++++++++++++++++++++++++ > > include/fwu.h | 4 + > > 4 files changed, 385 insertions(+) > > create mode 100644 drivers/fwu-mdata/Kconfig > > create mode 100644 drivers/fwu-mdata/Makefile > > create mode 100644 drivers/fwu-mdata/gpt_blk.c > > > > diff --git a/drivers/fwu-mdata/Kconfig b/drivers/fwu-mdata/Kconfig > > new file mode 100644 > > index 0000000000..36c4479a59 > > --- /dev/null > > +++ b/drivers/fwu-mdata/Kconfig > > @@ -0,0 +1,16 @@ > > +config FWU_MDATA > > + bool "Driver support for accessing FWU Metadata" > > + depends on DM > > + help > > + Enable support for accessing FWU Metadata partitions. The > > + FWU Metadata partitions reside on the same storage device > > + which contains the other FWU updatable firmware images. > > + > > +config FWU_MDATA_GPT_BLK > > + bool "FWU Metadata access for GPT partitioned Block devices" > > + select PARTITION_TYPE_GUID > > + select PARTITION_UUIDS > > + depends on FWU_MDATA && BLK && EFI_PARTITION > > + help > > + Enable support for accessing FWU Metadata on GPT partitioned > > + block devices. > > diff --git a/drivers/fwu-mdata/Makefile b/drivers/fwu-mdata/Makefile > > new file mode 100644 > > index 0000000000..3fee64c10c > > --- /dev/null > > +++ b/drivers/fwu-mdata/Makefile > > @@ -0,0 +1,8 @@ > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +# > > +# Copyright (c) 2022, Linaro Limited > > +# > > + > > + > > +obj-$(CONFIG_FWU_MDATA) += fwu-mdata-uclass.o > > +obj-$(CONFIG_FWU_MDATA_GPT_BLK) += gpt_blk.o > > diff --git a/drivers/fwu-mdata/gpt_blk.c b/drivers/fwu-mdata/gpt_blk.c > > new file mode 100644 > > index 0000000000..131c86bc55 > > --- /dev/null > > +++ b/drivers/fwu-mdata/gpt_blk.c > > @@ -0,0 +1,357 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Copyright (c) 2022, Linaro Limited > > + */ > > + > > +#define LOG_CATEGORY UCLASS_FWU_MDATA > > + > > +#include <blk.h> > > +#include <dm.h> > > +#include <efi_loader.h> > > +#include <fwu.h> > > +#include <fwu_mdata.h> > > +#include <log.h> > > +#include <memalign.h> > > +#include <part.h> > > +#include <part_efi.h> > > + > > +#include <dm/device-internal.h> > > +#include <linux/errno.h> > > +#include <linux/types.h> > > + > > +#define PRIMARY_PART BIT(0) > > +#define SECONDARY_PART BIT(1) > > +#define BOTH_PARTS (PRIMARY_PART | SECONDARY_PART) > > + > > +#define MDATA_READ BIT(0) > > +#define MDATA_WRITE BIT(1) > > + > > +static int gpt_get_mdata_partitions(struct blk_desc *desc, > > + uint mdata_parts[2]) > > This feels a bit weird. I'd prefer uint mdata_parts[] here
I had declared this as a pointer initially [1], but Etienne gave a review comment saying that he preferred it to be declared as an array with the elements specified [2]. -sughosh [1] - https://lists.denx.de/pipermail/u-boot/2022-September/495854.html [2] - https://lists.denx.de/pipermail/u-boot/2022-September/496061.html > > > +{ > > + int i, ret; > > + u32 nparts; > > + efi_guid_t part_type_guid; > > + struct disk_partition info; > > + const efi_guid_t fwu_mdata_guid = FWU_MDATA_GUID; > > + > > + nparts = 0; > > + for (i = 1; i < MAX_SEARCH_PARTITIONS && nparts < 2; i++) { > > + if (part_get_info(desc, i, &info)) > > + continue; > > + uuid_str_to_bin(info.type_guid, part_type_guid.b, > > + UUID_STR_FORMAT_GUID); > > + > > + if (!guidcmp(&fwu_mdata_guid, &part_type_guid)) { > > + mdata_parts[nparts] = i; > > + ++nparts; > > + } > > + } > > + > > + if (nparts != 2) { > > + log_debug("Expect two copies of the FWU metadata instead of > > %d\n", > > + nparts); > > + ret = -EINVAL; > > + } else { > > + ret = 0; > > + } > > + > > + return ret; > > +} > > + > > +static int gpt_get_mdata_disk_part(struct blk_desc *desc, > > + struct disk_partition *info, > > + u32 part_num) > > +{ > > + int ret; > > + char *mdata_guid_str = "8a7a84a0-8387-40f6-ab41-a8b9a5a60d23"; > > + > > + ret = part_get_info(desc, part_num, info); > > + if (ret < 0) { > > + log_debug("Unable to get the partition info for the FWU > > metadata part %d\n", > > + part_num); > > + return -ENOENT; > > + } > > + > > + /* Check that it is indeed the FWU metadata partition */ > > + if (!strncmp(info->type_guid, mdata_guid_str, UUID_STR_LEN)) { > > + /* Found the FWU metadata partition */ > > + return 0; > > + } > > + > > + return -ENOENT; > > +} > > + > > [...] > > With the argument fixed > Reviewed-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>