[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Linus Walleij]

From: Colin Cross <ccross@android.com>

ARM errata 727915 for PL310 has been updated to include a new
workaround required for PL310 r2p0 for l2x0_flush_all, which also
affects l2x0_clean_all in my testing. For r2p0, clean or flush
each set/way individually. For r3p0 or greater, use the debug
register for cleaning and flushing.

Requires exporting the cache_id, sets and ways detected in the
init function for later use.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Per Fransson <per.fransson.ml@gmail.com>
Signed-off-by: Colin Cross <ccross@android.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I found this thing in the Android tree, it looks like it could
be pretty serious so I'm mailing it out. If it is correct I
guess it should even be tagged for -stable?

v2: what about actually git commit --amend in the rebase changes
for -rc7? Sorry for the screwup...
---
 arch/arm/include/asm/hardware/cache-l2x0.h |    3 ++
 arch/arm/mm/cache-l2x0.c                   |   68 ++++++++++++++++++++++------
 2 files changed, 58 insertions(+), 13 deletions(-)

diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h b/arch/arm/include/asm/hardware/cache-l2x0.h
index c4c87bc..bd2c6a5 100644
--- a/arch/arm/include/asm/hardware/cache-l2x0.h
+++ b/arch/arm/include/asm/hardware/cache-l2x0.h
@@ -66,6 +66,7 @@
 #define   L2X0_STNDBY_MODE_EN		(1 << 0)
 
 /* Registers shifts and masks */
+#define L2X0_CACHE_ID_REV_MASK		(0x3f)
 #define L2X0_CACHE_ID_PART_MASK		(0xf << 6)
 #define L2X0_CACHE_ID_PART_L210		(1 << 6)
 #define L2X0_CACHE_ID_PART_L310		(3 << 6)
@@ -102,6 +103,8 @@
 
 #define L2X0_ADDR_FILTER_EN		1
 
+#define REV_PL310_R2P0				4
+
 #ifndef __ASSEMBLY__
 extern void __init l2x0_init(void __iomem *base, u32 aux_val, u32 aux_mask);
 #if defined(CONFIG_CACHE_L2X0) && defined(CONFIG_OF)
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 2a8e380..5868618 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -32,6 +32,9 @@ static void __iomem *l2x0_base;
 static DEFINE_RAW_SPINLOCK(l2x0_lock);
 static u32 l2x0_way_mask;	/* Bitmask of active ways */
 static u32 l2x0_size;
+static u32 l2x0_cache_id;
+static unsigned int l2x0_sets;
+static unsigned int l2x0_ways;
 static unsigned long sync_reg_offset = L2X0_CACHE_SYNC;
 
 struct l2x0_regs l2x0_saved_regs;
@@ -42,6 +45,13 @@ struct l2x0_of_data {
 	void (*resume)(void);
 };
 
+static inline bool is_pl310_rev(int rev)
+{
+	return (l2x0_cache_id &
+		(L2X0_CACHE_ID_PART_MASK | L2X0_CACHE_ID_REV_MASK)) ==
+			(L2X0_CACHE_ID_PART_L310 | rev);
+}
+
 static inline void cache_wait_way(void __iomem *reg, unsigned long mask)
 {
 	/* wait for cache operation by line or way to complete */
@@ -130,6 +140,23 @@ static void l2x0_cache_sync(void)
 	raw_spin_unlock_irqrestore(&l2x0_lock, flags);
 }
 
