[PATCH 0/3] SiFive DDR controller and EDAC support

[PATCH 0/3] SiFive DDR controller and EDAC support

[Yash Shah]

The series add supports for SiFive DDR controller driver. This driver
is use to manage the Cadence DDR controller present in SiFive SoCs.
Currently it manages only the EDAC feature of the DDR controller.
The series also adds Memory controller EDAC support for SiFive platform.
It register for notifier event from SiFive DDR controller driver.

The series is tested and based on Linux v5.8.

For testing on Hifive Unleashed:
1. Enable the ECC bit of DDR controller during DDR initialization
2. Erase the entire DRAM in bootloader stage
3. Using FWC feature of DDR controller force ecc error to test

Yash Shah (3):
  dt-bindings: riscv: Add DT documentation for DDR Controller in SiFive
    SoCs
  soc: sifive: Add SiFive specific Cadence DDR controller driver
  edac: sifive: Add EDAC support for Memory Controller in SiFive SoCs

 .../devicetree/bindings/riscv/sifive-ddr.yaml      |  41 ++++
 drivers/edac/Kconfig                               |   2 +-
 drivers/edac/sifive_edac.c                         | 117 ++++++++++++
 drivers/soc/sifive/Kconfig                         |   6 +
 drivers/soc/sifive/Makefile                        |   3 +-
 drivers/soc/sifive/sifive_ddr.c                    | 207 +++++++++++++++++++++
 include/soc/sifive/sifive_ddr.h                    |  73 ++++++++
 7 files changed, 447 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/riscv/sifive-ddr.yaml
 create mode 100644 drivers/soc/sifive/sifive_ddr.c
 create mode 100644 include/soc/sifive/sifive_ddr.h

-- 
2.7.4

[PATCH 1/3] dt-bindings: riscv: Add DT documentation for DDR Controller in SiFive SoCs

[Yash Shah]

Add device tree bindings for SiFive FU540 DDR controller driver

Signed-off-by: Yash Shah <yash.shah@sifive.com>
---
 .../devicetree/bindings/riscv/sifive-ddr.yaml      | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/riscv/sifive-ddr.yaml

diff --git a/Documentation/devicetree/bindings/riscv/sifive-ddr.yaml b/Documentation/devicetree/bindings/riscv/sifive-ddr.yaml
new file mode 100644
index 0000000..0288119
--- /dev/null
+++ b/Documentation/devicetree/bindings/riscv/sifive-ddr.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/riscv/sifive-ddr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SiFive DDR memory controller binding
+
+description: |
+  The Sifive DDR controller driver is used to manage the Cadence DDR
+  controller present in SiFive FU540-C000 SoC. Currently the driver is
+  used to manage EDAC feature of the DDR controller.
+
+maintainers:
+  - Yash Shah <yash.shah@sifive.com>
+
+properties:
+  compatible:
+    enum:
+      - sifive,fu540-c000-ddr
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    memory-controller@100b0000 {
+        compatible = "sifive,fu540-c000-ddr";
+        reg = <0x100b0000 0x4000>;
+        interrupts = <31>;
+    };
-- 
2.7.4

[PATCH 2/3] soc: sifive: Add SiFive specific Cadence DDR controller driver

[Yash Shah]

Add a driver to manage the Cadence DDR controller present on SiFive SoCs
At present the driver manages the EDAC feature of the DDR controller.
Additional features may be added to the driver in future to control
other aspects of the DDR controller.

Signed-off-by: Yash Shah <yash.shah@sifive.com>
---
 drivers/soc/sifive/Kconfig      |   6 ++
 drivers/soc/sifive/Makefile     |   3 +-
 drivers/soc/sifive/sifive_ddr.c | 207 ++++++++++++++++++++++++++++++++++++++++
 include/soc/sifive/sifive_ddr.h |  73 ++++++++++++++
 4 files changed, 288 insertions(+), 1 deletion(-)
 create mode 100644 drivers/soc/sifive/sifive_ddr.c
 create mode 100644 include/soc/sifive/sifive_ddr.h

diff --git a/drivers/soc/sifive/Kconfig b/drivers/soc/sifive/Kconfig
index 58cf8c4..f41d8fe 100644
--- a/drivers/soc/sifive/Kconfig
+++ b/drivers/soc/sifive/Kconfig
@@ -7,4 +7,10 @@ config SIFIVE_L2
 	help
 	  Support for the L2 cache controller on SiFive platforms.
 
+config SIFIVE_DDR
+	bool "Sifive DDR controller driver"
+	help
+	  Support for the management of cadence DDR controller on SiFive
+	  platforms.
+
 endif
