From mboxrd@z Thu Jan 1 00:00:00 1970 From: Amit Virdi Date: Tue, 6 Mar 2012 17:36:22 +0530 Subject: [U-Boot] [PATCH] USB:host: Attribute packed removed from usb structures In-Reply-To: <4F4DFD0C.4010707@st.com> References: <1330084720-16878-1-git-send-email-amit.virdi@st.com> <201202271327.01060.vapier@gentoo.org> <201202272153.29932.marek.vasut@gmail.com> <201202271956.23159.vapier@gentoo.org> <4F4DFD0C.4010707@st.com> Message-ID: <4F55FDBE.2000904@st.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Mike and Marek, On 2/29/2012 3:55 PM, Amit Virdi wrote: > Hello Mike, > > On 2/28/2012 6:26 AM, Mike Frysinger wrote: >> On Monday 27 February 2012 15:53:29 Marek Vasut wrote: >>>> On Monday 27 February 2012 08:14:26 Marek Vasut wrote: >>>>>> On 2/25/2012 3:42 PM, Albert ARIBAUD wrote: >>>>>>> Le 24/02/2012 12:58, Amit Virdi a ?crit : >>>>>>>> Packed attribute is forcing a bytewise write on device registers, >>>>>>>> there by, resulting in a misbehavior on gcc-4.4.1. >>>>>>>> Reverting the structures to non-packed >>>>>>> >>>>>>> If (just asking, not asserting) the issue is caused by fields being >>>>>>> u8 where u8 access is not possible, then should you not make the >>>>>>> fields u16 / u32 according to access requirements? >>>>>> >>>>>> The problem is not with the fields being of a different width. AFAIK, >>>>>> the packed attribute changes the generated code to access even the >>>>>> word field elements in a byte by byte manner >>>>>> >>>>>> Infact, there is a discussion on lkml that I can point >>>>>> https://lkml.org/lkml/2011/4/27/278 >>>>>> >>>>>> It seems that the discussion did not lead to a conclusion but it was >>>>>> sensible (at least for ARM) to remove the packed attribute from this >>>>>> structure >>>>> >>>>> What does the USB spec say ? It might be a HW bug? >>>> >>>> it isn't covered by the USB spec. these are structs for hardware >>>> registers in the EHCI usb host controller. >>> >>> I see ... so replacing them with unions of accessors where it colides >>> might >>> work ? >> >> i'm not entirely sure what you're asking, but i think you're pointing >> to the >> right answer: drivers/usb/host/ehci.h:ehci_{read,write}l() should >> *not* be >> casting/derferencing the pointers directly. they should instead be using >> standard {read,write}l() funcs from asm/io.h. >> >> Amit: can you post a new patch that does that instead ? don't touch the >> packed attribute, but change ehci_readl() to use readl() and >> ehci_writel() to >> use writel() ? > > I'll make the changes and post the patch after testing successfully. > I did the changes suggested by you and tested the build. The issue didn't come up. Then I reverted the code to the original (attributes retained and ehci directly de-referencing the pointers. The issue didn't come here too. Today, I used armv7-linux-gcc (GCC) v4.6.2 So I suspect there has been some fix done in the GCC. Now, even with the packed attributes, the word fields are accessed word-by-word in contrast to the earlier observed behavior (byte-by-byte). I could see ldr and str in the disassembly. May be, we can discard this patch and keep drivers/usb/host/ehci.h intact. Thanks Amit Virdi