[PATCH v8 02/13] FWU: Add FWU metadata structure and driver for accessing metadata

[Jassi Brar]

Hi Simon,

On Thu, Aug 18, 2022 at 12:50 PM Simon Glass <sjg at chromium.org> wrote:

> > > > +/**
> > > > + * struct fwu_image_bank_info - firmware image information
> > > > + * @image_uuid: Guid value of the image in this bank
> > > > + * @accepted: Acceptance status of the image
> > > > + * @reserved: Reserved
> > > > + *
> > > > + * The structure contains image specific fields which are
> > > > + * used to identify the image and to specify the image's
> > > > + * acceptance status
> > > > + */
> > > > +struct fwu_image_bank_info {
> > > > +       efi_guid_t  image_uuid;
> > > > +       uint32_t accepted;
> > > > +       uint32_t reserved;
> > > > +} __attribute__((__packed__));
> > >
> > > Why is this packed?
> >
> > This was based on a review comment from Masami [1], as he wanted to
> > use the same structure in the low level bootloader as well.
>
> It doesn't actually make any sense though. The packed struct is the same as
> the normal struct, so far as I can tell. What am I missing?
>
I think because we want the structure to be read/written onto
persistent storage, and possibly manipulated by entities other than
uboot.

I totally agree with all the rest of your comments on the patchset.

cheers!