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

Hi Uwe,

I thought you had started your vacation, since I have not seen any
message from you in the past two days.  That's why I'm holding the v7.
 So glad to see your message, the v7 will be out later today.  I'm
struggling to get the patch series accepted before your New Year
Holiday.

I have one comment embedded below.

2010/12/16 Uwe Kleine-K?nig [off-list ref]:

Hello Arnd,

On Wed, Dec 15, 2010 at 05:58:42PM +0100, Arnd Bergmann wrote:

quoted

On Wednesday 15 December 2010, Uwe Kleine-K?nig wrote:

quoted

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

quoted

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.

Then we should define a proper function for this with well-defined
behaviour. I would suggest defining a mxs_readl/mxs_writel here,
that is defined to have the same endianess as the mxs SOC, but
otherwise has the same properties as readl/writel.

I don't get your point here. ?What are the properties of readl/writel
you want here? ?The barrier? ?__mem_pci?

For me __mxs_setl is a proper function with well-defined behaviour, no?
(One thing I currently consider to argue is to make these .c file local
because different IPs might have different offsets for SET, CLR and TOG
or not support it at all, but other than that I'm happy with it.)

Though some IPs do not have SET/CLR/TOG registers, all IPs having them
share the same offset.

-- 
Regards,
Shawn