Hi Aneesh,
On Tue, Dec 13, 2011 at 06:59:45PM +0530, Aneesh V wrote:
> OMAP4 U-Boot is broken in the mainline. U-Boot wouldn't boot up on any
> OMAP4 platforms. I suspect this will be the case with any ARM platform
> that has enabled USB tty code. I git-bisected the issue to this patch.
> I did some analysis on it and here are my findings.
>
> aligned(2) indeed makes the sizeof(struct usb_endpoint_descriptor) ==
> 8. But that doesn't seem to be enough. struct acm_config_desc embeds
> fields of type usb_endpoint_descriptor and has the attribute 'packed'.
> As a result, these usb_endpoint_descriptor structures in
> acm_config_desc may have odd addresses and consequently wMaxPacketSize
> also has odd address. As far as I can see, this is not a new issue. But
> your patch fortunately or unfortunately brought it out. Here is how:
>
> When 'usb_endpoint_descriptor' didn't have the 'aligned' attribute,
> compiler took extra care while accessing wMaxPacketSize. It did it
> using two byte reads and combining them to make a 16-bit half-word.
> When aligned was added compiler replaced it with a 'ldrh' (load
> half-word) instruction that resulted in an abort due the odd address.
>
How unpleasent, nice that you were able to pinpoint where and how it fails
however.
> Now, I am not sure how to solve this problem. I tried the following and
> the boot issue is gone.
>
> diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
> index e2e87fe..2a961e9 100644
> --- a/drivers/serial/usbtty.c
> +++ b/drivers/serial/usbtty.c
> @@ -151,7 +151,7 @@ struct acm_config_desc {
> /* Slave Interface */
> struct usb_interface_descriptor data_class_interface;
> struct usb_endpoint_descriptor data_endpoints[NUM_ENDPOINTS-1];
> -} __attribute__((packed));
> +};
>
> static struct acm_config_desc
> acm_configuration_descriptors[NUM_CONFIGS] = {
> {
>
> I am not a USB expert, so not sure whether this works. Does that
> structure really have to be 'packed'? It will be great if USB experts
> can look into this and come up with a permanent solution at the
> earliest because quite a few platforms will be broken until then.
>
I am neither a USB expert, just the moron that apperantly broke a lot
of OMAP4 boards.
The way I see it there are 3 options at hand here:
1) revert my patch and look over the places where wMaxPacketSize is used
and wrap them in get/set_unaligned() (at least in non-arch specific code)
2) go forward with your approach
3) stop using usb_endpoint_descriptor in arrays
I think option nr 1 is the safest one here, but yes,
input from USB experts would be great.
Stefan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
