hi Patrick, On Wed, 13 Jul 2022 at 18:47, Patrick DELAUNAY <patrick.delau...@foss.st.com> wrote: > > Hi, > > On 7/4/22 07:16, 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 formated with GPT based partition scheme. > > > > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org> > > --- > > Changes since V5: > > * Changed the logic to store the GPT partitioned block device through > > a priv structure as suggested by Patrick > > * Used dev_read_prop() to get the phandle_p instead of > > ofnode_get_property() used earlier as suggested by Patrick > > * Make relevant functions static as suggested by Etienne > > > > drivers/fwu-mdata/Kconfig | 9 + > > drivers/fwu-mdata/Makefile | 1 + > > drivers/fwu-mdata/fwu_mdata_gpt_blk.c | 408 ++++++++++++++++++++++++++ > > include/fwu.h | 5 + > > 4 files changed, 423 insertions(+) > > create mode 100644 drivers/fwu-mdata/fwu_mdata_gpt_blk.c > > > > diff --git a/drivers/fwu-mdata/Kconfig b/drivers/fwu-mdata/Kconfig > > index d6a21c8e19..d5edef19d6 100644 > > --- a/drivers/fwu-mdata/Kconfig > > +++ b/drivers/fwu-mdata/Kconfig > > @@ -5,3 +5,12 @@ config DM_FWU_MDATA > > 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 DM && HAVE_BLOCK_DEVICE && 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 > > index e53a8c9983..313049f67a 100644 > > --- a/drivers/fwu-mdata/Makefile > > +++ b/drivers/fwu-mdata/Makefile > > @@ -4,3 +4,4 @@ > > # > > > > obj-$(CONFIG_DM_FWU_MDATA) += fwu-mdata-uclass.o > > +obj-$(CONFIG_FWU_MDATA_GPT_BLK) += fwu_mdata_gpt_blk.o > > diff --git a/drivers/fwu-mdata/fwu_mdata_gpt_blk.c > > b/drivers/fwu-mdata/fwu_mdata_gpt_blk.c > > new file mode 100644 > > index 0000000000..d904c9492d > > --- /dev/null > > +++ b/drivers/fwu-mdata/fwu_mdata_gpt_blk.c > > @@ -0,0 +1,408 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Copyright (c) 2022, Linaro Limited > > + */ > > #define LOG_CATEGORY UCLASS_FWU_MDATA
I think you had mentioned this in your earlier review as well. Sorry, I missed out on this. Will add in the next version. > > > + > > +#include <blk.h> > > +#include <dm.h> > > +#include <efi_loader.h> > > +#include <fwu.h> > > +#include <fwu_mdata.h> > > +#include <log.h> > > +#include <malloc.h> > > +#include <memalign.h> > > +#include <part.h> > > +#include <part_efi.h> > > + > > +#include <dm/device-internal.h> > > +#include <linux/errno.h> > > +#include <linux/types.h> > > +#include <u-boot/crc.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 const struct fwu_mdata_ops fwu_gpt_blk_ops = { > > + .mdata_check = fwu_gpt_mdata_check, > > + .get_mdata = fwu_gpt_get_mdata, > > + .update_mdata = fwu_gpt_update_mdata, > > +};UCLASS_FWU_MDATA > > > > + > > +static const struct udevice_id fwu_mdata_ids[] = { > > + { .compatible = "u-boot,fwu-mdata-gpt" }, > > + { } > > +}; > > + > > +U_BOOT_DRIVER(fwu_mdata_gpt_blk) = { > > + .name = "fwu-mdata-gpt-blk", > > + .id = UCLASS_FWU_MDATA, > > + .of_match = fwu_mdata_ids, > > + .ops = &fwu_gpt_blk_ops, > > + .probe = fwu_mdata_gpt_blk_probe, > > + .priv_auto = sizeof(struct fwu_mdata_gpt_blk_priv), > > +}; > > diff --git a/include/fwu.h b/include/fwu.h > > index e03cfff800..8259c75d12 100644 > > --- a/include/fwu.h > > +++ b/include/fwu.h > > @@ -14,6 +14,10 @@ > > struct fwu_mdata; > > struct udevice; > > > > +struct fwu_mdata_gpt_blk_priv { > > + struct udevice *blk_dev; > > +}; > > + > > NITS: really needed in .h file => private, only used by driver in .c ? I have put it here since this gets accessed in the board file as well to retrieve the block device which has the fwu metadata and the firmware images in fwu_plat_get_alt_num(). > > > > /** > > * @mdata_check: check the validity of the FWU metadata partitions > > * @get_mdata() - Get a FWU metadata copy > > @@ -39,6 +43,7 @@ int fwu_get_active_index(u32 *active_idx); > > int fwu_update_active_index(u32 active_idx); > > int fwu_get_image_alt_num(efi_guid_t *image_type_id, u32 update_bank, > > int *alt_num); > > +int fwu_verify_mdata(struct fwu_mdata *mdata, bool pri_part); > > int fwu_mdata_check(void); > > int fwu_revert_boot_index(void); > > int fwu_accept_image(efi_guid_t *img_type_id, u32 bank); > > > with the minor modification (LOG_CATEGORY) > > > Reviewed-by: Patrick Delaunay <patrick.delau...@foss.st.com> Thanks. -sughosh