Re: [PATCH v5 19/23] FWU: synquacer: Add FWU Multi bank update support for DeveloperBox

Jassi Brar
2022-07-19T15:31:27Z

On Mon, Jul 18, 2022 at 4:00 PM Tom Rini <trini@konsulko.com> wrote:
> On Mon, Jul 18, 2022 at 10:31:56AM -0500, Jassi Brar wrote:

> > > > > > > > +
> > > > > > > > +#define PLAT_METADATA_OFFSET 0x510000
> > > > > > > > +#define PLAT_METADATA_SIZE (sizeof(struct devbox_metadata))
> > > > > > > > +
> > > > > > > > +struct __packed devbox_metadata {
> > > > > > > > + u32 boot_index;
> > > > > > > > + u32 boot_count;
> > > > > > >
> > > > > > > There is the whole bootcount infrastructure for this. I think it would be much
> > > > > > > better to use that framework instead of creating parallel one.
> > > > > > >
> > > > > > Yes, this goes too.
> > > > >
> > > > > Is bootcount really suited for this case?
> > > > > AFAIK bootcount either requires device specific registers (which won't
> > > > > reset on reboots), or an environment you can write data to.
> > > > > But what if a user wants to disable writing the env variables and the
> > > > > device doesn't have a set of registers we can use?
> > > > >
> > > > Maybe it should be moved in 'struct fwu_mdata' ?
> > >
> > > I was mostly thinking on moving this count as another 'bootcount'
> > > method. So in case the user has disabled writing evn variables but he
> > > is booting with EFI he can use that.
> >
> > Sorry, not sure I understand.... IIUIC there has to be some persistent storage.
>
> No, there just has to be "somewhere" to do the counting. We've got a
> DDR backed driver, for example. So yes, I think we should try and use
> the bootcount framework here.
>
OK, for platforms that can preserve ram across reboot, using
non-persistent storage can work.
My platform neither preserves ram, nor has any warmreset-proof
registers. So I have to choose between saving the bootcount in efi-env
or in vendor specific structure next to the metadata. I prefer
metadata because it is common to all stages of boot. Any corrections
to this approach?

Thanks