[coreboot] Unifying IO accessor macros

[Alexandru Gagniuc]

On Tuesday, February 17, 2015 04:32:18 PM Julius Werner wrote:
> > The x86 was recently fixed to take in void *. The order of arguments was
> > discussed before, and the agreement is to use write*(addr, val);
> > This has the advantage of being consistent with other accessor functions,
> > such as PCI, and follows the logical C ordering of the assignment
> > operator. destination = value;
> 
> Oh, wow... that merged literally just this weekend. Talk about bad
> timing. I wasn't aware this was going on (and none of the reviewers
> bothered to tell me -.- ).
> 
Well, I tried pinging you on IRC a few times, but I didn't realize you prefer 
ML over IRC. Sorry about that. To be fair, Ron did warn me about you being on 
the same tracks.

> Guess that brings me back to the drawing board. I don't generally
> disagree with your reasonings about why you consider certain patterns
> better than others... at most I'd say that I don't think it's all that
> important at the end of the day (writel() has always referred to 32
> bits wherever I've seen it yet, and 64 could easily be writeq() if we
> ever need it), and that doing things the way people are used to from
> other/bigger projects (like Linux) might ultimately be more useful to
> us than doing what we consider "the right way".
> 
Well, we're firmware. We're special. We used to do hardware. Now we mostly 
deal with hardhardhardhardhardhardware. Every little imperfection propagates 
faster than light and multiplies exponentially. Linux doesn't deal with that. 
U-boot doesn't deal with that. Not quite a fair comparison.

> But I really don't have a strong opinion in either direction, I just
> care about unifying this and making sure there is a single, consistent
> pattern everywhere. If most people here think write32(void *addr, u32
> value) is the right way to go, I can instead switch ARM(64) over to
> that. It would be a pretty massive change, but since all those boards
> are Chromebooks for now I'm in a reasonably good position to ensure
> everything stays working.
> 
This reverse order hit me like a brick wall when I first started doing ARM on 
coreboot. But I was angry at ARM code for having gotten it wrong. I also hated 
uboot's setbits_le32(), but hey, at least it indicates bitness -- compare to 
setbits_lel().

> One issue we're left with is libpayload, which I think should
> definitely use the same pattern as coreboot. Right now it uses
> writel(v,a) on all architectures (typed on ARM and untyped on x86). I
> can change the library, but all external payloads will need to change
> their call sites themselves. Are we okay with that?

Personally, I'm okay with unifying the order/type of arguments. As long as the 
the payloads that we have in-tree are updated, the external ones shouldn't be 
that hard to do at a later time.

Alex