[RFC PATCH 0/7] EDAC drivers for Armada XP L2 and DDR

[RFC PATCH 0/7] EDAC drivers for Armada XP L2 and DDR

[Jan Luebbe]

This series add 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 are devm_ helpers for edac_mc_/edac_device_
allocation and registration, which make error handing and cleanup simpler. They
may already be mergeable.

It seems Chris and I had a race condition, he posted a driver for this
functionality just yesterday. Compared to his submission, the L2 and DDR
support is split into two drivers for this series, as they don't actually share
any functionality.

Some further differences in this series are:
- The error details are decoded and passed to the edac error handler.
- Multiple errors are counted even if the details are unavailable.
- The DDR RAM configuration is read back to fill out the DIMM structures.
- The DDR RAM error address is calculated from the bank/row/col information.
- The L2 injection registers are exposed via debugfs instead of sysfs
  (resulting in less driver code).

Chris, how do you want to proceed? We should probably combine our efforts in one
series. I've already picked up part of DDR RAM config reading from your series
for this submission. I'll send some questions/comments to your series, as well.

Jan Luebbe (7):
  ARM: l2c: move cache-aurora-l2.h to asm/hardware
  ARM: aurora-l2: add prefix to MAX_RANGE_SIZE
  EDAC: Add missing debugfs_create_x32 wrapper
  EDAC: Add devres helpers for
    edac_mc_alloc/edac_mc_add_mc(_with_groups)
  EDAC: Add devres helpers for
    edac_device_alloc_ctl_info/edac_device_add_device
  EDAC: Add driver for the AURORA L2 cache
  EDAC: Add driver for the Marvell Armada XP SDRAM controller

 arch/arm/include/asm/hardware/cache-aurora-l2.h | 104 +++++++
 arch/arm/mm/cache-aurora-l2.h                   |  55 ----
 arch/arm/mm/cache-l2x0.c                        |   6 +-
 drivers/edac/Kconfig                            |  14 +
 drivers/edac/Makefile                           |   2 +
 drivers/edac/armada_xp_mc_edac.c                | 366 ++++++++++++++++++++++++
 drivers/edac/aurora_l2_edac.c                   | 252 ++++++++++++++++
 drivers/edac/debugfs.c                          |  11 +
 drivers/edac/edac_device.c                      |  59 ++++
 drivers/edac/edac_device.h                      |  29 ++
 drivers/edac/edac_mc.c                          |  53 ++++
 drivers/edac/edac_mc.h                          |  26 ++
 drivers/edac/edac_module.h                      |   5 +
 13 files changed, 924 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_mc_edac.c
 create mode 100644 drivers/edac/aurora_l2_edac.c

-- 
2.11.0

[RFC PATCH 1/7] ARM: l2c: move cache-aurora-l2.h to asm/hardware

[Jan Luebbe]

This include file will be used by the AURORA EDAC code.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
 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

[RFC PATCH 2/7] ARM: aurora-l2: add prefix to MAX_RANGE_SIZE

[Jan Luebbe]

The macro name is too generic, so add a AURORA_ prefix.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
 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

[RFC PATCH 3/7] EDAC: Add missing debugfs_create_x32 wrapper

[Jan Luebbe]

We already have wrappers for x8 and x16.

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 014871e169cc..3d87cb15ab45 100644
--- a/drivers/edac/edac_module.h
+++ b/drivers/edac/edac_module.h
@@ -81,6 +81,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)					{ }
@@ -97,6 +99,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

[RFC PATCH 4/7] EDAC: Add devres helpers for edac_mc_alloc/edac_mc_add_mc(_with_groups)

[Jan Luebbe]

These helpers simplify error handling in the _probe functions and automate
deallocation in the _remove functions.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
 drivers/edac/edac_mc.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/edac/edac_mc.h | 26 +++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 480072139b7a..4d6759222592 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -517,6 +517,33 @@ void edac_mc_free(struct mem_ctl_info *mci)
 }
 EXPORT_SYMBOL_GPL(edac_mc_free);
 
+/**
+ * devm_edac_mc_free() - Internal helper to call edac_mc_free from a devres
+ *	action.
+ */
+static void devm_edac_mc_free(void *mci)
+{
+	edac_mc_free((struct mem_ctl_info *)mci);
+}
+
+struct mem_ctl_info *devm_edac_mc_alloc(struct device *dev,
+                                        unsigned mc_num,
+                                        unsigned n_layers,
+                                        struct edac_mc_layer *layers,
+                                        unsigned sz_pvt)
+{
+	struct mem_ctl_info *mci;
+	mci = edac_mc_alloc(mc_num, n_layers, layers, sz_pvt);
+	if (!mci)
+		return mci;
+
+	if (devm_add_action_or_reset(dev, devm_edac_mc_free, mci))
+		return NULL;
+
+	return mci;
+}
+EXPORT_SYMBOL_GPL(devm_edac_mc_alloc);
+
 bool edac_has_mcs(void)
 {
 	bool ret;
@@ -828,6 +855,32 @@ struct mem_ctl_info *edac_mc_del_mc(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(edac_mc_del_mc);
 
+/**
+ * devm_edac_mc_del_mc() - Internal helper to call edac_mc_del_mc from a devres
+ *	action.
+ */
+static void devm_edac_mc_del_mc(void *dev)
+{
+	edac_mc_del_mc((struct device *)dev);
+}
+
+int devm_edac_mc_add_mc_with_groups(struct device *dev,
+                                    struct mem_ctl_info *mci,
+                                    const struct attribute_group **groups)
+{
+	int ret;
+
+	ret = edac_mc_add_mc_with_groups(mci, groups);
+	if (ret)
+		return ret;
+
+	if (devm_add_action_or_reset(dev, devm_edac_mc_del_mc, dev))
+		return 1;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_edac_mc_add_mc_with_groups);
+
 static void edac_mc_scrub_block(unsigned long page, unsigned long offset,
 				u32 size)
 {
diff --git a/drivers/edac/edac_mc.h b/drivers/edac/edac_mc.h
index 5357800e418d..c89f4301ed3f 100644
--- a/drivers/edac/edac_mc.h
+++ b/drivers/edac/edac_mc.h
@@ -149,6 +149,32 @@ extern int edac_mc_add_mc_with_groups(struct mem_ctl_info *mci,
 extern void edac_mc_free(struct mem_ctl_info *mci);
 
 /**
+ * devm_edac_mc_alloc() - Helper to call edac_mc_alloc() and register it for
+ *	cleanup with devres.
+ *
+ * Returns:
+ *	On success, return a pointer to struct mem_ctl_info pointer;
+ *	%NULL otherwise
+ */
+struct mem_ctl_info *devm_edac_mc_alloc(struct device *dev,
+                                        unsigned mc_num,
+                                        unsigned n_layers,
+                                        struct edac_mc_layer *layers,
+                                        unsigned sz_pvt);
+
+/**
+ * devm_edac_mc_add_mc_with_groups() - Helper to call
+ *	edac_mc_add_mc_with_groups() and register it for cleanup with devres.
+ *
+ * Returns:
+ *	0 on Success, or an error code on failure
+ */
+int devm_edac_mc_add_mc_with_groups(struct device *dev,
+                                    struct mem_ctl_info *mci,
+                                    const struct attribute_group **groups);
+#define devm_edac_mc_add_mc(dev, mci)	devm_edac_mc_add_mc_with_groups(dev, mci, NULL)
+
+/**
  * edac_has_mcs() - Check if any MCs have been allocated.
  *
  * Returns:
-- 
2.11.0

[RFC PATCH 5/7] EDAC: Add devres helpers for edac_device_alloc_ctl_info/edac_device_add_device

[Jan Luebbe]

These helpers simplify error handling in the _probe functions and
automate deallocation in the _remove functions.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
 drivers/edac/edac_device.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/edac/edac_device.h | 29 +++++++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index 65cf2b9355c4..bfc4144840cf 100644
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -232,6 +232,41 @@ void edac_device_free_ctl_info(struct edac_device_ctl_info *ctl_info)
 }
 EXPORT_SYMBOL_GPL(edac_device_free_ctl_info);
 
+/**
+ * devm_edac_device_free_ctl_info() - Internal helper to call
+ *	edac_device_free_ctl_info from a devres action
+ */
+static void devm_edac_device_free_ctl_info(void *ctl_info)
+{
+	edac_device_free_ctl_info((struct edac_device_ctl_info *)ctl_info);
+}
+
+struct edac_device_ctl_info *devm_edac_device_alloc_ctl_info(
+	struct device *dev,
+	unsigned sz_private,
+	char *edac_device_name, unsigned nr_instances,
+	char *edac_block_name, unsigned nr_blocks,
+	unsigned offset_value,
+	struct edac_dev_sysfs_block_attribute *attrib_spec, unsigned nr_attrib,
+	int device_index)
+{
+	struct edac_device_ctl_info *ctl_info;
+	ctl_info = edac_device_alloc_ctl_info(sz_private,
+			edac_device_name, nr_instances,
+			edac_block_name, nr_blocks,
+			offset_value,
+			attrib_spec, nr_attrib,
+			device_index);
+	if (!ctl_info)
+		return ctl_info;
+
+	if (devm_add_action_or_reset(dev, devm_edac_device_free_ctl_info, ctl_info))
+		return NULL;
+
+	return ctl_info;
+}
+EXPORT_SYMBOL_GPL(devm_edac_device_alloc_ctl_info);
+
 /*
  * find_edac_device_by_dev
  *	scans the edac_device list for a specific 'struct device *'
@@ -539,6 +574,30 @@ struct edac_device_ctl_info *edac_device_del_device(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(edac_device_del_device);
 
+/**
+ * devm_edac_device_del_device() - Internal helper to call
+ *	edac_device_del_device from a devres action
+ */
+static void devm_edac_device_del_device(void *dev)
+{
+	edac_device_del_device((struct device *)dev);
+}
+
+int devm_edac_device_add_device(struct device *dev,
+		struct edac_device_ctl_info *edac_dev)
+{
+	int ret;
+
+	ret = edac_device_add_device(edac_dev);
+	if (ret)
+		return ret;
+
+	if (devm_add_action_or_reset(dev, devm_edac_device_del_device, dev))
+		return 1;
+
+	return 0;
+}
+
 static inline int edac_device_get_log_ce(struct edac_device_ctl_info *edac_dev)
 {
 	return edac_dev->log_ce;
diff --git a/drivers/edac/edac_device.h b/drivers/edac/edac_device.h
index 1aaba74ae411..73f7b98e8ec0 100644
--- a/drivers/edac/edac_device.h
+++ b/drivers/edac/edac_device.h
@@ -258,6 +258,24 @@ extern struct edac_device_ctl_info *edac_device_alloc_ctl_info(
 extern void edac_device_free_ctl_info(struct edac_device_ctl_info *ctl_info);
 
 /**
+ * devm_edac_device_alloc_ctl_info: Allocate a new managed edac device
+ *	control info structure
+ *
+ *	This function takes the same arguments as edac_device_alloc_ctl_info in
+ *	addition to the owning device. The structure is freed by devres using
+ *	edac_device_free_ctl_info.
+ */
+extern struct edac_device_ctl_info *devm_edac_device_alloc_ctl_info(
+		struct device *dev,
+		unsigned sizeof_private,
+		char *edac_device_name, unsigned nr_instances,
+		char *edac_block_name, unsigned nr_blocks,
+		unsigned offset_value,
+		struct edac_dev_sysfs_block_attribute *block_attributes,
+		unsigned nr_attribs,
+		int device_index);
+
+/**
  * edac_device_add_device: Insert the 'edac_dev' structure into the
  *	 edac_device global list and create sysfs entries associated with
  *	 edac_device structure.
@@ -286,6 +304,17 @@ extern int edac_device_add_device(struct edac_device_ctl_info *edac_dev);
 extern struct edac_device_ctl_info *edac_device_del_device(struct device *dev);
 
 /**
+ * devm_edac_device_add_device:
+ *	Helper to call edac_device_add_device and register it for cleanup with
+ *	devres.
+ *
+ * Returns:
+ *	0 on Success, or an error code on failure
+ */
+extern int devm_edac_device_add_device(struct device *dev,
+				       struct edac_device_ctl_info *edac_dev);
+
+/**
  * edac_device_handle_ue():
  *	perform a common output and handling of an 'edac_dev' UE event
  *
-- 
2.11.0

[RFC PATCH 6/7] EDAC: Add driver for the AURORA L2 cache

[Jan Luebbe]

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
 arch/arm/include/asm/hardware/cache-aurora-l2.h |  49 +++++
 drivers/edac/Kconfig                            |   7 +
 drivers/edac/Makefile                           |   1 +
 drivers/edac/aurora_l2_edac.c                   | 252 ++++++++++++++++++++++++
 4 files changed, 309 insertions(+)
 create mode 100644 drivers/edac/aurora_l2_edac.c

diff --git a/arch/arm/include/asm/hardware/cache-aurora-l2.h b/arch/arm/include/asm/hardware/cache-aurora-l2.h
index dc5c479ec4c3..80e4aad136df 100644
--- a/arch/arm/include/asm/hardware/cache-aurora-l2.h
+++ b/arch/arm/include/asm/hardware/cache-aurora-l2.h
@@ -14,6 +14,10 @@
 #ifndef __ASM_ARM_HARDWARE_AURORA_L2_H
 #define __ASM_ARM_HARDWARE_AURORA_L2_H
 
+/* Parity and ECC are configured in L2X0_AUX_CTRL */
+#define AURORA_CTRL_PARITY_EN	(1 << 21)
+#define AURORA_CTRL_ECC_EN	(1 << 20)
+
 #define AURORA_SYNC_REG		    0x700
 #define AURORA_RANGE_BASE_ADDR_REG  0x720
 #define AURORA_FLUSH_PHY_ADDR_REG   0x7f0
@@ -41,6 +45,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
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 96afb2aeed18..8d9f680c8545 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_AURORA_L2
+	bool "Marvell AURORA L2 Cache ECC"
+	depends on ARCH_MVEBU
+	help
+	  Support for error correction and detection on the Marvell AURORA L2
+	  cache controller (as found on ARMADA XP).
+
 config EDAC_SYNOPSYS
 	tristate "Synopsys DDR Memory Controller"
 	depends on ARCH_ZYNQ
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 0fd9ffa63299..04654da04fc9 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_AURORA_L2)		+= aurora_l2_edac.o
 obj-$(CONFIG_EDAC_SYNOPSYS)		+= synopsys_edac.o
 obj-$(CONFIG_EDAC_XGENE)		+= xgene_edac.o
diff --git a/drivers/edac/aurora_l2_edac.c b/drivers/edac/aurora_l2_edac.c
new file mode 100644
index 000000000000..8bb0ca03e5de
--- /dev/null
+++ b/drivers/edac/aurora_l2_edac.c
@@ -0,0 +1,252 @@
+/*
+ * 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_device.h"
+#include "edac_module.h"
+
+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;
+		switch ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_SOURCE_MASK)
+			>> AURORA_ERR_ATTR_CAP_ERR_SOURCE_OFFSET) {
+		case 0:
+			msg += sprintf(msg, "src=CPU0 ");
+			break;
+		case 1:
+			msg += sprintf(msg, "src=CPU1 ");
+			break;
+		case 2:
+			msg += sprintf(msg, "src=CPU2 ");
+			break;
+		case 3:
+			msg += sprintf(msg, "src=CPU3 ");
+			break;
+		case 7:
+			msg += sprintf(msg, "src=IO ");
+			break;
+		}
+		/* msg size <= 9 */
+		switch ((attr_cap & AURORA_ERR_ATTR_CAP_TRANS_TYPE_MASK)
+			>> AURORA_ERR_ATTR_CAP_TRANS_TYPE_OFFSET) {
+		case 0:
+			msg += sprintf(msg, "txn=Data-Read ");
+			break;
+		case 1:
+			msg += sprintf(msg, "txn=Isn-Read ");
+			break;
+		case 2:
+			msg += sprintf(msg, "txn=Clean-Flush ");
+			break;
+		case 3:
+			msg += sprintf(msg, "txn=Eviction ");
+			break;
+		case 4:
+			msg += sprintf(msg, "txn=Read-Modify-Write ");
+			break;
+		}
+		/* msg size <= 31 */
+		switch ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_TYPE_MASK)
+			>> AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET) {
+		case 0:
+			msg += sprintf(msg, "err=CorrECC ");
+			break;
+		case 1:
+			msg += sprintf(msg, "err=UnCorrECC ");
+			break;
+		case 2:
+			msg += sprintf(msg, "err=TagParity ");
+			break;
+		}
+		/* msg size <= 45 */
+		msg +=
+		    sprintf(msg, "addr=0x%x ",
+			    addr_cap & AURORA_ERR_ADDR_CAP_ADDR_MASK);
+		msg +=
+		    sprintf(msg, "index=0x%x ",
+			    (way_cap & AURORA_ERR_WAY_CAP_INDEX_MASK)
+			    >> AURORA_ERR_WAY_CAP_INDEX_OFFSET);
+		msg +=
+		    sprintf(msg, "way=0x%x",
+			    (way_cap & AURORA_ERR_WAY_CAP_WAY_MASK)
+			    >> AURORA_ERR_WAY_CAP_WAY_OFFSET);
+		/* msg size <= 92 */
+		/* 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)");
+
+	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;
+	uint32_t l2x0_aux_ctrl;
+
+	dci =
+	    devm_edac_device_alloc_ctl_info(&pdev->dev, sizeof(*drvdata), "cpu",
+					    1, "L", 1, 2, NULL, 0, 0);
+	if (!dci)
+		return -ENOMEM;
+
+	drvdata = dci->pvt_info;
+	dci->dev = &pdev->dev;
+	platform_set_drvdata(pdev, dci);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		dev_err(&pdev->dev, "Unable to get mem resource\n");
+		return -ENODEV;
+	}
+
+	drvdata->base = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(drvdata->base)) {
+		dev_err(&pdev->dev, "Unable to map regs\n");
+		return PTR_ERR(drvdata->base);
+	}
+
+	l2x0_aux_ctrl = readl(drvdata->base + L2X0_AUX_CTRL);
+	if (!(l2x0_aux_ctrl & AURORA_CTRL_PARITY_EN))
+		dev_warn(&pdev->dev, "tag parity is not enabled");
+	if (!(l2x0_aux_ctrl & AURORA_CTRL_ECC_EN))
+		dev_warn(&pdev->dev, "data ECC is not enabled");
+
+	id = of_match_device(aurora_l2_edac_of_match, &pdev->dev);
+	dci->edac_check = aurora_l2_edac_check;
+	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 (devm_edac_device_add_device(&pdev->dev, dci))
+		return -EINVAL;
+
+	drvdata->debugfs = edac_debugfs_create_dir(dev_name(&pdev->dev));
+	if (drvdata->debugfs) {
+		edac_debugfs_create_x32("inject_addr", S_IRUGO | S_IWUSR,
+					drvdata->debugfs,
+					&drvdata->inject_addr);
+		edac_debugfs_create_x32("inject_mask", S_IRUGO | S_IWUSR,
+					drvdata->debugfs,
+					&drvdata->inject_mask);
+		edac_debugfs_create_x8("inject_ctl", S_IRUGO | S_IWUSR,
+				       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);
+	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 = aurora_l2_edac_of_match,
+	},
+};
+
+module_platform_driver(aurora_l2_edac_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Pengutronix");
+MODULE_DESCRIPTION("EDAC Driver for Marvell Aurora L2 Cache");
-- 
2.11.0

[RFC PATCH 7/7] EDAC: Add driver for the Marvell Armada XP SDRAM controller

[Jan Luebbe]

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
 drivers/edac/Kconfig             |   7 +
 drivers/edac/Makefile            |   1 +
 drivers/edac/armada_xp_mc_edac.c | 366 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 374 insertions(+)
 create mode 100644 drivers/edac/armada_xp_mc_edac.c

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 8d9f680c8545..4189061489af 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 Memory Controller ECC"
+	depends on ARCH_MVEBU
+	help
+	  Support for error correction and detection on the Marvell Aramada XP
+	  memory controller.
+
 config EDAC_AURORA_L2
 	bool "Marvell AURORA L2 Cache ECC"
 	depends on ARCH_MVEBU
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 04654da04fc9..3497404e6d97 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -76,6 +76,7 @@ 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_mc_edac.o
 obj-$(CONFIG_EDAC_AURORA_L2)		+= aurora_l2_edac.o
 obj-$(CONFIG_EDAC_SYNOPSYS)		+= synopsys_edac.o
 obj-$(CONFIG_EDAC_XGENE)		+= xgene_edac.o
diff --git a/drivers/edac/armada_xp_mc_edac.c b/drivers/edac/armada_xp_mc_edac.c
new file mode 100644
index 000000000000..2ac298227d9c
--- /dev/null
+++ b/drivers/edac/armada_xp_mc_edac.c
@@ -0,0 +1,366 @@
+/*
+ * 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 "edac_mc.h"
+#include "edac_module.h"
+
+#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;
+
+	bool full_width; /* 32 or 64 bit */
+	bool cs_addr_sel[SDRAM_NUM_CS]; /* bank interleaving */
+
+	char msg[128];
+};
+
+/* derived from "DRAM Address Multiplexing" in the ARAMDA XP Functonal 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->full_width) { /* 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 { /* 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));
+	}
+}
+
+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 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_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_ECC_MASK))
+		dev_warn(mci->pdev, "SDRAM ECC is not enabled");
+
+	if (mci->tot_dimms != SDRAM_NUM_CS) {
+		dev_err(mci->pdev, "Invaild number of DIMMs");
+		return -EINVAL;
+	}
+
+	drvdata->full_width = !!(config & SDRAM_CONFIG_BUS_WIDTH_MASK);
+
+	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;
+	}
+
+	return 0;
+}
+
+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;
+
+	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
+	layers[0].size = SDRAM_NUM_CS;
+	layers[0].is_virt_csrow = true;
+
+	mci = devm_edac_mc_alloc(&pdev->dev, 0, ARRAY_SIZE(layers), layers,
+				 sizeof(*drvdata));
+	if (!mci)
+		return -ENOMEM;
+
+	drvdata = mci->pvt_info;
+	mci->pdev = &pdev->dev;
+	platform_set_drvdata(pdev, mci);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		dev_err(&pdev->dev, "Unable to get mem resource\n");
+		return -ENODEV;
+	}
+
+	drvdata->base = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(drvdata->base)) {
+		dev_err(&pdev->dev, "Unable to map regs\n");
+		return PTR_ERR(drvdata->base);
+	}
+
+	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;
+
+	if (armada_xp_mc_edac_read_config(mci))
+		return -EINVAL;
+
+	/* 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 (devm_edac_mc_add_mc(&pdev->dev, mci))
+		return -EINVAL;
+	edac_op_state = EDAC_OPSTATE_POLL;
+
+	return 0;
+}
+
+static struct platform_driver armada_xp_mc_edac_driver = {
+	.probe = armada_xp_mc_edac_probe,
+	.driver = {
+		.name = "armada_xp_mc_edac",
+		.of_match_table = armada_xp_mc_edac_of_match,
+	},
+};
+
+module_platform_driver(armada_xp_mc_edac_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Pengutronix");
+MODULE_DESCRIPTION("EDAC Driver for Marvell Armada XP SDRAM Controller");
-- 
2.11.0

[RFC PATCH 0/7] EDAC drivers for Armada XP L2 and DDR

[Borislav Petkov]

On Fri, Jun 09, 2017 at 10:31:16AM +0200, Jan Luebbe wrote:
> This series add 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 are devm_ helpers for edac_mc_/edac_device_
> allocation and registration, which make error handing and cleanup simpler. They
> may already be mergeable.
> 
> It seems Chris and I had a race condition, he posted a driver for this
> functionality just yesterday. Compared to his submission, the L2 and DDR
> support is split into two drivers for this series, as they don't actually share
> any functionality.
> 
> Some further differences in this series are:
> - The error details are decoded and passed to the edac error handler.
> - Multiple errors are counted even if the details are unavailable.
> - The DDR RAM configuration is read back to fill out the DIMM structures.
> - The DDR RAM error address is calculated from the bank/row/col information.
> - The L2 injection registers are exposed via debugfs instead of sysfs
>   (resulting in less driver code).
> 
> Chris, how do you want to proceed? We should probably combine our efforts in one
> series. I've already picked up part of DDR RAM config reading from your series
> for this submission. I'll send some questions/comments to your series, as well.
> 
> Jan Luebbe (7):
>   ARM: l2c: move cache-aurora-l2.h to asm/hardware
>   ARM: aurora-l2: add prefix to MAX_RANGE_SIZE
>   EDAC: Add missing debugfs_create_x32 wrapper
>   EDAC: Add devres helpers for
>     edac_mc_alloc/edac_mc_add_mc(_with_groups)
>   EDAC: Add devres helpers for
>     edac_device_alloc_ctl_info/edac_device_add_device
>   EDAC: Add driver for the AURORA L2 cache
>   EDAC: Add driver for the Marvell Armada XP SDRAM controller
> 
>  arch/arm/include/asm/hardware/cache-aurora-l2.h | 104 +++++++
>  arch/arm/mm/cache-aurora-l2.h                   |  55 ----
>  arch/arm/mm/cache-l2x0.c                        |   6 +-
>  drivers/edac/Kconfig                            |  14 +
>  drivers/edac/Makefile                           |   2 +
>  drivers/edac/armada_xp_mc_edac.c                | 366 ++++++++++++++++++++++++
>  drivers/edac/aurora_l2_edac.c                   | 252 ++++++++++++++++

Before we go any further, pls put all the Marvell RAS functionality
into a single file - I don't want a separate compilation unit per an IP
block.

Also, some of your commit messages are empty and I'm sure they could use
some text.

And yes, it'd be very productive not to get conflicting submissions from
you and Chris. :-)

Thanks.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[RFC PATCH 0/7] EDAC drivers for Armada XP L2 and DDR

[Jan Lübbe]

Hi Borislav,

On Fr, 2017-06-09 at 11:36 +0200, Borislav Petkov wrote:
> Before we go any further, pls put all the Marvell RAS functionality
> into a single file - I don't want a separate compilation unit per an
> IP block.
The L2 cache and DDR RAM controller are completely separate IP cores,
with separate register spaces, interrupts and so on. The cache is an ARM
design (some L2x0) extended by Marvell, while the DDR controller seems
to be unique to Marvell. It wouldn't surprise me if Marvell would decide
to reuse the DDR controller in another SoC using newer ARM cores with a
different cache.

> Also, some of your commit messages are empty and I'm sure they could
> use some text.
Sure, I mainly wanted to start the discussion with Chris. :-)

> And yes, it'd be very productive not to get conflicting submissions
> from you and Chris. :-)
Of course!

Regards,
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 |

[RFC PATCH 0/7] EDAC drivers for Armada XP L2 and DDR

[Borislav Petkov]

On Fri, Jun 09, 2017 at 03:23:48PM +0200, Jan L?bbe wrote:
> The L2 cache and DDR RAM controller are completely separate IP cores,
> with separate register spaces, interrupts and so on. The cache is an ARM
> design (some L2x0) extended by Marvell, while the DDR controller seems
> to be unique to Marvell. It wouldn't surprise me if Marvell would decide
> to reuse the DDR controller in another SoC using newer ARM cores with a
> different cache.

That's fine, we can carve it out then. We've done stuff like that
already, see drivers/edac/fsl_ddr_edac.c, for example.

So think of an EDAC driver as a platform driver. If a platform uses IP
from a different platform, then we mirror this in the driver design by
sharing compilation modules.

Thanks.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[RFC PATCH 0/7] EDAC drivers for Armada XP L2 and DDR

[Chris Packham]

Hi Jan,

On 09/06/17 20:53, Jan Luebbe wrote:
> This series add 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 are devm_ helpers for edac_mc_/edac_device_
> allocation and registration, which make error handing and cleanup simpler. They
> may already be mergeable.
> 
> It seems Chris and I had a race condition, he posted a driver for this
> functionality just yesterday. Compared to his submission, the L2 and DDR
> support is split into two drivers for this series, as they don't actually share
> any functionality.

I think the DDR controller is also applicable to Armada-38x (which is 
why I called it mvebu in my series). The L2 cache is different between 
Armada-XP and Armada-38x so it makes sense to split them.

> 
> Some further differences in this series are:
> - The error details are decoded and passed to the edac error handler.
> - Multiple errors are counted even if the details are unavailable.
> - The DDR RAM configuration is read back to fill out the DIMM structures.
> - The DDR RAM error address is calculated from the bank/row/col information.
> - The L2 injection registers are exposed via debugfs instead of sysfs
>    (resulting in less driver code).
> 
> Chris, how do you want to proceed? We should probably combine our efforts in one
> series. I've already picked up part of DDR RAM config reading from your series
> for this submission. I'll send some questions/comments to your series, as well.

I'm more than happy for you to take the lead on this one. I can test on 
a couple of different 98dx3236 boards (switch chips with integrated 
Armada-XP).

> 
> Jan Luebbe (7):
>    ARM: l2c: move cache-aurora-l2.h to asm/hardware
>    ARM: aurora-l2: add prefix to MAX_RANGE_SIZE
>    EDAC: Add missing debugfs_create_x32 wrapper
>    EDAC: Add devres helpers for
>      edac_mc_alloc/edac_mc_add_mc(_with_groups)
>    EDAC: Add devres helpers for
>      edac_device_alloc_ctl_info/edac_device_add_device
>    EDAC: Add driver for the AURORA L2 cache
>    EDAC: Add driver for the Marvell Armada XP SDRAM controller
> 
>   arch/arm/include/asm/hardware/cache-aurora-l2.h | 104 +++++++
>   arch/arm/mm/cache-aurora-l2.h                   |  55 ----
>   arch/arm/mm/cache-l2x0.c                        |   6 +-
>   drivers/edac/Kconfig                            |  14 +
>   drivers/edac/Makefile                           |   2 +
>   drivers/edac/armada_xp_mc_edac.c                | 366 ++++++++++++++++++++++++
>   drivers/edac/aurora_l2_edac.c                   | 252 ++++++++++++++++
>   drivers/edac/debugfs.c                          |  11 +
>   drivers/edac/edac_device.c                      |  59 ++++
>   drivers/edac/edac_device.h                      |  29 ++
>   drivers/edac/edac_mc.c                          |  53 ++++
>   drivers/edac/edac_mc.h                          |  26 ++
>   drivers/edac/edac_module.h                      |   5 +
>   13 files changed, 924 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_mc_edac.c
>   create mode 100644 drivers/edac/aurora_l2_edac.c
> 

[RFC PATCH 7/7] EDAC: Add driver for the Marvell Armada XP SDRAM controller

[Chris Packham]

On 09/06/17 20:53, Jan Luebbe wrote:
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> ---
>   drivers/edac/Kconfig             |   7 +
>   drivers/edac/Makefile            |   1 +
>   drivers/edac/armada_xp_mc_edac.c | 366 +++++++++++++++++++++++++++++++++++++++
>   3 files changed, 374 insertions(+)
>   create mode 100644 drivers/edac/armada_xp_mc_edac.c
> 
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 8d9f680c8545..4189061489af 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 Memory Controller ECC"
> +	depends on ARCH_MVEBU
> +	help
> +	  Support for error correction and detection on the Marvell Aramada XP
> +	  memory controller.
> +
>   config EDAC_AURORA_L2
>   	bool "Marvell AURORA L2 Cache ECC"
>   	depends on ARCH_MVEBU
> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
> index 04654da04fc9..3497404e6d97 100644
> --- a/drivers/edac/Makefile
> +++ b/drivers/edac/Makefile
> @@ -76,6 +76,7 @@ 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_mc_edac.o
>   obj-$(CONFIG_EDAC_AURORA_L2)		+= aurora_l2_edac.o
>   obj-$(CONFIG_EDAC_SYNOPSYS)		+= synopsys_edac.o
>   obj-$(CONFIG_EDAC_XGENE)		+= xgene_edac.o
> diff --git a/drivers/edac/armada_xp_mc_edac.c b/drivers/edac/armada_xp_mc_edac.c
> new file mode 100644
> index 000000000000..2ac298227d9c
> --- /dev/null
> +++ b/drivers/edac/armada_xp_mc_edac.c
> @@ -0,0 +1,366 @@
> +/*
> + * 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 "edac_mc.h"
> +#include "edac_module.h"
> +
> +#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;
> +
> +	bool full_width; /* 32 or 64 bit */

16-bit is used on the integrated version so this might need to be an 
enum. Or at least the presumption that true means 64-bit and false means 
32-bit or less.

> +	bool cs_addr_sel[SDRAM_NUM_CS]; /* bank interleaving */
> +
> +	char msg[128];
> +};
> +
> +/* derived from "DRAM Address Multiplexing" in the ARAMDA XP Functonal 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->full_width) { /* 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 { /* 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));
> +	}
> +}
> +
> +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 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_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_ECC_MASK))
> +		dev_warn(mci->pdev, "SDRAM ECC is not enabled");
> +
> +	if (mci->tot_dimms != SDRAM_NUM_CS) {
> +		dev_err(mci->pdev, "Invaild number of DIMMs");
> +		return -EINVAL;
> +	}
> +
> +	drvdata->full_width = !!(config & SDRAM_CONFIG_BUS_WIDTH_MASK);
> +
> +	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;
> +	}
> +
> +	return 0;
> +}
> +
> +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;
> +
> +	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
> +	layers[0].size = SDRAM_NUM_CS;
> +	layers[0].is_virt_csrow = true;
> +
> +	mci = devm_edac_mc_alloc(&pdev->dev, 0, ARRAY_SIZE(layers), layers,
> +				 sizeof(*drvdata));
> +	if (!mci)
> +		return -ENOMEM;
> +
> +	drvdata = mci->pvt_info;
> +	mci->pdev = &pdev->dev;
> +	platform_set_drvdata(pdev, mci);
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!r) {
> +		dev_err(&pdev->dev, "Unable to get mem resource\n");
> +		return -ENODEV;
> +	}
> +
> +	drvdata->base = devm_ioremap_resource(&pdev->dev, r);
> +	if (IS_ERR(drvdata->base)) {
> +		dev_err(&pdev->dev, "Unable to map regs\n");
> +		return PTR_ERR(drvdata->base);
> +	}
> +
> +	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;
> +
> +	if (armada_xp_mc_edac_read_config(mci))
> +		return -EINVAL;
> +
> +	/* 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 (devm_edac_mc_add_mc(&pdev->dev, mci))
> +		return -EINVAL;
> +	edac_op_state = EDAC_OPSTATE_POLL;

In theory interrupt mode is possible. I've just yet to decipher the 
datasheet. It can be added later if/when we figure it out.

> +
> +	return 0;
> +}
> +
> +static struct platform_driver armada_xp_mc_edac_driver = {
> +	.probe = armada_xp_mc_edac_probe,
> +	.driver = {
> +		.name = "armada_xp_mc_edac",
> +		.of_match_table = armada_xp_mc_edac_of_match,
> +	},
> +};
> +
> +module_platform_driver(armada_xp_mc_edac_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Pengutronix");
> +MODULE_DESCRIPTION("EDAC Driver for Marvell Armada XP SDRAM Controller");
> 

[RFC PATCH 6/7] EDAC: Add driver for the AURORA L2 cache

[Chris Packham]

Hi Jan,

Couple of suggestions below.

On 09/06/17 20:53, Jan Luebbe wrote:
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> ---
>   arch/arm/include/asm/hardware/cache-aurora-l2.h |  49 +++++
>   drivers/edac/Kconfig                            |   7 +
>   drivers/edac/Makefile                           |   1 +
>   drivers/edac/aurora_l2_edac.c                   | 252 ++++++++++++++++++++++++
>   4 files changed, 309 insertions(+)
>   create mode 100644 drivers/edac/aurora_l2_edac.c
> 
> diff --git a/arch/arm/include/asm/hardware/cache-aurora-l2.h b/arch/arm/include/asm/hardware/cache-aurora-l2.h
> index dc5c479ec4c3..80e4aad136df 100644
> --- a/arch/arm/include/asm/hardware/cache-aurora-l2.h
> +++ b/arch/arm/include/asm/hardware/cache-aurora-l2.h
> @@ -14,6 +14,10 @@
>   #ifndef __ASM_ARM_HARDWARE_AURORA_L2_H
>   #define __ASM_ARM_HARDWARE_AURORA_L2_H
>   
> +/* Parity and ECC are configured in L2X0_AUX_CTRL */
> +#define AURORA_CTRL_PARITY_EN	(1 << 21)
> +#define AURORA_CTRL_ECC_EN	(1 << 20)
> +
>   #define AURORA_SYNC_REG		    0x700
>   #define AURORA_RANGE_BASE_ADDR_REG  0x720
>   #define AURORA_FLUSH_PHY_ADDR_REG   0x7f0
> @@ -41,6 +45,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

In my version I re-sized the resource to absorb the 0x600 offset 
(something I borrowed from mpc85xx_edac.c). I'm not sure if it's a good 
practice but that may be one way around the problem of sharing the 
memory region with other drivers.

> +
> +#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
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 96afb2aeed18..8d9f680c8545 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_AURORA_L2
> +	bool "Marvell AURORA L2 Cache ECC"
> +	depends on ARCH_MVEBU

Does this also need to depend on OF or is that a given these days?

> +	help
> +	  Support for error correction and detection on the Marvell AURORA L2
> +	  cache controller (as found on ARMADA XP).
> +
>   config EDAC_SYNOPSYS
>   	tristate "Synopsys DDR Memory Controller"
>   	depends on ARCH_ZYNQ
> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
> index 0fd9ffa63299..04654da04fc9 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_AURORA_L2)		+= aurora_l2_edac.o
>   obj-$(CONFIG_EDAC_SYNOPSYS)		+= synopsys_edac.o
>   obj-$(CONFIG_EDAC_XGENE)		+= xgene_edac.o
> diff --git a/drivers/edac/aurora_l2_edac.c b/drivers/edac/aurora_l2_edac.c
> new file mode 100644
> index 000000000000..8bb0ca03e5de
> --- /dev/null
> +++ b/drivers/edac/aurora_l2_edac.c
> @@ -0,0 +1,252 @@
> +/*
> + * 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_device.h"
> +#include "edac_module.h"
> +
> +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;

Is it worth tracking the size of msg and using snprintf?

		size_t len = sizeof(drvdata->msg);
		int n = 0;
...
		n += snprintf(msg+n, len-n, "src=CPU0 ");

> +		switch ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_SOURCE_MASK)
> +			>> AURORA_ERR_ATTR_CAP_ERR_SOURCE_OFFSET) {
> +		case 0:
> +			msg += sprintf(msg, "src=CPU0 ");
> +			break;
> +		case 1:
> +			msg += sprintf(msg, "src=CPU1 ");
> +			break;
> +		case 2:
> +			msg += sprintf(msg, "src=CPU2 ");
> +			break;
> +		case 3:
> +			msg += sprintf(msg, "src=CPU3 ");
> +			break;
> +		case 7:
> +			msg += sprintf(msg, "src=IO ");
> +			break;
> +		}
> +		/* msg size <= 9 */
> +		switch ((attr_cap & AURORA_ERR_ATTR_CAP_TRANS_TYPE_MASK)
> +			>> AURORA_ERR_ATTR_CAP_TRANS_TYPE_OFFSET) {
> +		case 0:
> +			msg += sprintf(msg, "txn=Data-Read ");
> +			break;
> +		case 1:
> +			msg += sprintf(msg, "txn=Isn-Read ");
> +			break;
> +		case 2:
> +			msg += sprintf(msg, "txn=Clean-Flush ");
> +			break;
> +		case 3:
> +			msg += sprintf(msg, "txn=Eviction ");
> +			break;
> +		case 4:
> +			msg += sprintf(msg, "txn=Read-Modify-Write ");
> +			break;
> +		}
> +		/* msg size <= 31 */
> +		switch ((attr_cap & AURORA_ERR_ATTR_CAP_ERR_TYPE_MASK)
> +			>> AURORA_ERR_ATTR_CAP_ERR_TYPE_OFFSET) {
> +		case 0:
> +			msg += sprintf(msg, "err=CorrECC ");
> +			break;
> +		case 1:
> +			msg += sprintf(msg, "err=UnCorrECC ");
> +			break;
> +		case 2:
> +			msg += sprintf(msg, "err=TagParity ");
> +			break;
> +		}
> +		/* msg size <= 45 */
> +		msg +=
> +		    sprintf(msg, "addr=0x%x ",
> +			    addr_cap & AURORA_ERR_ADDR_CAP_ADDR_MASK);
> +		msg +=
> +		    sprintf(msg, "index=0x%x ",
> +			    (way_cap & AURORA_ERR_WAY_CAP_INDEX_MASK)
> +			    >> AURORA_ERR_WAY_CAP_INDEX_OFFSET);
> +		msg +=
> +		    sprintf(msg, "way=0x%x",
> +			    (way_cap & AURORA_ERR_WAY_CAP_WAY_MASK)
> +			    >> AURORA_ERR_WAY_CAP_WAY_OFFSET);
> +		/* msg size <= 92 */
> +		/* 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)");
> +
> +	aurora_l2_edac_inject(drvdata);

If/when this supports interrupts we'd need to consider how to do this 
(another debugfs file?). Perhaps it's worth creating 
aurora_l2_edac_poll() which would be implemented as

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;
> +	uint32_t l2x0_aux_ctrl;
> +
> +	dci =
> +	    devm_edac_device_alloc_ctl_info(&pdev->dev, sizeof(*drvdata), "cpu",
> +					    1, "L", 1, 2, NULL, 0, 0);
> +	if (!dci)
> +		return -ENOMEM;
> +
> +	drvdata = dci->pvt_info;
> +	dci->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, dci);
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!r) {
> +		dev_err(&pdev->dev, "Unable to get mem resource\n");
> +		return -ENODEV;
> +	}
> +
> +	drvdata->base = devm_ioremap_resource(&pdev->dev, r);
> +	if (IS_ERR(drvdata->base)) {
> +		dev_err(&pdev->dev, "Unable to map regs\n");
> +		return PTR_ERR(drvdata->base);
> +	}
> +
> +	l2x0_aux_ctrl = readl(drvdata->base + L2X0_AUX_CTRL);
> +	if (!(l2x0_aux_ctrl & AURORA_CTRL_PARITY_EN))
> +		dev_warn(&pdev->dev, "tag parity is not enabled");
> +	if (!(l2x0_aux_ctrl & AURORA_CTRL_ECC_EN))
> +		dev_warn(&pdev->dev, "data ECC is not enabled");
> +
> +	id = of_match_device(aurora_l2_edac_of_match, &pdev->dev);
> +	dci->edac_check = aurora_l2_edac_check;
> +	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 (devm_edac_device_add_device(&pdev->dev, dci))
> +		return -EINVAL;
> +
> +	drvdata->debugfs = edac_debugfs_create_dir(dev_name(&pdev->dev));
> +	if (drvdata->debugfs) {
> +		edac_debugfs_create_x32("inject_addr", S_IRUGO | S_IWUSR,
> +					drvdata->debugfs,
> +					&drvdata->inject_addr);
> +		edac_debugfs_create_x32("inject_mask", S_IRUGO | S_IWUSR,
> +					drvdata->debugfs,
> +					&drvdata->inject_mask);
> +		edac_debugfs_create_x8("inject_ctl", S_IRUGO | S_IWUSR,
> +				       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);
> +	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 = aurora_l2_edac_of_match,

	.of_match_table = of_match_ptr(aurora_l2_edac_of_match), ??

I don't know if we care about non-OF configs for this driver. The SDRAM 
one on the other hand does have at least one non-OF candidate (MV78200).

> +	},
> +};
> +
> +module_platform_driver(aurora_l2_edac_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Pengutronix");
> +MODULE_DESCRIPTION("EDAC Driver for Marvell Aurora L2 Cache");
>

With or without any of my suggestions

Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

[RFC PATCH 7/7] EDAC: Add driver for the Marvell Armada XP SDRAM controller

[Chris Packham]

On 09/06/17 20:53, Jan Luebbe wrote:
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> ---
>   drivers/edac/Kconfig             |   7 +
>   drivers/edac/Makefile            |   1 +
>   drivers/edac/armada_xp_mc_edac.c | 366 +++++++++++++++++++++++++++++++++++++++
>   3 files changed, 374 insertions(+)
>   create mode 100644 drivers/edac/armada_xp_mc_edac.c
> 
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 8d9f680c8545..4189061489af 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 Memory Controller ECC"
> +	depends on ARCH_MVEBU
> +	help
> +	  Support for error correction and detection on the Marvell Aramada XP
> +	  memory controller.
> +
>   config EDAC_AURORA_L2
>   	bool "Marvell AURORA L2 Cache ECC"
>   	depends on ARCH_MVEBU
> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
> index 04654da04fc9..3497404e6d97 100644
> --- a/drivers/edac/Makefile
> +++ b/drivers/edac/Makefile
> @@ -76,6 +76,7 @@ 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_mc_edac.o
>   obj-$(CONFIG_EDAC_AURORA_L2)		+= aurora_l2_edac.o
>   obj-$(CONFIG_EDAC_SYNOPSYS)		+= synopsys_edac.o
>   obj-$(CONFIG_EDAC_XGENE)		+= xgene_edac.o
> diff --git a/drivers/edac/armada_xp_mc_edac.c b/drivers/edac/armada_xp_mc_edac.c
> new file mode 100644
> index 000000000000..2ac298227d9c
> --- /dev/null
> +++ b/drivers/edac/armada_xp_mc_edac.c
> @@ -0,0 +1,366 @@
> +/*
> + * 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 "edac_mc.h"
> +#include "edac_module.h"
> +
> +#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;
> +
> +	bool full_width; /* 32 or 64 bit */
> +	bool cs_addr_sel[SDRAM_NUM_CS]; /* bank interleaving */
> +
> +	char msg[128];
> +};
> +
> +/* derived from "DRAM Address Multiplexing" in the ARAMDA XP Functonal Spec */

