[PATCH 0/3] Add Renesas RZ/N1x EDAC driver

[PATCH 0/3] Add Renesas RZ/N1x EDAC driver

[Paul Louvel]

The Cadence memory controller found on Renesas RZ/N1x SoCs supports ECC
with SECDED.

The memory controller found on the r9a06g032 supports at most a single
DIMM of DDR2/3, up to 2GB.

Add the EDAC driver for this memory controller, and the relevant
device-tree binding. Also add the EDAC node to the existing r9a06g032
SoC base device-tree.

Signed-off-by: Paul Louvel (Schneider Electric) <paul.louvel@bootlin.com>
---
Paul Louvel (3):
      dt-bindings: edac: cdns,ddr-edac: add Cadence DDR EDAC binding
      EDAC/cadence: Add Cadence DDR EDAC driver
      ARM: dts: renesas: r9a06g032: add EDAC node

 .../devicetree/bindings/edac/cdns,ddr-edac.yaml    |  62 ++++
 MAINTAINERS                                        |   7 +
 arch/arm/boot/dts/renesas/r9a06g032.dtsi           |  10 +
 drivers/edac/Kconfig                               |  10 +
 drivers/edac/Makefile                              |   1 +
 drivers/edac/cadence_edac.c                        | 333 +++++++++++++++++++++
 6 files changed, 423 insertions(+)
---
base-commit: d9f39b2c0579f313d14954a9ec584511c543aab5
change-id: 20260916-paul-v7-3-rc1-edac-1f479f96fc57

Best regards,
--  
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 1/3] dt-bindings: edac: cdns,ddr-edac: add Cadence DDR EDAC binding

[Paul Louvel]

Add the Cadence EDAC DT binding.

Signed-off-by: Paul Louvel (Schneider Electric) <paul.louvel@bootlin.com>
---
 .../devicetree/bindings/edac/cdns,ddr-edac.yaml    | 62 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 +++
 2 files changed, 68 insertions(+)

diff --git a/Documentation/devicetree/bindings/edac/cdns,ddr-edac.yaml b/Documentation/devicetree/bindings/edac/cdns,ddr-edac.yaml
new file mode 100644
index 000000000000..e0deeb3dfb81
--- /dev/null
+++ b/Documentation/devicetree/bindings/edac/cdns,ddr-edac.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/edac/cdns,ddr-edac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cadence DDR EDAC
+
+maintainers:
+  - Paul Louvel <paul.louvel@bootlin.com>
+
+description: >
+  The Cadence DDR supports DDR2 and DDR3 memory with or without ECC.
+  The bootloader must configure ECC mode in the memory controller.
+
+  The memory controller supports SECDED (single bit error correction, double bit
+  error detection). ECC scrubbing has to be done via software.
+
+properties:
+  compatible:
+    items:
+      - const: renesas,r9a06g032-ddr-edac # RZ/N1D
+      - const: renesas,rzn1-ddr-edac      # RZ/N1
+      - const: cdns,ddr-edac
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: DDR controller clock
+      - description: APB internal bus clock
+
+  clock-names:
+    items:
+      - const: ddrc
+      - const: pclk
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/r9a06g032-sysctrl.h>
+
+    edac: memory-controller@4000d000 {
+      compatible = "renesas,r9a06g032-ddr-edac", "renesas,rzn1-ddr-edac", "cdns,ddr-edac";
+      reg = <0x4000d000 0x1000>;
+      interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&sysctrl R9A06G032_CLK_DDRC>, <&sysctrl R9A06G032_HCLK_DDRC>;
+      clock-names = "ddrc", "pclk";
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 3a19da74d00c..60db3734df5b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5760,6 +5760,12 @@ L:	linux-mm@kvack.org
 S:	Maintained
 F:	tools/testing/selftests/cachestat/test_cachestat.c
 
+CADENCE DDR EDAC DRIVER
+M:	Paul Louvel <paul.louvel@bootlin.com>
+L:	linux-edac@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/edac/cdns,ddr-edac.yaml
+
 CADENCE MIPI-CSI2 BRIDGES
 M:	Maxime Ripard <mripard@kernel.org>
 L:	linux-media@vger.kernel.org

-- 
2.55.0

[PATCH 2/3] EDAC/cadence: Add Cadence DDR EDAC driver

[Paul Louvel]

Add the Cadence EDAC driver found on Renesas RZ/N1x SoC.
The memory controller supports ECC, software scrubbing, and SECDED.

Signed-off-by: Paul Louvel (Schneider Electric) <paul.louvel@bootlin.com>
---
 MAINTAINERS                 |   1 +
 drivers/edac/Kconfig        |  10 ++
 drivers/edac/Makefile       |   1 +
 drivers/edac/cadence_edac.c | 333 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 345 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 60db3734df5b..0c7ab1d22172 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5765,6 +5765,7 @@ M:	Paul Louvel <paul.louvel@bootlin.com>
 L:	linux-edac@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/edac/cdns,ddr-edac.yaml
+F:	drivers/edac/cadence_edac.c
 
 CADENCE MIPI-CSI2 BRIDGES
 M:	Maxime Ripard <mripard@kernel.org>
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index a44b85c440ca..1707fe76b53c 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -503,6 +503,16 @@ config EDAC_QCOM
 	  For debugging issues having to do with stability and overall system
 	  health, you should probably say 'Y' here.
 
+config EDAC_CADENCE
+	tristate "Cadence EDAC Controller"
+	depends on HAS_IOMEM && OF
+	depends on ARCH_RZN1 || COMPILE_TEST
+	help
+	  Support for error detection and correction on RZN1x SoCs. ECC must be
+	  configured by the bootloader.
+	  The controller supports single bit error correction, double bit error
+	  detection.
+
 config EDAC_ASPEED
 	tristate "Aspeed AST BMC SoC"
 	depends on ARCH_ASPEED
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index a37534300ab9..0d66a072b15c 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
+obj-$(CONFIG_EDAC_CADENCE)		+= cadence_edac.o
 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/cadence_edac.c b/drivers/edac/cadence_edac.c
new file mode 100644
index 000000000000..6567ac5147d0
--- /dev/null
+++ b/drivers/edac/cadence_edac.c
@@ -0,0 +1,333 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2015 Renesas Electronics Europe Ltd.
+ * Copyright 2026 Bootlin
+ *
+ * Based on highbank EDAC driver:
+ *
+ * Copyright 2011-2012 Calxeda, Inc.
+ */
+
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/edac.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/of_address.h>
+#include <linux/string.h>
+#include <linux/platform_device.h>
+
+#include "edac_mc.h"
+
+#define DRV_NAME "cdns_edac"
+
+#define REG_BYTE_SZ 4
+#define DDR_CTL(n) ((n) * REG_BYTE_SZ)
+
+#define CDNS_DDR_DDR_STAT DDR_CTL(0)
+#define CDNS_DDR_DDR_STAT_DRAM_CLASS GENMASK_U32(11, 8)
+#define CDNS_DDR_DDR_STAT_DRAM_DDR2 BIT(2)
+#define CDNS_DDR_DDR_STAT_GET_DRAM_CLASS(reg) FIELD_GET(CDNS_DDR_DDR_STAT_DRAM_CLASS, reg)
+
+#define CDNS_DDR_ECC_STAT DDR_CTL(36)
+#define CDNS_DDR_ECC_STAT_ENABLED BIT(16)
+#define CDNS_DDR_ECC_STAT_IS_ENABLED(reg) FIELD_GET(CDNS_DDR_ECC_STAT_ENABLED, reg)
+#define CDNS_DDR_ECC_STAT_FWC BIT(24)
+
+#define CDNS_DDR_ECC_XOR DDR_CTL(37)
+#define CDNS_DDR_ECC_XOR_CHECK_BITS GENMASK_U32(13, 0)
+
+#define CDNS_DDR_BUS_CTRL DDR_CTL(54)
+#define CDNS_DDR_BUS_CTRL_REDUC BIT(1)
+
+/* DDR Controller Error Registers */
+
+#define CDNS_DDR_ECC_U_ERR_ADDR DDR_CTL(38)
+#define CDNS_DDR_ECC_U_ERR_STAT DDR_CTL(39)
+
+#define CDNS_DDR_ECC_C_ERR_ADDR DDR_CTL(41)
+#define CDNS_DDR_ECC_C_ERR_STAT DDR_CTL(42)
+
+#define CDNS_DDR_ECC_ERR_STAT_GET_SYNDROME(reg) FIELD_GET(GENMASK(6, 0), reg)
+
+#define CDNS_DDR_PORT_CMD_ERR_ADDR DDR_CTL(61)
+#define CDNS_DDR_PORT_CMD_ERR_TYPE DDR_CTL(62)
+#define CDNS_DDR_PORT_CMD_ERR_TYPE_GET(reg) FIELD_GET(GENMASK_U32(10, 8), reg)
+
+/* DDR Controller Interrupt Registers */
+
+#define CDNS_DDR_ECC_INT_STAT DDR_CTL(56)
+#define CDNS_DDR_ECC_INT_STAT_CE BIT(3)
+#define CDNS_DDR_ECC_INT_STAT_MULTIPLE_CE BIT(4)
+#define CDNS_DDR_ECC_INT_STAT_UE BIT(5)
+#define CDNS_DDR_ECC_INT_STAT_MULTIPLE_UE BIT(6)
+#define CDNS_DDR_ECC_INT_STAT_PORT_CMD_CHAN BIT(7)
+
+#define CDNS_DDR_ECC_INT_ACK DDR_CTL(57)
+#define CDNS_DDR_ECC_INT_ACK_MASK GENMASK_U32(21, 0)
+
+#define CDNS_DDR_ECC_INT_CTRL DDR_CTL(58)
+#define CDNS_DDR_ECC_INT_CTRL_MASK GENMASK_U32(21, 0)
+#define CDNS_DDR_ECC_INT_CTRL_MASK_ALL BIT(22)
+#define CDNS_DDR_ECC_INT_CTRL_UNMASK(i) ((~(i)) & CDNS_DDR_ECC_INT_CTRL_MASK)
+
+struct cdns_mc_priv {
+	void __iomem *io_base;
+};
+
+static void cdns_rmw(struct cdns_mc_priv *drv, u32 reg, u32 mask, u32 val)
+{
+	u32 regval;
+
+	regval = readl(drv->io_base + reg);
+	FIELD_MODIFY(mask, &regval, val);
+	writel(regval, drv->io_base + reg);
+}
+
+static irqreturn_t cdns_mc_err_handler(int irq, void *dev_id)
+{
+	struct mem_ctl_info *mci = dev_id;
+	struct cdns_mc_priv *priv = mci->pvt_info;
+	u32 addr, status, err_addr, syndrome, reg;
+	char other_details_str[32];
+	u8 type;
+
+	/* Read the interrupt status register */
+	status = readl(priv->io_base + CDNS_DDR_ECC_INT_STAT);
+
+	/*
+	 * We can't know how many CE / UE occurred since last ACK in case of
+	 * multiple errors. Just report it.
+	 */
+
+	if ((status & CDNS_DDR_ECC_INT_STAT_UE) || (status & CDNS_DDR_ECC_INT_STAT_MULTIPLE_UE)) {
+		reg = readl(priv->io_base + CDNS_DDR_ECC_U_ERR_STAT);
+		syndrome = CDNS_DDR_ECC_ERR_STAT_GET_SYNDROME(reg);
+
+		err_addr = readl(priv->io_base + CDNS_DDR_ECC_U_ERR_ADDR);
+
+		edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci, 1, err_addr >> PAGE_SHIFT,
+				     err_addr & ~PAGE_MASK, syndrome, 0, 0, -1, mci->ctl_name, "");
+	}
+
+	if ((status & CDNS_DDR_ECC_INT_STAT_CE) || (status & CDNS_DDR_ECC_INT_STAT_MULTIPLE_CE)) {
+		reg = readl(priv->io_base + CDNS_DDR_ECC_C_ERR_STAT);
+		syndrome = CDNS_DDR_ECC_ERR_STAT_GET_SYNDROME(reg);
+
+		err_addr = readl(priv->io_base + CDNS_DDR_ECC_C_ERR_ADDR);
+
+		edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci, 1, err_addr >> PAGE_SHIFT,
+				     err_addr & ~PAGE_MASK, syndrome, 0, 0, -1, mci->ctl_name, "");
+	}
+
+	if (status & CDNS_DDR_ECC_INT_STAT_PORT_CMD_CHAN) {
+		addr = readl(priv->io_base + CDNS_DDR_PORT_CMD_ERR_ADDR);
+		reg = readl(priv->io_base + CDNS_DDR_PORT_CMD_ERR_TYPE);
+		type = CDNS_DDR_PORT_CMD_ERR_TYPE_GET(reg);
+
+		snprintf(other_details_str, sizeof(other_details_str), "type 0x%02x", type);
+
+		edac_mc_handle_error(HW_EVENT_ERR_INFO, mci, 1, addr >> PAGE_SHIFT,
+				     addr & ~PAGE_MASK, 0, 0, 0, -1, mci->ctl_name,
+				     other_details_str);
+	}
+
+	/* clear the error, clears the interrupt */
+	writel(status & CDNS_DDR_ECC_INT_ACK_MASK, priv->io_base + CDNS_DDR_ECC_INT_ACK);
+
+	return IRQ_HANDLED;
+}
+
+static void cdns_mc_err_inject(struct mem_ctl_info *mci, u16 synd)
+{
+	struct cdns_mc_priv *priv = mci->pvt_info;
+
+	cdns_rmw(priv, CDNS_DDR_ECC_XOR, CDNS_DDR_ECC_XOR_CHECK_BITS, synd);
+	cdns_rmw(priv, CDNS_DDR_ECC_STAT, CDNS_DDR_ECC_STAT_FWC, 1);
+}
+
+static ssize_t inject_ctrl_store(struct device *dev, struct device_attribute *attr, const char *buf,
+				 size_t count)
+{
+	struct mem_ctl_info *mci = to_mci(dev);
+	u16 synd;
+
+	if (kstrtou16(buf, 16, &synd))
+		return -EINVAL;
+
+	cdns_mc_err_inject(mci, synd);
+
+	return count;
+}
+
+static DEVICE_ATTR_WO(inject_ctrl);
+
+static struct attribute *cdns_edac_attrs[] = { &dev_attr_inject_ctrl.attr, NULL };
+
+ATTRIBUTE_GROUPS(cdns_edac);
+
+static void devm_cdns_edac_free(void *data)
+{
+	edac_mc_free(data);
+}
+
+static void devm_cdns_edac_del(void *data)
+{
+	edac_mc_del_mc(data);
+}
+
+static int cdns_get_mem_sz(resource_size_t *mem_sz)
+{
+	struct device_node *np;
+	struct resource res;
+	int ret;
+
+	np = of_find_node_by_name(NULL, "memory");
+	if (!np)
+		return -ENODEV;
+
+	ret = of_address_to_resource(np, 0, &res);
+
+	of_node_put(np);
+
+	if (ret)
+		return ret;
+
+	*mem_sz = resource_size(&res);
+
+	return 0;
+}
+
+static int cdns_mc_probe(struct platform_device *pdev)
+{
+struct edac_mc_layer layers[2];
+	struct clk_bulk_data clks[2] = {
+		{ .id = "ddrc" },
+		{ .id = "pclk" },
+	};
+	struct cdns_mc_priv *priv;
+	struct mem_ctl_info *mci;
+	struct dimm_info *dimm;
+	resource_size_t mem_sz;
+	int ret, irq;
+	u32 reg;
+
+	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
+	layers[0].size = 1;
+	layers[0].is_virt_csrow = true;
+	layers[1].type = EDAC_MC_LAYER_CHANNEL;
+	layers[1].size = 1;
+	layers[1].is_virt_csrow = false;
+	mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(struct cdns_mc_priv));
+	if (!mci)
+		return dev_err_probe(&pdev->dev, -ENOMEM, "unable to allocate edac mc\n");
+
+	ret = devm_add_action_or_reset(&pdev->dev, devm_cdns_edac_free, mci);
+	if (ret)
+		return ret;
+
+	mci->pdev = &pdev->dev;
+	priv = mci->pvt_info;
+
+	priv->io_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->io_base))
+		return dev_err_probe(&pdev->dev, PTR_ERR(priv->io_base), "unable to map regs\n");
+
+	ret = devm_clk_bulk_get_enable(&pdev->dev, ARRAY_SIZE(clks), clks);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "unable to enable clocks\n");
+
+	reg = readl(priv->io_base + CDNS_DDR_ECC_STAT);
+	if (!CDNS_DDR_ECC_STAT_IS_ENABLED(reg))
+		mci->edac_cap = EDAC_FLAG_NONE;
+	else
+		mci->edac_cap = EDAC_FLAG_SECDED;
+
+	mci->mtype_cap = MEM_FLAG_DDR2 | MEM_FLAG_DDR3;
+	mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
+	mci->mod_name = pdev->dev.driver->name;
+	mci->ctl_name = "cdns-ddr-ctrl";
+	mci->dev_name = dev_name(&pdev->dev);
+	mci->scrub_mode = SCRUB_SW_SRC;
+
+	ret = cdns_get_mem_sz(&mem_sz);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "unable to get memory size\n");
+
+	dimm = *mci->dimms;
+	dimm->nr_pages = PFN_UP(mem_sz);
+	dimm->grain = 4;
+	dimm->edac_mode = EDAC_SECDED;
+
+	/* Check if half datapath feature of the controller is active. */
+	reg = readl(priv->io_base + CDNS_DDR_BUS_CTRL);
+	if (reg & CDNS_DDR_BUS_CTRL_REDUC)
+		dimm->dtype = DEV_X8;
+	else
+		dimm->dtype = DEV_X16;
+
+	strscpy(dimm->label, "Channel#0_DIMM#0", sizeof(dimm->label));
+
+	reg = readl(priv->io_base + CDNS_DDR_DDR_STAT);
+	if (CDNS_DDR_DDR_STAT_GET_DRAM_CLASS(reg) == CDNS_DDR_DDR_STAT_DRAM_DDR2)
+		dimm->mtype = MEM_DDR2;
+	else
+		dimm->mtype = MEM_DDR3;
+
+	ret = edac_mc_add_mc_with_groups(mci, cdns_edac_groups);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to add mc\n");
+
+	ret = devm_add_action_or_reset(&pdev->dev, devm_cdns_edac_del, &pdev->dev);
+	if (ret)
+		return ret;
+
+	writel(CDNS_DDR_ECC_INT_CTRL_MASK_ALL, priv->io_base + CDNS_DDR_ECC_INT_CTRL);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return dev_err_probe(&pdev->dev, irq, "unable to get irq\n");
+
+	ret = devm_request_irq(&pdev->dev, irq, cdns_mc_err_handler, 0, dev_name(&pdev->dev), mci);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "unable to request irq %d\n", irq);
+
+	/*
+	 * Unmask ECC recoverable and unrecoverable interrupts, and port
+	 * command errors.
+	 */
+	writel(CDNS_DDR_ECC_INT_CTRL_UNMASK(
+		       CDNS_DDR_ECC_INT_STAT_CE | CDNS_DDR_ECC_INT_STAT_MULTIPLE_CE |
+		       CDNS_DDR_ECC_INT_STAT_UE | CDNS_DDR_ECC_INT_STAT_MULTIPLE_UE |
+		       CDNS_DDR_ECC_INT_STAT_PORT_CMD_CHAN),
+	       priv->io_base + CDNS_DDR_ECC_INT_CTRL);
+
+	edac_mc_chipset_printk(mci, KERN_DEBUG, DRV_NAME, "ECC %s",
+			       (mci->edac_cap == EDAC_FLAG_NONE) ? "disabled" : "enabled");
+
+	return 0;
+}
+
+static const struct of_device_id cdns_ddr_ctrl_of_match[] = {
+	{ .compatible = "cdns,ddr-edac" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, cdns_ddr_ctrl_of_match);
+
+static struct platform_driver cdns_mc_edac_driver = {
+	.probe = cdns_mc_probe,
+	.driver = {
+		.name = DRV_NAME,
+		.of_match_table = cdns_ddr_ctrl_of_match,
+	},
+};
+
+module_platform_driver(cdns_mc_edac_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Renesas Electronics Europe Ltd.");
+MODULE_AUTHOR("Paul Louvel <paul.louvel@bootlin.com>");
+MODULE_DESCRIPTION("EDAC driver for Cadence DDR controller");

-- 
2.55.0

[PATCH 3/3] ARM: dts: renesas: r9a06g032: add EDAC node

[Paul Louvel]

Add EDAC node to the SoC base device tree file.

Signed-off-by: Paul Louvel (Schneider Electric) <paul.louvel@bootlin.com>
---
 arch/arm/boot/dts/renesas/r9a06g032.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/renesas/r9a06g032.dtsi b/arch/arm/boot/dts/renesas/r9a06g032.dtsi
index 19c9bce0a26d..779221931fb6 100644
--- a/arch/arm/boot/dts/renesas/r9a06g032.dtsi
+++ b/arch/arm/boot/dts/renesas/r9a06g032.dtsi
@@ -164,6 +164,16 @@ dmamux: dma-router@a0 {
 			};
 		};
 
+		edac: memory-controller@4000d000 {
+			compatible = "renesas,r9a06g032-ddr-edac", "renesas,rzn1-ddr-edac",
+				     "cdns,ddr-edac";
+			reg = <0x4000d000 0x1000>;
+			interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&sysctrl R9A06G032_CLK_DDRC>, <&sysctrl R9A06G032_HCLK_DDRC>;
+			clock-names = "ddrc", "pclk";
+			status = "disabled";
+		};
+
 		udc: usb@4001e000 {
 			compatible = "renesas,r9a06g032-usbf", "renesas,rzn1-usbf";
 			reg = <0x4001e000 0x2000>;

-- 
2.55.0

Re: [PATCH 2/3] EDAC/cadence: Add Cadence DDR EDAC driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] core: stack use-after-free in devm_clk_bulk_get_enable()
- [Medium] irq: spurious IRQ detection defeated by unconditional IRQ_HANDLED
- [Medium] irq: missing hardware interrupt mask on driver removal
- [Low] build: build failure due to non-constant mask in FIELD_MODIFY()
- [Low] sysfs: data race during hardware RMW in cdns_rmw()
--

commit 6d767e55902586f690d28deaaff002c58978b206
Author: Paul Louvel <paul.louvel@bootlin.com>

EDAC/cadence: Add Cadence DDR EDAC driver

Add the Cadence EDAC driver found on Renesas RZ/N1x SoC.
The memory controller supports ECC, software scrubbing, and SECDED.

> diff --git a/drivers/edac/cadence_edac.c b/drivers/edac/cadence_edac.c
> new file mode 100644
> index 0000000000000..6567ac5147d05
> --- /dev/null
> +++ b/drivers/edac/cadence_edac.c
[ ... ]
> +static void cdns_rmw(struct cdns_mc_priv *drv, u32 reg, u32 mask, u32 val)
> +{
> +	u32 regval;
> +
> +	regval = readl(drv->io_base + reg);
> +	FIELD_MODIFY(mask, &regval, val);

[Severity: Low]
Does this compile under all configurations? FIELD_MODIFY requires the mask
argument to be a compile-time constant, as it internally asserts
BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask), ...). Since mask is passed as
a variable argument to cdns_rmw() in drivers/edac/cadence_edac.c, this can
lead to build failures if the compiler decides not to inline this function.

