On Wed, Feb 18, 2015 at 9:16 AM, Aaron Durbin <[email protected]> wrote: > On Tue, Feb 17, 2015 at 10:44 PM, Alexandru Gagniuc > <[email protected]> wrote: >> 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. >> > > > As I have noted on http://review.coreboot.org/#/c/7924/ it's very > short sighted to go this route. In assembling a coreboot stack (which > includes libpayload and the payload itself) the code usually comes > from different software systems. Those include libpayload, linux > kernel, u-boot, etc. They all have the write(val, addr) semantics. I > see no good reason to artificially erect an ever present barrier for > integrating code into a coreboot system. > > Alex, you've clearly stated your opinion you've not justified a reason > for keeping the barrier.
Hit "send" accidentally. Anyway, I'd like to hear why it's OK to purposefully keep this barrier in place. Thanks. -Aaron -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
