Patchwork [U-Boot] USB: Remove __attribute__ ((packed)) for struct ehci_hccr and ehci_hcor.

login
register
mail settings
Submitter Alexander Holler
Date April 1, 2011, 10:35 p.m.
Message ID <1301697344-4033-1-git-send-email-holler@ahsoftware.de>
Download mbox | patch
Permalink /patch/89362/
State Superseded
Delegated to: Marek Vasut
Headers show

Comments

Alexander Holler - April 1, 2011, 10:35 p.m.
Remove __attribute__ ((packed)) to prevent byte access to soc
registers in some gcc versions.

Having patches to enable ehci for the BeagleBoard lying around for
several month, this one was the show-stopper.

Credits have to go to Laine Walker-Avina <lwalkera@ieee.org> for
finding the problem.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 drivers/usb/host/ehci.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Wolfgang Denk - April 1, 2011, 10:53 p.m.
Dear Alexander Holler,

In message <1301697344-4033-1-git-send-email-holler@ahsoftware.de> you wrote:
> Remove __attribute__ ((packed)) to prevent byte access to soc
> registers in some gcc versions.
> 
> Having patches to enable ehci for the BeagleBoard lying around for
> several month, this one was the show-stopper.
> 
> Credits have to go to Laine Walker-Avina <lwalkera@ieee.org> for
> finding the problem.
> 
> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
> ---
>  drivers/usb/host/ehci.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

Can you PLEASE start playing by the rules?

This applies to you as well:
http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions

Why do you repost this stuff without any comments, versioning or
changelog?

Best regards,

Wolfgang Denk
Alexander Holler - April 1, 2011, 11:14 p.m.
Hello,

Am 02.04.2011 00:53, schrieb Wolfgang Denk:

> Why do you repost this stuff without any comments, versioning or
> changelog?

Because someone broke the TWL4030-RTC in 2.6.38 and those 4 message got 
send with a date from 2000.
That means those will never seen in the archives (for 2009), and not by 
people which are sorting their mailboxes by date.

So I thought it is good idea to resend that.

Regards,

Alexander Holler
Wolfgang Denk - April 1, 2011, 11:35 p.m.
Dear Alexander Holler,

In message <4D965C71.3010308@ahsoftware.de> you wrote:
> Hello,
> 
> Am 02.04.2011 00:53, schrieb Wolfgang Denk:
> 
> > Why do you repost this stuff without any comments, versioning or
> > changelog?
> 
> Because someone broke the TWL4030-RTC in 2.6.38 and those 4 message got 
> send with a date from 2000.
> That means those will never seen in the archives (for 2009), and not by 
> people which are sorting their mailboxes by date.
> 
> So I thought it is good idea to resend that.

Please reread: "... without any comments, versioning or changelog?"

Best regards,

Wolfgang Denk
Alexander Holler - April 1, 2011, 11:39 p.m.
Am 02.04.2011 01:35, schrieb Wolfgang Denk:
> Dear Alexander Holler,
>
> In message<4D965C71.3010308@ahsoftware.de>  you wrote:
>> Hello,
>>
>> Am 02.04.2011 00:53, schrieb Wolfgang Denk:
>>
>>> Why do you repost this stuff without any comments, versioning or
>>> changelog?
>>
>> Because someone broke the TWL4030-RTC in 2.6.38 and those 4 message got
>> send with a date from 2000.
>> That means those will never seen in the archives (for 2009), and not by
>> people which are sorting their mailboxes by date.
>>
>> So I thought it is good idea to resend that.
>
> Please reread: "... without any comments, versioning or changelog?"

I promise to not send any patches anymore.

Thanks,

Alexander Holler
Alexander Holler - April 2, 2011, 8:32 a.m.
Hello,

Am 02.04.2011 00:35, schrieb Alexander Holler:
> Remove __attribute__ ((packed)) to prevent byte access to soc
> registers in some gcc versions.
>
> Having patches to enable ehci for the BeagleBoard lying around for
> several month, this one was the show-stopper.
>
> Credits have to go to Laine Walker-Avina<lwalkera@ieee.org>  for
> finding the problem.
>
> Signed-off-by: Alexander Holler<holler@ahsoftware.de>
> ---
>   drivers/usb/host/ehci.h |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index 945ab64..df9f055 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -55,7 +55,7 @@ struct ehci_hccr {
>   #define HCS_N_PORTS(p)		(((p)>>  0)&  0xf)
>   	uint32_t cr_hccparams;
>   	uint8_t cr_hcsp_portrt[8];
> -} __attribute__ ((packed));
> +};
>
>   struct ehci_hcor {
>   	uint32_t or_usbcmd;
> @@ -85,7 +85,7 @@ struct ehci_hcor {
>   #define FLAG_CF		(1<<  0)	/* true:  we'll support "high speed" */
>   	uint32_t or_portsc[CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS];
>   	uint32_t or_systune;
> -} __attribute__ ((packed));
> +};
>
>   #define USBMODE		0x68		/* USB Device mode */
>   #define USBMODE_SDIS	(1<<  3)	/* Stream disable */

Before I'm killing someones 64bit+ machine with that patch:

The hint I've received was to use

+} __attribute__ ((packed, aligned(4)));

That works too. I haven't seen the original commit (I've only got told 
that credits have to go to Laine Walker-Avina), I don't know if the 
standard says something to that, I don't know if some 64bit+ SoC might 
choose to align that stuff otherwise and I will not send a v2 of that patch.

Regards,

Alexander

Patch

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 945ab64..df9f055 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -55,7 +55,7 @@  struct ehci_hccr {
 #define HCS_N_PORTS(p)		(((p) >> 0) & 0xf)
 	uint32_t cr_hccparams;
 	uint8_t cr_hcsp_portrt[8];
-} __attribute__ ((packed));
+};
 
 struct ehci_hcor {
 	uint32_t or_usbcmd;
@@ -85,7 +85,7 @@  struct ehci_hcor {
 #define FLAG_CF		(1 << 0)	/* true:  we'll support "high speed" */
 	uint32_t or_portsc[CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS];
 	uint32_t or_systune;
-} __attribute__ ((packed));
+};
 
 #define USBMODE		0x68		/* USB Device mode */
 #define USBMODE_SDIS	(1 << 3)	/* Stream disable */