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

[Jassi Brar]

On Fri, Oct 14, 2022 at 2:07 AM Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> On Sun, Oct 02, 2022 at 06:51:32PM -0500, jassisinghbrar at 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