+#ifdef CONFIG_PL310_ERRATA_727915
+static void l2x0_for_each_set_way(void __iomem *reg)
+{
+	int set;
+	int way;
+	unsigned long flags;
+
+	for (way = 0; way < l2x0_ways; way++) {
+		raw_spin_lock_irqsave(&l2x0_lock, flags);
+		for (set = 0; set < l2x0_sets; set++)
+			writel_relaxed((way << 28) | (set << 5), reg);
+		cache_sync();
+		raw_spin_unlock_irqrestore(&l2x0_lock, flags);
+	}
+}
+#endif
+
 static void __l2x0_flush_all(void)
 {
 	debug_writel(0x03);
@@ -143,6 +170,13 @@ static void l2x0_flush_all(void)
 {
 	unsigned long flags;
 
+#ifdef CONFIG_PL310_ERRATA_727915
+	if (is_pl310_rev(REV_PL310_R2P0)) {
+		l2x0_for_each_set_way(l2x0_base + L2X0_CLEAN_INV_LINE_IDX);
+		return;
+	}
+#endif
+
 	/* clean all ways */
 	raw_spin_lock_irqsave(&l2x0_lock, flags);
 	__l2x0_flush_all();
@@ -153,11 +187,20 @@ static void l2x0_clean_all(void)
 {
 	unsigned long flags;
 
+#ifdef CONFIG_PL310_ERRATA_727915
+	if (is_pl310_rev(REV_PL310_R2P0)) {
+		l2x0_for_each_set_way(l2x0_base + L2X0_CLEAN_LINE_IDX);
+		return;
+	}
+#endif
+
 	/* clean all ways */
 	raw_spin_lock_irqsave(&l2x0_lock, flags);
+	debug_writel(0x03);
 	writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_CLEAN_WAY);
 	cache_wait_way(l2x0_base + L2X0_CLEAN_WAY, l2x0_way_mask);
 	cache_sync();
+	debug_writel(0x00);
 	raw_spin_unlock_irqrestore(&l2x0_lock, flags);
 }
 
@@ -309,26 +352,24 @@ static void l2x0_unlock(u32 cache_id)
 void __init l2x0_init(void __iomem *base, u32 aux_val, u32 aux_mask)
 {
 	u32 aux;
-	u32 cache_id;
 	u32 way_size = 0;
-	int ways;
 	const char *type;
 
 	l2x0_base = base;
 
-	cache_id = readl_relaxed(l2x0_base + L2X0_CACHE_ID);
+	l2x0_cache_id = readl_relaxed(l2x0_base + L2X0_CACHE_ID);
 	aux = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
 
 	aux &= aux_mask;
 	aux |= aux_val;
 
 	/* Determine the number of ways */
-	switch (cache_id & L2X0_CACHE_ID_PART_MASK) {
+	switch (l2x0_cache_id & L2X0_CACHE_ID_PART_MASK) {
 	case L2X0_CACHE_ID_PART_L310:
 		if (aux & (1 << 16))
-			ways = 16;
+			l2x0_ways = 16;
 		else
-			ways = 8;
+			l2x0_ways = 8;
 		type = "L310";
 #ifdef CONFIG_PL310_ERRATA_753970
 		/* Unmapped register. */
@@ -337,24 +378,25 @@ void __init l2x0_init(void __iomem *base, u32 aux_val, u32 aux_mask)
 		outer_cache.set_debug = pl310_set_debug;
 		break;
 	case L2X0_CACHE_ID_PART_L210:
-		ways = (aux >> 13) & 0xf;
+		l2x0_ways = (aux >> 13) & 0xf;
 		type = "L210";
 		break;
 	default:
 		/* Assume unknown chips have 8 ways */
-		ways = 8;
+		l2x0_ways = 8;
 		type = "L2x0 series";
 		break;
 	}
 
-	l2x0_way_mask = (1 << ways) - 1;
+	l2x0_way_mask = (1 << l2x0_ways) - 1;
 
 	/*
 	 * L2 cache Size =  Way size * Number of ways
 	 */
 	way_size = (aux & L2X0_AUX_CTRL_WAY_SIZE_MASK) >> 17;
-	way_size = 1 << (way_size + 3);
-	l2x0_size = ways * way_size * SZ_1K;
+	way_size = SZ_1K << (way_size + 3);
+	l2x0_size = l2x0_ways * way_size;
+	l2x0_sets = way_size / CACHE_LINE_SIZE;
 
 	/*
 	 * Check if l2x0 controller is already enabled.
@@ -363,7 +405,7 @@ void __init l2x0_init(void __iomem *base, u32 aux_val, u32 aux_mask)
 	 */
 	if (!(readl_relaxed(l2x0_base + L2X0_CTRL) & 1)) {
 		/* Make sure that I&D is not locked down when starting */
-		l2x0_unlock(cache_id);
+		l2x0_unlock(l2x0_cache_id);
 
 		/* l2x0 controller is disabled */
 		writel_relaxed(aux, l2x0_base + L2X0_AUX_CTRL);
@@ -386,7 +428,7 @@ void __init l2x0_init(void __iomem *base, u32 aux_val, u32 aux_mask)
 
 	printk(KERN_INFO "%s cache controller enabled\n", type);
 	printk(KERN_INFO "l2x0: %d ways, CACHE_ID 0x%08x, AUX_CTRL 0x%08x, Cache size: %d B\n",
-			ways, cache_id, aux, l2x0_size);
+			l2x0_ways, l2x0_cache_id, aux, l2x0_size);
 }
 
 #ifdef CONFIG_OF
-- 
1.7.9.2

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Shilimkar, Santosh]

On Mon, May 21, 2012 at 2:11 AM, Linus Walleij
<linus.walleij@stericsson.com> wrote:
> From: Colin Cross <ccross@android.com>
>
> ARM errata 727915 for PL310 has been updated to include a new
> workaround required for PL310 r2p0 for l2x0_flush_all, which also
> affects l2x0_clean_all in my testing. For r2p0, clean or flush
> each set/way individually. For r3p0 or greater, use the debug
> register for cleaning and flushing.
>
> Requires exporting the cache_id, sets and ways detected in the
> init function for later use.
>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Per Fransson <per.fransson.ml@gmail.com>
> Signed-off-by: Colin Cross <ccross@android.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> I found this thing in the Android tree, it looks like it could
> be pretty serious so I'm mailing it out. If it is correct I
> guess it should even be tagged for -stable?
>
> v2: what about actually git commit --amend in the rebase changes
> for -rc7? Sorry for the screwup...
> ---
This was observed on OMAP4460 devices. Though the errata
document doesn't support it, the issue seen with l2 clean was
indeed fixed $subject fix.

Regards
Santosh

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Linus Walleij]