[Severity: Low]
Is it possible to corrupt the hardware state here due to concurrent access?
This read-modify-write sequence lacks synchronization. Since cdns_rmw() is
called from cdns_mc_err_inject() in drivers/edac/cadence_edac.c, which is
invoked via the inject_ctrl_store() sysfs callback, concurrent sysfs writes
could interleave and cause lost register updates.

> +	writel(regval, drv->io_base + reg);
> +}
> +
> +static irqreturn_t cdns_mc_err_handler(int irq, void *dev_id)
> +{
> +	struct mem_ctl_info *mci = dev_id;
> +	struct cdns_mc_priv *priv = mci->pvt_info;
> +	u32 addr, status, err_addr, syndrome, reg;
> +	char other_details_str[32];
> +	u8 type;
> +
> +	/* Read the interrupt status register */
> +	status = readl(priv->io_base + CDNS_DDR_ECC_INT_STAT);

[ ... ]

> +	/* clear the error, clears the interrupt */
> +	writel(status & CDNS_DDR_ECC_INT_ACK_MASK, priv->io_base + CDNS_DDR_ECC_INT_ACK);
> +
> +	return IRQ_HANDLED;

[Severity: Medium]
Can this unconditionally return IRQ_HANDLED even on spurious interrupts? If
the read status is 0, the handler won't process anything but still returns
IRQ_HANDLED. This prevents the core IRQ subsystem from detecting an unhandled
stuck IRQ line and disabling it, which might lead to an interrupt storm and a
hard lockup.

> +}

