Re: [PATCH 09/12] unicore32 machine related files: hardware registers
From: Arnd Bergmann <arnd@arndb.de>
Date: 2011-02-17 17:00:30
Also in:
lkml
On Wednesday 16 February 2011, Guan Xuetao wrote:
+#define io_p2v(x) ((x) - PKUNITY_IOSPACE_BASE) +#define io_v2p(x) ((x) + PKUNITY_IOSPACE_BASE) + +#ifndef __ASSEMBLY__ + +# define __REG(x) (*((volatile unsigned long *)io_p2v(x))) +# define __PREG(x) (io_v2p((unsigned long)&(x))) + +#else + +# define __REG(x) io_p2v(x) +# define __PREG(x) io_v2p(x)
#define PKUNITY_IOSPACE_BASE 0x80000000 /* 0x80000000 - 0xFFFFFFFF 2GB */
The typecasts look wrong here: - "volatile unsigned long*" is not the right pointer type to do I/O on. It should instead be "void __iomem *". Please use the "sparse" tool with "make C=1" to get warnings about incorrect pointer type accesses. - PKUNITY_IOSPACE_BASE seems to be both a virtual and a physical address, which is a bad idea, because it prevents a lot of the checks from working correctly. - The __REG/__PREG macros seem to have the same purpose as io_p2v/io_v2p, you should not require both. - The term IOSPACE is confusing, because it normally refers to the PCI PIO space, while you mean the SoC's MMIO region I would recommend defining these as #ifndef __ASSEMBLY__ #define PKUNITY_MMIO_VIRT ((void __iomem *)0x80000000) #else #define PKUNITY_MMIO_VIRT 0x80000000 #endif #define io_p2v(x) ((x) - PKUNITY_MMIO_VIRT) #define io_v2p(x) ((x) + PKUNITY_MMIO_VIRT) /* please remove the two macros below as soon as all users are changed to use io_p2v */ #define __REG(x) io_p2v(x) #define __PREG(x) io_v2p(x) Arnd