On Mon, May 21, 2012 at 8:04 AM, Shilimkar, Santosh
<santosh.shilimkar@ti.com> wrote:

> This was observed on OMAP4460 devices. Though the errata
> document doesn't support it, the issue seen with l2 clean was
> indeed fixed $subject fix.

Is that an Acked-by for OMAP4? :-)

Yours,
Linus Walleij

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Santosh Shilimkar]

On Monday 21 May 2012 02:50 PM, Linus Walleij wrote:
> On Mon, May 21, 2012 at 8:04 AM, Shilimkar, Santosh
> <santosh.shilimkar@ti.com> wrote:
> 
>> This was observed on OMAP4460 devices. Though the errata
>> document doesn't support it, the issue seen with l2 clean was
>> indeed fixed $subject fix.
> 
> Is that an Acked-by for OMAP4? :-)
> 
Yes.

Regards
Santosh

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Will Deacon]

Hi Santosh,

On Mon, May 21, 2012 at 07:04:21AM +0100, Shilimkar, Santosh wrote:
> On Mon, May 21, 2012 at 2:11 AM, Linus Walleij
> <linus.walleij@stericsson.com> wrote:
> > From: Colin Cross <ccross@android.com>
> >
> > ARM errata 727915 for PL310 has been updated to include a new
> > workaround required for PL310 r2p0 for l2x0_flush_all, which also
> > affects l2x0_clean_all in my testing. For r2p0, clean or flush
> > each set/way individually. For r3p0 or greater, use the debug
> > register for cleaning and flushing.
> >
> > Requires exporting the cache_id, sets and ways detected in the
> > init function for later use.
> >
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > Cc: Per Fransson <per.fransson.ml@gmail.com>
> > Signed-off-by: Colin Cross <ccross@android.com>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > I found this thing in the Android tree, it looks like it could
> > be pretty serious so I'm mailing it out. If it is correct I
> > guess it should even be tagged for -stable?
> >
> > v2: what about actually git commit --amend in the rebase changes
> > for -rc7? Sorry for the screwup...
> > ---
> This was observed on OMAP4460 devices. Though the errata
> document doesn't support it, the issue seen with l2 clean was
> indeed fixed $subject fix.

I'm not sure I follow. Which version of the PL310 is in OMAP4460? Are you
saying that the current workaround didn't work but the one in this patch
did? Do you have a testcase?

Cheers,

Will

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Will Deacon]

Hi Linus,

On Sun, May 20, 2012 at 09:41:45PM +0100, Linus Walleij wrote:
> From: Colin Cross <ccross@android.com>
> 
> ARM errata 727915 for PL310 has been updated to include a new
> workaround required for PL310 r2p0 for l2x0_flush_all, which also
> affects l2x0_clean_all in my testing. For r2p0, clean or flush
> each set/way individually. For r3p0 or greater, use the debug
> register for cleaning and flushing.
> 
> Requires exporting the cache_id, sets and ways detected in the
> init function for later use.

[...]

> +#ifdef CONFIG_PL310_ERRATA_727915
> +static void l2x0_for_each_set_way(void __iomem *reg)
> +{
> +	int set;
> +	int way;
> +	unsigned long flags;
> +
> +	for (way = 0; way < l2x0_ways; way++) {
> +		raw_spin_lock_irqsave(&l2x0_lock, flags);
> +		for (set = 0; set < l2x0_sets; set++)
> +			writel_relaxed((way << 28) | (set << 5), reg);
> +		cache_sync();
> +		raw_spin_unlock_irqrestore(&l2x0_lock, flags);
> +	}
> +}
> +#endif
> +
>  static void __l2x0_flush_all(void)
>  {
>  	debug_writel(0x03);
> @@ -143,6 +170,13 @@ static void l2x0_flush_all(void)
>  {
>  	unsigned long flags;
>  
> +#ifdef CONFIG_PL310_ERRATA_727915
> +	if (is_pl310_rev(REV_PL310_R2P0)) {
> +		l2x0_for_each_set_way(l2x0_base + L2X0_CLEAN_INV_LINE_IDX);
> +		return;
> +	}
> +#endif

If we're adding code like this to flush_all, maybe we'd be better off just
assigning a different function to outer_cache.flush_all if we detect that
the current outer cache is an affected PL310 variant. However, note that
__l2x0_flush_all is called on the disable path. Do we need to intercept that
too?

> @@ -153,11 +187,20 @@ static void l2x0_clean_all(void)
>  {
>  	unsigned long flags;
>  
> +#ifdef CONFIG_PL310_ERRATA_727915
> +	if (is_pl310_rev(REV_PL310_R2P0)) {
> +		l2x0_for_each_set_way(l2x0_base + L2X0_CLEAN_LINE_IDX);
> +		return;
> +	}
> +#endif

clean_all is only called as an optimisation when cleaning a large range.
Since the workaround resorts back to cleaning each set and way, it's
questionable whether the optimisation pays off. I'd be inclined just to
predicate the optimisation on the erratum not being set.

>  	/* clean all ways */
>  	raw_spin_lock_irqsave(&l2x0_lock, flags);
> +	debug_writel(0x03);
>  	writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_CLEAN_WAY);
>  	cache_wait_way(l2x0_base + L2X0_CLEAN_WAY, l2x0_way_mask);
>  	cache_sync();
> +	debug_writel(0x00);
>  	raw_spin_unlock_irqrestore(&l2x0_lock, flags);
>  }

What are these new debug writes for? I thought only clean and invalidate was
affected?

Will

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Santosh Shilimkar]

On Tuesday 22 May 2012 02:22 PM, Will Deacon wrote:
> Hi Santosh,
> 
> On Mon, May 21, 2012 at 07:04:21AM +0100, Shilimkar, Santosh wrote:
>> On Mon, May 21, 2012 at 2:11 AM, Linus Walleij
>> <linus.walleij@stericsson.com> wrote:
>>> From: Colin Cross <ccross@android.com>
>>>
>>> ARM errata 727915 for PL310 has been updated to include a new
>>> workaround required for PL310 r2p0 for l2x0_flush_all, which also
>>> affects l2x0_clean_all in my testing. For r2p0, clean or flush
>>> each set/way individually. For r3p0 or greater, use the debug
>>> register for cleaning and flushing.
>>>
>>> Requires exporting the cache_id, sets and ways detected in the
>>> init function for later use.
>>>
>>> Cc: Will Deacon <will.deacon@arm.com>
>>> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>> Cc: Per Fransson <per.fransson.ml@gmail.com>
>>> Signed-off-by: Colin Cross <ccross@android.com>
>>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>>> ---
>>> I found this thing in the Android tree, it looks like it could
>>> be pretty serious so I'm mailing it out. If it is correct I
>>> guess it should even be tagged for -stable?
>>>
>>> v2: what about actually git commit --amend in the rebase changes
>>> for -rc7? Sorry for the screwup...
>>> ---
>> This was observed on OMAP4460 devices. Though the errata
>> document doesn't support it, the issue seen with l2 clean was
>> indeed fixed $subject fix.
> 
> I'm not sure I follow. Which version of the PL310 is in OMAP4460? Are you
> saying that the current workaround didn't work but the one in this patch
> did? Do you have a testcase?
> 
It's r3p1.
IIRC, the errata fix I did(debug register) was only for flush
(clean & inv) but Colin found in his testing that it's needed
for clean_all as well.

I don't have testcase. Colin might be able to suggest one.

Regards
santosh

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Linus Walleij]

On Tue, May 22, 2012 at 11:21 AM, Will Deacon <will.deacon@arm.com> wrote:

>> +#ifdef CONFIG_PL310_ERRATA_727915
>> + ? ? if (is_pl310_rev(REV_PL310_R2P0)) {
>> + ? ? ? ? ? ? l2x0_for_each_set_way(l2x0_base + L2X0_CLEAN_INV_LINE_IDX);
>> + ? ? ? ? ? ? return;
>> + ? ? }
>> +#endif
>
> If we're adding code like this to flush_all, maybe we'd be better off just
> assigning a different function to outer_cache.flush_all if we detect that
> the current outer cache is an affected PL310 variant. However, note that
> __l2x0_flush_all is called on the disable path. Do we need to intercept that
> too?

No idea. I was mainly forwarding the patch after findng it in the
Android tree. I'm hoping Colin and Santosh can help us hash this out...

The thing is that since this is in the Android tree and vendors tend to
use that as a base for development rather than using kernel.org,
we will see this code deployed in a large number of systems, so I
wanted to expose it to the kernel community.

Yours,
Linus Walleij

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Will Deacon]

On Tue, May 22, 2012 at 10:22:48AM +0100, Santosh Shilimkar wrote:
> On Tuesday 22 May 2012 02:22 PM, Will Deacon wrote:
> > I'm not sure I follow. Which version of the PL310 is in OMAP4460? Are you
> > saying that the current workaround didn't work but the one in this patch
> > did? Do you have a testcase?
> > 
> It's r3p1.
> IIRC, the errata fix I did(debug register) was only for flush
> (clean & inv) but Colin found in his testing that it's needed
> for clean_all as well.

Ok, interesting.

> I don't have testcase. Colin might be able to suggest one.

That would be ideal. Then I can take it up with the hardware guys since I'd
like to confirm we're not just hiding a different bug with those writes.

Cheers,

Will

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Colin Cross]

