diff mbox

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

Message ID 1449756855-19473-1-git-send-email-linus.walleij@linaro.org
State Superseded
Headers show

Commit Message

Linus Walleij Dec. 10, 2015, 2:14 p.m. UTC 
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 <linux@arm.linux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

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

-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Mark Rutland Dec. 10, 2015, 2:32 p.m. UTC | #1 
On Thu, Dec 10, 2015 at 03:14:15PM +0100, 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.

> 

> Cc: Russell King <linux@arm.linux.org.uk>

> Cc: Arnd Bergmann <arnd@arndb.de>

> Cc: devicetree@vger.kernel.org

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---

>  Documentation/devicetree/bindings/arm/l2cc.txt |  1 +

>  arch/arm/mm/cache-l2x0.c                       | 13 ++++++++++---

>  2 files changed, 11 insertions(+), 3 deletions(-)

> 

> diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt

> index d181b7c4c522..aae7387acbdb 100644

> --- a/Documentation/devicetree/bindings/arm/l2cc.txt

> +++ b/Documentation/devicetree/bindings/arm/l2cc.txt

> @@ -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.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Dec. 14, 2015, 1:23 p.m. UTC | #2 
On Thu, Dec 10, 2015 at 3:20 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 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.

>>

>> Cc: Russell King <linux@arm.linux.org.uk>

>> Cc: Arnd Bergmann <arnd@arndb.de>

>> Cc: devicetree@vger.kernel.org

>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

>

> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Dec. 14, 2015, 1:30 p.m. UTC | #3 
On Thu, Dec 10, 2015 at 3:32 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Dec 10, 2015 at 03:14:15PM +0100, 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.

>>

>> Cc: Russell King <linux@arm.linux.org.uk>

>> Cc: Arnd Bergmann <arnd@arndb.de>

>> Cc: devicetree@vger.kernel.org

>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

>> ---

>>  Documentation/devicetree/bindings/arm/l2cc.txt |  1 +

>>  arch/arm/mm/cache-l2x0.c                       | 13 ++++++++++---

>>  2 files changed, 11 insertions(+), 3 deletions(-)

>>

>> diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt

>> index d181b7c4c522..aae7387acbdb 100644

>> --- a/Documentation/devicetree/bindings/arm/l2cc.txt

>> +++ b/Documentation/devicetree/bindings/arm/l2cc.txt

>> @@ -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.


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 <catalin.marinas@arm.com>
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: <stable@kernel.org>
    Tested-by: Linus Walleij <linus.walleij@stericsson.com>

    Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

    Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>


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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Will Deacon Dec. 14, 2015, 1:32 p.m. UTC | #4 
On Mon, Dec 14, 2015 at 02:30:53PM +0100, Linus Walleij wrote:
> On Thu, Dec 10, 2015 at 3:32 PM, Mark Rutland <mark.rutland@arm.com> wrote:

> > On Thu, Dec 10, 2015 at 03:14:15PM +0100, Linus Walleij wrote:

> >> diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt

> >> index d181b7c4c522..aae7387acbdb 100644

> >> --- a/Documentation/devicetree/bindings/arm/l2cc.txt

> >> +++ b/Documentation/devicetree/bindings/arm/l2cc.txt

> >> @@ -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.

> 

> 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 <catalin.marinas@arm.com>

> 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: <stable@kernel.org>

>     Tested-by: Linus Walleij <linus.walleij@stericsson.com>

>     Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

>     Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

> 

> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
index d181b7c4c522..aae7387acbdb 100644
--- a/Documentation/devicetree/bindings/arm/l2cc.txt
+++ b/Documentation/devicetree/bindings/arm/l2cc.txt
@@ -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),
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 3f3008e5c662..9f9d54271aad 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -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