[PATCH 6/7] EDAC/aspeed: Abstract SoC differences behind chip data

[Ryan Chen <ryan_chen@aspeedtech.com>]

The driver hard-codes the AST2400/2500/2600 register layout, ECC and
DRAM-type bits, memory types and write-protection key. Abstract these
SoC-specific details behind a per-SoC struct aspeed_edac_chip selected by
the compatible, and move the per-instance state (register base, lock)
into mci->pvt_info instead of globals, so controller variants that differ
in these details can be added as table data rather than by forking the
driver.

The __guarded_by() annotation on the register base moves with it, so the
build-time check that every access holds the lock is kept across the
conversion.

Only the AST2400 and AST2500 key-protect the interrupt control register
(MCR50); the AST2600 does not. Gate the unlock/relock on the chip carrying
a protection key and split the shared entry into keyed (AST2400/2500) and
unkeyed (AST2600) variants, so the AST2600 no longer performs the
unnecessary unlock.

Tested on an AST2600: A correctable error was injected from the console by
unlocking the controller and writing its ECC error inject test register:

  # mw 1e6e0000 fc600309
  # mw 1e6e00b0 81
  EDAC MC0: 1 CE address(es) not available on mc#0csrow#0channel#0 (csrow:0 channel:0 page:0x0 offset:0x0 grain:16 syndrome:0x0)
  EDAC MC0: 1 CE on mc#0csrow#0channel#0 (csrow:0 channel:0 page:0x8a543 offset:0xec0 grain:16 syndrome:0x0)

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
 drivers/edac/aspeed_edac.c | 154 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 109 insertions(+), 45 deletions(-)

diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
index 7bd552ee9a61..91df5d2df5f1 100644
--- a/drivers/edac/aspeed_edac.c
+++ b/drivers/edac/aspeed_edac.c
@@ -3,12 +3,14 @@
  * Copyright 2018, 2019 Cisco Systems
  */
 
+#include <linux/bitfield.h>
 #include <linux/cleanup.h>
 #include <linux/edac.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>
@@ -30,8 +32,22 @@
 #define ASPEED_MCR_INTR_CTRL_CNT_UNREC  GENMASK(15, 12)
 #define ASPEED_MCR_INTR_CTRL_ENABLE     (BIT(0) | BIT(1))
 
-static DEFINE_RAW_SPINLOCK(aspeed_lock);
-static void __iomem *aspeed_regs __guarded_by(&aspeed_lock);
+struct aspeed_edac_chip {
+	unsigned int conf_reg;
+	u32 conf_ecc;
+	u32 conf_dram_type;
+	enum mem_type dram_type[2];
+	unsigned long mtype_cap;
+	unsigned int prot_reg;
+	u32 prot_key;
+};
+
+struct aspeed_edac {
+	raw_spinlock_t lock;
+
+	void __iomem *regs __guarded_by(&lock);
+	const struct aspeed_edac_chip *chip;
+};
 
 static void count_rec(struct mem_ctl_info *mci, u8 rec_cnt, u32 rec_addr)
 {
@@ -96,32 +112,49 @@ static void count_un_rec(struct mem_ctl_info *mci, u8 un_rec_cnt,
 	}
 }
 
