* [PATCH v4 1/9] EDAC/aspeed: Set the DIMM grain
2026-09-30 5:14 [PATCH v4 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
@ 2026-09-30 5:14 ` Ryan Chen
2026-09-30 5:15 ` [PATCH v4 2/9] EDAC/aspeed: Free the interrupt before the mem_ctl_info on remove Ryan Chen
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Ryan Chen @ 2026-09-30 5:14 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, Ryan Chen
dimm->grain is never set, so every error is reported with a granularity
of zero, which edac_raw_mc_handle_error() warns about and substitutes
with 1.
The controller records the address of a failing access with 16-byte
granularity: MCR58 and MCR5C hold it in bits [30:4], with the low four
bits reserved. Set the grain to 16 accordingly.
Fixes: 9b7e6242ee4e ("EDAC, aspeed: Add an Aspeed AST2500 EDAC driver")
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v4:
- Explain where the grain value comes from rather than the warning it
silences.
- Drop the Cc: stable tag.
---
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 [flat|nested] 10+ messages in thread* [PATCH v4 2/9] EDAC/aspeed: Free the interrupt before the mem_ctl_info on remove
2026-09-30 5:14 [PATCH v4 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
2026-09-30 5:14 ` [PATCH v4 1/9] EDAC/aspeed: Set the DIMM grain Ryan Chen
@ 2026-09-30 5:15 ` Ryan Chen
2026-09-30 5:15 ` [PATCH v4 3/9] EDAC/aspeed: Clean up whitespace and include ordering Ryan Chen
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Ryan Chen @ 2026-09-30 5:15 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, Ryan Chen
The ECC interrupt is devm-managed, so it is only released after .remove()
has returned, and masking the controller does not wait for a handler
already running on another CPU. edac_mc_free() can therefore free the
mem_ctl_info the handler uses as its context while it is still running.
Fix the ordering and synchronise by freeing the interrupt prior to
releasing related memory.
Fixes: 9b7e6242ee4e ("EDAC, aspeed: Add an Aspeed AST2500 EDAC driver")
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v4:
- Drop the Cc: stable tag.
Changes in v2:
- New patch.
---
drivers/edac/aspeed_edac.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
index 83d60414f89a..e05ebed5c2f2 100644
--- a/drivers/edac/aspeed_edac.c
+++ b/drivers/edac/aspeed_edac.c
@@ -359,11 +359,16 @@ static int aspeed_probe(struct platform_device *pdev)
static void aspeed_remove(struct platform_device *pdev)
{
struct mem_ctl_info *mci;
+ int irq;
/* disable interrupts */
regmap_update_bits(aspeed_regmap, ASPEED_MCR_INTR_CTRL,
ASPEED_MCR_INTR_CTRL_ENABLE, 0);
+ irq = platform_get_irq(pdev, 0);
+ WARN_ON(irq < 0);
+ devm_free_irq(&pdev->dev, irq, platform_get_drvdata(pdev));
+
/* free resources */
mci = edac_mc_del_mc(&pdev->dev);
if (mci)
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v4 3/9] EDAC/aspeed: Clean up whitespace and include ordering
2026-09-30 5:14 [PATCH v4 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
2026-09-30 5:14 ` [PATCH v4 1/9] EDAC/aspeed: Set the DIMM grain Ryan Chen
2026-09-30 5:15 ` [PATCH v4 2/9] EDAC/aspeed: Free the interrupt before the mem_ctl_info on remove Ryan Chen
@ 2026-09-30 5:15 ` Ryan Chen
2026-09-30 5:15 ` [PATCH v4 4/9] EDAC/aspeed: Free the mem_ctl_info unconditionally on remove Ryan Chen
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Ryan Chen @ 2026-09-30 5:15 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, 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 e05ebed5c2f2..cd2a6fcca355 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;
@@ -375,7 +360,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 [flat|nested] 10+ messages in thread* [PATCH v4 4/9] EDAC/aspeed: Free the mem_ctl_info unconditionally on remove
2026-09-30 5:14 [PATCH v4 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (2 preceding siblings ...)
2026-09-30 5:15 ` [PATCH v4 3/9] EDAC/aspeed: Clean up whitespace and include ordering Ryan Chen
@ 2026-09-30 5:15 ` Ryan Chen
2026-09-30 5:15 ` [PATCH v4 5/9] dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC Ryan Chen
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Ryan Chen @ 2026-09-30 5:15 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, 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 | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
index cd2a6fcca355..352910e1defc 100644
--- a/drivers/edac/aspeed_edac.c
+++ b/drivers/edac/aspeed_edac.c
@@ -343,7 +343,7 @@ 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);
int irq;
/* disable interrupts */
@@ -352,12 +352,11 @@ static void aspeed_remove(struct platform_device *pdev)
irq = platform_get_irq(pdev, 0);
WARN_ON(irq < 0);
- devm_free_irq(&pdev->dev, irq, platform_get_drvdata(pdev));
+ devm_free_irq(&pdev->dev, irq, mci);
/* 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 [flat|nested] 10+ messages in thread* [PATCH v4 5/9] dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC
2026-09-30 5:14 [PATCH v4 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (3 preceding siblings ...)
2026-09-30 5:15 ` [PATCH v4 4/9] EDAC/aspeed: Free the mem_ctl_info unconditionally on remove Ryan Chen
@ 2026-09-30 5:15 ` Ryan Chen
2026-09-30 5:15 ` [PATCH v4 6/9] EDAC/aspeed: Replace regmap with direct register access Ryan Chen
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Ryan Chen @ 2026-09-30 5:15 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, Ryan Chen, Krzysztof Kozlowski
Add the "aspeed,ast2700-sdram-edac" compatible and note DDR5 support.
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.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 [flat|nested] 10+ messages in thread* [PATCH v4 6/9] EDAC/aspeed: Replace regmap with direct register access
2026-09-30 5:14 [PATCH v4 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (4 preceding siblings ...)
2026-09-30 5:15 ` [PATCH v4 5/9] dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC Ryan Chen
@ 2026-09-30 5:15 ` Ryan Chen
2026-09-30 5:15 ` [PATCH v4 7/9] EDAC/aspeed: Abstract SoC differences behind chip data Ryan Chen
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Ryan Chen @ 2026-09-30 5:15 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, 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.
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>
---
Changes in v2:
- Take the register lock with the irqsave variant in init_csrows() and in
aspeed_probe(); the interrupt handler takes the same lock in hardirq
context.
- Move the dev_dbg() of the interrupt status register out of the
raw_spinlock critical section in the interrupt handler.
- Opt aspeed_edac.o into context analysis in drivers/edac/Makefile, so
that the __guarded_by() annotation is actually checked.
- Note in the interrupt handler that the counter and interrupt flag
fields are read-only, so writing back the read value is harmless.
---
drivers/edac/Makefile | 1 +
drivers/edac/aspeed_edac.c | 131 +++++++++++++++++----------------------------
2 files changed, 51 insertions(+), 81 deletions(-)
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index a37534300ab9..9215dd0bb835 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_EDAC_SYNOPSYS) += synopsys_edac.o
obj-$(CONFIG_EDAC_XGENE) += xgene_edac.o
obj-$(CONFIG_EDAC_TI) += ti_edac.o
obj-$(CONFIG_EDAC_QCOM) += qcom_edac.o
+CONTEXT_ANALYSIS_aspeed_edac.o := y
obj-$(CONFIG_EDAC_ASPEED) += aspeed_edac.o
obj-$(CONFIG_EDAC_BLUEFIELD) += bluefield_edac.o
obj-$(CONFIG_EDAC_DMC520) += dmc520_edac.o
diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
index 352910e1defc..26d2c456cc0d 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,10 +100,27 @@ 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;
+ u8 rec_cnt, un_rec_cnt;
+ u32 reg50;
+
+ scoped_guard(raw_spinlock, &aspeed_lock) {
+ reg50 = readl(aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ 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. The
+ * counter and interrupt flag fields are read-only, so writing
+ * back the values read above leaves them unaffected.
+ */
+ 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);
+ }
- regmap_read(aspeed_regmap, ASPEED_MCR_INTR_CTRL, ®50);
dev_dbg(mci->pdev, "received edac interrupt w/ mcr register 50: 0x%x\n",
reg50);
@@ -161,20 +131,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 +138,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 +180,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 +218,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_irqsave, &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 +236,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 +246,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_irqsave, &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;
@@ -347,8 +317,7 @@ static void aspeed_remove(struct platform_device *pdev)
int irq;
/* disable interrupts */
- regmap_update_bits(aspeed_regmap, ASPEED_MCR_INTR_CTRL,
- ASPEED_MCR_INTR_CTRL_ENABLE, 0);
+ aspeed_set_irq(false);
irq = platform_get_irq(pdev, 0);
WARN_ON(irq < 0);
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v4 7/9] EDAC/aspeed: Abstract SoC differences behind chip data
2026-09-30 5:14 [PATCH v4 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (5 preceding siblings ...)
2026-09-30 5:15 ` [PATCH v4 6/9] EDAC/aspeed: Replace regmap with direct register access Ryan Chen
@ 2026-09-30 5:15 ` Ryan Chen
2026-09-30 5:15 ` [PATCH v4 8/9] EDAC/aspeed: Add AST2700 support Ryan Chen
2026-09-30 5:15 ` [PATCH v4 9/9] MAINTAINERS: Step down as Aspeed AST2500 EDAC driver maintainer Ryan Chen
8 siblings, 0 replies; 10+ messages in thread
From: Ryan Chen @ 2026-09-30 5:15 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, 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.
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>
---
Changes in v2:
- Carry over the irqsave lock acquisition in init_csrows() and the
dev_dbg() moved out of the raw_spinlock critical section.
- Store the interrupt number in struct aspeed_edac so that
aspeed_remove() can free the interrupt without looking it up again.
---
drivers/edac/aspeed_edac.c | 163 +++++++++++++++++++++++++++++++--------------
1 file changed, 114 insertions(+), 49 deletions(-)
diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
index 26d2c456cc0d..e88d9d2646de 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,23 @@
#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;
+ int irq;
+};
static void count_rec(struct mem_ctl_info *mci, u8 rec_cnt, u32 rec_addr)
{
@@ -96,37 +113,54 @@ 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);
- un_rec_addr = readl(aspeed_regs + ASPEED_MCR_ADDR_UNREC);
- rec_addr = readl(aspeed_regs + ASPEED_MCR_ADDR_REC);
+ priv = mci->pvt_info;
+
+ scoped_guard(raw_spinlock, &priv->lock) {
+ reg50 = readl(priv->regs + ASPEED_MCR_INTR_CTRL);
+ 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. The
* counter and interrupt flag fields are read-only, so writing
* back the values read above leaves them unaffected.
*/
- 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);
}
dev_dbg(mci->pdev, "received edac interrupt w/ mcr register 50: 0x%x\n",
reg50);
/* 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);
@@ -136,35 +170,36 @@ 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 aspeed_edac *priv, bool enable)
{
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)
{
+ struct aspeed_edac *priv = mci->pvt_info;
int irq;
int rc;
@@ -174,13 +209,15 @@ 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;
+ priv->irq = irq;
+
/* enable interrupts */
- aspeed_set_irq(true);
+ aspeed_set_irq(priv, true);
return 0;
}
@@ -188,11 +225,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 */
@@ -218,12 +257,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_irqsave, &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_irqsave, &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;
@@ -236,22 +275,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_irqsave, &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;
}
@@ -266,12 +309,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;
@@ -314,24 +362,41 @@ 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);
- int irq;
+ struct aspeed_edac *priv = mci->pvt_info;
/* disable interrupts */
- aspeed_set_irq(false);
+ aspeed_set_irq(priv, false);
- irq = platform_get_irq(pdev, 0);
- WARN_ON(irq < 0);
- devm_free_irq(&pdev->dev, irq, mci);
+ devm_free_irq(&pdev->dev, priv->irq, mci);
/* 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 [flat|nested] 10+ messages in thread* [PATCH v4 8/9] EDAC/aspeed: Add AST2700 support
2026-09-30 5:14 [PATCH v4 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (6 preceding siblings ...)
2026-09-30 5:15 ` [PATCH v4 7/9] EDAC/aspeed: Abstract SoC differences behind chip data Ryan Chen
@ 2026-09-30 5:15 ` Ryan Chen
2026-09-30 5:15 ` [PATCH v4 9/9] MAINTAINERS: Step down as Aspeed AST2500 EDAC driver maintainer Ryan Chen
8 siblings, 0 replies; 10+ messages in thread
From: Ryan Chen @ 2026-09-30 5:15 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, 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
devm_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>
---
Changes in v3:
- Write the AST2700 interrupt mask register outright rather than
read-modify-write: mask every source and unmask only the ECC ones.
The driver owns the line exclusively, so it should establish the
mask itself instead of deriving it from the register's contents.
- Mask the interrupts before requesting them rather than after, so the
mask is established before the handler can run.
Changes in v2:
- Return IRQ_NONE from ast2700_dramc_isr() when no ECC interrupt status
bit is set. The handler clears only the ECC bits, so unconditionally
claiming the interrupt could livelock the level-triggered line if
another DRAMC source was left unmasked.
- Add AST2700_INT_ECC for the pair of ECC interrupt bits, now used by both
the handler and the enable path.
- Expand the enable/disable ternary in ast2700_set_irq() into if/else,
matching the shape of aspeed_set_irq().
- Pass struct aspeed_edac to the set_irq() hooks instead of
struct mem_ctl_info, which the callees only used to reach pvt_info.
---
drivers/edac/aspeed_edac.c | 161 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 135 insertions(+), 26 deletions(-)
diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
index e88d9d2646de..6fe6314cda72 100644
--- a/drivers/edac/aspeed_edac.c
+++ b/drivers/edac/aspeed_edac.c
@@ -32,6 +32,24 @@
#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_INT_ECC (AST2700_INT_ECC_RECOVERABLE | \
+ AST2700_INT_ECC_UNRECOVERABLE)
+#define AST2700_INT_ALL GENMASK(16, 0)
+#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;
+
struct aspeed_edac_chip {
unsigned int conf_reg;
u32 conf_ecc;
@@ -40,6 +58,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 aspeed_edac *priv, bool enable);
};
struct aspeed_edac {
@@ -50,26 +70,34 @@ struct aspeed_edac {
int irq;
};
-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;
@@ -82,32 +110,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", "");
}
@@ -166,8 +196,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");
@@ -180,6 +210,54 @@ 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);
+ if (!(int_sts & AST2700_INT_ECC))
+ return IRQ_NONE;
+
+ 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, 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 aspeed_edac *priv, bool enable)
{
u32 val;
@@ -197,6 +275,19 @@ static void aspeed_set_irq(struct aspeed_edac *priv, bool enable)
aspeed_mcr_irq_update_exit(priv);
}
+static void ast2700_set_irq(struct aspeed_edac *priv, bool enable)
+{
+ u32 val = AST2700_INT_ALL;
+
+ guard(raw_spinlock_irqsave)(&priv->lock);
+
+ /* interrupts are enabled by clearing their mask bits */
+ if (enable)
+ val &= ~AST2700_INT_ECC;
+
+ writel(val, 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;
@@ -209,7 +300,9 @@ 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,
+ priv->chip->set_irq(priv, false);
+
+ rc = devm_request_irq(&pdev->dev, irq, priv->chip->isr, IRQF_TRIGGER_HIGH,
DRV_NAME, mci);
if (rc)
return rc;
@@ -217,7 +310,7 @@ static int config_irq(struct mem_ctl_info *mci, struct platform_device *pdev)
priv->irq = irq;
/* enable interrupts */
- aspeed_set_irq(priv, true);
+ priv->chip->set_irq(priv, true);
return 0;
}
@@ -365,7 +458,7 @@ static void aspeed_remove(struct platform_device *pdev)
struct aspeed_edac *priv = mci->pvt_info;
/* disable interrupts */
- aspeed_set_irq(priv, false);
+ priv->chip->set_irq(priv, false);
devm_free_irq(&pdev->dev, priv->irq, mci);
@@ -382,6 +475,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). */
@@ -391,12 +486,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 [flat|nested] 10+ messages in thread* [PATCH v4 9/9] MAINTAINERS: Step down as Aspeed AST2500 EDAC driver maintainer
2026-09-30 5:14 [PATCH v4 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (7 preceding siblings ...)
2026-09-30 5:15 ` [PATCH v4 8/9] EDAC/aspeed: Add AST2700 support Ryan Chen
@ 2026-09-30 5:15 ` Ryan Chen
8 siblings, 0 replies; 10+ messages in thread
From: Ryan Chen @ 2026-09-30 5:15 UTC (permalink / raw)
To: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Borislav Petkov,
Tony Luck
Cc: devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, Ryan Chen
From: Stefan M Schaeckeler <sschaeck@cisco.com>
I am stepping down as a maintainer for drivers/edac/aspeed_edac.c and
Documentation/devicetree/bindings/edac/aspeed,ast2400-sdram-edac.yaml
Move my entry from the MAINTAINERS file to CREDITS to reflect my past
contributions.
[ ryan: keep the EDAC-AST2500 entry and list myself as its reviewer
rather than removing it ]
Signed-off-by: Stefan M Schaeckeler <sschaeck@cisco.com>
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
Link: https://lore.kernel.org/r/20260920225846.4039017-1-sschaeck@cisco.com
---
Changes in v4:
- Merge Stefan's step-down patch into this one, keeping the MAINTAINERS
entry alive with a reviewer instead of removing it.
Changes in v3:
- New patch.
---
CREDITS | 8 ++++++++
MAINTAINERS | 2 +-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/CREDITS b/CREDITS
index 091412875a66..28e5b274bc28 100644
--- a/CREDITS
+++ b/CREDITS
@@ -3655,6 +3655,14 @@ D: I2O kernel layer (config, block, core, pci, net). I2O disk support for LILO
D: XScale (IOP, IXP) porting and other random ARM bits
S: Portland, OR
+N: Stefan Schaeckeler
+E: sschaeck@cisco.com
+E: schaecsn@gmx.net
+D: Author and initial maintainer of the Aspeed AST2500 EDAC driver
+S: 2253 Anna Dr.
+S: Santa Clara, CA 95050
+S: USA
+
N: Eric Schenk
E: Eric.Schenk@dna.lth.se
D: Random kernel debugging.
diff --git a/MAINTAINERS b/MAINTAINERS
index d413dac5c8b3..2001466fd6da 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9283,7 +9283,7 @@ F: Documentation/devicetree/bindings/memory-controllers/marvell,mvebu-sdram-cont
F: drivers/edac/armada_xp_*
EDAC-AST2500
-M: Stefan Schaeckeler <sschaeck@cisco.com>
+R: Ryan Chen <ryan_chen@aspeedtech.com>
S: Supported
F: Documentation/devicetree/bindings/edac/aspeed,ast2400-sdram-edac.yaml
F: drivers/edac/aspeed_edac.c
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread