Re: [PATCH v5 06/23] FWU: stm32mp1: Add helper functions for accessing FWU metadata
On Tue, 21 Jun 2022 at 15:19, Patrick DELAUNAY 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 > > --- > > 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 > > #include > > +#include > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -25,9 +27,11 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -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 > > +#include > > + > [...] > > + > > +#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
Re: [PATCH v5 06/23] FWU: stm32mp1: Add helper functions for accessing FWU metadata
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 --- 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 #include +#include #include #include #include +#include #include #include #include @@ -25,9 +27,11 @@ #include #include #include +#include #include #include #include +#include #include #include #include @@ -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 +#include + [...] + +#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
Re: [PATCH v5 06/23] FWU: stm32mp1: Add helper functions for accessing FWU metadata
hi Ilias,
On Fri, 10 Jun 2022 at 17:23, Ilias Apalodimas
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
> > ---
> > 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
> > #include
> > +#include
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -25,9 +27,11 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -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
> > +#include
> > +
> > +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, ))
> > + continue;
> > + uuid_str_to_bin(info.uuid, unique_part_guid.b,
> > + UUID_STR_FORMAT_GUID);
> > +
> > + if (!guidcmp(_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
Re: [PATCH v5 06/23] FWU: stm32mp1: Add helper functions for accessing FWU metadata
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
> ---
> 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
> #include
> +#include
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -25,9 +27,11 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -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
> +#include
> +
> +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, ))
> + continue;
> + uuid_str_to_bin(info.uuid, unique_part_guid.b,
> + UUID_STR_FORMAT_GUID);
> +
> + if (!guidcmp(_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
[PATCH v5 06/23] FWU: stm32mp1: Add helper functions for accessing FWU metadata
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
---
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
#include
+#include
#include
#include
#include
+#include
#include
#include
#include
@@ -25,9 +27,11 @@
#include
#include
#include
+#include
#include
#include
#include
+#include
#include
#include
#include
@@ -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
+#include
+
+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, ))
+ continue;
+ uuid_str_to_bin(info.uuid, unique_part_guid.b,
+ UUID_STR_FORMAT_GUID);
+
+ if (!guidcmp(_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, _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(_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_ */
--
2.25.1
