Hi Jassi,
On Fri, 19 Aug 2022 at 08:59, Jassi Brar <jassisinghb...@gmail.com> wrote:
>
> Hi Simon,
>
> On Thu, Aug 18, 2022 at 12:50 PM Simon Glass <s...@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.
But specifically, how does __packed help here? What are the other
entities doing that changes the format and what is __packed doing to
update that? This is actually standard C code.
>
> I totally agree with all the rest of your comments on the patchset.
Regards,
Simon
