[PATCH v4 6/6] fwu: DeveloperBox: add support for FWU

[Jassi Brar]

On Wed, 29 Mar 2023 at 08:02, Michal Simek <michal.simek at amd.com> wrote:
> On 3/27/23 23:16, jassisinghbrar at gmail.com wrote:

.....
> > +
> > +void fwu_plat_get_bootidx(uint *boot_idx)
> > +{
> > +     int ret;
> > +     u32 active_idx;
> > +     u32 *bootidx = boot_idx;
> > +
> > +     ret = fwu_get_active_index(&active_idx);
> > +
>
> nit: remove this newline
>
ok

> > +     if (ret < 0)
> > +             *bootidx = -1;
> > +
> > +     *bootidx = active_idx;
>
> Is this logic here right?
> If fwu_get_active_index fails you setup bootidx to -1
> and right after it you rewrite it to active_idx initialized in
> fwu_get_active_index() to mdata->active_index.
>
> It means why to do *bootidx = -1; at all?
>
yes :) it's a silly remnant of history of changes.
Actually this goes away after implementing the default/weak function.


> > +}
> > diff --git a/configs/synquacer_developerbox_defconfig b/configs/synquacer_developerbox_defconfig
> > index 09e12b739b..d09684153a 100644
> > --- a/configs/synquacer_developerbox_defconfig
> > +++ b/configs/synquacer_developerbox_defconfig
> > @@ -97,3 +97,11 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> >   CONFIG_EFI_CAPSULE_ON_DISK=y
> >   CONFIG_EFI_IGNORE_OSINDICATIONS=y
> >   CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> > +CONFIG_EFI_SECURE_BOOT=y
> > +CONFIG_FWU_MULTI_BANK_UPDATE=y
> > +CONFIG_FWU_MDATA=y
> > +CONFIG_FWU_MDATA_MTD=y
> > +CONFIG_FWU_NUM_BANKS=2
> > +CONFIG_FWU_NUM_IMAGES_PER_BANK=1
> > +CONFIG_CMD_FWU_METADATA=y
> > +CONFIG_TOOLS_MKFWUMDATA=y
>
> doesn't look like that it was created via savedefconfig.
>
Yes. I had some other config changes too and picked only the relevant
ones together.


> > +And make a FIP image.::
> > +
> > +  cp build/synquacer/release/fip.bin SPI_NOR_NEWFIP.fd
> > +  tools/fiptool/fiptool update --tb-fw build/synquacer/release/bl2.bin SPI_NOR_NEWFIP.fd
> > +
> > +UUIDs for the FWU Multi Bank Update
> > +-----------------------------------
> > +
> > +FWU multi-bank update requires some UUIDs. The DeveloperBox platform uses
> > +following UUIDs.
> > +
> > + - Location UUID for the FIP image: 17e86d77-41f9-4fd7-87ec-a55df9842de5
>
>
> In past you have it listed at flash node in DT. I see you have removed it
> between v3 and v4 without any note about it.
> Is it still needed? And should it be listed in DT spec again?
>
After the dt change, we no longer require this. But the location_uuid
is a standard member of an fwu_image_entry and cmd/fwu_mdata.c always
print it. So I think this should be seen as just what a platform wants
some unique id to be printed for the image (?).

Thanks Michal for reviewing v4.
cheers.