[PATCH v6 01/15] ARM: mxs: Add core definitions

On Wed, Dec 15, 2010 at 05:22:21PM +0100, Arnd Bergmann wrote:

On Monday 13 December 2010, Shawn Guo wrote:

quoted

Add core definitions for MXS-based SoC MX23 and MX28.

How different are these from the MXC SoCs? Is it really
worth having a totally separate plat-* directory for them?

They are different enough that Sascha and I suggested to seperate them.
The first version of this series integrated mxs into mach-imx +
plat-mxc.

AFAICT, combining the two would make it much easier to build
a kernel that works on mx23/28 as well as mx25 or the later
MXS implementations.

Right, but seeing the differences to mxc I vote for an arm-global
approach to build a cross-platform kernel.

quoted

diff --git a/arch/arm/mach-mxs/include/mach/hardware.h b/arch/arm/mach-mxs/include/mach/hardware.h
new file mode 100644
index 0000000..53e89a0
--- /dev/null
+++ b/arch/arm/mach-mxs/include/mach/hardware.h
+
+#ifndef __MACH_MXS_HARDWARE_H__
+#define __MACH_MXS_HARDWARE_H__
+
+#ifdef __ASSEMBLER__
+#define IOMEM(addr)	(addr)
+#else
+#define IOMEM(addr)	((void __force __iomem *)(addr))
+#endif

This looks like a rather ugly hack to hide misuse of __iomem
pointers. If you pass virtual addresses of I/O registers,
just make them always be of type (void __iomem *), so you
can actually benefit from the sparse annotations, rather
working against them.

Actually this is to define the symbols for virtual addresses.  And
because gas doesn't understand void * it gets a plain number.

And note, this is something that rmk suggested.
 

quoted

+
+#ifndef __ASSEMBLER__
+static inline void __mxs_setl(u32 mask, void __iomem *reg)
+{
+	__raw_writel(mask, reg + MXS_SET_ADDR);
+}
+
+static inline void __mxs_clrl(u32 mask, void __iomem *reg)
+{
+	__raw_writel(mask, reg + MXS_CLR_ADDR);
+}
+
+static inline void __mxs_togl(u32 mask, void __iomem *reg)
+{
+	__raw_writel(mask, reg + MXS_TOG_ADDR);
+}
+#endif

Why __raw_writel()? All regular I/O accesses should use
readl/writel etc, not the internal helpers.

nak, readl does little endian accesses, __raw_readl native endian.  So
for platform use __raw_readl is most of the time the better one.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |