ARM: l2x0: make it possible to disable outer sync from DT

PATCH Superseded Linus Walleij 4 participants 6 messages Dec 10, 2015 – Dec 14, 2015

Subsystems (2)

Device tree bindings Arm (32-bit)

lore

Share & tools

lore

Linus Walleij

author

2015-12-10 14:14 UTC #0 lore raw

PATCH 1/2 ARM: l2x0: make it possible to disable outer sync from DT

Some RealView platforms have broken outer_sync, see:
http://marc.info/?l=linux-kernel&m=144846940516899&w=2

We got rid of the custom barriers from the machine by disabling outer sync, but that was just for the boardfile case. We have to be able to do the same in the device tree case.

Since __l2c_init() is cloning and copying the L2C vtable, we pass an argument to this function to optionally numb the outer sync operation if desired, before initializing the cache.

After this we can set up the cache correctly on the RealView PB11MPCore, and it boots rock solid with the cache enabled. Before this, spurious crashes would occur if we try to set up the cache properly.

Cc Russell King <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc devicetree at vger.kernel.org

Signed-off-by Linus Walleij <[email protected]>

---
 Documentation/devicetree/bindings/arm/l2cc.txt |  1 +
 arch/arm/mm/cache-l2x0.c                       | 13 ++++++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

Documentation/devicetree/bindings/arm/l2cc.txt +1 -0

index d181b7c4c522..aae7387acbdb 100644
@@ -75,6 +75,7 @@ Optional properties:
   specified to indicate that such transforms are precluded.
 - arm,parity-enable : enable parity checking on the L2 cache (L220 or PL310).
 - arm,parity-disable : disable parity checking on the L2 cache (L220 or PL310).
+- arm,outer-sync-disable : disable the outer sync operation on the L2 cache.
 - prefetch-data : Data prefetch. Value: <0> (forcibly disable), <1>
   (forcibly enable), property absent (retain settings set by firmware)
 - prefetch-instr : Instruction prefetch. Value: <0> (forcibly disable),

arch/arm/mm/cache-l2x0.c +10 -3

