[PATCH v2] libertas: Fix alignment issues in libertas core

[Colin McCabe]

The argument for specifying struct layout explicitly is pretty clear.
If we relied on gcc to get it right "by accident," we would find that
gcc upgrades and minor code changes would break the driver in
mysterious ways.

I'm surprised that gcc generates poorer code when the layout is
specified explicitly, since after all, it should be the same layout.
Have you tried using __attribute__ ((aligned (8))) or similar?

regards,
Colin


On Wed, Jan 28, 2009 at 10:47 AM, Erwin Authried <eauth at softsys.co.at> wrote:
> Am Mittwoch, den 28.01.2009, 07:49 -0800 schrieb Andrey Yurovsky:
>> On Wed, Jan 28, 2009 at 12:34 AM, Erwin Authried <eauth at softsys.co.at> wrote:
>> > is it really necessary to pack all structures in hostcmd.h? The "packed"
>> > attribute makes the code larger and slower. I have changed the order in
>> > the lbs_private structure in dev.h a little bit so that resp_buf is on a
>> > 32-bit boundary. Then, I have removed most of the packed attributes in
>> > hostcmd.h (where all 16 or 32-bit Variables are on 16/32 bit boundaries
>> > anyway). The libertas module is a bit smaller, and I haven't seen any
>> > problem.
>>
>> Thanks Erwin.  What CPU or CPUs have you tested your changes on?  In
>> our case, the Blackfin (and, reportedly, the AVR32) needed us to
>> guarantee alignment whereas ARM and x86 worked fine as things were
>> before.  Also although it's less efficient, we can now add commands
>> more safely if we use the packed attribute consistently rather than
>> counting bytes, and there's a good chance that more host command
>> structures will be added in the near future.
>
> I have tested with ARM7TDMI (AT91FR40162), there is no aligment trap.
> The cpu simply ignores the lowest address bits and produces wrong
> assignments when the alignment isn't correct.
>
> -Erwin
>
>>
>> > Am Montag, den 12.01.2009, 13:14 -0800 schrieb Andrey Yurovsky:
>> >> Data structures that come over the wire from the WLAN firmware must be packed.
>> >> This fixes alignment problems on the blackfin architecture and, reportedly, on
>> >> the AVR32.
>> >>
>> >> This is a replacement for the previous version of this patch which had also
>> >> explicitly used get_unaligned_ macros.  As Johannes Berg pointed out, these
>> >> macros were unnecessary.
>> >>
> --
> Dipl.-Ing. Erwin Authried
> Softwareentwicklung und Systemdesign
>
>
> _______________________________________________
> libertas-dev mailing list
> libertas-dev at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/libertas-dev
>