I'd like to propose an API change/cleanup for a long-standing problem with architecture-independent code that people have hit and privately discussed/cursed multiple times already, but no one really had time to make the big change/fix yet. (Some earlier discussion among Chromium people about this is available at http://crbug.com/451388 )
For doing MMIO accesses to registers, x86 code has traditionally used a function with a signature like this:
void write32(unsigned long addr, u32 value); // equivalents for read32(), write8(), etc.
While ARM (and based on that, ARM64) has been brought up with both of these:
void write32(u32 value, void *addr); void writel(u32 value, void *addr);
There's two problems with this: first, the order of arguments (value and address) is switched around between the two architectures, meaning it is currently impossible to write any architecture-independent MMIO code (which blocks important features like generalizing src/lib/uart8250mem.c). Second, the x86 version is not type safe (it takes addresses as integers instead of pointers), making it extremely easy to accidentally swap address and value (especially due to all this architecture confusion about which order is correct). Changing the addresses to a pointer type will generate obvious compiler errors for most of those screw-ups, which I think is a good thing that x86 could benefit from as well.
I want to unify all architectures under a single, type safe version of these macros that will remove the confusion and allow writing architecture-independent MMIO code in the future. For this, I plan to:
1. Switch all existing uses of write32() (and friends) in ARM(64) code to writel(). 2. Remove write32() from all architectures but x86. 3. Add writel(u32 value, void *addr) which is compatible with other architectures to x86. 4. Pronounce write32(unsigned long addr, u32 value) deprecated for x86 and slowly phase it out over time (at least from common/recent code).
After this we'll be converged on writel(u32 value, void *addr) as the one and only way to do it across all architectures going forward. I know that there are probably people who prefer the name write32() aesthetically, or who have a strong preference for the first-address-then-value parameter order. I personally don't really like the way writel() looks myself, but at the end of the day it doesn't make a huge difference. I think this is the best solution because it has strong practical advantages and will allow us to solve the issue with the least amount of changes: most ARM(64) code already uses writel(), libpayload uses writel() across all architectures, and it's the standard for U-Boot and a majority of the Linux kernel which are popular import sources. Also, the amount of existing x86 code is probably too large to ever change (and appropriately test) it all... so it's useful to pick a solution that can leave the old untyped write32() in place on x86 as a deprecated fallback.
Since the changes required for this would mostly affect ARM(64) SoCs and boards, which are currently much more developed (and in flux) in the Chromium tree, I'm planning to only change this there for now and wait until it gets merged into coreboot mainline in the right order as part of the current upstreaming process. I still wanted to float the idea here first to give everyone a chance to chime in before it's all said and done.
-
Julius Werner