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

[PATCH v2 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>
---
Changes in v2:
- Patch 2:

    - FIELD_MODIFY() is always called with a compile-time constant mask
      in this driver. I see no problem here.

    - Use devm_clk_get_enabled() so the driver does not need to store
      struct clk_bulk_data outside the stack space.
      Clocks are not manipulated outside of probe.

    - Introduced a mutex for rmw operations in inject_ctrl_store() sysfs
      callback. Rename drv to priv in cdns_rmw().

    - Return IRQ_NONE if the stat int register is empty.

    - I see no reason to follow Sashiko last remark on this patch.
      Is an unhandled interrupt a big deal in this case ?

- Link to v1: https://patch.msgid.link/20260924-paul-v7-3-rc1-edac-v1-0-70be37c41a18@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                        | 340 +++++++++++++++++++++
 6 files changed, 430 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 v2 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 v2 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 | 340 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 352 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..7edac7795fa7
--- /dev/null
+++ b/drivers/edac/cadence_edac.c
@@ -0,0 +1,340 @@
+// 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/mutex.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;
+	struct mutex lock;
+};
+
+static void cdns_rmw(struct cdns_mc_priv *priv, u32 reg, u32 mask, u32 val)
+{
+	u32 regval;
+
+	mutex_lock(&priv->lock);
+	regval = readl(priv->io_base + reg);
+	FIELD_MODIFY(mask, &regval, val);
+	writel(regval, priv->io_base + reg);
+	mutex_unlock(&priv->lock);
+}
+
+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);
+	if (!status)
+		return IRQ_NONE;
+
+	/*
+	 * 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 cdns_mc_priv *priv;
+	struct mem_ctl_info *mci;
+	struct dimm_info *dimm;
+	resource_size_t mem_sz;
+	struct clk *clk;
+	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");
+
+	clk = devm_clk_get_enabled(&pdev->dev, "ddrc");
+	if (IS_ERR(clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(clk), "unable to get ddrc\n");
+
+	clk = devm_clk_get_enabled(&pdev->dev, "pclk");
+	if (IS_ERR(clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(clk), "unable to get pclk\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 v2 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 v2 2/3] EDAC/cadence: Add Cadence DDR EDAC driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] locking: uninitialized mutex priv->lock causes panic on sysfs write
- [Medium] driver-core: premature sysfs registration creates probe failure race
- [Low] build: FIELD_MODIFY fails build with non-constant mask parameter
- [Low] log: missing newline in edac_mc_chipset_printk format string
--

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

EDAC/cadence: Add Cadence DDR EDAC driver

This commit adds a new EDAC driver for the Cadence DDR memory controller
found on the Renesas RZ/N1x SoC. It supports single bit error correction,
double bit error detection, and software scrubbing.

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

[Severity: Low]
Will this cause a build failure on some configurations? FIELD_MODIFY
internally requires its mask parameter to be a compile-time constant,
enforced via a build-time check. Since cdns_rmw() takes mask as a standard
function parameter and isn't explicitly inlined, the compiler might not be
able to resolve it as a constant.

> +	writel(regval, priv->io_base + reg);
> +	mutex_unlock(&priv->lock);
> +}
[ ... ]
> +static int cdns_mc_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	mci->pdev = &pdev->dev;
> +	priv = mci->pvt_info;

[Severity: High]
Is priv->lock missing a call to mutex_init()? 

The edac_mc_alloc() function allocates mci->pvt_info with kzalloc(), meaning
the mutex is zero-initialized but never properly initialized. Calling
mutex_lock() on this in cdns_rmw() can corrupt the wait list or trigger a
kernel panic, which could be reached if userspace writes to the inject_ctrl
sysfs attribute.

> +
> +	priv->io_base = devm_platform_ioremap_resource(pdev, 0);
[ ... ]
> +	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);

[Severity: Medium]
Could exposing the sysfs interface before claiming the IRQ create a race
condition?

Calling edac_mc_add_mc_with_groups() registers the controller and exposes
sysfs attributes like inject_ctrl to userspace. If platform_get_irq() or the
subsequent devm_request_irq() defers or fails, the probe unwinds and tears
down the device. A concurrent userspace access during this window might
interact with a partially initialized or destroyed state.

> +	if (irq < 0)
> +		return dev_err_probe(&pdev->dev, irq, "unable to get irq\n");
[ ... ]
> +	edac_mc_chipset_printk(mci, KERN_DEBUG, DRV_NAME, "ECC %s",
> +			       (mci->edac_cap == EDAC_FLAG_NONE) ? "disabled" : "enabled");

[Severity: Low]
Is this format string missing a trailing newline? 

The edac_mc_chipset_printk() macro translates directly to a printk() call
and does not append a newline character automatically. Without it, subsequent
logs might end up appended to the same line in dmesg.

> +
> +	return 0;
> +}

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