Re: [PATCH v6 02/13] FWU: Add FWU metadata structure and driver for accessing metadata

2022-07-13 Thread Sughosh Ganu
hi Patrick,

On Wed, 13 Jul 2022 at 18:42, Patrick DELAUNAY
 wrote:
>
> Hi,
>
> On 7/4/22 07:16, Sughosh Ganu wrote:
> > In the FWU Multi Bank Update feature, the information about the
> > updatable images is stored as part of the metadata, which is stored on
> > a dedicated partition. Add the metadata structure, and a driver model
> > uclass which provides functions to access the metadata. These are
> > generic API's, and implementations can be added based on parameters
> > like how the metadata partition is accessed and what type of storage
> > device houses the metadata.
> >
> > Signed-off-by: Sughosh Ganu 
> > ---
> > Changes since V5:
> > * Change the parameter to the function fwu_plat_get_alt_num to pass
> >the FWU udevice pointer instead of passing the metadata device
> >directly.
> >
> >   drivers/Kconfig  |   2 +
> >   drivers/Makefile |   1 +
> >   drivers/fwu-mdata/Kconfig|   7 +
> >   drivers/fwu-mdata/Makefile   |   6 +
> >   drivers/fwu-mdata/fwu-mdata-uclass.c | 458 +++
> >   include/dm/uclass-id.h   |   1 +
> >   include/fwu.h|  49 +++
> >   include/fwu_mdata.h  |  67 
> >   8 files changed, 591 insertions(+)
> >   create mode 100644 drivers/fwu-mdata/Kconfig
> >   create mode 100644 drivers/fwu-mdata/Makefile
> >   create mode 100644 drivers/fwu-mdata/fwu-mdata-uclass.c
> >   create mode 100644 include/fwu.h
> >   create mode 100644 include/fwu_mdata.h
> >
> > diff --git a/drivers/Kconfig b/drivers/Kconfig
> > index b26ca8cf70..adc6079ecf 100644
> > --- a/drivers/Kconfig
> > +++ b/drivers/Kconfig
> > @@ -42,6 +42,8 @@ source "drivers/firmware/Kconfig"
> >
> >   source "drivers/fpga/Kconfig"
> >
> > +source "drivers/fwu-mdata/Kconfig"
> > +
> >   source "drivers/gpio/Kconfig"
> >
> >   source "drivers/hwspinlock/Kconfig"
> > diff --git a/drivers/Makefile b/drivers/Makefile
> > index 67c8af7442..901150bb35 100644
> > --- a/drivers/Makefile
> > +++ b/drivers/Makefile
> > @@ -83,6 +83,7 @@ obj-y += cache/
> >   obj-$(CONFIG_CPU) += cpu/
> >   obj-y += crypto/
> >   obj-$(CONFIG_FASTBOOT) += fastboot/
> > +obj-$(CONFIG_DM_FWU_MDATA) += fwu-mdata/
> >   obj-y += misc/
> >   obj-$(CONFIG_MMC) += mmc/
> >   obj-$(CONFIG_NVME) += nvme/
> > diff --git a/drivers/fwu-mdata/Kconfig b/drivers/fwu-mdata/Kconfig
> > new file mode 100644
> > index 00..d6a21c8e19
> > --- /dev/null
> > +++ b/drivers/fwu-mdata/Kconfig
> > @@ -0,0 +1,7 @@
> > +config DM_FWU_MDATA
> > + bool "Driver support for accessing FWU Metadata"
> > + depends on DM
> > + help
> > +   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.
> > diff --git a/drivers/fwu-mdata/Makefile b/drivers/fwu-mdata/Makefile
> > new file mode 100644
> > index 00..e53a8c9983
> > --- /dev/null
> > +++ b/drivers/fwu-mdata/Makefile
> > @@ -0,0 +1,6 @@
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +#
> > +# Copyright (c) 2022, Linaro Limited
> > +#
> > +
> > +obj-$(CONFIG_DM_FWU_MDATA) += fwu-mdata-uclass.o
> > diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c 
> > b/drivers/fwu-mdata/fwu-mdata-uclass.c
> > new file mode 100644
> > index 00..2092fcfc23
> > --- /dev/null
> > +++ b/drivers/fwu-mdata/fwu-mdata-uclass.c
> > @@ -0,0 +1,458 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2022, Linaro Limited
> > + */
> > +
>
> #define LOG_CATEGORY UCLASS_FWU_MDATA

Will add. Thanks.

-sughosh

