* [U-Boot] [PATCH] usb: remove unnecessary packed attributes
@ 2012-08-30 15:41 Lucas Stach
2012-08-30 15:49 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: Lucas Stach @ 2012-08-30 15:41 UTC (permalink / raw)
To: u-boot
We don't actually need to pack those structs, as they
are not used to do actual hardware access.
This avoids some unaligned accesses and therefore makes
the EHCI stack work on ARMv7 without compiler workarounds.
Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
drivers/usb/host/ehci-hcd.c | 2 +-
include/usb.h | 2 +-
2 Dateien ge?ndert, 2 Zeilen hinzugef?gt(+), 2 Zeilen entfernt(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 18b4bc6..2f4fa5e 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -45,7 +45,7 @@ static struct descriptor {
struct usb_linux_config_descriptor config;
struct usb_linux_interface_descriptor interface;
struct usb_endpoint_descriptor endpoint;
-} __attribute__ ((packed)) descriptor = {
+} descriptor = {
{
0x8, /* bDescLength */
0x29, /* bDescriptorType: hub descriptor */
diff --git a/include/usb.h b/include/usb.h
index ba3d169..63730ee 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -369,7 +369,7 @@ struct usb_hub_descriptor {
unsigned char PortPowerCtrlMask[(USB_MAXCHILDREN+1+7)/8];
/* DeviceRemovable and PortPwrCtrlMask want to be variable-length
bitmaps that hold max 255 entries. (bit0 is ignored) */
-} __attribute__ ((packed));
+};
struct usb_hub_device {
--
1.7.11.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] usb: remove unnecessary packed attributes
2012-08-30 15:41 [U-Boot] [PATCH] usb: remove unnecessary packed attributes Lucas Stach
@ 2012-08-30 15:49 ` Marek Vasut
2012-08-30 20:59 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: Marek Vasut @ 2012-08-30 15:49 UTC (permalink / raw)
To: u-boot
Dear Lucas Stach,
> We don't actually need to pack those structs, as they
> are not used to do actual hardware access.
>
> This avoids some unaligned accesses and therefore makes
> the EHCI stack work on ARMv7 without compiler workarounds.
>
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
I need to review this properly, will get back to it soon.
Until then, CCing other good fellows.
> ---
> drivers/usb/host/ehci-hcd.c | 2 +-
> include/usb.h | 2 +-
> 2 Dateien ge?ndert, 2 Zeilen hinzugef?gt(+), 2 Zeilen entfernt(-)
>
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 18b4bc6..2f4fa5e 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -45,7 +45,7 @@ static struct descriptor {
> struct usb_linux_config_descriptor config;
> struct usb_linux_interface_descriptor interface;
> struct usb_endpoint_descriptor endpoint;
> -} __attribute__ ((packed)) descriptor = {
> +} descriptor = {
> {
> 0x8, /* bDescLength */
> 0x29, /* bDescriptorType: hub descriptor */
> diff --git a/include/usb.h b/include/usb.h
> index ba3d169..63730ee 100644
> --- a/include/usb.h
> +++ b/include/usb.h
> @@ -369,7 +369,7 @@ struct usb_hub_descriptor {
> unsigned char PortPowerCtrlMask[(USB_MAXCHILDREN+1+7)/8];
> /* DeviceRemovable and PortPwrCtrlMask want to be variable-length
> bitmaps that hold max 255 entries. (bit0 is ignored) */
> -} __attribute__ ((packed));
> +};
>
>
> struct usb_hub_device {
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] usb: remove unnecessary packed attributes
2012-08-30 15:49 ` Marek Vasut
@ 2012-08-30 20:59 ` Marek Vasut
2012-08-30 21:42 ` Lucas Stach
0 siblings, 1 reply; 5+ messages in thread
From: Marek Vasut @ 2012-08-30 20:59 UTC (permalink / raw)
To: u-boot
Dear Marek Vasut,
> Dear Lucas Stach,
>
> > We don't actually need to pack those structs, as they
> > are not used to do actual hardware access.
> >
> > This avoids some unaligned accesses and therefore makes
> > the EHCI stack work on ARMv7 without compiler workarounds.
> >
> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
>
> I need to review this properly, will get back to it soon.
My 7th sense is telling me, these descriptors are sent across the bus.
ehci_submit_root() puts them into buffer and then ehci_submit_async() blasts
them. I believe they must be packed, otherwise the format expected by devices on
the bus won't match and it'll fall apart.
> Until then, CCing other good fellows.
>
> > ---
> >
> > drivers/usb/host/ehci-hcd.c | 2 +-
> > include/usb.h | 2 +-
> > 2 Dateien ge?ndert, 2 Zeilen hinzugef?gt(+), 2 Zeilen entfernt(-)
> >
> > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> > index 18b4bc6..2f4fa5e 100644
> > --- a/drivers/usb/host/ehci-hcd.c
> > +++ b/drivers/usb/host/ehci-hcd.c
> > @@ -45,7 +45,7 @@ static struct descriptor {
> >
> > struct usb_linux_config_descriptor config;
> > struct usb_linux_interface_descriptor interface;
> > struct usb_endpoint_descriptor endpoint;
> >
> > -} __attribute__ ((packed)) descriptor = {
> > +} descriptor = {
> >
> > {
> >
> > 0x8, /* bDescLength */
> > 0x29, /* bDescriptorType: hub descriptor */
> >
> > diff --git a/include/usb.h b/include/usb.h
> > index ba3d169..63730ee 100644
> > --- a/include/usb.h
> > +++ b/include/usb.h
> > @@ -369,7 +369,7 @@ struct usb_hub_descriptor {
> >
> > unsigned char PortPowerCtrlMask[(USB_MAXCHILDREN+1+7)/8];
> > /* DeviceRemovable and PortPwrCtrlMask want to be variable-length
> >
> > bitmaps that hold max 255 entries. (bit0 is ignored) */
> >
> > -} __attribute__ ((packed));
> > +};
> >
> > struct usb_hub_device {
>
> Best regards,
> Marek Vasut
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] usb: remove unnecessary packed attributes
2012-08-30 20:59 ` Marek Vasut
@ 2012-08-30 21:42 ` Lucas Stach
2012-08-30 21:56 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: Lucas Stach @ 2012-08-30 21:42 UTC (permalink / raw)
To: u-boot
Dear Marek,
it seems you are right. Though it works for me this may be pure luck. So
please disregard this patch, I'll fix this up more properly.
Thanks,
Lucas
Am Donnerstag, den 30.08.2012, 22:59 +0200 schrieb Marek Vasut:
> Dear Marek Vasut,
>
> > Dear Lucas Stach,
> >
> > > We don't actually need to pack those structs, as they
> > > are not used to do actual hardware access.
> > >
> > > This avoids some unaligned accesses and therefore makes
> > > the EHCI stack work on ARMv7 without compiler workarounds.
> > >
> > > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> >
> > I need to review this properly, will get back to it soon.
>
> My 7th sense is telling me, these descriptors are sent across the bus.
> ehci_submit_root() puts them into buffer and then ehci_submit_async() blasts
> them. I believe they must be packed, otherwise the format expected by devices on
> the bus won't match and it'll fall apart.
>
> > Until then, CCing other good fellows.
> >
> > > ---
> > >
> > > drivers/usb/host/ehci-hcd.c | 2 +-
> > > include/usb.h | 2 +-
> > > 2 Dateien ge?ndert, 2 Zeilen hinzugef?gt(+), 2 Zeilen entfernt(-)
> > >
> > > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> > > index 18b4bc6..2f4fa5e 100644
> > > --- a/drivers/usb/host/ehci-hcd.c
> > > +++ b/drivers/usb/host/ehci-hcd.c
> > > @@ -45,7 +45,7 @@ static struct descriptor {
> > >
> > > struct usb_linux_config_descriptor config;
> > > struct usb_linux_interface_descriptor interface;
> > > struct usb_endpoint_descriptor endpoint;
> > >
> > > -} __attribute__ ((packed)) descriptor = {
> > > +} descriptor = {
> > >
> > > {
> > >
> > > 0x8, /* bDescLength */
> > > 0x29, /* bDescriptorType: hub descriptor */
> > >
> > > diff --git a/include/usb.h b/include/usb.h
> > > index ba3d169..63730ee 100644
> > > --- a/include/usb.h
> > > +++ b/include/usb.h
> > > @@ -369,7 +369,7 @@ struct usb_hub_descriptor {
> > >
> > > unsigned char PortPowerCtrlMask[(USB_MAXCHILDREN+1+7)/8];
> > > /* DeviceRemovable and PortPwrCtrlMask want to be variable-length
> > >
> > > bitmaps that hold max 255 entries. (bit0 is ignored) */
> > >
> > > -} __attribute__ ((packed));
> > > +};
> > >
> > > struct usb_hub_device {
> >
> > Best regards,
> > Marek Vasut
>
> Best regards,
> Marek Vasut
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] usb: remove unnecessary packed attributes
2012-08-30 21:42 ` Lucas Stach
@ 2012-08-30 21:56 ` Marek Vasut
0 siblings, 0 replies; 5+ messages in thread
From: Marek Vasut @ 2012-08-30 21:56 UTC (permalink / raw)
To: u-boot
Dear Lucas Stach,
> Dear Marek,
>
> it seems you are right. Though it works for me this may be pure luck. So
> please disregard this patch, I'll fix this up more properly.
Cool thing we caught it :-)
http://www.denx.de/wiki/U-Boot/Patches btw read up here ... don't top-post ...
you know the drill.
> Thanks,
> Lucas
>
> Am Donnerstag, den 30.08.2012, 22:59 +0200 schrieb Marek Vasut:
> > Dear Marek Vasut,
> >
> > > Dear Lucas Stach,
> > >
> > > > We don't actually need to pack those structs, as they
> > > > are not used to do actual hardware access.
> > > >
> > > > This avoids some unaligned accesses and therefore makes
> > > > the EHCI stack work on ARMv7 without compiler workarounds.
> > > >
> > > > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > >
> > > I need to review this properly, will get back to it soon.
> >
> > My 7th sense is telling me, these descriptors are sent across the bus.
> > ehci_submit_root() puts them into buffer and then ehci_submit_async()
> > blasts them. I believe they must be packed, otherwise the format
> > expected by devices on the bus won't match and it'll fall apart.
> >
> > > Until then, CCing other good fellows.
> > >
> > > > ---
> > > >
> > > > drivers/usb/host/ehci-hcd.c | 2 +-
> > > > include/usb.h | 2 +-
> > > > 2 Dateien ge?ndert, 2 Zeilen hinzugef?gt(+), 2 Zeilen entfernt(-)
> > > >
> > > > diff --git a/drivers/usb/host/ehci-hcd.c
> > > > b/drivers/usb/host/ehci-hcd.c index 18b4bc6..2f4fa5e 100644
> > > > --- a/drivers/usb/host/ehci-hcd.c
> > > > +++ b/drivers/usb/host/ehci-hcd.c
> > > > @@ -45,7 +45,7 @@ static struct descriptor {
> > > >
> > > > struct usb_linux_config_descriptor config;
> > > > struct usb_linux_interface_descriptor interface;
> > > > struct usb_endpoint_descriptor endpoint;
> > > >
> > > > -} __attribute__ ((packed)) descriptor = {
> > > > +} descriptor = {
> > > >
> > > > {
> > > >
> > > > 0x8, /* bDescLength */
> > > > 0x29, /* bDescriptorType: hub descriptor */
> > > >
> > > > diff --git a/include/usb.h b/include/usb.h
> > > > index ba3d169..63730ee 100644
> > > > --- a/include/usb.h
> > > > +++ b/include/usb.h
> > > > @@ -369,7 +369,7 @@ struct usb_hub_descriptor {
> > > >
> > > > unsigned char PortPowerCtrlMask[(USB_MAXCHILDREN+1+7)/8];
> > > > /* DeviceRemovable and PortPwrCtrlMask want to be variable-
length
> > > >
> > > > bitmaps that hold max 255 entries. (bit0 is ignored) */
> > > >
> > > > -} __attribute__ ((packed));
> > > > +};
> > > >
> > > > struct usb_hub_device {
> > >
> > > Best regards,
> > > Marek Vasut
> >
> > Best regards,
> > Marek Vasut
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-08-30 21:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-30 15:41 [U-Boot] [PATCH] usb: remove unnecessary packed attributes Lucas Stach
2012-08-30 15:49 ` Marek Vasut
2012-08-30 20:59 ` Marek Vasut
2012-08-30 21:42 ` Lucas Stach
2012-08-30 21:56 ` Marek Vasut
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox