On Dec 29, 2013 7:17 PM, "ron minnich" <[email protected]> wrote: > > The functions are wrong in two ways. > > on the x86 side, the addr should be a void *. Had we done this long > ago we would have caught some bugs. > > On the ARM side, the addr should be first, not last, to be consistent > with other "addr"s in coreboot (e.g. pci space). So we'd like to fix > the argument order being backwards too.
Yep. I think we had caught this a while back, but left it as-is to be consistent with the u-boot macros. But since we've corrected a few things along the way, such as giving them explicit sizes and doing type checking, I suppose we may as well fix the argument ordering to be consistent with the rest of coreboot. > > It's been on my fix it list for months, it's pretty easy with > coccinnelle, I just have not had time. > > ron > > On Sun, Dec 29, 2013 at 5:16 PM, mrnuke <[email protected]> wrote: > > So, I found this: > > > > ARM: static inline void write32(uint32_t val, void *addr) > > x86: static inline void write32(unsigned long addr, uint32_t value) > > > > Now, isn't a 4-byte memory write a 4-byte memory write independent of > > architecture? Forget about the fact that the x86 version takes an unsigned > > long instead of a void * for the address; the ARM and x86 variants have their > > arguments mixed up. > > > > I actually came across this when trying to use our 8250 memmapped UART code on > > an ARM chip. The original code was written for x86, and hence, it can't work > > on ARM (yet). > > > > I propose that these simple memory accessors be unified. Since they refer to a > > memory address, I propose void * for the address parameter. I don't care what > > order the parameters come in as long it's consistent between arches. Any > > takers? > > > > > > Alex > > > > -- > > coreboot mailing list: [email protected] > > http://www.coreboot.org/mailman/listinfo/coreboot > > -- > coreboot mailing list: [email protected] > http://www.coreboot.org/mailman/listinfo/coreboot
-- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
