[PATCHv5 0/3] ARM: implement workaround for Cortex-A9/PL310/PCIe deadlock

[PATCHv5 0/3] ARM: implement workaround for Cortex-A9/PL310/PCIe deadlock

[Thomas Petazzoni]

Russell, Will, Catalin,

This patch series adresses a problem that affects the newer Marvell
Armada 375 and 38x SOCs, based on Cortex-A9+PL310, combined with the
Marvell PCIe hardware unit. When the hardware I/O coherency is
enabled, the combination of Cortex-A9/PL310/Marvell PCIe hardware unit
will quickly cause a deadlock when the PCIe bus is stressed.

The workaround for this problem has been suggested by ARM, and
consists in two things:

 (1) Map the PCIe regions as strongly-ordered

 (2) Disable the outer cache sync of the PL310 when hardware I/O
     coherency is used, since it is unneeded and causes the deadlock.

Some of the problems have already been solved and the corresponding
patches merged mainline. However, due to the L2CC cleanup done by
Russell King, the change to the PL310 driver was not merged, and there
are some consequences to the L2CC cleanup that need to be
addressed. The following three patches address those problems:

 * PATCH 1/3 extends the l2x0 cache driver with a new property
   "arm,io-coherent", valid for the PL310, which makes the driver
   disable the outer cache sync operation. This patch should be routed
   through Russell's tree.

 * PATCH 2/3 moves the registration of a quirk later, and is merely a
   preparation for PATCH 3/3. It should be merged by the mvebu
   maintainers.

 * PATCH 3/3 moves the initialization of the SCU, coherency and
   mvebu-mbus earlier (to ->init_irq instead of ->init_time), because
   we must adjust the Device Tree property of the PL310 cache
   controller *before* the L2CC driver is initialized, and it is
   initialized right after ->init_irq() is called. It should be merged
   by the mvebu maintainers.

This patch series is based on the current Linus tree, at
dfb945473ae8528fd885607b6fa843c676745e0c. Let me know if this is the
right tree to base this code on, or if it should be based on some
other version or tree.

Without this patch series, doing heavy PCIe traffic (like running 6 or
7 parallel dd processes reading from a SATA drive) during a few dozens
of seconds is sufficient to completely deadlock the system, so this is
really a bug fix.

Changes since v5:

 - Drop patches that have been merged during the 3.16 merge window.

 - Adapt the L2CC driver changes to the cleanups made by Russell King.

 - Add patches to ensure the L2CC DT property is added before the L2CC
   driver is initialized.

Changes since v4:

 - Re-introduce the patch to allow sub-architectures to override the
   memory type used for PCI I/O mappings, since switching to
   strongly-ordered for all platforms does not seem to be well
   accepted/understood at this point.

 - Remove the of_device_is_compatible() check for the PL310, when
   testing for 'arm,io-coherent'. Suggested by Rob Herring. However,
   the code tetsing 'arm,io-coherent' cannot be moved into
   pl310_of_setup(), because this function is called *before* the
   'outer_cache' structure is initialized.

 - Add a separate patch to use the pci_ioremap_set_mem_type() API in
   mach-mvebu/coherency.c.

Changes since v3:

 - Withdrawn all Acked-by tags since the changes compared to v3 are
   quite significant.

 - Instead of introducing a small mechanism to allow each
   sub-architecture to override the memory type used for PCI I/O
   mappings, simply make all of them mapped MT_UNCACHED instead of
   MT_DEVICE, as suggested by Arnd Bergmann. This also has the nice
   consequence that there is no longer a build dependency between
   PATCH 3/3 and PATCH 1/3. Suggested by Arnd Bergmann.

 - Change the name of the new property of the PL310 DT binding from
   the too generic 'dma-coherent' to 'arm,io-coherent'. Suggested by
   Rob Herring.

 - Instead of adding a complete set of L2 cache operations in
   cache-l2x0.c, simply nullify the outer_cache.sync operation when
   'arm,io-coherent' is specified. Suggested by Rob Herring.

 - Move the Armada 375/38x specific code from mach-mvebu/board-v7.c to
   mach-mvebu/coherency.c, which makes more sense. Suggested by Arnd
   Bergmann.

Changes since v2:

 - Added Acked-by from Catalin on "ARM: mm: allow sub-architectures to
   override PCI I/O memory type".

 - Dropped the patch fixing the of_update_property() function, since
   we're no longer using it.

 - Instead of using a different compatible string to identify PL310
   used in an I/O coherent configuration, use a separate boolean
   property. Suggested by Catalin.

 - Rework the mach-mvebu/coherency.c to add the boolean property
   "dma-coherent" when needed instead of updating the compatible
   string of the cache controller.

Changes since v1:

 - Instead of introducing separate l2x0 initialization functions, rely
   on a separate compatible string to identify whether we're coherent
   or not. The compatible string *has* to be modified at runtime,
   because Armada 375 and 38x are only I/O coherent when in SMP
   mode. In non-SMP mode, they are not I/O coherent, so we cannot
   change the DT to 'arm,pl310-coherent-cache'.

 - Addition of the drivers/of fix to be able to use
   of_update_property() early and fix up the PL310 compatible string,
   as explained in the previous item.

Thanks!

Thomas

Thomas Petazzoni (3):
  ARM: mm: add support for HW coherent systems in PL310 cache
  ARM: mvebu: move Armada 375 external abort logic as a quirk
  ARM: mvebu: update L2/PCIe deadlock workaround after L2CC cleanup

 Documentation/devicetree/bindings/arm/l2cc.txt |  3 +++
 arch/arm/mach-mvebu/board-v7.c                 | 29 +++++++++++++++---------
 arch/arm/mm/cache-l2x0.c                       | 31 ++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 10 deletions(-)

-- 
2.0.0

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

[PATCHv5 0/3] ARM: implement workaround for Cortex-A9/PL310/PCIe deadlock

[Thomas Petazzoni]

Russell, Will, Catalin,

This patch series adresses a problem that affects the newer Marvell
Armada 375 and 38x SOCs, based on Cortex-A9+PL310, combined with the
Marvell PCIe hardware unit. When the hardware I/O coherency is
enabled, the combination of Cortex-A9/PL310/Marvell PCIe hardware unit
will quickly cause a deadlock when the PCIe bus is stressed.

The workaround for this problem has been suggested by ARM, and
consists in two things:

 (1) Map the PCIe regions as strongly-ordered

 (2) Disable the outer cache sync of the PL310 when hardware I/O
     coherency is used, since it is unneeded and causes the deadlock.

Some of the problems have already been solved and the corresponding
patches merged mainline. However, due to the L2CC cleanup done by
Russell King, the change to the PL310 driver was not merged, and there
are some consequences to the L2CC cleanup that need to be
addressed. The following three patches address those problems:

 * PATCH 1/3 extends the l2x0 cache driver with a new property
   "arm,io-coherent", valid for the PL310, which makes the driver
   disable the outer cache sync operation. This patch should be routed
   through Russell's tree.

 * PATCH 2/3 moves the registration of a quirk later, and is merely a
   preparation for PATCH 3/3. It should be merged by the mvebu
   maintainers.

 * PATCH 3/3 moves the initialization of the SCU, coherency and
   mvebu-mbus earlier (to ->init_irq instead of ->init_time), because
   we must adjust the Device Tree property of the PL310 cache
   controller *before* the L2CC driver is initialized, and it is
   initialized right after ->init_irq() is called. It should be merged
   by the mvebu maintainers.

This patch series is based on the current Linus tree, at
dfb945473ae8528fd885607b6fa843c676745e0c. Let me know if this is the
right tree to base this code on, or if it should be based on some
other version or tree.

Without this patch series, doing heavy PCIe traffic (like running 6 or
7 parallel dd processes reading from a SATA drive) during a few dozens
of seconds is sufficient to completely deadlock the system, so this is
really a bug fix.

Changes since v5:

 - Drop patches that have been merged during the 3.16 merge window.

 - Adapt the L2CC driver changes to the cleanups made by Russell King.

 - Add patches to ensure the L2CC DT property is added before the L2CC
   driver is initialized.

Changes since v4:

 - Re-introduce the patch to allow sub-architectures to override the
   memory type used for PCI I/O mappings, since switching to
   strongly-ordered for all platforms does not seem to be well
   accepted/understood at this point.

 - Remove the of_device_is_compatible() check for the PL310, when
   testing for 'arm,io-coherent'. Suggested by Rob Herring. However,
   the code tetsing 'arm,io-coherent' cannot be moved into
   pl310_of_setup(), because this function is called *before* the
   'outer_cache' structure is initialized.

 - Add a separate patch to use the pci_ioremap_set_mem_type() API in
   mach-mvebu/coherency.c.

Changes since v3:

 - Withdrawn all Acked-by tags since the changes compared to v3 are
   quite significant.

 - Instead of introducing a small mechanism to allow each
   sub-architecture to override the memory type used for PCI I/O
   mappings, simply make all of them mapped MT_UNCACHED instead of
   MT_DEVICE, as suggested by Arnd Bergmann. This also has the nice
   consequence that there is no longer a build dependency between
   PATCH 3/3 and PATCH 1/3. Suggested by Arnd Bergmann.

 - Change the name of the new property of the PL310 DT binding from
   the too generic 'dma-coherent' to 'arm,io-coherent'. Suggested by
   Rob Herring.

 - Instead of adding a complete set of L2 cache operations in
   cache-l2x0.c, simply nullify the outer_cache.sync operation when
   'arm,io-coherent' is specified. Suggested by Rob Herring.

 - Move the Armada 375/38x specific code from mach-mvebu/board-v7.c to
   mach-mvebu/coherency.c, which makes more sense. Suggested by Arnd
   Bergmann.

Changes since v2:

 - Added Acked-by from Catalin on "ARM: mm: allow sub-architectures to
   override PCI I/O memory type".

 - Dropped the patch fixing the of_update_property() function, since
   we're no longer using it.

 - Instead of using a different compatible string to identify PL310
   used in an I/O coherent configuration, use a separate boolean
   property. Suggested by Catalin.

 - Rework the mach-mvebu/coherency.c to add the boolean property
   "dma-coherent" when needed instead of updating the compatible
   string of the cache controller.

Changes since v1:

 - Instead of introducing separate l2x0 initialization functions, rely
   on a separate compatible string to identify whether we're coherent
   or not. The compatible string *has* to be modified at runtime,
   because Armada 375 and 38x are only I/O coherent when in SMP
   mode. In non-SMP mode, they are not I/O coherent, so we cannot
   change the DT to 'arm,pl310-coherent-cache'.

 - Addition of the drivers/of fix to be able to use
   of_update_property() early and fix up the PL310 compatible string,
   as explained in the previous item.

Thanks!

Thomas

Thomas Petazzoni (3):
  ARM: mm: add support for HW coherent systems in PL310 cache
  ARM: mvebu: move Armada 375 external abort logic as a quirk
  ARM: mvebu: update L2/PCIe deadlock workaround after L2CC cleanup

 Documentation/devicetree/bindings/arm/l2cc.txt |  3 +++
 arch/arm/mach-mvebu/board-v7.c                 | 29 +++++++++++++++---------
 arch/arm/mm/cache-l2x0.c                       | 31 ++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 10 deletions(-)

-- 
2.0.0

[PATCHv5 1/3] ARM: mm: add support for HW coherent systems in PL310 cache

[Thomas Petazzoni]

When a PL310 cache is used on a system that provides hardware
coherency, the outer cache sync operation is useless, and can be
skipped. Moreover, on some systems, it is harmful as it causes
deadlocks between the Marvell coherency mechanism, the Marvell PCIe
controller and the Cortex-A9.

To avoid this, this commit introduces a new Device Tree property
'arm,io-coherent' for the L2 cache controller node, valid only for the
PL310 cache. It identifies the usage of the PL310 cache in an I/O
coherent configuration. Internally, it makes the driver disable the
outer cache sync operation.

Note that technically speaking, a fully coherent system wouldn't
require any of the other .outer_cache operations. However, in
practice, when booting secondary CPUs, these are not yet coherent, and
therefore a set of cache maintenance operations are necessary at this
point. This explains why we keep the other .outer_cache operations and
only ->sync is disabled.

