Hi Jacopo,
On Wed, Mar 13, 2024 at 01:41:09PM +0100, Jacopo Mondi wrote:
> > > > > +struct pisp_be_global_config {
> > > > > + __u32 bayer_enables;
> > > > > + __u32 rgb_enables;
> > > > > + __u8 bayer_order;
> > > > > + __u8 pad[3];
> > > > > +} __attribute__((packed));
> > > >
> > > > I wonder what is the current recommendation on packing the structs on
> > > > different ABIs. On some archs (e.g. ARM) this involves more inefficient
> > > > access of data on these structs and it would seem like that there are no
> > > > direct struct layout related implications from packing apart from the main
> > > > struct embedding other structs.
> > > >
> > > > The V4L2 IOCTL arguments have used packing just to be sure there are no
> > > > holes but I wonder if it makes sense here. I've argued for this, too, but
> > > > drawbacks exist as well.
> > > >
> > > > Any thoughts?
> > > >
> > > > How about checking this with pahole?
> > >
> > > I've run this through Hans' scripts as reported in the cover letter
> > >
> > > pahole: ABI OK
> > >
> > > But as Naush replied, this gets applied directly to the HW registers
> > > layout, so packing is needed
> >
> > Not really for that reason, no, but this is up to you.
> >
>
> For my education, why isn't the fact that this struct gets applied
> directly to HW a good reason to use packing ?
It's certainly a reason. But even without packing, the same can be often
achieved by relying on host CPU ABIs where Linux runs. This isn't an option
of course if you have e.g. misaligned fields but this is perhaps more
common in firmware ABIs.
As I noted, this is up to you.
--
Regards,
Sakari Ailus
