From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Tue, 14 Aug 2012 21:12:22 +0200
Subject: [U-Boot] [PATCH 1/1] USB: Fix strict aliasing in ohci-hcd
In-Reply-To: <502A9D5C.9060804@boundarydevices.com>
References: <1344721749-25644-1-git-send-email-troy.kisky@boundarydevices.com>
	<201208141933.29344.marex@denx.de>
	<502A9D5C.9060804@boundarydevices.com>
Message-ID: <201208142112.22348.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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