[coreboot] Inconsistent low-level arch functions between ARM and x86

David Hendricks david.hendricks at gmail.com
Mon Dec 30 06:43:53 CET 2013


On Dec 29, 2013 7:17 PM, "ron minnich" <rminnich at gmail.com> 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 <mr.nuke.me at gmail.com> 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: coreboot at coreboot.org
> > http://www.coreboot.org/mailman/listinfo/coreboot
>
> --
> coreboot mailing list: coreboot at coreboot.org
> http://www.coreboot.org/mailman/listinfo/coreboot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20131229/0383a9e4/attachment.html>


More information about the coreboot mailing list