[coreboot] patch: fix USB ports on DBE62, and other cs5536-based platforms

Marc Jones Marc.Jones at amd.com
Wed Jun 4 19:41:52 CEST 2008


ron minnich wrote:
> On Wed, Jun 4, 2008 at 9:46 AM, Marc Jones <Marc.Jones at amd.com> wrote:
>> ron minnich wrote:
>>> This change adds some debug prints, and a comment warning to dts on cs5536.
>>>
>>> Most importantly it fixes a simple programming error which made it so most of
>>> the sets on the USB were not doing anything. The bug is also in V2.
>>>
>>
>>> /* the /sizeof(u32) is to convert byte offsets into u32 offsets */
>>> #define HCCPARAMS             (0x08/sizeof(u32))
>>
>>
>> I don't understand this change. This is standard MMIO. The memory mapped
>> registers are defined 0h, 04h, 08h, 0Ah...
>>
>> You could
>> *(bar + 08h) |= 1 << 9;
>> or
>> *(bar + 09h) |= 1 << 1;
>>
> 
> 
> This is an old C gotcha. Offsets are scaled by the pointer size. bar
> is a u32 *. So you see this:
> *(bar + HCCPARAMS)
> 
> And it's easy to think that it is this:
> *(bar + 0xc)
> 
> but actually, because bar is a u32 *, it will actually dereference this:
> *(bar + 0xc*4) i.e. the sizeof a u32 ...
> 
> It has to work this way, because when you do bar++, it increments bar
> by 4, not 1.
> 
> This is one of the reasons why, for mmio, we ought to be using structs.
> 
> thanks
> 
> ron
> 

Doh! I see. It should be a byte pointer and then use the readl() abd 
writel() etc from io.h. I'll post a patch for v2 in a few minutes.

A struct for the entire device seems overkill for a couple of registers.
Marc

-- 
Marc Jones
Senior Firmware Engineer
(970) 226-9684 Office
mailto:Marc.Jones at amd.com
http://www.amd.com/embeddedprocessors





More information about the coreboot mailing list