Re: [PATCH anybus v3 4/6] bus: support HMS Anybus-S bus

From: Sven Van Asbroeck
Date: Fri Nov 09 2018 - 11:25:24 EST


On Thu, Nov 8, 2018 at 9:07 AM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> > +struct anybus_mbox_hdr {
> > + u16 id;
> > + u16 info;
> > + u16 cmd_num;
> > + u16 data_size;
> > + u16 frame_count;
> > + u16 frame_num;
> > + u16 offset_high;
> > + u16 offset_low;
> > + u16 extended[8];
> > +} __packed;
>
> I don't think you want this to be __packed, it won't change the layout
> but instead force byte accesses on architectures that don't have
> fast unaligned load/store instructions.
>
> Instead of the __packed, it's almost always better to ensure that
> the structure itself is aligned to a 16-bit address.
>

A general question about __packed.

My current understanding is this:
(please tell me if it's incorrect or incomplete)

+ without __packed, the compiler is free to pad the struct in whatever
way it feels is best.
+ with __packed, the fields have to be laid out EXACTLY as specified.

Is it possible that someone will compile this on an architecture that 'likes'
16-bit ints to be 32-bit aligned? If such an architecture does not currently
exist, could it appear in the future?

If the compiler changes the padding in the struct, the driver will
break, as the struct layout is tightly defined by the anybus spec.

Would it be better to stay safe, and keep __packed in case of such
tightly defined structures?