From: Martin Blumenstingl
> Sent: 04 January 2023 15:30
>
> Hi Ping-Ke, Hi David,
>
> On Sun, Jan 1, 2023 at 2:09 PM Ping-Ke Shih <pkshih@xxxxxxxxxxx> wrote:
> [...]
> > Yes, it should not use bit filed. Instead, use a __le16 for all fields, such as
> I think this can be done in a separate patch.
> My v2 of this patch has reduced these changes to a minimum, see [0]
>
> [...]
> > struct rtw8821ce_efuse {
> > ...
> > u8 data1; // offset 0x100
> > __le16 data2; // offset 0x101-0x102
> > ...
> > } __packed;
> >
> > Without __packed, compiler could has pad between data1 and data2,
> > and then get wrong result.
> My understanding is that this is the reason why we need __packed.
True, but does it really have to look like that?
I can't find that version (I don't have a net_next tree).
Possibly it should be 'u8 data2[2];'
Most hardware definitions align everything.
What you may want to do is add compile-time asserts for the
sizes of the structures.
Remember that if you have 16/32 bit fields in packed structures
on some architectures the compile has to generate code that does
byte loads and shifts.
The 'misaligned' property is lost when you take the address - so
you can easily generate a fault.
Adding __packed to a struct is a sledgehammer you really shouldn't need.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
