Patchwork [U-Boot,v2] exynos: ehci: added missing 'packed' attribute

login
register
mail settings
Submitter Mateusz Zalega
Date Aug. 21, 2013, 11:58 a.m.
Message ID <1377086283-727-1-git-send-email-m.zalega@samsung.com>
Download mbox | patch
Permalink /patch/268795/
State New
Delegated to: Minkyu Kang
Headers show

Comments

Mateusz Zalega - Aug. 21, 2013, 11:58 a.m.
Structure exynos_usb_phy lacked __attribute__ ((packed)), which might
have led to broken EHCI functionality in builds based on more recent
compilers.

Signed-off-by: Mateusz Zalega <m.zalega@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Minkyu Kang <mk7.kang@samsung.com>
Cc: Rajeshwari Shinde <rajeshwari.s@samsung.com>
---
Changes since v1:
- changed linux/compiler-gcc.h to linux/compiler.h

---
 arch/arm/include/asm/arch-exynos/ehci.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Mateusz Zalega - Aug. 21, 2013, 4:45 p.m.
On 08/21/13 13:58, Mateusz Zalega wrote:
> Structure exynos_usb_phy lacked __attribute__ ((packed)), which might
> have led to broken EHCI functionality in builds based on more recent
> compilers.
> 
> Signed-off-by: Mateusz Zalega <m.zalega@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Minkyu Kang <mk7.kang@samsung.com>
> Cc: Rajeshwari Shinde <rajeshwari.s@samsung.com>
> ---
> Changes since v1:
> - changed linux/compiler-gcc.h to linux/compiler.h
> 
> ---
>  arch/arm/include/asm/arch-exynos/ehci.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/arch-exynos/ehci.h b/arch/arm/include/asm/arch-exynos/ehci.h
> index d79f25c..c965b2c 100644
> --- a/arch/arm/include/asm/arch-exynos/ehci.h
> +++ b/arch/arm/include/asm/arch-exynos/ehci.h
> @@ -9,6 +9,7 @@
>  
>  #ifndef __ASM_ARM_ARCH_EHCI_H__
>  #define __ASM_ARM_ARCH_EHCI_H__
> +#include <linux/compiler.h>
>  
>  #define CLK_24MHZ		5
>  
> @@ -45,7 +46,7 @@ struct exynos_usb_phy {
>  	unsigned int usbotgsys;
>  	unsigned int reserved4;
>  	unsigned int usbotgtune;
> -};
> +} __packed;
>  
>  /* Switch on the VBUS power. */
>  int board_usb_vbus_init(void);
> 

I may have reacted too fast.

I assumed that we should keep __packed attribute here, because we're
referring to platform register and we'd better underline our need to
keep compiler from tampering with our layout.

It turned out this code will work just fine, because ARM ABI mandates
correct layout of structure members in such cases:

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/Babjddhe.html

It wouldn't work if there were any bitfields or unaligned members in
Exynos register structures, but after more careful examination I can
tell there are none.

Regards,

Patch

diff --git a/arch/arm/include/asm/arch-exynos/ehci.h b/arch/arm/include/asm/arch-exynos/ehci.h
index d79f25c..c965b2c 100644
--- a/arch/arm/include/asm/arch-exynos/ehci.h
+++ b/arch/arm/include/asm/arch-exynos/ehci.h
@@ -9,6 +9,7 @@ 
 
 #ifndef __ASM_ARM_ARCH_EHCI_H__
 #define __ASM_ARM_ARCH_EHCI_H__
+#include <linux/compiler.h>
 
 #define CLK_24MHZ		5
 
@@ -45,7 +46,7 @@  struct exynos_usb_phy {
 	unsigned int usbotgsys;
 	unsigned int reserved4;
 	unsigned int usbotgtune;
-};
+} __packed;
 
 /* Switch on the VBUS power. */
 int board_usb_vbus_init(void);