lkml.org 
[lkml]   [2011]   [Feb]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 09/12] unicore32 machine related files: hardware registers
Date
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



\
 
 \ /
  Last update: 2011-02-17 18:03    [W:0.384 / U:0.144 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site