* [PATCH v3 0/7] EDAC drivers for Armada XP L2 and DDR
@ 2017-11-10 9:03 Jan Luebbe
0 siblings, 0 replies; 35+ messages in thread
From: Jan Luebbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: linux-arm-kernel
Hi Boris,
finally I've found some time to address your feedback and resend this. On the
previous version, Russell said that he would merge the entire series once the
EDAC bits have been reviewed.
This series adds drivers for the L2 cache and DDR RAM ECC functionality as
found on the MV78230/MV78x60 SoCs. I've tested these changes with the MV78460
(on a custom board with a DDR3 ECC DIMM).
Also contained in this series is an additional debugfs wrapper.
Compared to the previous v2 series, the following changes have been made:
- Allocate EDAC structures later during probing and drop devres support
patches (requested by Boris)
- Make drvdata->width usage consistent according to the comment (suggested by
Chris)
Compared to the previous v1 series, the following changes have been made:
- Add the aurora-l2 register defines earlier in the series (suggested by
Russell King and Gregory CLEMENT )
- Changed the DT vendor prefix from "arm" to "marvell" for the ecc-enable/disable
properties on the aurora-l2 (suggested by Russell King)
- Fix some warnings reported by checkpatch
Compared to the original RFC series, the following changes have been made:
- Integrated Chris' patches for parity and ECC configuration via DT
- Merged the DDR RAM and L2 cache drivers (as requested by Boris, analogous
to fsl_ddr_edac.c and mpc85xx_edac.c)
- Added myself to MAINTAINERS (requested by Boris)
- L2 cache: Track the msg size and use snprintf (review comment by Chris)
- L2 cache: Split error injection from the check function (review comment by
Chris)
- DDR RAM: Allow 16 bit width in addition to 32 and 64 bit (review comment by
Chris)
- Use of_match_ptr() (review comments by Chris)
- Minor checkpatch cleanups
Chris Packham (2):
ARM: l2x0: support parity-enable/disable on aurora
ARM: l2x0: add marvell,ecc-enable property for aurora
Jan Luebbe (5):
ARM: l2c: move cache-aurora-l2.h to asm/hardware
ARM: aurora-l2: add prefix to MAX_RANGE_SIZE
ARM: aurora-l2: add defines for parity and ECC registers
EDAC: Add missing debugfs_create_x32 wrapper
EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
Documentation/devicetree/bindings/arm/l2c2x0.txt | 2 +
MAINTAINERS | 6 +
arch/arm/include/asm/hardware/cache-aurora-l2.h | 103 ++++
arch/arm/mm/cache-aurora-l2.h | 55 --
arch/arm/mm/cache-l2x0.c | 20 +-
drivers/edac/Kconfig | 7 +
drivers/edac/Makefile | 1 +
drivers/edac/armada_xp_edac.c | 658 +++++++++++++++++++++++
drivers/edac/debugfs.c | 11 +
drivers/edac/edac_module.h | 5 +
10 files changed, 810 insertions(+), 58 deletions(-)
create mode 100644 arch/arm/include/asm/hardware/cache-aurora-l2.h
delete mode 100644 arch/arm/mm/cache-aurora-l2.h
create mode 100644 drivers/edac/armada_xp_edac.c
--
2.11.0
^ permalink raw reply [flat|nested] 35+ messages in thread
* [v3,1/7] ARM: l2c: move cache-aurora-l2.h to asm/hardware
2017-11-10 9:03 [PATCH v3 0/7] EDAC drivers for Armada XP L2 and DDR Jan Luebbe
@ 2017-11-10 9:03 ` Jan Luebbe
-1 siblings, 0 replies; 35+ messages in thread
From: Jan Lübbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: Russell King - ARM Linux, Borislav Petkov, Chris Packham,
linux-arm-kernel, linux-edac
Cc: Jan Luebbe, kernel, Thomas Petazzoni, Gregory CLEMENT
This include file will be used by the AURORA EDAC code.
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
arch/arm/{mm => include/asm/hardware}/cache-aurora-l2.h | 0
arch/arm/mm/cache-l2x0.c | 2 +-
2 files changed, 1 insertion(+), 1 deletion(-)
rename arch/arm/{mm => include/asm/hardware}/cache-aurora-l2.h (100%)
diff --git a/arch/arm/mm/cache-aurora-l2.h b/arch/arm/include/asm/hardware/cache-aurora-l2.h
similarity index 100%
rename from arch/arm/mm/cache-aurora-l2.h
rename to arch/arm/include/asm/hardware/cache-aurora-l2.h
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 808efbb89b88..a00d6f7fd34c 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -30,8 +30,8 @@
#include <asm/cp15.h>
#include <asm/cputype.h>
#include <asm/hardware/cache-l2x0.h>
+#include <asm/hardware/cache-aurora-l2.h>
#include "cache-tauros3.h"
-#include "cache-aurora-l2.h"
struct l2c_init_data {
const char *type;
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 1/7] ARM: l2c: move cache-aurora-l2.h to asm/hardware
@ 2017-11-10 9:03 ` Jan Luebbe
0 siblings, 0 replies; 35+ messages in thread
From: Jan Luebbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: linux-arm-kernel
This include file will be used by the AURORA EDAC code.
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
arch/arm/{mm => include/asm/hardware}/cache-aurora-l2.h | 0
arch/arm/mm/cache-l2x0.c | 2 +-
2 files changed, 1 insertion(+), 1 deletion(-)
rename arch/arm/{mm => include/asm/hardware}/cache-aurora-l2.h (100%)
diff --git a/arch/arm/mm/cache-aurora-l2.h b/arch/arm/include/asm/hardware/cache-aurora-l2.h
similarity index 100%
rename from arch/arm/mm/cache-aurora-l2.h
rename to arch/arm/include/asm/hardware/cache-aurora-l2.h
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 808efbb89b88..a00d6f7fd34c 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -30,8 +30,8 @@
#include <asm/cp15.h>
#include <asm/cputype.h>
#include <asm/hardware/cache-l2x0.h>
+#include <asm/hardware/cache-aurora-l2.h>
#include "cache-tauros3.h"
-#include "cache-aurora-l2.h"
struct l2c_init_data {
const char *type;
--
2.11.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [v3,2/7] ARM: aurora-l2: add prefix to MAX_RANGE_SIZE
2017-11-10 9:03 [PATCH v3 0/7] EDAC drivers for Armada XP L2 and DDR Jan Luebbe
@ 2017-11-10 9:03 ` Jan Luebbe
-1 siblings, 0 replies; 35+ messages in thread
From: Jan Lübbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: Russell King - ARM Linux, Borislav Petkov, Chris Packham,
linux-arm-kernel, linux-edac
Cc: Jan Luebbe, kernel, Thomas Petazzoni, Gregory CLEMENT
The macro name is too generic, so add a AURORA_ prefix.
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
arch/arm/include/asm/hardware/cache-aurora-l2.h | 2 +-
arch/arm/mm/cache-l2x0.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/hardware/cache-aurora-l2.h b/arch/arm/include/asm/hardware/cache-aurora-l2.h
index c86124769831..dc5c479ec4c3 100644
--- a/arch/arm/include/asm/hardware/cache-aurora-l2.h
+++ b/arch/arm/include/asm/hardware/cache-aurora-l2.h
@@ -41,7 +41,7 @@
#define AURORA_ACR_FORCE_WRITE_THRO_POLICY \
(2 << AURORA_ACR_FORCE_WRITE_POLICY_OFFSET)
-#define MAX_RANGE_SIZE 1024
+#define AURORA_MAX_RANGE_SIZE 1024
#define AURORA_WAY_SIZE_SHIFT 2
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index a00d6f7fd34c..7d2d2a3c67d0 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1364,8 +1364,8 @@ static unsigned long aurora_range_end(unsigned long start, unsigned long end)
* since cache range operations stall the CPU pipeline
* until completion.
*/
- if (end > start + MAX_RANGE_SIZE)
- end = start + MAX_RANGE_SIZE;
+ if (end > start + AURORA_MAX_RANGE_SIZE)
+ end = start + AURORA_MAX_RANGE_SIZE;
/*
* Cache range operations can't straddle a page boundary.
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 2/7] ARM: aurora-l2: add prefix to MAX_RANGE_SIZE
@ 2017-11-10 9:03 ` Jan Luebbe
0 siblings, 0 replies; 35+ messages in thread
From: Jan Luebbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: linux-arm-kernel
The macro name is too generic, so add a AURORA_ prefix.
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
arch/arm/include/asm/hardware/cache-aurora-l2.h | 2 +-
arch/arm/mm/cache-l2x0.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/hardware/cache-aurora-l2.h b/arch/arm/include/asm/hardware/cache-aurora-l2.h
index c86124769831..dc5c479ec4c3 100644
--- a/arch/arm/include/asm/hardware/cache-aurora-l2.h
+++ b/arch/arm/include/asm/hardware/cache-aurora-l2.h
@@ -41,7 +41,7 @@
#define AURORA_ACR_FORCE_WRITE_THRO_POLICY \
(2 << AURORA_ACR_FORCE_WRITE_POLICY_OFFSET)
-#define MAX_RANGE_SIZE 1024
+#define AURORA_MAX_RANGE_SIZE 1024
#define AURORA_WAY_SIZE_SHIFT 2
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index a00d6f7fd34c..7d2d2a3c67d0 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1364,8 +1364,8 @@ static unsigned long aurora_range_end(unsigned long start, unsigned long end)
* since cache range operations stall the CPU pipeline
* until completion.
*/
- if (end > start + MAX_RANGE_SIZE)
- end = start + MAX_RANGE_SIZE;
+ if (end > start + AURORA_MAX_RANGE_SIZE)
+ end = start + AURORA_MAX_RANGE_SIZE;
/*
* Cache range operations can't straddle a page boundary.
--
2.11.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [v3,3/7] ARM: aurora-l2: add defines for parity and ECC registers
2017-11-10 9:03 [PATCH v3 0/7] EDAC drivers for Armada XP L2 and DDR Jan Luebbe
@ 2017-11-10 9:03 ` Jan Luebbe
-1 siblings, 0 replies; 35+ messages in thread
From: Jan Lübbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: Russell King - ARM Linux, Borislav Petkov, Chris Packham,
linux-arm-kernel, linux-edac
Cc: Jan Luebbe, kernel, Thomas Petazzoni, Gregory CLEMENT
These defines will be used by subsequent patches to add support for the
parity check and error correction functionality in the Aurora L2 cache
controller.
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
arch/arm/include/asm/hardware/cache-aurora-l2.h | 48 +++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/arch/arm/include/asm/hardware/cache-aurora-l2.h b/arch/arm/include/asm/hardware/cache-aurora-l2.h
index dc5c479ec4c3..c32be689d1f6 100644
--- a/arch/arm/include/asm/hardware/cache-aurora-l2.h
+++ b/arch/arm/include/asm/hardware/cache-aurora-l2.h
@@ -31,6 +31,9 @@
#define AURORA_ACR_REPLACEMENT_TYPE_SEMIPLRU \
(3 << AURORA_ACR_REPLACEMENT_OFFSET)
+#define AURORA_ACR_PARITY_EN (1 << 21)
+#define AURORA_ACR_ECC_EN (1 << 20)
+
#define AURORA_ACR_FORCE_WRITE_POLICY_OFFSET 0
#define AURORA_ACR_FORCE_WRITE_POLICY_MASK \
(0x3 << AURORA_ACR_FORCE_WRITE_POLICY_OFFSET)
@@ -41,6 +44,51 @@
#define AURORA_ACR_FORCE_WRITE_THRO_POLICY \
(2 << AURORA_ACR_FORCE_WRITE_POLICY_OFFSET)
+#define AURORA_ERR_CNT_REG 0x600
+#define AURORA_ERR_ATTR_CAP_REG 0x608
+#define AURORA_ERR_ADDR_CAP_REG 0x60c
+#define AURORA_ERR_WAY_CAP_REG 0x610
+#define AURORA_ERR_INJECT_CTL_REG 0x614
+#define AURORA_ERR_INJECT_MASK_REG 0x618
+
+#define AURORA_ERR_CNT_CLR_OFFSET 31
+#define AURORA_ERR_CNT_CLR \
+ (0x1 << AURORA_ERR_CNT_CLR_OFFSET)
+#define AURORA_ERR_CNT_UE_OFFSET 16
+#define AURORA_ERR_CNT_UE_MASK \
+ (0x7fff << AURORA_ERR_CNT_UE_OFFSET)
+#define AURORA_ERR_CNT_CE_OFFSET 0
+#define AURORA_ERR_CNT_CE_MASK \
+ (0xffff << AURORA_ERR_CNT_CE_OFFSET)
+
+#define AURORA_ERR_ATTR_CAP_ERR_SOURCE_OFFSET 16
+#define AURORA_ERR_ATTR_CAP_ERR_SOURCE_MASK \
+ (0x7 << AURORA_ERR_ATTR_CAP_ERR_SOURCE_OFFSET)
+#define AURORA_ERR_ATTR_CAP_TRANS_TYPE_OFFSET 12
+#define AURORA_ERR_ATTR_CAP_TRANS_TYPE_MASK \
+ (0xf << AURORA_ERR_ATTR_CAP_TRANS_TYPE_OFFSET)
+#define AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET 8
+#define AURORA_ERR_ATTR_CAP_ERR_TYPE_MASK \
+ (0x3 << AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET)
+#define AURORA_ERR_ATTR_CAP_VALID_OFFSET 0
+#define AURORA_ERR_ATTR_CAP_VALID \
+ (0x1 << AURORA_ERR_ATTR_CAP_VALID_OFFSET)
+
+#define AURORA_ERR_ADDR_CAP_ADDR_MASK 0xffffffe0
+
+#define AURORA_ERR_WAY_CAP_INDEX_OFFSET 8
+#define AURORA_ERR_WAY_CAP_INDEX_MASK \
+ (0xfff << AURORA_ERR_WAY_CAP_INDEX_OFFSET)
+#define AURORA_ERR_WAY_CAP_WAY_OFFSET 1
+#define AURORA_ERR_WAY_CAP_WAY_MASK \
+ (0xf << AURORA_ERR_WAY_CAP_WAY_OFFSET)
+
+#define AURORA_ERR_INJECT_CTL_ADDR_MASK 0xfffffff0
+#define AURORA_ERR_ATTR_CAP_TRANS_TYPE_OFFSET 12
+#define AURORA_ERR_INJECT_CTL_EN_MASK 0x3
+#define AURORA_ERR_INJECT_CTL_EN_PARITY 0x2
+#define AURORA_ERR_INJECT_CTL_EN_ECC 0x1
+
#define AURORA_MAX_RANGE_SIZE 1024
#define AURORA_WAY_SIZE_SHIFT 2
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 3/7] ARM: aurora-l2: add defines for parity and ECC registers
@ 2017-11-10 9:03 ` Jan Luebbe
0 siblings, 0 replies; 35+ messages in thread
From: Jan Luebbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: linux-arm-kernel
These defines will be used by subsequent patches to add support for the
parity check and error correction functionality in the Aurora L2 cache
controller.
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
arch/arm/include/asm/hardware/cache-aurora-l2.h | 48 +++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/arch/arm/include/asm/hardware/cache-aurora-l2.h b/arch/arm/include/asm/hardware/cache-aurora-l2.h
index dc5c479ec4c3..c32be689d1f6 100644
--- a/arch/arm/include/asm/hardware/cache-aurora-l2.h
+++ b/arch/arm/include/asm/hardware/cache-aurora-l2.h
@@ -31,6 +31,9 @@
#define AURORA_ACR_REPLACEMENT_TYPE_SEMIPLRU \
(3 << AURORA_ACR_REPLACEMENT_OFFSET)
+#define AURORA_ACR_PARITY_EN (1 << 21)
+#define AURORA_ACR_ECC_EN (1 << 20)
+
#define AURORA_ACR_FORCE_WRITE_POLICY_OFFSET 0
#define AURORA_ACR_FORCE_WRITE_POLICY_MASK \
(0x3 << AURORA_ACR_FORCE_WRITE_POLICY_OFFSET)
@@ -41,6 +44,51 @@
#define AURORA_ACR_FORCE_WRITE_THRO_POLICY \
(2 << AURORA_ACR_FORCE_WRITE_POLICY_OFFSET)
+#define AURORA_ERR_CNT_REG 0x600
+#define AURORA_ERR_ATTR_CAP_REG 0x608
+#define AURORA_ERR_ADDR_CAP_REG 0x60c
+#define AURORA_ERR_WAY_CAP_REG 0x610
+#define AURORA_ERR_INJECT_CTL_REG 0x614
+#define AURORA_ERR_INJECT_MASK_REG 0x618
+
+#define AURORA_ERR_CNT_CLR_OFFSET 31
+#define AURORA_ERR_CNT_CLR \
+ (0x1 << AURORA_ERR_CNT_CLR_OFFSET)
+#define AURORA_ERR_CNT_UE_OFFSET 16
+#define AURORA_ERR_CNT_UE_MASK \
+ (0x7fff << AURORA_ERR_CNT_UE_OFFSET)
+#define AURORA_ERR_CNT_CE_OFFSET 0
+#define AURORA_ERR_CNT_CE_MASK \
+ (0xffff << AURORA_ERR_CNT_CE_OFFSET)
+
+#define AURORA_ERR_ATTR_CAP_ERR_SOURCE_OFFSET 16
+#define AURORA_ERR_ATTR_CAP_ERR_SOURCE_MASK \
+ (0x7 << AURORA_ERR_ATTR_CAP_ERR_SOURCE_OFFSET)
+#define AURORA_ERR_ATTR_CAP_TRANS_TYPE_OFFSET 12
+#define AURORA_ERR_ATTR_CAP_TRANS_TYPE_MASK \
+ (0xf << AURORA_ERR_ATTR_CAP_TRANS_TYPE_OFFSET)
+#define AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET 8
+#define AURORA_ERR_ATTR_CAP_ERR_TYPE_MASK \
+ (0x3 << AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET)
+#define AURORA_ERR_ATTR_CAP_VALID_OFFSET 0
+#define AURORA_ERR_ATTR_CAP_VALID \
+ (0x1 << AURORA_ERR_ATTR_CAP_VALID_OFFSET)
+
+#define AURORA_ERR_ADDR_CAP_ADDR_MASK 0xffffffe0
+
+#define AURORA_ERR_WAY_CAP_INDEX_OFFSET 8
+#define AURORA_ERR_WAY_CAP_INDEX_MASK \
+ (0xfff << AURORA_ERR_WAY_CAP_INDEX_OFFSET)
+#define AURORA_ERR_WAY_CAP_WAY_OFFSET 1
+#define AURORA_ERR_WAY_CAP_WAY_MASK \
+ (0xf << AURORA_ERR_WAY_CAP_WAY_OFFSET)
+
+#define AURORA_ERR_INJECT_CTL_ADDR_MASK 0xfffffff0
+#define AURORA_ERR_ATTR_CAP_TRANS_TYPE_OFFSET 12
+#define AURORA_ERR_INJECT_CTL_EN_MASK 0x3
+#define AURORA_ERR_INJECT_CTL_EN_PARITY 0x2
+#define AURORA_ERR_INJECT_CTL_EN_ECC 0x1
+
#define AURORA_MAX_RANGE_SIZE 1024
#define AURORA_WAY_SIZE_SHIFT 2
--
2.11.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [v3,4/7] ARM: l2x0: support parity-enable/disable on aurora
2017-11-10 9:03 [PATCH v3 0/7] EDAC drivers for Armada XP L2 and DDR Jan Luebbe
@ 2017-11-10 9:03 ` Jan Luebbe
-1 siblings, 0 replies; 35+ messages in thread
From: Jan Lübbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: Russell King - ARM Linux, Borislav Petkov, Chris Packham,
linux-arm-kernel, linux-edac
Cc: kernel, Thomas Petazzoni, Gregory CLEMENT, Jan Luebbe
From: Chris Packham <chris.packham@alliedtelesis.co.nz>
The aurora cache on the Marvell Armada-XP SoC supports the same tag
parity features as the other l2x0 cache implementations.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
[jlu@pengutronix.de: use aurora specific define AURORA_ACR_PARITY_EN]
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
arch/arm/mm/cache-l2x0.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 7d2d2a3c67d0..b70bee74750d 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1505,6 +1505,13 @@ static void __init aurora_of_parse(const struct device_node *np,
mask |= AURORA_ACR_FORCE_WRITE_POLICY_MASK;
}
+ if (of_property_read_bool(np, "arm,parity-enable")) {
+ mask |= AURORA_ACR_PARITY_EN;
+ val |= AURORA_ACR_PARITY_EN;
+ } else if (of_property_read_bool(np, "arm,parity-disable")) {
+ mask |= AURORA_ACR_PARITY_EN;
+ }
+
*aux_val &= ~mask;
*aux_val |= val;
*aux_mask &= ~mask;
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 4/7] ARM: l2x0: support parity-enable/disable on aurora
@ 2017-11-10 9:03 ` Jan Luebbe
0 siblings, 0 replies; 35+ messages in thread
From: Jan Luebbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: linux-arm-kernel
From: Chris Packham <chris.packham@alliedtelesis.co.nz>
The aurora cache on the Marvell Armada-XP SoC supports the same tag
parity features as the other l2x0 cache implementations.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
[jlu at pengutronix.de: use aurora specific define AURORA_ACR_PARITY_EN]
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
arch/arm/mm/cache-l2x0.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 7d2d2a3c67d0..b70bee74750d 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1505,6 +1505,13 @@ static void __init aurora_of_parse(const struct device_node *np,
mask |= AURORA_ACR_FORCE_WRITE_POLICY_MASK;
}
+ if (of_property_read_bool(np, "arm,parity-enable")) {
+ mask |= AURORA_ACR_PARITY_EN;
+ val |= AURORA_ACR_PARITY_EN;
+ } else if (of_property_read_bool(np, "arm,parity-disable")) {
+ mask |= AURORA_ACR_PARITY_EN;
+ }
+
*aux_val &= ~mask;
*aux_val |= val;
*aux_mask &= ~mask;
--
2.11.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [v3,5/7] ARM: l2x0: add marvell,ecc-enable property for aurora
2017-11-10 9:03 [PATCH v3 0/7] EDAC drivers for Armada XP L2 and DDR Jan Luebbe
@ 2017-11-10 9:03 ` Jan Luebbe
-1 siblings, 0 replies; 35+ messages in thread
From: Jan Lübbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: Russell King - ARM Linux, Borislav Petkov, Chris Packham,
linux-arm-kernel, linux-edac
Cc: kernel, Thomas Petazzoni, Gregory CLEMENT, Jan Luebbe
From: Chris Packham <chris.packham@alliedtelesis.co.nz>
The aurora cache on the Marvell Armada-XP SoC supports ECC protection
for the L2 data arrays. Add a "marvell,ecc-enable" device tree property
which can be used to enable this.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
[jlu@pengutronix.de: use aurora specific define AURORA_ACR_ECC_EN]
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
Documentation/devicetree/bindings/arm/l2c2x0.txt | 2 ++
arch/arm/mm/cache-l2x0.c | 7 +++++++
2 files changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/l2c2x0.txt b/Documentation/devicetree/bindings/arm/l2c2x0.txt
index fbe6cb21f4cf..15a84f0ba9f1 100644
--- a/Documentation/devicetree/bindings/arm/l2c2x0.txt
+++ b/Documentation/devicetree/bindings/arm/l2c2x0.txt
@@ -76,6 +76,8 @@ 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).
+- marvell,ecc-enable : enable ECC protection on the L2 cache
+- marvell,ecc-disable : disable ECC protection on the L2 cache
- arm,outer-sync-disable : disable the outer sync operation on the L2 cache.
Some core tiles, especially ARM PB11MPCore have a faulty L220 cache that
will randomly hang unless outer sync operations are disabled.
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index b70bee74750d..644f786e4fa9 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1505,6 +1505,13 @@ static void __init aurora_of_parse(const struct device_node *np,
mask |= AURORA_ACR_FORCE_WRITE_POLICY_MASK;
}
+ if (of_property_read_bool(np, "marvell,ecc-enable")) {
+ mask |= AURORA_ACR_ECC_EN;
+ val |= AURORA_ACR_ECC_EN;
+ } else if (of_property_read_bool(np, "marvell,ecc-disable")) {
+ mask |= AURORA_ACR_ECC_EN;
+ }
+
if (of_property_read_bool(np, "arm,parity-enable")) {
mask |= AURORA_ACR_PARITY_EN;
val |= AURORA_ACR_PARITY_EN;
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 5/7] ARM: l2x0: add marvell,ecc-enable property for aurora
@ 2017-11-10 9:03 ` Jan Luebbe
0 siblings, 0 replies; 35+ messages in thread
From: Jan Luebbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: linux-arm-kernel
From: Chris Packham <chris.packham@alliedtelesis.co.nz>
The aurora cache on the Marvell Armada-XP SoC supports ECC protection
for the L2 data arrays. Add a "marvell,ecc-enable" device tree property
which can be used to enable this.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
[jlu at pengutronix.de: use aurora specific define AURORA_ACR_ECC_EN]
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
Documentation/devicetree/bindings/arm/l2c2x0.txt | 2 ++
arch/arm/mm/cache-l2x0.c | 7 +++++++
2 files changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/l2c2x0.txt b/Documentation/devicetree/bindings/arm/l2c2x0.txt
index fbe6cb21f4cf..15a84f0ba9f1 100644
--- a/Documentation/devicetree/bindings/arm/l2c2x0.txt
+++ b/Documentation/devicetree/bindings/arm/l2c2x0.txt
@@ -76,6 +76,8 @@ 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).
+- marvell,ecc-enable : enable ECC protection on the L2 cache
+- marvell,ecc-disable : disable ECC protection on the L2 cache
- arm,outer-sync-disable : disable the outer sync operation on the L2 cache.
Some core tiles, especially ARM PB11MPCore have a faulty L220 cache that
will randomly hang unless outer sync operations are disabled.
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index b70bee74750d..644f786e4fa9 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1505,6 +1505,13 @@ static void __init aurora_of_parse(const struct device_node *np,
mask |= AURORA_ACR_FORCE_WRITE_POLICY_MASK;
}
+ if (of_property_read_bool(np, "marvell,ecc-enable")) {
+ mask |= AURORA_ACR_ECC_EN;
+ val |= AURORA_ACR_ECC_EN;
+ } else if (of_property_read_bool(np, "marvell,ecc-disable")) {
+ mask |= AURORA_ACR_ECC_EN;
+ }
+
if (of_property_read_bool(np, "arm,parity-enable")) {
mask |= AURORA_ACR_PARITY_EN;
val |= AURORA_ACR_PARITY_EN;
--
2.11.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [v3,6/7] EDAC: Add missing debugfs_create_x32 wrapper
2017-11-10 9:03 [PATCH v3 0/7] EDAC drivers for Armada XP L2 and DDR Jan Luebbe
@ 2017-11-10 9:03 ` Jan Luebbe
-1 siblings, 0 replies; 35+ messages in thread
From: Jan Lübbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: Russell King - ARM Linux, Borislav Petkov, Chris Packham,
linux-arm-kernel, linux-edac
Cc: Jan Luebbe, kernel, Thomas Petazzoni, Gregory CLEMENT
We already have wrappers for x8 and x16, so add the missing x32 one.
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
drivers/edac/debugfs.c | 11 +++++++++++
drivers/edac/edac_module.h | 5 +++++
2 files changed, 16 insertions(+)
diff --git a/drivers/edac/debugfs.c b/drivers/edac/debugfs.c
index 92dbb7e2320c..268ede7a60b2 100644
--- a/drivers/edac/debugfs.c
+++ b/drivers/edac/debugfs.c
@@ -161,3 +161,14 @@ struct dentry *edac_debugfs_create_x16(const char *name, umode_t mode,
return debugfs_create_x16(name, mode, parent, value);
}
EXPORT_SYMBOL_GPL(edac_debugfs_create_x16);
+
+/* Wrapper for debugfs_create_x32() */
+struct dentry *edac_debugfs_create_x32(const char *name, umode_t mode,
+ struct dentry *parent, u32 *value)
+{
+ if (!parent)
+ parent = edac_debugfs;
+
+ return debugfs_create_x32(name, mode, parent, value);
+}
+EXPORT_SYMBOL_GPL(edac_debugfs_create_x32);
diff --git a/drivers/edac/edac_module.h b/drivers/edac/edac_module.h
index dec88dcea036..546b16e29221 100644
--- a/drivers/edac/edac_module.h
+++ b/drivers/edac/edac_module.h
@@ -82,6 +82,8 @@ struct dentry *
edac_debugfs_create_x8(const char *name, umode_t mode, struct dentry *parent, u8 *value);
struct dentry *
edac_debugfs_create_x16(const char *name, umode_t mode, struct dentry *parent, u16 *value);
+struct dentry *
+edac_debugfs_create_x32(const char *name, umode_t mode, struct dentry *parent, u32 *value);
#else
static inline int edac_debugfs_init(void) { return -ENODEV; }
static inline void edac_debugfs_exit(void) { }
@@ -98,6 +100,9 @@ edac_debugfs_create_x8(const char *name, umode_t mode,
static inline struct dentry *
edac_debugfs_create_x16(const char *name, umode_t mode,
struct dentry *parent, u16 *value) { return NULL; }
+static inline struct dentry *
+edac_debugfs_create_x32(const char *name, umode_t mode,
+ struct dentry *parent, u32 *value) { return NULL; }
#endif
/*
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 6/7] EDAC: Add missing debugfs_create_x32 wrapper
@ 2017-11-10 9:03 ` Jan Luebbe
0 siblings, 0 replies; 35+ messages in thread
From: Jan Luebbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: linux-arm-kernel
We already have wrappers for x8 and x16, so add the missing x32 one.
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
drivers/edac/debugfs.c | 11 +++++++++++
drivers/edac/edac_module.h | 5 +++++
2 files changed, 16 insertions(+)
diff --git a/drivers/edac/debugfs.c b/drivers/edac/debugfs.c
index 92dbb7e2320c..268ede7a60b2 100644
--- a/drivers/edac/debugfs.c
+++ b/drivers/edac/debugfs.c
@@ -161,3 +161,14 @@ struct dentry *edac_debugfs_create_x16(const char *name, umode_t mode,
return debugfs_create_x16(name, mode, parent, value);
}
EXPORT_SYMBOL_GPL(edac_debugfs_create_x16);
+
+/* Wrapper for debugfs_create_x32() */
+struct dentry *edac_debugfs_create_x32(const char *name, umode_t mode,
+ struct dentry *parent, u32 *value)
+{
+ if (!parent)
+ parent = edac_debugfs;
+
+ return debugfs_create_x32(name, mode, parent, value);
+}
+EXPORT_SYMBOL_GPL(edac_debugfs_create_x32);
diff --git a/drivers/edac/edac_module.h b/drivers/edac/edac_module.h
index dec88dcea036..546b16e29221 100644
--- a/drivers/edac/edac_module.h
+++ b/drivers/edac/edac_module.h
@@ -82,6 +82,8 @@ struct dentry *
edac_debugfs_create_x8(const char *name, umode_t mode, struct dentry *parent, u8 *value);
struct dentry *
edac_debugfs_create_x16(const char *name, umode_t mode, struct dentry *parent, u16 *value);
+struct dentry *
+edac_debugfs_create_x32(const char *name, umode_t mode, struct dentry *parent, u32 *value);
#else
static inline int edac_debugfs_init(void) { return -ENODEV; }
static inline void edac_debugfs_exit(void) { }
@@ -98,6 +100,9 @@ edac_debugfs_create_x8(const char *name, umode_t mode,
static inline struct dentry *
edac_debugfs_create_x16(const char *name, umode_t mode,
struct dentry *parent, u16 *value) { return NULL; }
+static inline struct dentry *
+edac_debugfs_create_x32(const char *name, umode_t mode,
+ struct dentry *parent, u32 *value) { return NULL; }
#endif
/*
--
2.11.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [v3,7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
2017-11-10 9:03 [PATCH v3 0/7] EDAC drivers for Armada XP L2 and DDR Jan Luebbe
@ 2017-11-10 9:03 ` Jan Luebbe
-1 siblings, 0 replies; 35+ messages in thread
From: Jan Lübbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: Russell King - ARM Linux, Borislav Petkov, Chris Packham,
linux-arm-kernel, linux-edac
Cc: Jan Luebbe, kernel, Thomas Petazzoni, Gregory CLEMENT
Add support for the ECC functionality as found in the DDR RAM and L2
cache controllers on the MV78230/MV78x60 SoCs. This driver has been
tested on the MV78460 (on a custom board with a DDR3 ECC DIMM).
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
MAINTAINERS | 6 +
drivers/edac/Kconfig | 7 +
drivers/edac/Makefile | 1 +
drivers/edac/armada_xp_edac.c | 658 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 672 insertions(+)
create mode 100644 drivers/edac/armada_xp_edac.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2f4e462aa4a2..e8c4f7518f11 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4900,6 +4900,12 @@ L: linux-edac@vger.kernel.org
S: Maintained
F: drivers/edac/amd64_edac*
+EDAC-ARMADA
+M: Jan Luebbe <jlu@pengutronix.de>
+L: linux-edac@vger.kernel.org
+S: Maintained
+F: drivers/edac/armada_xp_*
+
EDAC-CALXEDA
M: Robert Richter <rric@kernel.org>
L: linux-edac@vger.kernel.org
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 96afb2aeed18..4e123ec94a4a 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -443,6 +443,13 @@ config EDAC_ALTERA_SDMMC
Support for error detection and correction on the
Altera SDMMC FIFO Memory for Altera SoCs.
+config EDAC_ARMADA_XP
+ bool "Marvell Armada XP DDR and L2 Cache ECC"
+ depends on ARCH_MVEBU
+ help
+ Support for error correction and detection on the Marvell Aramada XP
+ DDR RAM and L2 cache controllers.
+
config EDAC_SYNOPSYS
tristate "Synopsys DDR Memory Controller"
depends on ARCH_ZYNQ
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 0fd9ffa63299..70a093cc976e 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -76,5 +76,6 @@ obj-$(CONFIG_EDAC_OCTEON_PCI) += octeon_edac-pci.o
obj-$(CONFIG_EDAC_THUNDERX) += thunderx_edac.o
obj-$(CONFIG_EDAC_ALTERA) += altera_edac.o
+obj-$(CONFIG_EDAC_ARMADA_XP) += armada_xp_edac.o
obj-$(CONFIG_EDAC_SYNOPSYS) += synopsys_edac.o
obj-$(CONFIG_EDAC_XGENE) += xgene_edac.o
diff --git a/drivers/edac/armada_xp_edac.c b/drivers/edac/armada_xp_edac.c
new file mode 100644
index 000000000000..cb9173b30aa9
--- /dev/null
+++ b/drivers/edac/armada_xp_edac.c
@@ -0,0 +1,658 @@
+/*
+ * Copyright (C) 2017 Pengutronix, Jan Luebbe <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/edac.h>
+#include <linux/of_platform.h>
+
+#include <asm/hardware/cache-l2x0.h>
+#include <asm/hardware/cache-aurora-l2.h>
+
+#include "edac_mc.h"
+#include "edac_device.h"
+#include "edac_module.h"
+
+/************************ EDAC MC (DDR RAM) ********************************/
+
+#define SDRAM_NUM_CS 4
+
+#define SDRAM_CONFIG_REG 0x0
+#define SDRAM_CONFIG_ECC_MASK BIT(18)
+#define SDRAM_CONFIG_REGISTERED_MASK BIT(17)
+#define SDRAM_CONFIG_BUS_WIDTH_MASK BIT(15)
+
+#define SDRAM_ADDR_CTRL_REG 0x10
+#define SDRAM_ADDR_CTRL_SIZE_HIGH_OFFSET(cs) (20+cs)
+#define SDRAM_ADDR_CTRL_SIZE_HIGH_MASK(cs) \
+ (0x1 << SDRAM_ADDR_CTRL_SIZE_HIGH_OFFSET(cs))
+#define SDRAM_ADDR_CTRL_ADDR_SEL_MASK(cs) BIT(16+cs)
+#define SDRAM_ADDR_CTRL_SIZE_LOW_OFFSET(cs) (cs*4+2)
+#define SDRAM_ADDR_CTRL_SIZE_LOW_MASK(cs) \
+ (0x3 << SDRAM_ADDR_CTRL_SIZE_LOW_OFFSET(cs))
+#define SDRAM_ADDR_CTRL_STRUCT_OFFSET(cs) (cs*4)
+#define SDRAM_ADDR_CTRL_STRUCT_MASK(cs) \
+ (0x3 << SDRAM_ADDR_CTRL_STRUCT_OFFSET(cs))
+
+#define SDRAM_ERR_DATA_H_REG 0x40
+#define SDRAM_ERR_DATA_L_REG 0x44
+
+#define SDRAM_ERR_RECV_ECC_REG 0x48
+#define SDRAM_ERR_RECV_ECC_VALUE_MASK 0xff
+
+#define SDRAM_ERR_CALC_ECC_REG 0x4c
+#define SDRAM_ERR_CALC_ECC_ROW_OFFSET 8
+#define SDRAM_ERR_CALC_ECC_ROW_MASK \
+ (0xffff << SDRAM_ERR_CALC_ECC_ROW_OFFSET)
+#define SDRAM_ERR_CALC_ECC_VALUE_MASK 0xff
+
+#define SDRAM_ERR_ADDR_REG 0x50
+#define SDRAM_ERR_ADDR_BANK_OFFSET 23
+#define SDRAM_ERR_ADDR_BANK_MASK \
+ (0x7 << SDRAM_ERR_ADDR_BANK_OFFSET)
+#define SDRAM_ERR_ADDR_COL_OFFSET 8
+#define SDRAM_ERR_ADDR_COL_MASK \
+ (0x7fff << SDRAM_ERR_ADDR_COL_OFFSET)
+#define SDRAM_ERR_ADDR_CS_OFFSET 1
+#define SDRAM_ERR_ADDR_CS_MASK \
+ (0x3 << SDRAM_ERR_ADDR_CS_OFFSET)
+#define SDRAM_ERR_ADDR_TYPE_MASK BIT(0)
+
+#define SDRAM_ERR_CTRL_REG 0x54
+#define SDRAM_ERR_CTRL_ERR_THR_OFFSET 16
+#define SDRAM_ERR_CTRL_ERR_THR_MASK \
+ (0xff << SDRAM_ERR_CTRL_ERR_THR_OFFSET)
+#define SDRAM_ERR_CTRL_ERR_PROP_MASK BIT(9)
+
+#define SDRAM_ERR_SBE_COUNT_REG 0x58
+#define SDRAM_ERR_DBE_COUNT_REG 0x5c
+
+#define SDRAM_ERR_CAUSE_ERR_REG 0xd0
+#define SDRAM_ERR_CAUSE_MSG_REG 0xd8
+#define SDRAM_ERR_CAUSE_DBE_MASK BIT(1)
+#define SDRAM_ERR_CAUSE_SBE_MASK BIT(0)
+
+#define SDRAM_RANK_CTRL_REG 0x1e0
+#define SDRAM_RANK_CTRL_EXIST_MASK(cs) BIT(cs)
+
+struct armada_xp_mc_edac_drvdata {
+ void __iomem *base;
+
+ unsigned int width; /* width in bytes */
+ bool cs_addr_sel[SDRAM_NUM_CS]; /* bank interleaving */
+
+ char msg[128];
+};
+
+/* derived from "DRAM Address Multiplexing" in the ARAMDA XP Functional Spec */
+static uint32_t armada_xp_mc_edac_calc_address(struct armada_xp_mc_edac_drvdata
+ *drvdata, uint8_t cs,
+ uint8_t bank, uint16_t row,
+ uint16_t col)
+{
+ if (drvdata->width == 8) { /* 64 bit */
+ if (drvdata->cs_addr_sel[cs]) /* bank interleaved */
+ return (((row & 0xfff8) << 16) |
+ ((bank & 0x7) << 16) |
+ ((row & 0x7) << 13) |
+ ((col & 0x3ff) << 3));
+ else
+ return (((row & 0xffff << 16) |
+ ((bank & 0x7) << 13) |
+ ((col & 0x3ff)) << 3));
+ } else if (drvdata->width == 4) { /* 32 bit */
+ if (drvdata->cs_addr_sel[cs]) /* bank interleaved */
+ return (((row & 0xfff0) << 15) |
+ ((bank & 0x7) << 16) |
+ ((row & 0xf) << 12) |
+ ((col & 0x3ff) << 2));
+ else
+ return (((row & 0xffff << 15) |
+ ((bank & 0x7) << 12) |
+ ((col & 0x3ff)) << 2));
+ } else { /* 16 bit */
+ if (drvdata->cs_addr_sel[cs]) /* bank interleaved */
+ return (((row & 0xffe0) << 14) |
+ ((bank & 0x7) << 16) |
+ ((row & 0x1f) << 11) |
+ ((col & 0x3ff) << 1));
+ else
+ return (((row & 0xffff << 14) |
+ ((bank & 0x7) << 11) |
+ ((col & 0x3ff)) << 1));
+ }
+}
+
+static void armada_xp_mc_edac_check(struct mem_ctl_info *mci)
+{
+ struct armada_xp_mc_edac_drvdata *drvdata = mci->pvt_info;
+ uint32_t data_h, data_l, recv_ecc, calc_ecc, addr;
+ uint32_t cnt_sbe, cnt_dbe, cause_err, cause_msg;
+ uint32_t row_val, col_val, bank_val, addr_val;
+ uint8_t syndrome_val, cs_val;
+ char *msg = drvdata->msg;
+
+ data_h = readl(drvdata->base + SDRAM_ERR_DATA_H_REG);
+ data_l = readl(drvdata->base + SDRAM_ERR_DATA_L_REG);
+ recv_ecc = readl(drvdata->base + SDRAM_ERR_RECV_ECC_REG);
+ calc_ecc = readl(drvdata->base + SDRAM_ERR_CALC_ECC_REG);
+ addr = readl(drvdata->base + SDRAM_ERR_ADDR_REG);
+ cnt_sbe = readl(drvdata->base + SDRAM_ERR_SBE_COUNT_REG);
+ cnt_dbe = readl(drvdata->base + SDRAM_ERR_DBE_COUNT_REG);
+ cause_err = readl(drvdata->base + SDRAM_ERR_CAUSE_ERR_REG);
+ cause_msg = readl(drvdata->base + SDRAM_ERR_CAUSE_MSG_REG);
+
+ /* clear cause registers */
+ writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
+ drvdata->base + SDRAM_ERR_CAUSE_ERR_REG);
+ writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
+ drvdata->base + SDRAM_ERR_CAUSE_MSG_REG);
+
+ /* clear error counter registers */
+ if (cnt_sbe)
+ writel(0, drvdata->base + SDRAM_ERR_SBE_COUNT_REG);
+ if (cnt_dbe)
+ writel(0, drvdata->base + SDRAM_ERR_DBE_COUNT_REG);
+
+ if (!cnt_sbe && !cnt_dbe)
+ return;
+
+ if ((addr & SDRAM_ERR_ADDR_TYPE_MASK) == 0) {
+ if (cnt_sbe)
+ cnt_sbe--;
+ else
+ dev_warn(mci->pdev, "inconsistent SBE count detected");
+ } else {
+ if (cnt_dbe)
+ cnt_dbe--;
+ else
+ dev_warn(mci->pdev, "inconsistent DBE count detected");
+ }
+
+ /* report earlier errors */
+ if (cnt_sbe)
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ cnt_sbe, /* error count */
+ 0, 0, 0, /* pfn, offset, syndrome */
+ -1, -1, -1, /* top, mid, low layer */
+ mci->ctl_name,
+ "details unavailable (multiple errors)");
+ if (cnt_dbe)
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ cnt_sbe, /* error count */
+ 0, 0, 0, /* pfn, offset, syndrome */
+ -1, -1, -1, /* top, mid, low layer */
+ mci->ctl_name,
+ "details unavailable (multiple errors)");
+
+ /* report details for most recent error */
+ cs_val = (addr & SDRAM_ERR_ADDR_CS_MASK)
+ >> SDRAM_ERR_ADDR_CS_OFFSET;
+ bank_val = (addr & SDRAM_ERR_ADDR_BANK_MASK)
+ >> SDRAM_ERR_ADDR_BANK_OFFSET;
+ row_val = (calc_ecc & SDRAM_ERR_CALC_ECC_ROW_MASK)
+ >> SDRAM_ERR_CALC_ECC_ROW_OFFSET;
+ col_val = (addr & SDRAM_ERR_ADDR_COL_MASK)
+ >> SDRAM_ERR_ADDR_COL_OFFSET;
+ syndrome_val = (recv_ecc ^ calc_ecc) & 0xff;
+ addr_val = armada_xp_mc_edac_calc_address(drvdata, cs_val, bank_val,
+ row_val, col_val);
+ msg += sprintf(msg, "row=0x%04x ", row_val); /* 11 chars */
+ msg += sprintf(msg, "bank=0x%x ", bank_val); /* 9 chars */
+ msg += sprintf(msg, "col=0x%04x ", col_val); /* 11 chars */
+ msg += sprintf(msg, "cs=%d", cs_val); /* 4 chars */
+
+ if ((addr & SDRAM_ERR_ADDR_TYPE_MASK) == 0) {
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ 1, /* error count */
+ addr_val >> PAGE_SHIFT,
+ addr_val & ~PAGE_MASK,
+ syndrome_val,
+ cs_val, -1, -1, /* top, mid, low layer */
+ mci->ctl_name, drvdata->msg);
+ } else {
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ 1, /* error count */
+ addr_val >> PAGE_SHIFT,
+ addr_val & ~PAGE_MASK,
+ syndrome_val,
+ cs_val, -1, -1, /* top, mid, low layer */
+ mci->ctl_name, drvdata->msg);
+ }
+}
+
+static void armada_xp_mc_edac_read_config(struct mem_ctl_info *mci)
+{
+ struct armada_xp_mc_edac_drvdata *drvdata = mci->pvt_info;
+ struct dimm_info *dimm;
+ unsigned int i, cs_struct, cs_size;
+ uint32_t config, addr_ctrl, rank_ctrl;
+
+ config = readl(drvdata->base + SDRAM_CONFIG_REG);
+ if (config & SDRAM_CONFIG_BUS_WIDTH_MASK)
+ drvdata->width = 8; /* 64 bit */
+ else
+ drvdata->width = 4; /* 32 bit */
+
+ addr_ctrl = readl(drvdata->base + SDRAM_ADDR_CTRL_REG);
+ rank_ctrl = readl(drvdata->base + SDRAM_RANK_CTRL_REG);
+ for (i = 0; i < SDRAM_NUM_CS; i++) {
+ dimm = mci->dimms[i];
+
+ if (!(rank_ctrl & SDRAM_RANK_CTRL_EXIST_MASK(i)))
+ continue;
+
+ drvdata->cs_addr_sel[i] =
+ !!(addr_ctrl & SDRAM_ADDR_CTRL_ADDR_SEL_MASK(i));
+
+ cs_struct = (addr_ctrl & SDRAM_ADDR_CTRL_STRUCT_MASK(i)) >>
+ SDRAM_ADDR_CTRL_STRUCT_OFFSET(i);
+ cs_size = ((addr_ctrl & SDRAM_ADDR_CTRL_SIZE_HIGH_MASK(i)) >>
+ (SDRAM_ADDR_CTRL_SIZE_HIGH_OFFSET(i) - 2) |
+ ((addr_ctrl & SDRAM_ADDR_CTRL_SIZE_LOW_MASK(i)) >>
+ SDRAM_ADDR_CTRL_SIZE_LOW_OFFSET(i)));
+ switch (cs_size) {
+ case 0: /* 2GBit */
+ dimm->nr_pages = (0x80000000ULL >> PAGE_SHIFT);
+ break;
+ case 1: /* 256MBit */
+ dimm->nr_pages = (0x10000000ULL >> PAGE_SHIFT);
+ break;
+ case 2: /* 512MBit */
+ dimm->nr_pages = (0x20000000ULL >> PAGE_SHIFT);
+ break;
+ case 3: /* 1GBit */
+ dimm->nr_pages = (0x40000000ULL >> PAGE_SHIFT);
+ break;
+ case 4: /* 4GBit */
+ dimm->nr_pages = (0x100000000ULL >> PAGE_SHIFT);
+ break;
+ case 5: /* 8GBit */
+ dimm->nr_pages = (0x200000000ULL >> PAGE_SHIFT);
+ break;
+ }
+ dimm->grain = 8;
+ dimm->dtype = cs_struct ? DEV_X16 : DEV_X8;
+ dimm->mtype = (config & SDRAM_CONFIG_REGISTERED_MASK) ?
+ MEM_RDDR3 : MEM_DDR3;
+ dimm->edac_mode = EDAC_SECDED;
+ }
+}
+
+static const struct of_device_id armada_xp_mc_edac_of_match[] = {
+ {.compatible = "marvell,armada-xp-sdram-controller",},
+ {},
+};
+MODULE_DEVICE_TABLE(of, armada_xp_mc_edac_of_match);
+
+static int armada_xp_mc_edac_probe(struct platform_device *pdev)
+{
+ const struct of_device_id *id;
+ struct mem_ctl_info *mci;
+ struct edac_mc_layer layers[1];
+ struct armada_xp_mc_edac_drvdata *drvdata;
+ struct resource *r;
+ void __iomem *base;
+ uint32_t config;
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!r) {
+ dev_err(&pdev->dev, "Unable to get mem resource\n");
+ return -ENODEV;
+ }
+
+ base = devm_ioremap_resource(&pdev->dev, r);
+ if (IS_ERR(base)) {
+ dev_err(&pdev->dev, "Unable to map regs\n");
+ return PTR_ERR(base);
+ }
+
+ config = readl(base + SDRAM_CONFIG_REG);
+ if (!(config & SDRAM_CONFIG_ECC_MASK)) {
+ dev_warn(&pdev->dev, "SDRAM ECC is not enabled");
+ return -EINVAL;
+ }
+
+ layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
+ layers[0].size = SDRAM_NUM_CS;
+ layers[0].is_virt_csrow = true;
+
+ mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*drvdata));
+ if (!mci)
+ return -ENOMEM;
+
+ drvdata = mci->pvt_info;
+ drvdata->base = base;
+ mci->pdev = &pdev->dev;
+ platform_set_drvdata(pdev, mci);
+
+ id = of_match_device(armada_xp_mc_edac_of_match, &pdev->dev);
+ mci->edac_check = armada_xp_mc_edac_check;
+ mci->mtype_cap = MEM_FLAG_DDR3;
+ mci->edac_cap = EDAC_FLAG_SECDED;
+ mci->mod_name = pdev->dev.driver->name;
+ mci->ctl_name = id ? id->compatible : "unknown";
+ mci->dev_name = dev_name(&pdev->dev);
+ mci->scrub_mode = SCRUB_NONE;
+
+ armada_xp_mc_edac_read_config(mci);
+
+ /* configure SBE threshold */
+ /* it seems that SBEs are not captured otherwise */
+ writel(1 << SDRAM_ERR_CTRL_ERR_THR_OFFSET,
+ drvdata->base + SDRAM_ERR_CTRL_REG);
+
+ /* clear cause registers */
+ writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
+ drvdata->base + SDRAM_ERR_CAUSE_ERR_REG);
+ writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
+ drvdata->base + SDRAM_ERR_CAUSE_MSG_REG);
+
+ /* clear counter registers */
+ writel(0, drvdata->base + SDRAM_ERR_SBE_COUNT_REG);
+ writel(0, drvdata->base + SDRAM_ERR_DBE_COUNT_REG);
+
+ if (edac_mc_add_mc(mci)) {
+ edac_mc_free(mci);
+ return -EINVAL;
+ }
+ edac_op_state = EDAC_OPSTATE_POLL;
+
+ return 0;
+}
+
+static int armada_xp_mc_edac_remove(struct platform_device *pdev)
+{
+ struct mem_ctl_info *mci = platform_get_drvdata(pdev);
+
+ edac_mc_del_mc(&pdev->dev);
+ edac_mc_free(mci);
+ platform_set_drvdata(pdev, NULL);
+
+ return 0;
+}
+
+static struct platform_driver armada_xp_mc_edac_driver = {
+ .probe = armada_xp_mc_edac_probe,
+ .remove = armada_xp_mc_edac_remove,
+ .driver = {
+ .name = "armada_xp_mc_edac",
+ .of_match_table = of_match_ptr(armada_xp_mc_edac_of_match),
+ },
+};
+
+/************************ EDAC Device (L2 Cache) ***************************/
+
+struct aurora_l2_edac_drvdata {
+ void __iomem *base;
+
+ char msg[128];
+
+ /* error injection via debugfs */
+ uint32_t inject_addr;
+ uint32_t inject_mask;
+ uint8_t inject_ctl;
+
+ struct dentry *debugfs;
+};
+
+static void aurora_l2_edac_inject(struct aurora_l2_edac_drvdata *drvdata)
+{
+ drvdata->inject_addr &= AURORA_ERR_INJECT_CTL_ADDR_MASK;
+ drvdata->inject_ctl &= AURORA_ERR_INJECT_CTL_EN_MASK;
+ writel(0, drvdata->base + AURORA_ERR_INJECT_CTL_REG);
+ writel(drvdata->inject_mask,
+ drvdata->base + AURORA_ERR_INJECT_MASK_REG);
+ writel(drvdata->inject_addr | drvdata->inject_ctl,
+ drvdata->base + AURORA_ERR_INJECT_CTL_REG);
+}
+
+static void aurora_l2_edac_check(struct edac_device_ctl_info *dci)
+{
+ struct aurora_l2_edac_drvdata *drvdata = dci->pvt_info;
+ uint32_t cnt, attr_cap, addr_cap, way_cap;
+ unsigned int cnt_ce, cnt_ue;
+
+ cnt = readl(drvdata->base + AURORA_ERR_CNT_REG);
+ attr_cap = readl(drvdata->base + AURORA_ERR_ATTR_CAP_REG);
+ addr_cap = readl(drvdata->base + AURORA_ERR_ADDR_CAP_REG);
+ way_cap = readl(drvdata->base + AURORA_ERR_WAY_CAP_REG);
+
+ cnt_ce = (cnt & AURORA_ERR_CNT_CE_MASK) >> AURORA_ERR_CNT_CE_OFFSET;
+ cnt_ue = (cnt & AURORA_ERR_CNT_UE_MASK) >> AURORA_ERR_CNT_UE_OFFSET;
+ /* clear error counter registers */
+ if (cnt_ce || cnt_ue)
+ writel(AURORA_ERR_CNT_CLR, drvdata->base + AURORA_ERR_CNT_REG);
+
+ if (attr_cap & AURORA_ERR_ATTR_CAP_VALID) {
+ char *msg = drvdata->msg;
+ size_t size = sizeof(drvdata->msg);
+ size_t len = 0;
+
+ switch ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_SOURCE_MASK)
+ >> AURORA_ERR_ATTR_CAP_ERR_SOURCE_OFFSET) {
+ case 0:
+ len += snprintf(msg+len, size-len, "src=CPU0 ");
+ break;
+ case 1:
+ len += snprintf(msg+len, size-len, "src=CPU1 ");
+ break;
+ case 2:
+ len += snprintf(msg+len, size-len, "src=CPU2 ");
+ break;
+ case 3:
+ len += snprintf(msg+len, size-len, "src=CPU3 ");
+ break;
+ case 7:
+ len += snprintf(msg+len, size-len, "src=IO ");
+ break;
+ }
+ switch ((attr_cap & AURORA_ERR_ATTR_CAP_TRANS_TYPE_MASK)
+ >> AURORA_ERR_ATTR_CAP_TRANS_TYPE_OFFSET) {
+ case 0:
+ len += snprintf(msg+len, size-len, "txn=Data-Read ");
+ break;
+ case 1:
+ len += snprintf(msg+len, size-len, "txn=Isn-Read ");
+ break;
+ case 2:
+ len += snprintf(msg+len, size-len, "txn=Clean-Flush ");
+ break;
+ case 3:
+ len += snprintf(msg+len, size-len, "txn=Eviction ");
+ break;
+ case 4:
+ len += snprintf(msg+len, size-len,
+ "txn=Read-Modify-Write ");
+ break;
+ }
+ switch ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_TYPE_MASK)
+ >> AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET) {
+ case 0:
+ len += snprintf(msg+len, size-len, "err=CorrECC ");
+ break;
+ case 1:
+ len += snprintf(msg+len, size-len, "err=UnCorrECC ");
+ break;
+ case 2:
+ len += snprintf(msg+len, size-len, "err=TagParity ");
+ break;
+ }
+ len += snprintf(msg+len, size-len, "addr=0x%x ",
+ addr_cap & AURORA_ERR_ADDR_CAP_ADDR_MASK);
+ len += snprintf(msg+len, size-len, "index=0x%x ",
+ (way_cap & AURORA_ERR_WAY_CAP_INDEX_MASK)
+ >> AURORA_ERR_WAY_CAP_INDEX_OFFSET);
+ len += snprintf(msg+len, size-len, "way=0x%x",
+ (way_cap & AURORA_ERR_WAY_CAP_WAY_MASK)
+ >> AURORA_ERR_WAY_CAP_WAY_OFFSET);
+ /* clear error capture registers */
+ writel(AURORA_ERR_ATTR_CAP_VALID,
+ drvdata->base + AURORA_ERR_ATTR_CAP_REG);
+ if ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_TYPE_MASK)
+ >> AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET) {
+ /* UnCorrECC or TagParity */
+ if (cnt_ue)
+ cnt_ue--;
+ edac_device_handle_ue(dci, 0, 0, drvdata->msg);
+ } else {
+ if (cnt_ce)
+ cnt_ce--;
+ edac_device_handle_ce(dci, 0, 0, drvdata->msg);
+ }
+ }
+
+ /* report remaining errors */
+ while (cnt_ue--)
+ edac_device_handle_ue(dci, 0, 0,
+ "details unavailable (multiple errors)");
+ while (cnt_ce--)
+ edac_device_handle_ue(dci, 0, 0,
+ "details unavailable (multiple errors)");
+}
+
+static void aurora_l2_edac_poll(struct edac_device_ctl_info *dci)
+{
+ struct aurora_l2_edac_drvdata *drvdata = dci->pvt_info;
+
+ aurora_l2_edac_check(dci);
+ aurora_l2_edac_inject(drvdata);
+}
+
+static const struct of_device_id aurora_l2_edac_of_match[] = {
+ {.compatible = "marvell,aurora-system-cache",},
+ {},
+};
+MODULE_DEVICE_TABLE(of, aurora_l2_edac_of_match);
+
+static int aurora_l2_edac_probe(struct platform_device *pdev)
+{
+ const struct of_device_id *id;
+ struct edac_device_ctl_info *dci;
+ struct aurora_l2_edac_drvdata *drvdata;
+ struct resource *r;
+ void __iomem *base;
+ uint32_t l2x0_aux_ctrl;
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!r) {
+ dev_err(&pdev->dev, "Unable to get mem resource\n");
+ return -ENODEV;
+ }
+
+ base = devm_ioremap_resource(&pdev->dev, r);
+ if (IS_ERR(base)) {
+ dev_err(&pdev->dev, "Unable to map regs\n");
+ return PTR_ERR(base);
+ }
+
+ l2x0_aux_ctrl = readl(base + L2X0_AUX_CTRL);
+ if (!(l2x0_aux_ctrl & AURORA_ACR_PARITY_EN))
+ dev_warn(&pdev->dev, "tag parity is not enabled");
+ if (!(l2x0_aux_ctrl & AURORA_ACR_ECC_EN))
+ dev_warn(&pdev->dev, "data ECC is not enabled");
+
+ dci = edac_device_alloc_ctl_info(sizeof(*drvdata),
+ "cpu", 1, "L", 1, 2, NULL, 0, 0);
+ if (!dci)
+ return -ENOMEM;
+
+ drvdata = dci->pvt_info;
+ drvdata->base = base;
+ dci->dev = &pdev->dev;
+ platform_set_drvdata(pdev, dci);
+
+ id = of_match_device(aurora_l2_edac_of_match, &pdev->dev);
+ dci->edac_check = aurora_l2_edac_poll;
+ dci->mod_name = pdev->dev.driver->name;
+ dci->ctl_name = id ? id->compatible : "unknown";
+ dci->dev_name = dev_name(&pdev->dev);
+
+ /* clear registers */
+ writel(AURORA_ERR_CNT_CLR, drvdata->base + AURORA_ERR_CNT_REG);
+ writel(AURORA_ERR_ATTR_CAP_VALID,
+ drvdata->base + AURORA_ERR_ATTR_CAP_REG);
+
+ if (edac_device_add_device(dci)) {
+ edac_device_free_ctl_info(dci);
+ return -EINVAL;
+ }
+
+ drvdata->debugfs = edac_debugfs_create_dir(dev_name(&pdev->dev));
+ if (drvdata->debugfs) {
+ edac_debugfs_create_x32("inject_addr", 0644,
+ drvdata->debugfs,
+ &drvdata->inject_addr);
+ edac_debugfs_create_x32("inject_mask", 0644,
+ drvdata->debugfs,
+ &drvdata->inject_mask);
+ edac_debugfs_create_x8("inject_ctl", 0644,
+ drvdata->debugfs, &drvdata->inject_ctl);
+ }
+
+ return 0;
+}
+
+static int aurora_l2_edac_remove(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
+ struct aurora_l2_edac_drvdata *drvdata = dci->pvt_info;
+
+ edac_debugfs_remove_recursive(drvdata->debugfs);
+ edac_device_del_device(&pdev->dev);
+ edac_device_free_ctl_info(dci);
+ platform_set_drvdata(pdev, NULL);
+
+ return 0;
+}
+
+static struct platform_driver aurora_l2_edac_driver = {
+ .probe = aurora_l2_edac_probe,
+ .remove = aurora_l2_edac_remove,
+ .driver = {
+ .name = "aurora_l2_edac",
+ .of_match_table = of_match_ptr(aurora_l2_edac_of_match),
+ },
+};
+
+/************************ Driver registration ******************************/
+
+static struct platform_driver * const drivers[] = {
+ &armada_xp_mc_edac_driver,
+ &aurora_l2_edac_driver,
+};
+
+static int __init armada_xp_edac_init(void)
+{
+ int res = 0;
+
+ /* only polling is supported */
+ edac_op_state = EDAC_OPSTATE_POLL;
+
+ res = platform_register_drivers(drivers, ARRAY_SIZE(drivers));
+ if (res)
+ pr_warn("Aramda XP EDAC drivers fail to register\n");
+
+ return 0;
+}
+module_init(armada_xp_edac_init);
+
+static void __exit armada_xp_edac_exit(void)
+{
+ platform_unregister_drivers(drivers, ARRAY_SIZE(drivers));
+}
+module_exit(armada_xp_edac_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Pengutronix");
+MODULE_DESCRIPTION("EDAC Drivers for Marvell Armada XP SDRAM and L2 Cache Controller");
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
@ 2017-11-10 9:03 ` Jan Luebbe
0 siblings, 0 replies; 35+ messages in thread
From: Jan Luebbe @ 2017-11-10 9:03 UTC (permalink / raw)
To: linux-arm-kernel
Add support for the ECC functionality as found in the DDR RAM and L2
cache controllers on the MV78230/MV78x60 SoCs. This driver has been
tested on the MV78460 (on a custom board with a DDR3 ECC DIMM).
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
MAINTAINERS | 6 +
drivers/edac/Kconfig | 7 +
drivers/edac/Makefile | 1 +
drivers/edac/armada_xp_edac.c | 658 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 672 insertions(+)
create mode 100644 drivers/edac/armada_xp_edac.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2f4e462aa4a2..e8c4f7518f11 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4900,6 +4900,12 @@ L: linux-edac at vger.kernel.org
S: Maintained
F: drivers/edac/amd64_edac*
+EDAC-ARMADA
+M: Jan Luebbe <jlu@pengutronix.de>
+L: linux-edac at vger.kernel.org
+S: Maintained
+F: drivers/edac/armada_xp_*
+
EDAC-CALXEDA
M: Robert Richter <rric@kernel.org>
L: linux-edac at vger.kernel.org
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 96afb2aeed18..4e123ec94a4a 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -443,6 +443,13 @@ config EDAC_ALTERA_SDMMC
Support for error detection and correction on the
Altera SDMMC FIFO Memory for Altera SoCs.
+config EDAC_ARMADA_XP
+ bool "Marvell Armada XP DDR and L2 Cache ECC"
+ depends on ARCH_MVEBU
+ help
+ Support for error correction and detection on the Marvell Aramada XP
+ DDR RAM and L2 cache controllers.
+
config EDAC_SYNOPSYS
tristate "Synopsys DDR Memory Controller"
depends on ARCH_ZYNQ
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 0fd9ffa63299..70a093cc976e 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -76,5 +76,6 @@ obj-$(CONFIG_EDAC_OCTEON_PCI) += octeon_edac-pci.o
obj-$(CONFIG_EDAC_THUNDERX) += thunderx_edac.o
obj-$(CONFIG_EDAC_ALTERA) += altera_edac.o
+obj-$(CONFIG_EDAC_ARMADA_XP) += armada_xp_edac.o
obj-$(CONFIG_EDAC_SYNOPSYS) += synopsys_edac.o
obj-$(CONFIG_EDAC_XGENE) += xgene_edac.o
diff --git a/drivers/edac/armada_xp_edac.c b/drivers/edac/armada_xp_edac.c
new file mode 100644
index 000000000000..cb9173b30aa9
--- /dev/null
+++ b/drivers/edac/armada_xp_edac.c
@@ -0,0 +1,658 @@
+/*
+ * Copyright (C) 2017 Pengutronix, Jan Luebbe <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/edac.h>
+#include <linux/of_platform.h>
+
+#include <asm/hardware/cache-l2x0.h>
+#include <asm/hardware/cache-aurora-l2.h>
+
+#include "edac_mc.h"
+#include "edac_device.h"
+#include "edac_module.h"
+
+/************************ EDAC MC (DDR RAM) ********************************/
+
+#define SDRAM_NUM_CS 4
+
+#define SDRAM_CONFIG_REG 0x0
+#define SDRAM_CONFIG_ECC_MASK BIT(18)
+#define SDRAM_CONFIG_REGISTERED_MASK BIT(17)
+#define SDRAM_CONFIG_BUS_WIDTH_MASK BIT(15)
+
+#define SDRAM_ADDR_CTRL_REG 0x10
+#define SDRAM_ADDR_CTRL_SIZE_HIGH_OFFSET(cs) (20+cs)
+#define SDRAM_ADDR_CTRL_SIZE_HIGH_MASK(cs) \
+ (0x1 << SDRAM_ADDR_CTRL_SIZE_HIGH_OFFSET(cs))
+#define SDRAM_ADDR_CTRL_ADDR_SEL_MASK(cs) BIT(16+cs)
+#define SDRAM_ADDR_CTRL_SIZE_LOW_OFFSET(cs) (cs*4+2)
+#define SDRAM_ADDR_CTRL_SIZE_LOW_MASK(cs) \
+ (0x3 << SDRAM_ADDR_CTRL_SIZE_LOW_OFFSET(cs))
+#define SDRAM_ADDR_CTRL_STRUCT_OFFSET(cs) (cs*4)
+#define SDRAM_ADDR_CTRL_STRUCT_MASK(cs) \
+ (0x3 << SDRAM_ADDR_CTRL_STRUCT_OFFSET(cs))
+
+#define SDRAM_ERR_DATA_H_REG 0x40
+#define SDRAM_ERR_DATA_L_REG 0x44
+
+#define SDRAM_ERR_RECV_ECC_REG 0x48
+#define SDRAM_ERR_RECV_ECC_VALUE_MASK 0xff
+
+#define SDRAM_ERR_CALC_ECC_REG 0x4c
+#define SDRAM_ERR_CALC_ECC_ROW_OFFSET 8
+#define SDRAM_ERR_CALC_ECC_ROW_MASK \
+ (0xffff << SDRAM_ERR_CALC_ECC_ROW_OFFSET)
+#define SDRAM_ERR_CALC_ECC_VALUE_MASK 0xff
+
+#define SDRAM_ERR_ADDR_REG 0x50
+#define SDRAM_ERR_ADDR_BANK_OFFSET 23
+#define SDRAM_ERR_ADDR_BANK_MASK \
+ (0x7 << SDRAM_ERR_ADDR_BANK_OFFSET)
+#define SDRAM_ERR_ADDR_COL_OFFSET 8
+#define SDRAM_ERR_ADDR_COL_MASK \
+ (0x7fff << SDRAM_ERR_ADDR_COL_OFFSET)
+#define SDRAM_ERR_ADDR_CS_OFFSET 1
+#define SDRAM_ERR_ADDR_CS_MASK \
+ (0x3 << SDRAM_ERR_ADDR_CS_OFFSET)
+#define SDRAM_ERR_ADDR_TYPE_MASK BIT(0)
+
+#define SDRAM_ERR_CTRL_REG 0x54
+#define SDRAM_ERR_CTRL_ERR_THR_OFFSET 16
+#define SDRAM_ERR_CTRL_ERR_THR_MASK \
+ (0xff << SDRAM_ERR_CTRL_ERR_THR_OFFSET)
+#define SDRAM_ERR_CTRL_ERR_PROP_MASK BIT(9)
+
+#define SDRAM_ERR_SBE_COUNT_REG 0x58
+#define SDRAM_ERR_DBE_COUNT_REG 0x5c
+
+#define SDRAM_ERR_CAUSE_ERR_REG 0xd0
+#define SDRAM_ERR_CAUSE_MSG_REG 0xd8
+#define SDRAM_ERR_CAUSE_DBE_MASK BIT(1)
+#define SDRAM_ERR_CAUSE_SBE_MASK BIT(0)
+
+#define SDRAM_RANK_CTRL_REG 0x1e0
+#define SDRAM_RANK_CTRL_EXIST_MASK(cs) BIT(cs)
+
+struct armada_xp_mc_edac_drvdata {
+ void __iomem *base;
+
+ unsigned int width; /* width in bytes */
+ bool cs_addr_sel[SDRAM_NUM_CS]; /* bank interleaving */
+
+ char msg[128];
+};
+
+/* derived from "DRAM Address Multiplexing" in the ARAMDA XP Functional Spec */
+static uint32_t armada_xp_mc_edac_calc_address(struct armada_xp_mc_edac_drvdata
+ *drvdata, uint8_t cs,
+ uint8_t bank, uint16_t row,
+ uint16_t col)
+{
+ if (drvdata->width == 8) { /* 64 bit */
+ if (drvdata->cs_addr_sel[cs]) /* bank interleaved */
+ return (((row & 0xfff8) << 16) |
+ ((bank & 0x7) << 16) |
+ ((row & 0x7) << 13) |
+ ((col & 0x3ff) << 3));
+ else
+ return (((row & 0xffff << 16) |
+ ((bank & 0x7) << 13) |
+ ((col & 0x3ff)) << 3));
+ } else if (drvdata->width == 4) { /* 32 bit */
+ if (drvdata->cs_addr_sel[cs]) /* bank interleaved */
+ return (((row & 0xfff0) << 15) |
+ ((bank & 0x7) << 16) |
+ ((row & 0xf) << 12) |
+ ((col & 0x3ff) << 2));
+ else
+ return (((row & 0xffff << 15) |
+ ((bank & 0x7) << 12) |
+ ((col & 0x3ff)) << 2));
+ } else { /* 16 bit */
+ if (drvdata->cs_addr_sel[cs]) /* bank interleaved */
+ return (((row & 0xffe0) << 14) |
+ ((bank & 0x7) << 16) |
+ ((row & 0x1f) << 11) |
+ ((col & 0x3ff) << 1));
+ else
+ return (((row & 0xffff << 14) |
+ ((bank & 0x7) << 11) |
+ ((col & 0x3ff)) << 1));
+ }
+}
+
+static void armada_xp_mc_edac_check(struct mem_ctl_info *mci)
+{
+ struct armada_xp_mc_edac_drvdata *drvdata = mci->pvt_info;
+ uint32_t data_h, data_l, recv_ecc, calc_ecc, addr;
+ uint32_t cnt_sbe, cnt_dbe, cause_err, cause_msg;
+ uint32_t row_val, col_val, bank_val, addr_val;
+ uint8_t syndrome_val, cs_val;
+ char *msg = drvdata->msg;
+
+ data_h = readl(drvdata->base + SDRAM_ERR_DATA_H_REG);
+ data_l = readl(drvdata->base + SDRAM_ERR_DATA_L_REG);
+ recv_ecc = readl(drvdata->base + SDRAM_ERR_RECV_ECC_REG);
+ calc_ecc = readl(drvdata->base + SDRAM_ERR_CALC_ECC_REG);
+ addr = readl(drvdata->base + SDRAM_ERR_ADDR_REG);
+ cnt_sbe = readl(drvdata->base + SDRAM_ERR_SBE_COUNT_REG);
+ cnt_dbe = readl(drvdata->base + SDRAM_ERR_DBE_COUNT_REG);
+ cause_err = readl(drvdata->base + SDRAM_ERR_CAUSE_ERR_REG);
+ cause_msg = readl(drvdata->base + SDRAM_ERR_CAUSE_MSG_REG);
+
+ /* clear cause registers */
+ writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
+ drvdata->base + SDRAM_ERR_CAUSE_ERR_REG);
+ writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
+ drvdata->base + SDRAM_ERR_CAUSE_MSG_REG);
+
+ /* clear error counter registers */
+ if (cnt_sbe)
+ writel(0, drvdata->base + SDRAM_ERR_SBE_COUNT_REG);
+ if (cnt_dbe)
+ writel(0, drvdata->base + SDRAM_ERR_DBE_COUNT_REG);
+
+ if (!cnt_sbe && !cnt_dbe)
+ return;
+
+ if ((addr & SDRAM_ERR_ADDR_TYPE_MASK) == 0) {
+ if (cnt_sbe)
+ cnt_sbe--;
+ else
+ dev_warn(mci->pdev, "inconsistent SBE count detected");
+ } else {
+ if (cnt_dbe)
+ cnt_dbe--;
+ else
+ dev_warn(mci->pdev, "inconsistent DBE count detected");
+ }
+
+ /* report earlier errors */
+ if (cnt_sbe)
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ cnt_sbe, /* error count */
+ 0, 0, 0, /* pfn, offset, syndrome */
+ -1, -1, -1, /* top, mid, low layer */
+ mci->ctl_name,
+ "details unavailable (multiple errors)");
+ if (cnt_dbe)
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ cnt_sbe, /* error count */
+ 0, 0, 0, /* pfn, offset, syndrome */
+ -1, -1, -1, /* top, mid, low layer */
+ mci->ctl_name,
+ "details unavailable (multiple errors)");
+
+ /* report details for most recent error */
+ cs_val = (addr & SDRAM_ERR_ADDR_CS_MASK)
+ >> SDRAM_ERR_ADDR_CS_OFFSET;
+ bank_val = (addr & SDRAM_ERR_ADDR_BANK_MASK)
+ >> SDRAM_ERR_ADDR_BANK_OFFSET;
+ row_val = (calc_ecc & SDRAM_ERR_CALC_ECC_ROW_MASK)
+ >> SDRAM_ERR_CALC_ECC_ROW_OFFSET;
+ col_val = (addr & SDRAM_ERR_ADDR_COL_MASK)
+ >> SDRAM_ERR_ADDR_COL_OFFSET;
+ syndrome_val = (recv_ecc ^ calc_ecc) & 0xff;
+ addr_val = armada_xp_mc_edac_calc_address(drvdata, cs_val, bank_val,
+ row_val, col_val);
+ msg += sprintf(msg, "row=0x%04x ", row_val); /* 11 chars */
+ msg += sprintf(msg, "bank=0x%x ", bank_val); /* 9 chars */
+ msg += sprintf(msg, "col=0x%04x ", col_val); /* 11 chars */
+ msg += sprintf(msg, "cs=%d", cs_val); /* 4 chars */
+
+ if ((addr & SDRAM_ERR_ADDR_TYPE_MASK) == 0) {
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ 1, /* error count */
+ addr_val >> PAGE_SHIFT,
+ addr_val & ~PAGE_MASK,
+ syndrome_val,
+ cs_val, -1, -1, /* top, mid, low layer */
+ mci->ctl_name, drvdata->msg);
+ } else {
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ 1, /* error count */
+ addr_val >> PAGE_SHIFT,
+ addr_val & ~PAGE_MASK,
+ syndrome_val,
+ cs_val, -1, -1, /* top, mid, low layer */
+ mci->ctl_name, drvdata->msg);
+ }
+}
+
+static void armada_xp_mc_edac_read_config(struct mem_ctl_info *mci)
+{
+ struct armada_xp_mc_edac_drvdata *drvdata = mci->pvt_info;
+ struct dimm_info *dimm;
+ unsigned int i, cs_struct, cs_size;
+ uint32_t config, addr_ctrl, rank_ctrl;
+
+ config = readl(drvdata->base + SDRAM_CONFIG_REG);
+ if (config & SDRAM_CONFIG_BUS_WIDTH_MASK)
+ drvdata->width = 8; /* 64 bit */
+ else
+ drvdata->width = 4; /* 32 bit */
+
+ addr_ctrl = readl(drvdata->base + SDRAM_ADDR_CTRL_REG);
+ rank_ctrl = readl(drvdata->base + SDRAM_RANK_CTRL_REG);
+ for (i = 0; i < SDRAM_NUM_CS; i++) {
+ dimm = mci->dimms[i];
+
+ if (!(rank_ctrl & SDRAM_RANK_CTRL_EXIST_MASK(i)))
+ continue;
+
+ drvdata->cs_addr_sel[i] =
+ !!(addr_ctrl & SDRAM_ADDR_CTRL_ADDR_SEL_MASK(i));
+
+ cs_struct = (addr_ctrl & SDRAM_ADDR_CTRL_STRUCT_MASK(i)) >>
+ SDRAM_ADDR_CTRL_STRUCT_OFFSET(i);
+ cs_size = ((addr_ctrl & SDRAM_ADDR_CTRL_SIZE_HIGH_MASK(i)) >>
+ (SDRAM_ADDR_CTRL_SIZE_HIGH_OFFSET(i) - 2) |
+ ((addr_ctrl & SDRAM_ADDR_CTRL_SIZE_LOW_MASK(i)) >>
+ SDRAM_ADDR_CTRL_SIZE_LOW_OFFSET(i)));
+ switch (cs_size) {
+ case 0: /* 2GBit */
+ dimm->nr_pages = (0x80000000ULL >> PAGE_SHIFT);
+ break;
+ case 1: /* 256MBit */
+ dimm->nr_pages = (0x10000000ULL >> PAGE_SHIFT);
+ break;
+ case 2: /* 512MBit */
+ dimm->nr_pages = (0x20000000ULL >> PAGE_SHIFT);
+ break;
+ case 3: /* 1GBit */
+ dimm->nr_pages = (0x40000000ULL >> PAGE_SHIFT);
+ break;
+ case 4: /* 4GBit */
+ dimm->nr_pages = (0x100000000ULL >> PAGE_SHIFT);
+ break;
+ case 5: /* 8GBit */
+ dimm->nr_pages = (0x200000000ULL >> PAGE_SHIFT);
+ break;
+ }
+ dimm->grain = 8;
+ dimm->dtype = cs_struct ? DEV_X16 : DEV_X8;
+ dimm->mtype = (config & SDRAM_CONFIG_REGISTERED_MASK) ?
+ MEM_RDDR3 : MEM_DDR3;
+ dimm->edac_mode = EDAC_SECDED;
+ }
+}
+
+static const struct of_device_id armada_xp_mc_edac_of_match[] = {
+ {.compatible = "marvell,armada-xp-sdram-controller",},
+ {},
+};
+MODULE_DEVICE_TABLE(of, armada_xp_mc_edac_of_match);
+
+static int armada_xp_mc_edac_probe(struct platform_device *pdev)
+{
+ const struct of_device_id *id;
+ struct mem_ctl_info *mci;
+ struct edac_mc_layer layers[1];
+ struct armada_xp_mc_edac_drvdata *drvdata;
+ struct resource *r;
+ void __iomem *base;
+ uint32_t config;
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!r) {
+ dev_err(&pdev->dev, "Unable to get mem resource\n");
+ return -ENODEV;
+ }
+
+ base = devm_ioremap_resource(&pdev->dev, r);
+ if (IS_ERR(base)) {
+ dev_err(&pdev->dev, "Unable to map regs\n");
+ return PTR_ERR(base);
+ }
+
+ config = readl(base + SDRAM_CONFIG_REG);
+ if (!(config & SDRAM_CONFIG_ECC_MASK)) {
+ dev_warn(&pdev->dev, "SDRAM ECC is not enabled");
+ return -EINVAL;
+ }
+
+ layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
+ layers[0].size = SDRAM_NUM_CS;
+ layers[0].is_virt_csrow = true;
+
+ mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*drvdata));
+ if (!mci)
+ return -ENOMEM;
+
+ drvdata = mci->pvt_info;
+ drvdata->base = base;
+ mci->pdev = &pdev->dev;
+ platform_set_drvdata(pdev, mci);
+
+ id = of_match_device(armada_xp_mc_edac_of_match, &pdev->dev);
+ mci->edac_check = armada_xp_mc_edac_check;
+ mci->mtype_cap = MEM_FLAG_DDR3;
+ mci->edac_cap = EDAC_FLAG_SECDED;
+ mci->mod_name = pdev->dev.driver->name;
+ mci->ctl_name = id ? id->compatible : "unknown";
+ mci->dev_name = dev_name(&pdev->dev);
+ mci->scrub_mode = SCRUB_NONE;
+
+ armada_xp_mc_edac_read_config(mci);
+
+ /* configure SBE threshold */
+ /* it seems that SBEs are not captured otherwise */
+ writel(1 << SDRAM_ERR_CTRL_ERR_THR_OFFSET,
+ drvdata->base + SDRAM_ERR_CTRL_REG);
+
+ /* clear cause registers */
+ writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
+ drvdata->base + SDRAM_ERR_CAUSE_ERR_REG);
+ writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
+ drvdata->base + SDRAM_ERR_CAUSE_MSG_REG);
+
+ /* clear counter registers */
+ writel(0, drvdata->base + SDRAM_ERR_SBE_COUNT_REG);
+ writel(0, drvdata->base + SDRAM_ERR_DBE_COUNT_REG);
+
+ if (edac_mc_add_mc(mci)) {
+ edac_mc_free(mci);
+ return -EINVAL;
+ }
+ edac_op_state = EDAC_OPSTATE_POLL;
+
+ return 0;
+}
+
+static int armada_xp_mc_edac_remove(struct platform_device *pdev)
+{
+ struct mem_ctl_info *mci = platform_get_drvdata(pdev);
+
+ edac_mc_del_mc(&pdev->dev);
+ edac_mc_free(mci);
+ platform_set_drvdata(pdev, NULL);
+
+ return 0;
+}
+
+static struct platform_driver armada_xp_mc_edac_driver = {
+ .probe = armada_xp_mc_edac_probe,
+ .remove = armada_xp_mc_edac_remove,
+ .driver = {
+ .name = "armada_xp_mc_edac",
+ .of_match_table = of_match_ptr(armada_xp_mc_edac_of_match),
+ },
+};
+
+/************************ EDAC Device (L2 Cache) ***************************/
+
+struct aurora_l2_edac_drvdata {
+ void __iomem *base;
+
+ char msg[128];
+
+ /* error injection via debugfs */
+ uint32_t inject_addr;
+ uint32_t inject_mask;
+ uint8_t inject_ctl;
+
+ struct dentry *debugfs;
+};
+
+static void aurora_l2_edac_inject(struct aurora_l2_edac_drvdata *drvdata)
+{
+ drvdata->inject_addr &= AURORA_ERR_INJECT_CTL_ADDR_MASK;
+ drvdata->inject_ctl &= AURORA_ERR_INJECT_CTL_EN_MASK;
+ writel(0, drvdata->base + AURORA_ERR_INJECT_CTL_REG);
+ writel(drvdata->inject_mask,
+ drvdata->base + AURORA_ERR_INJECT_MASK_REG);
+ writel(drvdata->inject_addr | drvdata->inject_ctl,
+ drvdata->base + AURORA_ERR_INJECT_CTL_REG);
+}
+
+static void aurora_l2_edac_check(struct edac_device_ctl_info *dci)
+{
+ struct aurora_l2_edac_drvdata *drvdata = dci->pvt_info;
+ uint32_t cnt, attr_cap, addr_cap, way_cap;
+ unsigned int cnt_ce, cnt_ue;
+
+ cnt = readl(drvdata->base + AURORA_ERR_CNT_REG);
+ attr_cap = readl(drvdata->base + AURORA_ERR_ATTR_CAP_REG);
+ addr_cap = readl(drvdata->base + AURORA_ERR_ADDR_CAP_REG);
+ way_cap = readl(drvdata->base + AURORA_ERR_WAY_CAP_REG);
+
+ cnt_ce = (cnt & AURORA_ERR_CNT_CE_MASK) >> AURORA_ERR_CNT_CE_OFFSET;
+ cnt_ue = (cnt & AURORA_ERR_CNT_UE_MASK) >> AURORA_ERR_CNT_UE_OFFSET;
+ /* clear error counter registers */
+ if (cnt_ce || cnt_ue)
+ writel(AURORA_ERR_CNT_CLR, drvdata->base + AURORA_ERR_CNT_REG);
+
+ if (attr_cap & AURORA_ERR_ATTR_CAP_VALID) {
+ char *msg = drvdata->msg;
+ size_t size = sizeof(drvdata->msg);
+ size_t len = 0;
+
+ switch ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_SOURCE_MASK)
+ >> AURORA_ERR_ATTR_CAP_ERR_SOURCE_OFFSET) {
+ case 0:
+ len += snprintf(msg+len, size-len, "src=CPU0 ");
+ break;
+ case 1:
+ len += snprintf(msg+len, size-len, "src=CPU1 ");
+ break;
+ case 2:
+ len += snprintf(msg+len, size-len, "src=CPU2 ");
+ break;
+ case 3:
+ len += snprintf(msg+len, size-len, "src=CPU3 ");
+ break;
+ case 7:
+ len += snprintf(msg+len, size-len, "src=IO ");
+ break;
+ }
+ switch ((attr_cap & AURORA_ERR_ATTR_CAP_TRANS_TYPE_MASK)
+ >> AURORA_ERR_ATTR_CAP_TRANS_TYPE_OFFSET) {
+ case 0:
+ len += snprintf(msg+len, size-len, "txn=Data-Read ");
+ break;
+ case 1:
+ len += snprintf(msg+len, size-len, "txn=Isn-Read ");
+ break;
+ case 2:
+ len += snprintf(msg+len, size-len, "txn=Clean-Flush ");
+ break;
+ case 3:
+ len += snprintf(msg+len, size-len, "txn=Eviction ");
+ break;
+ case 4:
+ len += snprintf(msg+len, size-len,
+ "txn=Read-Modify-Write ");
+ break;
+ }
+ switch ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_TYPE_MASK)
+ >> AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET) {
+ case 0:
+ len += snprintf(msg+len, size-len, "err=CorrECC ");
+ break;
+ case 1:
+ len += snprintf(msg+len, size-len, "err=UnCorrECC ");
+ break;
+ case 2:
+ len += snprintf(msg+len, size-len, "err=TagParity ");
+ break;
+ }
+ len += snprintf(msg+len, size-len, "addr=0x%x ",
+ addr_cap & AURORA_ERR_ADDR_CAP_ADDR_MASK);
+ len += snprintf(msg+len, size-len, "index=0x%x ",
+ (way_cap & AURORA_ERR_WAY_CAP_INDEX_MASK)
+ >> AURORA_ERR_WAY_CAP_INDEX_OFFSET);
+ len += snprintf(msg+len, size-len, "way=0x%x",
+ (way_cap & AURORA_ERR_WAY_CAP_WAY_MASK)
+ >> AURORA_ERR_WAY_CAP_WAY_OFFSET);
+ /* clear error capture registers */
+ writel(AURORA_ERR_ATTR_CAP_VALID,
+ drvdata->base + AURORA_ERR_ATTR_CAP_REG);
+ if ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_TYPE_MASK)
+ >> AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET) {
+ /* UnCorrECC or TagParity */
+ if (cnt_ue)
+ cnt_ue--;
+ edac_device_handle_ue(dci, 0, 0, drvdata->msg);
+ } else {
+ if (cnt_ce)
+ cnt_ce--;
+ edac_device_handle_ce(dci, 0, 0, drvdata->msg);
+ }
+ }
+
+ /* report remaining errors */
+ while (cnt_ue--)
+ edac_device_handle_ue(dci, 0, 0,
+ "details unavailable (multiple errors)");
+ while (cnt_ce--)
+ edac_device_handle_ue(dci, 0, 0,
+ "details unavailable (multiple errors)");
+}
+
+static void aurora_l2_edac_poll(struct edac_device_ctl_info *dci)
+{
+ struct aurora_l2_edac_drvdata *drvdata = dci->pvt_info;
+
+ aurora_l2_edac_check(dci);
+ aurora_l2_edac_inject(drvdata);
+}
+
+static const struct of_device_id aurora_l2_edac_of_match[] = {
+ {.compatible = "marvell,aurora-system-cache",},
+ {},
+};
+MODULE_DEVICE_TABLE(of, aurora_l2_edac_of_match);
+
+static int aurora_l2_edac_probe(struct platform_device *pdev)
+{
+ const struct of_device_id *id;
+ struct edac_device_ctl_info *dci;
+ struct aurora_l2_edac_drvdata *drvdata;
+ struct resource *r;
+ void __iomem *base;
+ uint32_t l2x0_aux_ctrl;
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!r) {
+ dev_err(&pdev->dev, "Unable to get mem resource\n");
+ return -ENODEV;
+ }
+
+ base = devm_ioremap_resource(&pdev->dev, r);
+ if (IS_ERR(base)) {
+ dev_err(&pdev->dev, "Unable to map regs\n");
+ return PTR_ERR(base);
+ }
+
+ l2x0_aux_ctrl = readl(base + L2X0_AUX_CTRL);
+ if (!(l2x0_aux_ctrl & AURORA_ACR_PARITY_EN))
+ dev_warn(&pdev->dev, "tag parity is not enabled");
+ if (!(l2x0_aux_ctrl & AURORA_ACR_ECC_EN))
+ dev_warn(&pdev->dev, "data ECC is not enabled");
+
+ dci = edac_device_alloc_ctl_info(sizeof(*drvdata),
+ "cpu", 1, "L", 1, 2, NULL, 0, 0);
+ if (!dci)
+ return -ENOMEM;
+
+ drvdata = dci->pvt_info;
+ drvdata->base = base;
+ dci->dev = &pdev->dev;
+ platform_set_drvdata(pdev, dci);
+
+ id = of_match_device(aurora_l2_edac_of_match, &pdev->dev);
+ dci->edac_check = aurora_l2_edac_poll;
+ dci->mod_name = pdev->dev.driver->name;
+ dci->ctl_name = id ? id->compatible : "unknown";
+ dci->dev_name = dev_name(&pdev->dev);
+
+ /* clear registers */
+ writel(AURORA_ERR_CNT_CLR, drvdata->base + AURORA_ERR_CNT_REG);
+ writel(AURORA_ERR_ATTR_CAP_VALID,
+ drvdata->base + AURORA_ERR_ATTR_CAP_REG);
+
+ if (edac_device_add_device(dci)) {
+ edac_device_free_ctl_info(dci);
+ return -EINVAL;
+ }
+
+ drvdata->debugfs = edac_debugfs_create_dir(dev_name(&pdev->dev));
+ if (drvdata->debugfs) {
+ edac_debugfs_create_x32("inject_addr", 0644,
+ drvdata->debugfs,
+ &drvdata->inject_addr);
+ edac_debugfs_create_x32("inject_mask", 0644,
+ drvdata->debugfs,
+ &drvdata->inject_mask);
+ edac_debugfs_create_x8("inject_ctl", 0644,
+ drvdata->debugfs, &drvdata->inject_ctl);
+ }
+
+ return 0;
+}
+
+static int aurora_l2_edac_remove(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
+ struct aurora_l2_edac_drvdata *drvdata = dci->pvt_info;
+
+ edac_debugfs_remove_recursive(drvdata->debugfs);
+ edac_device_del_device(&pdev->dev);
+ edac_device_free_ctl_info(dci);
+ platform_set_drvdata(pdev, NULL);
+
+ return 0;
+}
+
+static struct platform_driver aurora_l2_edac_driver = {
+ .probe = aurora_l2_edac_probe,
+ .remove = aurora_l2_edac_remove,
+ .driver = {
+ .name = "aurora_l2_edac",
+ .of_match_table = of_match_ptr(aurora_l2_edac_of_match),
+ },
+};
+
+/************************ Driver registration ******************************/
+
+static struct platform_driver * const drivers[] = {
+ &armada_xp_mc_edac_driver,
+ &aurora_l2_edac_driver,
+};
+
+static int __init armada_xp_edac_init(void)
+{
+ int res = 0;
+
+ /* only polling is supported */
+ edac_op_state = EDAC_OPSTATE_POLL;
+
+ res = platform_register_drivers(drivers, ARRAY_SIZE(drivers));
+ if (res)
+ pr_warn("Aramda XP EDAC drivers fail to register\n");
+
+ return 0;
+}
+module_init(armada_xp_edac_init);
+
+static void __exit armada_xp_edac_exit(void)
+{
+ platform_unregister_drivers(drivers, ARRAY_SIZE(drivers));
+}
+module_exit(armada_xp_edac_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Pengutronix");
+MODULE_DESCRIPTION("EDAC Drivers for Marvell Armada XP SDRAM and L2 Cache Controller");
--
2.11.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [v3,7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
2017-11-10 9:03 ` [PATCH v3 7/7] " Jan Luebbe
@ 2017-11-12 20:15 ` Chris Packham
-1 siblings, 0 replies; 35+ messages in thread
From: Chris Packham @ 2017-11-12 20:15 UTC (permalink / raw)
To: Jan Luebbe, Russell King - ARM Linux, Borislav Petkov,
linux-arm-kernel, linux-edac
Cc: kernel, Thomas Petazzoni, Gregory CLEMENT
On 10/11/17 22:03, Jan Luebbe wrote:
> +static const struct of_device_id armada_xp_mc_edac_of_match[] = {
> + {.compatible = "marvell,armada-xp-sdram-controller",},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, armada_xp_mc_edac_of_match);
FYI I've also found that Armada-370 and Armada-38x will be compatible
with this driver (as well as the 98dx3236 I mentioned on the initial
series). Again the difference is that all of these have a 32-bit DDR
interface. Also in the interim I've got my hands on a custom Armada-38x
board with ECC.
Once this lands I'll re-send my follow up patches. To add support for
these SoCs.
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
@ 2017-11-12 20:15 ` Chris Packham
0 siblings, 0 replies; 35+ messages in thread
From: Chris Packham @ 2017-11-12 20:15 UTC (permalink / raw)
To: linux-arm-kernel
On 10/11/17 22:03, Jan Luebbe wrote:
> +static const struct of_device_id armada_xp_mc_edac_of_match[] = {
> + {.compatible = "marvell,armada-xp-sdram-controller",},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, armada_xp_mc_edac_of_match);
FYI I've also found that Armada-370 and Armada-38x will be compatible
with this driver (as well as the 98dx3236 I mentioned on the initial
series). Again the difference is that all of these have a 32-bit DDR
interface. Also in the interim I've got my hands on a custom Armada-38x
board with ECC.
Once this lands I'll re-send my follow up patches. To add support for
these SoCs.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [v3,7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
2017-11-10 9:03 ` [PATCH v3 7/7] " Jan Luebbe
@ 2017-11-16 19:31 ` Borislav Petkov
-1 siblings, 0 replies; 35+ messages in thread
From: Borislav Petkov @ 2017-11-16 19:31 UTC (permalink / raw)
To: Jan Luebbe
Cc: Russell King - ARM Linux, Chris Packham, linux-arm-kernel,
linux-edac, kernel, Thomas Petazzoni, Gregory CLEMENT
On Fri, Nov 10, 2017 at 10:03:08AM +0100, Jan Luebbe wrote:
> Add support for the ECC functionality as found in the DDR RAM and L2
> cache controllers on the MV78230/MV78x60 SoCs. This driver has been
> tested on the MV78460 (on a custom board with a DDR3 ECC DIMM).
>
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
...
> diff --git a/drivers/edac/armada_xp_edac.c b/drivers/edac/armada_xp_edac.c
> new file mode 100644
> index 000000000000..cb9173b30aa9
> --- /dev/null
> +++ b/drivers/edac/armada_xp_edac.c
> @@ -0,0 +1,658 @@
> +/*
> + * Copyright (C) 2017 Pengutronix, Jan Luebbe <kernel@pengutronix.de>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/edac.h>
> +#include <linux/of_platform.h>
> +
> +#include <asm/hardware/cache-l2x0.h>
I get this when I cross-build it:
drivers/edac/armada_xp_edac.c:19:37: fatal error: asm/hardware/cache-l2x0.h: No such file or directory
#include <asm/hardware/cache-l2x0.h>
^
compilation terminated.
make[2]: *** [drivers/edac/armada_xp_edac.o] Error 1
make[1]: *** [drivers/edac] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [drivers] Error 2
and that driver depends on ARCH_MVEBU which is arm64, AFAICT.
That header is
arch/arm/include/asm/hardware/cache-l2x0.h
however and not in arch/arm64/.
I could very well be missing something though...
> +#include <asm/hardware/cache-aurora-l2.h>
> +
> +#include "edac_mc.h"
> +#include "edac_device.h"
> +#include "edac_module.h"
> +
> +/************************ EDAC MC (DDR RAM) ********************************/
> +
> +#define SDRAM_NUM_CS 4
> +
> +#define SDRAM_CONFIG_REG 0x0
> +#define SDRAM_CONFIG_ECC_MASK BIT(18)
> +#define SDRAM_CONFIG_REGISTERED_MASK BIT(17)
> +#define SDRAM_CONFIG_BUS_WIDTH_MASK BIT(15)
> +
> +#define SDRAM_ADDR_CTRL_REG 0x10
> +#define SDRAM_ADDR_CTRL_SIZE_HIGH_OFFSET(cs) (20+cs)
> +#define SDRAM_ADDR_CTRL_SIZE_HIGH_MASK(cs) \
> + (0x1 << SDRAM_ADDR_CTRL_SIZE_HIGH_OFFSET(cs))
> +#define SDRAM_ADDR_CTRL_ADDR_SEL_MASK(cs) BIT(16+cs)
> +#define SDRAM_ADDR_CTRL_SIZE_LOW_OFFSET(cs) (cs*4+2)
> +#define SDRAM_ADDR_CTRL_SIZE_LOW_MASK(cs) \
> + (0x3 << SDRAM_ADDR_CTRL_SIZE_LOW_OFFSET(cs))
> +#define SDRAM_ADDR_CTRL_STRUCT_OFFSET(cs) (cs*4)
> +#define SDRAM_ADDR_CTRL_STRUCT_MASK(cs) \
> + (0x3 << SDRAM_ADDR_CTRL_STRUCT_OFFSET(cs))
> +
> +#define SDRAM_ERR_DATA_H_REG 0x40
> +#define SDRAM_ERR_DATA_L_REG 0x44
> +
> +#define SDRAM_ERR_RECV_ECC_REG 0x48
> +#define SDRAM_ERR_RECV_ECC_VALUE_MASK 0xff
> +
> +#define SDRAM_ERR_CALC_ECC_REG 0x4c
> +#define SDRAM_ERR_CALC_ECC_ROW_OFFSET 8
> +#define SDRAM_ERR_CALC_ECC_ROW_MASK \
> + (0xffff << SDRAM_ERR_CALC_ECC_ROW_OFFSET)
> +#define SDRAM_ERR_CALC_ECC_VALUE_MASK 0xff
> +
> +#define SDRAM_ERR_ADDR_REG 0x50
> +#define SDRAM_ERR_ADDR_BANK_OFFSET 23
> +#define SDRAM_ERR_ADDR_BANK_MASK \
> + (0x7 << SDRAM_ERR_ADDR_BANK_OFFSET)
> +#define SDRAM_ERR_ADDR_COL_OFFSET 8
> +#define SDRAM_ERR_ADDR_COL_MASK \
> + (0x7fff << SDRAM_ERR_ADDR_COL_OFFSET)
> +#define SDRAM_ERR_ADDR_CS_OFFSET 1
> +#define SDRAM_ERR_ADDR_CS_MASK \
> + (0x3 << SDRAM_ERR_ADDR_CS_OFFSET)
> +#define SDRAM_ERR_ADDR_TYPE_MASK BIT(0)
> +
> +#define SDRAM_ERR_CTRL_REG 0x54
> +#define SDRAM_ERR_CTRL_ERR_THR_OFFSET 16
> +#define SDRAM_ERR_CTRL_ERR_THR_MASK \
> + (0xff << SDRAM_ERR_CTRL_ERR_THR_OFFSET)
> +#define SDRAM_ERR_CTRL_ERR_PROP_MASK BIT(9)
> +
> +#define SDRAM_ERR_SBE_COUNT_REG 0x58
> +#define SDRAM_ERR_DBE_COUNT_REG 0x5c
> +
> +#define SDRAM_ERR_CAUSE_ERR_REG 0xd0
> +#define SDRAM_ERR_CAUSE_MSG_REG 0xd8
> +#define SDRAM_ERR_CAUSE_DBE_MASK BIT(1)
> +#define SDRAM_ERR_CAUSE_SBE_MASK BIT(0)
> +
> +#define SDRAM_RANK_CTRL_REG 0x1e0
> +#define SDRAM_RANK_CTRL_EXIST_MASK(cs) BIT(cs)
Align those define names and values vertically. They can stick out and
overflow the 80 cols rule:
#define SDRAM_ERR_CTRL_ERR_THR_OFFSET 16
#define SDRAM_ERR_CTRL_ERR_THR_MASK (0xff << SDRAM_ERR_CTRL_ERR_THR_OFFSET)
#define SDRAM_ERR_CTRL_ERR_PROP_MASK BIT(9)
also, you can shorten those defines too - there's "ERR" twice in the
name.
> +
> +struct armada_xp_mc_edac_drvdata {
That struct name is too long. So long that it makes you break parameter
names below.
For example, you don't need that lenghty prefix "armada_xp_mc_edac"
prepended to static symbols. Same for the function names below.
> + void __iomem *base;
> +
> + unsigned int width; /* width in bytes */
> + bool cs_addr_sel[SDRAM_NUM_CS]; /* bank interleaving */
Put all side comments over the respective line they're commenting.
> +
> + char msg[128];
> +};
> +
> +/* derived from "DRAM Address Multiplexing" in the ARAMDA XP Functional Spec */
> +static uint32_t armada_xp_mc_edac_calc_address(struct armada_xp_mc_edac_drvdata
> + *drvdata, uint8_t cs,
> + uint8_t bank, uint16_t row,
> + uint16_t col)
> +{
> + if (drvdata->width == 8) { /* 64 bit */
> + if (drvdata->cs_addr_sel[cs]) /* bank interleaved */
> + return (((row & 0xfff8) << 16) |
> + ((bank & 0x7) << 16) |
> + ((row & 0x7) << 13) |
> + ((col & 0x3ff) << 3));
> + else
> + return (((row & 0xffff << 16) |
> + ((bank & 0x7) << 13) |
> + ((col & 0x3ff)) << 3));
> + } else if (drvdata->width == 4) { /* 32 bit */
> + if (drvdata->cs_addr_sel[cs]) /* bank interleaved */
> + return (((row & 0xfff0) << 15) |
> + ((bank & 0x7) << 16) |
> + ((row & 0xf) << 12) |
> + ((col & 0x3ff) << 2));
> + else
> + return (((row & 0xffff << 15) |
> + ((bank & 0x7) << 12) |
> + ((col & 0x3ff)) << 2));
> + } else { /* 16 bit */
> + if (drvdata->cs_addr_sel[cs]) /* bank interleaved */
> + return (((row & 0xffe0) << 14) |
> + ((bank & 0x7) << 16) |
> + ((row & 0x1f) << 11) |
> + ((col & 0x3ff) << 1));
> + else
> + return (((row & 0xffff << 14) |
> + ((bank & 0x7) << 11) |
> + ((col & 0x3ff)) << 1));
> + }
> +}
> +
> +static void armada_xp_mc_edac_check(struct mem_ctl_info *mci)
> +{
> + struct armada_xp_mc_edac_drvdata *drvdata = mci->pvt_info;
> + uint32_t data_h, data_l, recv_ecc, calc_ecc, addr;
> + uint32_t cnt_sbe, cnt_dbe, cause_err, cause_msg;
> + uint32_t row_val, col_val, bank_val, addr_val;
> + uint8_t syndrome_val, cs_val;
> + char *msg = drvdata->msg;
> +
> + data_h = readl(drvdata->base + SDRAM_ERR_DATA_H_REG);
> + data_l = readl(drvdata->base + SDRAM_ERR_DATA_L_REG);
> + recv_ecc = readl(drvdata->base + SDRAM_ERR_RECV_ECC_REG);
> + calc_ecc = readl(drvdata->base + SDRAM_ERR_CALC_ECC_REG);
> + addr = readl(drvdata->base + SDRAM_ERR_ADDR_REG);
> + cnt_sbe = readl(drvdata->base + SDRAM_ERR_SBE_COUNT_REG);
> + cnt_dbe = readl(drvdata->base + SDRAM_ERR_DBE_COUNT_REG);
> + cause_err = readl(drvdata->base + SDRAM_ERR_CAUSE_ERR_REG);
> + cause_msg = readl(drvdata->base + SDRAM_ERR_CAUSE_MSG_REG);
Align vertically for better readability:
data_h = readl(drvdata->base + SDRAM_ERR_DATA_H_REG);
data_l = readl(drvdata->base + SDRAM_ERR_DATA_L_REG);
recv_ecc = readl(drvdata->base + SDRAM_ERR_RECV_ECC_REG);
calc_ecc = readl(drvdata->base + SDRAM_ERR_CALC_ECC_REG);
...
> +
> + /* clear cause registers */
> + writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
> + drvdata->base + SDRAM_ERR_CAUSE_ERR_REG);
> + writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
> + drvdata->base + SDRAM_ERR_CAUSE_MSG_REG);
> +
> + /* clear error counter registers */
> + if (cnt_sbe)
> + writel(0, drvdata->base + SDRAM_ERR_SBE_COUNT_REG);
> + if (cnt_dbe)
> + writel(0, drvdata->base + SDRAM_ERR_DBE_COUNT_REG);
> +
> + if (!cnt_sbe && !cnt_dbe)
> + return;
> +
> + if ((addr & SDRAM_ERR_ADDR_TYPE_MASK) == 0) {
if (!(addr & SDRAM_ERR_ADDR_TYPE_MASK)) {
> + if (cnt_sbe)
> + cnt_sbe--;
> + else
> + dev_warn(mci->pdev, "inconsistent SBE count detected");
> + } else {
> + if (cnt_dbe)
> + cnt_dbe--;
> + else
> + dev_warn(mci->pdev, "inconsistent DBE count detected");
> + }
> +
> + /* report earlier errors */
> + if (cnt_sbe)
> + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
> + cnt_sbe, /* error count */
> + 0, 0, 0, /* pfn, offset, syndrome */
> + -1, -1, -1, /* top, mid, low layer */
> + mci->ctl_name,
> + "details unavailable (multiple errors)");
> + if (cnt_dbe)
> + edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
> + cnt_sbe, /* error count */
> + 0, 0, 0, /* pfn, offset, syndrome */
> + -1, -1, -1, /* top, mid, low layer */
> + mci->ctl_name,
> + "details unavailable (multiple errors)");
> +
> + /* report details for most recent error */
> + cs_val = (addr & SDRAM_ERR_ADDR_CS_MASK)
> + >> SDRAM_ERR_ADDR_CS_OFFSET;
> + bank_val = (addr & SDRAM_ERR_ADDR_BANK_MASK)
> + >> SDRAM_ERR_ADDR_BANK_OFFSET;
> + row_val = (calc_ecc & SDRAM_ERR_CALC_ECC_ROW_MASK)
> + >> SDRAM_ERR_CALC_ECC_ROW_OFFSET;
> + col_val = (addr & SDRAM_ERR_ADDR_COL_MASK)
> + >> SDRAM_ERR_ADDR_COL_OFFSET;
Let those stick out and align vertically.
> + syndrome_val = (recv_ecc ^ calc_ecc) & 0xff;
> + addr_val = armada_xp_mc_edac_calc_address(drvdata, cs_val, bank_val,
> + row_val, col_val);
> + msg += sprintf(msg, "row=0x%04x ", row_val); /* 11 chars */
> + msg += sprintf(msg, "bank=0x%x ", bank_val); /* 9 chars */
> + msg += sprintf(msg, "col=0x%04x ", col_val); /* 11 chars */
> + msg += sprintf(msg, "cs=%d", cs_val); /* 4 chars */
> +
> + if ((addr & SDRAM_ERR_ADDR_TYPE_MASK) == 0) {
if (!(...
as above.
> + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
> + 1, /* error count */
> + addr_val >> PAGE_SHIFT,
> + addr_val & ~PAGE_MASK,
> + syndrome_val,
> + cs_val, -1, -1, /* top, mid, low layer */
> + mci->ctl_name, drvdata->msg);
> + } else {
> + edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
> + 1, /* error count */
> + addr_val >> PAGE_SHIFT,
> + addr_val & ~PAGE_MASK,
> + syndrome_val,
> + cs_val, -1, -1, /* top, mid, low layer */
> + mci->ctl_name, drvdata->msg);
> + }
> +}
> +
> +static void armada_xp_mc_edac_read_config(struct mem_ctl_info *mci)
> +{
> + struct armada_xp_mc_edac_drvdata *drvdata = mci->pvt_info;
> + struct dimm_info *dimm;
> + unsigned int i, cs_struct, cs_size;
> + uint32_t config, addr_ctrl, rank_ctrl;
Please sort function local variables declaration in a reverse christmas
tree order:
<type> longest_variable_name;
<type> shorter_var_name;
<type> even_shorter;
<type> i;
> +
> + config = readl(drvdata->base + SDRAM_CONFIG_REG);
> + if (config & SDRAM_CONFIG_BUS_WIDTH_MASK)
> + drvdata->width = 8; /* 64 bit */
> + else
> + drvdata->width = 4; /* 32 bit */
> +
> + addr_ctrl = readl(drvdata->base + SDRAM_ADDR_CTRL_REG);
> + rank_ctrl = readl(drvdata->base + SDRAM_RANK_CTRL_REG);
> + for (i = 0; i < SDRAM_NUM_CS; i++) {
> + dimm = mci->dimms[i];
> +
> + if (!(rank_ctrl & SDRAM_RANK_CTRL_EXIST_MASK(i)))
> + continue;
> +
> + drvdata->cs_addr_sel[i] =
> + !!(addr_ctrl & SDRAM_ADDR_CTRL_ADDR_SEL_MASK(i));
> +
> + cs_struct = (addr_ctrl & SDRAM_ADDR_CTRL_STRUCT_MASK(i)) >>
> + SDRAM_ADDR_CTRL_STRUCT_OFFSET(i);
> + cs_size = ((addr_ctrl & SDRAM_ADDR_CTRL_SIZE_HIGH_MASK(i)) >>
> + (SDRAM_ADDR_CTRL_SIZE_HIGH_OFFSET(i) - 2) |
> + ((addr_ctrl & SDRAM_ADDR_CTRL_SIZE_LOW_MASK(i)) >>
> + SDRAM_ADDR_CTRL_SIZE_LOW_OFFSET(i)));
Align vertically and let them stick out. As they are now, they're almost
unreadable.
> + switch (cs_size) {
> + case 0: /* 2GBit */
> + dimm->nr_pages = (0x80000000ULL >> PAGE_SHIFT);
> + break;
> + case 1: /* 256MBit */
> + dimm->nr_pages = (0x10000000ULL >> PAGE_SHIFT);
> + break;
> + case 2: /* 512MBit */
> + dimm->nr_pages = (0x20000000ULL >> PAGE_SHIFT);
> + break;
> + case 3: /* 1GBit */
> + dimm->nr_pages = (0x40000000ULL >> PAGE_SHIFT);
> + break;
> + case 4: /* 4GBit */
> + dimm->nr_pages = (0x100000000ULL >> PAGE_SHIFT);
> + break;
> + case 5: /* 8GBit */
> + dimm->nr_pages = (0x200000000ULL >> PAGE_SHIFT);
That's silly. Just write the pages in decimal numbers. You have the
comments explaining what those numbers are.
> + break;
> + }
> + dimm->grain = 8;
> + dimm->dtype = cs_struct ? DEV_X16 : DEV_X8;
> + dimm->mtype = (config & SDRAM_CONFIG_REGISTERED_MASK) ?
> + MEM_RDDR3 : MEM_DDR3;
> + dimm->edac_mode = EDAC_SECDED;
> + }
> +}
> +
> +static const struct of_device_id armada_xp_mc_edac_of_match[] = {
> + {.compatible = "marvell,armada-xp-sdram-controller",},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, armada_xp_mc_edac_of_match);
> +
> +static int armada_xp_mc_edac_probe(struct platform_device *pdev)
> +{
> + const struct of_device_id *id;
> + struct mem_ctl_info *mci;
> + struct edac_mc_layer layers[1];
> + struct armada_xp_mc_edac_drvdata *drvdata;
> + struct resource *r;
> + void __iomem *base;
> + uint32_t config;
Please sort function local variables declaration in a reverse christmas
tree order:
<type> longest_variable_name;
<type> shorter_var_name;
<type> even_shorter;
<type> i;
> +
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!r) {
> + dev_err(&pdev->dev, "Unable to get mem resource\n");
> + return -ENODEV;
> + }
> +
> + base = devm_ioremap_resource(&pdev->dev, r);
> + if (IS_ERR(base)) {
> + dev_err(&pdev->dev, "Unable to map regs\n");
> + return PTR_ERR(base);
> + }
> +
> + config = readl(base + SDRAM_CONFIG_REG);
> + if (!(config & SDRAM_CONFIG_ECC_MASK)) {
> + dev_warn(&pdev->dev, "SDRAM ECC is not enabled");
> + return -EINVAL;
> + }
> +
> + layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
> + layers[0].size = SDRAM_NUM_CS;
> + layers[0].is_virt_csrow = true;
> +
> + mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*drvdata));
> + if (!mci)
> + return -ENOMEM;
> +
> + drvdata = mci->pvt_info;
> + drvdata->base = base;
> + mci->pdev = &pdev->dev;
> + platform_set_drvdata(pdev, mci);
> +
> + id = of_match_device(armada_xp_mc_edac_of_match, &pdev->dev);
> + mci->edac_check = armada_xp_mc_edac_check;
> + mci->mtype_cap = MEM_FLAG_DDR3;
> + mci->edac_cap = EDAC_FLAG_SECDED;
> + mci->mod_name = pdev->dev.driver->name;
> + mci->ctl_name = id ? id->compatible : "unknown";
> + mci->dev_name = dev_name(&pdev->dev);
> + mci->scrub_mode = SCRUB_NONE;
> +
> + armada_xp_mc_edac_read_config(mci);
> +
> + /* configure SBE threshold */
> + /* it seems that SBEs are not captured otherwise */
> + writel(1 << SDRAM_ERR_CTRL_ERR_THR_OFFSET,
> + drvdata->base + SDRAM_ERR_CTRL_REG);
> +
> + /* clear cause registers */
> + writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
> + drvdata->base + SDRAM_ERR_CAUSE_ERR_REG);
> + writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
> + drvdata->base + SDRAM_ERR_CAUSE_MSG_REG);
Let those stick out and align vertically.
> +
> + /* clear counter registers */
> + writel(0, drvdata->base + SDRAM_ERR_SBE_COUNT_REG);
> + writel(0, drvdata->base + SDRAM_ERR_DBE_COUNT_REG);
> +
> + if (edac_mc_add_mc(mci)) {
> + edac_mc_free(mci);
> + return -EINVAL;
> + }
> + edac_op_state = EDAC_OPSTATE_POLL;
> +
> + return 0;
> +}
> +
> +static int armada_xp_mc_edac_remove(struct platform_device *pdev)
> +{
> + struct mem_ctl_info *mci = platform_get_drvdata(pdev);
> +
> + edac_mc_del_mc(&pdev->dev);
> + edac_mc_free(mci);
> + platform_set_drvdata(pdev, NULL);
> +
> + return 0;
> +}
> +
> +static struct platform_driver armada_xp_mc_edac_driver = {
> + .probe = armada_xp_mc_edac_probe,
> + .remove = armada_xp_mc_edac_remove,
> + .driver = {
> + .name = "armada_xp_mc_edac",
> + .of_match_table = of_match_ptr(armada_xp_mc_edac_of_match),
> + },
> +};
> +
> +/************************ EDAC Device (L2 Cache) ***************************/
> +
> +struct aurora_l2_edac_drvdata {
> + void __iomem *base;
> +
> + char msg[128];
> +
> + /* error injection via debugfs */
> + uint32_t inject_addr;
> + uint32_t inject_mask;
> + uint8_t inject_ctl;
> +
> + struct dentry *debugfs;
> +};
> +
> +static void aurora_l2_edac_inject(struct aurora_l2_edac_drvdata *drvdata)
The whole EDAC error injection functionality should be behind
CONFIG_EDAC_DEBUG. You don't want error injection capabilities exposed
on a production system.
> +{
> + drvdata->inject_addr &= AURORA_ERR_INJECT_CTL_ADDR_MASK;
> + drvdata->inject_ctl &= AURORA_ERR_INJECT_CTL_EN_MASK;
> + writel(0, drvdata->base + AURORA_ERR_INJECT_CTL_REG);
> + writel(drvdata->inject_mask,
> + drvdata->base + AURORA_ERR_INJECT_MASK_REG);
> + writel(drvdata->inject_addr | drvdata->inject_ctl,
> + drvdata->base + AURORA_ERR_INJECT_CTL_REG);
Let those stick out and align vertically.
> +}
> +
> +static void aurora_l2_edac_check(struct edac_device_ctl_info *dci)
> +{
> + struct aurora_l2_edac_drvdata *drvdata = dci->pvt_info;
> + uint32_t cnt, attr_cap, addr_cap, way_cap;
> + unsigned int cnt_ce, cnt_ue;
> +
> + cnt = readl(drvdata->base + AURORA_ERR_CNT_REG);
> + attr_cap = readl(drvdata->base + AURORA_ERR_ATTR_CAP_REG);
> + addr_cap = readl(drvdata->base + AURORA_ERR_ADDR_CAP_REG);
> + way_cap = readl(drvdata->base + AURORA_ERR_WAY_CAP_REG);
> +
> + cnt_ce = (cnt & AURORA_ERR_CNT_CE_MASK) >> AURORA_ERR_CNT_CE_OFFSET;
> + cnt_ue = (cnt & AURORA_ERR_CNT_UE_MASK) >> AURORA_ERR_CNT_UE_OFFSET;
> + /* clear error counter registers */
> + if (cnt_ce || cnt_ue)
> + writel(AURORA_ERR_CNT_CLR, drvdata->base + AURORA_ERR_CNT_REG);
> +
> + if (attr_cap & AURORA_ERR_ATTR_CAP_VALID) {
if (!(attr_cap & AURORA_ERR_ATTR_CAP_VALID))
goto clear_remaining;
saves you an indentation level.
> + char *msg = drvdata->msg;
> + size_t size = sizeof(drvdata->msg);
> + size_t len = 0;
> +
> + switch ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_SOURCE_MASK)
> + >> AURORA_ERR_ATTR_CAP_ERR_SOURCE_OFFSET) {
> + case 0:
> + len += snprintf(msg+len, size-len, "src=CPU0 ");
> + break;
> + case 1:
> + len += snprintf(msg+len, size-len, "src=CPU1 ");
> + break;
> + case 2:
> + len += snprintf(msg+len, size-len, "src=CPU2 ");
> + break;
> + case 3:
> + len += snprintf(msg+len, size-len, "src=CPU3 ");
> + break;
> + case 7:
> + len += snprintf(msg+len, size-len, "src=IO ");
> + break;
> + }
Lemme try to shorten that:
src = (attr_cap & AURORA_ERR_ATTR_SRC_MSK) >> AURORA_ERR_ATTR_SRC_OFF;
if (src <= 3)
len += snprintf(msg+len, size-len, "src=CPU%d ", src);
else
len += snprintf(msg+len, size-len, "src=IO ");
Yeah, those AURORA_* defines could be shorter.
> + switch ((attr_cap & AURORA_ERR_ATTR_CAP_TRANS_TYPE_MASK)
> + >> AURORA_ERR_ATTR_CAP_TRANS_TYPE_OFFSET) {
Ditto.
> + case 0:
> + len += snprintf(msg+len, size-len, "txn=Data-Read ");
> + break;
> + case 1:
> + len += snprintf(msg+len, size-len, "txn=Isn-Read ");
> + break;
> + case 2:
> + len += snprintf(msg+len, size-len, "txn=Clean-Flush ");
> + break;
> + case 3:
> + len += snprintf(msg+len, size-len, "txn=Eviction ");
> + break;
> + case 4:
> + len += snprintf(msg+len, size-len,
> + "txn=Read-Modify-Write ");
> + break;
> + }
> + switch ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_TYPE_MASK)
> + >> AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET) {
> + case 0:
> + len += snprintf(msg+len, size-len, "err=CorrECC ");
> + break;
> + case 1:
> + len += snprintf(msg+len, size-len, "err=UnCorrECC ");
> + break;
> + case 2:
> + len += snprintf(msg+len, size-len, "err=TagParity ");
> + break;
> + }
> + len += snprintf(msg+len, size-len, "addr=0x%x ",
> + addr_cap & AURORA_ERR_ADDR_CAP_ADDR_MASK);
> + len += snprintf(msg+len, size-len, "index=0x%x ",
> + (way_cap & AURORA_ERR_WAY_CAP_INDEX_MASK)
> + >> AURORA_ERR_WAY_CAP_INDEX_OFFSET);
> + len += snprintf(msg+len, size-len, "way=0x%x",
> + (way_cap & AURORA_ERR_WAY_CAP_WAY_MASK)
> + >> AURORA_ERR_WAY_CAP_WAY_OFFSET);
Let it stick out.
<---- newline here.
> + /* clear error capture registers */
> + writel(AURORA_ERR_ATTR_CAP_VALID,
> + drvdata->base + AURORA_ERR_ATTR_CAP_REG);
Let it stick out.
> + if ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_TYPE_MASK)
> + >> AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET) {
Ditto.
> + /* UnCorrECC or TagParity */
> + if (cnt_ue)
> + cnt_ue--;
> + edac_device_handle_ue(dci, 0, 0, drvdata->msg);
> + } else {
> + if (cnt_ce)
> + cnt_ce--;
> + edac_device_handle_ce(dci, 0, 0, drvdata->msg);
> + }
> + }
> +
> + /* report remaining errors */
> + while (cnt_ue--)
> + edac_device_handle_ue(dci, 0, 0,
> + "details unavailable (multiple errors)");
> + while (cnt_ce--)
> + edac_device_handle_ue(dci, 0, 0,
> + "details unavailable (multiple errors)");
Ditto. You get the idea...
> +}
> +
> +static void aurora_l2_edac_poll(struct edac_device_ctl_info *dci)
> +{
> + struct aurora_l2_edac_drvdata *drvdata = dci->pvt_info;
> +
> + aurora_l2_edac_check(dci);
> + aurora_l2_edac_inject(drvdata);
> +}
> +
> +static const struct of_device_id aurora_l2_edac_of_match[] = {
> + {.compatible = "marvell,aurora-system-cache",},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, aurora_l2_edac_of_match);
> +
> +static int aurora_l2_edac_probe(struct platform_device *pdev)
> +{
> + const struct of_device_id *id;
> + struct edac_device_ctl_info *dci;
> + struct aurora_l2_edac_drvdata *drvdata;
> + struct resource *r;
> + void __iomem *base;
> + uint32_t l2x0_aux_ctrl;
Please sort function local variables declaration in a reverse christmas
tree order:
<type> longest_variable_name;
<type> shorter_var_name;
<type> even_shorter;
<type> i;
> +
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!r) {
> + dev_err(&pdev->dev, "Unable to get mem resource\n");
> + return -ENODEV;
> + }
> +
> + base = devm_ioremap_resource(&pdev->dev, r);
> + if (IS_ERR(base)) {
> + dev_err(&pdev->dev, "Unable to map regs\n");
> + return PTR_ERR(base);
> + }
> +
> + l2x0_aux_ctrl = readl(base + L2X0_AUX_CTRL);
> + if (!(l2x0_aux_ctrl & AURORA_ACR_PARITY_EN))
> + dev_warn(&pdev->dev, "tag parity is not enabled");
> + if (!(l2x0_aux_ctrl & AURORA_ACR_ECC_EN))
> + dev_warn(&pdev->dev, "data ECC is not enabled");
> +
> + dci = edac_device_alloc_ctl_info(sizeof(*drvdata),
> + "cpu", 1, "L", 1, 2, NULL, 0, 0);
> + if (!dci)
> + return -ENOMEM;
> +
> + drvdata = dci->pvt_info;
> + drvdata->base = base;
> + dci->dev = &pdev->dev;
> + platform_set_drvdata(pdev, dci);
> +
> + id = of_match_device(aurora_l2_edac_of_match, &pdev->dev);
> + dci->edac_check = aurora_l2_edac_poll;
> + dci->mod_name = pdev->dev.driver->name;
> + dci->ctl_name = id ? id->compatible : "unknown";
> + dci->dev_name = dev_name(&pdev->dev);
> +
> + /* clear registers */
> + writel(AURORA_ERR_CNT_CLR, drvdata->base + AURORA_ERR_CNT_REG);
> + writel(AURORA_ERR_ATTR_CAP_VALID,
> + drvdata->base + AURORA_ERR_ATTR_CAP_REG);
> +
> + if (edac_device_add_device(dci)) {
> + edac_device_free_ctl_info(dci);
> + return -EINVAL;
> + }
> +
> + drvdata->debugfs = edac_debugfs_create_dir(dev_name(&pdev->dev));
> + if (drvdata->debugfs) {
> + edac_debugfs_create_x32("inject_addr", 0644,
> + drvdata->debugfs,
> + &drvdata->inject_addr);
> + edac_debugfs_create_x32("inject_mask", 0644,
> + drvdata->debugfs,
> + &drvdata->inject_mask);
> + edac_debugfs_create_x8("inject_ctl", 0644,
> + drvdata->debugfs, &drvdata->inject_ctl);
> + }
> +
> + return 0;
> +}
> +
> +static int aurora_l2_edac_remove(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
> + struct aurora_l2_edac_drvdata *drvdata = dci->pvt_info;
> +
> + edac_debugfs_remove_recursive(drvdata->debugfs);
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
> + platform_set_drvdata(pdev, NULL);
> +
> + return 0;
> +}
> +
> +static struct platform_driver aurora_l2_edac_driver = {
> + .probe = aurora_l2_edac_probe,
> + .remove = aurora_l2_edac_remove,
> + .driver = {
> + .name = "aurora_l2_edac",
> + .of_match_table = of_match_ptr(aurora_l2_edac_of_match),
> + },
> +};
> +
> +/************************ Driver registration ******************************/
> +
> +static struct platform_driver * const drivers[] = {
> + &armada_xp_mc_edac_driver,
> + &aurora_l2_edac_driver,
> +};
> +
> +static int __init armada_xp_edac_init(void)
> +{
> + int res = 0;
No need to zero it.
> +
> + /* only polling is supported */
> + edac_op_state = EDAC_OPSTATE_POLL;
> +
> + res = platform_register_drivers(drivers, ARRAY_SIZE(drivers));
> + if (res)
> + pr_warn("Aramda XP EDAC drivers fail to register\n");
> +
> + return 0;
> +}
> +module_init(armada_xp_edac_init);
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
@ 2017-11-16 19:31 ` Borislav Petkov
0 siblings, 0 replies; 35+ messages in thread
From: Borislav Petkov @ 2017-11-16 19:31 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Nov 10, 2017 at 10:03:08AM +0100, Jan Luebbe wrote:
> Add support for the ECC functionality as found in the DDR RAM and L2
> cache controllers on the MV78230/MV78x60 SoCs. This driver has been
> tested on the MV78460 (on a custom board with a DDR3 ECC DIMM).
>
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
...
> diff --git a/drivers/edac/armada_xp_edac.c b/drivers/edac/armada_xp_edac.c
> new file mode 100644
> index 000000000000..cb9173b30aa9
> --- /dev/null
> +++ b/drivers/edac/armada_xp_edac.c
> @@ -0,0 +1,658 @@
> +/*
> + * Copyright (C) 2017 Pengutronix, Jan Luebbe <kernel@pengutronix.de>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/edac.h>
> +#include <linux/of_platform.h>
> +
> +#include <asm/hardware/cache-l2x0.h>
I get this when I cross-build it:
drivers/edac/armada_xp_edac.c:19:37: fatal error: asm/hardware/cache-l2x0.h: No such file or directory
#include <asm/hardware/cache-l2x0.h>
^
compilation terminated.
make[2]: *** [drivers/edac/armada_xp_edac.o] Error 1
make[1]: *** [drivers/edac] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [drivers] Error 2
and that driver depends on ARCH_MVEBU which is arm64, AFAICT.
That header is
arch/arm/include/asm/hardware/cache-l2x0.h
however and not in arch/arm64/.
I could very well be missing something though...
> +#include <asm/hardware/cache-aurora-l2.h>
> +
> +#include "edac_mc.h"
> +#include "edac_device.h"
> +#include "edac_module.h"
> +
> +/************************ EDAC MC (DDR RAM) ********************************/
> +
> +#define SDRAM_NUM_CS 4
> +
> +#define SDRAM_CONFIG_REG 0x0
> +#define SDRAM_CONFIG_ECC_MASK BIT(18)
> +#define SDRAM_CONFIG_REGISTERED_MASK BIT(17)
> +#define SDRAM_CONFIG_BUS_WIDTH_MASK BIT(15)
> +
> +#define SDRAM_ADDR_CTRL_REG 0x10
> +#define SDRAM_ADDR_CTRL_SIZE_HIGH_OFFSET(cs) (20+cs)
> +#define SDRAM_ADDR_CTRL_SIZE_HIGH_MASK(cs) \
> + (0x1 << SDRAM_ADDR_CTRL_SIZE_HIGH_OFFSET(cs))
> +#define SDRAM_ADDR_CTRL_ADDR_SEL_MASK(cs) BIT(16+cs)
> +#define SDRAM_ADDR_CTRL_SIZE_LOW_OFFSET(cs) (cs*4+2)
> +#define SDRAM_ADDR_CTRL_SIZE_LOW_MASK(cs) \
> + (0x3 << SDRAM_ADDR_CTRL_SIZE_LOW_OFFSET(cs))
> +#define SDRAM_ADDR_CTRL_STRUCT_OFFSET(cs) (cs*4)
> +#define SDRAM_ADDR_CTRL_STRUCT_MASK(cs) \
> + (0x3 << SDRAM_ADDR_CTRL_STRUCT_OFFSET(cs))
> +
> +#define SDRAM_ERR_DATA_H_REG 0x40
> +#define SDRAM_ERR_DATA_L_REG 0x44
> +
> +#define SDRAM_ERR_RECV_ECC_REG 0x48
> +#define SDRAM_ERR_RECV_ECC_VALUE_MASK 0xff
> +
> +#define SDRAM_ERR_CALC_ECC_REG 0x4c
> +#define SDRAM_ERR_CALC_ECC_ROW_OFFSET 8
> +#define SDRAM_ERR_CALC_ECC_ROW_MASK \
> + (0xffff << SDRAM_ERR_CALC_ECC_ROW_OFFSET)
> +#define SDRAM_ERR_CALC_ECC_VALUE_MASK 0xff
> +
> +#define SDRAM_ERR_ADDR_REG 0x50
> +#define SDRAM_ERR_ADDR_BANK_OFFSET 23
> +#define SDRAM_ERR_ADDR_BANK_MASK \
> + (0x7 << SDRAM_ERR_ADDR_BANK_OFFSET)
> +#define SDRAM_ERR_ADDR_COL_OFFSET 8
> +#define SDRAM_ERR_ADDR_COL_MASK \
> + (0x7fff << SDRAM_ERR_ADDR_COL_OFFSET)
> +#define SDRAM_ERR_ADDR_CS_OFFSET 1
> +#define SDRAM_ERR_ADDR_CS_MASK \
> + (0x3 << SDRAM_ERR_ADDR_CS_OFFSET)
> +#define SDRAM_ERR_ADDR_TYPE_MASK BIT(0)
> +
> +#define SDRAM_ERR_CTRL_REG 0x54
> +#define SDRAM_ERR_CTRL_ERR_THR_OFFSET 16
> +#define SDRAM_ERR_CTRL_ERR_THR_MASK \
> + (0xff << SDRAM_ERR_CTRL_ERR_THR_OFFSET)
> +#define SDRAM_ERR_CTRL_ERR_PROP_MASK BIT(9)
> +
> +#define SDRAM_ERR_SBE_COUNT_REG 0x58
> +#define SDRAM_ERR_DBE_COUNT_REG 0x5c
> +
> +#define SDRAM_ERR_CAUSE_ERR_REG 0xd0
> +#define SDRAM_ERR_CAUSE_MSG_REG 0xd8
> +#define SDRAM_ERR_CAUSE_DBE_MASK BIT(1)
> +#define SDRAM_ERR_CAUSE_SBE_MASK BIT(0)
> +
> +#define SDRAM_RANK_CTRL_REG 0x1e0
> +#define SDRAM_RANK_CTRL_EXIST_MASK(cs) BIT(cs)
Align those define names and values vertically. They can stick out and
overflow the 80 cols rule:
#define SDRAM_ERR_CTRL_ERR_THR_OFFSET 16
#define SDRAM_ERR_CTRL_ERR_THR_MASK (0xff << SDRAM_ERR_CTRL_ERR_THR_OFFSET)
#define SDRAM_ERR_CTRL_ERR_PROP_MASK BIT(9)
also, you can shorten those defines too - there's "ERR" twice in the
name.
> +
> +struct armada_xp_mc_edac_drvdata {
That struct name is too long. So long that it makes you break parameter
names below.
For example, you don't need that lenghty prefix "armada_xp_mc_edac"
prepended to static symbols. Same for the function names below.
> + void __iomem *base;
> +
> + unsigned int width; /* width in bytes */
> + bool cs_addr_sel[SDRAM_NUM_CS]; /* bank interleaving */
Put all side comments over the respective line they're commenting.
> +
> + char msg[128];
> +};
> +
> +/* derived from "DRAM Address Multiplexing" in the ARAMDA XP Functional Spec */
> +static uint32_t armada_xp_mc_edac_calc_address(struct armada_xp_mc_edac_drvdata
> + *drvdata, uint8_t cs,
> + uint8_t bank, uint16_t row,
> + uint16_t col)
> +{
> + if (drvdata->width == 8) { /* 64 bit */
> + if (drvdata->cs_addr_sel[cs]) /* bank interleaved */
> + return (((row & 0xfff8) << 16) |
> + ((bank & 0x7) << 16) |
> + ((row & 0x7) << 13) |
> + ((col & 0x3ff) << 3));
> + else
> + return (((row & 0xffff << 16) |
> + ((bank & 0x7) << 13) |
> + ((col & 0x3ff)) << 3));
> + } else if (drvdata->width == 4) { /* 32 bit */
> + if (drvdata->cs_addr_sel[cs]) /* bank interleaved */
> + return (((row & 0xfff0) << 15) |
> + ((bank & 0x7) << 16) |
> + ((row & 0xf) << 12) |
> + ((col & 0x3ff) << 2));
> + else
> + return (((row & 0xffff << 15) |
> + ((bank & 0x7) << 12) |
> + ((col & 0x3ff)) << 2));
> + } else { /* 16 bit */
> + if (drvdata->cs_addr_sel[cs]) /* bank interleaved */
> + return (((row & 0xffe0) << 14) |
> + ((bank & 0x7) << 16) |
> + ((row & 0x1f) << 11) |
> + ((col & 0x3ff) << 1));
> + else
> + return (((row & 0xffff << 14) |
> + ((bank & 0x7) << 11) |
> + ((col & 0x3ff)) << 1));
> + }
> +}
> +
> +static void armada_xp_mc_edac_check(struct mem_ctl_info *mci)
> +{
> + struct armada_xp_mc_edac_drvdata *drvdata = mci->pvt_info;
> + uint32_t data_h, data_l, recv_ecc, calc_ecc, addr;
> + uint32_t cnt_sbe, cnt_dbe, cause_err, cause_msg;
> + uint32_t row_val, col_val, bank_val, addr_val;
> + uint8_t syndrome_val, cs_val;
> + char *msg = drvdata->msg;
> +
> + data_h = readl(drvdata->base + SDRAM_ERR_DATA_H_REG);
> + data_l = readl(drvdata->base + SDRAM_ERR_DATA_L_REG);
> + recv_ecc = readl(drvdata->base + SDRAM_ERR_RECV_ECC_REG);
> + calc_ecc = readl(drvdata->base + SDRAM_ERR_CALC_ECC_REG);
> + addr = readl(drvdata->base + SDRAM_ERR_ADDR_REG);
> + cnt_sbe = readl(drvdata->base + SDRAM_ERR_SBE_COUNT_REG);
> + cnt_dbe = readl(drvdata->base + SDRAM_ERR_DBE_COUNT_REG);
> + cause_err = readl(drvdata->base + SDRAM_ERR_CAUSE_ERR_REG);
> + cause_msg = readl(drvdata->base + SDRAM_ERR_CAUSE_MSG_REG);
Align vertically for better readability:
data_h = readl(drvdata->base + SDRAM_ERR_DATA_H_REG);
data_l = readl(drvdata->base + SDRAM_ERR_DATA_L_REG);
recv_ecc = readl(drvdata->base + SDRAM_ERR_RECV_ECC_REG);
calc_ecc = readl(drvdata->base + SDRAM_ERR_CALC_ECC_REG);
...
> +
> + /* clear cause registers */
> + writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
> + drvdata->base + SDRAM_ERR_CAUSE_ERR_REG);
> + writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
> + drvdata->base + SDRAM_ERR_CAUSE_MSG_REG);
> +
> + /* clear error counter registers */
> + if (cnt_sbe)
> + writel(0, drvdata->base + SDRAM_ERR_SBE_COUNT_REG);
> + if (cnt_dbe)
> + writel(0, drvdata->base + SDRAM_ERR_DBE_COUNT_REG);
> +
> + if (!cnt_sbe && !cnt_dbe)
> + return;
> +
> + if ((addr & SDRAM_ERR_ADDR_TYPE_MASK) == 0) {
if (!(addr & SDRAM_ERR_ADDR_TYPE_MASK)) {
> + if (cnt_sbe)
> + cnt_sbe--;
> + else
> + dev_warn(mci->pdev, "inconsistent SBE count detected");
> + } else {
> + if (cnt_dbe)
> + cnt_dbe--;
> + else
> + dev_warn(mci->pdev, "inconsistent DBE count detected");
> + }
> +
> + /* report earlier errors */
> + if (cnt_sbe)
> + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
> + cnt_sbe, /* error count */
> + 0, 0, 0, /* pfn, offset, syndrome */
> + -1, -1, -1, /* top, mid, low layer */
> + mci->ctl_name,
> + "details unavailable (multiple errors)");
> + if (cnt_dbe)
> + edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
> + cnt_sbe, /* error count */
> + 0, 0, 0, /* pfn, offset, syndrome */
> + -1, -1, -1, /* top, mid, low layer */
> + mci->ctl_name,
> + "details unavailable (multiple errors)");
> +
> + /* report details for most recent error */
> + cs_val = (addr & SDRAM_ERR_ADDR_CS_MASK)
> + >> SDRAM_ERR_ADDR_CS_OFFSET;
> + bank_val = (addr & SDRAM_ERR_ADDR_BANK_MASK)
> + >> SDRAM_ERR_ADDR_BANK_OFFSET;
> + row_val = (calc_ecc & SDRAM_ERR_CALC_ECC_ROW_MASK)
> + >> SDRAM_ERR_CALC_ECC_ROW_OFFSET;
> + col_val = (addr & SDRAM_ERR_ADDR_COL_MASK)
> + >> SDRAM_ERR_ADDR_COL_OFFSET;
Let those stick out and align vertically.
> + syndrome_val = (recv_ecc ^ calc_ecc) & 0xff;
> + addr_val = armada_xp_mc_edac_calc_address(drvdata, cs_val, bank_val,
> + row_val, col_val);
> + msg += sprintf(msg, "row=0x%04x ", row_val); /* 11 chars */
> + msg += sprintf(msg, "bank=0x%x ", bank_val); /* 9 chars */
> + msg += sprintf(msg, "col=0x%04x ", col_val); /* 11 chars */
> + msg += sprintf(msg, "cs=%d", cs_val); /* 4 chars */
> +
> + if ((addr & SDRAM_ERR_ADDR_TYPE_MASK) == 0) {
if (!(...
as above.
> + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
> + 1, /* error count */
> + addr_val >> PAGE_SHIFT,
> + addr_val & ~PAGE_MASK,
> + syndrome_val,
> + cs_val, -1, -1, /* top, mid, low layer */
> + mci->ctl_name, drvdata->msg);
> + } else {
> + edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
> + 1, /* error count */
> + addr_val >> PAGE_SHIFT,
> + addr_val & ~PAGE_MASK,
> + syndrome_val,
> + cs_val, -1, -1, /* top, mid, low layer */
> + mci->ctl_name, drvdata->msg);
> + }
> +}
> +
> +static void armada_xp_mc_edac_read_config(struct mem_ctl_info *mci)
> +{
> + struct armada_xp_mc_edac_drvdata *drvdata = mci->pvt_info;
> + struct dimm_info *dimm;
> + unsigned int i, cs_struct, cs_size;
> + uint32_t config, addr_ctrl, rank_ctrl;
Please sort function local variables declaration in a reverse christmas
tree order:
<type> longest_variable_name;
<type> shorter_var_name;
<type> even_shorter;
<type> i;
> +
> + config = readl(drvdata->base + SDRAM_CONFIG_REG);
> + if (config & SDRAM_CONFIG_BUS_WIDTH_MASK)
> + drvdata->width = 8; /* 64 bit */
> + else
> + drvdata->width = 4; /* 32 bit */
> +
> + addr_ctrl = readl(drvdata->base + SDRAM_ADDR_CTRL_REG);
> + rank_ctrl = readl(drvdata->base + SDRAM_RANK_CTRL_REG);
> + for (i = 0; i < SDRAM_NUM_CS; i++) {
> + dimm = mci->dimms[i];
> +
> + if (!(rank_ctrl & SDRAM_RANK_CTRL_EXIST_MASK(i)))
> + continue;
> +
> + drvdata->cs_addr_sel[i] =
> + !!(addr_ctrl & SDRAM_ADDR_CTRL_ADDR_SEL_MASK(i));
> +
> + cs_struct = (addr_ctrl & SDRAM_ADDR_CTRL_STRUCT_MASK(i)) >>
> + SDRAM_ADDR_CTRL_STRUCT_OFFSET(i);
> + cs_size = ((addr_ctrl & SDRAM_ADDR_CTRL_SIZE_HIGH_MASK(i)) >>
> + (SDRAM_ADDR_CTRL_SIZE_HIGH_OFFSET(i) - 2) |
> + ((addr_ctrl & SDRAM_ADDR_CTRL_SIZE_LOW_MASK(i)) >>
> + SDRAM_ADDR_CTRL_SIZE_LOW_OFFSET(i)));
Align vertically and let them stick out. As they are now, they're almost
unreadable.
> + switch (cs_size) {
> + case 0: /* 2GBit */
> + dimm->nr_pages = (0x80000000ULL >> PAGE_SHIFT);
> + break;
> + case 1: /* 256MBit */
> + dimm->nr_pages = (0x10000000ULL >> PAGE_SHIFT);
> + break;
> + case 2: /* 512MBit */
> + dimm->nr_pages = (0x20000000ULL >> PAGE_SHIFT);
> + break;
> + case 3: /* 1GBit */
> + dimm->nr_pages = (0x40000000ULL >> PAGE_SHIFT);
> + break;
> + case 4: /* 4GBit */
> + dimm->nr_pages = (0x100000000ULL >> PAGE_SHIFT);
> + break;
> + case 5: /* 8GBit */
> + dimm->nr_pages = (0x200000000ULL >> PAGE_SHIFT);
That's silly. Just write the pages in decimal numbers. You have the
comments explaining what those numbers are.
> + break;
> + }
> + dimm->grain = 8;
> + dimm->dtype = cs_struct ? DEV_X16 : DEV_X8;
> + dimm->mtype = (config & SDRAM_CONFIG_REGISTERED_MASK) ?
> + MEM_RDDR3 : MEM_DDR3;
> + dimm->edac_mode = EDAC_SECDED;
> + }
> +}
> +
> +static const struct of_device_id armada_xp_mc_edac_of_match[] = {
> + {.compatible = "marvell,armada-xp-sdram-controller",},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, armada_xp_mc_edac_of_match);
> +
> +static int armada_xp_mc_edac_probe(struct platform_device *pdev)
> +{
> + const struct of_device_id *id;
> + struct mem_ctl_info *mci;
> + struct edac_mc_layer layers[1];
> + struct armada_xp_mc_edac_drvdata *drvdata;
> + struct resource *r;
> + void __iomem *base;
> + uint32_t config;
Please sort function local variables declaration in a reverse christmas
tree order:
<type> longest_variable_name;
<type> shorter_var_name;
<type> even_shorter;
<type> i;
> +
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!r) {
> + dev_err(&pdev->dev, "Unable to get mem resource\n");
> + return -ENODEV;
> + }
> +
> + base = devm_ioremap_resource(&pdev->dev, r);
> + if (IS_ERR(base)) {
> + dev_err(&pdev->dev, "Unable to map regs\n");
> + return PTR_ERR(base);
> + }
> +
> + config = readl(base + SDRAM_CONFIG_REG);
> + if (!(config & SDRAM_CONFIG_ECC_MASK)) {
> + dev_warn(&pdev->dev, "SDRAM ECC is not enabled");
> + return -EINVAL;
> + }
> +
> + layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
> + layers[0].size = SDRAM_NUM_CS;
> + layers[0].is_virt_csrow = true;
> +
> + mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*drvdata));
> + if (!mci)
> + return -ENOMEM;
> +
> + drvdata = mci->pvt_info;
> + drvdata->base = base;
> + mci->pdev = &pdev->dev;
> + platform_set_drvdata(pdev, mci);
> +
> + id = of_match_device(armada_xp_mc_edac_of_match, &pdev->dev);
> + mci->edac_check = armada_xp_mc_edac_check;
> + mci->mtype_cap = MEM_FLAG_DDR3;
> + mci->edac_cap = EDAC_FLAG_SECDED;
> + mci->mod_name = pdev->dev.driver->name;
> + mci->ctl_name = id ? id->compatible : "unknown";
> + mci->dev_name = dev_name(&pdev->dev);
> + mci->scrub_mode = SCRUB_NONE;
> +
> + armada_xp_mc_edac_read_config(mci);
> +
> + /* configure SBE threshold */
> + /* it seems that SBEs are not captured otherwise */
> + writel(1 << SDRAM_ERR_CTRL_ERR_THR_OFFSET,
> + drvdata->base + SDRAM_ERR_CTRL_REG);
> +
> + /* clear cause registers */
> + writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
> + drvdata->base + SDRAM_ERR_CAUSE_ERR_REG);
> + writel(~(SDRAM_ERR_CAUSE_DBE_MASK | SDRAM_ERR_CAUSE_SBE_MASK),
> + drvdata->base + SDRAM_ERR_CAUSE_MSG_REG);
Let those stick out and align vertically.
> +
> + /* clear counter registers */
> + writel(0, drvdata->base + SDRAM_ERR_SBE_COUNT_REG);
> + writel(0, drvdata->base + SDRAM_ERR_DBE_COUNT_REG);
> +
> + if (edac_mc_add_mc(mci)) {
> + edac_mc_free(mci);
> + return -EINVAL;
> + }
> + edac_op_state = EDAC_OPSTATE_POLL;
> +
> + return 0;
> +}
> +
> +static int armada_xp_mc_edac_remove(struct platform_device *pdev)
> +{
> + struct mem_ctl_info *mci = platform_get_drvdata(pdev);
> +
> + edac_mc_del_mc(&pdev->dev);
> + edac_mc_free(mci);
> + platform_set_drvdata(pdev, NULL);
> +
> + return 0;
> +}
> +
> +static struct platform_driver armada_xp_mc_edac_driver = {
> + .probe = armada_xp_mc_edac_probe,
> + .remove = armada_xp_mc_edac_remove,
> + .driver = {
> + .name = "armada_xp_mc_edac",
> + .of_match_table = of_match_ptr(armada_xp_mc_edac_of_match),
> + },
> +};
> +
> +/************************ EDAC Device (L2 Cache) ***************************/
> +
> +struct aurora_l2_edac_drvdata {
> + void __iomem *base;
> +
> + char msg[128];
> +
> + /* error injection via debugfs */
> + uint32_t inject_addr;
> + uint32_t inject_mask;
> + uint8_t inject_ctl;
> +
> + struct dentry *debugfs;
> +};
> +
> +static void aurora_l2_edac_inject(struct aurora_l2_edac_drvdata *drvdata)
The whole EDAC error injection functionality should be behind
CONFIG_EDAC_DEBUG. You don't want error injection capabilities exposed
on a production system.
> +{
> + drvdata->inject_addr &= AURORA_ERR_INJECT_CTL_ADDR_MASK;
> + drvdata->inject_ctl &= AURORA_ERR_INJECT_CTL_EN_MASK;
> + writel(0, drvdata->base + AURORA_ERR_INJECT_CTL_REG);
> + writel(drvdata->inject_mask,
> + drvdata->base + AURORA_ERR_INJECT_MASK_REG);
> + writel(drvdata->inject_addr | drvdata->inject_ctl,
> + drvdata->base + AURORA_ERR_INJECT_CTL_REG);
Let those stick out and align vertically.
> +}
> +
> +static void aurora_l2_edac_check(struct edac_device_ctl_info *dci)
> +{
> + struct aurora_l2_edac_drvdata *drvdata = dci->pvt_info;
> + uint32_t cnt, attr_cap, addr_cap, way_cap;
> + unsigned int cnt_ce, cnt_ue;
> +
> + cnt = readl(drvdata->base + AURORA_ERR_CNT_REG);
> + attr_cap = readl(drvdata->base + AURORA_ERR_ATTR_CAP_REG);
> + addr_cap = readl(drvdata->base + AURORA_ERR_ADDR_CAP_REG);
> + way_cap = readl(drvdata->base + AURORA_ERR_WAY_CAP_REG);
> +
> + cnt_ce = (cnt & AURORA_ERR_CNT_CE_MASK) >> AURORA_ERR_CNT_CE_OFFSET;
> + cnt_ue = (cnt & AURORA_ERR_CNT_UE_MASK) >> AURORA_ERR_CNT_UE_OFFSET;
> + /* clear error counter registers */
> + if (cnt_ce || cnt_ue)
> + writel(AURORA_ERR_CNT_CLR, drvdata->base + AURORA_ERR_CNT_REG);
> +
> + if (attr_cap & AURORA_ERR_ATTR_CAP_VALID) {
if (!(attr_cap & AURORA_ERR_ATTR_CAP_VALID))
goto clear_remaining;
saves you an indentation level.
> + char *msg = drvdata->msg;
> + size_t size = sizeof(drvdata->msg);
> + size_t len = 0;
> +
> + switch ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_SOURCE_MASK)
> + >> AURORA_ERR_ATTR_CAP_ERR_SOURCE_OFFSET) {
> + case 0:
> + len += snprintf(msg+len, size-len, "src=CPU0 ");
> + break;
> + case 1:
> + len += snprintf(msg+len, size-len, "src=CPU1 ");
> + break;
> + case 2:
> + len += snprintf(msg+len, size-len, "src=CPU2 ");
> + break;
> + case 3:
> + len += snprintf(msg+len, size-len, "src=CPU3 ");
> + break;
> + case 7:
> + len += snprintf(msg+len, size-len, "src=IO ");
> + break;
> + }
Lemme try to shorten that:
src = (attr_cap & AURORA_ERR_ATTR_SRC_MSK) >> AURORA_ERR_ATTR_SRC_OFF;
if (src <= 3)
len += snprintf(msg+len, size-len, "src=CPU%d ", src);
else
len += snprintf(msg+len, size-len, "src=IO ");
Yeah, those AURORA_* defines could be shorter.
> + switch ((attr_cap & AURORA_ERR_ATTR_CAP_TRANS_TYPE_MASK)
> + >> AURORA_ERR_ATTR_CAP_TRANS_TYPE_OFFSET) {
Ditto.
> + case 0:
> + len += snprintf(msg+len, size-len, "txn=Data-Read ");
> + break;
> + case 1:
> + len += snprintf(msg+len, size-len, "txn=Isn-Read ");
> + break;
> + case 2:
> + len += snprintf(msg+len, size-len, "txn=Clean-Flush ");
> + break;
> + case 3:
> + len += snprintf(msg+len, size-len, "txn=Eviction ");
> + break;
> + case 4:
> + len += snprintf(msg+len, size-len,
> + "txn=Read-Modify-Write ");
> + break;
> + }
> + switch ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_TYPE_MASK)
> + >> AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET) {
> + case 0:
> + len += snprintf(msg+len, size-len, "err=CorrECC ");
> + break;
> + case 1:
> + len += snprintf(msg+len, size-len, "err=UnCorrECC ");
> + break;
> + case 2:
> + len += snprintf(msg+len, size-len, "err=TagParity ");
> + break;
> + }
> + len += snprintf(msg+len, size-len, "addr=0x%x ",
> + addr_cap & AURORA_ERR_ADDR_CAP_ADDR_MASK);
> + len += snprintf(msg+len, size-len, "index=0x%x ",
> + (way_cap & AURORA_ERR_WAY_CAP_INDEX_MASK)
> + >> AURORA_ERR_WAY_CAP_INDEX_OFFSET);
> + len += snprintf(msg+len, size-len, "way=0x%x",
> + (way_cap & AURORA_ERR_WAY_CAP_WAY_MASK)
> + >> AURORA_ERR_WAY_CAP_WAY_OFFSET);
Let it stick out.
<---- newline here.
> + /* clear error capture registers */
> + writel(AURORA_ERR_ATTR_CAP_VALID,
> + drvdata->base + AURORA_ERR_ATTR_CAP_REG);
Let it stick out.
> + if ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_TYPE_MASK)
> + >> AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET) {
Ditto.
> + /* UnCorrECC or TagParity */
> + if (cnt_ue)
> + cnt_ue--;
> + edac_device_handle_ue(dci, 0, 0, drvdata->msg);
> + } else {
> + if (cnt_ce)
> + cnt_ce--;
> + edac_device_handle_ce(dci, 0, 0, drvdata->msg);
> + }
> + }
> +
> + /* report remaining errors */
> + while (cnt_ue--)
> + edac_device_handle_ue(dci, 0, 0,
> + "details unavailable (multiple errors)");
> + while (cnt_ce--)
> + edac_device_handle_ue(dci, 0, 0,
> + "details unavailable (multiple errors)");
Ditto. You get the idea...
> +}
> +
> +static void aurora_l2_edac_poll(struct edac_device_ctl_info *dci)
> +{
> + struct aurora_l2_edac_drvdata *drvdata = dci->pvt_info;
> +
> + aurora_l2_edac_check(dci);
> + aurora_l2_edac_inject(drvdata);
> +}
> +
> +static const struct of_device_id aurora_l2_edac_of_match[] = {
> + {.compatible = "marvell,aurora-system-cache",},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, aurora_l2_edac_of_match);
> +
> +static int aurora_l2_edac_probe(struct platform_device *pdev)
> +{
> + const struct of_device_id *id;
> + struct edac_device_ctl_info *dci;
> + struct aurora_l2_edac_drvdata *drvdata;
> + struct resource *r;
> + void __iomem *base;
> + uint32_t l2x0_aux_ctrl;
Please sort function local variables declaration in a reverse christmas
tree order:
<type> longest_variable_name;
<type> shorter_var_name;
<type> even_shorter;
<type> i;
> +
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!r) {
> + dev_err(&pdev->dev, "Unable to get mem resource\n");
> + return -ENODEV;
> + }
> +
> + base = devm_ioremap_resource(&pdev->dev, r);
> + if (IS_ERR(base)) {
> + dev_err(&pdev->dev, "Unable to map regs\n");
> + return PTR_ERR(base);
> + }
> +
> + l2x0_aux_ctrl = readl(base + L2X0_AUX_CTRL);
> + if (!(l2x0_aux_ctrl & AURORA_ACR_PARITY_EN))
> + dev_warn(&pdev->dev, "tag parity is not enabled");
> + if (!(l2x0_aux_ctrl & AURORA_ACR_ECC_EN))
> + dev_warn(&pdev->dev, "data ECC is not enabled");
> +
> + dci = edac_device_alloc_ctl_info(sizeof(*drvdata),
> + "cpu", 1, "L", 1, 2, NULL, 0, 0);
> + if (!dci)
> + return -ENOMEM;
> +
> + drvdata = dci->pvt_info;
> + drvdata->base = base;
> + dci->dev = &pdev->dev;
> + platform_set_drvdata(pdev, dci);
> +
> + id = of_match_device(aurora_l2_edac_of_match, &pdev->dev);
> + dci->edac_check = aurora_l2_edac_poll;
> + dci->mod_name = pdev->dev.driver->name;
> + dci->ctl_name = id ? id->compatible : "unknown";
> + dci->dev_name = dev_name(&pdev->dev);
> +
> + /* clear registers */
> + writel(AURORA_ERR_CNT_CLR, drvdata->base + AURORA_ERR_CNT_REG);
> + writel(AURORA_ERR_ATTR_CAP_VALID,
> + drvdata->base + AURORA_ERR_ATTR_CAP_REG);
> +
> + if (edac_device_add_device(dci)) {
> + edac_device_free_ctl_info(dci);
> + return -EINVAL;
> + }
> +
> + drvdata->debugfs = edac_debugfs_create_dir(dev_name(&pdev->dev));
> + if (drvdata->debugfs) {
> + edac_debugfs_create_x32("inject_addr", 0644,
> + drvdata->debugfs,
> + &drvdata->inject_addr);
> + edac_debugfs_create_x32("inject_mask", 0644,
> + drvdata->debugfs,
> + &drvdata->inject_mask);
> + edac_debugfs_create_x8("inject_ctl", 0644,
> + drvdata->debugfs, &drvdata->inject_ctl);
> + }
> +
> + return 0;
> +}
> +
> +static int aurora_l2_edac_remove(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
> + struct aurora_l2_edac_drvdata *drvdata = dci->pvt_info;
> +
> + edac_debugfs_remove_recursive(drvdata->debugfs);
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
> + platform_set_drvdata(pdev, NULL);
> +
> + return 0;
> +}
> +
> +static struct platform_driver aurora_l2_edac_driver = {
> + .probe = aurora_l2_edac_probe,
> + .remove = aurora_l2_edac_remove,
> + .driver = {
> + .name = "aurora_l2_edac",
> + .of_match_table = of_match_ptr(aurora_l2_edac_of_match),
> + },
> +};
> +
> +/************************ Driver registration ******************************/
> +
> +static struct platform_driver * const drivers[] = {
> + &armada_xp_mc_edac_driver,
> + &aurora_l2_edac_driver,
> +};
> +
> +static int __init armada_xp_edac_init(void)
> +{
> + int res = 0;
No need to zero it.
> +
> + /* only polling is supported */
> + edac_op_state = EDAC_OPSTATE_POLL;
> +
> + res = platform_register_drivers(drivers, ARRAY_SIZE(drivers));
> + if (res)
> + pr_warn("Aramda XP EDAC drivers fail to register\n");
> +
> + return 0;
> +}
> +module_init(armada_xp_edac_init);
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [v3,6/7] EDAC: Add missing debugfs_create_x32 wrapper
2017-11-10 9:03 ` [PATCH v3 6/7] " Jan Luebbe
@ 2017-11-16 19:32 ` Borislav Petkov
-1 siblings, 0 replies; 35+ messages in thread
From: Borislav Petkov @ 2017-11-16 19:32 UTC (permalink / raw)
To: Jan Luebbe
Cc: Russell King - ARM Linux, Chris Packham, linux-arm-kernel,
linux-edac, kernel, Thomas Petazzoni, Gregory CLEMENT
On Fri, Nov 10, 2017 at 10:03:07AM +0100, Jan Luebbe wrote:
> We already have wrappers for x8 and x16, so add the missing x32 one.
>
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> ---
> drivers/edac/debugfs.c | 11 +++++++++++
> drivers/edac/edac_module.h | 5 +++++
> 2 files changed, 16 insertions(+)
Reviewed-by: Borislav Petkov <bp@suse.de>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 6/7] EDAC: Add missing debugfs_create_x32 wrapper
@ 2017-11-16 19:32 ` Borislav Petkov
0 siblings, 0 replies; 35+ messages in thread
From: Borislav Petkov @ 2017-11-16 19:32 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Nov 10, 2017 at 10:03:07AM +0100, Jan Luebbe wrote:
> We already have wrappers for x8 and x16, so add the missing x32 one.
>
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> ---
> drivers/edac/debugfs.c | 11 +++++++++++
> drivers/edac/edac_module.h | 5 +++++
> 2 files changed, 16 insertions(+)
Reviewed-by: Borislav Petkov <bp@suse.de>
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [v3,7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
2017-11-16 19:31 ` [PATCH v3 7/7] " Borislav Petkov
@ 2017-11-16 21:05 ` Andrew Lunn
-1 siblings, 0 replies; 35+ messages in thread
From: Andrew Lunn @ 2017-11-16 21:05 UTC (permalink / raw)
To: Borislav Petkov
Cc: Jan Luebbe, Thomas Petazzoni, Russell King - ARM Linux,
Chris Packham, kernel, Gregory CLEMENT, linux-arm-kernel,
linux-edac
> and that driver depends on ARCH_MVEBU which is arm64, AFAICT.
ARCH_MVEBU is both 32 and 64. It refers more to a collection of
devices which mvebu processor have. Some of these processors are
32bit, some are 64bit.
Andrew
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
@ 2017-11-16 21:05 ` Andrew Lunn
0 siblings, 0 replies; 35+ messages in thread
From: Andrew Lunn @ 2017-11-16 21:05 UTC (permalink / raw)
To: linux-arm-kernel
> and that driver depends on ARCH_MVEBU which is arm64, AFAICT.
ARCH_MVEBU is both 32 and 64. It refers more to a collection of
devices which mvebu processor have. Some of these processors are
32bit, some are 64bit.
Andrew
^ permalink raw reply [flat|nested] 35+ messages in thread
* [v3,7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
2017-11-10 9:03 ` [PATCH v3 7/7] " Jan Luebbe
@ 2018-01-08 22:00 ` Chris Packham
-1 siblings, 0 replies; 35+ messages in thread
From: Chris Packham @ 2018-01-08 22:00 UTC (permalink / raw)
To: Jan Luebbe, Russell King - ARM Linux, Borislav Petkov,
linux-arm-kernel, linux-edac
Cc: kernel, Thomas Petazzoni, Gregory CLEMENT
Hi Jan,
On 10/11/17 22:03, Jan Luebbe wrote:
> Add support for the ECC functionality as found in the DDR RAM and L2
> cache controllers on the MV78230/MV78x60 SoCs. This driver has been
> tested on the MV78460 (on a custom board with a DDR3 ECC DIMM).
>
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
What is the current state of this? I see there were some comments from
Borislav that need addressing.
I've got my patches for Armada-380 and 98dx3236 ready. They're fairly
minor so I might send them anyway just to get any feedback. They
probably could be incorporated into a v4 series if you are willing to
carry them.
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
@ 2018-01-08 22:00 ` Chris Packham
0 siblings, 0 replies; 35+ messages in thread
From: Chris Packham @ 2018-01-08 22:00 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jan,
On 10/11/17 22:03, Jan Luebbe wrote:
> Add support for the ECC functionality as found in the DDR RAM and L2
> cache controllers on the MV78230/MV78x60 SoCs. This driver has been
> tested on the MV78460 (on a custom board with a DDR3 ECC DIMM).
>
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
What is the current state of this? I see there were some comments from
Borislav that need addressing.
I've got my patches for Armada-380 and 98dx3236 ready. They're fairly
minor so I might send them anyway just to get any feedback. They
probably could be incorporated into a v4 series if you are willing to
carry them.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [v3,7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
2018-01-08 22:00 ` [PATCH v3 7/7] " Chris Packham
@ 2018-01-09 9:18 ` Jan Lübbe
-1 siblings, 0 replies; 35+ messages in thread
From: Jan Lübbe @ 2018-01-09 9:18 UTC (permalink / raw)
To: Chris Packham, Russell King - ARM Linux, Borislav Petkov,
linux-arm-kernel, linux-edac
Cc: kernel, Thomas Petazzoni, Gregory CLEMENT
Hi Chris,
On Mon, 2018-01-08 at 22:00 +0000, Chris Packham wrote:
> Hi Jan,
>
> On 10/11/17 22:03, Jan Luebbe wrote:
> > Add support for the ECC functionality as found in the DDR RAM and
> > L2 cache controllers on the MV78230/MV78x60 SoCs. This driver has
> > been tested on the MV78460 (on a custom board with a DDR3 ECC
> > DIMM).
> >
> > Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
>
> What is the current state of this? I see there were some comments
> from Borislav that need addressing.
>
> I've got my patches for Armada-380 and 98dx3236 ready. They're
> fairly minor so I might send them anyway just to get any feedback.
> They probably could be incorporated into a v4 series if you are
> willing to carry them.
This series is still on my todo list, but I currently don't have the
time to work on it (and probably also not in the next weeks). If you
have some spare time to pick it up and address Borislav's comments, I
wouldn't mind at all. ;)
Thanks,
Jan
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
@ 2018-01-09 9:18 ` Jan Lübbe
0 siblings, 0 replies; 35+ messages in thread
From: Jan Lübbe @ 2018-01-09 9:18 UTC (permalink / raw)
To: linux-arm-kernel
Hi Chris,
On Mon, 2018-01-08 at 22:00 +0000, Chris Packham wrote:
> Hi Jan,
>
> On 10/11/17 22:03, Jan Luebbe wrote:
> > Add support for the ECC functionality as found in the DDR RAM and
> > L2 cache controllers on the MV78230/MV78x60 SoCs. This driver has
> > been tested on the MV78460 (on a custom board with a DDR3 ECC
> > DIMM).
> >
> > Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
>
> What is the current state of this? I see there were some comments
> from Borislav that need addressing.
>
> I've got my patches for Armada-380 and 98dx3236 ready. They're
> fairly minor so I might send them anyway just to get any feedback.
> They probably could be incorporated into a v4 series if you are
> willing to carry them.
This series is still on my todo list, but I currently don't have the
time to work on it (and probably also not in the next weeks). If you
have some spare time to pick it up and address Borislav's comments, I
wouldn't mind at all. ;)
Thanks,
Jan
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 35+ messages in thread
* [v3,7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
2017-11-16 19:31 ` [PATCH v3 7/7] " Borislav Petkov
@ 2018-01-10 21:54 ` Chris Packham
-1 siblings, 0 replies; 35+ messages in thread
From: Chris Packham @ 2018-01-10 21:54 UTC (permalink / raw)
To: Borislav Petkov, Jan Luebbe
Cc: Russell King - ARM Linux, linux-arm-kernel, linux-edac, kernel,
Thomas Petazzoni, Gregory CLEMENT
Hi Borislav,
On 17/11/17 08:32, Borislav Petkov wrote:
> On Fri, Nov 10, 2017 at 10:03:08AM +0100, Jan Luebbe wrote:
>> Add support for the ECC functionality as found in the DDR RAM and L2
>> cache controllers on the MV78230/MV78x60 SoCs. This driver has been
>> tested on the MV78460 (on a custom board with a DDR3 ECC DIMM).
>>
>> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
>
> ...
>
>> diff --git a/drivers/edac/armada_xp_edac.c b/drivers/edac/armada_xp_edac.c
>> new file mode 100644
>> index 000000000000..cb9173b30aa9
>> --- /dev/null
>> +++ b/drivers/edac/armada_xp_edac.c
>> @@ -0,0 +1,658 @@
>> +/*
>> + * Copyright (C) 2017 Pengutronix, Jan Luebbe <kernel@pengutronix.de>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/edac.h>
>> +#include <linux/of_platform.h>
>> +
>> +#include <asm/hardware/cache-l2x0.h>
>
> I get this when I cross-build it:
>
> drivers/edac/armada_xp_edac.c:19:37: fatal error: asm/hardware/cache-l2x0.h: No such file or directory
> #include <asm/hardware/cache-l2x0.h>
> ^
> compilation terminated.
> make[2]: *** [drivers/edac/armada_xp_edac.o] Error 1
> make[1]: *** [drivers/edac] Error 2
> make[1]: *** Waiting for unfinished jobs....
> make: *** [drivers] Error 2
>
>
> and that driver depends on ARCH_MVEBU which is arm64, AFAICT.
>
> That header is
>
> arch/arm/include/asm/hardware/cache-l2x0.h
>
> however and not in arch/arm64/.
>
> I could very well be missing something though...
>
As Andrew said ARCH_MVEBU includes both 32-bit and 64-bit processors.
In terms of fixing this issue there are a few options. My first blunt
tool approach is
- depends on ARCH_MVEBU
+ depends on ARCH_MVEBU && !ARM64
But that's not a particularly common pattern outside of arch/arm
Another option would be
- depends on ARCH_MVEBU
+ depends on MACH_MVEBU_V7
There seems to be plenty of precedent for depending on a MACH family in
other drivers.
We could even go as far as wrapping header include and L2 cache parts
with #ifdef CONFIG_CACHE_L2X0.
Do you have any preference for which option to take? I'm leaning towards
MACH_MVEBU_V7.
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
@ 2018-01-10 21:54 ` Chris Packham
0 siblings, 0 replies; 35+ messages in thread
From: Chris Packham @ 2018-01-10 21:54 UTC (permalink / raw)
To: linux-arm-kernel
Hi Borislav,
On 17/11/17 08:32, Borislav Petkov wrote:
> On Fri, Nov 10, 2017 at 10:03:08AM +0100, Jan Luebbe wrote:
>> Add support for the ECC functionality as found in the DDR RAM and L2
>> cache controllers on the MV78230/MV78x60 SoCs. This driver has been
>> tested on the MV78460 (on a custom board with a DDR3 ECC DIMM).
>>
>> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
>
> ...
>
>> diff --git a/drivers/edac/armada_xp_edac.c b/drivers/edac/armada_xp_edac.c
>> new file mode 100644
>> index 000000000000..cb9173b30aa9
>> --- /dev/null
>> +++ b/drivers/edac/armada_xp_edac.c
>> @@ -0,0 +1,658 @@
>> +/*
>> + * Copyright (C) 2017 Pengutronix, Jan Luebbe <kernel@pengutronix.de>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/edac.h>
>> +#include <linux/of_platform.h>
>> +
>> +#include <asm/hardware/cache-l2x0.h>
>
> I get this when I cross-build it:
>
> drivers/edac/armada_xp_edac.c:19:37: fatal error: asm/hardware/cache-l2x0.h: No such file or directory
> #include <asm/hardware/cache-l2x0.h>
> ^
> compilation terminated.
> make[2]: *** [drivers/edac/armada_xp_edac.o] Error 1
> make[1]: *** [drivers/edac] Error 2
> make[1]: *** Waiting for unfinished jobs....
> make: *** [drivers] Error 2
>
>
> and that driver depends on ARCH_MVEBU which is arm64, AFAICT.
>
> That header is
>
> arch/arm/include/asm/hardware/cache-l2x0.h
>
> however and not in arch/arm64/.
>
> I could very well be missing something though...
>
As Andrew said ARCH_MVEBU includes both 32-bit and 64-bit processors.
In terms of fixing this issue there are a few options. My first blunt
tool approach is
- depends on ARCH_MVEBU
+ depends on ARCH_MVEBU && !ARM64
But that's not a particularly common pattern outside of arch/arm
Another option would be
- depends on ARCH_MVEBU
+ depends on MACH_MVEBU_V7
There seems to be plenty of precedent for depending on a MACH family in
other drivers.
We could even go as far as wrapping header include and L2 cache parts
with #ifdef CONFIG_CACHE_L2X0.
Do you have any preference for which option to take? I'm leaning towards
MACH_MVEBU_V7.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [v3,7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
2017-11-16 19:31 ` [PATCH v3 7/7] " Borislav Petkov
@ 2018-01-10 23:05 ` Chris Packham
-1 siblings, 0 replies; 35+ messages in thread
From: Chris Packham @ 2018-01-10 23:05 UTC (permalink / raw)
To: Borislav Petkov, Jan Luebbe
Cc: Russell King - ARM Linux, linux-arm-kernel, linux-edac, kernel,
Thomas Petazzoni, Gregory CLEMENT
Hi Borislav,
On 17/11/17 08:32, Borislav Petkov wrote:
> That struct name is too long. So long that it makes you break parameter
> names below.
>
> For example, you don't need that lenghty prefix "armada_xp_mc_edac"
> prepended to static symbols. Same for the function names below.
Would you suggest dropping "armada_xp" altogether or just shortening to
"axp"? What about dropping "edac" as well?
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
@ 2018-01-10 23:05 ` Chris Packham
0 siblings, 0 replies; 35+ messages in thread
From: Chris Packham @ 2018-01-10 23:05 UTC (permalink / raw)
To: linux-arm-kernel
Hi Borislav,
On 17/11/17 08:32, Borislav Petkov wrote:
> That struct name is too long. So long that it makes you break parameter
> names below.
>
> For example, you don't need that lenghty prefix "armada_xp_mc_edac"
> prepended to static symbols. Same for the function names below.
Would you suggest dropping "armada_xp" altogether or just shortening to
"axp"? What about dropping "edac" as well?
^ permalink raw reply [flat|nested] 35+ messages in thread
* [v3,7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
2018-01-10 21:54 ` [PATCH v3 7/7] " Chris Packham
@ 2018-01-11 19:28 ` Borislav Petkov
-1 siblings, 0 replies; 35+ messages in thread
From: Borislav Petkov @ 2018-01-11 19:28 UTC (permalink / raw)
To: Chris Packham
Cc: Jan Luebbe, Russell King - ARM Linux, linux-arm-kernel,
linux-edac, kernel, Thomas Petazzoni, Gregory CLEMENT
On Wed, Jan 10, 2018 at 09:54:29PM +0000, Chris Packham wrote:
> Do you have any preference for which option to take? I'm leaning towards
> MACH_MVEBU_V7.
With my layman attempts at looking at ARM Kconfig organization, this
makes sense to me too.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
@ 2018-01-11 19:28 ` Borislav Petkov
0 siblings, 0 replies; 35+ messages in thread
From: Borislav Petkov @ 2018-01-11 19:28 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jan 10, 2018 at 09:54:29PM +0000, Chris Packham wrote:
> Do you have any preference for which option to take? I'm leaning towards
> MACH_MVEBU_V7.
With my layman attempts at looking at ARM Kconfig organization, this
makes sense to me too.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [v3,7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
2018-01-10 23:05 ` [PATCH v3 7/7] " Chris Packham
@ 2018-01-11 19:31 ` Borislav Petkov
-1 siblings, 0 replies; 35+ messages in thread
From: Borislav Petkov @ 2018-01-11 19:31 UTC (permalink / raw)
To: Chris Packham
Cc: Jan Luebbe, Russell King - ARM Linux, linux-arm-kernel,
linux-edac, kernel, Thomas Petazzoni, Gregory CLEMENT
On Wed, Jan 10, 2018 at 11:05:48PM +0000, Chris Packham wrote:
> Would you suggest dropping "armada_xp" altogether or just shortening to
> "axp"? What about dropping "edac" as well?
Whatever you prefer. And yes, my personal preference is short symbols so
I'd drop "edac" too.
I mean, we know it is "edac" - the whole driver is edac :-) and those
functions are static so their name being unique makes only partial sense
in stack traces and if the compiler hasn't inlined them...
Thx.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC
@ 2018-01-11 19:31 ` Borislav Petkov
0 siblings, 0 replies; 35+ messages in thread
From: Borislav Petkov @ 2018-01-11 19:31 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jan 10, 2018 at 11:05:48PM +0000, Chris Packham wrote:
> Would you suggest dropping "armada_xp" altogether or just shortening to
> "axp"? What about dropping "edac" as well?
Whatever you prefer. And yes, my personal preference is short symbols so
I'd drop "edac" too.
I mean, we know it is "edac" - the whole driver is edac :-) and those
functions are static so their name being unique makes only partial sense
in stack traces and if the compiler hasn't inlined them...
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2018-01-11 19:31 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-10 9:03 [PATCH v3 0/7] EDAC drivers for Armada XP L2 and DDR Jan Luebbe
2017-11-10 9:03 [v3,1/7] ARM: l2c: move cache-aurora-l2.h to asm/hardware Jan Lübbe
2017-11-10 9:03 ` [PATCH v3 1/7] " Jan Luebbe
2017-11-10 9:03 [v3,2/7] ARM: aurora-l2: add prefix to MAX_RANGE_SIZE Jan Lübbe
2017-11-10 9:03 ` [PATCH v3 2/7] " Jan Luebbe
2017-11-10 9:03 [v3,3/7] ARM: aurora-l2: add defines for parity and ECC registers Jan Lübbe
2017-11-10 9:03 ` [PATCH v3 3/7] " Jan Luebbe
2017-11-10 9:03 [v3,4/7] ARM: l2x0: support parity-enable/disable on aurora Jan Lübbe
2017-11-10 9:03 ` [PATCH v3 4/7] " Jan Luebbe
2017-11-10 9:03 [v3,5/7] ARM: l2x0: add marvell,ecc-enable property for aurora Jan Lübbe
2017-11-10 9:03 ` [PATCH v3 5/7] " Jan Luebbe
2017-11-10 9:03 [v3,6/7] EDAC: Add missing debugfs_create_x32 wrapper Jan Lübbe
2017-11-10 9:03 ` [PATCH v3 6/7] " Jan Luebbe
2017-11-10 9:03 [v3,7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC Jan Lübbe
2017-11-10 9:03 ` [PATCH v3 7/7] " Jan Luebbe
2017-11-12 20:15 [v3,7/7] " Chris Packham
2017-11-12 20:15 ` [PATCH v3 7/7] " Chris Packham
2017-11-16 19:31 [v3,7/7] " Borislav Petkov
2017-11-16 19:31 ` [PATCH v3 7/7] " Borislav Petkov
2017-11-16 19:32 [v3,6/7] EDAC: Add missing debugfs_create_x32 wrapper Borislav Petkov
2017-11-16 19:32 ` [PATCH v3 6/7] " Borislav Petkov
2017-11-16 21:05 [v3,7/7] EDAC: Add driver for the Marvell Armada XP SDRAM and L2 cache ECC Andrew Lunn
2017-11-16 21:05 ` [PATCH v3 7/7] " Andrew Lunn
2018-01-08 22:00 [v3,7/7] " Chris Packham
2018-01-08 22:00 ` [PATCH v3 7/7] " Chris Packham
2018-01-09 9:18 [v3,7/7] " Jan Lübbe
2018-01-09 9:18 ` [PATCH v3 7/7] " Jan Lübbe
2018-01-10 21:54 [v3,7/7] " Chris Packham
2018-01-10 21:54 ` [PATCH v3 7/7] " Chris Packham
2018-01-10 23:05 [v3,7/7] " Chris Packham
2018-01-10 23:05 ` [PATCH v3 7/7] " Chris Packham
2018-01-11 19:28 [v3,7/7] " Borislav Petkov
2018-01-11 19:28 ` [PATCH v3 7/7] " Borislav Petkov
2018-01-11 19:31 [v3,7/7] " Borislav Petkov
2018-01-11 19:31 ` [PATCH v3 7/7] " Borislav Petkov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.