[PATCH] ARM: errata: pl310 cache sync operation may be faulty

On Wed, Feb 16, 2011 at 11:32:12 +0100, Catalin Marinas wrote:

On 16 February 2011 06:03, Srinidhi KASAGAR
[off-list ref] wrote:

quoted

On Tue, Feb 15, 2011 at 12:34:22 +0100, Russell King - ARM Linux wrote:

quoted

On Tue, Feb 15, 2011 at 04:48:03PM +0530, srinidhi kasagar wrote:

quoted

+#ifdef ARM_ERRATA_753970
+#define L2X0_DUMMY_REG ?0x740
+ ? /* write to an unmmapped register */
+ ? writel_relaxed(0, base + L2X0_DUMMY_REG);
+ ? cache_wait(base + L2X0_CACHE_SYNC, 1);
+#else
? ? writel_relaxed(0, base + L2X0_CACHE_SYNC);
? ? cache_wait(base + L2X0_CACHE_SYNC, 1);
+#endif

So why wrap cache_wait() up in that horrible ifdef as well - and why not
put the dummy register definition along side the other register definitions?

[...]

quoted

--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c

@@ -49,8 +49,14 @@ static inline void cache_wait(void __iomem *reg, unsigned long mask)

?static inline void cache_sync(void)
?{
? ? ? ?void __iomem *base = l2x0_base;
+
+#ifdef CONFIG_ARM_ERRATA_753970
+ ? ? ? /* write to an unmmapped register */
+ ? ? ? writel_relaxed(0, base + L2X0_DUMMY_REG);
+#else
? ? ? ?writel_relaxed(0, base + L2X0_CACHE_SYNC);
? ? ? ?cache_wait(base + L2X0_CACHE_SYNC, 1);
+#endif
?}

You could still leave cache_wait() after #endif, even though it is a
no-op. I think it is clearer that the erratum workaround only targets
the sync.

updated patch below.