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

[Borislav Petkov]

On Mon, Sep 07, 2020 at 11:17:59AM +0530, Yash Shah wrote:
> Add Memory controller EDAC support to the SiFive platform EDAC driver.
> It registers for ECC notifier events from the memory controller.
> 
> Signed-off-by: Yash Shah 
> Reviewed-by: Palmer Dabbelt 
> Acked-by: Palmer Dabbelt 

Reviewed-by is usually enough and stronger than Acked-by. See sections

12) When to use Acked-by:, Cc:, and Co-developed-by:
13) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:

in Documentation/process/submitting-patches.rst.

With that addressed:

Acked-by: Borislav Petkov 

-- 
Regards/Gruss,
Boris.

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

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

[Yash Shah]

Add Memory controller EDAC support to the SiFive platform EDAC driver.
It registers for ECC notifier events from the memory controller.

Signed-off-by: Yash Shah 
Reviewed-by: Palmer Dabbelt 
Acked-by: Palmer Dabbelt 
---
 drivers/edac/Kconfig   |   2 +-
 drivers/edac/sifive_edac.c | 119 -
 2 files changed, 118 insertions(+), 3 deletions(-)

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..17dd556 100644
--- a/drivers/edac/sifive_edac.c
+++ b/drivers/edac/sifive_edac.c
@@ -11,14 +11,119 @@
 #include 
 #include "edac_module.h"
 #include 
+#include 
 
 #define DRVNAME "sifive_edac"
+#define 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(>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(>dev, "Failed mem allocation for mc instance\n");
+   return -ENOMEM;
+   }
+
+   p->mci->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(>dev);
+   p->mci->dev_name = dev_name(>dev);
+   p->mci->mod_name = 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, EDAC_MOD_NAME,
+   "Failed to register with EDAC core\n");
+   goto err;
+   }
+
+   if (IS_ENABLED(CONFIG_SIFIVE_DDR))
+   register_sifive_ddr_error_notifier(>notifier);
+
+   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);
+
+   if (IS_ENABLED(CONFIG_SIFIVE_DDR))
+   unregister_sifive_ddr_error_notifier(>notifier);
+
+   edac_mc_del_mc(>dev);
+   edac_mc_free(p->mci);
+
+   return 0;
+}
+
 /**
  * EDAC error callback
  *
@@ -67,7 +172,8 @@ static int ecc_register(struct platform_device *pdev)
goto err;
}
 
-   register_sifive_l2_error_notifier(>notifier);
+   if (IS_ENABLED(CONFIG_SIFIVE_L2))
+   register_sifive_l2_error_notifier(>notifier);
 
return 0;
 
@@ -81,7 +187,9 @@ static int ecc_unregister(struct