index 3f3008e5c662..9f9d54271aad 100644
@@ -790,7 +790,7 @@ static const struct l2c_init_data l2c310_init_fns __initconst = {
 };
 
 static int __init __l2c_init(const struct l2c_init_data *data,
-			     u32 aux_val, u32 aux_mask, u32 cache_id)
+			     u32 aux_val, u32 aux_mask, u32 cache_id, bool nosync)
 {
 	struct outer_cache_fns fns;
 	unsigned way_size_bits, ways;

@@ -866,6 +866,10 @@ static int __init __l2c_init(const struct l2c_init_data *data,
 	fns.configure = outer_cache.configure;
 	if (data->fixup)
 		data->fixup(l2x0_base, cache_id, &fns);
+	if (nosync) {
+		pr_info("L2C: disabling outer sync\n");
+		fns.sync = NULL;
+	}
 
 	/*
 	 * Check if l2x0 controller is already enabled.  If we are booting

@@ -925,7 +929,7 @@ void __init l2x0_init(void __iomem *base, u32 aux_val, u32 aux_mask)
 	if (data->save)
 		data->save(l2x0_base);
 
-	__l2c_init(data, aux_val, aux_mask, cache_id);
+	__l2c_init(data, aux_val, aux_mask, cache_id, false);
 }
 
 #ifdef CONFIG_OF

@@ -1724,6 +1728,7 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
 	struct resource res;
 	u32 cache_id, old_aux;
 	u32 cache_level = 2;
+	bool nosync = false;
 
 	np = of_find_matching_node(NULL, l2x0_ids);
 	if (!np)

@@ -1762,6 +1767,8 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
 	if (cache_level != 2)
 		pr_err("L2C: device tree specifies invalid cache level\n");
 
+	nosync = of_property_read_bool(np, "arm,outer-sync-disable");
+
 	/* Read back current (default) hardware configuration */
 	if (data->save)
 		data->save(l2x0_base);

@@ -1776,6 +1783,6 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
 	else
 		cache_id = readl_relaxed(l2x0_base + L2X0_CACHE_ID);
 
-	return __l2c_init(data, aux_val, aux_mask, cache_id);
+	return __l2c_init(data, aux_val, aux_mask, cache_id, nosync);
 }
 #endif

Signature · 1 line

2.4.3

Arnd Bergmann

2015-12-10 14:20 UTC #1 lore raw

PATCH 1/2 ARM: l2x0: make it possible to disable outer sync from DT

Quoted 17 lines · On Thursday 10 December 2015 15:14:15 Linus Walleij wrote:

Some RealView platforms have broken outer_sync, see:
http://marc.info/?l=linux-kernel&m=144846940516899&w=2
We got rid of the custom barriers from the machine by disabling outer sync, but that was just for the boardfile case. We have to be able to do the same in the device tree case.
Since __l2c_init() is cloning and copying the L2C vtable, we pass an argument to this function to optionally numb the outer sync operation if desired, before initializing the cache.
After this we can set up the cache correctly on the RealView PB11MPCore, and it boots rock solid with the cache enabled. Before this, spurious crashes would occur if we try to set up the cache properly.

Quoted 4 lines

Cc Russell King <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc devicetree at vger.kernel.org
Signed-off-by Linus Walleij <[email protected]>

This has the same effect as my "ARM: realview: remove private barrier implementation" patch and replaces part of that, correct?

Arnd

Linus Walleij

author

2015-12-14 13:23 UTC #2 lore raw

PATCH 1/2 ARM: l2x0: make it possible to disable outer sync from DT

Quoted 18 lines · Arnd Bergmann

Quoted 17 lines · On Thursday 10 December 2015 15:14:15 Linus Walleij wrote:
Some RealView platforms have broken outer_sync, see:
http://marc.info/?l=linux-kernel&m=144846940516899&w=2
We got rid of the custom barriers from the machine by disabling outer sync, but that was just for the boardfile case. We have to be able to do the same in the device tree case.
Since __l2c_init() is cloning and copying the L2C vtable, we pass an argument to this function to optionally numb the outer sync operation if desired, before initializing the cache.
After this we can set up the cache correctly on the RealView PB11MPCore, and it boots rock solid with the cache enabled. Before this, spurious crashes would occur if we try to set up the cache properly.

Quoted 7 lines

Quoted 4 lines
Cc Russell King <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc devicetree at vger.kernel.org
Signed-off-by Linus Walleij <[email protected]>
This has the same effect as my "ARM: realview: remove private barrier implementation" patch and replaces part of that, correct?

Yes, but applied to a DT-only bootpath with a minimum of board file code, kept in l2x0.c just this bool in the DT node.

Yours,
Linus Walleij

Mark Rutland

2015-12-10 14:32 UTC #3 lore raw

PATCH 1/2 ARM: l2x0: make it possible to disable outer sync from DT

In reply to Linus Walleij

Quoted 31 lines · Linus Walleij

Some RealView platforms have broken outer_sync, see:
http://marc.info/?l=linux-kernel&m=144846940516899&w=2
We got rid of the custom barriers from the machine by disabling outer sync, but that was just for the boardfile case. We have to be able to do the same in the device tree case.
Since __l2c_init() is cloning and copying the L2C vtable, we pass an argument to this function to optionally numb the outer sync operation if desired, before initializing the cache.
After this we can set up the cache correctly on the RealView PB11MPCore, and it boots rock solid with the cache enabled. Before this, spurious crashes would occur if we try to set up the cache properly.
Cc Russell King <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc devicetree at vger.kernel.org
Signed-off-by Linus Walleij <[email protected]>
---
 Documentation/devicetree/bindings/arm/l2cc.txt |  1 +
 arch/arm/mm/cache-l2x0.c                       | 13 ++++++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/arm/l2cc.txt +0 -0
index d181b7c4c522..aae7387acbdb 100644

Documentation/devicetree/bindings/arm/l2cc.txt Optional properties: · line 78

Quoted 4 lines

Documentation/devicetree/bindings/arm/l2cc.txt +1 -0

@@ -75,6 +75,7 @@ Optional properties:
   specified to indicate that such transforms are precluded.
 - arm,parity-enable : enable parity checking on the L2 cache (L220 or PL310).
 - arm,parity-disable : disable parity checking on the L2 cache (L220 or PL310).
+- arm,outer-sync-disable : disable the outer sync operation on the L2 cache.

I'm not sure what this means from a HW perspective. The "outer sync" operation is Linux terminology and doesn't show up in the TRM for L220.

What is the (integration? HW?) bug that we're trying to avoid the effect of? It sounds like we should be describing that.

We should at least have a better definition of what this means we must avoid.

Thanks
Mark.

Linus Walleij

author

2015-12-14 13:30 UTC #4 lore raw

PATCH 1/2 ARM: l2x0: make it possible to disable outer sync from DT

Quoted 40 lines · Mark Rutland

Quoted 31 lines · Linus Walleij
Some RealView platforms have broken outer_sync, see:
http://marc.info/?l=linux-kernel&m=144846940516899&w=2
We got rid of the custom barriers from the machine by disabling outer sync, but that was just for the boardfile case. We have to be able to do the same in the device tree case.
Since __l2c_init() is cloning and copying the L2C vtable, we pass an argument to this function to optionally numb the outer sync operation if desired, before initializing the cache.
After this we can set up the cache correctly on the RealView PB11MPCore, and it boots rock solid with the cache enabled. Before this, spurious crashes would occur if we try to set up the cache properly.
Cc Russell King <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc devicetree at vger.kernel.org
Signed-off-by Linus Walleij <[email protected]>
---
 Documentation/devicetree/bindings/arm/l2cc.txt |  1 +
 arch/arm/mm/cache-l2x0.c                       | 13 ++++++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/arm/l2cc.txt +0 -0
index d181b7c4c522..aae7387acbdb 100644
Documentation/devicetree/bindings/arm/l2cc.txt Optional properties: · line 78
Quoted 4 lines
Documentation/devicetree/bindings/arm/l2cc.txt +1 -0
@@ -75,6 +75,7 @@ Optional properties:
   specified to indicate that such transforms are precluded.
 - arm,parity-enable : enable parity checking on the L2 cache (L220 or PL310).
 - arm,parity-disable : disable parity checking on the L2 cache (L220 or PL310).
+- arm,outer-sync-disable : disable the outer sync operation on the L2 cache.
I'm not sure what this means from a HW perspective. The "outer sync" operation is Linux terminology and doesn't show up in the TRM for L220.

Quoted 5 lines

What is the (integration? HW?) bug that we're trying to avoid the effect of? It sounds like we should be describing that.
We should at least have a better definition of what this means we must avoid.

So this code is a bit ancient, and we're trying to migrate it to device tree.

It was inspired by this board file patch from Arnd:
http://marc.info/?l=linux-arm-kernel&m=144846938616893&w=2

Then I go back and dig in the code and I find this:

commit 2503a5ecd86c002506001eba432c524ea009fe7f

Author: Catalin Marinas <[email protected]>
Date:   Thu Jul 1 13:21:47 2010 +0100

ARM: 6201/1: RealView: Do not use outer_sync() on ARM11MPCore
boards with L220

RealView boards with certain revisions of the L220 cache controller (ARM11* processors only) may have issues (hardware deadlock) with the
recent changes to
the mb() barrier implementation (DSB followed by an L2 cache
sync). The patch
redefines the RealView ARM11MPCore mandatory barriers without the
outer_sync()
call.

Cc <[email protected]>

Tested-by Linus Walleij <[email protected]>

Signed-off-by Catalin Marinas <[email protected]>

Signed-off-by Russell King <[email protected]>

And that is as much as it says.

It is working around a hardware lockup in some core tiles. Is it enough if I add this
to the explanation or do we need to send Catalin or Will down in the archives to read netlists? ;)