On Tue, May 22, 2012 at 2:34 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, May 22, 2012 at 10:22:48AM +0100, Santosh Shilimkar wrote:
>> On Tuesday 22 May 2012 02:22 PM, Will Deacon wrote:
>> > I'm not sure I follow. Which version of the PL310 is in OMAP4460? Are you
>> > saying that the current workaround didn't work but the one in this patch
>> > did? Do you have a testcase?
>> >
>> It's r3p1.
>> IIRC, the errata fix I did(debug register) was only for flush
>> (clean & inv) but Colin found in his testing that it's needed
>> for clean_all as well.
>
> Ok, interesting.
>
>> I don't have testcase. Colin might be able to suggest one.
>
> That would be ideal. Then I can take it up with the hardware guys since I'd
> like to confirm we're not just hiding a different bug with those writes.

I was using an omap 4430, triggering l2x0_clean_all from a GPU ioctl
by calling outer_clean_range(0, ULONG_MAX).  The problem was very
reproducible at the time, and using the persistent_trace code I posted
a while back I was able to narrow it down to hanging the cpu (not an
infinite loop) inside cache_wait_way.  It was 100% reproducible just
booting a development version of Android, but not if I called
l2x0_clean_all during boot, so it may depend on cache state or load.

outer_clean_range() will often get called on a 1080p frame, which is
8MB of data.  Even the set/way clean all is likely to be faster than
cleaning 8MB by MVA in a 1MB L2.

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Will Deacon]

On Tue, May 22, 2012 at 07:36:23PM +0100, Colin Cross wrote:
> I was using an omap 4430, triggering l2x0_clean_all from a GPU ioctl
> by calling outer_clean_range(0, ULONG_MAX).  The problem was very
> reproducible at the time, and using the persistent_trace code I posted
> a while back I was able to narrow it down to hanging the cpu (not an
> infinite loop) inside cache_wait_way.  It was 100% reproducible just
> booting a development version of Android, but not if I called
> l2x0_clean_all during boot, so it may depend on cache state or load.

Right, ok. This is definitely a different problem though. Firstly, r3p1 is
not affected by the original erratum and secondly the symptoms do not
include deadlock. The clean during boot also sounds highly suspicious -- is
there a chance you boot the kernel with a dirty L2? Is the L2 enabled or
disabled when entering the kernel [I think u8500 has it enabled]?

Will

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Colin Cross]

On Fri, May 25, 2012 at 7:56 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, May 22, 2012 at 07:36:23PM +0100, Colin Cross wrote:
>> I was using an omap 4430, triggering l2x0_clean_all from a GPU ioctl
>> by calling outer_clean_range(0, ULONG_MAX). ?The problem was very
>> reproducible at the time, and using the persistent_trace code I posted
>> a while back I was able to narrow it down to hanging the cpu (not an
>> infinite loop) inside cache_wait_way. ?It was 100% reproducible just
>> booting a development version of Android, but not if I called
>> l2x0_clean_all during boot, so it may depend on cache state or load.
>
> Right, ok. This is definitely a different problem though. Firstly, r3p1 is
> not affected by the original erratum and secondly the symptoms do not
> include deadlock. The clean during boot also sounds highly suspicious -- is
> there a chance you boot the kernel with a dirty L2? Is the L2 enabled or
> disabled when entering the kernel [I think u8500 has it enabled]?

In a previous thread, I wrote that the 4430 I was using had r2p0.  I
can't verify the accuracy of that claim right now, but my patch was
based on that assumption.  Where are you getting r3p1 from?

When I said I couldn't reproduce the issue with clean during boot, I
meant that I couldn't trigger the issue during boot by calling clean
repeatedly.  I could still reproduce the issue after boot even if the
cache was cleaned during boot.

The reference to 727915 came from discussions with TI, but given that
it is a deadlock and not a memory corruption, you're probably right
that it is not the same issue.

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Will Deacon]

Hi Colin,

