On Wed, Mar 1, 2023 at 5:16 AM Etienne Carriere etienne.carriere@linaro.org wrote:
On Tue, 28 Feb 2023 at 01:52, jassisinghbrar@gmail.com wrote:
From: Jassi Brar jaswinder.singh@linaro.org
Instead of each i/f having to implement their own meta-data verification and storage, move the logic in common code. This simplifies the i/f code much simpler and compact.
...
diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c b/drivers/fwu-mdata/fwu-mdata-uclass.c index b477e9603f..e03773c584 100644 --- a/drivers/fwu-mdata/fwu-mdata-uclass.c +++ b/drivers/fwu-mdata/fwu-mdata-uclass.c @@ -16,6 +16,40 @@ #include <linux/types.h> #include <u-boot/crc.h>
+/**
- fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata()
- Return: 0 if OK, -ve on error
- */
Inline description only in header file, or duplicated in source and header files?
This is the original practice in FWU stack - to have the description in header as well as source code. I just didn't want to stick out.
diff --git a/include/fwu.h b/include/fwu.h index 0919ced812..1a700c9e6a 100644 --- a/include/fwu.h +++ b/include/fwu.h @@ -24,6 +24,26 @@ struct fwu_mdata_gpt_blk_priv {
- @update_mdata() - Update the FWU metadata copy
*/ struct fwu_mdata_ops {
/*** read_mdata() - Populate the asked FWU metadata copy* @dev: FWU metadata device* @mdata: Copy of the FWU metadata@mdata: Output FWU mdata read
* @primary: If primary or secondary copy of meta-data is to be reads/meta-data/FWU metadata/ Ditto in .write_mdata description
ok
+/**
- fwu_get_verified_mdata() - Read, verify and return the FWU metadata
- Read both the metadata copies from the storage media, verify their checksum,
- and ascertain that both copies match. If one of the copies has gone bad,
- restore it from the good copy.
@mdata: Output FWU metadata read or NULL
ok
+static int fwu_sync_mdata(struct fwu_mdata *mdata, int part) +{
void *buf = &mdata->version;int err;if (part == BOTH_PARTS) {err = fwu_sync_mdata(mdata, SECONDARY_PART);if (err)return err;part = PRIMARY_PART;}/** Calculate the crc32 for the updated FWU metadata* and put the updated value in the FWU metadata crc32* field*/mdata->crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32));err = fwu_write_mdata(g_dev, mdata, part & PRIMARY_PART ? true : false);Since this expects part is either PRIMARY_PART or SECONDARY_PART, prefer: err = fwu_write_mdata(g_dev, mdata, part == PRIMARY_PART);
And ditto below: part == PRIMARY_PART ? "primary": "secondary");
yes, now that we handle out the BOTH_PARTS case already.
+int fwu_get_verified_mdata(struct fwu_mdata *mdata) +{
int err;bool parts_ok[2] = { false };struct fwu_mdata s, *parts_mdata[2];parts_mdata[0] = &g_mdata;parts_mdata[1] = &s;/* if mdata already read and ready */err = mdata_crc_check(parts_mdata[0]);if (!err)goto ret_mdata;/* else read, verify and, if needed, fix mdata */for (int i = 0; i < 2; i++) {Define "int i;" at function block entry?
Hmm... I prefer this way - limiting scope of the scratch variables.
thanks.