s/Functonal/Functional/

> +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->full_width) { /* 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 { /* 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));
> +	}

Here's my version for the 16-bit interface from the 98X3236 Control and 
Management Functional Spec

	/* 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));

I think I followed your derivation against the Armada XP spec. I can 
provide the relevant tables off-line if you want to double-check them.
		
> +}
> +
> +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 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_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_ECC_MASK))
> +		dev_warn(mci->pdev, "SDRAM ECC is not enabled");
> +
> +	if (mci->tot_dimms != SDRAM_NUM_CS) {
> +		dev_err(mci->pdev, "Invaild number of DIMMs");
> +		return -EINVAL;
> +	}
> +
> +	drvdata->full_width = !!(config & SDRAM_CONFIG_BUS_WIDTH_MASK);
> +
> +	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;
> +	}
> +
> +	return 0;
> +}
> +
> +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;
> +
> +	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
> +	layers[0].size = SDRAM_NUM_CS;
> +	layers[0].is_virt_csrow = true;
> +
> +	mci = devm_edac_mc_alloc(&pdev->dev, 0, ARRAY_SIZE(layers), layers,
> +				 sizeof(*drvdata));
> +	if (!mci)
> +		return -ENOMEM;
> +
> +	drvdata = mci->pvt_info;
> +	mci->pdev = &pdev->dev;
> +	platform_set_drvdata(pdev, mci);
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!r) {
> +		dev_err(&pdev->dev, "Unable to get mem resource\n");
> +		return -ENODEV;
> +	}
> +
> +	drvdata->base = devm_ioremap_resource(&pdev->dev, r);
> +	if (IS_ERR(drvdata->base)) {
> +		dev_err(&pdev->dev, "Unable to map regs\n");
> +		return PTR_ERR(drvdata->base);
> +	}
> +
> +	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;
> +
> +	if (armada_xp_mc_edac_read_config(mci))
> +		return -EINVAL;
> +
> +	/* 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 (devm_edac_mc_add_mc(&pdev->dev, mci))
> +		return -EINVAL;
> +	edac_op_state = EDAC_OPSTATE_POLL;
> +
> +	return 0;
> +}
> +
> +static struct platform_driver armada_xp_mc_edac_driver = {
> +	.probe = armada_xp_mc_edac_probe,
> +	.driver = {
> +		.name = "armada_xp_mc_edac",
> +		.of_match_table = armada_xp_mc_edac_of_match,

	.of_match_table = of_match_ptr(armada_xp_mc_edac_of_match),

> +	},
> +};
> +
> +module_platform_driver(armada_xp_mc_edac_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Pengutronix");
> +MODULE_DESCRIPTION("EDAC Driver for Marvell Armada XP SDRAM Controller");
> 

[RFC PATCH 0/7] EDAC drivers for Armada XP L2 and DDR

[Borislav Petkov]

On Sun, Jun 11, 2017 at 10:07:23PM +0000, Chris Packham wrote:
> I'm more than happy for you to take the lead on this one. I can test on 
> a couple of different 98dx3236 boards (switch chips with integrated 
> Armada-XP).

Right, and I'd need an entry for this driver in MAINTAINERS so that I
know who to CC on bugs.

Thanks.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.