On Thu, 20 Jan 2022 at 16:29, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > On 1/19/22 19:55, Sughosh Ganu wrote: > > Add helper functions needed for accessing the FWU metadata which > > contains information on the updatable images. These functions have > > been added for the STM32MP157C-DK2 board which has the updatable > > images on the uSD card, formatted as GPT partitions. > > > > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org> > > It is unclear why you are creating platform specific code here.
This is primarily code that will be dependent on the platform. For e.g. the number of banks of firmware is a decision of the platform. So, there is a function, fwu_plat_get_update_index which will return the bank number which should be used for updating the firmware images. Similarly for getting the DFU alt_num. This will depend on things like the device that the platform uses for storing the firmware images. There aren't too many functions defined here. I will cover this aspect in the documentation patch which you have asked for. > > All of this metadata can be put into the control devicetree? If not > create a driver class for it. Creating a driver class for accessing the metadata seems like a decent solution to me. I will work on this. -sughosh > > Unfortunately you don't provide any design document. > > > Best regards > > Heinrich > > > > --- > > > > Changes since V2: > > * Change the implementation of fwu_plat_get_alt_num to get the devnum > > in the function before calling gpt_plat_get_alt_num > > > > board/st/stm32mp1/stm32mp1.c | 176 ++++++++++++++++++++++++++++ > > include/fwu.h | 5 + > > lib/fwu_updates/fwu_mdata_gpt_blk.c | 7 +- > > 3 files changed, 185 insertions(+), 3 deletions(-) > > > > diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c > > index 84592677e4..66cbe3f798 100644 > > --- a/board/st/stm32mp1/stm32mp1.c > > +++ b/board/st/stm32mp1/stm32mp1.c > > @@ -7,10 +7,13 @@ > > > > #include <common.h> > > #include <adc.h> > > +#include <blk.h> > > #include <bootm.h> > > #include <clk.h> > > #include <config.h> > > +#include <dfu.h> > > #include <dm.h> > > +#include <efi_loader.h> > > #include <env.h> > > #include <env_internal.h> > > #include <fdt_support.h> > > @@ -23,9 +26,11 @@ > > #include <log.h> > > #include <malloc.h> > > #include <misc.h> > > +#include <mmc.h> > > #include <mtd_node.h> > > #include <net.h> > > #include <netdev.h> > > +#include <part.h> > > #include <phy.h> > > #include <remoteproc.h> > > #include <reset.h> > > @@ -938,3 +943,174 @@ static void board_copro_image_process(ulong fw_image, > > size_t fw_size) > > } > > > > U_BOOT_FIT_LOADABLE_HANDLER(IH_TYPE_COPRO, board_copro_image_process); > > + > > +#if defined(CONFIG_FWU_MULTI_BANK_UPDATE) > > +#include <fwu.h> > > +#include <fwu_mdata.h> > > + > > +static int gpt_plat_get_alt_num(int dev_num, void *identifier) > > +{ > > + int i; > > + int ret = -1; > > + u32 part; > > + int alt_num = dfu_get_alt_number(); > > + struct dfu_entity *dfu; > > + > > + part = *(u32 *)identifier; > > + dfu_init_env_entities(NULL, NULL); > > + > > + for (i = 0; i < alt_num; i++) { > > + dfu = dfu_get_entity(i); > > + > > + if (!dfu) > > + continue; > > + > > + /* > > + * Currently, Multi Bank update > > + * feature is being supported > > + * only on GPT partitioned > > + * MMC/SD devices. > > + */ > > + if (dfu->dev_type != DFU_DEV_MMC) > > + continue; > > + > > + if (dfu->layout == DFU_RAW_ADDR && > > + dfu->data.mmc.dev_num == dev_num && > > + dfu->data.mmc.part == part) { > > + ret = dfu->alt; > > + break; > > + } > > + } > > + > > + dfu_free_entities(); > > + > > + return ret; > > +} > > + > > +int fwu_plat_get_alt_num(void *identifier) > > +{ > > + int ret; > > + struct blk_desc *desc; > > + > > + ret = fwu_plat_get_blk_desc(&desc); > > + if (ret < 0) { > > + log_err("Block device not found\n"); > > + return -ENODEV; > > + } > > + > > + return gpt_plat_get_alt_num(desc->devnum, identifier); > > +} > > + > > +static int plat_fill_gpt_partition_guids(struct blk_desc *desc, > > + efi_guid_t **part_guid_arr) > > +{ > > + int i, ret = 0; > > + u32 part; > > + struct dfu_entity *dfu; > > + struct disk_partition info; > > + efi_guid_t part_type_guid; > > + int alt_num = dfu_get_alt_number(); > > + > > + dfu_init_env_entities(NULL, NULL); > > + > > + for (i = 0, part = 1; i < alt_num; i++) { > > + dfu = dfu_get_entity(i); > > + > > + if (!dfu) > > + continue; > > + > > + /* > > + * Currently, Multi Bank update > > + * feature is being supported > > + * only on GPT partitioned > > + * MMC/SD devices. > > + */ > > + if (dfu->dev_type != DFU_DEV_MMC) > > + continue; > > + > > + if (part_get_info(desc, part, &info)) { > > + part++; > > + continue; > > + } > > + > > + uuid_str_to_bin(info.type_guid, part_type_guid.b, > > + UUID_STR_FORMAT_GUID); > > + guidcpy((*part_guid_arr + i), &part_type_guid); > > + part++; > > + } > > + > > + dfu_free_entities(); > > + > > + return ret; > > +} > > + > > +int fwu_plat_fill_partition_guids(efi_guid_t **part_guid_arr) > > +{ > > + int ret; > > + struct blk_desc *desc; > > + > > + ret = fwu_plat_get_blk_desc(&desc); > > + if (ret < 0) { > > + log_err("Block device not found\n"); > > + return -ENODEV; > > + } > > + > > + return plat_fill_gpt_partition_guids(desc, part_guid_arr); > > +} > > + > > +int fwu_plat_get_update_index(u32 *update_idx) > > +{ > > + int ret; > > + u32 active_idx; > > + > > + ret = fwu_get_active_index(&active_idx); > > + > > + if (ret < 0) > > + return -1; > > + > > + *update_idx = active_idx ^= 0x1; > > + > > + return ret; > > +} > > + > > +int fwu_plat_get_blk_desc(struct blk_desc **desc) > > +{ > > + int ret; > > + struct mmc *mmc; > > + struct udevice *dev; > > + > > + /* > > + * Initial support is being added for the DK2 > > + * platform > > + */ > > + if (CONFIG_IS_ENABLED(TARGET_ST_STM32MP15x) && > > + (of_machine_is_compatible("st,stm32mp157c-dk2"))) { > > + ret = uclass_get_device(UCLASS_MMC, 0, &dev); > > + if (ret) > > + return -1; > > + > > + mmc = mmc_get_mmc_dev(dev); > > + if (!mmc) > > + return -1; > > + > > + if (mmc_init(mmc)) > > + return -1; > > + > > + *desc = mmc_get_blk_desc(mmc); > > + if (!*desc) > > + return -1; > > + } > > + > > + return 0; > > +} > > + > > +struct fwu_mdata_ops *get_plat_fwu_mdata_ops(void) > > +{ > > + if (CONFIG_IS_ENABLED(TARGET_ST_STM32MP15x) && > > + (of_machine_is_compatible("st,stm32mp157c-dk2"))) { > > + return &fwu_gpt_blk_ops; > > + } > > + > > + return NULL; > > +} > > +#endif /* CONFIG_FWU_MULTI_BANK_UPDATE */ > > diff --git a/include/fwu.h b/include/fwu.h > > index 12f7eecdb0..b23a93ac40 100644 > > --- a/include/fwu.h > > +++ b/include/fwu.h > > @@ -59,4 +59,9 @@ int fwu_revert_boot_index(void); > > int fwu_accept_image(efi_guid_t *img_type_id, u32 bank); > > int fwu_clear_accept_image(efi_guid_t *img_type_id, u32 bank); > > > > +int fwu_plat_get_update_index(u32 *update_idx); > > +int fwu_plat_get_blk_desc(struct blk_desc **desc); > > +int fwu_plat_get_alt_num(void *identifier); > > +int fwu_plat_fill_partition_guids(efi_guid_t **part_guid_arr); > > + > > #endif /* _FWU_H_ */ > > diff --git a/lib/fwu_updates/fwu_mdata_gpt_blk.c > > b/lib/fwu_updates/fwu_mdata_gpt_blk.c > > index cb47ddf4a7..796b08e76f 100644 > > --- a/lib/fwu_updates/fwu_mdata_gpt_blk.c > > +++ b/lib/fwu_updates/fwu_mdata_gpt_blk.c > > @@ -37,6 +37,7 @@ static int gpt_get_mdata_partitions(struct blk_desc *desc, > > struct disk_partition info; > > const efi_guid_t fwu_mdata_guid = FWU_MDATA_GUID; > > > > + mdata_parts = 0; > > for (i = 1; i < MAX_SEARCH_PARTITIONS; i++) { > > if (part_get_info(desc, i, &info)) > > continue; > > @@ -324,7 +325,7 @@ out: > > int fwu_gpt_get_active_index(u32 *active_idx) > > { > > int ret; > > - struct fwu_mdata *mdata; > > + struct fwu_mdata *mdata = NULL; > > > > ret = gpt_get_mdata(&mdata); > > if (ret < 0) { > > @@ -355,7 +356,7 @@ static int gpt_get_image_alt_num(struct blk_desc *desc, > > { > > int ret, i; > > u32 part; > > - struct fwu_mdata *mdata; > > + struct fwu_mdata *mdata = NULL; > > struct fwu_image_entry *img_entry; > > struct fwu_image_bank_info *img_bank_info; > > struct disk_partition info; > > @@ -459,7 +460,7 @@ static int fwu_gpt_set_clear_image_accept(efi_guid_t > > *img_type_id, > > void *buf; > > int ret, i; > > u32 nimages; > > - struct fwu_mdata *mdata; > > + struct fwu_mdata *mdata = NULL; > > struct fwu_image_entry *img_entry; > > struct fwu_image_bank_info *img_bank_info; > > >