While in theory any write to a PL310 register could cause the
deadlock, in practice, disabling ->sync is sufficient to workaround
the deadlock, since the other cache maintenance operations are only
used in very specific situations.

Contrary to previous versions of this patch, this new version does not
simply NULL-ify the ->sync member, because the l2c_init_data
structures are now 'const' and therefore cannot be modified, which is
a good thing. Therefore, this patch introduces a separate
l2c_init_data instance, called of_l2c310_coherent_data.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
This patch is based on the latest mainline, as it depends on the L2CC
cleanup from Russell King.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/l2cc.txt |  3 +++
 arch/arm/mm/cache-l2x0.c                       | 31 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
index b513cb8..af527ee 100644
--- a/Documentation/devicetree/bindings/arm/l2cc.txt
+++ b/Documentation/devicetree/bindings/arm/l2cc.txt
@@ -40,6 +40,9 @@ Optional properties:
 - arm,filter-ranges : <start length> Starting address and length of window to
   filter. Addresses in the filter window are directed to the M1 port. Other
   addresses will go to the M0 port.
+- arm,io-coherent : indicates that the system is operating in an hardware
+  I/O coherent mode. Valid only when the arm,pl310-cache compatible
+  string is used.
 - interrupts : 1 combined interrupt.
 - cache-id-part: cache id part number to be used if it is not present
   on hardware
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index efc5cab..076172b 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1069,6 +1069,33 @@ static const struct l2c_init_data of_l2c310_data __initconst = {
 };
 
 /*
+ * This is a variant of the of_l2c310_data with .sync set to
+ * NULL. Outer sync operations are not needed when the system is I/O
+ * coherent, and potentially harmful in certain situations (PCIe/PL310
+ * deadlock on Armada 375/38x due to hardware I/O coherency). The
+ * other operations are kept because they are infrequent (therefore do
+ * not cause the deadlock in practice) and needed for secondary CPU
+ * boot and other power management activities.
+ */
+static const struct l2c_init_data of_l2c310_coherent_data __initconst = {
+	.type = "L2C-310 Coherent",
+	.way_size_0 = SZ_8K,
+	.num_lock = 8,
+	.of_parse = l2c310_of_parse,
+	.enable = l2c310_enable,
+	.fixup = l2c310_fixup,
+	.save  = l2c310_save,
+	.outer_cache = {
+		.inv_range   = l2c210_inv_range,
+		.clean_range = l2c210_clean_range,
+		.flush_range = l2c210_flush_range,
+		.flush_all   = l2c210_flush_all,
+		.disable     = l2c310_disable,
+		.resume      = l2c310_resume,
+	},
+};
+
+/*
  * Note that the end addresses passed to Linux primitives are
  * noninclusive, while the hardware cache range operations use
  * inclusive start and end addresses.
@@ -1487,6 +1514,10 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
 
 	data = of_match_node(l2x0_ids, np)->data;
 
+	if (of_device_is_compatible(np, "arm,pl310-cache") &&
+	    of_property_read_bool(np, "arm,io-coherent"))
+		data = &of_l2c310_coherent_data;
+
 	old_aux = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
 	if (old_aux != ((old_aux & aux_mask) | aux_val)) {
 		pr_warn("L2C: platform modifies aux control register: 0x%08x -> 0x%08x\n",
-- 
2.0.0

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

[PATCHv5 1/3] ARM: mm: add support for HW coherent systems in PL310 cache

[Thomas Petazzoni]

When a PL310 cache is used on a system that provides hardware
coherency, the outer cache sync operation is useless, and can be
skipped. Moreover, on some systems, it is harmful as it causes
deadlocks between the Marvell coherency mechanism, the Marvell PCIe
controller and the Cortex-A9.

To avoid this, this commit introduces a new Device Tree property
'arm,io-coherent' for the L2 cache controller node, valid only for the
PL310 cache. It identifies the usage of the PL310 cache in an I/O
coherent configuration. Internally, it makes the driver disable the
outer cache sync operation.

Note that technically speaking, a fully coherent system wouldn't
require any of the other .outer_cache operations. However, in
practice, when booting secondary CPUs, these are not yet coherent, and
therefore a set of cache maintenance operations are necessary at this
point. This explains why we keep the other .outer_cache operations and
only ->sync is disabled.

While in theory any write to a PL310 register could cause the
deadlock, in practice, disabling ->sync is sufficient to workaround
the deadlock, since the other cache maintenance operations are only
used in very specific situations.

Contrary to previous versions of this patch, this new version does not
simply NULL-ify the ->sync member, because the l2c_init_data
structures are now 'const' and therefore cannot be modified, which is
a good thing. Therefore, this patch introduces a separate
l2c_init_data instance, called of_l2c310_coherent_data.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
This patch is based on the latest mainline, as it depends on the L2CC
cleanup from Russell King.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 Documentation/devicetree/bindings/arm/l2cc.txt |  3 +++
 arch/arm/mm/cache-l2x0.c                       | 31 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
index b513cb8..af527ee 100644
--- a/Documentation/devicetree/bindings/arm/l2cc.txt
+++ b/Documentation/devicetree/bindings/arm/l2cc.txt
@@ -40,6 +40,9 @@ Optional properties:
 - arm,filter-ranges : <start length> Starting address and length of window to
   filter. Addresses in the filter window are directed to the M1 port. Other
   addresses will go to the M0 port.
+- arm,io-coherent : indicates that the system is operating in an hardware
+  I/O coherent mode. Valid only when the arm,pl310-cache compatible
+  string is used.
 - interrupts : 1 combined interrupt.
 - cache-id-part: cache id part number to be used if it is not present
   on hardware
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index efc5cab..076172b 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1069,6 +1069,33 @@ static const struct l2c_init_data of_l2c310_data __initconst = {
 };
 
 /*
+ * This is a variant of the of_l2c310_data with .sync set to
+ * NULL. Outer sync operations are not needed when the system is I/O
+ * coherent, and potentially harmful in certain situations (PCIe/PL310
+ * deadlock on Armada 375/38x due to hardware I/O coherency). The
+ * other operations are kept because they are infrequent (therefore do
+ * not cause the deadlock in practice) and needed for secondary CPU
+ * boot and other power management activities.
+ */
+static const struct l2c_init_data of_l2c310_coherent_data __initconst = {
+	.type = "L2C-310 Coherent",
+	.way_size_0 = SZ_8K,
+	.num_lock = 8,
+	.of_parse = l2c310_of_parse,
+	.enable = l2c310_enable,
+	.fixup = l2c310_fixup,
+	.save  = l2c310_save,
+	.outer_cache = {
+		.inv_range   = l2c210_inv_range,
+		.clean_range = l2c210_clean_range,
+		.flush_range = l2c210_flush_range,
+		.flush_all   = l2c210_flush_all,
+		.disable     = l2c310_disable,
+		.resume      = l2c310_resume,
+	},
+};
+
+/*
  * Note that the end addresses passed to Linux primitives are
  * noninclusive, while the hardware cache range operations use
  * inclusive start and end addresses.
@@ -1487,6 +1514,10 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
 
 	data = of_match_node(l2x0_ids, np)->data;
 
+	if (of_device_is_compatible(np, "arm,pl310-cache") &&
+	    of_property_read_bool(np, "arm,io-coherent"))
+		data = &of_l2c310_coherent_data;
+
 	old_aux = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
 	if (old_aux != ((old_aux & aux_mask) | aux_val)) {
 		pr_warn("L2C: platform modifies aux control register: 0x%08x -> 0x%08x\n",
-- 
2.0.0

[PATCHv5 2/3] ARM: mvebu: move Armada 375 external abort logic as a quirk

[Thomas Petazzoni]

In preparation to a small re-organization of the initialization
sequence in board-v7.c, this commit moves the registration of the
custom external abort handler on Armada 375 later in the boot
sequence, and makes it more similar to the other quirks that we
already have. There is indeed no need to register this abort handler
particularly early, it simply needs to be registered before switching
to userspace.

In addition to this, this commit makes the registration of the custom
abort handler conditional on Armada 375 Z1, because Armada 375 A0 and
later iterations are not affected by the issue.

This commit was tested on both Armada 375 Z1 and Armada 375 A0
platforms.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/mach-mvebu/board-v7.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
index 8bb742f..804c952 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -78,10 +78,17 @@ static void __init mvebu_timer_and_clk_init(void)
 	mvebu_scu_enable();
 	coherency_init();
 	BUG_ON(mvebu_mbus_dt_init(coherency_available()));
+}
+
+static void __init external_abort_quirk(void)
+{
+	u32 dev, rev;
 
-	if (of_machine_is_compatible("marvell,armada375"))
-		hook_fault_code(16 + 6, armada_375_external_abort_wa, SIGBUS, 0,
-				"imprecise external abort");
+	if (mvebu_get_soc_id(&dev, &rev) == 0 && rev > ARMADA_375_Z1_REV)
+		return;
+
+	hook_fault_code(16 + 6, armada_375_external_abort_wa, SIGBUS, 0,
+			"imprecise external abort");
 }
 
 static void __init i2c_quirk(void)
@@ -169,8 +176,10 @@ static void __init mvebu_dt_init(void)
 {
 	if (of_machine_is_compatible("plathome,openblocks-ax3-4"))
 		i2c_quirk();
-	if (of_machine_is_compatible("marvell,a375-db"))
+	if (of_machine_is_compatible("marvell,a375-db")) {
+		external_abort_quirk();
 		thermal_quirk();
+	}
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
-- 
2.0.0

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

[PATCHv5 2/3] ARM: mvebu: move Armada 375 external abort logic as a quirk

[Thomas Petazzoni]

In preparation to a small re-organization of the initialization
sequence in board-v7.c, this commit moves the registration of the
custom external abort handler on Armada 375 later in the boot
sequence, and makes it more similar to the other quirks that we
already have. There is indeed no need to register this abort handler
particularly early, it simply needs to be registered before switching
to userspace.

In addition to this, this commit makes the registration of the custom
abort handler conditional on Armada 375 Z1, because Armada 375 A0 and
later iterations are not affected by the issue.

This commit was tested on both Armada 375 Z1 and Armada 375 A0
platforms.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/mach-mvebu/board-v7.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
index 8bb742f..804c952 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -78,10 +78,17 @@ static void __init mvebu_timer_and_clk_init(void)
 	mvebu_scu_enable();
 	coherency_init();
 	BUG_ON(mvebu_mbus_dt_init(coherency_available()));
+}
+
+static void __init external_abort_quirk(void)
+{
+	u32 dev, rev;
 
-	if (of_machine_is_compatible("marvell,armada375"))
-		hook_fault_code(16 + 6, armada_375_external_abort_wa, SIGBUS, 0,
-				"imprecise external abort");
+	if (mvebu_get_soc_id(&dev, &rev) == 0 && rev > ARMADA_375_Z1_REV)
+		return;
+
+	hook_fault_code(16 + 6, armada_375_external_abort_wa, SIGBUS, 0,
+			"imprecise external abort");
 }
 
 static void __init i2c_quirk(void)
@@ -169,8 +176,10 @@ static void __init mvebu_dt_init(void)
 {
 	if (of_machine_is_compatible("plathome,openblocks-ax3-4"))
 		i2c_quirk();
-	if (of_machine_is_compatible("marvell,a375-db"))
+	if (of_machine_is_compatible("marvell,a375-db")) {
+		external_abort_quirk();
 		thermal_quirk();
+	}
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
-- 
2.0.0

[PATCHv5 3/3] ARM: mvebu: update L2/PCIe deadlock workaround after L2CC cleanup

[Thomas Petazzoni]

Commit 497a92308af8e9385fa3d135f7f416a997e4b93b ("ARM: mvebu:
implement L2/PCIe deadlock workaround") introduced some logic in
coherency.c to adjust the PL310 cache controller Device Tree node of
Armada 375 and Armada 38x platform to include the 'arm,io-coherent'
property if the system is running with hardware I/O coherency enabled.

However, with the L2CC driver cleanup done by Russell King, the
initialization of the L2CC driver has been moved earlier, and is now
part of the init_IRQ() ARM function in
arch/arm/kernel/irq.c. Therefore, calling coherency_init() in
->init_time() is now too late, as the Device Tree property gets added
too late (after the L2CC driver has been initialized).

In order to fix this, this commit removes the ->init_time() callback
use in board-v7.c and replaces it with an ->init_irq() callback. We
therefore no longer use the default ->init_irq() callback, but we now
use the default ->init_time() callback.

In this newly introduced ->init_irq() callback, we call irqchip_init()
which is the default behavior when ->init_irq() isn't defined, and
then do the initialization related to the coherency: SCU, coherency
fabric, and mvebu-mbus (which is needed to start secondary CPUs).

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/mach-mvebu/board-v7.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
index 804c952..b2524d6 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -23,6 +23,7 @@
 #include <linux/mbus.h>
 #include <linux/signal.h>
 #include <linux/slab.h>
+#include <linux/irqchip.h>
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
@@ -71,10 +72,9 @@ static int armada_375_external_abort_wa(unsigned long addr, unsigned int fsr,
 	return 1;
 }
 
-static void __init mvebu_timer_and_clk_init(void)
+static void __init mvebu_init_irq(void)
 {
-	of_clk_init(NULL);
-	clocksource_of_init();
+	irqchip_init();
 	mvebu_scu_enable();
 	coherency_init();
 	BUG_ON(mvebu_mbus_dt_init(coherency_available()));
@@ -194,7 +194,7 @@ DT_MACHINE_START(ARMADA_370_XP_DT, "Marvell Armada 370/XP (Device Tree)")
 	.l2c_aux_mask	= ~0,
 	.smp		= smp_ops(armada_xp_smp_ops),
 	.init_machine	= mvebu_dt_init,
-	.init_time	= mvebu_timer_and_clk_init,
+	.init_irq       = mvebu_init_irq,
 	.restart	= mvebu_restart,
 	.dt_compat	= armada_370_xp_dt_compat,
 MACHINE_END
@@ -207,7 +207,7 @@ static const char * const armada_375_dt_compat[] = {
 DT_MACHINE_START(ARMADA_375_DT, "Marvell Armada 375 (Device Tree)")
 	.l2c_aux_val	= 0,
 	.l2c_aux_mask	= ~0,
-	.init_time	= mvebu_timer_and_clk_init,
+	.init_irq       = mvebu_init_irq,
 	.init_machine	= mvebu_dt_init,
 	.restart	= mvebu_restart,
 	.dt_compat	= armada_375_dt_compat,
@@ -222,7 +222,7 @@ static const char * const armada_38x_dt_compat[] = {
 DT_MACHINE_START(ARMADA_38X_DT, "Marvell Armada 380/385 (Device Tree)")
 	.l2c_aux_val	= 0,
 	.l2c_aux_mask	= ~0,
-	.init_time	= mvebu_timer_and_clk_init,
+	.init_irq       = mvebu_init_irq,
 	.restart	= mvebu_restart,
 	.dt_compat	= armada_38x_dt_compat,
 MACHINE_END
-- 
2.0.0

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

[PATCHv5 3/3] ARM: mvebu: update L2/PCIe deadlock workaround after L2CC cleanup

[Thomas Petazzoni]

Commit 497a92308af8e9385fa3d135f7f416a997e4b93b ("ARM: mvebu:
implement L2/PCIe deadlock workaround") introduced some logic in
coherency.c to adjust the PL310 cache controller Device Tree node of
Armada 375 and Armada 38x platform to include the 'arm,io-coherent'
property if the system is running with hardware I/O coherency enabled.

However, with the L2CC driver cleanup done by Russell King, the
initialization of the L2CC driver has been moved earlier, and is now
part of the init_IRQ() ARM function in
arch/arm/kernel/irq.c. Therefore, calling coherency_init() in
->init_time() is now too late, as the Device Tree property gets added
too late (after the L2CC driver has been initialized).

In order to fix this, this commit removes the ->init_time() callback
use in board-v7.c and replaces it with an ->init_irq() callback. We
therefore no longer use the default ->init_irq() callback, but we now
use the default ->init_time() callback.

In this newly introduced ->init_irq() callback, we call irqchip_init()
which is the default behavior when ->init_irq() isn't defined, and
then do the initialization related to the coherency: SCU, coherency
fabric, and mvebu-mbus (which is needed to start secondary CPUs).

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/mach-mvebu/board-v7.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
index 804c952..b2524d6 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -23,6 +23,7 @@
 #include <linux/mbus.h>
 #include <linux/signal.h>
 #include <linux/slab.h>
+#include <linux/irqchip.h>
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
@@ -71,10 +72,9 @@ static int armada_375_external_abort_wa(unsigned long addr, unsigned int fsr,
 	return 1;
 }
 
-static void __init mvebu_timer_and_clk_init(void)
+static void __init mvebu_init_irq(void)
 {
-	of_clk_init(NULL);
-	clocksource_of_init();
+	irqchip_init();
 	mvebu_scu_enable();
 	coherency_init();
 	BUG_ON(mvebu_mbus_dt_init(coherency_available()));
@@ -194,7 +194,7 @@ DT_MACHINE_START(ARMADA_370_XP_DT, "Marvell Armada 370/XP (Device Tree)")
 	.l2c_aux_mask	= ~0,
 	.smp		= smp_ops(armada_xp_smp_ops),
 	.init_machine	= mvebu_dt_init,
-	.init_time	= mvebu_timer_and_clk_init,
+	.init_irq       = mvebu_init_irq,
 	.restart	= mvebu_restart,
 	.dt_compat	= armada_370_xp_dt_compat,
 MACHINE_END
@@ -207,7 +207,7 @@ static const char * const armada_375_dt_compat[] = {
 DT_MACHINE_START(ARMADA_375_DT, "Marvell Armada 375 (Device Tree)")
 	.l2c_aux_val	= 0,
 	.l2c_aux_mask	= ~0,
-	.init_time	= mvebu_timer_and_clk_init,
+	.init_irq       = mvebu_init_irq,
 	.init_machine	= mvebu_dt_init,
 	.restart	= mvebu_restart,
 	.dt_compat	= armada_375_dt_compat,
@@ -222,7 +222,7 @@ static const char * const armada_38x_dt_compat[] = {
 DT_MACHINE_START(ARMADA_38X_DT, "Marvell Armada 380/385 (Device Tree)")
 	.l2c_aux_val	= 0,
 	.l2c_aux_mask	= ~0,
-	.init_time	= mvebu_timer_and_clk_init,
+	.init_irq       = mvebu_init_irq,
 	.restart	= mvebu_restart,
 	.dt_compat	= armada_38x_dt_compat,
 MACHINE_END
-- 
2.0.0

Re: [PATCHv5 1/3] ARM: mm: add support for HW coherent systems in PL310 cache

[Rob Herring]

On Thu, Jun 12, 2014 at 10:09 AM, Thomas Petazzoni
<thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> When a PL310 cache is used on a system that provides hardware
> coherency, the outer cache sync operation is useless, and can be
> skipped. Moreover, on some systems, it is harmful as it causes
> deadlocks between the Marvell coherency mechanism, the Marvell PCIe
> controller and the Cortex-A9.
>
> To avoid this, this commit introduces a new Device Tree property
> 'arm,io-coherent' for the L2 cache controller node, valid only for the
> PL310 cache. It identifies the usage of the PL310 cache in an I/O
> coherent configuration. Internally, it makes the driver disable the
> outer cache sync operation.
>
> Note that technically speaking, a fully coherent system wouldn't
> require any of the other .outer_cache operations. However, in
> practice, when booting secondary CPUs, these are not yet coherent, and
> therefore a set of cache maintenance operations are necessary at this
> point. This explains why we keep the other .outer_cache operations and
> only ->sync is disabled.
>
> While in theory any write to a PL310 register could cause the
> deadlock, in practice, disabling ->sync is sufficient to workaround
> the deadlock, since the other cache maintenance operations are only
> used in very specific situations.
>
> Contrary to previous versions of this patch, this new version does not
> simply NULL-ify the ->sync member, because the l2c_init_data
> structures are now 'const' and therefore cannot be modified, which is
> a good thing. Therefore, this patch introduces a separate
> l2c_init_data instance, called of_l2c310_coherent_data.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> ---
> This patch is based on the latest mainline, as it depends on the L2CC
> cleanup from Russell King.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/arm/l2cc.txt |  3 +++
>  arch/arm/mm/cache-l2x0.c                       | 31 ++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
> index b513cb8..af527ee 100644
> --- a/Documentation/devicetree/bindings/arm/l2cc.txt
> +++ b/Documentation/devicetree/bindings/arm/l2cc.txt
> @@ -40,6 +40,9 @@ Optional properties:
>  - arm,filter-ranges : <start length> Starting address and length of window to
>    filter. Addresses in the filter window are directed to the M1 port. Other
>    addresses will go to the M0 port.
> +- arm,io-coherent : indicates that the system is operating in an hardware
> +  I/O coherent mode. Valid only when the arm,pl310-cache compatible
> +  string is used.
>  - interrupts : 1 combined interrupt.
>  - cache-id-part: cache id part number to be used if it is not present
>    on hardware
> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index efc5cab..076172b 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -1069,6 +1069,33 @@ static const struct l2c_init_data of_l2c310_data __initconst = {
>  };
>
>  /*
> + * This is a variant of the of_l2c310_data with .sync set to
> + * NULL. Outer sync operations are not needed when the system is I/O
> + * coherent, and potentially harmful in certain situations (PCIe/PL310
> + * deadlock on Armada 375/38x due to hardware I/O coherency). The
> + * other operations are kept because they are infrequent (therefore do
> + * not cause the deadlock in practice) and needed for secondary CPU
> + * boot and other power management activities.
> + */
> +static const struct l2c_init_data of_l2c310_coherent_data __initconst = {
> +       .type = "L2C-310 Coherent",
> +       .way_size_0 = SZ_8K,
> +       .num_lock = 8,
> +       .of_parse = l2c310_of_parse,
> +       .enable = l2c310_enable,
> +       .fixup = l2c310_fixup,
> +       .save  = l2c310_save,
> +       .outer_cache = {
> +               .inv_range   = l2c210_inv_range,
> +               .clean_range = l2c210_clean_range,
> +               .flush_range = l2c210_flush_range,
> +               .flush_all   = l2c210_flush_all,
> +               .disable     = l2c310_disable,
> +               .resume      = l2c310_resume,
> +       },
> +};
> +
> +/*
>   * Note that the end addresses passed to Linux primitives are
>   * noninclusive, while the hardware cache range operations use
>   * inclusive start and end addresses.
> @@ -1487,6 +1514,10 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
>
>         data = of_match_node(l2x0_ids, np)->data;
>
> +       if (of_device_is_compatible(np, "arm,pl310-cache") &&
> +           of_property_read_bool(np, "arm,io-coherent"))
> +               data = &of_l2c310_coherent_data;
> +
>         old_aux = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
>         if (old_aux != ((old_aux & aux_mask) | aux_val)) {
>                 pr_warn("L2C: platform modifies aux control register: 0x%08x -> 0x%08x\n",
> --
> 2.0.0
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv5 1/3] ARM: mm: add support for HW coherent systems in PL310 cache

[Rob Herring]

On Thu, Jun 12, 2014 at 10:09 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> When a PL310 cache is used on a system that provides hardware
> coherency, the outer cache sync operation is useless, and can be
> skipped. Moreover, on some systems, it is harmful as it causes
> deadlocks between the Marvell coherency mechanism, the Marvell PCIe
> controller and the Cortex-A9.
>
> To avoid this, this commit introduces a new Device Tree property
> 'arm,io-coherent' for the L2 cache controller node, valid only for the
> PL310 cache. It identifies the usage of the PL310 cache in an I/O
> coherent configuration. Internally, it makes the driver disable the
> outer cache sync operation.
>
> Note that technically speaking, a fully coherent system wouldn't
> require any of the other .outer_cache operations. However, in
> practice, when booting secondary CPUs, these are not yet coherent, and
> therefore a set of cache maintenance operations are necessary at this
> point. This explains why we keep the other .outer_cache operations and
> only ->sync is disabled.
>
> While in theory any write to a PL310 register could cause the
> deadlock, in practice, disabling ->sync is sufficient to workaround
> the deadlock, since the other cache maintenance operations are only
> used in very specific situations.
>
> Contrary to previous versions of this patch, this new version does not
> simply NULL-ify the ->sync member, because the l2c_init_data
> structures are now 'const' and therefore cannot be modified, which is
> a good thing. Therefore, this patch introduces a separate
> l2c_init_data instance, called of_l2c310_coherent_data.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Acked-by: Rob Herring <robh@kernel.org>

> ---
> This patch is based on the latest mainline, as it depends on the L2CC
> cleanup from Russell King.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/arm/l2cc.txt |  3 +++
>  arch/arm/mm/cache-l2x0.c                       | 31 ++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
> index b513cb8..af527ee 100644
> --- a/Documentation/devicetree/bindings/arm/l2cc.txt
> +++ b/Documentation/devicetree/bindings/arm/l2cc.txt
> @@ -40,6 +40,9 @@ Optional properties:
>  - arm,filter-ranges : <start length> Starting address and length of window to
>    filter. Addresses in the filter window are directed to the M1 port. Other
>    addresses will go to the M0 port.
> +- arm,io-coherent : indicates that the system is operating in an hardware
> +  I/O coherent mode. Valid only when the arm,pl310-cache compatible
> +  string is used.
>  - interrupts : 1 combined interrupt.
>  - cache-id-part: cache id part number to be used if it is not present
>    on hardware
> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index efc5cab..076172b 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -1069,6 +1069,33 @@ static const struct l2c_init_data of_l2c310_data __initconst = {
>  };
>
>  /*
> + * This is a variant of the of_l2c310_data with .sync set to
> + * NULL. Outer sync operations are not needed when the system is I/O
> + * coherent, and potentially harmful in certain situations (PCIe/PL310
> + * deadlock on Armada 375/38x due to hardware I/O coherency). The
> + * other operations are kept because they are infrequent (therefore do
> + * not cause the deadlock in practice) and needed for secondary CPU
> + * boot and other power management activities.
> + */
> +static const struct l2c_init_data of_l2c310_coherent_data __initconst = {
> +       .type = "L2C-310 Coherent",
> +       .way_size_0 = SZ_8K,
> +       .num_lock = 8,
> +       .of_parse = l2c310_of_parse,
> +       .enable = l2c310_enable,
> +       .fixup = l2c310_fixup,
> +       .save  = l2c310_save,
> +       .outer_cache = {
> +               .inv_range   = l2c210_inv_range,
> +               .clean_range = l2c210_clean_range,
> +               .flush_range = l2c210_flush_range,
> +               .flush_all   = l2c210_flush_all,
> +               .disable     = l2c310_disable,
> +               .resume      = l2c310_resume,
> +       },
> +};
> +
> +/*
>   * Note that the end addresses passed to Linux primitives are
>   * noninclusive, while the hardware cache range operations use
>   * inclusive start and end addresses.
> @@ -1487,6 +1514,10 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
>
>         data = of_match_node(l2x0_ids, np)->data;
>
> +       if (of_device_is_compatible(np, "arm,pl310-cache") &&
> +           of_property_read_bool(np, "arm,io-coherent"))
> +               data = &of_l2c310_coherent_data;
> +
>         old_aux = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
>         if (old_aux != ((old_aux & aux_mask) | aux_val)) {
>                 pr_warn("L2C: platform modifies aux control register: 0x%08x -> 0x%08x\n",
> --
> 2.0.0
>

Re: [PATCHv5 2/3] ARM: mvebu: move Armada 375 external abort logic as a quirk

[Jason Cooper]

On Thu, Jun 12, 2014 at 05:09:31PM +0200, Thomas Petazzoni wrote:
> In preparation to a small re-organization of the initialization
> sequence in board-v7.c, this commit moves the registration of the
> custom external abort handler on Armada 375 later in the boot
> sequence, and makes it more similar to the other quirks that we
> already have. There is indeed no need to register this abort handler
> particularly early, it simply needs to be registered before switching
> to userspace.
> 
> In addition to this, this commit makes the registration of the custom
> abort handler conditional on Armada 375 Z1, because Armada 375 A0 and
> later iterations are not affected by the issue.
> 
> This commit was tested on both Armada 375 Z1 and Armada 375 A0
> platforms.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  arch/arm/mach-mvebu/board-v7.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)

Patches 2 and 3 applied to mvebu/soc

thx,

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

[PATCHv5 2/3] ARM: mvebu: move Armada 375 external abort logic as a quirk

[Jason Cooper]

On Thu, Jun 12, 2014 at 05:09:31PM +0200, Thomas Petazzoni wrote:
> In preparation to a small re-organization of the initialization
> sequence in board-v7.c, this commit moves the registration of the
> custom external abort handler on Armada 375 later in the boot
> sequence, and makes it more similar to the other quirks that we
> already have. There is indeed no need to register this abort handler
> particularly early, it simply needs to be registered before switching
> to userspace.
> 
> In addition to this, this commit makes the registration of the custom
> abort handler conditional on Armada 375 Z1, because Armada 375 A0 and
> later iterations are not affected by the issue.
> 
> This commit was tested on both Armada 375 Z1 and Armada 375 A0
> platforms.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/arm/mach-mvebu/board-v7.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)

Patches 2 and 3 applied to mvebu/soc

thx,

Jason.

Re: [PATCHv5 1/3] ARM: mm: add support for HW coherent systems in PL310 cache

[Catalin Marinas]

On Thu, Jun 12, 2014 at 04:09:30PM +0100, Thomas Petazzoni wrote:
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -1069,6 +1069,33 @@ static const struct l2c_init_data of_l2c310_data __initconst = {
>  };
>  
>  /*
> + * This is a variant of the of_l2c310_data with .sync set to
> + * NULL. Outer sync operations are not needed when the system is I/O
> + * coherent, and potentially harmful in certain situations (PCIe/PL310
> + * deadlock on Armada 375/38x due to hardware I/O coherency). The
> + * other operations are kept because they are infrequent (therefore do
> + * not cause the deadlock in practice) and needed for secondary CPU
> + * boot and other power management activities.
> + */
> +static const struct l2c_init_data of_l2c310_coherent_data __initconst = {
> +	.type = "L2C-310 Coherent",
> +	.way_size_0 = SZ_8K,
> +	.num_lock = 8,
> +	.of_parse = l2c310_of_parse,
> +	.enable = l2c310_enable,
> +	.fixup = l2c310_fixup,
> +	.save  = l2c310_save,
> +	.outer_cache = {
> +		.inv_range   = l2c210_inv_range,
> +		.clean_range = l2c210_clean_range,
> +		.flush_range = l2c210_flush_range,
> +		.flush_all   = l2c210_flush_all,
> +		.disable     = l2c310_disable,
> +		.resume      = l2c310_resume,
> +	},
> +};
> +
> +/*
>   * Note that the end addresses passed to Linux primitives are
>   * noninclusive, while the hardware cache range operations use
>   * inclusive start and end addresses.
> @@ -1487,6 +1514,10 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
>  
>  	data = of_match_node(l2x0_ids, np)->data;
>  
> +	if (of_device_is_compatible(np, "arm,pl310-cache") &&
> +	    of_property_read_bool(np, "arm,io-coherent"))
> +		data = &of_l2c310_coherent_data;

I don't have a better way without duplicating the l2c_init_data
structure since the fixup function does not take a device_node
pointer. If it did, you could have added the check in l2c310_fixup and
zeroed the sync pointer there.

Anyway, your approach works for me as well:

Acked-by: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv5 1/3] ARM: mm: add support for HW coherent systems in PL310 cache

[Catalin Marinas]

On Thu, Jun 12, 2014 at 04:09:30PM +0100, Thomas Petazzoni wrote:
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -1069,6 +1069,33 @@ static const struct l2c_init_data of_l2c310_data __initconst = {
>  };
>  
>  /*
> + * This is a variant of the of_l2c310_data with .sync set to
> + * NULL. Outer sync operations are not needed when the system is I/O
> + * coherent, and potentially harmful in certain situations (PCIe/PL310
> + * deadlock on Armada 375/38x due to hardware I/O coherency). The
> + * other operations are kept because they are infrequent (therefore do
> + * not cause the deadlock in practice) and needed for secondary CPU
> + * boot and other power management activities.
> + */
> +static const struct l2c_init_data of_l2c310_coherent_data __initconst = {
> +	.type = "L2C-310 Coherent",
> +	.way_size_0 = SZ_8K,
> +	.num_lock = 8,
> +	.of_parse = l2c310_of_parse,
> +	.enable = l2c310_enable,
> +	.fixup = l2c310_fixup,
> +	.save  = l2c310_save,
> +	.outer_cache = {
> +		.inv_range   = l2c210_inv_range,
> +		.clean_range = l2c210_clean_range,
> +		.flush_range = l2c210_flush_range,
> +		.flush_all   = l2c210_flush_all,
> +		.disable     = l2c310_disable,
> +		.resume      = l2c310_resume,
> +	},
> +};
> +
> +/*
>   * Note that the end addresses passed to Linux primitives are
>   * noninclusive, while the hardware cache range operations use
>   * inclusive start and end addresses.
> @@ -1487,6 +1514,10 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
>  
>  	data = of_match_node(l2x0_ids, np)->data;
>  
> +	if (of_device_is_compatible(np, "arm,pl310-cache") &&
> +	    of_property_read_bool(np, "arm,io-coherent"))
> +		data = &of_l2c310_coherent_data;

I don't have a better way without duplicating the l2c_init_data
structure since the fixup function does not take a device_node
pointer. If it did, you could have added the check in l2c310_fixup and
zeroed the sync pointer there.

Anyway, your approach works for me as well:

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

Re: [PATCHv5 1/3] ARM: mm: add support for HW coherent systems in PL310 cache

[Thomas Petazzoni]

Dear Catalin Marinas,

On Mon, 30 Jun 2014 18:32:17 +0100, Catalin Marinas wrote:

> > +/*
> >   * Note that the end addresses passed to Linux primitives are
> >   * noninclusive, while the hardware cache range operations use
> >   * inclusive start and end addresses.
> > @@ -1487,6 +1514,10 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
> >  
> >  	data = of_match_node(l2x0_ids, np)->data;
> >  
> > +	if (of_device_is_compatible(np, "arm,pl310-cache") &&
> > +	    of_property_read_bool(np, "arm,io-coherent"))
> > +		data = &of_l2c310_coherent_data;
> 
> I don't have a better way without duplicating the l2c_init_data
> structure since the fixup function does not take a device_node
> pointer. If it did, you could have added the check in l2c310_fixup and
> zeroed the sync pointer there.
> 
> Anyway, your approach works for me as well:
> 
> Acked-by: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>

Thanks for the confirmation. Note that it comes a bit too late though:
the patch is already in 3.16-rc3:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/arch/arm/mm/cache-l2x0.c?id=98ea2dba65932ffc456b6d7b11b8a0624e2f7b95.

However, I'm interested in hearing your opinion about the I/O coherency
discussion in !SMP, and especially whether the TTB flags need to be
consistent with the PMD flags in terms of cache policy and
shareability. See
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/263524.html.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv5 1/3] ARM: mm: add support for HW coherent systems in PL310 cache

[Thomas Petazzoni]

Dear Catalin Marinas,

On Mon, 30 Jun 2014 18:32:17 +0100, Catalin Marinas wrote:

> > +/*
> >   * Note that the end addresses passed to Linux primitives are
> >   * noninclusive, while the hardware cache range operations use
> >   * inclusive start and end addresses.
> > @@ -1487,6 +1514,10 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
> >  
> >  	data = of_match_node(l2x0_ids, np)->data;
> >  
> > +	if (of_device_is_compatible(np, "arm,pl310-cache") &&
> > +	    of_property_read_bool(np, "arm,io-coherent"))
> > +		data = &of_l2c310_coherent_data;
> 
> I don't have a better way without duplicating the l2c_init_data
> structure since the fixup function does not take a device_node
> pointer. If it did, you could have added the check in l2c310_fixup and
> zeroed the sync pointer there.
> 
> Anyway, your approach works for me as well:
> 
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks for the confirmation. Note that it comes a bit too late though:
the patch is already in 3.16-rc3:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/arch/arm/mm/cache-l2x0.c?id=98ea2dba65932ffc456b6d7b11b8a0624e2f7b95.

However, I'm interested in hearing your opinion about the I/O coherency
discussion in !SMP, and especially whether the TTB flags need to be
consistent with the PMD flags in terms of cache policy and
shareability. See
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/263524.html.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com