Hi Sughosh,
On Thu, 23 Jun 2022 at 08:24, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Etienne,
On Tue, 21 Jun 2022 at 16:24, Etienne Carriere etienne.carriere@linaro.org wrote:
Hello Sughosh,
On Thu, 9 Jun 2022 at 14:30, Sughosh Ganu sughosh.ganu@linaro.org 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 sughosh.ganu@linaro.org
drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/fwu-mdata/Kconfig | 7 + drivers/fwu-mdata/Makefile | 6 + drivers/fwu-mdata/fwu-mdata-uclass.c | 459 +++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/fwu.h | 49 +++ include/fwu_mdata.h | 67 ++++ 8 files changed, 592 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
<snip>
diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c b/drivers/fwu-mdata/fwu-mdata-uclass.c new file mode 100644 index 0000000000..1530ceb01d --- /dev/null +++ b/drivers/fwu-mdata/fwu-mdata-uclass.c
<snip>
+/**
- fwu_get_mdata() - Get a FWU metadata copy
- @mdata: Copy of the FWU metadata
- Get a valid copy of the FWU metadata.
- Return: 0 if OK, -ve on error
- */
+int fwu_get_mdata(struct fwu_mdata **mdata)
Is there a real need for this function to allocate an instance of struct mdata. I think it would be clearer if it was the caller's responsibility to allocate/free the structure.
Or maybe rename this function fwu_alloc_and_copy_mdata() to highlight that the function gives an allocated copy of the data.
I guess I can put a comment in the function description saying that the function is responsible for the allocation of the metadata structure.
I think it would be better.
One should be careful when calling these API functions as some act on a local copy (retrieved from fw_get_mdata()) while other functions modify straight fwu-mdata in the storage media.
Did you find any function which is modifying the metadata on the storage device directly. The API fwu_update_mdata() is supposed to be doing that. If you have come across any function which is directly modifying the metadata on the storage media, please let me know and I will fix it.
Many functions do so: fwu_clear_accept_image(), fwu_clear_accept_image(), fwu_resert_boot_index(), etc... Actually all generic functions do so while only fwu_get_mdata() and fwu_update_mdata() act on a RAM copy.
Maybe fwu-mdata ops should have a status field for when a RAM copy was exported and used to prevent direct updates to mdata in storage until caller releases (fw_put_mdata()?) the exposed copy. Would this scheme be overkilling...
Or maybe fwu_clear_accept_image() and other helper functions could also require a mdata RAM reference to act on, letting the caller also go through fwu_get_mdata()/fwu_update_mdata().
etienne
<snip>