From mboxrd@z Thu Jan  1 00:00:00 1970
From: Amit Virdi <amit.virdi@st.com>
Date: Wed, 29 Feb 2012 15:55:16 +0530
Subject: [U-Boot] [PATCH] USB:host: Attribute packed removed from usb
 structures
In-Reply-To: <201202271956.23159.vapier@gentoo.org>
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>
Message-ID: <4F4DFD0C.4010707@st.com>
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

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.

Thanks
Amit Virdi