[PATCH v2] drivers/misc: Add ASpeed LPC control driver
Greg KH
gregkh at linuxfoundation.org
Mon Jan 16 21:45:37 AEDT 2017
On Sun, Jan 15, 2017 at 10:45:15PM -0600, Benjamin Herrenschmidt wrote:
> On Mon, 2017-01-16 at 09:43 +1100, Cyril Bur wrote:
> > > > +struct aspeed_lpc_ctrl_mapping {
> > > > + __u8 window_type;
> > > > + __u8 window_id;
> > > > + __u32 addr;
> > > > + __u32 offset;
> > > > + __u32 size;
> > >
> > > That's some crazy alignment, do you really mean to put a 32bit value
> > > aligned like that? Will it work properly on your systems?
> > >
> >
> > Well, in my probably unrealistic and rushed testing it did work - but
> > then this was all with one compiler on one machine so I'm not
> > surprised. I'll put the u8s at the end.
>
> This structure isn't packed, so the alignment is fine, the compiler
> will align "addr" to a 32-bit boundary. What might have been better
> would have be to be explicit about it by adding two u8 of padding
> (or a u16) but the above works. Moving things around just makes the
> structure more weird, the window and id make more sense being at the
> beginning.
For an ioctl structure like this, can you guarantee that the "padding"
will always be the same? Last time I looked, the C standard didn't say
anything about that, so I would strongly recommend making it so that no
padding is needed at all...
thanks,
greg k-h
More information about the openbmc
mailing list