Thread (64 messages) flat view 64 messages, 7 authors, 2011-06-29

[PATCH] USB: ehci: use packed, aligned(4) instead of removing the packed attribute

From: arnd@arndb.de (Arnd Bergmann)
Date: 2011-06-20 20:39:56
Also in: lkml

On Monday 20 June 2011 22:28:49 Nicolas Pitre wrote:
On Mon, 20 Jun 2011, Arnd Bergmann wrote:
quoted
On Monday 20 June 2011 19:39:34 Alexander Holler wrote:
quoted
That packed without an additional aligned() caused errors on ARM with 
gcc 4.6 is another problem which got (currently) fixed by removing packed.
Packed caused errors because it is *wrong*. The code as it was used undefined
behavior in the language.
I wouldn't call this issue as such, but this is a Red herring.

Could you please provide a pointer to the structure definition so a 
second opinion to the usefulness of __packed there could be provided?  
The structures in question are ehci_caps, ehci_regs and ehci_dbg_port.
The patch that remove the __packed attribute was 139540170 "USB: ehci:
remove structure packing from ehci_def".

The reason why I consider it a bug is that an access to a register
using readl/writel on the structure requires casting a pointer with
byte alignment to a pointer with word alignment, which is undefined
in C. Gcc just tries to be helpful and work around this by turning
the access into bytewise load/store instructions. In older gcc versions,
it would not do that if you happen to also case from non-volatile to
volatile pointer, but according to Uli that was not an intentional
feature of gcc but the ARM code just worked by pure coincidence.
If it is not matching any of the fairly limited cases where having 
__packed is relevant then we can just confirm that it should go.
It's already gone.

	Arnd
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help