-static irqreturn_t mcr_isr(int irq, void *arg)
+static void aspeed_mcr_irq_update_enter(struct aspeed_edac *priv)
+	__must_hold(&priv->lock)
+{
+	if (priv->chip->prot_key)
+		writel(priv->chip->prot_key, priv->regs + priv->chip->prot_reg);
+}
+
+static void aspeed_mcr_irq_update_exit(struct aspeed_edac *priv)
+	__must_hold(&priv->lock)
+{
+	if (priv->chip->prot_key)
+		writel(~priv->chip->prot_key, priv->regs + priv->chip->prot_reg);
+}
+
+static irqreturn_t aspeed_mcr_isr(int irq, void *arg)
 {
 	struct mem_ctl_info *mci = arg;
 	u32 rec_addr, un_rec_addr;
+	struct aspeed_edac *priv;
 	u8 rec_cnt, un_rec_cnt;
 	u32 reg50;
 
-	scoped_guard(raw_spinlock, &aspeed_lock) {
-		reg50 = readl(aspeed_regs + ASPEED_MCR_INTR_CTRL);
+	priv = mci->pvt_info;
+
+	scoped_guard(raw_spinlock, &priv->lock) {
+		reg50 = readl(priv->regs + ASPEED_MCR_INTR_CTRL);
 		dev_dbg(mci->pdev, "received edac interrupt w/ mcr register 50: 0x%x\n",
 			reg50);
-		un_rec_addr = readl(aspeed_regs + ASPEED_MCR_ADDR_UNREC);
-		rec_addr = readl(aspeed_regs + ASPEED_MCR_ADDR_REC);
+		un_rec_addr = readl(priv->regs + ASPEED_MCR_ADDR_UNREC);
+		rec_addr = readl(priv->regs + ASPEED_MCR_ADDR_REC);
 
 		/* clearing the counters needs a set-then-clear of CLEAR */
-		writel(ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+		aspeed_mcr_irq_update_enter(priv);
 		writel(reg50 | ASPEED_MCR_INTR_CTRL_CLEAR,
-		       aspeed_regs + ASPEED_MCR_INTR_CTRL);
+		       priv->regs + ASPEED_MCR_INTR_CTRL);
 		writel(reg50 & ~ASPEED_MCR_INTR_CTRL_CLEAR,
-		       aspeed_regs + ASPEED_MCR_INTR_CTRL);
-		writel(~ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+		       priv->regs + ASPEED_MCR_INTR_CTRL);
+		aspeed_mcr_irq_update_exit(priv);
 	}
 
 	/* collect data about recoverable and unrecoverable errors */
-	rec_cnt = (reg50 & ASPEED_MCR_INTR_CTRL_CNT_REC) >> 16;
-	un_rec_cnt = (reg50 & ASPEED_MCR_INTR_CTRL_CNT_UNREC) >> 12;
+	rec_cnt = FIELD_GET(ASPEED_MCR_INTR_CTRL_CNT_REC, reg50);
+	un_rec_cnt = FIELD_GET(ASPEED_MCR_INTR_CTRL_CNT_UNREC, reg50);
 
 	dev_dbg(mci->pdev, "%d recoverable interrupts and %d unrecoverable interrupts\n",
 		rec_cnt, un_rec_cnt);
@@ -131,34 +164,35 @@ static irqreturn_t mcr_isr(int irq, void *arg)
 	count_un_rec(mci, un_rec_cnt, un_rec_addr);
 
 	if (!rec_cnt && !un_rec_cnt)
-		dev_dbg(mci->pdev, "received edac interrupt, but did not find any ECC counters\n");
+		dev_dbg_ratelimited(mci->pdev, "received edac interrupt, but did not find any ECC counters\n");
 
-	scoped_guard(raw_spinlock, &aspeed_lock)
-		reg50 = readl(aspeed_regs + ASPEED_MCR_INTR_CTRL);
+	scoped_guard(raw_spinlock, &priv->lock)
+		reg50 = readl(priv->regs + ASPEED_MCR_INTR_CTRL);
 	dev_dbg(mci->pdev, "edac interrupt handled. mcr reg 50 is now: 0x%x\n",
 		reg50);
 
 	return IRQ_HANDLED;
 }
 
-static void aspeed_set_irq(bool enable)
+static void aspeed_set_irq(struct mem_ctl_info *mci, bool enable)
 {
+	struct aspeed_edac *priv = mci->pvt_info;
 	u32 val;
 
-	guard(raw_spinlock_irqsave)(&aspeed_lock);
+	guard(raw_spinlock_irqsave)(&priv->lock);
 
-	val = readl(aspeed_regs + ASPEED_MCR_INTR_CTRL);
+	val = readl(priv->regs + ASPEED_MCR_INTR_CTRL);
 	if (enable)
 		val |= ASPEED_MCR_INTR_CTRL_ENABLE;
 	else
 		val &= ~ASPEED_MCR_INTR_CTRL_ENABLE;
 
-	writel(ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
-	writel(val, aspeed_regs + ASPEED_MCR_INTR_CTRL);
-	writel(~ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+	aspeed_mcr_irq_update_enter(priv);
+	writel(val, priv->regs + ASPEED_MCR_INTR_CTRL);
+	aspeed_mcr_irq_update_exit(priv);
 }
 
-static int config_irq(void *ctx, struct platform_device *pdev)
+static int config_irq(struct mem_ctl_info *mci, struct platform_device *pdev)
 {
 	int irq;
 	int rc;
@@ -169,13 +203,13 @@ static int config_irq(void *ctx, struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
-	rc = devm_request_irq(&pdev->dev, irq, mcr_isr, IRQF_TRIGGER_HIGH,
-			      DRV_NAME, ctx);
+	rc = devm_request_irq(&pdev->dev, irq, aspeed_mcr_isr, IRQF_TRIGGER_HIGH,
+			      DRV_NAME, mci);
 	if (rc)
 		return rc;
 
 	/* enable interrupts */
-	aspeed_set_irq(true);
+	aspeed_set_irq(mci, true);
 
 	return 0;
 }
@@ -183,11 +217,13 @@ static int config_irq(void *ctx, struct platform_device *pdev)
 static int init_csrows(struct mem_ctl_info *mci)
 {
 	struct csrow_info *csrow = mci->csrows[0];
-	u32 nr_pages, dram_type;
-	struct dimm_info *dimm;
+	struct aspeed_edac *priv = mci->pvt_info;
 	struct device_node *np;
+	struct dimm_info *dimm;
 	struct resource r;
-	u32 reg04;
+	unsigned int type;
+	u32 nr_pages;
+	u32 conf;
 	int rc;
 
 	/* retrieve info about physical memory from device tree */
@@ -213,12 +249,12 @@ static int init_csrows(struct mem_ctl_info *mci)
 	nr_pages = resource_size(&r) >> PAGE_SHIFT;
 	csrow->last_page = csrow->first_page + nr_pages - 1;
 
-	scoped_guard(raw_spinlock, &aspeed_lock)
-		reg04 = readl(aspeed_regs + ASPEED_MCR_CONF);
-	dram_type = (reg04 & ASPEED_MCR_CONF_DRAM_TYPE) ? MEM_DDR4 : MEM_DDR3;
+	scoped_guard(raw_spinlock, &priv->lock)
+		conf = readl(priv->regs + priv->chip->conf_reg);
+	type = field_get(priv->chip->conf_dram_type, conf);
 
 	dimm = csrow->channels[0]->dimm;
-	dimm->mtype = dram_type;
+	dimm->mtype = priv->chip->dram_type[type];
 	dimm->edac_mode = EDAC_SECDED;
 	dimm->nr_pages = nr_pages / csrow->nr_channels;
 	dimm->grain = 16;
@@ -231,22 +267,26 @@ static int init_csrows(struct mem_ctl_info *mci)
 
 static int aspeed_probe(struct platform_device *pdev)
 {
+	const struct aspeed_edac_chip *chip;
+	struct device *dev = &pdev->dev;
 	struct edac_mc_layer layers[2];
+	struct aspeed_edac *priv;
 	struct mem_ctl_info *mci;
 	void __iomem *regs;
-	u32 reg04;
+	u32 conf;
 	int rc;
 
+	chip = of_device_get_match_data(dev);
+	if (!chip)
+		return -EINVAL;
+
 	regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);
 
-	scoped_guard(raw_spinlock, &aspeed_lock)
-		aspeed_regs = regs;
-
 	/* bail out if ECC mode is not configured */
-	reg04 = readl(regs + ASPEED_MCR_CONF);
-	if (!(reg04 & ASPEED_MCR_CONF_ECC)) {
+	conf = readl(regs + chip->conf_reg);
+	if (!field_get(chip->conf_ecc, conf)) {
 		dev_err(&pdev->dev, "ECC mode is not configured in u-boot\n");
 		return -EPERM;
 	}
@@ -261,12 +301,17 @@ static int aspeed_probe(struct platform_device *pdev)
 	layers[1].size = 1;
 	layers[1].is_virt_csrow = false;
 
-	mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, 0);
+	mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*priv));
 	if (!mci)
 		return -ENOMEM;
 
+	priv = mci->pvt_info;
+	priv->chip = chip;
+	scoped_guard(raw_spinlock_init, &priv->lock)
+		priv->regs = regs;
+
 	mci->pdev = &pdev->dev;
-	mci->mtype_cap = MEM_FLAG_DDR3 | MEM_FLAG_DDR4;
+	mci->mtype_cap = chip->mtype_cap;
 	mci->edac_ctl_cap = EDAC_FLAG_SECDED;
 	mci->edac_cap = EDAC_FLAG_SECDED;
 	mci->scrub_cap = SCRUB_FLAG_HW_SRC;
@@ -311,17 +356,36 @@ static void aspeed_remove(struct platform_device *pdev)
 	struct mem_ctl_info *mci = platform_get_drvdata(pdev);
 
 	/* disable interrupts */
-	aspeed_set_irq(false);
+	aspeed_set_irq(mci, false);
 
 	/* free resources */
 	edac_mc_del_mc(&pdev->dev);
 	edac_mc_free(mci);
 }
 
+static const struct aspeed_edac_chip ast2400_edac = {
+	.conf_reg = ASPEED_MCR_CONF,
+	.conf_ecc = ASPEED_MCR_CONF_ECC,
+	.conf_dram_type = ASPEED_MCR_CONF_DRAM_TYPE,
+	.dram_type = { MEM_DDR3, MEM_DDR4 },
+	.mtype_cap = MEM_FLAG_DDR3 | MEM_FLAG_DDR4,
+	.prot_reg = ASPEED_MCR_PROT,
+	.prot_key = ASPEED_MCR_PROT_PASSWD,
+};
+
+/* The AST2600 does not key-protect the interrupt control register (MCR50). */
+static const struct aspeed_edac_chip ast2600_edac = {
+	.conf_reg = ASPEED_MCR_CONF,
+	.conf_ecc = ASPEED_MCR_CONF_ECC,
+	.conf_dram_type = ASPEED_MCR_CONF_DRAM_TYPE,
+	.dram_type = { MEM_DDR3, MEM_DDR4 },
+	.mtype_cap = MEM_FLAG_DDR3 | MEM_FLAG_DDR4,
+};
+
 static const struct of_device_id aspeed_of_match[] = {
-	{ .compatible = "aspeed,ast2400-sdram-edac" },
-	{ .compatible = "aspeed,ast2500-sdram-edac" },
-	{ .compatible = "aspeed,ast2600-sdram-edac" },
+	{ .compatible = "aspeed,ast2400-sdram-edac", .data = &ast2400_edac },
+	{ .compatible = "aspeed,ast2500-sdram-edac", .data = &ast2400_edac },
+	{ .compatible = "aspeed,ast2600-sdram-edac", .data = &ast2600_edac },
 	{},
 };
 

-- 
2.34.1