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 -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
