Re: [PATCH v5 03/23] FWU: Add FWU metadata access driver for GPT partitioned block devices

2022-06-28 Thread Sughosh Ganu
hi Etienne,

On Tue, 21 Jun 2022 at 16:26, Etienne Carriere
 wrote:
>
> Hello Sughosh,
>
> On Thu, 9 Jun 2022 at 14:30, 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 
> > ---
> >  drivers/fwu-mdata/Kconfig |   9 +
> >  drivers/fwu-mdata/Makefile|   1 +
> >  drivers/fwu-mdata/fwu_mdata_gpt_blk.c | 404 ++
> >  include/fwu.h |   2 +
> >  4 files changed, 416 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 7fec7171f4..12a5b4fe04 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 00..329bd3779b
> > --- /dev/null
> > +++ b/drivers/fwu-mdata/fwu_mdata_gpt_blk.c
> > @@ -0,0 +1,404 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2022, Linaro Limited
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define PRIMARY_PART   BIT(0)
> > +#define SECONDARY_PART BIT(1)
> > +#define BOTH_PARTS (PRIMARY_PART | SECONDARY_PART)
> > +
> > +#define MDATA_READ BIT(0)
> > +#define MDATA_WRITEBIT(1)
> > +



> > +
> > +static int gpt_get_mdata(struct blk_desc *desc, struct fwu_mdata **mdata)
> > +{
> > +   int ret;
> > +   u16 primary_mpart = 0, secondary_mpart = 0;
> > +
> > +   ret = gpt_get_mdata_partitions(desc, _mpart,
> > +  _mpart);
> > +
> > +   if (ret < 0) {
> > +   log_err("Error getting the FWU metadata partitions\n");
> > +   return -ENODEV;
> > +   }
> > +
> > +   *mdata = malloc(sizeof(struct fwu_mdata));
> > +   if (!*mdata) {
> > +   log_err("Unable to allocate memory for reading FWU 
> > metadata\n");
> > +   return -ENOMEM;
> > +   }
> > +
> > +   ret = gpt_read_mdata(desc, *mdata, primary_mpart);
> > +   if (ret < 0) {
> > +   log_err("Failed to read the FWU metadata from the 
> > device\n");
> > +   return -EIO;
> > +   }
> > +
> > +   ret = fwu_verify_mdata(*mdata, 1);
> > +   if (!ret)
>
> Upon success, I think this function should also either ensure
> secondary_part contains a valid copy of primary part,
> Maybe this function should call gpt_check_mdata_validity() and then
> read mdata content.

Okay

>
> > +   return 0;
> > +
> > +   /*
> > +* Verification of the primary FWU metadata copy failed.
> > +* Try to read the replica.
> > +*/
> > +   memset(*mdata, 0, sizeof(struct fwu_mdata));
> > +   ret = gpt_read_mdata(desc, *mdata, secondary_mpart);
> > +   if (ret < 0) {
> > +   log_err("Failed to read the FWU metadata from the 
> > device\n");
> > +   return -EIO;
> > +   }
> > +
> > +   ret = fwu_verify_mdata(*mdata, 0);
> > +   if (!ret)
> > +   return 0;
> > +
> > +   /* Both the FWU metadata copies are corrupted. */
> > +   return -1;
> > +}
> > +
> > +static int gpt_check_mdata_validity(struct udevice *dev)
> > +{
> > +   int ret;
> > +   struct blk_desc *desc;
> > +   struct fwu_mdata pri_mdata;
> > +   struct fwu_mdata secondary_mdata;
> > +   u16 primary_mpart = 0, secondary_mpart = 0;
> > +   u16 

Re: [PATCH v5 03/23] FWU: Add FWU metadata access driver for GPT partitioned block devices

2022-06-28 Thread Sughosh Ganu
hi Patrick,
Apologies for the late reply. I had missed out replying to the review
comments on this patch. There are some review comments on the
Synquacer patches which need to be taken care of by another engineer.
Once those review comments are taken care of, I will be sending the
next version.

On Tue, 21 Jun 2022 at 15:04, Patrick DELAUNAY
 wrote:
>
> Hi,
>
> On 6/9/22 14:29, 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 
> > ---
> >   drivers/fwu-mdata/Kconfig |   9 +
> >   drivers/fwu-mdata/Makefile|   1 +
> >   drivers/fwu-mdata/fwu_mdata_gpt_blk.c | 404 ++
> >   include/fwu.h |   2 +
> >   4 files changed, 416 insertions(+)
> >   create mode 100644 drivers/fwu-mdata/fwu_mdata_gpt_blk.c
> >



> > diff --git a/drivers/fwu-mdata/Makefile b/drivers/fwu-mdata/Makefile
> > index 7fec7171f4..12a5b4fe04 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
>
>
> It is strange to have '_' and '-' in file name for the same directory
>
> => to be coherent = fwu-mdata-gpt-blk.c

I see this kind of naming style in many other directories under
drivers/. The uclass file is named using the foo-uclass.c, while the
other driver files are named bar_driver.c

>
>
> > 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 00..329bd3779b
> > --- /dev/null
> > +++ b/drivers/fwu-mdata/fwu_mdata_gpt_blk.c
> > @@ -0,0 +1,404 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2022, Linaro Limited
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#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)
> > +
> > +
>
>
> [...]
>
> > +
> > +int fwu_gpt_mdata_check(struct udevice *dev)
> > +{
> > + /*
> > +  * Check if both the copies of the FWU metadata are
> > +  * valid. If one has gone bad, restore it from the
> > +  * other good copy.
> > +  */
> > + return gpt_check_mdata_validity(dev);
> > +}
> > +
> > +int fwu_gpt_get_mdata(struct udevice *dev, struct fwu_mdata **mdata)
> > +{
> > + struct blk_desc *desc;
> > +
> > + desc = dev_get_uclass_plat(dev_get_priv(dev));
>
> as dev = fwu_mdata_gpt_blk(UCLASS_FWU_MDATA)
>
> dev_get_priv(dev) => get value saved in dev_set_priv(dev, mdata_dev);
>
> even if it is OK, it not clear here.
>
> can you add a struct to prepare addition of other elements in privdata:
>
> struct fwu_mdata_gpt_blk_priv {
> struct udevice *blk_dev;
> }
>
>
> + struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
>
> + struct blk_desc *desc;
>
> + desc = dev_get_uclass_plat(priv->blk_dev);

Okay. Will add a priv structure as you suggest.

>
>
> > + if (!desc) {
> > + log_err("Block device not found\n");
> > + return -ENODEV;
> > + }
> > +
> > + return gpt_get_mdata(desc, mdata);
> > +}
> > +
> > +int fwu_get_mdata_device(struct udevice *dev, struct udevice **mdata_dev)
> > +{
> > + u32 phandle;
> > + int ret, size;
> > + struct udevice *parent, *child;
> > + const fdt32_t *phandle_p = NULL;
> > +
> > + phandle_p = ofnode_get_property(dev_ofnode(dev), "fwu-mdata-store",
> > + );
>
> phandle_p = dev_read_prop(dev, "fwu-mdata-store", );
>
>
> it is more simple

Okay

>
>
> > + if (!phandle_p) {
> > + log_err("fwu-mdata-store property not found\n");
> > + return -ENOENT;
> > + }
> > +
> > + phandle = fdt32_to_cpu(*phandle_p);
>
>
> or phandle can be read directly by
>
> + ret =dev_read_phandle_with_args(dev, "fwu-mdata-store", NULL, 0, 0,
> phandle_p)

I did not understand this review comment properly. In any case, I am
using the API, dev_read_prop that you suggested above to read the
phandle pointer.

>
> +   if (ret) {
> +   log_err("fwu-mdata-store property not found\n");
> +   return ret;
> +   }
>
> > +
> > + ret = device_get_global_by_ofnode(ofnode_get_by_phandle(phandle),
> > +   );
> > + if (ret)
> > +   

Re: [PATCH v5 03/23] FWU: Add FWU metadata access driver for GPT partitioned block devices

2022-06-22 Thread Patrick DELAUNAY

Hi

On 6/21/22 11:34, Patrick DELAUNAY wrote:

Hi,

On 6/9/22 14:29, 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 
---
  drivers/fwu-mdata/Kconfig |   9 +
  drivers/fwu-mdata/Makefile    |   1 +
  drivers/fwu-mdata/fwu_mdata_gpt_blk.c | 404 ++
  include/fwu.h |   2 +
  4 files changed, 416 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 7fec7171f4..12a5b4fe04 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



It is strange to have '_' and '-' in file name for the same directory

=> to be coherent = fwu-mdata-gpt-blk.c


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 00..329bd3779b
--- /dev/null
+++ b/drivers/fwu-mdata/fwu_mdata_gpt_blk.c
@@ -0,0 +1,404 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2022, Linaro Limited
+ */



+ #define LOG_CATEGORY UCLASS_FWU_MDATA

For command log filtering by uclass


+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+

[...]


Regards

Patrick



Re: [PATCH v5 03/23] FWU: Add FWU metadata access driver for GPT partitioned block devices

2022-06-21 Thread Etienne Carriere
Hello Sughosh,

On Thu, 9 Jun 2022 at 14:30, 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 
> ---
>  drivers/fwu-mdata/Kconfig |   9 +
>  drivers/fwu-mdata/Makefile|   1 +
>  drivers/fwu-mdata/fwu_mdata_gpt_blk.c | 404 ++
>  include/fwu.h |   2 +
>  4 files changed, 416 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 7fec7171f4..12a5b4fe04 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 00..329bd3779b
> --- /dev/null
> +++ b/drivers/fwu-mdata/fwu_mdata_gpt_blk.c
> @@ -0,0 +1,404 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2022, Linaro Limited
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define PRIMARY_PART   BIT(0)
> +#define SECONDARY_PART BIT(1)
> +#define BOTH_PARTS (PRIMARY_PART | SECONDARY_PART)
> +
> +#define MDATA_READ BIT(0)
> +#define MDATA_WRITEBIT(1)
> +
> +static int gpt_get_mdata_partitions(struct blk_desc *desc,
> +   u16 *primary_mpart,
> +   u16 *secondary_mpart)
> +{
> +   int i, ret;
> +   u32 mdata_parts;
> +   efi_guid_t part_type_guid;
> +   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, ))
> +   continue;
> +   uuid_str_to_bin(info.type_guid, part_type_guid.b,
> +   UUID_STR_FORMAT_GUID);
> +
> +   if (!guidcmp(_mdata_guid, _type_guid)) {
> +   ++mdata_parts;
> +   if (!*primary_mpart)
> +   *primary_mpart = i;
> +   else
> +   *secondary_mpart = i;
> +   }
> +   }
> +
> +   if (mdata_parts != 2) {
> +   log_err("Expect two copies of the FWU metadata instead of 
> %d\n",
> +   mdata_parts);
> +   ret = -EINVAL;
> +   } else {
> +   ret = 0;
> +   }
> +
> +   return ret;
> +}
> +
> +static int gpt_get_mdata_disk_part(struct blk_desc *desc,
> +  struct disk_partition *info,
> +  u32 part_num)
> +{
> +   int ret;
> +   char *mdata_guid_str = "8a7a84a0-8387-40f6-ab41-a8b9a5a60d23";
> +
> +   ret = part_get_info(desc, part_num, info);
> +   if (ret < 0) {
> +   log_err("Unable to get the partition info for the FWU 
> metadata part %d",
> +   part_num);
> +   return -1;
> +   }
> +
> +   /* Check that it is indeed the FWU metadata partition */
> +   if (!strncmp(info->type_guid, mdata_guid_str, UUID_STR_LEN)) {
> +   /* Found the FWU metadata partition */
> +   return 0;
> +   }
> +
> +   return -1;
> +}
> +
> +static int gpt_read_write_mdata(struct blk_desc *desc,
> +   struct fwu_mdata *mdata,
> +   u8 access, u32 part_num)
> +{
> +   int ret;
> +   u32 len, blk_start, blkcnt;
> +   struct disk_partition info;
> +
> +   

Re: [PATCH v5 03/23] FWU: Add FWU metadata access driver for GPT partitioned block devices

2022-06-21 Thread Patrick DELAUNAY

Hi,

On 6/9/22 14:29, 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 
---
  drivers/fwu-mdata/Kconfig |   9 +
  drivers/fwu-mdata/Makefile|   1 +
  drivers/fwu-mdata/fwu_mdata_gpt_blk.c | 404 ++
  include/fwu.h |   2 +
  4 files changed, 416 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 7fec7171f4..12a5b4fe04 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



It is strange to have '_' and '-' in file name for the same directory

=> to be coherent = fwu-mdata-gpt-blk.c



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 00..329bd3779b
--- /dev/null
+++ b/drivers/fwu-mdata/fwu_mdata_gpt_blk.c
@@ -0,0 +1,404 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2022, Linaro Limited
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#define PRIMARY_PART   BIT(0)
+#define SECONDARY_PART BIT(1)
+#define BOTH_PARTS (PRIMARY_PART | SECONDARY_PART)
+
+#define MDATA_READ BIT(0)
+#define MDATA_WRITEBIT(1)
+
+



[...]


+
+int fwu_gpt_mdata_check(struct udevice *dev)
+{
+   /*
+* Check if both the copies of the FWU metadata are
+* valid. If one has gone bad, restore it from the
+* other good copy.
+*/
+   return gpt_check_mdata_validity(dev);
+}
+
+int fwu_gpt_get_mdata(struct udevice *dev, struct fwu_mdata **mdata)
+{
+   struct blk_desc *desc;
+
+   desc = dev_get_uclass_plat(dev_get_priv(dev));


as dev = fwu_mdata_gpt_blk(UCLASS_FWU_MDATA)

dev_get_priv(dev) => get value saved in dev_set_priv(dev, mdata_dev);

even if it is OK, it not clear here.

can you add a struct to prepare addition of other elements in privdata:

struct fwu_mdata_gpt_blk_priv {
struct udevice *blk_dev;
}


+ struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);

+ struct blk_desc *desc;

+ desc = dev_get_uclass_plat(priv->blk_dev);



+   if (!desc) {
+   log_err("Block device not found\n");
+   return -ENODEV;
+   }
+
+   return gpt_get_mdata(desc, mdata);
+}
+
+int fwu_get_mdata_device(struct udevice *dev, struct udevice **mdata_dev)
+{
+   u32 phandle;
+   int ret, size;
+   struct udevice *parent, *child;
+   const fdt32_t *phandle_p = NULL;
+
+   phandle_p = ofnode_get_property(dev_ofnode(dev), "fwu-mdata-store",
+   );


phandle_p = dev_read_prop(dev, "fwu-mdata-store", );


it is more simple



+   if (!phandle_p) {
+   log_err("fwu-mdata-store property not found\n");
+   return -ENOENT;
+   }
+
+   phandle = fdt32_to_cpu(*phandle_p);



or phandle can be read directly by

+ ret =dev_read_phandle_with_args(dev, "fwu-mdata-store", NULL, 0, 0, 
phandle_p)


+   if (ret) {
+   log_err("fwu-mdata-store property not found\n");
+   return ret;
+   }


+
+   ret = device_get_global_by_ofnode(ofnode_get_by_phandle(phandle),
+ );
+   if (ret)
+   return ret;
+
+   ret = -ENODEV;
+   for (device_find_first_child(parent, ); child;
+device_find_next_child()) {
+   if (device_get_uclass_id(child) == UCLASS_BLK) {
+   *mdata_dev = child;
+   ret = 0;
+   }
+   }
+
+   return ret;
+}
+
+static int fwu_mdata_gpt_blk_probe(struct udevice *dev)
+{
+   int ret;
+ 

[PATCH v5 03/23] FWU: Add FWU metadata access driver for GPT partitioned block devices

2022-06-09 Thread Sughosh Ganu
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 
---
 drivers/fwu-mdata/Kconfig |   9 +
 drivers/fwu-mdata/Makefile|   1 +
 drivers/fwu-mdata/fwu_mdata_gpt_blk.c | 404 ++
 include/fwu.h |   2 +
 4 files changed, 416 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 7fec7171f4..12a5b4fe04 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 00..329bd3779b
--- /dev/null
+++ b/drivers/fwu-mdata/fwu_mdata_gpt_blk.c
@@ -0,0 +1,404 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2022, Linaro Limited
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#define PRIMARY_PART   BIT(0)
+#define SECONDARY_PART BIT(1)
+#define BOTH_PARTS (PRIMARY_PART | SECONDARY_PART)
+
+#define MDATA_READ BIT(0)
+#define MDATA_WRITEBIT(1)
+
+static int gpt_get_mdata_partitions(struct blk_desc *desc,
+   u16 *primary_mpart,
+   u16 *secondary_mpart)
+{
+   int i, ret;
+   u32 mdata_parts;
+   efi_guid_t part_type_guid;
+   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, ))
+   continue;
+   uuid_str_to_bin(info.type_guid, part_type_guid.b,
+   UUID_STR_FORMAT_GUID);
+
+   if (!guidcmp(_mdata_guid, _type_guid)) {
+   ++mdata_parts;
+   if (!*primary_mpart)
+   *primary_mpart = i;
+   else
+   *secondary_mpart = i;
+   }
+   }
+
+   if (mdata_parts != 2) {
+   log_err("Expect two copies of the FWU metadata instead of %d\n",
+   mdata_parts);
+   ret = -EINVAL;
+   } else {
+   ret = 0;
+   }
+
+   return ret;
+}
+
+static int gpt_get_mdata_disk_part(struct blk_desc *desc,
+  struct disk_partition *info,
+  u32 part_num)
+{
+   int ret;
+   char *mdata_guid_str = "8a7a84a0-8387-40f6-ab41-a8b9a5a60d23";
+
+   ret = part_get_info(desc, part_num, info);
+   if (ret < 0) {
+   log_err("Unable to get the partition info for the FWU metadata 
part %d",
+   part_num);
+   return -1;
+   }
+
+   /* Check that it is indeed the FWU metadata partition */
+   if (!strncmp(info->type_guid, mdata_guid_str, UUID_STR_LEN)) {
+   /* Found the FWU metadata partition */
+   return 0;
+   }
+
+   return -1;
+}
+
+static int gpt_read_write_mdata(struct blk_desc *desc,
+   struct fwu_mdata *mdata,
+   u8 access, u32 part_num)
+{
+   int ret;
+   u32 len, blk_start, blkcnt;
+   struct disk_partition info;
+
+   ALLOC_CACHE_ALIGN_BUFFER_PAD(struct fwu_mdata, mdata_aligned, 1,
+desc->blksz);
+
+   ret = gpt_get_mdata_disk_part(desc, , part_num);
+   if (ret < 0) {
+   printf("Unable to get the FWU metadata partition\n");
+   return -ENODEV;
+   }
+
+   len = sizeof(*mdata);
+   blkcnt = BLOCK_CNT(len, desc);