diff mbox series

[v5,06/23] FWU: stm32mp1: Add helper functions for accessing FWU metadata

Message ID 20220609123010.1017463-7-sughosh.ganu@linaro.org
State New
Headers show
Series FWU: Add FWU Multi Bank Update feature support | expand

Commit Message

Sughosh Ganu June 9, 2022, 12:29 p.m. UTC
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.ganu@linaro.org>
---
 board/st/stm32mp1/stm32mp1.c | 115 +++++++++++++++++++++++++++++++++++
 include/fwu.h                |   2 +
 2 files changed, 117 insertions(+)

Comments

Ilias Apalodimas June 10, 2022, 11:53 a.m. UTC | #1
Hi Sughosh, 

On Thu, Jun 09, 2022 at 05:59:53PM +0530, 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.ganu@linaro.org>
> ---
>  board/st/stm32mp1/stm32mp1.c | 115 +++++++++++++++++++++++++++++++++++
>  include/fwu.h                |   2 +
>  2 files changed, 117 insertions(+)
> 
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 62d98ad776..e68bf09955 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -7,9 +7,11 @@
>  
>  #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>
> @@ -25,9 +27,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>
> @@ -967,3 +971,114 @@ 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 get_gpt_dfu_identifier(struct blk_desc *desc, efi_guid_t *image_guid)
> +{
> +	int i;
> +	struct disk_partition info;
> +	efi_guid_t unique_part_guid;
> +
> +	for (i = 1; i < MAX_SEARCH_PARTITIONS; i++) {
> +		if (part_get_info(desc, i, &info))
> +			continue;
> +		uuid_str_to_bin(info.uuid, unique_part_guid.b,
> +				UUID_STR_FORMAT_GUID);
> +
> +		if (!guidcmp(&unique_part_guid, image_guid))
> +			return i;
> +	}
> +
> +	log_err("No partition found with image_guid %pUs\n", image_guid);
> +	return -ENOENT;
> +}
> +
> +static int gpt_plat_get_alt_num(struct blk_desc *desc, efi_guid_t *image_guid,
> +				int *alt_num)

Does this really need to be defined per platform?

Most of the stuff in here are generic apart from the info of were  the
metadata is stored.  So wouldn't it better to move this in the generic API 
and add an argument for the dfu device type?

The platform portion would then just call this function with an extra arg
e.g DFU_DEV_MMC

> +{
> +	int ret = -1;
> +	int i, part, dev_num;
> +	int nalt;
> +	struct dfu_entity *dfu;
> +
> +	dev_num = desc->devnum;
> +	part = get_gpt_dfu_identifier(desc, image_guid);
> +	if (part < 0)
> +		return -ENOENT;
> +
> +	dfu_init_env_entities(NULL, NULL);

[...]


Regards
/Ilias
Sughosh Ganu June 13, 2022, 12:37 p.m. UTC | #2
hi Ilias,

On Fri, 10 Jun 2022 at 17:23, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Sughosh,
>
> On Thu, Jun 09, 2022 at 05:59:53PM +0530, 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.ganu@linaro.org>
> > ---
> >  board/st/stm32mp1/stm32mp1.c | 115 +++++++++++++++++++++++++++++++++++
> >  include/fwu.h                |   2 +
> >  2 files changed, 117 insertions(+)
> >
> > diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> > index 62d98ad776..e68bf09955 100644
> > --- a/board/st/stm32mp1/stm32mp1.c
> > +++ b/board/st/stm32mp1/stm32mp1.c
> > @@ -7,9 +7,11 @@
> >
> >  #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>
> > @@ -25,9 +27,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>
> > @@ -967,3 +971,114 @@ 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 get_gpt_dfu_identifier(struct blk_desc *desc, efi_guid_t *image_guid)
> > +{
> > +     int i;
> > +     struct disk_partition info;
> > +     efi_guid_t unique_part_guid;
> > +
> > +     for (i = 1; i < MAX_SEARCH_PARTITIONS; i++) {
> > +             if (part_get_info(desc, i, &info))
> > +                     continue;
> > +             uuid_str_to_bin(info.uuid, unique_part_guid.b,
> > +                             UUID_STR_FORMAT_GUID);
> > +
> > +             if (!guidcmp(&unique_part_guid, image_guid))
> > +                     return i;
> > +     }
> > +
> > +     log_err("No partition found with image_guid %pUs\n", image_guid);
> > +     return -ENOENT;
> > +}
> > +
> > +static int gpt_plat_get_alt_num(struct blk_desc *desc, efi_guid_t *image_guid,
> > +                             int *alt_num)
>
> Does this really need to be defined per platform?
>
> Most of the stuff in here are generic apart from the info of were  the
> metadata is stored.  So wouldn't it better to move this in the generic API
> and add an argument for the dfu device type?
>
> The platform portion would then just call this function with an extra arg
> e.g DFU_DEV_MMC

Yes, it makes sense. I will make the change that you suggest. Thanks.

-sughosh

>
> > +{
> > +     int ret = -1;
> > +     int i, part, dev_num;
> > +     int nalt;
> > +     struct dfu_entity *dfu;
> > +
> > +     dev_num = desc->devnum;
> > +     part = get_gpt_dfu_identifier(desc, image_guid);
> > +     if (part < 0)
> > +             return -ENOENT;
> > +
> > +     dfu_init_env_entities(NULL, NULL);
>
> [...]
>
>
> Regards
> /Ilias
Patrick Delaunay June 21, 2022, 9:49 a.m. UTC | #3
Hi,

On 6/9/22 14:29, 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.ganu@linaro.org>
> ---
>   board/st/stm32mp1/stm32mp1.c | 115 +++++++++++++++++++++++++++++++++++
>   include/fwu.h                |   2 +
>   2 files changed, 117 insertions(+)
>
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 62d98ad776..e68bf09955 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -7,9 +7,11 @@
>   
>   #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>
> @@ -25,9 +27,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>
> @@ -967,3 +971,114 @@ 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>
> +
[...]
> +
> +#endif /* CONFIG_FWU_MULTI_BANK_UPDATE */
> diff --git a/include/fwu.h b/include/fwu.h
> index 3b1ee4e83e..36e58afa29 100644
> --- a/include/fwu.h
> +++ b/include/fwu.h
> @@ -46,6 +46,8 @@ 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);
>   
> +


Added empty line


>   int fwu_plat_get_alt_num(struct udevice *dev, efi_guid_t *image_guid,
>   			 int *alt_num);
> +int fwu_plat_get_update_index(u32 *update_idx);
>   #endif /* _FWU_H_ */


And I am agree with Ilias remark, should be generic

=> search on the current UCLASS_FWU_MDATA

        perhaps need a new ops in u-class ? as implementation can be 
different for GPT and MTD.


Patrick
Sughosh Ganu June 23, 2022, 6:04 a.m. UTC | #4
On Tue, 21 Jun 2022 at 15:19, Patrick DELAUNAY
<patrick.delaunay@foss.st.com> wrote:
>
> Hi,
>
> On 6/9/22 14:29, 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.ganu@linaro.org>
> > ---
> >   board/st/stm32mp1/stm32mp1.c | 115 +++++++++++++++++++++++++++++++++++
> >   include/fwu.h                |   2 +
> >   2 files changed, 117 insertions(+)
> >
> > diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> > index 62d98ad776..e68bf09955 100644
> > --- a/board/st/stm32mp1/stm32mp1.c
> > +++ b/board/st/stm32mp1/stm32mp1.c
> > @@ -7,9 +7,11 @@
> >
> >   #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>
> > @@ -25,9 +27,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>
> > @@ -967,3 +971,114 @@ 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>
> > +
> [...]
> > +
> > +#endif /* CONFIG_FWU_MULTI_BANK_UPDATE */
> > diff --git a/include/fwu.h b/include/fwu.h
> > index 3b1ee4e83e..36e58afa29 100644
> > --- a/include/fwu.h
> > +++ b/include/fwu.h
> > @@ -46,6 +46,8 @@ 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);
> >
> > +
>
>
> Added empty line