>
>
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define IMAGE_ACCEPT_SET BIT(0)
> > +#define IMAGE_ACCEPT_CLEAR   BIT(1)
> > +
> > +static int fwu_get_dev_ops(struct udevice **dev,
> > +const struct fwu_mdata_ops **ops)
> > +{
> > + int ret;
> > +
> > + ret = uclass_get_device(UCLASS_FWU_MDATA, 0, dev);
> > + if (ret) {
> > + log_debug("Cannot find fwu device\n");
> > + return ret;
> > + }
> > +
> > + if ((*ops = device_get_ops(*dev)) == NULL) {
> > + log_debug("Cannot get fwu device ops\n");
> > + return -ENOSYS;
> > + }
> > +
> > + return 0;
> > +}
> > +
>
> (...)
>
> with this minor update
>
> Reviewed-by: Patrick Delaunay 
>
> Thanks
> Patrick
>


Re: [PATCH v6 02/13] FWU: Add FWU metadata structure and driver for accessing metadata

2022-07-13 Thread Patrick DELAUNAY

Hi,

On 7/4/22 07:16, Sughosh Ganu wrote:

In the FWU Multi Bank Update feature, the information about the
updatable images is stored as part of the metadata, which is stored on
a dedicated partition. Add the metadata structure, and a driver model
uclass which provides functions to access the metadata. These are
generic API's, and implementations can be added based on parameters
like how the metadata partition is accessed and what type of storage
device houses the metadata.

Signed-off-by: Sughosh Ganu 
---
Changes since V5:
* Change the parameter to the function fwu_plat_get_alt_num to pass
   the FWU udevice pointer instead of passing the metadata device
   directly.

  drivers/Kconfig  |   2 +
  drivers/Makefile |   1 +
  drivers/fwu-mdata/Kconfig|   7 +
  drivers/fwu-mdata/Makefile   |   6 +
  drivers/fwu-mdata/fwu-mdata-uclass.c | 458 +++
  include/dm/uclass-id.h   |   1 +
  include/fwu.h|  49 +++
  include/fwu_mdata.h  |  67 
  8 files changed, 591 insertions(+)
  create mode 100644 drivers/fwu-mdata/Kconfig
  create mode 100644 drivers/fwu-mdata/Makefile
  create mode 100644 drivers/fwu-mdata/fwu-mdata-uclass.c
  create mode 100644 include/fwu.h
  create mode 100644 include/fwu_mdata.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index b26ca8cf70..adc6079ecf 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -42,6 +42,8 @@ source "drivers/firmware/Kconfig"
  
  source "drivers/fpga/Kconfig"
  
+source "drivers/fwu-mdata/Kconfig"

+
  source "drivers/gpio/Kconfig"
  
  source "drivers/hwspinlock/Kconfig"

diff --git a/drivers/Makefile b/drivers/Makefile
index 67c8af7442..901150bb35 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -83,6 +83,7 @@ obj-y += cache/
  obj-$(CONFIG_CPU) += cpu/
  obj-y += crypto/
  obj-$(CONFIG_FASTBOOT) += fastboot/
+obj-$(CONFIG_DM_FWU_MDATA) += fwu-mdata/
  obj-y += misc/
  obj-$(CONFIG_MMC) += mmc/
  obj-$(CONFIG_NVME) += nvme/
diff --git a/drivers/fwu-mdata/Kconfig b/drivers/fwu-mdata/Kconfig
new file mode 100644
index 00..d6a21c8e19
--- /dev/null
+++ b/drivers/fwu-mdata/Kconfig
@@ -0,0 +1,7 @@
+config DM_FWU_MDATA
+   bool "Driver support for accessing FWU Metadata"
+   depends on DM
+   help
+ 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.
diff --git a/drivers/fwu-mdata/Makefile b/drivers/fwu-mdata/Makefile
new file mode 100644
index 00..e53a8c9983
--- /dev/null
+++ b/drivers/fwu-mdata/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# Copyright (c) 2022, Linaro Limited
+#
+
+obj-$(CONFIG_DM_FWU_MDATA) += fwu-mdata-uclass.o
diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c 
b/drivers/fwu-mdata/fwu-mdata-uclass.c
new file mode 100644
index 00..2092fcfc23
--- /dev/null
+++ b/drivers/fwu-mdata/fwu-mdata-uclass.c
@@ -0,0 +1,458 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022, Linaro Limited
+ */
+


#define LOG_CATEGORY UCLASS_FWU_MDATA



+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define IMAGE_ACCEPT_SET   BIT(0)
+#define IMAGE_ACCEPT_CLEAR BIT(1)
+
+static int fwu_get_dev_ops(struct udevice **dev,
+  const struct fwu_mdata_ops **ops)
+{
+   int ret;
+
+   ret = uclass_get_device(UCLASS_FWU_MDATA, 0, dev);
+   if (ret) {
+   log_debug("Cannot find fwu device\n");
+   return ret;
+   }
+
+   if ((*ops = device_get_ops(*dev)) == NULL) {
+   log_debug("Cannot get fwu device ops\n");
+   return -ENOSYS;
+   }
+
+   return 0;
+}
+


(...)

with this minor update

Reviewed-by: Patrick Delaunay 

Thanks
Patrick