[PATCH 5/7] EDAC/aspeed: Replace regmap with direct register access
Ryan Chen
ryan_chen at aspeedtech.com
Tue Aug 11 22:48:29 PDT 2026
The driver instantiates its own regmap purely as an MMIO wrapper: it has
no register cache, uses custom reg_read/reg_write callbacks, and is not
shared as a syscon with other drivers. So it brings nothing here beyond
the spinlock that regmap takes around each access when fast_io is set.
Drop the regmap and access the registers directly with readl()/writel()
under an explicit raw spinlock, held across the whole read-modify-write so
the controller is unlocked once around the grouped writes rather than on
every register write. The lock is a raw_spinlock_t because the ECC
interrupt handler runs in hardirq context, where under PREEMPT_RT a
sleeping spinlock could not be acquired.
Annotate the register base with __guarded_by() so that, under
CONFIG_WARN_CONTEXT_ANALYSIS, the compiler checks at build time that every
hardware register access is performed while holding the lock.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ryan Chen <ryan_chen at aspeedtech.com>
---
drivers/edac/aspeed_edac.c | 132 +++++++++++++++++----------------------------
1 file changed, 48 insertions(+), 84 deletions(-)
diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
index 8bfeb21d3204..7bd552ee9a61 100644
--- a/drivers/edac/aspeed_edac.c
+++ b/drivers/edac/aspeed_edac.c
@@ -3,6 +3,7 @@
* Copyright 2018, 2019 Cisco Systems
*/
+#include <linux/cleanup.h>
#include <linux/edac.h>
#include <linux/init.h>
#include <linux/interrupt.h>
@@ -10,7 +11,7 @@
#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/platform_device.h>
-#include <linux/regmap.h>
+#include <linux/spinlock.h>
#include "edac_module.h"
#define DRV_NAME "aspeed-edac"
@@ -20,7 +21,6 @@
#define ASPEED_MCR_INTR_CTRL 0x50 /* interrupt control/status register */
#define ASPEED_MCR_ADDR_UNREC 0x58 /* address of first un-recoverable error */
#define ASPEED_MCR_ADDR_REC 0x5c /* address of last recoverable error */
-#define ASPEED_MCR_LAST ASPEED_MCR_ADDR_REC
#define ASPEED_MCR_PROT_PASSWD 0xfc600309
#define ASPEED_MCR_CONF_DRAM_TYPE BIT(4)
@@ -30,55 +30,8 @@
#define ASPEED_MCR_INTR_CTRL_CNT_UNREC GENMASK(15, 12)
#define ASPEED_MCR_INTR_CTRL_ENABLE (BIT(0) | BIT(1))
-static struct regmap *aspeed_regmap;
-
-static int regmap_reg_write(void *context, unsigned int reg, unsigned int val)
-{
- void __iomem *regs = (void __iomem *)context;
-
- /* enable write to MCR register set */
- writel(ASPEED_MCR_PROT_PASSWD, regs + ASPEED_MCR_PROT);
-
- writel(val, regs + reg);
-
- /* disable write to MCR register set */
- writel(~ASPEED_MCR_PROT_PASSWD, regs + ASPEED_MCR_PROT);
-
- return 0;
-}
-
-static int regmap_reg_read(void *context, unsigned int reg, unsigned int *val)
-{
- void __iomem *regs = (void __iomem *)context;
-
- *val = readl(regs + reg);
-
- return 0;
-}
-
-static bool regmap_is_volatile(struct device *dev, unsigned int reg)
-{
- switch (reg) {
- case ASPEED_MCR_PROT:
- case ASPEED_MCR_INTR_CTRL:
- case ASPEED_MCR_ADDR_UNREC:
- case ASPEED_MCR_ADDR_REC:
- return true;
- default:
- return false;
- }
-}
-
-static const struct regmap_config aspeed_regmap_config = {
- .reg_bits = 32,
- .val_bits = 32,
- .reg_stride = 4,
- .max_register = ASPEED_MCR_LAST,
- .reg_write = regmap_reg_write,
- .reg_read = regmap_reg_read,
- .volatile_reg = regmap_is_volatile,
- .fast_io = true,
-};
+static DEFINE_RAW_SPINLOCK(aspeed_lock);
+static void __iomem *aspeed_regs __guarded_by(&aspeed_lock);
static void count_rec(struct mem_ctl_info *mci, u8 rec_cnt, u32 rec_addr)
{
@@ -147,12 +100,24 @@ static irqreturn_t mcr_isr(int irq, void *arg)
{
struct mem_ctl_info *mci = arg;
u32 rec_addr, un_rec_addr;
- u32 reg50, reg5c, reg58;
- u8 rec_cnt, un_rec_cnt;
-
- regmap_read(aspeed_regmap, ASPEED_MCR_INTR_CTRL, ®50);
- dev_dbg(mci->pdev, "received edac interrupt w/ mcr register 50: 0x%x\n",
- reg50);
+ u8 rec_cnt, un_rec_cnt;
+ u32 reg50;
+
+ scoped_guard(raw_spinlock, &aspeed_lock) {
+ reg50 = readl(aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ dev_dbg(mci->pdev, "received edac interrupt w/ mcr register 50: 0x%x\n",
+ reg50);
+ un_rec_addr = readl(aspeed_regs + ASPEED_MCR_ADDR_UNREC);
+ rec_addr = readl(aspeed_regs + ASPEED_MCR_ADDR_REC);
+
+ /* clearing the counters needs a set-then-clear of CLEAR */
+ writel(ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+ writel(reg50 | ASPEED_MCR_INTR_CTRL_CLEAR,
+ aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ writel(reg50 & ~ASPEED_MCR_INTR_CTRL_CLEAR,
+ aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ writel(~ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+ }
/* collect data about recoverable and unrecoverable errors */
rec_cnt = (reg50 & ASPEED_MCR_INTR_CTRL_CNT_REC) >> 16;
@@ -161,20 +126,6 @@ static irqreturn_t mcr_isr(int irq, void *arg)
dev_dbg(mci->pdev, "%d recoverable interrupts and %d unrecoverable interrupts\n",
rec_cnt, un_rec_cnt);
- regmap_read(aspeed_regmap, ASPEED_MCR_ADDR_UNREC, ®58);
- un_rec_addr = reg58;
-
- regmap_read(aspeed_regmap, ASPEED_MCR_ADDR_REC, ®5c);
- rec_addr = reg5c;
-
- /* clear interrupt flags and error counters: */
- regmap_update_bits(aspeed_regmap, ASPEED_MCR_INTR_CTRL,
- ASPEED_MCR_INTR_CTRL_CLEAR,
- ASPEED_MCR_INTR_CTRL_CLEAR);
-
- regmap_update_bits(aspeed_regmap, ASPEED_MCR_INTR_CTRL,
- ASPEED_MCR_INTR_CTRL_CLEAR, 0);
-
/* process recoverable and unrecoverable errors */
count_rec(mci, rec_cnt, rec_addr);
count_un_rec(mci, un_rec_cnt, un_rec_addr);
@@ -182,13 +133,31 @@ static irqreturn_t mcr_isr(int irq, void *arg)
if (!rec_cnt && !un_rec_cnt)
dev_dbg(mci->pdev, "received edac interrupt, but did not find any ECC counters\n");
- regmap_read(aspeed_regmap, ASPEED_MCR_INTR_CTRL, ®50);
+ scoped_guard(raw_spinlock, &aspeed_lock)
+ reg50 = readl(aspeed_regs + ASPEED_MCR_INTR_CTRL);
dev_dbg(mci->pdev, "edac interrupt handled. mcr reg 50 is now: 0x%x\n",
reg50);
return IRQ_HANDLED;
}
+static void aspeed_set_irq(bool enable)
+{
+ u32 val;
+
+ guard(raw_spinlock_irqsave)(&aspeed_lock);
+
+ val = readl(aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ if (enable)
+ val |= ASPEED_MCR_INTR_CTRL_ENABLE;
+ else
+ val &= ~ASPEED_MCR_INTR_CTRL_ENABLE;
+
+ writel(ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+ writel(val, aspeed_regs + ASPEED_MCR_INTR_CTRL);
+ writel(~ASPEED_MCR_PROT_PASSWD, aspeed_regs + ASPEED_MCR_PROT);
+}
+
static int config_irq(void *ctx, struct platform_device *pdev)
{
int irq;
@@ -206,9 +175,7 @@ static int config_irq(void *ctx, struct platform_device *pdev)
return rc;
/* enable interrupts */
- regmap_update_bits(aspeed_regmap, ASPEED_MCR_INTR_CTRL,
- ASPEED_MCR_INTR_CTRL_ENABLE,
- ASPEED_MCR_INTR_CTRL_ENABLE);
+ aspeed_set_irq(true);
return 0;
}
@@ -246,7 +213,8 @@ static int init_csrows(struct mem_ctl_info *mci)
nr_pages = resource_size(&r) >> PAGE_SHIFT;
csrow->last_page = csrow->first_page + nr_pages - 1;
- regmap_read(aspeed_regmap, ASPEED_MCR_CONF, ®04);
+ scoped_guard(raw_spinlock, &aspeed_lock)
+ reg04 = readl(aspeed_regs + ASPEED_MCR_CONF);
dram_type = (reg04 & ASPEED_MCR_CONF_DRAM_TYPE) ? MEM_DDR4 : MEM_DDR3;
dimm = csrow->channels[0]->dimm;
@@ -263,7 +231,6 @@ static int init_csrows(struct mem_ctl_info *mci)
static int aspeed_probe(struct platform_device *pdev)
{
- struct device *dev = &pdev->dev;
struct edac_mc_layer layers[2];
struct mem_ctl_info *mci;
void __iomem *regs;
@@ -274,13 +241,11 @@ static int aspeed_probe(struct platform_device *pdev)
if (IS_ERR(regs))
return PTR_ERR(regs);
- aspeed_regmap = devm_regmap_init(dev, NULL, (__force void *)regs,
- &aspeed_regmap_config);
- if (IS_ERR(aspeed_regmap))
- return PTR_ERR(aspeed_regmap);
+ scoped_guard(raw_spinlock, &aspeed_lock)
+ aspeed_regs = regs;
/* bail out if ECC mode is not configured */
- regmap_read(aspeed_regmap, ASPEED_MCR_CONF, ®04);
+ reg04 = readl(regs + ASPEED_MCR_CONF);
if (!(reg04 & ASPEED_MCR_CONF_ECC)) {
dev_err(&pdev->dev, "ECC mode is not configured in u-boot\n");
return -EPERM;
@@ -346,8 +311,7 @@ static void aspeed_remove(struct platform_device *pdev)
struct mem_ctl_info *mci = platform_get_drvdata(pdev);
/* disable interrupts */
- regmap_update_bits(aspeed_regmap, ASPEED_MCR_INTR_CTRL,
- ASPEED_MCR_INTR_CTRL_ENABLE, 0);
+ aspeed_set_irq(false);
/* free resources */
edac_mc_del_mc(&pdev->dev);
--
2.34.1
More information about the linux-arm-kernel
mailing list