On Fri, May 25, 2012 at 09:04:22PM +0100, Colin Cross wrote:
> On Fri, May 25, 2012 at 7:56 AM, Will Deacon <will.deacon@arm.com> wrote:
> > Right, ok. This is definitely a different problem though. Firstly, r3p1 is
> > not affected by the original erratum and secondly the symptoms do not
> > include deadlock. The clean during boot also sounds highly suspicious -- is
> > there a chance you boot the kernel with a dirty L2? Is the L2 enabled or
> > disabled when entering the kernel [I think u8500 has it enabled]?
> 
> In a previous thread, I wrote that the 4430 I was using had r2p0.  I
> can't verify the accuracy of that claim right now, but my patch was
> based on that assumption.  Where are you getting r3p1 from?

Apologies, that's what Santosh mentioned here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/101302.html

so, assuming that it's the same problem, it seems that both r3p1 and r2p0
are affected.

> When I said I couldn't reproduce the issue with clean during boot, I
> meant that I couldn't trigger the issue during boot by calling clean
> repeatedly.  I could still reproduce the issue after boot even if the
> cache was cleaned during boot.

Ok, that's interesting. I wonder if erratum #769419 is to blame? We now have
a workaround in the kernel for that, so you could give it a try if you're in
a position to do so.

Cheers,

Will

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Shilimkar, Santosh]

On Mon, May 28, 2012 at 3:41 PM, Will Deacon <will.deacon@arm.com> wrote:
> Hi Colin,
>
> On Fri, May 25, 2012 at 09:04:22PM +0100, Colin Cross wrote:
>> On Fri, May 25, 2012 at 7:56 AM, Will Deacon <will.deacon@arm.com> wrote:
>> > Right, ok. This is definitely a different problem though. Firstly, r3p1 is
>> > not affected by the original erratum and secondly the symptoms do not
>> > include deadlock. The clean during boot also sounds highly suspicious -- is
>> > there a chance you boot the kernel with a dirty L2? Is the L2 enabled or
>> > disabled when entering the kernel [I think u8500 has it enabled]?
>>
>> In a previous thread, I wrote that the 4430 I was using had r2p0. ?I
>> can't verify the accuracy of that claim right now, but my patch was
>> based on that assumption. ?Where are you getting r3p1 from?
>
> Apologies, that's what Santosh mentioned here:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/101302.html
>
I was mainly talking about OMAP4460 and the WA extension needed
for clean_all along with clean and invalidate.

May be since patch has both changes, for r3px and r2px, the
confusion came in.

Regards
Santosh

[PATCH v2] RFC: ARM: cache-l2x0: update workaround for PL310 errata 727915

[Colin Cross]

On Mon, May 28, 2012 at 3:11 AM, Will Deacon <will.deacon@arm.com> wrote:
> Hi Colin,
>
> On Fri, May 25, 2012 at 09:04:22PM +0100, Colin Cross wrote:
>> On Fri, May 25, 2012 at 7:56 AM, Will Deacon <will.deacon@arm.com> wrote:
>> > Right, ok. This is definitely a different problem though. Firstly, r3p1 is
>> > not affected by the original erratum and secondly the symptoms do not
>> > include deadlock. The clean during boot also sounds highly suspicious -- is
>> > there a chance you boot the kernel with a dirty L2? Is the L2 enabled or
>> > disabled when entering the kernel [I think u8500 has it enabled]?
>>
>> In a previous thread, I wrote that the 4430 I was using had r2p0. ?I
>> can't verify the accuracy of that claim right now, but my patch was
>> based on that assumption. ?Where are you getting r3p1 from?
>
> Apologies, that's what Santosh mentioned here:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/101302.html
>
> so, assuming that it's the same problem, it seems that both r3p1 and r2p0
> are affected.

Ah, I missed that.  The issue I was seeing only affected 4430 (r2p0).

>> When I said I couldn't reproduce the issue with clean during boot, I
>> meant that I couldn't trigger the issue during boot by calling clean
>> repeatedly. ?I could still reproduce the issue after boot even if the
>> cache was cleaned during boot.
>
> Ok, that's interesting. I wonder if erratum #769419 is to blame? We now have
> a workaround in the kernel for that, so you could give it a try if you're in
> a position to do so.

I don't have any 4430 devices any more.