* [U-Boot] [PATCH 1/1] USB: Fix strict aliasing in ohci-hcd
@ 2012-08-11 21:49 Troy Kisky
2012-08-12 0:07 ` Marek Vasut
2012-08-14 17:33 ` Marek Vasut
0 siblings, 2 replies; 5+ messages in thread
From: Troy Kisky @ 2012-08-11 21:49 UTC (permalink / raw)
To: u-boot
commit 5f6aa03fda2a0a79940765865c1e4266be8a75f8
USB: Fix complaints about strict aliasing in OHCI-HCD
tried to fix this, but gcc4.4 still complains. So, this
patch basically reverts the above and does a simpler fix.
also, the above commit incorrectly changed
/* corresponds to data_buf[4-7] */
datab [1] = 0;
to
/* corresponds to databuf.u8[4-7] */
databuf.u8[1] = 0;
This patch also fixes that.
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
drivers/usb/host/ohci-hcd.c | 70 +++++++++++++++++++------------------------
1 file changed, 31 insertions(+), 39 deletions(-)
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index d24f2f1..9f47351 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1261,19 +1261,11 @@ static int ohci_submit_rh_msg(struct usb_device *dev, unsigned long pipe,
int leni = transfer_len;
int len = 0;
int stat = 0;
- __u32 datab[4];
- union {
- void *ptr;
- __u8 *u8;
- __u16 *u16;
- __u32 *u32;
- } databuf;
__u16 bmRType_bReq;
__u16 wValue;
__u16 wIndex;
__u16 wLength;
-
- databuf.u32 = (__u32 *)datab;
+ ALLOC_ALIGN_BUFFER(__u8, databuf, 16, sizeof(u32));
#ifdef DEBUG
pkt_print(NULL, dev, pipe, buffer, transfer_len,
@@ -1304,20 +1296,20 @@ pkt_print(NULL, dev, pipe, buffer, transfer_len,
*/
case RH_GET_STATUS:
- databuf.u16[0] = cpu_to_le16(1);
+ *(u16 *)databuf = cpu_to_le16(1);
OK(2);
case RH_GET_STATUS | RH_INTERFACE:
- databuf.u16[0] = cpu_to_le16(0);
+ *(u16 *)databuf = cpu_to_le16(0);
OK(2);
case RH_GET_STATUS | RH_ENDPOINT:
- databuf.u16[0] = cpu_to_le16(0);
+ *(u16 *)databuf = cpu_to_le16(0);
OK(2);
case RH_GET_STATUS | RH_CLASS:
- databuf.u32[0] = cpu_to_le32(
+ *(u32 *)databuf = cpu_to_le32(
RD_RH_STAT & ~(RH_HS_CRWE | RH_HS_DRWE));
OK(4);
case RH_GET_STATUS | RH_OTHER | RH_CLASS:
- databuf.u32[0] = cpu_to_le32(RD_RH_PORTSTAT);
+ *(u32 *)databuf = cpu_to_le32(RD_RH_PORTSTAT);
OK(4);
case RH_CLEAR_FEATURE | RH_ENDPOINT:
@@ -1381,14 +1373,14 @@ pkt_print(NULL, dev, pipe, buffer, transfer_len,
min_t(unsigned int,
sizeof(root_hub_dev_des),
wLength));
- databuf.ptr = root_hub_dev_des; OK(len);
+ databuf = root_hub_dev_des; OK(len);
case (0x02): /* configuration descriptor */
len = min_t(unsigned int,
leni,
min_t(unsigned int,
sizeof(root_hub_config_des),
wLength));
- databuf.ptr = root_hub_config_des; OK(len);
+ databuf = root_hub_config_des; OK(len);
case (0x03): /* string descriptors */
if (wValue == 0x0300) {
len = min_t(unsigned int,
@@ -1396,7 +1388,7 @@ pkt_print(NULL, dev, pipe, buffer, transfer_len,
min_t(unsigned int,
sizeof(root_hub_str_index0),
wLength));
- databuf.ptr = root_hub_str_index0;
+ databuf = root_hub_str_index0;
OK(len);
}
if (wValue == 0x0301) {
@@ -1405,7 +1397,7 @@ pkt_print(NULL, dev, pipe, buffer, transfer_len,
min_t(unsigned int,
sizeof(root_hub_str_index1),
wLength));
- databuf.ptr = root_hub_str_index1;
+ databuf = root_hub_str_index1;
OK(len);
}
default:
@@ -1417,40 +1409,40 @@ pkt_print(NULL, dev, pipe, buffer, transfer_len,
{
__u32 temp = roothub_a(&gohci);
- databuf.u8[0] = 9; /* min length; */
- databuf.u8[1] = 0x29;
- databuf.u8[2] = temp & RH_A_NDP;
+ databuf[0] = 9; /* min length; */
+ databuf[1] = 0x29;
+ databuf[2] = temp & RH_A_NDP;
#ifdef CONFIG_AT91C_PQFP_UHPBUG
- databuf.u8[2] = (databuf.u8[2] == 2) ? 1 : 0;
+ databuf[2] = (databuf[2] == 2) ? 1 : 0;
#endif
- databuf.u8[3] = 0;
+ databuf[3] = 0;
if (temp & RH_A_PSM) /* per-port power switching? */
- databuf.u8[3] |= 0x1;
+ databuf[3] |= 0x1;
if (temp & RH_A_NOCP) /* no overcurrent reporting? */
- databuf.u8[3] |= 0x10;
+ databuf[3] |= 0x10;
else if (temp & RH_A_OCPM)/* per-port overcurrent reporting? */
- databuf.u8[3] |= 0x8;
+ databuf[3] |= 0x8;
- /* corresponds to databuf.u8[4-7] */
- databuf.u8[1] = 0;
- databuf.u8[5] = (temp & RH_A_POTPGT) >> 24;
+ databuf[4] = 0;
+ databuf[5] = (temp & RH_A_POTPGT) >> 24;
+ databuf[6] = 0;
temp = roothub_b(&gohci);
- databuf.u8[7] = temp & RH_B_DR;
- if (databuf.u8[2] < 7) {
- databuf.u8[8] = 0xff;
+ databuf[7] = temp & RH_B_DR;
+ if (databuf[2] < 7) {
+ databuf[8] = 0xff;
} else {
- databuf.u8[0] += 2;
- databuf.u8[8] = (temp & RH_B_DR) >> 8;
- databuf.u8[10] = databuf.u8[9] = 0xff;
+ databuf[0] += 2;
+ databuf[8] = (temp & RH_B_DR) >> 8;
+ databuf[10] = databuf[9] = 0xff;
}
len = min_t(unsigned int, leni,
- min_t(unsigned int, databuf.u8[0], wLength));
+ min_t(unsigned int, databuf[0], wLength));
OK(len);
}
case RH_GET_CONFIGURATION:
- databuf.u8[0] = 0x01;
+ databuf[0] = 0x01;
OK(1);
case RH_SET_CONFIGURATION:
@@ -1469,8 +1461,8 @@ pkt_print(NULL, dev, pipe, buffer, transfer_len,
#endif
len = min_t(int, len, leni);
- if (data != databuf.ptr)
- memcpy(data, databuf.ptr, len);
+ if (data != databuf)
+ memcpy(data, databuf, len);
dev->act_len = len;
dev->status = stat;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* [U-Boot] [PATCH 1/1] USB: Fix strict aliasing in ohci-hcd
2012-08-11 21:49 [U-Boot] [PATCH 1/1] USB: Fix strict aliasing in ohci-hcd Troy Kisky
@ 2012-08-12 0:07 ` Marek Vasut
2012-08-14 17:33 ` Marek Vasut
1 sibling, 0 replies; 5+ messages in thread
From: Marek Vasut @ 2012-08-12 0:07 UTC (permalink / raw)
To: u-boot
Dear Troy Kisky,
> commit 5f6aa03fda2a0a79940765865c1e4266be8a75f8
> USB: Fix complaints about strict aliasing in OHCI-HCD
>
> tried to fix this, but gcc4.4 still complains. So, this
> patch basically reverts the above and does a simpler fix.
>
> also, the above commit incorrectly changed
> /* corresponds to data_buf[4-7] */
> datab [1] = 0;
> to
>
> /* corresponds to databuf.u8[4-7] */
> databuf.u8[1] = 0;
>
> This patch also fixes that.
[...]
AGAIN?! :-C
If I were to run git log on that file, I'd get like five such "fixes" :-(
I'll review this properly tomorrow, this will be certainly a very pleasant
reviewing time ;-)
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH 1/1] USB: Fix strict aliasing in ohci-hcd
2012-08-11 21:49 [U-Boot] [PATCH 1/1] USB: Fix strict aliasing in ohci-hcd Troy Kisky
2012-08-12 0:07 ` Marek Vasut
@ 2012-08-14 17:33 ` Marek Vasut
2012-08-14 18:47 ` Troy Kisky
1 sibling, 1 reply; 5+ messages in thread
From: Marek Vasut @ 2012-08-14 17:33 UTC (permalink / raw)
To: u-boot
Dear Troy Kisky,
> commit 5f6aa03fda2a0a79940765865c1e4266be8a75f8
> USB: Fix complaints about strict aliasing in OHCI-HCD
>
> tried to fix this, but gcc4.4 still complains. So, this
> patch basically reverts the above and does a simpler fix.
>
> also, the above commit incorrectly changed
> /* corresponds to data_buf[4-7] */
> datab [1] = 0;
> to
>
> /* corresponds to databuf.u8[4-7] */
> databuf.u8[1] = 0;
>
> This patch also fixes that.
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
> drivers/usb/host/ohci-hcd.c | 70
> +++++++++++++++++++------------------------ 1 file changed, 31
> insertions(+), 39 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
> index d24f2f1..9f47351 100644
> --- a/drivers/usb/host/ohci-hcd.c
> +++ b/drivers/usb/host/ohci-hcd.c
> @@ -1261,19 +1261,11 @@ static int ohci_submit_rh_msg(struct usb_device
> *dev, unsigned long pipe, int leni = transfer_len;
> int len = 0;
> int stat = 0;
> - __u32 datab[4];
> - union {
> - void *ptr;
> - __u8 *u8;
> - __u16 *u16;
> - __u32 *u32;
> - } databuf;
> __u16 bmRType_bReq;
> __u16 wValue;
> __u16 wIndex;
> __u16 wLength;
> -
> - databuf.u32 = (__u32 *)datab;
> + ALLOC_ALIGN_BUFFER(__u8, databuf, 16, sizeof(u32));
Ok, pardon my sloppiness in reviews ... but why not alloc this cache aligned?
That way we'd also circumvent the issues with caches we'd eventually hit anyway.
[...]
The rest is OK.
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH 1/1] USB: Fix strict aliasing in ohci-hcd
2012-08-14 17:33 ` Marek Vasut
@ 2012-08-14 18:47 ` Troy Kisky
2012-08-14 19:12 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: Troy Kisky @ 2012-08-14 18:47 UTC (permalink / raw)
To: u-boot
On 8/14/2012 10:33 AM, Marek Vasut wrote:
> Dear Troy Kisky,
>
>> commit 5f6aa03fda2a0a79940765865c1e4266be8a75f8
>> USB: Fix complaints about strict aliasing in OHCI-HCD
>>
>> tried to fix this, but gcc4.4 still complains. So, this
>> patch basically reverts the above and does a simpler fix.
>>
>> also, the above commit incorrectly changed
>> /* corresponds to data_buf[4-7] */
>> datab [1] = 0;
>> to
>>
>> /* corresponds to databuf.u8[4-7] */
>> databuf.u8[1] = 0;
>>
>> This patch also fixes that.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>> ---
>> drivers/usb/host/ohci-hcd.c | 70
>> +++++++++++++++++++------------------------ 1 file changed, 31
>> insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
>> index d24f2f1..9f47351 100644
>> --- a/drivers/usb/host/ohci-hcd.c
>> +++ b/drivers/usb/host/ohci-hcd.c
>> @@ -1261,19 +1261,11 @@ static int ohci_submit_rh_msg(struct usb_device
>> *dev, unsigned long pipe, int leni = transfer_len;
>> int len = 0;
>> int stat = 0;
>> - __u32 datab[4];
>> - union {
>> - void *ptr;
>> - __u8 *u8;
>> - __u16 *u16;
>> - __u32 *u32;
>> - } databuf;
>> __u16 bmRType_bReq;
>> __u16 wValue;
>> __u16 wIndex;
>> __u16 wLength;
>> -
>> - databuf.u32 = (__u32 *)datab;
>> + ALLOC_ALIGN_BUFFER(__u8, databuf, 16, sizeof(u32));
> Ok, pardon my sloppiness in reviews ... but why not alloc this cache aligned?
> That way we'd also circumvent the issues with caches we'd eventually hit anyway.
>
> [...]
>
> The rest is OK.
>
Line 1464 has
if (data != databuf)
memcpy(data, databuf, len);
So as far as I can tell, databuf is always copied and there are no cache
issues to circumvent.
But I have no complaint if you desire to cacheline aligned anyway...
Troy
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH 1/1] USB: Fix strict aliasing in ohci-hcd
2012-08-14 18:47 ` Troy Kisky
@ 2012-08-14 19:12 ` Marek Vasut
0 siblings, 0 replies; 5+ messages in thread
From: Marek Vasut @ 2012-08-14 19:12 UTC (permalink / raw)
To: u-boot
Dear Troy Kisky,
> On 8/14/2012 10:33 AM, Marek Vasut wrote:
> > Dear Troy Kisky,
> >
> >> commit 5f6aa03fda2a0a79940765865c1e4266be8a75f8
> >>
> >> USB: Fix complaints about strict aliasing in OHCI-HCD
> >>
> >> tried to fix this, but gcc4.4 still complains. So, this
> >> patch basically reverts the above and does a simpler fix.
> >>
> >> also, the above commit incorrectly changed
> >>
> >> /* corresponds to data_buf[4-7] */
> >> datab [1] = 0;
> >>
> >> to
> >>
> >> /* corresponds to databuf.u8[4-7] */
> >> databuf.u8[1] = 0;
> >>
> >> This patch also fixes that.
> >>
> >> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> >> ---
> >>
> >> drivers/usb/host/ohci-hcd.c | 70
> >>
> >> +++++++++++++++++++------------------------ 1 file changed, 31
> >> insertions(+), 39 deletions(-)
> >>
> >> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
> >> index d24f2f1..9f47351 100644
> >> --- a/drivers/usb/host/ohci-hcd.c
> >> +++ b/drivers/usb/host/ohci-hcd.c
> >> @@ -1261,19 +1261,11 @@ static int ohci_submit_rh_msg(struct usb_device
> >> *dev, unsigned long pipe, int leni = transfer_len;
> >>
> >> int len = 0;
> >> int stat = 0;
> >>
> >> - __u32 datab[4];
> >> - union {
> >> - void *ptr;
> >> - __u8 *u8;
> >> - __u16 *u16;
> >> - __u32 *u32;
> >> - } databuf;
> >>
> >> __u16 bmRType_bReq;
> >> __u16 wValue;
> >> __u16 wIndex;
> >> __u16 wLength;
> >>
> >> -
> >> - databuf.u32 = (__u32 *)datab;
> >> + ALLOC_ALIGN_BUFFER(__u8, databuf, 16, sizeof(u32));
> >
> > Ok, pardon my sloppiness in reviews ... but why not alloc this cache
> > aligned? That way we'd also circumvent the issues with caches we'd
> > eventually hit anyway.
> >
> > [...]
> >
> > The rest is OK.
>
> Line 1464 has
>
> if (data != databuf)
> memcpy(data, databuf, len);
>
> So as far as I can tell, databuf is always copied and there are no cache
> issues to circumvent.
> But I have no complaint if you desire to cacheline aligned anyway...
Oh ok ... this is worse than I thought ... ok, I'll apply as-is
> Troy
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-08-14 19:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-11 21:49 [U-Boot] [PATCH 1/1] USB: Fix strict aliasing in ohci-hcd Troy Kisky
2012-08-12 0:07 ` Marek Vasut
2012-08-14 17:33 ` Marek Vasut
2012-08-14 18:47 ` Troy Kisky
2012-08-14 19:12 ` Marek Vasut
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox