* [PATCH 0/7] Add Aspeed AST2700 SDRAM EDAC support
@ 2026-08-12 5:48 Ryan Chen
2026-08-12 5:48 ` [PATCH 1/7] dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC Ryan Chen
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: Ryan Chen @ 2026-08-12 5:48 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck, Sebastian Andrzej Siewior, Clark Williams,
Steven Rostedt
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, linux-rt-devel, Ryan Chen, stable
Add ECC error reporting for the Aspeed AST2700 SoC to the existing
aspeed_edac driver.
The AST2700 memory controller keeps the same overall EDAC programming
model as the earlier Aspeed BMC SoCs, but uses a different register
layout, a split interrupt status/clear/mask scheme and DDR4/DDR5 memory.
Rather than fork the driver, the existing code is first tidied and
generalised, then the AST2700 is added as one more per-SoC variant.
The series is organised as:
- dt-bindings: document the new "aspeed,ast2700-sdram-edac" compatible;
- set dimm->grain to the controllers' ECC granularity, fixing a
pre-existing WARN_ON in the EDAC core on the first reported error
(carries a Fixes: tag and Cc: stable);
- clean up whitespace and include ordering so the following changes
start from a consistent style;
- free the mem_ctl_info unconditionally on remove, dropping a NULL
check on a value that cannot be NULL there;
- drop the regmap, which serves only as an MMIO wrapper here,
in favour of direct readl()/writel() under an explicit raw spinlock,
annotating the register base with __guarded_by() so that the locking
is checked at build time under CONFIG_WARN_CONTEXT_ANALYSIS;
- abstract the SoC-specific details (register layout, ECC/DRAM-type
bits, memory types, write-protection key) behind per-SoC chip data;
- add the AST2700 support, teaching the shared error-reporting helpers
to report an error without an address for its single shared
failure-address register.
Tested with the memory controller's ECC error injection on both an
existing SoC and the new one:
- AST2600: an injected correctable error is reported through the EDAC
interface as a CE with the recorded failure address, confirming the
regmap removal and the chip-data refactor do not regress the existing
controllers.
- AST2700: an injected correctable error is likewise reported as a CE
with the expected failure address.
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Ryan Chen (7):
dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC
EDAC/aspeed: Set the DIMM grain
EDAC/aspeed: Clean up whitespace and include ordering
EDAC/aspeed: Free the mem_ctl_info unconditionally on remove
EDAC/aspeed: Replace regmap with direct register access
EDAC/aspeed: Abstract SoC differences behind chip data
EDAC/aspeed: Add AST2700 support
.../bindings/edac/aspeed,ast2400-sdram-edac.yaml | 6 +-
drivers/edac/aspeed_edac.c | 414 +++++++++++++--------
2 files changed, 268 insertions(+), 152 deletions(-)
---
base-commit: 5464985e42c04e335fb30e38fbc409c997db9bec
change-id: 20260625-edac-8e960e02e7f7
Best regards,
--
Ryan Chen <ryan_chen@aspeedtech.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/7] dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC
2026-08-12 5:48 [PATCH 0/7] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
@ 2026-08-12 5:48 ` Ryan Chen
2026-08-12 5:48 ` [PATCH 2/7] EDAC/aspeed: Set the DIMM grain Ryan Chen
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Ryan Chen @ 2026-08-12 5:48 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck, Sebastian Andrzej Siewior, Clark Williams,
Steven Rostedt
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, linux-rt-devel, Ryan Chen
Add the "aspeed,ast2700-sdram-edac" compatible and note DDR5 support.
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
.../devicetree/bindings/edac/aspeed,ast2400-sdram-edac.yaml | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/edac/aspeed,ast2400-sdram-edac.yaml b/Documentation/devicetree/bindings/edac/aspeed,ast2400-sdram-edac.yaml
index 09735826d707..685b6815f293 100644
--- a/Documentation/devicetree/bindings/edac/aspeed,ast2400-sdram-edac.yaml
+++ b/Documentation/devicetree/bindings/edac/aspeed,ast2400-sdram-edac.yaml
@@ -10,8 +10,9 @@ maintainers:
- Stefan Schaeckeler <sschaeck@cisco.com>
description: >
- The Aspeed BMC SoC supports DDR3 and DDR4 memory with and without ECC (error
- correction check).
+ The Aspeed BMC SoCs support DDR memory with and without ECC (error
+ correction check): DDR3 and DDR4 on the AST2400, AST2500 and AST2600,
+ and DDR4 and DDR5 on the AST2700.
The memory controller supports SECDED (single bit error correction, double bit
error detection) and single bit error auto scrubbing by reserving 8 bits for
@@ -25,6 +26,7 @@ properties:
- aspeed,ast2400-sdram-edac
- aspeed,ast2500-sdram-edac
- aspeed,ast2600-sdram-edac
+ - aspeed,ast2700-sdram-edac
reg:
maxItems: 1
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/7] EDAC/aspeed: Set the DIMM grain
2026-08-12 5:48 [PATCH 0/7] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
2026-08-12 5:48 ` [PATCH 1/7] dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC Ryan Chen
@ 2026-08-12 5:48 ` Ryan Chen
2026-08-12 5:48 ` [PATCH 3/7] EDAC/aspeed: Clean up whitespace and include ordering Ryan Chen
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Ryan Chen @ 2026-08-12 5:48 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck, Sebastian Andrzej Siewior, Clark Williams,
Steven Rostedt
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, linux-rt-devel, Ryan Chen, stable
The driver never sets dimm->grain, leaving it zero. Since commit
3724ace582d9 ("EDAC/mc: Fix grain_bits calculation")
edac_raw_mc_handle_error() runs WARN_ON_ONCE(!e->grain) and forces the
grain to 1, so the first ECC error reported on any Aspeed BMC SoC emits a
warning splat, e.g. on the AST2600:
WARNING: CPU: 0 PID: 0 at drivers/edac/edac_mc.c:924 edac_raw_mc_handle_error+0x4b4/0x604
...
edac_raw_mc_handle_error from edac_mc_handle_error+0x364/0x4a8
edac_mc_handle_error from count_rec+0xdc/0x124
count_rec from mcr_isr+0x110/0x1e8
Fixes: 9b7e6242ee4e ("EDAC, aspeed: Add an Aspeed AST2500 EDAC driver")
Cc: stable@vger.kernel.org
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
drivers/edac/aspeed_edac.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
index 6e069b255595..83d60414f89a 100644
--- a/drivers/edac/aspeed_edac.c
+++ b/drivers/edac/aspeed_edac.c
@@ -266,6 +266,7 @@ static int init_csrows(struct mem_ctl_info *mci)
dimm->mtype = dram_type;
dimm->edac_mode = EDAC_SECDED;
dimm->nr_pages = nr_pages / csrow->nr_channels;
+ dimm->grain = 16;
dev_dbg(mci->pdev, "initialized dimm with first_page=0x%lx and nr_pages=0x%x\n",
csrow->first_page, nr_pages);
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/7] EDAC/aspeed: Clean up whitespace and include ordering
2026-08-12 5:48 [PATCH 0/7] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
2026-08-12 5:48 ` [PATCH 1/7] dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC Ryan Chen
2026-08-12 5:48 ` [PATCH 2/7] EDAC/aspeed: Set the DIMM grain Ryan Chen
@ 2026-08-12 5:48 ` Ryan Chen
2026-08-12 5:48 ` [PATCH 4/7] EDAC/aspeed: Free the mem_ctl_info unconditionally on remove Ryan Chen
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Ryan Chen @ 2026-08-12 5:48 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck, Sebastian Andrzej Siewior, Clark Williams,
Steven Rostedt
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, linux-rt-devel, Ryan Chen
The driver separates functions and definition groups with two blank
lines where the kernel style uses one, its headers are not sorted, and it
includes linux/stop_machine.h without using it. Collapse the double blank
lines, drop the unused include and sort the rest alphabetically so the
following changes start from a consistent style.
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
drivers/edac/aspeed_edac.c | 34 +++++++++-------------------------
1 file changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
index 83d60414f89a..71535e0b6bad 100644
--- a/drivers/edac/aspeed_edac.c
+++ b/drivers/edac/aspeed_edac.c
@@ -4,20 +4,17 @@
*/
#include <linux/edac.h>
-#include <linux/module.h>
#include <linux/init.h>
#include <linux/interrupt.h>
-#include <linux/platform_device.h>
-#include <linux/stop_machine.h>
#include <linux/io.h>
+#include <linux/module.h>
#include <linux/of_address.h>
+#include <linux/platform_device.h>
#include <linux/regmap.h>
#include "edac_module.h"
-
#define DRV_NAME "aspeed-edac"
-
#define ASPEED_MCR_PROT 0x00 /* protection key register */
#define ASPEED_MCR_CONF 0x04 /* configuration register */
#define ASPEED_MCR_INTR_CTRL 0x50 /* interrupt control/status register */
@@ -25,19 +22,16 @@
#define ASPEED_MCR_ADDR_REC 0x5c /* address of last recoverable error */
#define ASPEED_MCR_LAST ASPEED_MCR_ADDR_REC
-
-#define ASPEED_MCR_PROT_PASSWD 0xfc600309
-#define ASPEED_MCR_CONF_DRAM_TYPE BIT(4)
-#define ASPEED_MCR_CONF_ECC BIT(7)
-#define ASPEED_MCR_INTR_CTRL_CLEAR BIT(31)
-#define ASPEED_MCR_INTR_CTRL_CNT_REC GENMASK(23, 16)
-#define ASPEED_MCR_INTR_CTRL_CNT_UNREC GENMASK(15, 12)
-#define ASPEED_MCR_INTR_CTRL_ENABLE (BIT(0) | BIT(1))
-
+#define ASPEED_MCR_PROT_PASSWD 0xfc600309
+#define ASPEED_MCR_CONF_DRAM_TYPE BIT(4)
+#define ASPEED_MCR_CONF_ECC BIT(7)
+#define ASPEED_MCR_INTR_CTRL_CLEAR BIT(31)
+#define ASPEED_MCR_INTR_CTRL_CNT_REC GENMASK(23, 16)
+#define ASPEED_MCR_INTR_CTRL_CNT_UNREC GENMASK(15, 12)
+#define ASPEED_MCR_INTR_CTRL_ENABLE (BIT(0) | BIT(1))
static struct regmap *aspeed_regmap;
-
static int regmap_reg_write(void *context, unsigned int reg, unsigned int val)
{
void __iomem *regs = (void __iomem *)context;
@@ -53,7 +47,6 @@ static int regmap_reg_write(void *context, unsigned int reg, unsigned int val)
return 0;
}
-
static int regmap_reg_read(void *context, unsigned int reg, unsigned int *val)
{
void __iomem *regs = (void __iomem *)context;
@@ -76,7 +69,6 @@ static bool regmap_is_volatile(struct device *dev, unsigned int reg)
}
}
-
static const struct regmap_config aspeed_regmap_config = {
.reg_bits = 32,
.val_bits = 32,
@@ -88,7 +80,6 @@ static const struct regmap_config aspeed_regmap_config = {
.fast_io = true,
};
-
static void count_rec(struct mem_ctl_info *mci, u8 rec_cnt, u32 rec_addr)
{
struct csrow_info *csrow = mci->csrows[0];
@@ -120,7 +111,6 @@ static void count_rec(struct mem_ctl_info *mci, u8 rec_cnt, u32 rec_addr)
0, 0, -1, "", "");
}
-
static void count_un_rec(struct mem_ctl_info *mci, u8 un_rec_cnt,
u32 un_rec_addr)
{
@@ -153,7 +143,6 @@ static void count_un_rec(struct mem_ctl_info *mci, u8 un_rec_cnt,
}
}
-
static irqreturn_t mcr_isr(int irq, void *arg)
{
struct mem_ctl_info *mci = arg;
@@ -200,7 +189,6 @@ static irqreturn_t mcr_isr(int irq, void *arg)
return IRQ_HANDLED;
}
-
static int config_irq(void *ctx, struct platform_device *pdev)
{
int irq;
@@ -225,7 +213,6 @@ static int config_irq(void *ctx, struct platform_device *pdev)
return 0;
}
-
static int init_csrows(struct mem_ctl_info *mci)
{
struct csrow_info *csrow = mci->csrows[0];
@@ -274,7 +261,6 @@ static int init_csrows(struct mem_ctl_info *mci)
return 0;
}
-
static int aspeed_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -355,7 +341,6 @@ static int aspeed_probe(struct platform_device *pdev)
return rc;
}
-
static void aspeed_remove(struct platform_device *pdev)
{
struct mem_ctl_info *mci;
@@ -370,7 +355,6 @@ static void aspeed_remove(struct platform_device *pdev)
edac_mc_free(mci);
}
-
static const struct of_device_id aspeed_of_match[] = {
{ .compatible = "aspeed,ast2400-sdram-edac" },
{ .compatible = "aspeed,ast2500-sdram-edac" },
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/7] EDAC/aspeed: Free the mem_ctl_info unconditionally on remove
2026-08-12 5:48 [PATCH 0/7] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (2 preceding siblings ...)
2026-08-12 5:48 ` [PATCH 3/7] EDAC/aspeed: Clean up whitespace and include ordering Ryan Chen
@ 2026-08-12 5:48 ` Ryan Chen
2026-08-12 5:48 ` [PATCH 5/7] EDAC/aspeed: Replace regmap with direct register access Ryan Chen
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Ryan Chen @ 2026-08-12 5:48 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck, Sebastian Andrzej Siewior, Clark Williams,
Steven Rostedt
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, linux-rt-devel, Ryan Chen
aspeed_remove() is a driver .remove callback, so it only runs for a device
that has already probed successfully and registered its mem_ctl_info. In
that case edac_mc_del_mc() always returns the same, valid pointer, and the
NULL check on its return value can never be false.
Fetch the mem_ctl_info from the platform device's driver data instead and
free it unconditionally, dropping the redundant check. This also decouples
the teardown from the return value of edac_mc_del_mc().
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
drivers/edac/aspeed_edac.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
index 71535e0b6bad..8bfeb21d3204 100644
--- a/drivers/edac/aspeed_edac.c
+++ b/drivers/edac/aspeed_edac.c
@@ -343,16 +343,15 @@ static int aspeed_probe(struct platform_device *pdev)
static void aspeed_remove(struct platform_device *pdev)
{
- struct mem_ctl_info *mci;
+ struct mem_ctl_info *mci = platform_get_drvdata(pdev);
/* disable interrupts */
regmap_update_bits(aspeed_regmap, ASPEED_MCR_INTR_CTRL,
ASPEED_MCR_INTR_CTRL_ENABLE, 0);
/* free resources */
- mci = edac_mc_del_mc(&pdev->dev);
- if (mci)
- edac_mc_free(mci);
+ edac_mc_del_mc(&pdev->dev);
+ edac_mc_free(mci);
}
static const struct of_device_id aspeed_of_match[] = {
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 5/7] EDAC/aspeed: Replace regmap with direct register access
2026-08-12 5:48 [PATCH 0/7] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (3 preceding siblings ...)
2026-08-12 5:48 ` [PATCH 4/7] EDAC/aspeed: Free the mem_ctl_info unconditionally on remove Ryan Chen
@ 2026-08-12 5:48 ` Ryan Chen
2026-08-12 5:48 ` [PATCH 6/7] EDAC/aspeed: Abstract SoC differences behind chip data Ryan Chen
2026-08-12 5:48 ` [PATCH 7/7] EDAC/aspeed: Add AST2700 support Ryan Chen
6 siblings, 0 replies; 8+ messages in thread
From: Ryan Chen @ 2026-08-12 5:48 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck, Sebastian Andrzej Siewior, Clark Williams,
Steven Rostedt
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, linux-rt-devel, Ryan Chen
The driver instantiates its own regmap purely as an MMIO wrapper: it has
no register cache, uses custom reg_read/reg_write callbacks, and is not
shared as a syscon with other drivers. So it brings nothing here beyond
the spinlock that regmap takes around each access when fast_io is set.
Drop the regmap and access the registers directly with readl()/writel()
under an explicit raw spinlock, held across the whole read-modify-write so
the controller is unlocked once around the grouped writes rather than on
every register write. The lock is a raw_spinlock_t because the ECC
interrupt handler runs in hardirq context, where under PREEMPT_RT a
sleeping spinlock could not be acquired.
Annotate the register base with __guarded_by() so that, under
CONFIG_WARN_CONTEXT_ANALYSIS, the compiler checks at build time that every
hardware register access is performed while holding the lock.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
drivers/edac/aspeed_edac.c | 132 +++++++++++++++++----------------------------
1 file changed, 48 insertions(+), 84 deletions(-)
diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
index 8bfeb21d3204..7bd552ee9a61 100644
--- a/drivers/edac/aspeed_edac.c
+++ b/drivers/edac/aspeed_edac.c
@@ -3,6 +3,7 @@
* Copyright 2018, 2019 Cisco Systems
*/
+#include <linux/cleanup.h>
#include <linux/edac.h>
#include <linux/init.h>
#include <linux/interrupt.h>
@@ -10,7 +11,7 @@
#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/platform_device.h>
-#include <linux/regmap.h>
+#include <linux/spinlock.h>
#include "edac_module.h"
#define DRV_NAME "aspeed-edac"
@@ -20,7 +21,6 @@
#define ASPEED_MCR_INTR_CTRL 0x50 /* interrupt control/status register */
#define ASPEED_MCR_ADDR_UNREC 0x58 /* address of first un-recoverable error */
#define ASPEED_MCR_ADDR_REC 0x5c /* address of last recoverable error */
-#define ASPEED_MCR_LAST ASPEED_MCR_ADDR_REC
#define ASPEED_MCR_PROT_PASSWD 0xfc600309
#define ASPEED_MCR_CONF_DRAM_TYPE BIT(4)
@@ -30,55 +30,8 @@
#define ASPEED_MCR_INTR_CTRL_CNT_UNREC GENMASK(15, 12)
#define ASPEED_MCR_INTR_CTRL_ENABLE (BIT(0) | BIT(1))
-static struct regmap *aspeed_regmap;
-
-static int regmap_reg_write(void *context, unsigned int reg, unsigned int val)
-{
- void __iomem *regs = (void __iomem *)context;
-
- /* enable write to MCR register set */
- writel(ASPEED_MCR_PROT_PASSWD, regs + ASPEED_MCR_PROT);
-
- writel(val, regs + reg);
-
- /* disable write to MCR register set */
- writel(~ASPEED_MCR_PROT_PASSWD, regs + ASPEED_MCR_PROT);
-
- return 0;
-}
-
-static int regmap_reg_read(void *context, unsigned int reg, unsigned int *val)
-{
- void __iomem *regs = (void __iomem *)context;
-
- *val = readl(regs + reg);
-
- return 0;
-}
-
-static bool regmap_is_volatile(struct device *dev, unsigned int reg)
-{
- switch (reg) {
- case ASPEED_MCR_PROT:
- case ASPEED_MCR_INTR_CTRL:
- case ASPEED_MCR_ADDR_UNREC:
- case ASPEED_MCR_ADDR_REC:
- return true;
- default:
- return false;
- }
-}
-
-static const struct regmap_config aspeed_regmap_config = {
- .reg_bits = 32,
- .val_bits = 32,
- .reg_stride = 4,
- .max_register = ASPEED_MCR_LAST,
- .reg_write = regmap_reg_write,
- .reg_read = regmap_reg_read,
- .volatile_reg = regmap_is_volatile,
- .fast_io = true,
-};
+static DEFINE_RAW_SPINLOCK(aspeed_lock);
+static void __iomem *aspeed_regs __guarded_by(&aspeed_lock);
static void count_rec(struct mem_ctl_info *mci, u8 rec_cnt, u32 rec_addr)
{
@@ -147,12 +100,24 @@ static irqreturn_t mcr_isr(int irq, void *arg)
{
struct mem_ctl_info *mci = arg;
u32 rec_addr, un_rec_addr;
- u32 reg50, reg5c, reg58;
- u8 rec_cnt, un_rec_cnt;
-
- regmap_read(aspeed_regmap, ASPEED_MCR_INTR_CTRL, ®50);
- dev_dbg(mci->pdev, "received edac interrupt w/ mcr register 50: 0x%x\n",
- reg50);
+ u8 rec_cnt, un_rec_cnt;
+ u32 reg50;
+
+ scoped_guard(raw_spinlock, &aspeed_lock) {
+ reg50 = readl(aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ dev_dbg(mci->pdev, "received edac interrupt w/ mcr register 50: 0x%x\n",
+ reg50);
+ un_rec_addr = readl(aspeed_regs + ASPEED_MCR_ADDR_UNREC);
+ rec_addr = readl(aspeed_regs + ASPEED_MCR_ADDR_REC);
+
+ /* clearing the counters needs a set-then-clear of CLEAR */
+ writel(ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+ writel(reg50 | ASPEED_MCR_INTR_CTRL_CLEAR,
+ aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ writel(reg50 & ~ASPEED_MCR_INTR_CTRL_CLEAR,
+ aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ writel(~ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+ }
/* collect data about recoverable and unrecoverable errors */
rec_cnt = (reg50 & ASPEED_MCR_INTR_CTRL_CNT_REC) >> 16;
@@ -161,20 +126,6 @@ static irqreturn_t mcr_isr(int irq, void *arg)
dev_dbg(mci->pdev, "%d recoverable interrupts and %d unrecoverable interrupts\n",
rec_cnt, un_rec_cnt);
- regmap_read(aspeed_regmap, ASPEED_MCR_ADDR_UNREC, ®58);
- un_rec_addr = reg58;
-
- regmap_read(aspeed_regmap, ASPEED_MCR_ADDR_REC, ®5c);
- rec_addr = reg5c;
-
- /* clear interrupt flags and error counters: */
- regmap_update_bits(aspeed_regmap, ASPEED_MCR_INTR_CTRL,
- ASPEED_MCR_INTR_CTRL_CLEAR,
- ASPEED_MCR_INTR_CTRL_CLEAR);
-
- regmap_update_bits(aspeed_regmap, ASPEED_MCR_INTR_CTRL,
- ASPEED_MCR_INTR_CTRL_CLEAR, 0);
-
/* process recoverable and unrecoverable errors */
count_rec(mci, rec_cnt, rec_addr);
count_un_rec(mci, un_rec_cnt, un_rec_addr);
@@ -182,13 +133,31 @@ static irqreturn_t mcr_isr(int irq, void *arg)
if (!rec_cnt && !un_rec_cnt)
dev_dbg(mci->pdev, "received edac interrupt, but did not find any ECC counters\n");
- regmap_read(aspeed_regmap, ASPEED_MCR_INTR_CTRL, ®50);
+ scoped_guard(raw_spinlock, &aspeed_lock)
+ reg50 = readl(aspeed_regs + ASPEED_MCR_INTR_CTRL);
dev_dbg(mci->pdev, "edac interrupt handled. mcr reg 50 is now: 0x%x\n",
reg50);
return IRQ_HANDLED;
}
+static void aspeed_set_irq(bool enable)
+{
+ u32 val;
+
+ guard(raw_spinlock_irqsave)(&aspeed_lock);
+
+ val = readl(aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ if (enable)
+ val |= ASPEED_MCR_INTR_CTRL_ENABLE;
+ else
+ val &= ~ASPEED_MCR_INTR_CTRL_ENABLE;
+
+ writel(ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+ writel(val, aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ writel(~ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+}
+
static int config_irq(void *ctx, struct platform_device *pdev)
{
int irq;
@@ -206,9 +175,7 @@ static int config_irq(void *ctx, struct platform_device *pdev)
return rc;
/* enable interrupts */
- regmap_update_bits(aspeed_regmap, ASPEED_MCR_INTR_CTRL,
- ASPEED_MCR_INTR_CTRL_ENABLE,
- ASPEED_MCR_INTR_CTRL_ENABLE);
+ aspeed_set_irq(true);
return 0;
}
@@ -246,7 +213,8 @@ static int init_csrows(struct mem_ctl_info *mci)
nr_pages = resource_size(&r) >> PAGE_SHIFT;
csrow->last_page = csrow->first_page + nr_pages - 1;
- regmap_read(aspeed_regmap, ASPEED_MCR_CONF, ®04);
+ scoped_guard(raw_spinlock, &aspeed_lock)
+ reg04 = readl(aspeed_regs + ASPEED_MCR_CONF);
dram_type = (reg04 & ASPEED_MCR_CONF_DRAM_TYPE) ? MEM_DDR4 : MEM_DDR3;
dimm = csrow->channels[0]->dimm;
@@ -263,7 +231,6 @@ static int init_csrows(struct mem_ctl_info *mci)
static int aspeed_probe(struct platform_device *pdev)
{
- struct device *dev = &pdev->dev;
struct edac_mc_layer layers[2];
struct mem_ctl_info *mci;
void __iomem *regs;
@@ -274,13 +241,11 @@ static int aspeed_probe(struct platform_device *pdev)
if (IS_ERR(regs))
return PTR_ERR(regs);
- aspeed_regmap = devm_regmap_init(dev, NULL, (__force void *)regs,
- &aspeed_regmap_config);
- if (IS_ERR(aspeed_regmap))
- return PTR_ERR(aspeed_regmap);
+ scoped_guard(raw_spinlock, &aspeed_lock)
+ aspeed_regs = regs;
/* bail out if ECC mode is not configured */
- regmap_read(aspeed_regmap, ASPEED_MCR_CONF, ®04);
+ reg04 = readl(regs + ASPEED_MCR_CONF);
if (!(reg04 & ASPEED_MCR_CONF_ECC)) {
dev_err(&pdev->dev, "ECC mode is not configured in u-boot\n");
return -EPERM;
@@ -346,8 +311,7 @@ static void aspeed_remove(struct platform_device *pdev)
struct mem_ctl_info *mci = platform_get_drvdata(pdev);
/* disable interrupts */
- regmap_update_bits(aspeed_regmap, ASPEED_MCR_INTR_CTRL,
- ASPEED_MCR_INTR_CTRL_ENABLE, 0);
+ aspeed_set_irq(false);
/* free resources */
edac_mc_del_mc(&pdev->dev);
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 6/7] EDAC/aspeed: Abstract SoC differences behind chip data
2026-08-12 5:48 [PATCH 0/7] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (4 preceding siblings ...)
2026-08-12 5:48 ` [PATCH 5/7] EDAC/aspeed: Replace regmap with direct register access Ryan Chen
@ 2026-08-12 5:48 ` Ryan Chen
2026-08-12 5:48 ` [PATCH 7/7] EDAC/aspeed: Add AST2700 support Ryan Chen
6 siblings, 0 replies; 8+ messages in thread
From: Ryan Chen @ 2026-08-12 5:48 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck, Sebastian Andrzej Siewior, Clark Williams,
Steven Rostedt
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, linux-rt-devel, Ryan Chen
The driver hard-codes the AST2400/2500/2600 register layout, ECC and
DRAM-type bits, memory types and write-protection key. Abstract these
SoC-specific details behind a per-SoC struct aspeed_edac_chip selected by
the compatible, and move the per-instance state (register base, lock)
into mci->pvt_info instead of globals, so controller variants that differ
in these details can be added as table data rather than by forking the
driver.
The __guarded_by() annotation on the register base moves with it, so the
build-time check that every access holds the lock is kept across the
conversion.
Only the AST2400 and AST2500 key-protect the interrupt control register
(MCR50); the AST2600 does not. Gate the unlock/relock on the chip carrying
a protection key and split the shared entry into keyed (AST2400/2500) and
unkeyed (AST2600) variants, so the AST2600 no longer performs the
unnecessary unlock.
Tested on an AST2600: A correctable error was injected from the console by
unlocking the controller and writing its ECC error inject test register:
# mw 1e6e0000 fc600309
# mw 1e6e00b0 81
EDAC MC0: 1 CE address(es) not available on mc#0csrow#0channel#0 (csrow:0 channel:0 page:0x0 offset:0x0 grain:16 syndrome:0x0)
EDAC MC0: 1 CE on mc#0csrow#0channel#0 (csrow:0 channel:0 page:0x8a543 offset:0xec0 grain:16 syndrome:0x0)
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
drivers/edac/aspeed_edac.c | 154 ++++++++++++++++++++++++++++++++-------------
1 file changed, 109 insertions(+), 45 deletions(-)
diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
index 7bd552ee9a61..91df5d2df5f1 100644
--- a/drivers/edac/aspeed_edac.c
+++ b/drivers/edac/aspeed_edac.c
@@ -3,12 +3,14 @@
* Copyright 2018, 2019 Cisco Systems
*/
+#include <linux/bitfield.h>
#include <linux/cleanup.h>
#include <linux/edac.h>
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/platform_device.h>
#include <linux/spinlock.h>
@@ -30,8 +32,22 @@
#define ASPEED_MCR_INTR_CTRL_CNT_UNREC GENMASK(15, 12)
#define ASPEED_MCR_INTR_CTRL_ENABLE (BIT(0) | BIT(1))
-static DEFINE_RAW_SPINLOCK(aspeed_lock);
-static void __iomem *aspeed_regs __guarded_by(&aspeed_lock);
+struct aspeed_edac_chip {
+ unsigned int conf_reg;
+ u32 conf_ecc;
+ u32 conf_dram_type;
+ enum mem_type dram_type[2];
+ unsigned long mtype_cap;
+ unsigned int prot_reg;
+ u32 prot_key;
+};
+
+struct aspeed_edac {
+ raw_spinlock_t lock;
+
+ void __iomem *regs __guarded_by(&lock);
+ const struct aspeed_edac_chip *chip;
+};
static void count_rec(struct mem_ctl_info *mci, u8 rec_cnt, u32 rec_addr)
{
@@ -96,32 +112,49 @@ static void count_un_rec(struct mem_ctl_info *mci, u8 un_rec_cnt,
}
}
-static irqreturn_t mcr_isr(int irq, void *arg)
+static void aspeed_mcr_irq_update_enter(struct aspeed_edac *priv)
+ __must_hold(&priv->lock)
+{
+ if (priv->chip->prot_key)
+ writel(priv->chip->prot_key, priv->regs + priv->chip->prot_reg);
+}
+
+static void aspeed_mcr_irq_update_exit(struct aspeed_edac *priv)
+ __must_hold(&priv->lock)
+{
+ if (priv->chip->prot_key)
+ writel(~priv->chip->prot_key, priv->regs + priv->chip->prot_reg);
+}
+
+static irqreturn_t aspeed_mcr_isr(int irq, void *arg)
{
struct mem_ctl_info *mci = arg;
u32 rec_addr, un_rec_addr;
+ struct aspeed_edac *priv;
u8 rec_cnt, un_rec_cnt;
u32 reg50;
- scoped_guard(raw_spinlock, &aspeed_lock) {
- reg50 = readl(aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ priv = mci->pvt_info;
+
+ scoped_guard(raw_spinlock, &priv->lock) {
+ reg50 = readl(priv->regs + ASPEED_MCR_INTR_CTRL);
dev_dbg(mci->pdev, "received edac interrupt w/ mcr register 50: 0x%x\n",
reg50);
- un_rec_addr = readl(aspeed_regs + ASPEED_MCR_ADDR_UNREC);
- rec_addr = readl(aspeed_regs + ASPEED_MCR_ADDR_REC);
+ un_rec_addr = readl(priv->regs + ASPEED_MCR_ADDR_UNREC);
+ rec_addr = readl(priv->regs + ASPEED_MCR_ADDR_REC);
/* clearing the counters needs a set-then-clear of CLEAR */
- writel(ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+ aspeed_mcr_irq_update_enter(priv);
writel(reg50 | ASPEED_MCR_INTR_CTRL_CLEAR,
- aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ priv->regs + ASPEED_MCR_INTR_CTRL);
writel(reg50 & ~ASPEED_MCR_INTR_CTRL_CLEAR,
- aspeed_regs + ASPEED_MCR_INTR_CTRL);
- writel(~ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+ priv->regs + ASPEED_MCR_INTR_CTRL);
+ aspeed_mcr_irq_update_exit(priv);
}
/* collect data about recoverable and unrecoverable errors */
- rec_cnt = (reg50 & ASPEED_MCR_INTR_CTRL_CNT_REC) >> 16;
- un_rec_cnt = (reg50 & ASPEED_MCR_INTR_CTRL_CNT_UNREC) >> 12;
+ rec_cnt = FIELD_GET(ASPEED_MCR_INTR_CTRL_CNT_REC, reg50);
+ un_rec_cnt = FIELD_GET(ASPEED_MCR_INTR_CTRL_CNT_UNREC, reg50);
dev_dbg(mci->pdev, "%d recoverable interrupts and %d unrecoverable interrupts\n",
rec_cnt, un_rec_cnt);
@@ -131,34 +164,35 @@ static irqreturn_t mcr_isr(int irq, void *arg)
count_un_rec(mci, un_rec_cnt, un_rec_addr);
if (!rec_cnt && !un_rec_cnt)
- dev_dbg(mci->pdev, "received edac interrupt, but did not find any ECC counters\n");
+ dev_dbg_ratelimited(mci->pdev, "received edac interrupt, but did not find any ECC counters\n");
- scoped_guard(raw_spinlock, &aspeed_lock)
- reg50 = readl(aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ scoped_guard(raw_spinlock, &priv->lock)
+ reg50 = readl(priv->regs + ASPEED_MCR_INTR_CTRL);
dev_dbg(mci->pdev, "edac interrupt handled. mcr reg 50 is now: 0x%x\n",
reg50);
return IRQ_HANDLED;
}
-static void aspeed_set_irq(bool enable)
+static void aspeed_set_irq(struct mem_ctl_info *mci, bool enable)
{
+ struct aspeed_edac *priv = mci->pvt_info;
u32 val;
- guard(raw_spinlock_irqsave)(&aspeed_lock);
+ guard(raw_spinlock_irqsave)(&priv->lock);
- val = readl(aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ val = readl(priv->regs + ASPEED_MCR_INTR_CTRL);
if (enable)
val |= ASPEED_MCR_INTR_CTRL_ENABLE;
else
val &= ~ASPEED_MCR_INTR_CTRL_ENABLE;
- writel(ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
- writel(val, aspeed_regs + ASPEED_MCR_INTR_CTRL);
- writel(~ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+ aspeed_mcr_irq_update_enter(priv);
+ writel(val, priv->regs + ASPEED_MCR_INTR_CTRL);
+ aspeed_mcr_irq_update_exit(priv);
}
-static int config_irq(void *ctx, struct platform_device *pdev)
+static int config_irq(struct mem_ctl_info *mci, struct platform_device *pdev)
{
int irq;
int rc;
@@ -169,13 +203,13 @@ static int config_irq(void *ctx, struct platform_device *pdev)
if (irq < 0)
return irq;
- rc = devm_request_irq(&pdev->dev, irq, mcr_isr, IRQF_TRIGGER_HIGH,
- DRV_NAME, ctx);
+ rc = devm_request_irq(&pdev->dev, irq, aspeed_mcr_isr, IRQF_TRIGGER_HIGH,
+ DRV_NAME, mci);
if (rc)
return rc;
/* enable interrupts */
- aspeed_set_irq(true);
+ aspeed_set_irq(mci, true);
return 0;
}
@@ -183,11 +217,13 @@ static int config_irq(void *ctx, struct platform_device *pdev)
static int init_csrows(struct mem_ctl_info *mci)
{
struct csrow_info *csrow = mci->csrows[0];
- u32 nr_pages, dram_type;
- struct dimm_info *dimm;
+ struct aspeed_edac *priv = mci->pvt_info;
struct device_node *np;
+ struct dimm_info *dimm;
struct resource r;
- u32 reg04;
+ unsigned int type;
+ u32 nr_pages;
+ u32 conf;
int rc;
/* retrieve info about physical memory from device tree */
@@ -213,12 +249,12 @@ static int init_csrows(struct mem_ctl_info *mci)
nr_pages = resource_size(&r) >> PAGE_SHIFT;
csrow->last_page = csrow->first_page + nr_pages - 1;
- scoped_guard(raw_spinlock, &aspeed_lock)
- reg04 = readl(aspeed_regs + ASPEED_MCR_CONF);
- dram_type = (reg04 & ASPEED_MCR_CONF_DRAM_TYPE) ? MEM_DDR4 : MEM_DDR3;
+ scoped_guard(raw_spinlock, &priv->lock)
+ conf = readl(priv->regs + priv->chip->conf_reg);
+ type = field_get(priv->chip->conf_dram_type, conf);
dimm = csrow->channels[0]->dimm;
- dimm->mtype = dram_type;
+ dimm->mtype = priv->chip->dram_type[type];
dimm->edac_mode = EDAC_SECDED;
dimm->nr_pages = nr_pages / csrow->nr_channels;
dimm->grain = 16;
@@ -231,22 +267,26 @@ static int init_csrows(struct mem_ctl_info *mci)
static int aspeed_probe(struct platform_device *pdev)
{
+ const struct aspeed_edac_chip *chip;
+ struct device *dev = &pdev->dev;
struct edac_mc_layer layers[2];
+ struct aspeed_edac *priv;
struct mem_ctl_info *mci;
void __iomem *regs;
- u32 reg04;
+ u32 conf;
int rc;
+ chip = of_device_get_match_data(dev);
+ if (!chip)
+ return -EINVAL;
+
regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(regs))
return PTR_ERR(regs);
- scoped_guard(raw_spinlock, &aspeed_lock)
- aspeed_regs = regs;
-
/* bail out if ECC mode is not configured */
- reg04 = readl(regs + ASPEED_MCR_CONF);
- if (!(reg04 & ASPEED_MCR_CONF_ECC)) {
+ conf = readl(regs + chip->conf_reg);
+ if (!field_get(chip->conf_ecc, conf)) {
dev_err(&pdev->dev, "ECC mode is not configured in u-boot\n");
return -EPERM;
}
@@ -261,12 +301,17 @@ static int aspeed_probe(struct platform_device *pdev)
layers[1].size = 1;
layers[1].is_virt_csrow = false;
- mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, 0);
+ mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*priv));
if (!mci)
return -ENOMEM;
+ priv = mci->pvt_info;
+ priv->chip = chip;
+ scoped_guard(raw_spinlock_init, &priv->lock)
+ priv->regs = regs;
+
mci->pdev = &pdev->dev;
- mci->mtype_cap = MEM_FLAG_DDR3 | MEM_FLAG_DDR4;
+ mci->mtype_cap = chip->mtype_cap;
mci->edac_ctl_cap = EDAC_FLAG_SECDED;
mci->edac_cap = EDAC_FLAG_SECDED;
mci->scrub_cap = SCRUB_FLAG_HW_SRC;
@@ -311,17 +356,36 @@ static void aspeed_remove(struct platform_device *pdev)
struct mem_ctl_info *mci = platform_get_drvdata(pdev);
/* disable interrupts */
- aspeed_set_irq(false);
+ aspeed_set_irq(mci, false);
/* free resources */
edac_mc_del_mc(&pdev->dev);
edac_mc_free(mci);
}
+static const struct aspeed_edac_chip ast2400_edac = {
+ .conf_reg = ASPEED_MCR_CONF,
+ .conf_ecc = ASPEED_MCR_CONF_ECC,
+ .conf_dram_type = ASPEED_MCR_CONF_DRAM_TYPE,
+ .dram_type = { MEM_DDR3, MEM_DDR4 },
+ .mtype_cap = MEM_FLAG_DDR3 | MEM_FLAG_DDR4,
+ .prot_reg = ASPEED_MCR_PROT,
+ .prot_key = ASPEED_MCR_PROT_PASSWD,
+};
+
+/* The AST2600 does not key-protect the interrupt control register (MCR50). */
+static const struct aspeed_edac_chip ast2600_edac = {
+ .conf_reg = ASPEED_MCR_CONF,
+ .conf_ecc = ASPEED_MCR_CONF_ECC,
+ .conf_dram_type = ASPEED_MCR_CONF_DRAM_TYPE,
+ .dram_type = { MEM_DDR3, MEM_DDR4 },
+ .mtype_cap = MEM_FLAG_DDR3 | MEM_FLAG_DDR4,
+};
+
static const struct of_device_id aspeed_of_match[] = {
- { .compatible = "aspeed,ast2400-sdram-edac" },
- { .compatible = "aspeed,ast2500-sdram-edac" },
- { .compatible = "aspeed,ast2600-sdram-edac" },
+ { .compatible = "aspeed,ast2400-sdram-edac", .data = &ast2400_edac },
+ { .compatible = "aspeed,ast2500-sdram-edac", .data = &ast2400_edac },
+ { .compatible = "aspeed,ast2600-sdram-edac", .data = &ast2600_edac },
{},
};
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 7/7] EDAC/aspeed: Add AST2700 support
2026-08-12 5:48 [PATCH 0/7] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (5 preceding siblings ...)
2026-08-12 5:48 ` [PATCH 6/7] EDAC/aspeed: Abstract SoC differences behind chip data Ryan Chen
@ 2026-08-12 5:48 ` Ryan Chen
6 siblings, 0 replies; 8+ messages in thread
From: Ryan Chen @ 2026-08-12 5:48 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck, Sebastian Andrzej Siewior, Clark Williams,
Steven Rostedt
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, linux-rt-devel, Ryan Chen
Add SDRAM ECC reporting for the Aspeed AST2700. Its DRAMC has a different
register layout, a split interrupt status/clear/mask scheme, DDR4/DDR5
memory and interrupt registers that are not key-protected.
Its interrupt status/clear and enable sequences differ from the earlier
SoCs, so add per-chip isr() and set_irq() hooks and route the request_irq
and enable/disable paths through them, keeping the existing AST2400/2500/
2600 behaviour under the shared aspeed_mcr_isr()/aspeed_set_irq().
Unlike the earlier SoCs it records a single failure address shared by
both error types, so extend the shared count_rec()/count_un_rec()
helpers with a have_addr flag to report an error without an address
(existing SoCs pass have_addr = true, unchanged) and widen their address
argument to phys_addr_t as the AST2700 address can exceed 32 bits.
Tested on an AST2700: A correctable error was injected from the console by
unlocking the controller and writing its ECC error inject test register:
# mw 12c00000 1688a8a8
# mw 12c00080 31
EDAC MC0: 1 CE on mc#0csrow#0channel#0 (csrow:0 channel:0 page:0x40f6da offset:0xdb0 grain:16 syndrome:0x0)
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
drivers/edac/aspeed_edac.c | 154 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 128 insertions(+), 26 deletions(-)
diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
index 91df5d2df5f1..8b48044a00d1 100644
--- a/drivers/edac/aspeed_edac.c
+++ b/drivers/edac/aspeed_edac.c
@@ -32,6 +32,19 @@
#define ASPEED_MCR_INTR_CTRL_CNT_UNREC GENMASK(15, 12)
#define ASPEED_MCR_INTR_CTRL_ENABLE (BIT(0) | BIT(1))
+#define AST2700_INT_STS 0x04
+#define AST2700_INT_CLR 0x08
+#define AST2700_INT_MASK 0x0c
+#define AST2700_INT_ECC_RECOVERABLE BIT(5)
+#define AST2700_INT_ECC_UNRECOVERABLE BIT(4)
+#define AST2700_MCFG 0x10
+#define AST2700_MCFG_ECC BIT(6)
+#define AST2700_MCFG_DRAM_TYPE BIT(0) /* 0=DDR4, 1=DDR5 */
+#define AST2700_ECC_STS 0x78
+#define AST2700_ECC_REC_CNT GENMASK(15, 8)
+#define AST2700_ECC_UNREC_CNT GENMASK(7, 0)
+#define AST2700_ECC_FAIL_ADDR 0x7c
+
struct aspeed_edac_chip {
unsigned int conf_reg;
u32 conf_ecc;
@@ -40,6 +53,8 @@ struct aspeed_edac_chip {
unsigned long mtype_cap;
unsigned int prot_reg;
u32 prot_key;
+ irqreturn_t (*isr)(int irq, void *arg);
+ void (*set_irq)(struct mem_ctl_info *mci, bool enable);
};
struct aspeed_edac {
@@ -49,26 +64,34 @@ struct aspeed_edac {
const struct aspeed_edac_chip *chip;
};
-static void count_rec(struct mem_ctl_info *mci, u8 rec_cnt, u32 rec_addr)
+static void count_rec(struct mem_ctl_info *mci, u8 rec_cnt, phys_addr_t rec_addr,
+ bool have_addr)
{
struct csrow_info *csrow = mci->csrows[0];
- u32 page, offset, syndrome;
+ unsigned long page, offset, syndrome;
if (!rec_cnt)
return;
- /* report first few errors (if there are) */
- /* note: no addresses are recorded */
- if (rec_cnt > 1) {
+ /*
+ * Report the errors whose address is not recorded: all of them when
+ * no address is available, otherwise all but the last one (reported
+ * with its address below).
+ */
+ if (rec_cnt > 1 || !have_addr) {
/* page, offset and syndrome are not available */
page = 0;
offset = 0;
syndrome = 0;
- edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci, rec_cnt-1,
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ have_addr ? rec_cnt - 1 : rec_cnt,
page, offset, syndrome, 0, 0, -1,
"address(es) not available", "");
}
+ if (!have_addr)
+ return;
+
/* report last error */
/* note: rec_addr is the last recoverable error addr */
page = rec_addr >> PAGE_SHIFT;
@@ -81,32 +104,34 @@ static void count_rec(struct mem_ctl_info *mci, u8 rec_cnt, u32 rec_addr)
}
static void count_un_rec(struct mem_ctl_info *mci, u8 un_rec_cnt,
- u32 un_rec_addr)
+ phys_addr_t un_rec_addr, bool have_addr)
{
struct csrow_info *csrow = mci->csrows[0];
- u32 page, offset, syndrome;
+ unsigned long page, offset, syndrome;
if (!un_rec_cnt)
return;
- /* report 1. error */
- /* note: un_rec_addr is the first unrecoverable error addr */
- page = un_rec_addr >> PAGE_SHIFT;
- offset = un_rec_addr & ~PAGE_MASK;
- /* syndrome is not available */
- syndrome = 0;
- edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci, 1,
- csrow->first_page + page, offset, syndrome,
- 0, 0, -1, "", "");
+ /* report the first error with its address when one is available */
+ if (have_addr) {
+ /* note: un_rec_addr is the first unrecoverable error addr */
+ page = un_rec_addr >> PAGE_SHIFT;
+ offset = un_rec_addr & ~PAGE_MASK;
+ /* syndrome is not available */
+ syndrome = 0;
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci, 1,
+ csrow->first_page + page, offset, syndrome,
+ 0, 0, -1, "", "");
+ }
- /* report further errors (if there are) */
- /* note: no addresses are recorded */
- if (un_rec_cnt > 1) {
+ /* report the remaining errors without a recorded address */
+ if (un_rec_cnt > 1 || !have_addr) {
/* page, offset and syndrome are not available */
page = 0;
offset = 0;
syndrome = 0;
- edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci, un_rec_cnt-1,
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ have_addr ? un_rec_cnt - 1 : un_rec_cnt,
page, offset, syndrome, 0, 0, -1,
"address(es) not available", "");
}
@@ -160,8 +185,8 @@ static irqreturn_t aspeed_mcr_isr(int irq, void *arg)
rec_cnt, un_rec_cnt);
/* process recoverable and unrecoverable errors */
- count_rec(mci, rec_cnt, rec_addr);
- count_un_rec(mci, un_rec_cnt, un_rec_addr);
+ count_rec(mci, rec_cnt, rec_addr, true);
+ count_un_rec(mci, un_rec_cnt, un_rec_addr, true);
if (!rec_cnt && !un_rec_cnt)
dev_dbg_ratelimited(mci->pdev, "received edac interrupt, but did not find any ECC counters\n");
@@ -174,6 +199,52 @@ static irqreturn_t aspeed_mcr_isr(int irq, void *arg)
return IRQ_HANDLED;
}
+static irqreturn_t ast2700_dramc_isr(int irq, void *arg)
+{
+ u32 int_sts, ecc_sts, fail_addr;
+ struct mem_ctl_info *mci = arg;
+ struct aspeed_edac *priv;
+ u8 rec_cnt, un_rec_cnt;
+ phys_addr_t addr;
+
+ priv = mci->pvt_info;
+
+ scoped_guard(raw_spinlock, &priv->lock) {
+ int_sts = readl(priv->regs + AST2700_INT_STS);
+ ecc_sts = readl(priv->regs + AST2700_ECC_STS);
+ fail_addr = readl(priv->regs + AST2700_ECC_FAIL_ADDR);
+
+ /* the interrupt registers are not key-protected; clear only ECC */
+ writel(int_sts & (AST2700_INT_ECC_RECOVERABLE | AST2700_INT_ECC_UNRECOVERABLE),
+ priv->regs + AST2700_INT_CLR);
+ }
+
+ rec_cnt = FIELD_GET(AST2700_ECC_REC_CNT, ecc_sts);
+ un_rec_cnt = FIELD_GET(AST2700_ECC_UNREC_CNT, ecc_sts);
+
+ /* the register holds address bits [35:4], in units of 16 bytes */
+ addr = (phys_addr_t)fail_addr << 4;
+
+ /*
+ * The controller records only the address of the latest failure,
+ * shared by both error types. When only one type occurred it owns
+ * that address; when both occurred attribute it to the uncorrectable
+ * error and report the corrected ones without an address.
+ */
+ if (un_rec_cnt && !rec_cnt) {
+ count_un_rec(mci, un_rec_cnt, addr, true);
+ } else if (!un_rec_cnt && rec_cnt) {
+ count_rec(mci, rec_cnt, addr, true);
+ } else if (un_rec_cnt && rec_cnt) {
+ count_un_rec(mci, un_rec_cnt, addr, true);
+ count_rec(mci, rec_cnt, 0, false);
+ } else {
+ dev_dbg_ratelimited(mci->pdev, "received interrupt with no ECC counters set\n");
+ }
+
+ return IRQ_HANDLED;
+}
+
static void aspeed_set_irq(struct mem_ctl_info *mci, bool enable)
{
struct aspeed_edac *priv = mci->pvt_info;
@@ -192,8 +263,22 @@ static void aspeed_set_irq(struct mem_ctl_info *mci, bool enable)
aspeed_mcr_irq_update_exit(priv);
}
+static void ast2700_set_irq(struct mem_ctl_info *mci, bool enable)
+{
+ u32 mask = AST2700_INT_ECC_RECOVERABLE | AST2700_INT_ECC_UNRECOVERABLE;
+ struct aspeed_edac *priv = mci->pvt_info;
+ u32 val;
+
+ guard(raw_spinlock_irqsave)(&priv->lock);
+
+ /* interrupts are enabled by clearing their mask bits */
+ val = readl(priv->regs + AST2700_INT_MASK);
+ writel(enable ? (val & ~mask) : (val | mask), priv->regs + AST2700_INT_MASK);
+}
+
static int config_irq(struct mem_ctl_info *mci, struct platform_device *pdev)
{
+ struct aspeed_edac *priv = mci->pvt_info;
int irq;
int rc;
@@ -203,13 +288,13 @@ static int config_irq(struct mem_ctl_info *mci, struct platform_device *pdev)
if (irq < 0)
return irq;
- rc = devm_request_irq(&pdev->dev, irq, aspeed_mcr_isr, IRQF_TRIGGER_HIGH,
+ rc = devm_request_irq(&pdev->dev, irq, priv->chip->isr, IRQF_TRIGGER_HIGH,
DRV_NAME, mci);
if (rc)
return rc;
/* enable interrupts */
- aspeed_set_irq(mci, true);
+ priv->chip->set_irq(mci, true);
return 0;
}
@@ -354,9 +439,10 @@ static int aspeed_probe(struct platform_device *pdev)
static void aspeed_remove(struct platform_device *pdev)
{
struct mem_ctl_info *mci = platform_get_drvdata(pdev);
+ struct aspeed_edac *priv = mci->pvt_info;
/* disable interrupts */
- aspeed_set_irq(mci, false);
+ priv->chip->set_irq(mci, false);
/* free resources */
edac_mc_del_mc(&pdev->dev);
@@ -371,6 +457,8 @@ static const struct aspeed_edac_chip ast2400_edac = {
.mtype_cap = MEM_FLAG_DDR3 | MEM_FLAG_DDR4,
.prot_reg = ASPEED_MCR_PROT,
.prot_key = ASPEED_MCR_PROT_PASSWD,
+ .isr = aspeed_mcr_isr,
+ .set_irq = aspeed_set_irq,
};
/* The AST2600 does not key-protect the interrupt control register (MCR50). */
@@ -380,12 +468,26 @@ static const struct aspeed_edac_chip ast2600_edac = {
.conf_dram_type = ASPEED_MCR_CONF_DRAM_TYPE,
.dram_type = { MEM_DDR3, MEM_DDR4 },
.mtype_cap = MEM_FLAG_DDR3 | MEM_FLAG_DDR4,
+ .isr = aspeed_mcr_isr,
+ .set_irq = aspeed_set_irq,
+};
+
+/* The AST2700 interrupt registers are not key-protected either. */
+static const struct aspeed_edac_chip ast2700_edac = {
+ .conf_reg = AST2700_MCFG,
+ .conf_ecc = AST2700_MCFG_ECC,
+ .conf_dram_type = AST2700_MCFG_DRAM_TYPE,
+ .dram_type = { MEM_DDR4, MEM_DDR5 },
+ .mtype_cap = MEM_FLAG_DDR4 | MEM_FLAG_DDR5,
+ .isr = ast2700_dramc_isr,
+ .set_irq = ast2700_set_irq,
};
static const struct of_device_id aspeed_of_match[] = {
{ .compatible = "aspeed,ast2400-sdram-edac", .data = &ast2400_edac },
{ .compatible = "aspeed,ast2500-sdram-edac", .data = &ast2400_edac },
{ .compatible = "aspeed,ast2600-sdram-edac", .data = &ast2600_edac },
+ { .compatible = "aspeed,ast2700-sdram-edac", .data = &ast2700_edac },
{},
};
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-08-12 5:49 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12 5:48 [PATCH 0/7] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
2026-08-12 5:48 ` [PATCH 1/7] dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC Ryan Chen
2026-08-12 5:48 ` [PATCH 2/7] EDAC/aspeed: Set the DIMM grain Ryan Chen
2026-08-12 5:48 ` [PATCH 3/7] EDAC/aspeed: Clean up whitespace and include ordering Ryan Chen
2026-08-12 5:48 ` [PATCH 4/7] EDAC/aspeed: Free the mem_ctl_info unconditionally on remove Ryan Chen
2026-08-12 5:48 ` [PATCH 5/7] EDAC/aspeed: Replace regmap with direct register access Ryan Chen
2026-08-12 5:48 ` [PATCH 6/7] EDAC/aspeed: Abstract SoC differences behind chip data Ryan Chen
2026-08-12 5:48 ` [PATCH 7/7] EDAC/aspeed: Add AST2700 support Ryan Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox