David Hendricks (dhendrix@chromium.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/3045
-gerrit
commit 63f74abee8713140ab7a6ed1a24b98c00eb10b70 Author: David Hendricks dhendrix@chromium.org Date: Mon Apr 8 20:01:18 2013 -0700
armv7: replace read/write macros with inlines
This enables type checking for safety as to help prevent errors like http://review.coreboot.org/#/c/3038/ . Now compilation fails if the wrong type is passed into readb/readw/readl/writeb/writew/writel or other macros in io.h.
This also deprecates readw/writew. The previous definition was 16-bits which is incorrect since wordsize on ARM is 32-bits and there was only 1 instance of writew (#if 0'd anyway). Going forward we should always use read{8,16,32} and write{8,16,32} where N specifies the exact length rather than relying on ambiguous definitions of wordsize.
Since many macros relied on __raw_*, which were basically the same (minus data memory barrier instructions), this patch also gets rid of __raw_*. There were parts of the code which ended up using these macros consecutively, for example: setbits_le32(®s->ch_cfg, SPI_CH_RST); clrbits_le32(®s->ch_cfg, SPI_CH_RST);
In such cases the safe versions of readl() and writel() should be used anyway.
Note: This also fixes two dubious casts as to avoid breaking compilation.
Change-Id: I8850933f68ea3a9b615d00ebd422f7c242268f1c Signed-off-by: David Hendricks dhendrix@chromium.org --- src/arch/armv7/include/arch/io.h | 222 ++++++++---------------------- src/cpu/samsung/exynos5-common/cpu_info.c | 2 +- src/cpu/samsung/exynos5250/clock.c | 6 +- src/cpu/samsung/exynos5250/uart.c | 2 +- 4 files changed, 66 insertions(+), 166 deletions(-)
diff --git a/src/arch/armv7/include/arch/io.h b/src/arch/armv7/include/arch/io.h index b99e014..e62d579 100644 --- a/src/arch/armv7/include/arch/io.h +++ b/src/arch/armv7/include/arch/io.h @@ -1,6 +1,8 @@ /* - * linux/include/asm-arm/io.h + * Originally imported from linux/include/asm-arm/io.h. This file has changed + * substantially since then. * + * Copyright (C) 2013 Google Inc. * Copyright (C) 1996-2000 Russell King * * This program is free software; you can redistribute it and/or modify @@ -8,6 +10,7 @@ * published by the Free Software Foundation. * * Modifications: + * 08-Apr-2013 G Replaced several macros with inlines for type safety. * 16-Sep-1996 RMK Inlined the inx/outx functions & optimised for both * constant addresses and variable addresses. * 04-Dec-1997 RMK Moved a lot of this stuff to the new architecture @@ -24,102 +27,53 @@ #include <arch/cache.h> /* for dmb() */ #include <arch/byteorder.h>
-static inline void sync(void) +static inline uint8_t read8(const void *addr) { + uint8_t v = *(volatile uint8_t *)addr; + dmb(); + return v; }
-/* - * Generic virtual read/write. Note that we don't support half-word - * read/writes. We define __arch_*[bl] here, and leave __arch_*w - * to the architecture specific code. - */ -#define __arch_getb(a) (*(volatile unsigned char *)(a)) -#define __arch_getw(a) (*(volatile unsigned short *)(a)) -#define __arch_getl(a) (*(volatile unsigned int *)(a)) - -#define __arch_putb(v,a) (*(volatile unsigned char *)(a) = (v)) -#define __arch_putw(v,a) (*(volatile unsigned short *)(a) = (v)) -#define __arch_putl(v,a) (*(volatile unsigned int *)(a) = (v)) - -#if 0 -extern inline void __raw_writesb(unsigned int addr, const void *data, int bytelen) +static inline uint16_t read16(const void *addr) { - uint8_t *buf = (uint8_t *)data; - while(bytelen--) - __arch_putb(*buf++, addr); + uint16_t v = *(volatile uint16_t *)addr; + dmb(); + return v; }
-extern inline void __raw_writesw(unsigned int addr, const void *data, int wordlen) +static inline uint32_t read32(const void *addr) { - uint16_t *buf = (uint16_t *)data; - while(wordlen--) - __arch_putw(*buf++, addr); + uint32_t v = *(volatile uint32_t *)addr; + dmb(); + return v; }
-extern inline void __raw_writesl(unsigned int addr, const void *data, int longlen) +static inline void write8(uint8_t val, const void *addr) { - uint32_t *buf = (uint32_t *)data; - while(longlen--) - __arch_putl(*buf++, addr); + dmb(); + *(volatile uint8_t *)addr = val; }
-extern inline void __raw_readsb(unsigned int addr, void *data, int bytelen) +static inline void write16(uint16_t val, const void *addr) { - uint8_t *buf = (uint8_t *)data; - while(bytelen--) - *buf++ = __arch_getb(addr); + dmb(); + *(volatile uint16_t *)addr = val; }
-extern inline void __raw_readsw(unsigned int addr, void *data, int wordlen) +static inline void write32(uint32_t val, const void *addr) { - uint16_t *buf = (uint16_t *)data; - while(wordlen--) - *buf++ = __arch_getw(addr); + dmb(); + *(volatile uint32_t *)addr = val; }
-extern inline void __raw_readsl(unsigned int addr, void *data, int longlen) -{ - uint32_t *buf = (uint32_t *)data; - while(longlen--) - *buf++ = __arch_getl(addr); -} -#endif - -#define __raw_writeb(v,a) __arch_putb(v,a) -#define __raw_writew(v,a) __arch_putw(v,a) -#define __raw_writel(v,a) __arch_putl(v,a) - -#define __raw_readb(a) __arch_getb(a) -#define __raw_readw(a) __arch_getw(a) -#define __raw_readl(a) __arch_getl(a) - -/* - * TODO: The kernel offers some more advanced versions of barriers, it might - * have some advantages to use them instead of the simple one here. - */ -#define __iormb() dmb() -#define __iowmb() dmb() - -#define writeb(v,c) ({ u8 __v = v; __iowmb(); __arch_putb(__v,c); __v; }) -#define writew(v,c) ({ u16 __v = v; __iowmb(); __arch_putw(__v,c); __v; }) -#define writel(v,c) ({ u32 __v = v; __iowmb(); __arch_putl(__v,c); __v; }) - -#define readb(c) ({ u8 __v = __arch_getb(c); __iormb(); __v; }) -#define readw(c) ({ u16 __v = __arch_getw(c); __iormb(); __v; }) -#define readl(c) ({ u32 __v = __arch_getl(c); __iormb(); __v; }) - /* - * The compiler seems to be incapable of optimising constants - * properly. Spell it out to the compiler in some cases. - * These are only valid for small values of "off" (< 1<<12) - */ -#define __raw_base_writeb(val,base,off) __arch_base_putb(val,base,off) -#define __raw_base_writew(val,base,off) __arch_base_putw(val,base,off) -#define __raw_base_writel(val,base,off) __arch_base_putl(val,base,off) - -#define __raw_base_readb(base,off) __arch_base_getb(base,off) -#define __raw_base_readw(base,off) __arch_base_getw(base,off) -#define __raw_base_readl(base,off) __arch_base_getl(base,off) + * FIXME: These are to avoid breaking existing ARM code. We should eventually + * re-factor all code to specify the data length intended. + */ +#define readb(a) read8(a) +#define writeb(v,a) write8(v,a) +#define readl(a) read32(a) +#define writel(v,a) write32(v,a)
/* * Clear and set bits in one shot. These macros can be used to clear and @@ -129,8 +83,8 @@ extern inline void __raw_readsl(unsigned int addr, void *data, int longlen) * in the 'set' parameter. */
-#define out_arch(type,endian,a,v) __raw_write##type(cpu_to_##endian(v),a) -#define in_arch(type,endian,a) endian##_to_cpu(__raw_read##type(a)) +#define out_arch(type,endian,a,v) write##type(cpu_to_##endian(v),a) +#define in_arch(type,endian,a) endian##_to_cpu(read##type(a))
#define out_le32(a,v) out_arch(l,le32,a,v) #define out_le16(a,v) out_arch(w,le16,a,v) @@ -144,8 +98,8 @@ extern inline void __raw_readsl(unsigned int addr, void *data, int longlen) #define in_be32(a) in_arch(l,be32,a) #define in_be16(a) in_arch(w,be16,a)
-#define out_8(a,v) __raw_writeb(v,a) -#define in_8(a) __raw_readb(a) +#define out_8(a,v) writeb(v,a) +#define in_8(a) readb(a)
#define clrbits(type, addr, clear) \ out_##type((addr), in_##type(addr) & ~(clear)) @@ -177,13 +131,6 @@ extern inline void __raw_readsl(unsigned int addr, void *data, int longlen) #define clrsetbits_8(addr, clear, set) clrsetbits(8, addr, clear, set)
/* - * Now, pick up the machine-defined IO definitions - */ -#if 0 /* XXX###XXX */ -#include <asm/arch/io.h> -#endif /* XXX###XXX */ - -/* * IO port access primitives * ------------------------- * @@ -202,27 +149,26 @@ extern inline void __raw_readsl(unsigned int addr, void *data, int longlen) * address to a memory address. * * Note that we prevent GCC re-ordering or caching values in expressions - * by introducing sequence points into the in*() definitions. Note that - * __raw_* do not guarantee this behaviour. + * by introducing sequence points into the in*() definitions. * * The {in,out}[bwl] macros are for emulating x86-style PCI/ISA IO space. */ #ifdef __io -#define outb(v,p) __raw_writeb(v,__io(p)) -#define outw(v,p) __raw_writew(cpu_to_le16(v),__io(p)) -#define outl(v,p) __raw_writel(cpu_to_le32(v),__io(p)) +#define outb(v,p) writeb(v,__io(p)) +#define outw(v,p) writew(cpu_to_le16(v),__io(p)) +#define outl(v,p) writel(cpu_to_le32(v),__io(p))
-#define inb(p) ({ unsigned int __v = __raw_readb(__io(p)); __v; }) -#define inw(p) ({ unsigned int __v = le16_to_cpu(__raw_readw(__io(p))); __v; }) -#define inl(p) ({ unsigned int __v = le32_to_cpu(__raw_readl(__io(p))); __v; }) +#define inb(p) ({ unsigned int __v = readb(__io(p)); __v; }) +#define inw(p) ({ unsigned int __v = le16_to_cpu(readw(__io(p))); __v; }) +#define inl(p) ({ unsigned int __v = le32_to_cpu(readl(__io(p))); __v; })
-#define outsb(p,d,l) __raw_writesb(__io(p),d,l) -#define outsw(p,d,l) __raw_writesw(__io(p),d,l) -#define outsl(p,d,l) __raw_writesl(__io(p),d,l) +#define outsb(p,d,l) writesb(__io(p),d,l) +#define outsw(p,d,l) writesw(__io(p),d,l) +#define outsl(p,d,l) writesl(__io(p),d,l)
-#define insb(p,d,l) __raw_readsb(__io(p),d,l) -#define insw(p,d,l) __raw_readsw(__io(p),d,l) -#define insl(p,d,l) __raw_readsl(__io(p),d,l) +#define insb(p,d,l) readsb(__io(p),d,l) +#define insw(p,d,l) readsw(__io(p),d,l) +#define insl(p,d,l) readsl(__io(p),d,l) #endif
#define outb_p(val,port) outb((val),(port)) @@ -285,13 +231,13 @@ extern void __readwrite_bug(const char *fn); */ #ifdef __mem_pci
-#define readb(c) ({ unsigned int __v = __raw_readb(__mem_pci(c)); __v; }) -#define readw(c) ({ unsigned int __v = le16_to_cpu(__raw_readw(__mem_pci(c))); __v; }) -#define readl(c) ({ unsigned int __v = le32_to_cpu(__raw_readl(__mem_pci(c))); __v; }) +#define readb(c) ({ unsigned int __v = readb(__mem_pci(c)); __v; }) +#define readw(c) ({ unsigned int __v = le16_to_cpu(readw(__mem_pci(c))); __v; }) +#define readl(c) ({ unsigned int __v = le32_to_cpu(readl(__mem_pci(c))); __v; })
-#define writeb(v,c) __raw_writeb(v,__mem_pci(c)) -#define writew(v,c) __raw_writew(cpu_to_le16(v),__mem_pci(c)) -#define writel(v,c) __raw_writel(cpu_to_le32(v),__mem_pci(c)) +#define writeb(v,c) writeb(v,__mem_pci(c)) +#define writew(v,c) writew(cpu_to_le16(v),__mem_pci(c)) +#define writel(v,c) writel(cpu_to_le32(v),__mem_pci(c))
#define memset_io(c,v,l) _memset_io(__mem_pci(c),(v),(l)) #define memcpy_fromio(a,c,l) _memcpy_fromio((a),__mem_pci(c),(l)) @@ -316,66 +262,20 @@ check_signature(unsigned long io_addr, const unsigned char *signature, out: return retval; } - -#elif !defined(readb) - -#define readb(addr) (__readwrite_bug("readb"),0) -#define readw(addr) (__readwrite_bug("readw"),0) -#define readl(addr) (__readwrite_bug("readl"),0) -#define writeb(v,addr) __readwrite_bug("writeb") -#define writew(v,addr) __readwrite_bug("writew") -#define writel(v,addr) __readwrite_bug("writel") - -#define eth_io_copy_and_sum(a,b,c,d) __readwrite_bug("eth_io_copy_and_sum") - -#define check_signature(io,sig,len) (0) - #endif /* __mem_pci */
-/* FIXME(dhendrix): added to make uart8250_mem code happy. Note: lL */ -static inline __attribute__((always_inline)) uint8_t read8(unsigned long addr) -{ - return readb(addr); -} - -static inline __attribute__((always_inline)) uint16_t read16(unsigned long addr) -{ - return readw(addr); -} - -static inline __attribute__((always_inline)) uint32_t read32(unsigned long addr) -{ - return readl(addr); -} - -static inline __attribute__((always_inline)) void write8(unsigned long addr, uint8_t value) -{ - writeb(value, addr); -} - -static inline __attribute__((always_inline)) void write16(unsigned long addr, uint16_t value) -{ - writew(value, addr); -} - -static inline __attribute__((always_inline)) void write32(unsigned long addr, uint32_t value) -{ - writel(value, addr); -} - - /* * If this architecture has ISA IO, then define the isa_read/isa_write * macros. */ #ifdef __mem_isa
-#define isa_readb(addr) __raw_readb(__mem_isa(addr)) -#define isa_readw(addr) __raw_readw(__mem_isa(addr)) -#define isa_readl(addr) __raw_readl(__mem_isa(addr)) -#define isa_writeb(val,addr) __raw_writeb(val,__mem_isa(addr)) -#define isa_writew(val,addr) __raw_writew(val,__mem_isa(addr)) -#define isa_writel(val,addr) __raw_writel(val,__mem_isa(addr)) +#define isa_readb(addr) readb(__mem_isa(addr)) +#define isa_readw(addr) readw(__mem_isa(addr)) +#define isa_readl(addr) readl(__mem_isa(addr)) +#define isa_writeb(val,addr) writeb(val,__mem_isa(addr)) +#define isa_writew(val,addr) writew(val,__mem_isa(addr)) +#define isa_writel(val,addr) writel(val,__mem_isa(addr)) #define isa_memset_io(a,b,c) _memset_io(__mem_isa(a),(b),(c)) #define isa_memcpy_fromio(a,b,c) _memcpy_fromio((a),__mem_isa(b),(c)) #define isa_memcpy_toio(a,b,c) _memcpy_toio(__mem_isa((a)),(b),(c)) diff --git a/src/cpu/samsung/exynos5-common/cpu_info.c b/src/cpu/samsung/exynos5-common/cpu_info.c index dec3c77..6b75473 100644 --- a/src/cpu/samsung/exynos5-common/cpu_info.c +++ b/src/cpu/samsung/exynos5-common/cpu_info.c @@ -65,7 +65,7 @@ int s5p_get_cpu_rev(void)
void s5p_set_cpu_id(void) { - s5p_cpu_id = readl(EXYNOS_PRO_ID); + s5p_cpu_id = readl((void *)EXYNOS_PRO_ID); s5p_cpu_id = (0xC000 | ((s5p_cpu_id & 0x00FFF000) >> 12));
/* diff --git a/src/cpu/samsung/exynos5250/clock.c b/src/cpu/samsung/exynos5250/clock.c index 6b79272..3622e28 100644 --- a/src/cpu/samsung/exynos5250/clock.c +++ b/src/cpu/samsung/exynos5250/clock.c @@ -379,7 +379,7 @@ void set_mmc_clk(int dev_index, unsigned int div) { struct exynos5_clock *clk = samsung_get_base_clock(); - unsigned int addr; + unsigned int *addr; unsigned int val;
/* @@ -389,9 +389,9 @@ void set_mmc_clk(int dev_index, unsigned int div) * MMC2_PRE_RATIO [15:8], MMC3_PRE_RATIO [31:24] */ if (dev_index < 2) { - addr = (unsigned int)&clk->div_fsys1; + addr = &clk->div_fsys1; } else { - addr = (unsigned int)&clk->div_fsys2; + addr = &clk->div_fsys2; dev_index -= 2; }
diff --git a/src/cpu/samsung/exynos5250/uart.c b/src/cpu/samsung/exynos5250/uart.c index 75c5763..34d8e08 100644 --- a/src/cpu/samsung/exynos5250/uart.c +++ b/src/cpu/samsung/exynos5250/uart.c @@ -106,7 +106,7 @@ static void serial_setbrg_dev(void) */ #if 0 if (s5p_uart_divslot()) - writew(udivslot[val % 16], &uart->rest.slot); + writel(udivslot[val % 16], &uart->rest.slot); else writeb(val % 16, &uart->rest.value); #endif