Re: [PATCH v5 13/23] FWU: Add FWU metadata access driver for non-GPT MTD devices

2022-06-21 Thread Patrick DELAUNAY

Hi,

On 6/9/22 14:30, Sughosh Ganu wrote:

From: Masami Hiramatsu 

For the platform which doesn't have GPT partitions for the firmware
but on MTD devices, the FWU metadata is stored on MTD device as raw
image at specific offset. This driver gives the access methods
for the FWU metadata information on such MTD devices.

Signed-off-by: Masami Hiramatsu 
Signed-off-by: Sughosh Ganu 
---
  drivers/fwu-mdata/Kconfig |   8 +
  drivers/fwu-mdata/Makefile|   1 +
  drivers/fwu-mdata/fwu_mdata_mtd.c | 308 ++
  3 files changed, 317 insertions(+)
  create mode 100644 drivers/fwu-mdata/fwu_mdata_mtd.c

diff --git a/drivers/fwu-mdata/Kconfig b/drivers/fwu-mdata/Kconfig
index d5edef19d6..a8fa9ad783 100644
--- a/drivers/fwu-mdata/Kconfig
+++ b/drivers/fwu-mdata/Kconfig
@@ -14,3 +14,11 @@ config FWU_MDATA_GPT_BLK
help
  Enable support for accessing FWU Metadata on GPT partitioned
  block devices.
+
+config FWU_MDATA_MTD
+   bool "FWU Metadata access for non-GPT MTD devices"
+   depends on DM_FWU_MDATA && MTD
+   help
+ Enable support for accessing FWU Metadata on non-partitioned
+ (or non-GPT partitioned, e.g. partition nodes in devicetree)
+ MTD devices.
diff --git a/drivers/fwu-mdata/Makefile b/drivers/fwu-mdata/Makefile
index 12a5b4fe04..c574c59be2 100644
--- a/drivers/fwu-mdata/Makefile
+++ b/drivers/fwu-mdata/Makefile
@@ -5,3 +5,4 @@
  
  obj-$(CONFIG_DM_FWU_MDATA) += fwu-mdata-uclass.o

  obj-$(CONFIG_FWU_MDATA_GPT_BLK) += fwu_mdata_gpt_blk.o
+obj-$(CONFIG_FWU_MDATA_MTD) += fwu_mdata_mtd.o
diff --git a/drivers/fwu-mdata/fwu_mdata_mtd.c 
b/drivers/fwu-mdata/fwu_mdata_mtd.c
new file mode 100644
index 00..9eb471e73e
--- /dev/null
+++ b/drivers/fwu-mdata/fwu_mdata_mtd.c
@@ -0,0 +1,308 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2022, Linaro Limited
+ */
+


+ #define LOG_CATEGORY UCLASS_FWU_MDATA

it is requested for log command to filter by uclass


the next include is not needed ?

#include 


