Re: [PATCH 16/17] unicore32: add (void __iomem *) to io_p2v macro

[Arnd Bergmann]

On Sunday 27 February 2011, Guan Xuetao wrote:
> -#define PKUNITY_AC97_CONR              __REG(PKUNITY_AC97_BASE + 0x0000)
> -#define PKUNITY_AC97_OCR               __REG(PKUNITY_AC97_BASE + 0x0004)
> -#define PKUNITY_AC97_ICR               __REG(PKUNITY_AC97_BASE + 0x0008)
> -#define PKUNITY_AC97_CRAC              __REG(PKUNITY_AC97_BASE + 0x000C)
> -#define PKUNITY_AC97_INTR              __REG(PKUNITY_AC97_BASE + 0x0010)
> -#define PKUNITY_AC97_INTRSTAT          __REG(PKUNITY_AC97_BASE + 0x0014)
> -#define PKUNITY_AC97_INTRCLEAR         __REG(PKUNITY_AC97_BASE + 0x0018)
> -#define PKUNITY_AC97_ENABLE            __REG(PKUNITY_AC97_BASE + 0x001C)
> -#define PKUNITY_AC97_OUT_FIFO          __REG(PKUNITY_AC97_BASE + 0x0020)
> -#define PKUNITY_AC97_IN_FIFO           __REG(PKUNITY_AC97_BASE + 0x0030)
> +#define PKUNITY_AC97_CONR              io_p2v(PKUNITY_AC97_BASE + 0x0000)
> +#define PKUNITY_AC97_OCR               io_p2v(PKUNITY_AC97_BASE + 0x0004)
> +#define PKUNITY_AC97_ICR               io_p2v(PKUNITY_AC97_BASE + 0x0008)
> +#define PKUNITY_AC97_CRAC              io_p2v(PKUNITY_AC97_BASE + 0x000C)
> +#define PKUNITY_AC97_INTR              io_p2v(PKUNITY_AC97_BASE + 0x0010)

One more comment on the types here (applies to the entire patch):

It would be more straightforward to the define the underlying base
addresses using io_p2v, and then do a simple addition, instead of
another macro that adds a type cast:

#define PKUNITY_SYSTEM_AHB	io_p2v(0xC0000000)
#define PKUNITY_PERIPHERAL_AHB	io_p2v(0xEE000000)

#define PKUNITY_UART0_BASE	(PKUNITY_PERIPHERAL_AHB + 0x00000000)
#define PKUNITY_AC97_BASE	(PKUNITY_PERIPHERAL_AHB + 0x00400000)

#define PKUNITY_AC97_CONR	(PKUNITY_AC97_BASE + 0x0000)
#define PKUNITY_AC97_OCR	(PKUNITY_AC97_BASE + 0x0004)

It would be nice if you could do that for the entire set of hardware
headers.

In the long run, I would recommend moving away from hardcoded I/O addresses
entirely, but you can do that at the same time as moving to a flattened
device tree. When you do that, every driver will do an individual ioremap
to get the virtual address for a physical location, rather than doing it
once at boot time for all hardware. This makes the code more flexible when
dealing with multiple SoC implemetations, but it's not something that
you need to worry about too much for the initial merge.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/