[PATCHv2 1/4] fwu: gpt: use cached meta-data partition numbers

[Etienne Carriere]

On Mon, 2 Jan 2023 at 18:15, Jassi Brar <jaswinder.singh at linaro.org> wrote:
>
> On Thu, 22 Dec 2022 at 06:45, Ilias Apalodimas
> <ilias.apalodimas at linaro.org> wrote:
> >
> > On Fri, Dec 02, 2022 at 09:16:51PM -0600, jassisinghbrar at gmail.com wrote:
> > > From: Jassi Brar <jaswinder.singh at linaro.org>
> > >
> > > Use cached values and avoid parsing and scanning through partitions
> > > everytime for meta-data partitions because they can't change after bootup.
> > >
> > > Acked-by: Etienne Carriere <etienne.carriere at linaro.org>
> > > Signed-off-by: Jassi Brar <jaswinder.singh at linaro.org>
> > > ---
> > >  drivers/fwu-mdata/gpt_blk.c | 43 +++++++++++++++++++++----------------
> > >  1 file changed, 24 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/fwu-mdata/gpt_blk.c b/drivers/fwu-mdata/gpt_blk.c
> > > index d35ce49c5c..28f5d23e1e 100644
> > > --- a/drivers/fwu-mdata/gpt_blk.c
> > > +++ b/drivers/fwu-mdata/gpt_blk.c
> > > @@ -24,8 +24,9 @@ enum {
> > >       MDATA_WRITE,
> > >  };
> > >
> > > -static int gpt_get_mdata_partitions(struct blk_desc *desc,
> > > -                                 uint mdata_parts[2])
> > > +static uint g_mdata_part[2]; /* = {0, 0} to check against uninit parts */
> > > +
> > > +static int gpt_get_mdata_partitions(struct blk_desc *desc)
> > >  {
> > >       int i, ret;
> > >       u32 nparts;
> > > @@ -33,18 +34,19 @@ static int gpt_get_mdata_partitions(struct blk_desc *desc,
> > >       struct disk_partition info;
> > >       const efi_guid_t fwu_mdata_guid = FWU_MDATA_GUID;
> > >
> > > +     /* if primary and secondary partitions already found */
> > > +     if (g_mdata_part[0] && g_mdata_part[1])
> > > +             return 0;
> > > +
> > >       nparts = 0;
> > > -     for (i = 1; i < MAX_SEARCH_PARTITIONS; i++) {
> > > +     for (i = 1; i < MAX_SEARCH_PARTITIONS && nparts < 2; i++) {
> > >               if (part_get_info(desc, i, &info))
> > >                       continue;
> > >               uuid_str_to_bin(info.type_guid, part_type_guid.b,
> > >                               UUID_STR_FORMAT_GUID);
> > >
> > > -             if (!guidcmp(&fwu_mdata_guid, &part_type_guid)) {
> > > -                     if (nparts < 2)
> > > -                             mdata_parts[nparts] = i;
> > > -                     ++nparts;
> > > -             }
> > > +             if (!guidcmp(&fwu_mdata_guid, &part_type_guid))
> > > +                     g_mdata_part[nparts++] = i;
> >
> > The reason the 'if (nparts < 2)' was outside the main loop was to show
> > errors in case the user defined more than two partitions.  Can we keep it
> > like that or am I the only being paranoid here?
> >
> I am not sure if that is an 'error', because FWU only cares about the
> first two partitions it comes across. There is no way other ones could
> impact the operations.
> And if fwu code is responsible for correctness of the setup, there are
> things that we already don't care about. So maybe we keep the code
> simple?
>
> cheers.

I agree with Jassi. We can keep code simple here. The embedded code is
not designed to valide DT content but to use it.

Etienne