+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+struct fwu_mdata_mtd_priv {
+   struct mtd_info *mtd;
+   u32 pri_offset;
+   u32 sec_offset;
+};
+
+enum fwu_mtd_op {
+   FWU_MTD_READ,
+   FWU_MTD_WRITE,
+};
+
+static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size)
+{
+   return !do_div(size, mtd->erasesize);
+}
+
+static int mtd_io_data(struct mtd_info *mtd, u32 offs, u32 size, void *data,
+  enum fwu_mtd_op op)
+{
+   struct mtd_oob_ops io_op ={};



missing space : ' = {};'


+   u64 lock_offs, lock_len;
+   size_t len;
+   void *buf;
+   int ret;
+
+   if (!mtd_is_aligned_with_block_size(mtd, offs))
+   return -EINVAL;
+   lock_offs = offs;
+   lock_len = round_up(size, mtd->erasesize);
+
+   ret = mtd_unlock(mtd, lock_offs, lock_len);
+   if (ret && ret != -EOPNOTSUPP)
+   return ret;
+
+   if (op == FWU_MTD_WRITE) {
+   struct erase_info erase_op = {};
+
+   /* This will expand erase size to align with the block size */
+   erase_op.mtd = mtd;
+   erase_op.addr = lock_offs;
+   erase_op.len = lock_len;
+   erase_op.scrub = 0;
+
+   ret = mtd_erase(mtd, &erase_op);
+   if (ret)
+   goto lock_out;
+   }
+
+   /* Also, expand the write size to align with the write size */
+   len = round_up(size, mtd->writesize);
+
+   buf = memalign(ARCH_DMA_MINALIGN, len);
+   if (!buf) {
+   ret = -ENOMEM;
+   goto lock_out;
+   }
+   io_op.mode = MTD_OPS_AUTO_OOB;
+   io_op.len = len;
+   io_op.ooblen = 0;
+   io_op.datbuf = buf;
+   io_op.oobbuf = NULL;
+
+   if (op == FWU_MTD_WRITE) {
+   memcpy(buf, data, size);
+   ret = mtd_write_oob(mtd, offs, &io_op);
+   } else {
+   ret = mtd_read_oob(mtd, offs, &io_op);
+   if (!ret)
+   memcpy(data, buf, size);
+   }
+   free(buf);
+
+lock_out:
+   mtd_lock(mtd, lock_offs, lock_len);
+
+   return ret;
+}
+
+static int fwu_mtd_load_mdata(struct mtd_info *mtd, struct fwu_mdata **mdata,
+ u32 offs, bool primary)
+{
+   size_t size = sizeof(struct fwu_mdata);
+   int ret;
+
+   *mdata = malloc(size);
+   if (!*mdata)
+   return -ENOMEM;
+
+   ret = mtd_io_data(mtd, offs, size, (void *)*mdata, FWU_MTD_READ);
+   if (ret >= 0) {
+   ret = fwu_verify_mdata(*mdata, primary);
+   if (ret < 0) {
+   free(*mdata);
+   *mdata = NULL;
+   }
+   }
+
+   return ret;
+}
+
+static int fwu_mtd_load_primary_mdata(struct fwu_mdata_mtd_priv *mtd_priv,
+   

Re: [PATCH v5 13/23] FWU: Add FWU metadata access driver for non-GPT MTD devices

2022-06-21 Thread Etienne Carriere
On Thu, 9 Jun 2022 at 14:31, Sughosh Ganu  wrote:
>
> From: Masami Hiramatsu 
>
> For the platform which doesn't have GPT partitions for the firmware
> but on MTD devices, the FWU metadata is stored on MTD device as raw
> image at specific offset. This driver gives the access methods
> for the FWU metadata information on such MTD devices.
>
> Signed-off-by: Masami Hiramatsu 
> Signed-off-by: Sughosh Ganu 
> ---
>  drivers/fwu-mdata/Kconfig |   8 +
>  drivers/fwu-mdata/Makefile|   1 +
>  drivers/fwu-mdata/fwu_mdata_mtd.c | 308 ++
>  3 files changed, 317 insertions(+)
>  create mode 100644 drivers/fwu-mdata/fwu_mdata_mtd.c
>
> diff --git a/drivers/fwu-mdata/Kconfig b/drivers/fwu-mdata/Kconfig
> index d5edef19d6..a8fa9ad783 100644
> --- a/drivers/fwu-mdata/Kconfig
> +++ b/drivers/fwu-mdata/Kconfig
> @@ -14,3 +14,11 @@ config FWU_MDATA_GPT_BLK
> help
>   Enable support for accessing FWU Metadata on GPT partitioned
>   block devices.
> +
> +config FWU_MDATA_MTD
> +   bool "FWU Metadata access for non-GPT MTD devices"
> +   depends on DM_FWU_MDATA && MTD
> +   help
> + Enable support for accessing FWU Metadata on non-partitioned
> + (or non-GPT partitioned, e.g. partition nodes in devicetree)
> + MTD devices.
> diff --git a/drivers/fwu-mdata/Makefile b/drivers/fwu-mdata/Makefile
> index 12a5b4fe04..c574c59be2 100644
> --- a/drivers/fwu-mdata/Makefile
> +++ b/drivers/fwu-mdata/Makefile
> @@ -5,3 +5,4 @@
>
>  obj-$(CONFIG_DM_FWU_MDATA) += fwu-mdata-uclass.o
>  obj-$(CONFIG_FWU_MDATA_GPT_BLK) += fwu_mdata_gpt_blk.o
> +obj-$(CONFIG_FWU_MDATA_MTD) += fwu_mdata_mtd.o
> diff --git a/drivers/fwu-mdata/fwu_mdata_mtd.c 
> b/drivers/fwu-mdata/fwu_mdata_mtd.c
> new file mode 100644
> index 00..9eb471e73e
> --- /dev/null
> +++ b/drivers/fwu-mdata/fwu_mdata_mtd.c
> @@ -0,0 +1,308 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2022, Linaro Limited
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +struct fwu_mdata_mtd_priv {
> +   struct mtd_info *mtd;
> +   u32 pri_offset;
> +   u32 sec_offset;
> +};

Add an inline description of the fields.

> +
> +enum fwu_mtd_op {
> +   FWU_MTD_READ,
> +   FWU_MTD_WRITE,
> +};
> +
> +static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size)
> +{
> +   return !do_div(size, mtd->erasesize);
> +}
> +
> +static int mtd_io_data(struct mtd_info *mtd, u32 offs, u32 size, void *data,
> +  enum fwu_mtd_op op)
> +{
> +   struct mtd_oob_ops io_op ={};
> +   u64 lock_offs, lock_len;
> +   size_t len;
> +   void *buf;
> +   int ret;
> +
> +   if (!mtd_is_aligned_with_block_size(mtd, offs))
> +   return -EINVAL;
> +   lock_offs = offs;
> +   lock_len = round_up(size, mtd->erasesize);
> +
> +   ret = mtd_unlock(mtd, lock_offs, lock_len);
> +   if (ret && ret != -EOPNOTSUPP)
> +   return ret;
> +
> +   if (op == FWU_MTD_WRITE) {
> +   struct erase_info erase_op = {};
> +
> +   /* This will expand erase size to align with the block size */
> +   erase_op.mtd = mtd;
> +   erase_op.addr = lock_offs;
> +   erase_op.len = lock_len;
> +   erase_op.scrub = 0;
> +
> +   ret = mtd_erase(mtd, &erase_op);
> +   if (ret)
> +   goto lock_out;
> +   }
> +
> +   /* Also, expand the write size to align with the write size */
> +   len = round_up(size, mtd->writesize);
> +
> +   buf = memalign(ARCH_DMA_MINALIGN, len);
> +   if (!buf) {
> +   ret = -ENOMEM;
> +   goto lock_out;
> +   }
> +   io_op.mode = MTD_OPS_AUTO_OOB;
> +   io_op.len = len;
> +   io_op.ooblen = 0;
> +   io_op.datbuf = buf;
> +   io_op.oobbuf = NULL;
> +
> +   if (op == FWU_MTD_WRITE) {
> +   memcpy(buf, data, size);
> +   ret = mtd_write_oob(mtd, offs, &io_op);
> +   } else {
> +   ret = mtd_read_oob(mtd, offs, &io_op);
> +   if (!ret)
> +   memcpy(data, buf, size);
> +   }
> +   free(buf);
> +
> +lock_out:
> +   mtd_lock(mtd, lock_offs, lock_len);
> +
> +   return ret;
> +}
> +
> +static int fwu_mtd_load_mdata(struct mtd_info *mtd, struct fwu_mdata **mdata,
> + u32 offs, bool primary)
> +{
> +   size_t size = sizeof(struct fwu_mdata);
> +   int ret;
> +
> +   *mdata = malloc(size);
> +   if (!*mdata)
> +   return -ENOMEM;
> +
> +   ret = mtd_io_data(mtd, offs, size, (void *)*mdata, FWU_MTD_READ);
> +   if (ret >= 0) {
> +   ret = fwu_verify_mdata(*mdata, primary);
> +   if (ret < 0) {
> +   free(*mdata);

[PATCH v5 13/23] FWU: Add FWU metadata access driver for non-GPT MTD devices

2022-06-09 Thread Sughosh Ganu
From: Masami Hiramatsu 

For the platform which doesn't have GPT partitions for the firmware
but on MTD devices, the FWU metadata is stored on MTD device as raw
image at specific offset. This driver gives the access methods
for the FWU metadata information on such MTD devices.

Signed-off-by: Masami Hiramatsu 
Signed-off-by: Sughosh Ganu 
---
 drivers/fwu-mdata/Kconfig |   8 +
 drivers/fwu-mdata/Makefile|   1 +
 drivers/fwu-mdata/fwu_mdata_mtd.c | 308 ++
 3 files changed, 317 insertions(+)
 create mode 100644 drivers/fwu-mdata/fwu_mdata_mtd.c

diff --git a/drivers/fwu-mdata/Kconfig b/drivers/fwu-mdata/Kconfig
index d5edef19d6..a8fa9ad783 100644
--- a/drivers/fwu-mdata/Kconfig
+++ b/drivers/fwu-mdata/Kconfig
@@ -14,3 +14,11 @@ config FWU_MDATA_GPT_BLK
help
  Enable support for accessing FWU Metadata on GPT partitioned
  block devices.
+
+config FWU_MDATA_MTD
+   bool "FWU Metadata access for non-GPT MTD devices"
+   depends on DM_FWU_MDATA && MTD
+   help
+ Enable support for accessing FWU Metadata on non-partitioned
+ (or non-GPT partitioned, e.g. partition nodes in devicetree)
+ MTD devices.
diff --git a/drivers/fwu-mdata/Makefile b/drivers/fwu-mdata/Makefile
index 12a5b4fe04..c574c59be2 100644
--- a/drivers/fwu-mdata/Makefile
+++ b/drivers/fwu-mdata/Makefile
@@ -5,3 +5,4 @@
 
 obj-$(CONFIG_DM_FWU_MDATA) += fwu-mdata-uclass.o
 obj-$(CONFIG_FWU_MDATA_GPT_BLK) += fwu_mdata_gpt_blk.o
+obj-$(CONFIG_FWU_MDATA_MTD) += fwu_mdata_mtd.o
diff --git a/drivers/fwu-mdata/fwu_mdata_mtd.c 
b/drivers/fwu-mdata/fwu_mdata_mtd.c
new file mode 100644
index 00..9eb471e73e
--- /dev/null
+++ b/drivers/fwu-mdata/fwu_mdata_mtd.c
@@ -0,0 +1,308 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2022, Linaro Limited
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+struct fwu_mdata_mtd_priv {
+   struct mtd_info *mtd;
+   u32 pri_offset;
+   u32 sec_offset;
+};
+
+enum fwu_mtd_op {
+   FWU_MTD_READ,
+   FWU_MTD_WRITE,
+};
+
+static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size)
+{
+   return !do_div(size, mtd->erasesize);
+}
+
+static int mtd_io_data(struct mtd_info *mtd, u32 offs, u32 size, void *data,
+  enum fwu_mtd_op op)
+{
+   struct mtd_oob_ops io_op ={};
+   u64 lock_offs, lock_len;
+   size_t len;
+   void *buf;
+   int ret;
+
+   if (!mtd_is_aligned_with_block_size(mtd, offs))
+   return -EINVAL;
+   lock_offs = offs;
+   lock_len = round_up(size, mtd->erasesize);
+
+   ret = mtd_unlock(mtd, lock_offs, lock_len);
+   if (ret && ret != -EOPNOTSUPP)
+   return ret;
+
+   if (op == FWU_MTD_WRITE) {
+   struct erase_info erase_op = {};
+
+   /* This will expand erase size to align with the block size */
+   erase_op.mtd = mtd;
+   erase_op.addr = lock_offs;
+   erase_op.len = lock_len;
+   erase_op.scrub = 0;
+
+   ret = mtd_erase(mtd, &erase_op);
+   if (ret)
+   goto lock_out;
+   }
+
+   /* Also, expand the write size to align with the write size */
+   len = round_up(size, mtd->writesize);
+
+   buf = memalign(ARCH_DMA_MINALIGN, len);
+   if (!buf) {
+   ret = -ENOMEM;
+   goto lock_out;
+   }
+   io_op.mode = MTD_OPS_AUTO_OOB;
+   io_op.len = len;
+   io_op.ooblen = 0;
+   io_op.datbuf = buf;
+   io_op.oobbuf = NULL;
+
+   if (op == FWU_MTD_WRITE) {
+   memcpy(buf, data, size);
+   ret = mtd_write_oob(mtd, offs, &io_op);
+   } else {
+   ret = mtd_read_oob(mtd, offs, &io_op);
+   if (!ret)
+   memcpy(data, buf, size);
+   }
+   free(buf);
+
+lock_out:
+   mtd_lock(mtd, lock_offs, lock_len);
+
+   return ret;
+}
+
+static int fwu_mtd_load_mdata(struct mtd_info *mtd, struct fwu_mdata **mdata,
+ u32 offs, bool primary)
+{
+   size_t size = sizeof(struct fwu_mdata);
+   int ret;
+
+   *mdata = malloc(size);
+   if (!*mdata)
+   return -ENOMEM;
+
+   ret = mtd_io_data(mtd, offs, size, (void *)*mdata, FWU_MTD_READ);
+   if (ret >= 0) {
+   ret = fwu_verify_mdata(*mdata, primary);
+   if (ret < 0) {
+   free(*mdata);
+   *mdata = NULL;
+   }
+   }
+
+   return ret;
+}
+
+static int fwu_mtd_load_primary_mdata(struct fwu_mdata_mtd_priv *mtd_priv,
+struct fwu_mdata **mdata)
+{
+   return fwu_mtd_load_mdata(mtd_priv->mtd, mdata, mtd_priv->pri_offset, 
true);
+}
+
+static int fwu_mtd_load_secondary_mdata(struct fwu_mdata_mtd_priv *mtd_priv,
+