[coreboot-gerrit] New patch to review for coreboot: 20ffca9 armv7: replace read/write macros with inlines

David Hendricks (dhendrix@chromium.org) gerrit at coreboot.org
Tue Apr 9 05:53:32 CEST 2013


David Hendricks (dhendrix at chromium.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/3045

-gerrit

commit 20ffca9c24a5d9a4e4908607a4a7088e1afe2c53
Author: David Hendricks <dhendrix at 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.
    
    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(&regs->ch_cfg, SPI_CH_RST);
    	clrbits_le32(&regs->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 at chromium.org>
---
 src/arch/armv7/include/arch/io.h          | 194 +++++++++---------------------
 src/cpu/samsung/exynos5-common/cpu_info.c |   2 +-
 src/cpu/samsung/exynos5250/clock.c        |   6 +-
 3 files changed, 63 insertions(+), 139 deletions(-)

diff --git a/src/arch/armv7/include/arch/io.h b/src/arch/armv7/include/arch/io.h
index b99e014..e6e235f 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,44 @@
 #include <arch/cache.h>		/* for dmb() */
 #include <arch/byteorder.h>
 
-static inline void sync(void)
+static inline unsigned char readb(const void *addr)
 {
+	unsigned char v = *(volatile unsigned char *)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)
-{
-	uint8_t *buf = (uint8_t *)data;
-	while(bytelen--)
-		__arch_putb(*buf++, addr);
-}
-
-extern inline void __raw_writesw(unsigned int addr, const void *data, int wordlen)
+static inline unsigned short readw(const void *addr)
 {
-	uint16_t *buf = (uint16_t *)data;
-	while(wordlen--)
-		__arch_putw(*buf++, addr);
+	unsigned short v = *(volatile unsigned short *)addr;
+	dmb();
+	return v;
 }
 
-extern inline void __raw_writesl(unsigned int addr, const void *data, int longlen)
+static inline unsigned int readl(const void *addr)
 {
-	uint32_t *buf = (uint32_t *)data;
-	while(longlen--)
-		__arch_putl(*buf++, addr);
+	unsigned int v = *(volatile unsigned int *)addr;
+	dmb();
+	return v;
 }
 
-extern inline void __raw_readsb(unsigned int addr, void *data, int bytelen)
+static inline void writeb(unsigned char val, const void *addr)
 {
-	uint8_t *buf = (uint8_t *)data;
-	while(bytelen--)
-		*buf++ = __arch_getb(addr);
+	dmb();
+	*(volatile unsigned char *)addr = val;
 }
 
-extern inline void __raw_readsw(unsigned int addr, void *data, int wordlen)
+static inline void writew(unsigned short val, const void *addr)
 {
-	uint16_t *buf = (uint16_t *)data;
-	while(wordlen--)
-		*buf++ = __arch_getw(addr);
+	dmb();
+	*(volatile unsigned short *)addr = val;
 }
 
-extern inline void __raw_readsl(unsigned int addr, void *data, int longlen)
+static inline void writel(unsigned int val, const void *addr)
 {
-	uint32_t *buf = (uint32_t *)data;
-	while(longlen--)
-		*buf++ = __arch_getl(addr);
+	dmb();
+	*(volatile unsigned int*)addr = val;
 }
-#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)
 
 /*
  * Clear and set bits in one shot. These macros can be used to clear and
@@ -129,8 +74,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 +89,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 +122,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
  *  -------------------------
  *
@@ -208,21 +146,21 @@ extern inline void __raw_readsl(unsigned int addr, void *data, int longlen)
  * 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 +223,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,49 +254,35 @@ 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);
+	return readb((unsigned char *)addr);
 }
 
 static inline __attribute__((always_inline)) uint16_t read16(unsigned long addr)
 {
-	return readw(addr);
+	return readw((unsigned short *)addr);
 }
 
 static inline __attribute__((always_inline)) uint32_t read32(unsigned long addr)
 {
-	return readl(addr);
+	return readl((unsigned int *)addr);
 }
 
-static inline __attribute__((always_inline)) void write8(unsigned long addr, uint8_t value)
+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)
+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)
+static inline __attribute__((always_inline)) void write32(unsigned long *addr, uint32_t value)
 {
 	writel(value, addr);
 }
@@ -370,12 +294,12 @@ static inline __attribute__((always_inline)) void write32(unsigned long addr, ui
  */
 #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;
 	}
 



More information about the coreboot-gerrit mailing list