Yours,
Linus Walleij

Will Deacon

2015-12-14 13:32 UTC #5 lore raw

PATCH 1/2 ARM: l2x0: make it possible to disable outer sync from DT

Quoted 49 lines · Linus Walleij

Quoted 14 lines · Mark Rutland
Documentation/devicetree/bindings/arm/l2cc.txt Optional properties: · line 78
Quoted 9 lines · Linus Walleij
Documentation/devicetree/bindings/arm/l2cc.txt +1 -0
index d181b7c4c522..aae7387acbdb 100644
@@ -75,6 +75,7 @@ Optional properties:
   specified to indicate that such transforms are precluded.
 - arm,parity-enable : enable parity checking on the L2 cache (L220 or PL310).
 - arm,parity-disable : disable parity checking on the L2 cache (L220 or PL310).
+- arm,outer-sync-disable : disable the outer sync operation on the L2 cache.
I'm not sure what this means from a HW perspective. The "outer sync" operation is Linux terminology and doesn't show up in the TRM for L220.
Quoted 5 lines
What is the (integration? HW?) bug that we're trying to avoid the effect of? It sounds like we should be describing that.
We should at least have a better definition of what this means we must avoid.
So this code is a bit ancient, and we're trying to migrate it to device tree.
It was inspired by this board file patch from Arnd:
http://marc.info/?l=linux-arm-kernel&m=144846938616893&w=2
Then I go back and dig in the code and I find this:
commit 2503a5ecd86c002506001eba432c524ea009fe7f
Author: Catalin Marinas <[email protected]>
Date:   Thu Jul 1 13:21:47 2010 +0100
ARM: 6201/1: RealView: Do not use outer_sync() on ARM11MPCore
boards with L220
RealView boards with certain revisions of the L220 cache controller (ARM11* processors only) may have issues (hardware deadlock) with the
recent changes to
the mb() barrier implementation (DSB followed by an L2 cache
sync). The patch
redefines the RealView ARM11MPCore mandatory barriers without the
outer_sync()
call.
Cc <[email protected]>
Tested-by Linus Walleij <[email protected]>
Signed-off-by Catalin Marinas <[email protected]>
Signed-off-by Russell King <[email protected]>

Quoted 6 lines

And that is as much as it says.
It is working around a hardware lockup in some core tiles. Is it enough if I add this
to the explanation or do we need to send Catalin or Will down in the archives to read netlists? ;)

/me delegates to Rutland.

Mark -- still want a better definition? ;)

Will