USB: Remove attribute ((packed)) for struct ehci_hccr and ehci_hcor.

PATCH New Alexander Holler 2 participants 6 messages Apr 1, 2011 – Apr 2, 2011

Subsystems (2)

USB USB ehci driver

lore

Share & tools

lore

Alexander Holler

author

2011-04-01 22:35 UTC #0 lore raw

PATCH 1/1 USB: Remove __attribute__ ((packed)) for struct ehci_hccr and ehci_hcor.

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 <[email protected]> for finding the problem.

Signed-off-by Alexander Holler <[email protected]>

---
 drivers/usb/host/ehci.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/usb/host/ehci.h +2 -2

index 945ab64..df9f055 100644
@@ -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 */

Signature · 1 line

1.7.3.4

Wolfgang Denk

2011-04-01 22:53 UTC #1 lore raw

PATCH 1/1 USB: Remove __attribute__ ((packed)) for struct ehci_hccr and ehci_hcor.

Dear Alexander Holler,

Quoted 6 lines · In message <[email protected]> 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.

Quoted 7 lines

Credits have to go to Laine Walker-Avina <[email protected]> for finding the problem.
Signed-off-by Alexander Holler <[email protected]>
---
 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

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Crash programs fail because they are based on the theory  that,  with
nine women pregnant, you can get a baby a month.  - Wernher von Braun

Alexander Holler

author

2011-04-01 23:14 UTC #2 lore raw

PATCH 1/1 USB: Remove __attribute__ ((packed)) for struct ehci_hccr and ehci_hcor.

Hello,

Am 02.04.2011 00:53, schrieb Wolfgang Denk:

Quoted 2 lines

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

2011-04-01 23:35 UTC #3 lore raw

PATCH 1/1 USB: Remove __attribute__ ((packed)) for struct ehci_hccr and ehci_hcor.

Dear Alexander Holler,

Quoted 7 lines · In message <[email protected]> you wrote:

Hello,
Am 02.04.2011 00:53, schrieb Wolfgang Denk:
Quoted 2 lines
Why do you repost this stuff without any comments, versioning or changelog?

Quoted 6 lines

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

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The value of marriage is not that adults produce children, but  that
children produce adults."                            - Peter De Vries

Alexander Holler

author

2011-04-01 23:39 UTC #4 lore raw

PATCH 1/1 USB: Remove __attribute__ ((packed)) for struct ehci_hccr and ehci_hcor.

Am 02.04.2011 01:35, schrieb Wolfgang Denk:

Quoted 10 lines

Dear Alexander Holler,
Quoted 7 lines · In message<[email protected]> you wrote:
Hello,
Am 02.04.2011 00:53, schrieb Wolfgang Denk:
Quoted 2 lines
Why do you repost this stuff without any comments, versioning or changelog?

Quoted 8 lines

Quoted 6 lines
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

author

2011-04-02 08:32 UTC #5 lore raw

PATCH 1/1 USB: Remove __attribute__ ((packed)) for struct ehci_hccr and ehci_hcor.

In reply to Alexander Holler

Hello,

Am 02.04.2011 00:35, schrieb Alexander Holler:

Quoted 28 lines

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<[email protected]> for finding the problem.
Signed-off-by Alexander Holler<[email protected]>
---
  drivers/usb/host/ehci.h |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)
drivers/usb/host/ehci.h +1 -1
index 945ab64..df9f055 100644
@@ -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;

drivers/usb/host/ehci.h struct ehci_hcor { · line 91

Quoted 8 lines

drivers/usb/host/ehci.h +1 -1

@@ -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