On Wed, Dec 14, 2011 at 3:24 AM, Aneesh V <ane...@ti.com> wrote: > Copying Wolfgang as he seems to be the author of this module. > > > On Wednesday 14 December 2011 01:45 AM, Stefan Kristiansson wrote: >> >> 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. > > > Wolfgang, > > What's your recommendation to solve this problem. Does the ACM > structure really have to be 'packed'? The packed attribute is resulting > in some of its fields to be at odd address, which in turn generates > alignment faults on access. My original mail has more details. > > This is now affecting any ARM board that has USB_TTY enabled.
Adding Wolfgang this time :) -- Tom _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot