RE: [PATCH 1/2] ARM: l2x0: Errata fix for flush by Wayoperationcan cause data corruption

2011-03-07 Thread Santosh Shilimkar
Thanks Will for reporting it.

 -Original Message-
 From: linux-arm-kernel-boun...@lists.infradead.org [mailto:linux-
 arm-kernel-boun...@lists.infradead.org] On Behalf Of Will Deacon
 Sent: Monday, March 07, 2011 5:38 PM
 To: 'Santosh Shilimkar'; Russell King - ARM Linux
 Cc: t...@atomide.com; Catalin Marinas; linux-omap@vger.kernel.org;
 linux-arm-ker...@lists.infradead.org
 Subject: RE: [PATCH 1/2] ARM: l2x0: Errata fix for flush by
 Wayoperationcan cause data corruption

 Hi Santosh,

   On Sun, Feb 27, 2011 at 12:00:21PM +, Russell King - ARM
 Linux
   wrote:
 +#else
 +/* Optimised out for non-errata case */
 +static inline void debug_writel(unsigned long val)
 +{
  }
   
#define l2x0_set_debug  NULL
   
 +#endif
  
   I notice you got rid of the inline function.  Have you tried
   building this without the errata enabled?
 
  I accidently dropped the inline function while
  incorporating the comment from you. :(
 
  Fixed it. Updated version # 6770/1

 This version of the patch (as it appears in -next) is broken:


 +#define debug_writel(val)outer_cache.set_debug(val)
 +
 +static void l2x0_set_debug(unsigned long val)
 +{
 + writel(val, l2x0_base + L2X0_DEBUG_CTRL);

This should have been writel_relaxed() to avoid the
cache sync.

  }

 [...]

 @@ -119,9 +120,11 @@ static void l2x0_flush_all(void)

   /* clean all ways */
   spin_lock_irqsave(l2x0_lock, flags);
 + debug_writel(0x03);
   writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_CLEAN_INV_WAY);
   cache_wait_way(l2x0_base + L2X0_CLEAN_INV_WAY, l2x0_way_mask);
   cache_sync();
 + debug_writel(0x00);
   spin_unlock_irqrestore(l2x0_lock, flags);
  }


 This deadlocks because the writel forces an outer cache sync, which
 then tries to acquire the spinlock which is held by the calling
 function.

 If you change l2x0_set_debug to use writel_relaxed then you can
 avoid the problem.

Ya understood. I couldn't test non-errata case because
direct right doesn't work because of security and henced missed
it.

Below is the updated version. Also attached.

Russell,
Do you want me to push this to patch system or you can
apply this one?


Regards,
Santosh
--
From 9167fd4eff8adf1f8be772cdb734bfc0f2061354 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar santosh.shilim...@ti.com
Date: Mon, 28 Feb 2011 13:31:09 +0100
Subject: [PATCH] ARM: 6770/1: l2x0: Errata fix for flush by Way operation
can cause data corruption

PL310 implements the Clean  Invalidate by Way L2 cache maintenance
operation (offset 0x7FC). This operation runs in background so that
PL310 can handle normal accesses while it is in progress. Under very
rare circumstances, due to this erratum, write data can be lost when
PL310 treats a cacheable write transaction during a Clean  Invalidate
by Way operation.

Workaround:
Disable Write-Back and Cache Linefill (Debug Control Register)
Clean  Invalidate by Way (0x7FC)
Re-enable Write-Back and Cache Linefill (Debug Control Register)

This patch also removes any OMAP dependency on PL310 Errata's

Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
Acked-by: Catalin Marinas catalin.mari...@arm.com
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
---
 arch/arm/Kconfig  |   15 ---
 arch/arm/include/asm/outercache.h |1 +
 arch/arm/mm/cache-l2x0.c  |   32 ++--
 3 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 65ea7bb..ef41f7e 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1135,7 +1135,7 @@ config ARM_ERRATA_742231

 config PL310_ERRATA_588369
bool Clean  Invalidate maintenance operations do not invalidate
clean lines
-   depends on CACHE_L2X0  ARCH_OMAP4
+   depends on CACHE_L2X0
help
   The PL310 L2 cache controller implements three types of Clean 
   Invalidate maintenance operations: by Physical Address
@@ -1144,8 +1144,7 @@ config PL310_ERRATA_588369
   clean operation followed immediately by an invalidate
operation,
   both performing to the same memory location. This functionality
   is not correctly implemented in PL310 as clean lines are not
-  invalidated as a result of these operations. Note that this
errata
-  uses Texas Instrument's secure monitor api.
+  invalidated as a result of these operations.

 config ARM_ERRATA_720789
bool ARM errata: TLBIASIDIS and TLBIMVAIS operations can
broadcast a faulty ASID
@@ -1172,6 +1171,16 @@ config ARM_ERRATA_743622
  visible impact on the overall performance or power consumption
of the
  processor.

+config PL310_ERRATA_727915
+   bool Background Clean  Invalidate by Way operation can cause
data corruption
+   depends on CACHE_L2X0
+   help
+ PL310 implements the Clean  Invalidate by Way L2 cache
maintenance
+ operation (offset 0x7FC

Re: [PATCH 1/2] ARM: l2x0: Errata fix for flush by Wayoperationcan cause data corruption

2011-03-07 Thread Russell King - ARM Linux
On Mon, Mar 07, 2011 at 05:55:22PM +0530, Santosh Shilimkar wrote:
 Below is the updated version. Also attached.
 
 Russell,
 Do you want me to push this to patch system or you can
 apply this one?

Patch system please.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html