Re: [PATCHv2 1/5] FWU: Add FWU metadata access driver for MTD storage regions

On Fri, Oct 14, 2022 at 2:07 AM Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

On Sun, Oct 02, 2022 at 06:51:32PM -0500, jassisinghbrar@gmail.com wrote:

...

+static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size) +{

• return !do_div(size, mtd->erasesize);
  

+}

Can we please add some sphinx style documentation overall ?

I can but I thought that is for public api and not static helper functions?

...

+static int fwu_mtd_check_mdata(struct udevice *dev) +{

• struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev);
  

• struct fwu_mdata primary, secondary;
  

• bool pri = false, sec = false;
  

• int ret;
  

• ret = fwu_mtd_load_mdata(mtd_priv->mtd, &primary,
  

•                                                  mtd_priv->pri_offset, FWU_MDATA_PRIMARY);
  

• if (ret < 0)
  

•         log_debug("Failed to read the primary mdata: %d\n", ret);
  

• else
  

•         pri = true;
  

• ret = fwu_mtd_load_mdata(mtd_priv->mtd, &secondary,
  

•                                                  mtd_priv->sec_offset, FWU_MDATA_SECONDARY);
  

• if (ret < 0)
  

•         log_debug("Failed to read the secondary mdata: %d\n", ret);
  

• else
  

•         sec = true;
  

• if (pri && sec) {
  

•         if (memcmp(&primary, &secondary, sizeof(struct fwu_mdata))) {
  

•                 log_debug("The primary and the secondary mdata are different\n");
  

•                 ret = -1;
  

•         }
  

• } else if (pri) {
  

•         ret = fwu_mtd_save_secondary_mdata(mtd_priv, &primary);
  

•         if (ret < 0)
  

•                 log_debug("Restoring secondary mdata partition failed\n");
  

• } else if (sec) {
  

•         ret = fwu_mtd_save_primary_mdata(mtd_priv, &secondary);
  

•         if (ret < 0)
  

•                 log_debug("Restoring primary mdata partition failed\n");
  

• }
  

Same on this one. The requirements here are

• Read our metadata
• Compare the 2 partitions

The only thing that's 'hardware' specific here is reading the metadata. We should at least unify the comparing part and restoration in case of failures, no ?

Yes. Since redundant copy of meta-data is a requirement of the spec, we should maintain that in the fwu core code. Maybe something like adding 'bool primary' argument to get_mdata() and update_mdata()

thanks