Re: [RFC PATCH v3 3/9] FWU: stm32mp1: Add helper functions for accessing FWU metadata

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Sughosh, 

[...]
>  }
>  
>  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;

And that's fine.  But can you add a pr_warn or something on this check?  So
if anyone tries to implement this on a non-sd card can get a clear
indication of what's not working?

> +
> +		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)

ditto 

> +			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;

Should these be part of the original patch?

>  
>  	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;
>  
> -- 
> 2.17.1
>