Will remove

>
>
> >   int fwu_plat_get_alt_num(struct udevice *dev, efi_guid_t *image_guid,
> >                        int *alt_num);
> > +int fwu_plat_get_update_index(u32 *update_idx);
> >   #endif /* _FWU_H_ */
>
>
> And I am agree with Ilias remark, should be generic
>
> => search on the current UCLASS_FWU_MDATA
>
>         perhaps need a new ops in u-class ? as implementation can be
> different for GPT and MTD.

My understanding of Ilias's comments was that the function can be
generic for all GPT based platforms. But I will check if this can be
reused for both GPT and MTD devices, on the lines that you mention
above. Thanks.

-sughosh
diff mbox series

Patch

diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index 62d98ad776..e68bf09955 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -7,9 +7,11 @@ 
 
 #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>
@@ -25,9 +27,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>
@@ -967,3 +971,114 @@  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 get_gpt_dfu_identifier(struct blk_desc *desc, efi_guid_t *image_guid)
+{
+	int i;
+	struct disk_partition info;
+	efi_guid_t unique_part_guid;
+
+	for (i = 1; i < MAX_SEARCH_PARTITIONS; i++) {
+		if (part_get_info(desc, i, &info))
+			continue;
+		uuid_str_to_bin(info.uuid, unique_part_guid.b,
+				UUID_STR_FORMAT_GUID);
+
+		if (!guidcmp(&unique_part_guid, image_guid))
+			return i;
+	}
+
+	log_err("No partition found with image_guid %pUs\n", image_guid);
+	return -ENOENT;
+}
+
+static int gpt_plat_get_alt_num(struct blk_desc *desc, efi_guid_t *image_guid,
+				int *alt_num)
+{
+	int ret = -1;
+	int i, part, dev_num;
+	int nalt;
+	struct dfu_entity *dfu;
+
+	dev_num = desc->devnum;
+	part = get_gpt_dfu_identifier(desc, image_guid);
+	if (part < 0)
+		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;
+
+		/*
+		 * 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) {
+			*alt_num = dfu->alt;
+			ret = 0;
+			break;
+		}
+	}
+
+	dfu_free_entities();
+
+	return ret;
+}
+
+int fwu_plat_get_alt_num(struct udevice *dev, efi_guid_t *image_guid,
+			 int *alt_num)
+{
+	struct blk_desc *desc;
+
+	desc = dev_get_uclass_plat(dev);
+	if (!desc) {
+		log_err("Block device not found\n");
+		return -ENODEV;
+	}
+
+	return gpt_plat_get_alt_num(desc, image_guid, alt_num);
+}
+
+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;
+}
+
+#endif /* CONFIG_FWU_MULTI_BANK_UPDATE */
diff --git a/include/fwu.h b/include/fwu.h
index 3b1ee4e83e..36e58afa29 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -46,6 +46,8 @@  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_alt_num(struct udevice *dev, efi_guid_t *image_guid,
 			 int *alt_num);
+int fwu_plat_get_update_index(u32 *update_idx);
 #endif /* _FWU_H_ */