[coreboot] Unifying IO accessor macros

Julius Werner jwerner at chromium.org
Wed Feb 18 01:32:18 CET 2015


> 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 -.- ).

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".

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.

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?



More information about the coreboot mailing list