diff --git a/drivers/soc/sifive/Makefile b/drivers/soc/sifive/Makefile
index b5caff7..b4acb5c 100644
--- a/drivers/soc/sifive/Makefile
+++ b/drivers/soc/sifive/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
-obj-$(CONFIG_SIFIVE_L2)	+= sifive_l2_cache.o
+obj-$(CONFIG_SIFIVE_L2)		+= sifive_l2_cache.o
+obj-$(CONFIG_SIFIVE_DDR)	+= sifive_ddr.o
diff --git a/drivers/soc/sifive/sifive_ddr.c b/drivers/soc/sifive/sifive_ddr.c
new file mode 100644
index 0000000..b1b421c
--- /dev/null
+++ b/drivers/soc/sifive/sifive_ddr.c
@@ -0,0 +1,207 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SiFive specific cadence DDR controller Driver
+ *
+ * Copyright (C) 2019-2020 SiFive, Inc.
+ *
+ */
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/uaccess.h>
+#include <linux/delay.h>
+#include <soc/sifive/sifive_ddr.h>
+
+static ATOMIC_NOTIFIER_HEAD(ddr_err_chain);
+
+int register_sifive_ddr_error_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_register(&ddr_err_chain, nb);
+}
+EXPORT_SYMBOL_GPL(register_sifive_ddr_error_notifier);
+
+int unregister_sifive_ddr_error_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_unregister(&ddr_err_chain, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_sifive_ddr_error_notifier);
+
+static void handle_ce(struct sifive_ddr_priv *pv)
+{
+	u64 err_c_addr = 0x0;
+	u64 err_c_data = 0x0;
+	u32 err_c_synd, err_c_id;
+	u32 sig_val_l, sig_val_h;
+
+	sig_val_l = readl(pv->reg + ECC_C_ADDR_L_REG);
+	sig_val_h = (readl(pv->reg + ECC_C_ADDR_H_REG) &
+		     ECC_ADDR_H_MASK);
+	err_c_addr = (((ulong)sig_val_h << CTL_REG_WIDTH_SHIFT) | sig_val_l);
+
+	sig_val_l = readl(pv->reg + ECC_C_DATA_L_REG);
+	sig_val_h = readl(pv->reg + ECC_C_DATA_H_REG);
+	err_c_data = (((ulong)sig_val_h << CTL_REG_WIDTH_SHIFT) | sig_val_l);
+
+	err_c_id = ((readl(pv->reg + ECC_U_C_ID_REG) &
+		     ECC_C_ID_MASK) >> ECC_C_ID_SHIFT);
+
+	err_c_synd = ((readl(pv->reg + ECC_C_SYND_REG) &
+		      ECC_SYND_MASK) >> ECC_SYND_SHIFT);
+
+	pv->error_count = 1;
+	pv->page_frame_number = err_c_addr >> PAGE_SHIFT;
+	pv->offset_in_page = err_c_addr & ~PAGE_MASK;
+	pv->syndrome = err_c_synd;
+	pv->top_layer = 0;
+	pv->mid_layer = 0;
+	pv->low_layer = -1;
+
+	atomic_notifier_call_chain(&ddr_err_chain, SIFIVE_DDR_ERR_TYPE_CE, pv);
+}
+
+static void handle_ue(struct sifive_ddr_priv *pv)
+{
+	u64 err_u_addr = 0x0;
+	u64 err_u_data = 0x0;
+	u32 err_u_synd, err_u_id;
+	u32 sig_val_l, sig_val_h;
+
+	sig_val_l = readl(pv->reg + ECC_U_ADDR_L_REG);
+	sig_val_h = (readl(pv->reg + ECC_U_ADDR_H_REG) &
+		     ECC_ADDR_H_MASK);
+	err_u_addr = (((ulong)sig_val_h << CTL_REG_WIDTH_SHIFT) | sig_val_l);
+
+	sig_val_l = readl(pv->reg + ECC_U_DATA_L_REG);
+	sig_val_h = readl(pv->reg + ECC_U_DATA_H_REG);
+	err_u_data = (((ulong)sig_val_h << CTL_REG_WIDTH_SHIFT) | sig_val_l);
+
+	err_u_id = ((readl(pv->reg + ECC_U_C_ID_REG) &
+		    ECC_U_ID_MASK) >> ECC_U_ID_SHIFT);
+
+	err_u_synd = ((readl(pv->reg + ECC_U_SYND_REG) &
+		      ECC_SYND_MASK) >> ECC_SYND_SHIFT);
+
+	pv->error_count = 1;
+	pv->page_frame_number = err_u_addr >> PAGE_SHIFT;
+	pv->offset_in_page = err_u_addr & ~PAGE_MASK;
+	pv->syndrome = err_u_synd;
+	pv->top_layer = 0;
+	pv->mid_layer = 0;
+	pv->low_layer = -1;
+
+	atomic_notifier_call_chain(&ddr_err_chain, SIFIVE_DDR_ERR_TYPE_UE, pv);
+}
+
+static irqreturn_t ecc_isr(int irq, void *ptr)
+{
+	struct sifive_ddr_priv *pv = ptr;
+	u32 intr_status;
+	u32 val;
+
+	/* Check the intr status and confirm ECC error intr */
+	intr_status = readl(pv->reg + ECC_CTL_INT_STATUS_REG);
+
+	dev_dbg(pv->dev, "InterruptStatus : 0x%x\n", intr_status);
+	val = intr_status & (ECC_INT_CE_UE_MASK);
+	if (!((val & ECC_CE_INTR_MASK) || (val & ECC_UE_INTR_MASK)))
+		return IRQ_NONE;
+
+	if (val & ECC_CE_INTR_MASK) {
+		handle_ce(pv);
+
+		/* Clear the interrupt source */
+		if (val & ECC_INT_CE_EVENT)
+			writel(ECC_INT_CE_EVENT,
+			       pv->reg + ECC_CTL_INT_ACK_REG);
+		else if (val & ECC_INT_SECOND_CE_EVENT)
+			writel(ECC_INT_SECOND_CE_EVENT,
+			       pv->reg + ECC_CTL_INT_ACK_REG);
+		else
+			dev_err(pv->dev, "Failed to clear IRQ\n");
+	}
+
+	if (val & ECC_UE_INTR_MASK) {
+		handle_ue(pv);
+
+		/* Clear the interrupt source */
+		if (val & ECC_INT_UE_EVENT)
+			writel(ECC_INT_UE_EVENT,
+			       pv->reg + ECC_CTL_INT_ACK_REG);
+		else if (val & ECC_INT_SECOND_UE_EVENT)
+			writel(ECC_INT_SECOND_UE_EVENT,
+			       pv->reg + ECC_CTL_INT_ACK_REG);
+		else
+			dev_err(pv->dev, "Failed to clear IRQ\n");
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int sifive_ddr_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct sifive_ddr_priv *priv;
+	struct resource *res;
+	int ret, irq;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->reg = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->reg))
+		return PTR_ERR(priv->reg);
+
+	irq = platform_get_irq(pdev, 0);
+	ret = devm_request_irq(dev, irq, ecc_isr, 0, "sifive-fu540-ddr", priv);
+	if (ret) {
+		dev_err(dev, "request_irq failed\n");
+		return ret;
+	}
+
+	/* Enable & set CE/UE Interrupts for DDR4 Controller */
+	writel(~(ECC_INT_CE_UE_MASK), priv->reg + ECC_CTL_INT_MASK_REG);
+
+	platform_set_drvdata(pdev, priv);
+	dev_info(dev, "SiFive DDR probe successful\n");
+
+	return 0;
+}
+
+static int sifive_ddr_remove(struct platform_device *pdev)
+{
+	struct sifive_ddr_priv *priv = platform_get_drvdata(pdev);
+
+	/* Disable All ECC Interrupts for DDR4 Controller */
+	writel(ECC_INT_CE_UE_MASK, priv->reg + ECC_CTL_INT_MASK_REG);
+
+	return 0;
+}
+
+static const struct of_device_id sifive_ddr_of_match[] = {
+	{ .compatible = "sifive,fu540-c000-ddr"},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, sifive_ddr_of_match);
+
+static struct platform_driver sifive_ddr_driver = {
+	.driver = {
+		.name = "sifive-ddr",
+		.of_match_table = sifive_ddr_of_match,
+	},
+	.probe = sifive_ddr_probe,
+	.remove = sifive_ddr_remove,
+};
+
+module_platform_driver(sifive_ddr_driver);
+
+MODULE_AUTHOR("SiFive");
+MODULE_DESCRIPTION("SiFive DDR Controller driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/soc/sifive/sifive_ddr.h b/include/soc/sifive/sifive_ddr.h
new file mode 100644
index 0000000..2ff8623
--- /dev/null
+++ b/include/soc/sifive/sifive_ddr.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * SiFive DDR Controller header file
+ *
+ */
+
+#ifndef __SOC_SIFIVE_DDR_H
+#define __SOC_SIFIVE_DDR_H
+
+int register_sifive_ddr_error_notifier(struct notifier_block *nb);
+int unregister_sifive_ddr_error_notifier(struct notifier_block *nb);
+
+struct sifive_ddr_priv {
+	void __iomem *reg;
+	struct device *dev;
+	u16 error_count;
+	unsigned long page_frame_number;
+	unsigned long offset_in_page;
+	unsigned long syndrome;
+	int top_layer;
+	int mid_layer;
+	int low_layer;
+};
+
+#define SIFIVE_DDR_ERR_TYPE_CE	(0)
+#define SIFIVE_DDR_ERR_TYPE_UE	(1)
+#define SIFIVE_DDR_EDAC_GRAIN	(1)
+#define SIFIVE_MEM_TYPE_DDR4	0xA
+#define SIFIVE_DDR_WIDTH_16	(2)
+#define SIFIVE_DDR_WIDTH_32	(1)
+#define SIFIVE_DDR_WIDTH_64	(0)
+
+#define DDR_CTL_MEM_TYPE_REG	0x000
+#define DDR_CTL_MEM_WIDTH_REG	0x004
+#define ECC_CTL_CONTROL_REG	0x174
+#define ECC_U_ADDR_L_REG	0x180
+#define ECC_U_ADDR_H_REG	0x184
+#define ECC_U_DATA_L_REG	0x188
+#define ECC_U_DATA_H_REG	0x18c
+
+#define ECC_C_ADDR_L_REG	0x190
+#define ECC_C_ADDR_H_REG	0x194
+#define ECC_C_DATA_L_REG	0x198
+#define ECC_C_DATA_H_REG	0x19c
+#define ECC_U_C_ID_REG		0x1A0
+#define ECC_CTL_INT_STATUS_REG	0x210
+#define ECC_CTL_INT_ACK_REG	0x218
+#define ECC_CTL_INT_MASK_REG	0x220
+#define ECC_C_SYND_REG		ECC_C_ADDR_H_REG
+#define ECC_U_SYND_REG		ECC_U_ADDR_H_REG
+
+#define ECC_CTL_MTYPE_MASK	GENMASK(11, 8)
+#define CTL_MEM_MAX_WIDTH_MASK	GENMASK(31, 24)
+#define ECC_ADDR_H_MASK		GENMASK(3, 0)
+#define ECC_INT_CE_UE_MASK	GENMASK(6, 3)
+#define ECC_CE_INTR_MASK	GENMASK(4, 3)
+#define ECC_UE_INTR_MASK	GENMASK(6, 5)
+#define ECC_INT_CE_EVENT	BIT(3)
+#define ECC_INT_SECOND_CE_EVENT	BIT(4)
+#define ECC_INT_UE_EVENT	BIT(5)
+#define ECC_INT_SECOND_UE_EVENT	BIT(6)
+#define ECC_CTL_ECC_ENABLE	BIT(16)
+
+#define ECC_C_ID_MASK		GENMASK(28, 16)
+#define ECC_U_ID_MASK		GENMASK(12, 0)
+#define ECC_C_ID_SHIFT		(16)
+#define ECC_U_ID_SHIFT		(0)
+#define ECC_SYND_MASK		GENMASK(15, 8)
+#define ECC_SYND_SHIFT		(8)
+
+#define CTL_REG_WIDTH_SHIFT	(32)
+
+#endif /* __SOC_SIFIVE_DDR_H */
-- 
2.7.4

[PATCH 3/3] edac: sifive: Add EDAC support for Memory Controller in SiFive SoCs

[Yash Shah]

Add Memory controller EDAC support in exisiting SiFive platform EDAC
driver. It registers for notifier events from the SiFive DDR controller
driver for DDR ECC events.

Signed-off-by: Yash Shah <yash.shah@sifive.com>
---
 drivers/edac/Kconfig       |   2 +-
 drivers/edac/sifive_edac.c | 117 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 118 insertions(+), 1 deletion(-)

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 7b6ec30..f8b3b53 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -462,7 +462,7 @@ config EDAC_ALTERA_SDMMC
 
 config EDAC_SIFIVE
 	bool "Sifive platform EDAC driver"
-	depends on EDAC=y && SIFIVE_L2
+	depends on EDAC=y && (SIFIVE_L2 || SIFIVE_DDR)
 	help
 	  Support for error detection and correction on the SiFive SoCs.
 
diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c
index 3a3dcb1..cf032685 100644
--- a/drivers/edac/sifive_edac.c
+++ b/drivers/edac/sifive_edac.c
@@ -11,14 +11,120 @@
 #include <linux/platform_device.h>
 #include "edac_module.h"
 #include <soc/sifive/sifive_l2_cache.h>
+#include <soc/sifive/sifive_ddr.h>
 
 #define DRVNAME "sifive_edac"
+#define SIFIVE_EDAC_MOD_NAME "Sifive ECC Manager"
 
 struct sifive_edac_priv {
 	struct notifier_block notifier;
 	struct edac_device_ctl_info *dci;
 };
 
+struct sifive_edac_mc_priv {
+	struct notifier_block notifier;
+	struct mem_ctl_info *mci;
+};
+
+/**
+ * EDAC MC error callback
+ *
+ * @event: non-zero if unrecoverable.
+ */
+static
+int ecc_mc_err_event(struct notifier_block *this, unsigned long event, void *ptr)
+{
+	struct sifive_ddr_priv *priv = ptr;
+	struct sifive_edac_mc_priv *p;
+
+	p = container_of(this, struct sifive_edac_mc_priv, notifier);
+	if (event == SIFIVE_DDR_ERR_TYPE_UE) {
+		edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, p->mci,
+				     priv->error_count, priv->page_frame_number,
+				     priv->offset_in_page, priv->syndrome,
+				     priv->top_layer, priv->mid_layer,
+				     priv->low_layer, p->mci->ctl_name, "");
+	} else if (event == SIFIVE_DDR_ERR_TYPE_CE) {
+		edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, p->mci,
+				     priv->error_count, priv->page_frame_number,
+				     priv->offset_in_page, priv->syndrome,
+				     priv->top_layer, priv->mid_layer,
+				     priv->low_layer, p->mci->ctl_name, "");
+	}
+
+	return NOTIFY_OK;
+}
+
+static int ecc_mc_register(struct platform_device *pdev)
+{
+	struct sifive_edac_mc_priv *p;
+	struct edac_mc_layer layers[1];
+	int ret;
+
+	p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+
+	p->notifier.notifier_call = ecc_mc_err_event;
+	platform_set_drvdata(pdev, p);
+
+	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
+	layers[0].size = 1;
+	layers[0].is_virt_csrow = true;
+
+	p->mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, 0);
+	if (!p->mci) {
+		dev_err(&pdev->dev, "Failed mem allocation for mc instance\n");
+		return -ENOMEM;
+	}
+
+	p->mci->pdev = &pdev->dev;
+	/* Initialize controller capabilities */
+	p->mci->mtype_cap = MEM_FLAG_DDR4;
+	p->mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
+	p->mci->edac_cap = EDAC_FLAG_SECDED;
+	p->mci->scrub_cap = SCRUB_UNKNOWN;
+	p->mci->scrub_mode = SCRUB_HW_PROG;
+	p->mci->ctl_name = dev_name(&pdev->dev);
+	p->mci->dev_name = dev_name(&pdev->dev);
+	p->mci->mod_name = SIFIVE_EDAC_MOD_NAME;
+	p->mci->ctl_page_to_phys = NULL;
+
+	/* Interrupt feature is supported by cadence mc */
+	edac_op_state = EDAC_OPSTATE_INT;
+
+	ret = edac_mc_add_mc(p->mci);
+	if (ret) {
+		edac_printk(KERN_ERR, SIFIVE_EDAC_MOD_NAME,
+			    "Failed to register with EDAC core\n");
+		goto err;
+	}
+
+#ifdef CONFIG_SIFIVE_DDR
+	register_sifive_ddr_error_notifier(&p->notifier);
+#endif
+
+	return 0;
+
+err:
+	edac_mc_free(p->mci);
+
+	return -ENXIO;
+}
+
+static int ecc_mc_unregister(struct platform_device *pdev)
+{
+	struct sifive_edac_mc_priv *p = platform_get_drvdata(pdev);
+
+#ifdef CONFIG_SIFIVE_DDR
+	unregister_sifive_ddr_error_notifier(&p->notifier);
+#endif
+	edac_mc_del_mc(&pdev->dev);
+	edac_mc_free(p->mci);
+
+	return 0;
+}
+
 /**
  * EDAC error callback
  *
@@ -67,7 +173,9 @@ static int ecc_register(struct platform_device *pdev)
 		goto err;
 	}
 
+#ifdef CONFIG_SIFIVE_L2
 	register_sifive_l2_error_notifier(&p->notifier);
+#endif
 
 	return 0;
 
@@ -81,7 +189,9 @@ static int ecc_unregister(struct platform_device *pdev)
 {
 	struct sifive_edac_priv *p = platform_get_drvdata(pdev);
 
+#ifdef CONFIG_SIFIVE_L2
 	unregister_sifive_l2_error_notifier(&p->notifier);
+#endif
 	edac_device_del_device(&pdev->dev);
 	edac_device_free_ctl_info(p->dci);
 
@@ -102,12 +212,19 @@ static int __init sifive_edac_init(void)
 	if (ret)
 		platform_device_unregister(sifive_pdev);
 
+	ret = ecc_mc_register(sifive_pdev);
+	if (ret) {
+		ecc_unregister(sifive_pdev);
+		platform_device_unregister(sifive_pdev);
+	}
+
 	return ret;
 }
 
 static void __exit sifive_edac_exit(void)
 {
 	ecc_unregister(sifive_pdev);
+	ecc_mc_unregister(sifive_pdev);
 	platform_device_unregister(sifive_pdev);
 }
 
-- 
2.7.4

Re: [PATCH 2/3] soc: sifive: Add SiFive specific Cadence DDR controller driver

[Palmer Dabbelt]

On Tue, 25 Aug 2020 05:06:21 PDT (-0700), yash.shah@sifive.com wrote:
> Add a driver to manage the Cadence DDR controller present on SiFive SoCs
> At present the driver manages the EDAC feature of the DDR controller.
> Additional features may be added to the driver in future to control
> other aspects of the DDR controller.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> ---
>  drivers/soc/sifive/Kconfig      |   6 ++
>  drivers/soc/sifive/Makefile     |   3 +-
>  drivers/soc/sifive/sifive_ddr.c | 207 ++++++++++++++++++++++++++++++++++++++++
>  include/soc/sifive/sifive_ddr.h |  73 ++++++++++++++
>  4 files changed, 288 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/soc/sifive/sifive_ddr.c
>  create mode 100644 include/soc/sifive/sifive_ddr.h
>
> diff --git a/drivers/soc/sifive/Kconfig b/drivers/soc/sifive/Kconfig
> index 58cf8c4..f41d8fe 100644
> --- a/drivers/soc/sifive/Kconfig
> +++ b/drivers/soc/sifive/Kconfig
> @@ -7,4 +7,10 @@ config SIFIVE_L2
>  	help
>  	  Support for the L2 cache controller on SiFive platforms.
>
> +config SIFIVE_DDR
> +	bool "Sifive DDR controller driver"
> +	help
> +	  Support for the management of cadence DDR controller on SiFive
> +	  platforms.
> +
>  endif
> diff --git a/drivers/soc/sifive/Makefile b/drivers/soc/sifive/Makefile
> index b5caff7..b4acb5c 100644
> --- a/drivers/soc/sifive/Makefile
> +++ b/drivers/soc/sifive/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>
> -obj-$(CONFIG_SIFIVE_L2)	+= sifive_l2_cache.o
> +obj-$(CONFIG_SIFIVE_L2)		+= sifive_l2_cache.o
> +obj-$(CONFIG_SIFIVE_DDR)	+= sifive_ddr.o
> diff --git a/drivers/soc/sifive/sifive_ddr.c b/drivers/soc/sifive/sifive_ddr.c
> new file mode 100644
> index 0000000..b1b421c
> --- /dev/null
> +++ b/drivers/soc/sifive/sifive_ddr.c
> @@ -0,0 +1,207 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SiFive specific cadence DDR controller Driver
> + *
> + * Copyright (C) 2019-2020 SiFive, Inc.
> + *
> + */
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +#include <linux/uaccess.h>
> +#include <linux/delay.h>
> +#include <soc/sifive/sifive_ddr.h>
> +
> +static ATOMIC_NOTIFIER_HEAD(ddr_err_chain);
> +
> +int register_sifive_ddr_error_notifier(struct notifier_block *nb)
> +{
> +	return atomic_notifier_chain_register(&ddr_err_chain, nb);
> +}
> +EXPORT_SYMBOL_GPL(register_sifive_ddr_error_notifier);
> +
> +int unregister_sifive_ddr_error_notifier(struct notifier_block *nb)
> +{
> +	return atomic_notifier_chain_unregister(&ddr_err_chain, nb);
> +}
> +EXPORT_SYMBOL_GPL(unregister_sifive_ddr_error_notifier);
> +
> +static void handle_ce(struct sifive_ddr_priv *pv)
> +{
> +	u64 err_c_addr = 0x0;
> +	u64 err_c_data = 0x0;
> +	u32 err_c_synd, err_c_id;
> +	u32 sig_val_l, sig_val_h;
> +
> +	sig_val_l = readl(pv->reg + ECC_C_ADDR_L_REG);
> +	sig_val_h = (readl(pv->reg + ECC_C_ADDR_H_REG) &
> +		     ECC_ADDR_H_MASK);
> +	err_c_addr = (((ulong)sig_val_h << CTL_REG_WIDTH_SHIFT) | sig_val_l);
> +
> +	sig_val_l = readl(pv->reg + ECC_C_DATA_L_REG);
> +	sig_val_h = readl(pv->reg + ECC_C_DATA_H_REG);
> +	err_c_data = (((ulong)sig_val_h << CTL_REG_WIDTH_SHIFT) | sig_val_l);
> +
> +	err_c_id = ((readl(pv->reg + ECC_U_C_ID_REG) &
> +		     ECC_C_ID_MASK) >> ECC_C_ID_SHIFT);
> +
> +	err_c_synd = ((readl(pv->reg + ECC_C_SYND_REG) &
> +		      ECC_SYND_MASK) >> ECC_SYND_SHIFT);
> +
> +	pv->error_count = 1;
> +	pv->page_frame_number = err_c_addr >> PAGE_SHIFT;
> +	pv->offset_in_page = err_c_addr & ~PAGE_MASK;
> +	pv->syndrome = err_c_synd;
> +	pv->top_layer = 0;
> +	pv->mid_layer = 0;
> +	pv->low_layer = -1;
> +
> +	atomic_notifier_call_chain(&ddr_err_chain, SIFIVE_DDR_ERR_TYPE_CE, pv);
> +}
> +
> +static void handle_ue(struct sifive_ddr_priv *pv)
> +{
> +	u64 err_u_addr = 0x0;
> +	u64 err_u_data = 0x0;
> +	u32 err_u_synd, err_u_id;
> +	u32 sig_val_l, sig_val_h;
> +
> +	sig_val_l = readl(pv->reg + ECC_U_ADDR_L_REG);
> +	sig_val_h = (readl(pv->reg + ECC_U_ADDR_H_REG) &
> +		     ECC_ADDR_H_MASK);
> +	err_u_addr = (((ulong)sig_val_h << CTL_REG_WIDTH_SHIFT) | sig_val_l);
> +
> +	sig_val_l = readl(pv->reg + ECC_U_DATA_L_REG);
> +	sig_val_h = readl(pv->reg + ECC_U_DATA_H_REG);
> +	err_u_data = (((ulong)sig_val_h << CTL_REG_WIDTH_SHIFT) | sig_val_l);
> +
> +	err_u_id = ((readl(pv->reg + ECC_U_C_ID_REG) &
> +		    ECC_U_ID_MASK) >> ECC_U_ID_SHIFT);
> +
> +	err_u_synd = ((readl(pv->reg + ECC_U_SYND_REG) &
> +		      ECC_SYND_MASK) >> ECC_SYND_SHIFT);
> +
> +	pv->error_count = 1;
> +	pv->page_frame_number = err_u_addr >> PAGE_SHIFT;
> +	pv->offset_in_page = err_u_addr & ~PAGE_MASK;
> +	pv->syndrome = err_u_synd;
> +	pv->top_layer = 0;
> +	pv->mid_layer = 0;
> +	pv->low_layer = -1;
> +
> +	atomic_notifier_call_chain(&ddr_err_chain, SIFIVE_DDR_ERR_TYPE_UE, pv);
> +}
> +
> +static irqreturn_t ecc_isr(int irq, void *ptr)
> +{
> +	struct sifive_ddr_priv *pv = ptr;
> +	u32 intr_status;
> +	u32 val;
> +
> +	/* Check the intr status and confirm ECC error intr */
> +	intr_status = readl(pv->reg + ECC_CTL_INT_STATUS_REG);
> +
> +	dev_dbg(pv->dev, "InterruptStatus : 0x%x\n", intr_status);
> +	val = intr_status & (ECC_INT_CE_UE_MASK);
> +	if (!((val & ECC_CE_INTR_MASK) || (val & ECC_UE_INTR_MASK)))
> +		return IRQ_NONE;
> +
> +	if (val & ECC_CE_INTR_MASK) {
> +		handle_ce(pv);
> +
> +		/* Clear the interrupt source */
> +		if (val & ECC_INT_CE_EVENT)
> +			writel(ECC_INT_CE_EVENT,
> +			       pv->reg + ECC_CTL_INT_ACK_REG);
> +		else if (val & ECC_INT_SECOND_CE_EVENT)
> +			writel(ECC_INT_SECOND_CE_EVENT,
> +			       pv->reg + ECC_CTL_INT_ACK_REG);
> +		else
> +			dev_err(pv->dev, "Failed to clear IRQ\n");
> +	}
> +
> +	if (val & ECC_UE_INTR_MASK) {
> +		handle_ue(pv);
> +
> +		/* Clear the interrupt source */
> +		if (val & ECC_INT_UE_EVENT)
> +			writel(ECC_INT_UE_EVENT,
> +			       pv->reg + ECC_CTL_INT_ACK_REG);
> +		else if (val & ECC_INT_SECOND_UE_EVENT)
> +			writel(ECC_INT_SECOND_UE_EVENT,
> +			       pv->reg + ECC_CTL_INT_ACK_REG);
> +		else
> +			dev_err(pv->dev, "Failed to clear IRQ\n");
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int sifive_ddr_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct sifive_ddr_priv *priv;
> +	struct resource *res;
> +	int ret, irq;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->dev = dev;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->reg = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(priv->reg))
> +		return PTR_ERR(priv->reg);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	ret = devm_request_irq(dev, irq, ecc_isr, 0, "sifive-fu540-ddr", priv);
> +	if (ret) {
> +		dev_err(dev, "request_irq failed\n");
> +		return ret;
> +	}
> +
> +	/* Enable & set CE/UE Interrupts for DDR4 Controller */
> +	writel(~(ECC_INT_CE_UE_MASK), priv->reg + ECC_CTL_INT_MASK_REG);
> +
> +	platform_set_drvdata(pdev, priv);
> +	dev_info(dev, "SiFive DDR probe successful\n");

I'm not sure what the rules are here, but this doesn't seem like a particularly
useful print.

> +
> +	return 0;
> +}
> +
> +static int sifive_ddr_remove(struct platform_device *pdev)
> +{
> +	struct sifive_ddr_priv *priv = platform_get_drvdata(pdev);
> +
> +	/* Disable All ECC Interrupts for DDR4 Controller */
> +	writel(ECC_INT_CE_UE_MASK, priv->reg + ECC_CTL_INT_MASK_REG);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id sifive_ddr_of_match[] = {
> +	{ .compatible = "sifive,fu540-c000-ddr"},
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, sifive_ddr_of_match);
> +
> +static struct platform_driver sifive_ddr_driver = {
> +	.driver = {
> +		.name = "sifive-ddr",
> +		.of_match_table = sifive_ddr_of_match,
> +	},
> +	.probe = sifive_ddr_probe,
> +	.remove = sifive_ddr_remove,
> +};
> +
> +module_platform_driver(sifive_ddr_driver);
> +
> +MODULE_AUTHOR("SiFive");
> +MODULE_DESCRIPTION("SiFive DDR Controller driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/soc/sifive/sifive_ddr.h b/include/soc/sifive/sifive_ddr.h
> new file mode 100644
> index 0000000..2ff8623
> --- /dev/null
> +++ b/include/soc/sifive/sifive_ddr.h
> @@ -0,0 +1,73 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * SiFive DDR Controller header file
> + *
> + */
> +
> +#ifndef __SOC_SIFIVE_DDR_H
> +#define __SOC_SIFIVE_DDR_H
> +
> +int register_sifive_ddr_error_notifier(struct notifier_block *nb);
> +int unregister_sifive_ddr_error_notifier(struct notifier_block *nb);
> +
> +struct sifive_ddr_priv {
> +	void __iomem *reg;
> +	struct device *dev;
> +	u16 error_count;
> +	unsigned long page_frame_number;
> +	unsigned long offset_in_page;
> +	unsigned long syndrome;
> +	int top_layer;
> +	int mid_layer;
> +	int low_layer;
> +};
> +
> +#define SIFIVE_DDR_ERR_TYPE_CE	(0)
> +#define SIFIVE_DDR_ERR_TYPE_UE	(1)
> +#define SIFIVE_DDR_EDAC_GRAIN	(1)
> +#define SIFIVE_MEM_TYPE_DDR4	0xA
> +#define SIFIVE_DDR_WIDTH_16	(2)
> +#define SIFIVE_DDR_WIDTH_32	(1)
> +#define SIFIVE_DDR_WIDTH_64	(0)
> +
> +#define DDR_CTL_MEM_TYPE_REG	0x000
> +#define DDR_CTL_MEM_WIDTH_REG	0x004
> +#define ECC_CTL_CONTROL_REG	0x174
> +#define ECC_U_ADDR_L_REG	0x180
> +#define ECC_U_ADDR_H_REG	0x184
> +#define ECC_U_DATA_L_REG	0x188
> +#define ECC_U_DATA_H_REG	0x18c
> +
> +#define ECC_C_ADDR_L_REG	0x190
> +#define ECC_C_ADDR_H_REG	0x194
> +#define ECC_C_DATA_L_REG	0x198
> +#define ECC_C_DATA_H_REG	0x19c
> +#define ECC_U_C_ID_REG		0x1A0
> +#define ECC_CTL_INT_STATUS_REG	0x210
> +#define ECC_CTL_INT_ACK_REG	0x218
> +#define ECC_CTL_INT_MASK_REG	0x220
> +#define ECC_C_SYND_REG		ECC_C_ADDR_H_REG
> +#define ECC_U_SYND_REG		ECC_U_ADDR_H_REG
> +
> +#define ECC_CTL_MTYPE_MASK	GENMASK(11, 8)
> +#define CTL_MEM_MAX_WIDTH_MASK	GENMASK(31, 24)
> +#define ECC_ADDR_H_MASK		GENMASK(3, 0)
> +#define ECC_INT_CE_UE_MASK	GENMASK(6, 3)
> +#define ECC_CE_INTR_MASK	GENMASK(4, 3)
> +#define ECC_UE_INTR_MASK	GENMASK(6, 5)
> +#define ECC_INT_CE_EVENT	BIT(3)
> +#define ECC_INT_SECOND_CE_EVENT	BIT(4)
> +#define ECC_INT_UE_EVENT	BIT(5)
> +#define ECC_INT_SECOND_UE_EVENT	BIT(6)
> +#define ECC_CTL_ECC_ENABLE	BIT(16)
> +
> +#define ECC_C_ID_MASK		GENMASK(28, 16)
> +#define ECC_U_ID_MASK		GENMASK(12, 0)
> +#define ECC_C_ID_SHIFT		(16)
> +#define ECC_U_ID_SHIFT		(0)
> +#define ECC_SYND_MASK		GENMASK(15, 8)
> +#define ECC_SYND_SHIFT		(8)
> +
> +#define CTL_REG_WIDTH_SHIFT	(32)
> +
> +#endif /* __SOC_SIFIVE_DDR_H */

Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>

Re: [PATCH 1/3] dt-bindings: riscv: Add DT documentation for DDR Controller in SiFive SoCs

[Palmer Dabbelt]

On Tue, 25 Aug 2020 05:06:20 PDT (-0700), yash.shah@sifive.com wrote:
> Add device tree bindings for SiFive FU540 DDR controller driver
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> ---
>  .../devicetree/bindings/riscv/sifive-ddr.yaml      | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/riscv/sifive-ddr.yaml
>
> diff --git a/Documentation/devicetree/bindings/riscv/sifive-ddr.yaml b/Documentation/devicetree/bindings/riscv/sifive-ddr.yaml
> new file mode 100644
> index 0000000..0288119
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/riscv/sifive-ddr.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/riscv/sifive-ddr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SiFive DDR memory controller binding
> +
> +description: |
> +  The Sifive DDR controller driver is used to manage the Cadence DDR
> +  controller present in SiFive FU540-C000 SoC. Currently the driver is
> +  used to manage EDAC feature of the DDR controller.
> +
> +maintainers:
> +  - Yash Shah <yash.shah@sifive.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - sifive,fu540-c000-ddr
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    memory-controller@100b0000 {
> +        compatible = "sifive,fu540-c000-ddr";
> +        reg = <0x100b0000 0x4000>;
> +        interrupts = <31>;
> +    };

Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>

Re: [PATCH 3/3] edac: sifive: Add EDAC support for Memory Controller in SiFive SoCs

[Palmer Dabbelt]

On Tue, 25 Aug 2020 05:06:22 PDT (-0700), yash.shah@sifive.com wrote:
> Add Memory controller EDAC support in exisiting SiFive platform EDAC
> driver. It registers for notifier events from the SiFive DDR controller
> driver for DDR ECC events.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> ---
>  drivers/edac/Kconfig       |   2 +-
>  drivers/edac/sifive_edac.c | 117 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 118 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 7b6ec30..f8b3b53 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -462,7 +462,7 @@ config EDAC_ALTERA_SDMMC
>
>  config EDAC_SIFIVE
>  	bool "Sifive platform EDAC driver"
> -	depends on EDAC=y && SIFIVE_L2
> +	depends on EDAC=y && (SIFIVE_L2 || SIFIVE_DDR)
>  	help
>  	  Support for error detection and correction on the SiFive SoCs.
>
> diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c
> index 3a3dcb1..cf032685 100644
> --- a/drivers/edac/sifive_edac.c
> +++ b/drivers/edac/sifive_edac.c
> @@ -11,14 +11,120 @@
>  #include <linux/platform_device.h>
>  #include "edac_module.h"
>  #include <soc/sifive/sifive_l2_cache.h>
> +#include <soc/sifive/sifive_ddr.h>
>
>  #define DRVNAME "sifive_edac"
> +#define SIFIVE_EDAC_MOD_NAME "Sifive ECC Manager"
>
>  struct sifive_edac_priv {
>  	struct notifier_block notifier;
>  	struct edac_device_ctl_info *dci;
>  };
>
> +struct sifive_edac_mc_priv {
> +	struct notifier_block notifier;
> +	struct mem_ctl_info *mci;
> +};
> +
> +/**
> + * EDAC MC error callback
> + *
> + * @event: non-zero if unrecoverable.
> + */
> +static
> +int ecc_mc_err_event(struct notifier_block *this, unsigned long event, void *ptr)
> +{
> +	struct sifive_ddr_priv *priv = ptr;
> +	struct sifive_edac_mc_priv *p;
> +
> +	p = container_of(this, struct sifive_edac_mc_priv, notifier);
> +	if (event == SIFIVE_DDR_ERR_TYPE_UE) {
> +		edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, p->mci,
> +				     priv->error_count, priv->page_frame_number,
> +				     priv->offset_in_page, priv->syndrome,
> +				     priv->top_layer, priv->mid_layer,
> +				     priv->low_layer, p->mci->ctl_name, "");
> +	} else if (event == SIFIVE_DDR_ERR_TYPE_CE) {
> +		edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, p->mci,
> +				     priv->error_count, priv->page_frame_number,
> +				     priv->offset_in_page, priv->syndrome,
> +				     priv->top_layer, priv->mid_layer,
> +				     priv->low_layer, p->mci->ctl_name, "");
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static int ecc_mc_register(struct platform_device *pdev)
> +{
> +	struct sifive_edac_mc_priv *p;
> +	struct edac_mc_layer layers[1];
> +	int ret;
> +
> +	p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
> +	if (!p)
> +		return -ENOMEM;
> +
> +	p->notifier.notifier_call = ecc_mc_err_event;
> +	platform_set_drvdata(pdev, p);
> +
> +	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
> +	layers[0].size = 1;
> +	layers[0].is_virt_csrow = true;
> +
> +	p->mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, 0);
> +	if (!p->mci) {
> +		dev_err(&pdev->dev, "Failed mem allocation for mc instance\n");
> +		return -ENOMEM;
> +	}
> +
> +	p->mci->pdev = &pdev->dev;
> +	/* Initialize controller capabilities */
> +	p->mci->mtype_cap = MEM_FLAG_DDR4;
> +	p->mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
> +	p->mci->edac_cap = EDAC_FLAG_SECDED;
> +	p->mci->scrub_cap = SCRUB_UNKNOWN;
> +	p->mci->scrub_mode = SCRUB_HW_PROG;
> +	p->mci->ctl_name = dev_name(&pdev->dev);
> +	p->mci->dev_name = dev_name(&pdev->dev);
> +	p->mci->mod_name = SIFIVE_EDAC_MOD_NAME;
> +	p->mci->ctl_page_to_phys = NULL;
> +
> +	/* Interrupt feature is supported by cadence mc */
> +	edac_op_state = EDAC_OPSTATE_INT;
> +
> +	ret = edac_mc_add_mc(p->mci);
> +	if (ret) {
> +		edac_printk(KERN_ERR, SIFIVE_EDAC_MOD_NAME,
> +			    "Failed to register with EDAC core\n");
> +		goto err;
> +	}
> +
> +#ifdef CONFIG_SIFIVE_DDR
> +	register_sifive_ddr_error_notifier(&p->notifier);
> +#endif
> +
> +	return 0;
> +
> +err:
> +	edac_mc_free(p->mci);
> +
> +	return -ENXIO;
> +}
> +
> +static int ecc_mc_unregister(struct platform_device *pdev)
> +{
> +	struct sifive_edac_mc_priv *p = platform_get_drvdata(pdev);
> +
> +#ifdef CONFIG_SIFIVE_DDR
> +	unregister_sifive_ddr_error_notifier(&p->notifier);
> +#endif
> +	edac_mc_del_mc(&pdev->dev);
> +	edac_mc_free(p->mci);
> +
> +	return 0;
> +}
> +
>  /**
>   * EDAC error callback
>   *
> @@ -67,7 +173,9 @@ static int ecc_register(struct platform_device *pdev)
>  		goto err;
>  	}
>
> +#ifdef CONFIG_SIFIVE_L2
>  	register_sifive_l2_error_notifier(&p->notifier);
> +#endif
>
>  	return 0;
>
> @@ -81,7 +189,9 @@ static int ecc_unregister(struct platform_device *pdev)
>  {
>  	struct sifive_edac_priv *p = platform_get_drvdata(pdev);
>
> +#ifdef CONFIG_SIFIVE_L2
>  	unregister_sifive_l2_error_notifier(&p->notifier);
> +#endif
>  	edac_device_del_device(&pdev->dev);
>  	edac_device_free_ctl_info(p->dci);
>
> @@ -102,12 +212,19 @@ static int __init sifive_edac_init(void)
>  	if (ret)
>  		platform_device_unregister(sifive_pdev);
>
> +	ret = ecc_mc_register(sifive_pdev);
> +	if (ret) {
> +		ecc_unregister(sifive_pdev);
> +		platform_device_unregister(sifive_pdev);
> +	}
> +
>  	return ret;
>  }
>
>  static void __exit sifive_edac_exit(void)
>  {
>  	ecc_unregister(sifive_pdev);
> +	ecc_mc_unregister(sifive_pdev);
>  	platform_device_unregister(sifive_pdev);
>  }

Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>

Re: [PATCH 0/3] SiFive DDR controller and EDAC support

[Palmer Dabbelt]

On Tue, 25 Aug 2020 05:06:19 PDT (-0700), yash.shah@sifive.com wrote:
> The series add supports for SiFive DDR controller driver. This driver
> is use to manage the Cadence DDR controller present in SiFive SoCs.
> Currently it manages only the EDAC feature of the DDR controller.
> The series also adds Memory controller EDAC support for SiFive platform.
> It register for notifier event from SiFive DDR controller driver.
>
> The series is tested and based on Linux v5.8.
>
> For testing on Hifive Unleashed:
> 1. Enable the ECC bit of DDR controller during DDR initialization
> 2. Erase the entire DRAM in bootloader stage
> 3. Using FWC feature of DDR controller force ecc error to test
>
> Yash Shah (3):
>   dt-bindings: riscv: Add DT documentation for DDR Controller in SiFive
>     SoCs
>   soc: sifive: Add SiFive specific Cadence DDR controller driver
>   edac: sifive: Add EDAC support for Memory Controller in SiFive SoCs
>
>  .../devicetree/bindings/riscv/sifive-ddr.yaml      |  41 ++++
>  drivers/edac/Kconfig                               |   2 +-
>  drivers/edac/sifive_edac.c                         | 117 ++++++++++++
>  drivers/soc/sifive/Kconfig                         |   6 +
>  drivers/soc/sifive/Makefile                        |   3 +-
>  drivers/soc/sifive/sifive_ddr.c                    | 207 +++++++++++++++++++++
>  include/soc/sifive/sifive_ddr.h                    |  73 ++++++++
>  7 files changed, 447 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/riscv/sifive-ddr.yaml
>  create mode 100644 drivers/soc/sifive/sifive_ddr.c
>  create mode 100644 include/soc/sifive/sifive_ddr.h

Thanks.  These look good to me and I'm happy to take them through the RISC-V
tree, but I'm going to wait for a bit to see if there are any comments from the
maintainers of the various subsystems before doing so.

Re: [PATCH 0/3] SiFive DDR controller and EDAC support

[Borislav Petkov]

On Tue, Aug 25, 2020 at 09:02:54AM -0700, Palmer Dabbelt wrote:
> Thanks.  These look good to me and I'm happy to take them through the RISC-V
> tree, but I'm going to wait for a bit to see if there are any comments from the
> maintainers of the various subsystems before doing so.

I'll have a look at the EDAC bits these days and give you an ACK if
they're ok.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 0/3] SiFive DDR controller and EDAC support

[Palmer Dabbelt]

On Tue, 25 Aug 2020 09:19:58 PDT (-0700), bp@alien8.de wrote:
> On Tue, Aug 25, 2020 at 09:02:54AM -0700, Palmer Dabbelt wrote:
>> Thanks.  These look good to me and I'm happy to take them through the RISC-V
>> tree, but I'm going to wait for a bit to see if there are any comments from the
>> maintainers of the various subsystems before doing so.
>
> I'll have a look at the EDAC bits these days and give you an ACK if
> they're ok.

Thanks!

Re: [PATCH 3/3] edac: sifive: Add EDAC support for Memory Controller in SiFive SoCs

[Borislav Petkov]

> Subject: Re: [PATCH 3/3] edac: sifive: Add EDAC support for Memory Controller in SiFive SoCs

Fix subject prefix: "EDAC/sifive: ..."

On Tue, Aug 25, 2020 at 05:36:22PM +0530, Yash Shah wrote:
> Add Memory controller EDAC support in exisiting SiFive platform EDAC

s/in exisiting/to the/

> driver. It registers for notifier events from the SiFive DDR controller
> driver for DDR ECC events.

Simplify:

"It registers for ECC notifier events from the memory controller."

> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> ---
>  drivers/edac/Kconfig       |   2 +-
>  drivers/edac/sifive_edac.c | 117 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 118 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 7b6ec30..f8b3b53 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -462,7 +462,7 @@ config EDAC_ALTERA_SDMMC
>  
>  config EDAC_SIFIVE
>  	bool "Sifive platform EDAC driver"
> -	depends on EDAC=y && SIFIVE_L2
> +	depends on EDAC=y && (SIFIVE_L2 || SIFIVE_DDR)
>  	help
>  	  Support for error detection and correction on the SiFive SoCs.
>  
> diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c
> index 3a3dcb1..cf032685 100644
> --- a/drivers/edac/sifive_edac.c
> +++ b/drivers/edac/sifive_edac.c
> @@ -11,14 +11,120 @@
>  #include <linux/platform_device.h>
>  #include "edac_module.h"
>  #include <soc/sifive/sifive_l2_cache.h>
> +#include <soc/sifive/sifive_ddr.h>
>  
>  #define DRVNAME "sifive_edac"
> +#define SIFIVE_EDAC_MOD_NAME "Sifive ECC Manager"

s/SIFIVE_EDAC_MOD_NAME/EDAC_MOD_NAME/g

like the other EDAC drivers.

...

> +static int ecc_mc_register(struct platform_device *pdev)
> +{
> +	struct sifive_edac_mc_priv *p;
> +	struct edac_mc_layer layers[1];
> +	int ret;
> +
> +	p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
> +	if (!p)
> +		return -ENOMEM;
> +
> +	p->notifier.notifier_call = ecc_mc_err_event;
> +	platform_set_drvdata(pdev, p);
> +
> +	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
> +	layers[0].size = 1;
> +	layers[0].is_virt_csrow = true;
> +
> +	p->mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, 0);
> +	if (!p->mci) {
> +		dev_err(&pdev->dev, "Failed mem allocation for mc instance\n");
> +		return -ENOMEM;
> +	}
> +
> +	p->mci->pdev = &pdev->dev;
> +	/* Initialize controller capabilities */
> +	p->mci->mtype_cap = MEM_FLAG_DDR4;
> +	p->mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
> +	p->mci->edac_cap = EDAC_FLAG_SECDED;
> +	p->mci->scrub_cap = SCRUB_UNKNOWN;
> +	p->mci->scrub_mode = SCRUB_HW_PROG;
> +	p->mci->ctl_name = dev_name(&pdev->dev);
> +	p->mci->dev_name = dev_name(&pdev->dev);
> +	p->mci->mod_name = SIFIVE_EDAC_MOD_NAME;
> +	p->mci->ctl_page_to_phys = NULL;
> +
> +	/* Interrupt feature is supported by cadence mc */
> +	edac_op_state = EDAC_OPSTATE_INT;
> +
> +	ret = edac_mc_add_mc(p->mci);
> +	if (ret) {
> +		edac_printk(KERN_ERR, SIFIVE_EDAC_MOD_NAME,
> +			    "Failed to register with EDAC core\n");
> +		goto err;
> +	}
> +
> +#ifdef CONFIG_SIFIVE_DDR

It seems all that ifdeffery can be replaced with

	if (IS_ENABLED(CONFIG_...))

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

RE: [PATCH 3/3] edac: sifive: Add EDAC support for Memory Controller in SiFive SoCs

[Yash Shah]

> -----Original Message-----
> From: Borislav Petkov <bp@alien8.de>
> Sent: 31 August 2020 14:22
> To: Yash Shah <yash.shah@openfive.com>
> Cc: robh+dt@kernel.org; palmer@dabbelt.com; Paul Walmsley ( Sifive)
> <paul.walmsley@sifive.com>; mchehab@kernel.org; tony.luck@intel.com;
> aou@eecs.berkeley.edu; james.morse@arm.com; rrichter@marvell.com;
> devicetree@vger.kernel.org; linux-riscv@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-edac@vger.kernel.org; Sachin Ghadi
> <sachin.ghadi@openfive.com>
> Subject: Re: [PATCH 3/3] edac: sifive: Add EDAC support for Memory
> Controller in SiFive SoCs
> 
> [External Email] Do not click links or attachments unless you recognize the
> sender and know the content is safe
> 
> > Subject: Re: [PATCH 3/3] edac: sifive: Add EDAC support for Memory
> > Controller in SiFive SoCs
> 
> Fix subject prefix: "EDAC/sifive: ..."
> 
> On Tue, Aug 25, 2020 at 05:36:22PM +0530, Yash Shah wrote:
> > Add Memory controller EDAC support in exisiting SiFive platform EDAC
> 
> s/in exisiting/to the/
> 
> > driver. It registers for notifier events from the SiFive DDR
> > controller driver for DDR ECC events.
> 
> Simplify:
> 
> "It registers for ECC notifier events from the memory controller."
> 
> > Signed-off-by: Yash Shah <yash.shah@sifive.com>
> > ---
> >  drivers/edac/Kconfig       |   2 +-
> >  drivers/edac/sifive_edac.c | 117
> > +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 118 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig index
> > 7b6ec30..f8b3b53 100644
> > --- a/drivers/edac/Kconfig
> > +++ b/drivers/edac/Kconfig
> > @@ -462,7 +462,7 @@ config EDAC_ALTERA_SDMMC
> >
> >  config EDAC_SIFIVE
> >       bool "Sifive platform EDAC driver"
> > -     depends on EDAC=y && SIFIVE_L2
> > +     depends on EDAC=y && (SIFIVE_L2 || SIFIVE_DDR)
> >       help
> >         Support for error detection and correction on the SiFive SoCs.
> >
> > diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c
> > index 3a3dcb1..cf032685 100644
> > --- a/drivers/edac/sifive_edac.c
> > +++ b/drivers/edac/sifive_edac.c
> > @@ -11,14 +11,120 @@
> >  #include <linux/platform_device.h>
> >  #include "edac_module.h"
> >  #include <soc/sifive/sifive_l2_cache.h>
> > +#include <soc/sifive/sifive_ddr.h>
> >
> >  #define DRVNAME "sifive_edac"
> > +#define SIFIVE_EDAC_MOD_NAME "Sifive ECC Manager"
> 
> s/SIFIVE_EDAC_MOD_NAME/EDAC_MOD_NAME/g
> 
> like the other EDAC drivers.
> 

Sure, will make all the above suggested textual changes in v2.

> ...
> 
> > +static int ecc_mc_register(struct platform_device *pdev) {
> > +     struct sifive_edac_mc_priv *p;
> > +     struct edac_mc_layer layers[1];
> > +     int ret;
> > +
> > +     p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
> > +     if (!p)
> > +             return -ENOMEM;
> > +
> > +     p->notifier.notifier_call = ecc_mc_err_event;
> > +     platform_set_drvdata(pdev, p);
> > +
> > +     layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
> > +     layers[0].size = 1;
> > +     layers[0].is_virt_csrow = true;
> > +
> > +     p->mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, 0);
> > +     if (!p->mci) {
> > +             dev_err(&pdev->dev, "Failed mem allocation for mc instance\n");
> > +             return -ENOMEM;
> > +     }
> > +
> > +     p->mci->pdev = &pdev->dev;
> > +     /* Initialize controller capabilities */
> > +     p->mci->mtype_cap = MEM_FLAG_DDR4;
> > +     p->mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
> > +     p->mci->edac_cap = EDAC_FLAG_SECDED;
> > +     p->mci->scrub_cap = SCRUB_UNKNOWN;
> > +     p->mci->scrub_mode = SCRUB_HW_PROG;
> > +     p->mci->ctl_name = dev_name(&pdev->dev);
> > +     p->mci->dev_name = dev_name(&pdev->dev);
> > +     p->mci->mod_name = SIFIVE_EDAC_MOD_NAME;
> > +     p->mci->ctl_page_to_phys = NULL;
> > +
> > +     /* Interrupt feature is supported by cadence mc */
> > +     edac_op_state = EDAC_OPSTATE_INT;
> > +
> > +     ret = edac_mc_add_mc(p->mci);
> > +     if (ret) {
> > +             edac_printk(KERN_ERR, SIFIVE_EDAC_MOD_NAME,
> > +                         "Failed to register with EDAC core\n");
> > +             goto err;
> > +     }
> > +
> > +#ifdef CONFIG_SIFIVE_DDR
> 
> It seems all that ifdeffery can be replaced with
> 
>         if (IS_ENABLED(CONFIG_...))

Yes, will replace all the ifdeffery in v2
Thanks for the review.

- Yash

> 
> Thx.
> 
> --
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette