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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
On Tue, 25 Aug 2020 05:06:21 PDT (-0700), [email protected] 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 <[email protected]>
> ---
> 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 <[email protected]>
Acked-by: Palmer Dabbelt <[email protected]>
On Tue, 25 Aug 2020 05:06:19 PDT (-0700), [email protected] 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.
On Tue, 25 Aug 2020 05:06:22 PDT (-0700), [email protected] 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 <[email protected]>
> ---
> 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 <[email protected]>
Acked-by: Palmer Dabbelt <[email protected]>
On Tue, 25 Aug 2020 09:19:58 PDT (-0700), [email protected] 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!
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
> 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 <[email protected]>
> ---
> 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
> -----Original Message-----
> From: Borislav Petkov <[email protected]>
> Sent: 31 August 2020 14:22
> To: Yash Shah <[email protected]>
> Cc: [email protected]; [email protected]; Paul Walmsley ( Sifive)
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Sachin Ghadi
> <[email protected]>
> 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 <[email protected]>
> > ---
> > 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