From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Fri, 31 Aug 2012 01:29:05 +0200
Subject: [U-Boot] [PATCH] usb: do explicit unaligned accesses
In-Reply-To: <1346368414-993-1-git-send-email-dev@lynxeye.de>
References: <1346368414-993-1-git-send-email-dev@lynxeye.de>
Message-ID: <201208310129.06184.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 Lucas Stach,

> usb_hub_descriptor has to be packed as it's used for
> communication with the device. Member wHubCharacteristics
> violates the natural alignment rules.
> 
> Use explicit unaligned access functions for this member.
> Fixes ARMv7 traping while using USB.

Shouldn't a properly configured compiler prevent such behavior?

> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  common/usb_hub.c            | 14 +++++++++-----
>  drivers/usb/host/ehci-hcd.c |  7 +++++--
>  2 Dateien ge?ndert, 14 Zeilen hinzugef?gt(+), 7 Zeilen entfernt(-)
> 
> diff --git a/common/usb_hub.c b/common/usb_hub.c
> index 53d939c..b8cd990 100644
> --- a/common/usb_hub.c
> +++ b/common/usb_hub.c
> @@ -43,6 +43,7 @@
>  #include <common.h>
>  #include <command.h>
>  #include <asm/processor.h>
> +#include <asm/unaligned.h>
>  #include <linux/ctype.h>
>  #include <asm/byteorder.h>
>  #include <asm/unaligned.h>
> @@ -269,6 +270,7 @@ static int usb_hub_configure(struct usb_device *dev)
>  	int i;
>  	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, USB_BUFSIZ);
>  	unsigned char *bitmap;
> +	short hubCharacteristics;
>  	struct usb_hub_descriptor *descriptor;
>  	struct usb_hub_device *hub;
>  #ifdef USB_HUB_DEBUG
> @@ -304,8 +306,9 @@ static int usb_hub_configure(struct usb_device *dev)
>  	}
>  	memcpy((unsigned char *)&hub->desc, buffer, descriptor->bLength);
>  	/* adjust 16bit values */
> -	hub->desc.wHubCharacteristics =
> -				le16_to_cpu(descriptor->wHubCharacteristics);
> +	put_unaligned(le16_to_cpu(get_unaligned(
> +			&descriptor->wHubCharacteristics)),
> +			&descriptor->wHubCharacteristics);
>  	/* set the bitmap */
>  	bitmap = (unsigned char *)&hub->desc.DeviceRemovable[0];
>  	/* devices not removable by default */
> @@ -322,7 +325,8 @@ static int usb_hub_configure(struct usb_device *dev)
>  	dev->maxchild = descriptor->bNbrPorts;
>  	USB_HUB_PRINTF("%d ports detected\n", dev->maxchild);
> 
> -	switch (hub->desc.wHubCharacteristics & HUB_CHAR_LPSM) {
> +	hubCharacteristics = get_unaligned(&hub->desc.wHubCharacteristics);
> +	switch (hubCharacteristics & HUB_CHAR_LPSM) {
>  	case 0x00:
>  		USB_HUB_PRINTF("ganged power switching\n");
>  		break;
> @@ -335,12 +339,12 @@ static int usb_hub_configure(struct usb_device *dev)
>  		break;
>  	}
> 
> -	if (hub->desc.wHubCharacteristics & HUB_CHAR_COMPOUND)
> +	if (hubCharacteristics & HUB_CHAR_COMPOUND)
>  		USB_HUB_PRINTF("part of a compound device\n");
>  	else
>  		USB_HUB_PRINTF("standalone hub\n");
> 
> -	switch (hub->desc.wHubCharacteristics & HUB_CHAR_OCPM) {
> +	switch (hubCharacteristics & HUB_CHAR_OCPM) {
>  	case 0x00:
>  		USB_HUB_PRINTF("global over-current protection\n");
>  		break;
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index bfea192..d90e94d 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -22,6 +22,7 @@
>   */
>  #include <common.h>
>  #include <asm/byteorder.h>
> +#include <asm/unaligned.h>
>  #include <usb.h>
>  #include <asm/io.h>
>  #include <malloc.h>
> @@ -876,10 +877,12 @@ int usb_lowlevel_init(int index, void **controller)
>  	debug("Register %x NbrPorts %d\n", reg, descriptor.hub.bNbrPorts);
>  	/* Port Indicators */
>  	if (HCS_INDICATOR(reg))
> -		descriptor.hub.wHubCharacteristics |= 0x80;
> +		put_unaligned(get_unaligned(&descriptor.hub.wHubCharacteristics)
> +				| 0x80, &descriptor.hub.wHubCharacteristics);
>  	/* Port Power Control */
>  	if (HCS_PPC(reg))
> -		descriptor.hub.wHubCharacteristics |= 0x01;
> +		put_unaligned(get_unaligned(&descriptor.hub.wHubCharacteristics)
> +				| 0x01, &descriptor.hub.wHubCharacteristics);
> 
>  	/* Start the host controller. */
>  	cmd = ehci_readl(&ehcic[index].hcor->or_usbcmd);

Best regards,
Marek Vasut