[ ... ]

> +static int cdns_mc_probe(struct platform_device *pdev)
> +{
> +struct edac_mc_layer layers[2];
> +	struct clk_bulk_data clks[2] = {
> +		{ .id = "ddrc" },
> +		{ .id = "pclk" },
> +	};

[ ... ]

> +	ret = devm_clk_bulk_get_enable(&pdev->dev, ARRAY_SIZE(clks), clks);

[Severity: High]
Does this code pass a stack pointer to the devres framework? The clks array is
allocated on the local stack in cdns_mc_probe() in drivers/edac/cadence_edac.c.
When passed to devm_clk_bulk_get_enable(), the devres framework stores this
pointer internally. If the device is later unbound or if a subsequent probe
step fails, the cleanup function will dereference this stale stack pointer to
disable the clocks, resulting in a use-after-free and likely a kernel panic.

[ ... ]

> +	/*
> +	 * Unmask ECC recoverable and unrecoverable interrupts, and port
> +	 * command errors.
> +	 */
> +	writel(CDNS_DDR_ECC_INT_CTRL_UNMASK(
> +		       CDNS_DDR_ECC_INT_STAT_CE | CDNS_DDR_ECC_INT_STAT_MULTIPLE_CE |
> +		       CDNS_DDR_ECC_INT_STAT_UE | CDNS_DDR_ECC_INT_STAT_MULTIPLE_UE |
> +		       CDNS_DDR_ECC_INT_STAT_PORT_CMD_CHAN),
> +	       priv->io_base + CDNS_DDR_ECC_INT_CTRL);

[Severity: Medium]
Are these interrupts properly disabled when the driver is removed? Hardware
interrupts are explicitly unmasked here in cdns_mc_probe() in
drivers/edac/cadence_edac.c, but there does not appear to be a .remove
callback or a custom devm action to mask them again. When the driver is
unbound, devres will deregister the software IRQ handler, but the hardware
will remain active. If the hardware asserts an interrupt after removal, it
will be unhandled.

> +
> +	edac_mc_chipset_printk(mci, KERN_DEBUG, DRV_NAME, "ECC %s",
> +			       (mci->edac_cap == EDAC_FLAG_NONE) ? "disabled" : "enabled");
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-paul-v7-3-rc1-edac-v1-0-70be37c41a18@bootlin.com?part=2