* [PATCH v3 1/9] dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC
2026-09-16 5:30 [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
@ 2026-09-16 5:30 ` Ryan Chen
2026-09-16 5:30 ` [PATCH v3 2/9] EDAC/aspeed: Set the DIMM grain Ryan Chen
` (9 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Ryan Chen @ 2026-09-16 5:30 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] 15+ messages in thread* [PATCH v3 2/9] EDAC/aspeed: Set the DIMM grain
2026-09-16 5:30 [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
2026-09-16 5:30 ` [PATCH v3 1/9] dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC Ryan Chen
@ 2026-09-16 5:30 ` Ryan Chen
2026-09-25 5:45 ` Borislav Petkov
2026-09-16 5:30 ` [PATCH v3 3/9] EDAC/aspeed: Free the interrupt before the mem_ctl_info on remove Ryan Chen
` (8 subsequent siblings)
10 siblings, 1 reply; 15+ messages in thread
From: Ryan Chen @ 2026-09-16 5:30 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, 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")
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
Cc: stable@vger.kernel.org
---
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] 15+ messages in thread* Re: [PATCH v3 2/9] EDAC/aspeed: Set the DIMM grain
2026-09-16 5:30 ` [PATCH v3 2/9] EDAC/aspeed: Set the DIMM grain Ryan Chen
@ 2026-09-25 5:45 ` Borislav Petkov
0 siblings, 0 replies; 15+ messages in thread
From: Borislav Petkov @ 2026-09-25 5:45 UTC (permalink / raw)
To: Ryan Chen
Cc: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Tony Luck,
devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, stable
On Wed, Sep 16, 2026 at 01:30:39PM +0800, Ryan Chen wrote:
> 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
All of that is fine and dandy but you should talk about why the grain is
chosen to be 16 here. The warning is just to make you choose a proper grain
for your hw and doesn't need that much 'splainin'.
> Fixes: 9b7e6242ee4e ("EDAC, aspeed: Add an Aspeed AST2500 EDAC driver")
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> Cc: stable@vger.kernel.org
Is this really stable material and if so, why?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 3/9] EDAC/aspeed: Free the interrupt before the mem_ctl_info on remove
2026-09-16 5:30 [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
2026-09-16 5:30 ` [PATCH v3 1/9] dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC Ryan Chen
2026-09-16 5:30 ` [PATCH v3 2/9] EDAC/aspeed: Set the DIMM grain Ryan Chen
@ 2026-09-16 5:30 ` Ryan Chen
2026-09-16 5:30 ` [PATCH v3 4/9] EDAC/aspeed: Clean up whitespace and include ordering Ryan Chen
` (7 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Ryan Chen @ 2026-09-16 5:30 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, stable
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>
Cc: stable@vger.kernel.org
---
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] 15+ messages in thread* [PATCH v3 4/9] EDAC/aspeed: Clean up whitespace and include ordering
2026-09-16 5:30 [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (2 preceding siblings ...)
2026-09-16 5:30 ` [PATCH v3 3/9] EDAC/aspeed: Free the interrupt before the mem_ctl_info on remove Ryan Chen
@ 2026-09-16 5:30 ` Ryan Chen
2026-09-16 5:30 ` [PATCH v3 5/9] EDAC/aspeed: Free the mem_ctl_info unconditionally on remove Ryan Chen
` (6 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Ryan Chen @ 2026-09-16 5:30 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] 15+ messages in thread* [PATCH v3 5/9] EDAC/aspeed: Free the mem_ctl_info unconditionally on remove
2026-09-16 5:30 [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (3 preceding siblings ...)
2026-09-16 5:30 ` [PATCH v3 4/9] EDAC/aspeed: Clean up whitespace and include ordering Ryan Chen
@ 2026-09-16 5:30 ` Ryan Chen
2026-09-16 5:30 ` [PATCH v3 6/9] EDAC/aspeed: Replace regmap with direct register access Ryan Chen
` (5 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Ryan Chen @ 2026-09-16 5:30 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] 15+ messages in thread* [PATCH v3 6/9] EDAC/aspeed: Replace regmap with direct register access
2026-09-16 5:30 [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (4 preceding siblings ...)
2026-09-16 5:30 ` [PATCH v3 5/9] EDAC/aspeed: Free the mem_ctl_info unconditionally on remove Ryan Chen
@ 2026-09-16 5:30 ` Ryan Chen
2026-09-16 5:30 ` [PATCH v3 7/9] EDAC/aspeed: Abstract SoC differences behind chip data Ryan Chen
` (4 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Ryan Chen @ 2026-09-16 5:30 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] 15+ messages in thread* [PATCH v3 7/9] EDAC/aspeed: Abstract SoC differences behind chip data
2026-09-16 5:30 [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (5 preceding siblings ...)
2026-09-16 5:30 ` [PATCH v3 6/9] EDAC/aspeed: Replace regmap with direct register access Ryan Chen
@ 2026-09-16 5:30 ` Ryan Chen
2026-09-16 5:30 ` [PATCH v3 8/9] EDAC/aspeed: Add AST2700 support Ryan Chen
` (3 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Ryan Chen @ 2026-09-16 5:30 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] 15+ messages in thread* [PATCH v3 8/9] EDAC/aspeed: Add AST2700 support
2026-09-16 5:30 [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (6 preceding siblings ...)
2026-09-16 5:30 ` [PATCH v3 7/9] EDAC/aspeed: Abstract SoC differences behind chip data Ryan Chen
@ 2026-09-16 5:30 ` Ryan Chen
2026-09-16 5:30 ` [PATCH v3 9/9] MAINTAINERS: Add a reviewer for the Aspeed EDAC driver Ryan Chen
` (2 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Ryan Chen @ 2026-09-16 5:30 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] 15+ messages in thread* [PATCH v3 9/9] MAINTAINERS: Add a reviewer for the Aspeed EDAC driver
2026-09-16 5:30 [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (7 preceding siblings ...)
2026-09-16 5:30 ` [PATCH v3 8/9] EDAC/aspeed: Add AST2700 support Ryan Chen
@ 2026-09-16 5:30 ` Ryan Chen
2026-09-22 2:53 ` [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
2026-09-25 5:44 ` Borislav Petkov
10 siblings, 0 replies; 15+ messages in thread
From: Ryan Chen @ 2026-09-16 5:30 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 myself as a reviewer to be copied on changes to the driver.
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v3:
- New patch.
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index d413dac5c8b3..5bd0109f0f1e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9284,6 +9284,7 @@ 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] 15+ messages in thread* RE: [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support
2026-09-16 5:30 [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (8 preceding siblings ...)
2026-09-16 5:30 ` [PATCH v3 9/9] MAINTAINERS: Add a reviewer for the Aspeed EDAC driver Ryan Chen
@ 2026-09-22 2:53 ` Ryan Chen
2026-09-22 3:19 ` Borislav Petkov
2026-09-25 5:44 ` Borislav Petkov
10 siblings, 1 reply; 15+ messages in thread
From: Ryan Chen @ 2026-09-22 2:53 UTC (permalink / raw)
To: Ryan Chen, 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, Krzysztof Kozlowski, stable
Hello Boris,
Stefan has resigned from the EDAC entry:
https://lkml.org/lkml/2026/9/20/1295
That leaves his patch and 9/9 of this series touching the same lines,
and the entry ending up with a reviewer and no maintainer.
You suggested a reviewer entry first, and I understood the reasoning.
But with Stefan gone the choice is now between orphaning the driver and
someone taking it, so I'd rather ask again: I'm happy to take the M:
line. I work on these SoCs at ASPEED and have the hardware and the
documentation for all four variants.
If you'd still rather not, that's fine - tell me and I'll respin 9/9 so
the entry reflects reality instead.
Ryan
> Subject: [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support
>
> 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.
>
> Patches 2-5 are bug fixes and cleanups to the existing driver. Patches 6 and 7
> rework the implementation to accommodate the AST2700. Patch 8 adds the
> AST2700 support itself, and patch 9 adds a reviewer entry for the driver.
>
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> ---
> Changes in v3:
> - Establish the AST2700 interrupt mask in the driver rather than
> deriving it from the register's contents: ast2700_set_irq() now
> writes the mask outright, masking every source and unmasking only
> the ECC ones, and the mask is established before the interrupt is
> requested.
> - New patch: add myself as a reviewer for the driver in MAINTAINERS.
> - Tested on an AST2600 with CONFIG_PROVE_LOCKING=y; injected correctable
> errors are reported from the interrupt handler with no lockdep reports.
> - Link to v2:
> https://lore.kernel.org/r/20260824-edac-v2-0-c8d8bb693586@aspeedtech.co
> m
>
> Changes in v2:
> - New patch: free the ECC interrupt before the mem_ctl_info in
> aspeed_remove(). The interrupt is requested with devm_request_irq() and
> is only released after .remove() has returned, so edac_mc_free() could
> free the handler's context while the handler was still running on
> another CPU.
> - Acquire 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. The
> check is opt-in per object, so without it the __guarded_by() annotation
> added by the same patch was never actually verified.
> - 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.
> - Tested v2 on both AST2600 and AST2700 by injecting a correctable
> error with the memory controller's ECC error injection; each is
> reported as a CE with the expected failure address.
> - Link to v1:
> https://lore.kernel.org/r/20260812-edac-v1-0-03992edea297@aspeedtech.co
> m
>
> ---
> Ryan Chen (9):
> dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC
> EDAC/aspeed: Set the DIMM grain
> EDAC/aspeed: Free the interrupt before the mem_ctl_info on remove
> 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
> MAINTAINERS: Add a reviewer for the Aspeed EDAC driver
>
> .../bindings/edac/aspeed,ast2400-sdram-edac.yaml | 6 +-
> MAINTAINERS | 1 +
> drivers/edac/Makefile | 1 +
> drivers/edac/aspeed_edac.c | 426
> ++++++++++++++-------
> 4 files changed, 285 insertions(+), 149 deletions(-)
> ---
> base-commit: 5464985e42c04e335fb30e38fbc409c997db9bec
> change-id: 20260625-edac-8e960e02e7f7
>
> Best regards,
> --
> Ryan Chen <ryan_chen@aspeedtech.com>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support
2026-09-22 2:53 ` [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
@ 2026-09-22 3:19 ` Borislav Petkov
2026-09-22 5:25 ` Ryan Chen
0 siblings, 1 reply; 15+ messages in thread
From: Borislav Petkov @ 2026-09-22 3:19 UTC (permalink / raw)
To: Ryan Chen
Cc: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Tony Luck,
devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, Krzysztof Kozlowski, stable
Hi Ryan,
On Tue, Sep 22, 2026 at 02:53:27AM +0000, Ryan Chen wrote:
> Stefan has resigned from the EDAC entry:
>
> https://lkml.org/lkml/2026/9/20/1295
Please do not use rando pages from search engines for pointing to patches, we
have lore.kernel.org for that.
Also, please do not top-post on public MLs but put your text under the text
you're replying to.
We have the whole development process explained here, please take some time to
read the most important points:
https://www.kernel.org/doc/html/latest/process/development-process.html
> That leaves his patch and 9/9 of this series touching the same lines,
> and the entry ending up with a reviewer and no maintainer.
>
> You suggested a reviewer entry first, and I understood the reasoning.
> But with Stefan gone the choice is now between orphaning the driver and
> someone taking it, so I'd rather ask again: I'm happy to take the M:
> line. I work on these SoCs at ASPEED and have the hardware and the
> documentation for all four variants.
That's much appreciated but you don't have to worry - the driver won't be
orphaned and you'll be the reviewer and tester of patches for it. And I'll
simply add Stefan's patch at the end and amend the hunk marking the driver as
orphan and instead add your hunk which is making you a reviewer.
> If you'd still rather not, that's fine - tell me and I'll respin 9/9 so
> the entry reflects reality instead.
Let's see how we work together this way first.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 15+ messages in thread* RE: [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support
2026-09-22 3:19 ` Borislav Petkov
@ 2026-09-22 5:25 ` Ryan Chen
0 siblings, 0 replies; 15+ messages in thread
From: Ryan Chen @ 2026-09-22 5:25 UTC (permalink / raw)
To: Borislav Petkov
Cc: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Tony Luck,
devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, Krzysztof Kozlowski, stable
> Subject: Re: [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support
>
> Hi Ryan,
>
> On Tue, Sep 22, 2026 at 02:53:27AM +0000, Ryan Chen wrote:
> > Stefan has resigned from the EDAC entry:
> >
> > https://lkml.org/lkml/2026/9/20/1295
>
> Please do not use rando pages from search engines for pointing to patches, we
> have lore.kernel.org for that.
>
> Also, please do not top-post on public MLs but put your text under the text
> you're replying to.
>
> We have the whole development process explained here, please take some
> time to read the most important points:
>
> https://www.kernel.org/doc/html/latest/process/development-process.html
>
Understood.
> > That leaves his patch and 9/9 of this series touching the same lines,
> > and the entry ending up with a reviewer and no maintainer.
> >
> > You suggested a reviewer entry first, and I understood the reasoning.
> > But with Stefan gone the choice is now between orphaning the driver
> > and someone taking it, so I'd rather ask again: I'm happy to take the M:
> > line. I work on these SoCs at ASPEED and have the hardware and the
> > documentation for all four variants.
>
> That's much appreciated but you don't have to worry - the driver won't be
> orphaned and you'll be the reviewer and tester of patches for it. And I'll simply
> add Stefan's patch at the end and amend the hunk marking the driver as
> orphan and instead add your hunk which is making you a reviewer.
>
> > If you'd still rather not, that's fine - tell me and I'll respin 9/9
> > so the entry reflects reality instead.
>
> Let's see how we work together this way first.
>
Appreciate your help.
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support
2026-09-16 5:30 [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
` (9 preceding siblings ...)
2026-09-22 2:53 ` [PATCH v3 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
@ 2026-09-25 5:44 ` Borislav Petkov
10 siblings, 0 replies; 15+ messages in thread
From: Borislav Petkov @ 2026-09-25 5:44 UTC (permalink / raw)
To: Ryan Chen
Cc: Stefan Schaeckeler, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, Andrew Jeffery, Tony Luck,
devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-edac, Borislav Petkov, Krzysztof Kozlowski, stable
On Wed, Sep 16, 2026 at 01:30:37PM +0800, Ryan Chen wrote:
> 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.
>
> Patches 2-5 are bug fixes and cleanups to the existing driver.
You always put the bug fixes first in a set. So it should be 1-4 bugfixes,
rest new features.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 15+ messages in thread