* [PATCH 0/2] EDAC Support for SiFive SoCs
@ 2019-03-20 11:52 ` Yash Shah
0 siblings, 0 replies; 32+ messages in thread
From: Yash Shah @ 2019-03-20 11:52 UTC (permalink / raw)
To: linux-riscv, linux-edac, palmer, paul.walmsley
Cc: linux-kernel, robh+dt, mark.rutland, aou, bp, mchehab,
devicetree, sachin.ghadi, Yash Shah
This patch series adds an EDAC driver and DT documentation
for FU540-C000 chip.
Initially L2 Cache controller is added as a subcomponent to this driver.
This patchset is based on Linux 5.0-rc8 and tested on HiFive Unleashed
board with additional board related patches needed for testing can be
found at dev/yashs/L2_cache_controller branch of:
https://github.com/yashshah7/riscv-linux.git
Yash Shah (2):
edac: sifive: Add DT documentation for SiFive EDAC driver and
subcomponent
edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
.../devicetree/bindings/edac/sifive-edac.txt | 40 +++
arch/riscv/Kconfig | 1 +
drivers/edac/Kconfig | 13 +
drivers/edac/Makefile | 1 +
drivers/edac/sifive_edac.c | 297 +++++++++++++++++++++
5 files changed, 352 insertions(+)
create mode 100644 Documentation/devicetree/bindings/edac/sifive-edac.txt
create mode 100644 drivers/edac/sifive_edac.c
--
1.9.1
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 0/2] EDAC Support for SiFive SoCs
@ 2019-03-20 11:52 ` Yash Shah
0 siblings, 0 replies; 32+ messages in thread
From: Yash Shah @ 2019-03-20 11:52 UTC (permalink / raw)
To: linux-riscv, linux-edac, palmer, paul.walmsley
Cc: mark.rutland, devicetree, aou, linux-kernel, sachin.ghadi,
Yash Shah, robh+dt, bp, mchehab
This patch series adds an EDAC driver and DT documentation
for FU540-C000 chip.
Initially L2 Cache controller is added as a subcomponent to this driver.
This patchset is based on Linux 5.0-rc8 and tested on HiFive Unleashed
board with additional board related patches needed for testing can be
found at dev/yashs/L2_cache_controller branch of:
https://github.com/yashshah7/riscv-linux.git
Yash Shah (2):
edac: sifive: Add DT documentation for SiFive EDAC driver and
subcomponent
edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
.../devicetree/bindings/edac/sifive-edac.txt | 40 +++
arch/riscv/Kconfig | 1 +
drivers/edac/Kconfig | 13 +
drivers/edac/Makefile | 1 +
drivers/edac/sifive_edac.c | 297 +++++++++++++++++++++
5 files changed, 352 insertions(+)
create mode 100644 Documentation/devicetree/bindings/edac/sifive-edac.txt
create mode 100644 drivers/edac/sifive_edac.c
--
1.9.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 1/2] edac: sifive: Add DT documentation for SiFive EDAC driver and subcomponent
@ 2019-03-20 11:52 ` Yash Shah
0 siblings, 0 replies; 32+ messages in thread
From: Yash Shah @ 2019-03-20 11:52 UTC (permalink / raw)
To: linux-riscv, linux-edac, palmer, paul.walmsley
Cc: linux-kernel, robh+dt, mark.rutland, aou, bp, mchehab,
devicetree, sachin.ghadi, Yash Shah
DT documentation for EDAC driver added.
DT documentation for subcomponent L2 cache controller also added.
Signed-off-by: Yash Shah <yash.shah@sifive.com>
---
.../devicetree/bindings/edac/sifive-edac.txt | 40 ++++++++++++++++++++++
1 file changed, 40 insertions(+)
create mode 100644 Documentation/devicetree/bindings/edac/sifive-edac.txt
diff --git a/Documentation/devicetree/bindings/edac/sifive-edac.txt b/Documentation/devicetree/bindings/edac/sifive-edac.txt
new file mode 100644
index 0000000..c0e3ac7
--- /dev/null
+++ b/Documentation/devicetree/bindings/edac/sifive-edac.txt
@@ -0,0 +1,40 @@
+SiFive ECC Manager
+This driver uses the EDAC framework to implement the SiFive ECC Manager.
+
+Required Properties:
+- compatible : Should be "sifive,ecc-manager"
+- #address-cells: must be 2
+- #size-cells: must be 2
+- ranges : standard definition, should translate from local addresses
+
+Subcomponents:
+
+L2 Cache ECC
+Required Properties:
+- compatible: Should be "sifive,<chip>-ccache" and "sifive,ccache<version>".
+ Supported compatible strings are:
+ "sifive,fu540-c000-ccache" for the SiFive cache controller v0 as integrated
+ onto the SiFive FU540 chip, and "sifive,ccache0" for the SiFive
+ cache controller v0 IP block with no chip integration tweaks.
+ Please refer to sifive-blocks-ip-versioning.txt for details
+- interrupt-parent: Must be core interrupt controller
+- interrupts: Must contain 3 entries (DirError, DataError and DataFail signals
+- reg: Physical base address and size of L2 cache controller registers map
+ A second range can indicate L2 Loosely Integrated Memory
+
+Example:
+
+eccmgr: eccmgr {
+ compatible = "sifive,ecc-manager";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ l2-ecc@2010000 {
+ compatible = "sifive,fu540-c000-ccache", "sifive,ccache0";
+ interrupt-parent = <&plic0>;
+ interrupts = <1 2 3>;
+ reg = <0x0 0x2010000 0x0 0x1000 0x0 0x8000000 0x0 0x2000000>;
+ };
+};
+
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [1/2] edac: sifive: Add DT documentation for SiFive EDAC driver and subcomponent
@ 2019-03-20 11:52 ` Yash Shah
0 siblings, 0 replies; 32+ messages in thread
From: Yash Shah @ 2019-03-20 11:52 UTC (permalink / raw)
To: linux-riscv, linux-edac, palmer, paul.walmsley
Cc: linux-kernel, robh+dt, mark.rutland, aou, bp, mchehab,
devicetree, sachin.ghadi, Yash Shah
DT documentation for EDAC driver added.
DT documentation for subcomponent L2 cache controller also added.
Signed-off-by: Yash Shah <yash.shah@sifive.com>
---
.../devicetree/bindings/edac/sifive-edac.txt | 40 ++++++++++++++++++++++
1 file changed, 40 insertions(+)
create mode 100644 Documentation/devicetree/bindings/edac/sifive-edac.txt
diff --git a/Documentation/devicetree/bindings/edac/sifive-edac.txt b/Documentation/devicetree/bindings/edac/sifive-edac.txt
new file mode 100644
index 0000000..c0e3ac7
--- /dev/null
+++ b/Documentation/devicetree/bindings/edac/sifive-edac.txt
@@ -0,0 +1,40 @@
+SiFive ECC Manager
+This driver uses the EDAC framework to implement the SiFive ECC Manager.
+
+Required Properties:
+- compatible : Should be "sifive,ecc-manager"
+- #address-cells: must be 2
+- #size-cells: must be 2
+- ranges : standard definition, should translate from local addresses
+
+Subcomponents:
+
+L2 Cache ECC
+Required Properties:
+- compatible: Should be "sifive,<chip>-ccache" and "sifive,ccache<version>".
+ Supported compatible strings are:
+ "sifive,fu540-c000-ccache" for the SiFive cache controller v0 as integrated
+ onto the SiFive FU540 chip, and "sifive,ccache0" for the SiFive
+ cache controller v0 IP block with no chip integration tweaks.
+ Please refer to sifive-blocks-ip-versioning.txt for details
+- interrupt-parent: Must be core interrupt controller
+- interrupts: Must contain 3 entries (DirError, DataError and DataFail signals
+- reg: Physical base address and size of L2 cache controller registers map
+ A second range can indicate L2 Loosely Integrated Memory
+
+Example:
+
+eccmgr: eccmgr {
+ compatible = "sifive,ecc-manager";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ l2-ecc@2010000 {
+ compatible = "sifive,fu540-c000-ccache", "sifive,ccache0";
+ interrupt-parent = <&plic0>;
+ interrupts = <1 2 3>;
+ reg = <0x0 0x2010000 0x0 0x1000 0x0 0x8000000 0x0 0x2000000>;
+ };
+};
+
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 1/2] edac: sifive: Add DT documentation for SiFive EDAC driver and subcomponent
@ 2019-03-20 11:52 ` Yash Shah
0 siblings, 0 replies; 32+ messages in thread
From: Yash Shah @ 2019-03-20 11:52 UTC (permalink / raw)
To: linux-riscv, linux-edac, palmer, paul.walmsley
Cc: mark.rutland, devicetree, aou, linux-kernel, sachin.ghadi,
Yash Shah, robh+dt, bp, mchehab
DT documentation for EDAC driver added.
DT documentation for subcomponent L2 cache controller also added.
Signed-off-by: Yash Shah <yash.shah@sifive.com>
---
.../devicetree/bindings/edac/sifive-edac.txt | 40 ++++++++++++++++++++++
1 file changed, 40 insertions(+)
create mode 100644 Documentation/devicetree/bindings/edac/sifive-edac.txt
diff --git a/Documentation/devicetree/bindings/edac/sifive-edac.txt b/Documentation/devicetree/bindings/edac/sifive-edac.txt
new file mode 100644
index 0000000..c0e3ac7
--- /dev/null
+++ b/Documentation/devicetree/bindings/edac/sifive-edac.txt
@@ -0,0 +1,40 @@
+SiFive ECC Manager
+This driver uses the EDAC framework to implement the SiFive ECC Manager.
+
+Required Properties:
+- compatible : Should be "sifive,ecc-manager"
+- #address-cells: must be 2
+- #size-cells: must be 2
+- ranges : standard definition, should translate from local addresses
+
+Subcomponents:
+
+L2 Cache ECC
+Required Properties:
+- compatible: Should be "sifive,<chip>-ccache" and "sifive,ccache<version>".
+ Supported compatible strings are:
+ "sifive,fu540-c000-ccache" for the SiFive cache controller v0 as integrated
+ onto the SiFive FU540 chip, and "sifive,ccache0" for the SiFive
+ cache controller v0 IP block with no chip integration tweaks.
+ Please refer to sifive-blocks-ip-versioning.txt for details
+- interrupt-parent: Must be core interrupt controller
+- interrupts: Must contain 3 entries (DirError, DataError and DataFail signals
+- reg: Physical base address and size of L2 cache controller registers map
+ A second range can indicate L2 Loosely Integrated Memory
+
+Example:
+
+eccmgr: eccmgr {
+ compatible = "sifive,ecc-manager";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ l2-ecc@2010000 {
+ compatible = "sifive,fu540-c000-ccache", "sifive,ccache0";
+ interrupt-parent = <&plic0>;
+ interrupts = <1 2 3>;
+ reg = <0x0 0x2010000 0x0 0x1000 0x0 0x8000000 0x0 0x2000000>;
+ };
+};
+
--
1.9.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-20 11:52 ` Yash Shah
0 siblings, 0 replies; 32+ messages in thread
From: Yash Shah @ 2019-03-20 11:52 UTC (permalink / raw)
To: linux-riscv, linux-edac, palmer, paul.walmsley
Cc: linux-kernel, robh+dt, mark.rutland, aou, bp, mchehab,
devicetree, sachin.ghadi, Yash Shah
This EDAC driver supports:
- Initial configuration reporting on bootup via debug logs
- ECC event monitoring and reporting through the EDAC framework
- ECC event injection
This driver is partially based on pnd2_edac.c and altera_edac.c
Initially L2 Cache controller is added as a subcomponent to
this EDAC driver.
Signed-off-by: Yash Shah <yash.shah@sifive.com>
---
arch/riscv/Kconfig | 1 +
drivers/edac/Kconfig | 13 ++
drivers/edac/Makefile | 1 +
drivers/edac/sifive_edac.c | 297 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 312 insertions(+)
create mode 100644 drivers/edac/sifive_edac.c
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 515fc3c..fede4b6 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -49,6 +49,7 @@ config RISCV
select RISCV_TIMER
select GENERIC_IRQ_MULTI_HANDLER
select ARCH_HAS_PTE_SPECIAL
+ select EDAC_SUPPORT
config MMU
def_bool y
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index e286b5b..112d9d1 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -440,6 +440,19 @@ config EDAC_ALTERA_SDMMC
Support for error detection and correction on the
Altera SDMMC FIFO Memory for Altera SoCs.
+config EDAC_SIFIVE
+ tristate "Sifive ECC"
+ depends on RISCV
+ help
+ Support for error detection and correction on the SiFive SoCs.
+
+config EDAC_SIFIVE_L2
+ bool "SiFive L2 Cache ECC"
+ depends on EDAC_SIFIVE=y
+ help
+ Support for error detection and correction of the L2 cache
+ memory on SiFive SoCs.
+
config EDAC_SYNOPSYS
tristate "Synopsys DDR Memory Controller"
depends on ARCH_ZYNQ || ARCH_ZYNQMP
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 716096d..b16dce8 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_EDAC_OCTEON_PCI) += octeon_edac-pci.o
obj-$(CONFIG_EDAC_THUNDERX) += thunderx_edac.o
obj-$(CONFIG_EDAC_ALTERA) += altera_edac.o
+obj-$(CONFIG_EDAC_SIFIVE) += sifive_edac.o
obj-$(CONFIG_EDAC_SYNOPSYS) += synopsys_edac.o
obj-$(CONFIG_EDAC_XGENE) += xgene_edac.o
obj-$(CONFIG_EDAC_TI) += ti_edac.o
diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c
new file mode 100644
index 0000000..e11ae6b5
--- /dev/null
+++ b/drivers/edac/sifive_edac.c
@@ -0,0 +1,297 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SiFive EDAC Driver
+ *
+ * Copyright (C) 2018-2019 SiFive, Inc.
+ *
+ */
+#include <linux/edac.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include "edac_module.h"
+
+#define SIFIVE_EDAC_DIRFIX_LOW 0x100
+#define SIFIVE_EDAC_DIRFIX_HIGH 0x104
+#define SIFIVE_EDAC_DIRFIX_COUNT 0x108
+
+#define SIFIVE_EDAC_DATFIX_LOW 0x140
+#define SIFIVE_EDAC_DATFIX_HIGH 0x144
+#define SIFIVE_EDAC_DATFIX_COUNT 0x148
+
+#define SIFIVE_EDAC_DATFAIL_LOW 0x160
+#define SIFIVE_EDAC_DATFAIL_HIGH 0x164
+#define SIFIVE_EDAC_DATFAIL_COUNT 0x168
+
+#define SIFIVE_EDAC_ECCINJECTERR 0x40
+#define SIFIVE_EDAC_CONFIG 0x00
+
+#define SIFIVE_EDAC_MAX_INTR 3
+
+/************************* EDAC Parent Probe *************************/
+
+static const struct of_device_id sifive_edac_device_of_match[];
+
+static const struct of_device_id sifive_edac_of_match[] = {
+ { .compatible = "sifive,ecc-manager" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, sifive_edac_of_match);
+
+static int sifive_edac_probe(struct platform_device *pdev)
+{
+ of_platform_populate(pdev->dev.of_node, sifive_edac_device_of_match,
+ NULL, &pdev->dev);
+ return 0;
+}
+
+static struct platform_driver sifive_edac_driver = {
+ .probe = sifive_edac_probe,
+ .driver = {
+ .name = "ecc_manager",
+ .of_match_table = sifive_edac_of_match,
+ },
+};
+module_platform_driver(sifive_edac_driver);
+
+struct sifive_edac_device_prv {
+ void (*setup)(struct edac_device_ctl_info *dci);
+ irqreturn_t (*ecc_irq_handler)(int irq, void *dev_id);
+ const struct file_operations *inject_fops;
+};
+
+struct sifive_edac_device_dev {
+ void __iomem *base;
+ int irq[SIFIVE_EDAC_MAX_INTR];
+ struct sifive_edac_device_prv *data;
+ char *edac_dev_name;
+};
+
+enum {
+ dir_corr = 0,
+ data_corr,
+ data_uncorr,
+};
+
+static struct dentry *sifive_edac_test;
+
+static ssize_t sifive_edac_l2_write(struct file *file, const char __user *data,
+ size_t count, loff_t *ppos)
+{
+ struct edac_device_ctl_info *dci = file->private_data;
+ struct sifive_edac_device_dev *drvdata = dci->pvt_info;
+ unsigned int val;
+
+ if (kstrtouint_from_user(data, count, 0, &val))
+ return -EINVAL;
+ if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
+ writel(val, drvdata->base + SIFIVE_EDAC_ECCINJECTERR);
+ else
+ return -EINVAL;
+ return count;
+}
+
+static const struct file_operations sifive_edac_l2_fops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .write = sifive_edac_l2_write
+};
+
+static void setup_sifive_debug(struct edac_device_ctl_info *edac_dci,
+ const struct sifive_edac_device_prv *prv)
+{
+ struct sifive_edac_device_dev *drvdata = edac_dci->pvt_info;
+
+ if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
+ return;
+
+ sifive_edac_test = edac_debugfs_create_dir(drvdata->edac_dev_name);
+ if (!sifive_edac_test)
+ return;
+
+ if (!edac_debugfs_create_file("sifive_debug_inject_error", 0200,
+ sifive_edac_test, edac_dci,
+ prv->inject_fops))
+ debugfs_remove_recursive(sifive_edac_test);
+}
+
+static void teardown_sifive_debug(void)
+{
+ debugfs_remove_recursive(sifive_edac_test);
+}
+
+/*
+ * sifive_edac_l2_int_handler - ISR function for l2 cache controller
+ * @irq: Irq Number
+ * @device: Pointer to the edac device controller instance
+ *
+ * This routine is triggered whenever there is ECC error detected
+ *
+ * Return: Always returns IRQ_HANDLED
+ */
+static irqreturn_t sifive_edac_l2_int_handler(int irq, void *device)
+{
+ struct edac_device_ctl_info *dci =
+ (struct edac_device_ctl_info *)device;
+ struct sifive_edac_device_dev *drvdata = dci->pvt_info;
+ u32 regval, add_h, add_l;
+
+ if (irq == drvdata->irq[dir_corr]) {
+ add_h = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_HIGH);
+ add_l = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_LOW);
+ dev_err(dci->dev,
+ "DirError at address 0x%08X.%08X\n", add_h, add_l);
+ regval = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_COUNT);
+ edac_device_handle_ce(dci, 0, 0, "DirECCFix");
+ }
+ if (irq == drvdata->irq[data_corr]) {
+ add_h = readl(drvdata->base + SIFIVE_EDAC_DATFIX_HIGH);
+ add_l = readl(drvdata->base + SIFIVE_EDAC_DATFIX_LOW);
+ dev_err(dci->dev,
+ "DataError at address 0x%08X.%08X\n", add_h, add_l);
+ regval = readl(drvdata->base + SIFIVE_EDAC_DATFIX_COUNT);
+ edac_device_handle_ce(dci, 0, 0, "DatECCFix");
+ }
+ if (irq == drvdata->irq[data_uncorr]) {
+ add_h = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_HIGH);
+ add_l = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_LOW);
+ dev_err(dci->dev,
+ "DataFail at address 0x%08X.%08X\n", add_h, add_l);
+ regval = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_COUNT);
+ edac_device_handle_ue(dci, 0, 0, "DatECCFail");
+ }
+
+ return IRQ_HANDLED;
+}
+
+static void sifive_edac_l2_config_read(struct edac_device_ctl_info *dci)
+{
+ struct sifive_edac_device_dev *drvdata = dci->pvt_info;
+ u32 regval, val;
+
+ regval = readl(drvdata->base + SIFIVE_EDAC_CONFIG);
+ val = regval & 0xFF;
+ dev_info(dci->dev, "No. of Banks in the cache: %d\n", val);
+ val = (regval & 0xFF00) >> 8;
+ dev_info(dci->dev, "No. of ways per bank: %d\n", val);
+ val = (regval & 0xFF0000) >> 16;
+ dev_info(dci->dev, "Sets per bank: %llu\n", (uint64_t)1 << val);
+ val = (regval & 0xFF000000) >> 24;
+ dev_info(dci->dev,
+ "Bytes per cache block: %llu\n", (uint64_t)1 << val);
+}
+
+static const struct sifive_edac_device_prv l2ecc_data = {
+ .setup = sifive_edac_l2_config_read,
+ .inject_fops = &sifive_edac_l2_fops,
+ .ecc_irq_handler = sifive_edac_l2_int_handler,
+};
+
+/*
+ * sifive_edac_device_probe()
+ * This is a generic EDAC device driver that will support
+ * various SiFive memory devices as well as the memories
+ * for other peripherals. Module specific initialization is
+ * done by passing the function index in the device tree.
+ */
+static int sifive_edac_device_probe(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *dci;
+ struct sifive_edac_device_dev *drvdata;
+ int rc, i;
+ struct resource *res;
+ void __iomem *baseaddr;
+ struct device_node *np = pdev->dev.of_node;
+ char *ecc_name = (char *)np->name;
+ static int dev_instance;
+
+ /* Get the data from the platform device */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ baseaddr = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(baseaddr))
+ return PTR_ERR(baseaddr);
+
+ dci = edac_device_alloc_ctl_info(sizeof(*drvdata), ecc_name,
+ 1, ecc_name, 1, 1, NULL, 0,
+ dev_instance++);
+ if (IS_ERR(dci))
+ return PTR_ERR(dci);
+
+ drvdata = dci->pvt_info;
+ drvdata->base = baseaddr;
+ drvdata->edac_dev_name = ecc_name;
+ dci->dev = &pdev->dev;
+ dci->mod_name = "Sifive ECC Manager";
+ dci->ctl_name = dev_name(&pdev->dev);
+ dci->dev_name = dev_name(&pdev->dev);
+
+ /* Get driver specific data for this EDAC device */
+ drvdata->data = of_match_node(sifive_edac_device_of_match, np)->data;
+
+ setup_sifive_debug(dci, drvdata->data);
+
+ if (drvdata->data->setup)
+ drvdata->data->setup(dci);
+
+ for (i = 0; i < SIFIVE_EDAC_MAX_INTR; i++) {
+ drvdata->irq[i] = platform_get_irq(pdev, i);
+ rc = devm_request_irq(&pdev->dev, drvdata->irq[i],
+ sifive_edac_l2_int_handler, 0,
+ dev_name(&pdev->dev), (void *)dci);
+ if (rc) {
+ dev_err(&pdev->dev,
+ "Could not request IRQ %d\n", drvdata->irq[i]);
+ goto del_edac_device;
+ }
+ }
+
+ rc = edac_device_add_device(dci);
+ if (rc) {
+ dev_err(&pdev->dev, "failed to register with EDAC core\n");
+ goto del_edac_device;
+ }
+
+ return rc;
+
+del_edac_device:
+ teardown_sifive_debug();
+ edac_device_del_device(&pdev->dev);
+ edac_device_free_ctl_info(dci);
+
+ return rc;
+}
+
+static int sifive_edac_device_remove(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
+
+ teardown_sifive_debug();
+ edac_device_del_device(&pdev->dev);
+ edac_device_free_ctl_info(dci);
+
+ return 0;
+}
+
+static const struct of_device_id sifive_edac_device_of_match[] = {
+ { .compatible = "sifive,ccache0", .data = &l2ecc_data },
+ { /* end of table */ },
+};
+MODULE_DEVICE_TABLE(of, sifive_edac_device_of_match);
+
+static struct platform_driver sifive_edac_device_driver = {
+ .driver = {
+ .name = "sifive_edac_device",
+ .owner = THIS_MODULE,
+ .of_match_table = sifive_edac_device_of_match,
+ },
+ .probe = sifive_edac_device_probe,
+ .remove = sifive_edac_device_remove,
+};
+
+module_platform_driver(sifive_edac_device_driver);
+
+MODULE_AUTHOR("SiFive Inc.");
+MODULE_DESCRIPTION("SiFive EDAC driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-20 11:52 ` Yash Shah
0 siblings, 0 replies; 32+ messages in thread
From: Yash Shah @ 2019-03-20 11:52 UTC (permalink / raw)
To: linux-riscv, linux-edac, palmer, paul.walmsley
Cc: linux-kernel, robh+dt, mark.rutland, aou, bp, mchehab,
devicetree, sachin.ghadi, Yash Shah
This EDAC driver supports:
- Initial configuration reporting on bootup via debug logs
- ECC event monitoring and reporting through the EDAC framework
- ECC event injection
This driver is partially based on pnd2_edac.c and altera_edac.c
Initially L2 Cache controller is added as a subcomponent to
this EDAC driver.
Signed-off-by: Yash Shah <yash.shah@sifive.com>
---
arch/riscv/Kconfig | 1 +
drivers/edac/Kconfig | 13 ++
drivers/edac/Makefile | 1 +
drivers/edac/sifive_edac.c | 297 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 312 insertions(+)
create mode 100644 drivers/edac/sifive_edac.c
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 515fc3c..fede4b6 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -49,6 +49,7 @@ config RISCV
select RISCV_TIMER
select GENERIC_IRQ_MULTI_HANDLER
select ARCH_HAS_PTE_SPECIAL
+ select EDAC_SUPPORT
config MMU
def_bool y
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index e286b5b..112d9d1 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -440,6 +440,19 @@ config EDAC_ALTERA_SDMMC
Support for error detection and correction on the
Altera SDMMC FIFO Memory for Altera SoCs.
+config EDAC_SIFIVE
+ tristate "Sifive ECC"
+ depends on RISCV
+ help
+ Support for error detection and correction on the SiFive SoCs.
+
+config EDAC_SIFIVE_L2
+ bool "SiFive L2 Cache ECC"
+ depends on EDAC_SIFIVE=y
+ help
+ Support for error detection and correction of the L2 cache
+ memory on SiFive SoCs.
+
config EDAC_SYNOPSYS
tristate "Synopsys DDR Memory Controller"
depends on ARCH_ZYNQ || ARCH_ZYNQMP
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 716096d..b16dce8 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_EDAC_OCTEON_PCI) += octeon_edac-pci.o
obj-$(CONFIG_EDAC_THUNDERX) += thunderx_edac.o
obj-$(CONFIG_EDAC_ALTERA) += altera_edac.o
+obj-$(CONFIG_EDAC_SIFIVE) += sifive_edac.o
obj-$(CONFIG_EDAC_SYNOPSYS) += synopsys_edac.o
obj-$(CONFIG_EDAC_XGENE) += xgene_edac.o
obj-$(CONFIG_EDAC_TI) += ti_edac.o
diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c
new file mode 100644
index 0000000..e11ae6b5
--- /dev/null
+++ b/drivers/edac/sifive_edac.c
@@ -0,0 +1,297 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SiFive EDAC Driver
+ *
+ * Copyright (C) 2018-2019 SiFive, Inc.
+ *
+ */
+#include <linux/edac.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include "edac_module.h"
+
+#define SIFIVE_EDAC_DIRFIX_LOW 0x100
+#define SIFIVE_EDAC_DIRFIX_HIGH 0x104
+#define SIFIVE_EDAC_DIRFIX_COUNT 0x108
+
+#define SIFIVE_EDAC_DATFIX_LOW 0x140
+#define SIFIVE_EDAC_DATFIX_HIGH 0x144
+#define SIFIVE_EDAC_DATFIX_COUNT 0x148
+
+#define SIFIVE_EDAC_DATFAIL_LOW 0x160
+#define SIFIVE_EDAC_DATFAIL_HIGH 0x164
+#define SIFIVE_EDAC_DATFAIL_COUNT 0x168
+
+#define SIFIVE_EDAC_ECCINJECTERR 0x40
+#define SIFIVE_EDAC_CONFIG 0x00
+
+#define SIFIVE_EDAC_MAX_INTR 3
+
+/************************* EDAC Parent Probe *************************/
+
+static const struct of_device_id sifive_edac_device_of_match[];
+
+static const struct of_device_id sifive_edac_of_match[] = {
+ { .compatible = "sifive,ecc-manager" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, sifive_edac_of_match);
+
+static int sifive_edac_probe(struct platform_device *pdev)
+{
+ of_platform_populate(pdev->dev.of_node, sifive_edac_device_of_match,
+ NULL, &pdev->dev);
+ return 0;
+}
+
+static struct platform_driver sifive_edac_driver = {
+ .probe = sifive_edac_probe,
+ .driver = {
+ .name = "ecc_manager",
+ .of_match_table = sifive_edac_of_match,
+ },
+};
+module_platform_driver(sifive_edac_driver);
+
+struct sifive_edac_device_prv {
+ void (*setup)(struct edac_device_ctl_info *dci);
+ irqreturn_t (*ecc_irq_handler)(int irq, void *dev_id);
+ const struct file_operations *inject_fops;
+};
+
+struct sifive_edac_device_dev {
+ void __iomem *base;
+ int irq[SIFIVE_EDAC_MAX_INTR];
+ struct sifive_edac_device_prv *data;
+ char *edac_dev_name;
+};
+
+enum {
+ dir_corr = 0,
+ data_corr,
+ data_uncorr,
+};
+
+static struct dentry *sifive_edac_test;
+
+static ssize_t sifive_edac_l2_write(struct file *file, const char __user *data,
+ size_t count, loff_t *ppos)
+{
+ struct edac_device_ctl_info *dci = file->private_data;
+ struct sifive_edac_device_dev *drvdata = dci->pvt_info;
+ unsigned int val;
+
+ if (kstrtouint_from_user(data, count, 0, &val))
+ return -EINVAL;
+ if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
+ writel(val, drvdata->base + SIFIVE_EDAC_ECCINJECTERR);
+ else
+ return -EINVAL;
+ return count;
+}
+
+static const struct file_operations sifive_edac_l2_fops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .write = sifive_edac_l2_write
+};
+
+static void setup_sifive_debug(struct edac_device_ctl_info *edac_dci,
+ const struct sifive_edac_device_prv *prv)
+{
+ struct sifive_edac_device_dev *drvdata = edac_dci->pvt_info;
+
+ if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
+ return;
+
+ sifive_edac_test = edac_debugfs_create_dir(drvdata->edac_dev_name);
+ if (!sifive_edac_test)
+ return;
+
+ if (!edac_debugfs_create_file("sifive_debug_inject_error", 0200,
+ sifive_edac_test, edac_dci,
+ prv->inject_fops))
+ debugfs_remove_recursive(sifive_edac_test);
+}
+
+static void teardown_sifive_debug(void)
+{
+ debugfs_remove_recursive(sifive_edac_test);
+}
+
+/*
+ * sifive_edac_l2_int_handler - ISR function for l2 cache controller
+ * @irq: Irq Number
+ * @device: Pointer to the edac device controller instance
+ *
+ * This routine is triggered whenever there is ECC error detected
+ *
+ * Return: Always returns IRQ_HANDLED
+ */
+static irqreturn_t sifive_edac_l2_int_handler(int irq, void *device)
+{
+ struct edac_device_ctl_info *dci =
+ (struct edac_device_ctl_info *)device;
+ struct sifive_edac_device_dev *drvdata = dci->pvt_info;
+ u32 regval, add_h, add_l;
+
+ if (irq == drvdata->irq[dir_corr]) {
+ add_h = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_HIGH);
+ add_l = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_LOW);
+ dev_err(dci->dev,
+ "DirError at address 0x%08X.%08X\n", add_h, add_l);
+ regval = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_COUNT);
+ edac_device_handle_ce(dci, 0, 0, "DirECCFix");
+ }
+ if (irq == drvdata->irq[data_corr]) {
+ add_h = readl(drvdata->base + SIFIVE_EDAC_DATFIX_HIGH);
+ add_l = readl(drvdata->base + SIFIVE_EDAC_DATFIX_LOW);
+ dev_err(dci->dev,
+ "DataError at address 0x%08X.%08X\n", add_h, add_l);
+ regval = readl(drvdata->base + SIFIVE_EDAC_DATFIX_COUNT);
+ edac_device_handle_ce(dci, 0, 0, "DatECCFix");
+ }
+ if (irq == drvdata->irq[data_uncorr]) {
+ add_h = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_HIGH);
+ add_l = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_LOW);
+ dev_err(dci->dev,
+ "DataFail at address 0x%08X.%08X\n", add_h, add_l);
+ regval = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_COUNT);
+ edac_device_handle_ue(dci, 0, 0, "DatECCFail");
+ }
+
+ return IRQ_HANDLED;
+}
+
+static void sifive_edac_l2_config_read(struct edac_device_ctl_info *dci)
+{
+ struct sifive_edac_device_dev *drvdata = dci->pvt_info;
+ u32 regval, val;
+
+ regval = readl(drvdata->base + SIFIVE_EDAC_CONFIG);
+ val = regval & 0xFF;
+ dev_info(dci->dev, "No. of Banks in the cache: %d\n", val);
+ val = (regval & 0xFF00) >> 8;
+ dev_info(dci->dev, "No. of ways per bank: %d\n", val);
+ val = (regval & 0xFF0000) >> 16;
+ dev_info(dci->dev, "Sets per bank: %llu\n", (uint64_t)1 << val);
+ val = (regval & 0xFF000000) >> 24;
+ dev_info(dci->dev,
+ "Bytes per cache block: %llu\n", (uint64_t)1 << val);
+}
+
+static const struct sifive_edac_device_prv l2ecc_data = {
+ .setup = sifive_edac_l2_config_read,
+ .inject_fops = &sifive_edac_l2_fops,
+ .ecc_irq_handler = sifive_edac_l2_int_handler,
+};
+
+/*
+ * sifive_edac_device_probe()
+ * This is a generic EDAC device driver that will support
+ * various SiFive memory devices as well as the memories
+ * for other peripherals. Module specific initialization is
+ * done by passing the function index in the device tree.
+ */
+static int sifive_edac_device_probe(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *dci;
+ struct sifive_edac_device_dev *drvdata;
+ int rc, i;
+ struct resource *res;
+ void __iomem *baseaddr;
+ struct device_node *np = pdev->dev.of_node;
+ char *ecc_name = (char *)np->name;
+ static int dev_instance;
+
+ /* Get the data from the platform device */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ baseaddr = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(baseaddr))
+ return PTR_ERR(baseaddr);
+
+ dci = edac_device_alloc_ctl_info(sizeof(*drvdata), ecc_name,
+ 1, ecc_name, 1, 1, NULL, 0,
+ dev_instance++);
+ if (IS_ERR(dci))
+ return PTR_ERR(dci);
+
+ drvdata = dci->pvt_info;
+ drvdata->base = baseaddr;
+ drvdata->edac_dev_name = ecc_name;
+ dci->dev = &pdev->dev;
+ dci->mod_name = "Sifive ECC Manager";
+ dci->ctl_name = dev_name(&pdev->dev);
+ dci->dev_name = dev_name(&pdev->dev);
+
+ /* Get driver specific data for this EDAC device */
+ drvdata->data = of_match_node(sifive_edac_device_of_match, np)->data;
+
+ setup_sifive_debug(dci, drvdata->data);
+
+ if (drvdata->data->setup)
+ drvdata->data->setup(dci);
+
+ for (i = 0; i < SIFIVE_EDAC_MAX_INTR; i++) {
+ drvdata->irq[i] = platform_get_irq(pdev, i);
+ rc = devm_request_irq(&pdev->dev, drvdata->irq[i],
+ sifive_edac_l2_int_handler, 0,
+ dev_name(&pdev->dev), (void *)dci);
+ if (rc) {
+ dev_err(&pdev->dev,
+ "Could not request IRQ %d\n", drvdata->irq[i]);
+ goto del_edac_device;
+ }
+ }
+
+ rc = edac_device_add_device(dci);
+ if (rc) {
+ dev_err(&pdev->dev, "failed to register with EDAC core\n");
+ goto del_edac_device;
+ }
+
+ return rc;
+
+del_edac_device:
+ teardown_sifive_debug();
+ edac_device_del_device(&pdev->dev);
+ edac_device_free_ctl_info(dci);
+
+ return rc;
+}
+
+static int sifive_edac_device_remove(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
+
+ teardown_sifive_debug();
+ edac_device_del_device(&pdev->dev);
+ edac_device_free_ctl_info(dci);
+
+ return 0;
+}
+
+static const struct of_device_id sifive_edac_device_of_match[] = {
+ { .compatible = "sifive,ccache0", .data = &l2ecc_data },
+ { /* end of table */ },
+};
+MODULE_DEVICE_TABLE(of, sifive_edac_device_of_match);
+
+static struct platform_driver sifive_edac_device_driver = {
+ .driver = {
+ .name = "sifive_edac_device",
+ .owner = THIS_MODULE,
+ .of_match_table = sifive_edac_device_of_match,
+ },
+ .probe = sifive_edac_device_probe,
+ .remove = sifive_edac_device_remove,
+};
+
+module_platform_driver(sifive_edac_device_driver);
+
+MODULE_AUTHOR("SiFive Inc.");
+MODULE_DESCRIPTION("SiFive EDAC driver");
+MODULE_LICENSE("GPL v2");
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-20 11:52 ` Yash Shah
0 siblings, 0 replies; 32+ messages in thread
From: Yash Shah @ 2019-03-20 11:52 UTC (permalink / raw)
To: linux-riscv, linux-edac, palmer, paul.walmsley
Cc: mark.rutland, devicetree, aou, linux-kernel, sachin.ghadi,
Yash Shah, robh+dt, bp, mchehab
This EDAC driver supports:
- Initial configuration reporting on bootup via debug logs
- ECC event monitoring and reporting through the EDAC framework
- ECC event injection
This driver is partially based on pnd2_edac.c and altera_edac.c
Initially L2 Cache controller is added as a subcomponent to
this EDAC driver.
Signed-off-by: Yash Shah <yash.shah@sifive.com>
---
arch/riscv/Kconfig | 1 +
drivers/edac/Kconfig | 13 ++
drivers/edac/Makefile | 1 +
drivers/edac/sifive_edac.c | 297 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 312 insertions(+)
create mode 100644 drivers/edac/sifive_edac.c
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 515fc3c..fede4b6 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -49,6 +49,7 @@ config RISCV
select RISCV_TIMER
select GENERIC_IRQ_MULTI_HANDLER
select ARCH_HAS_PTE_SPECIAL
+ select EDAC_SUPPORT
config MMU
def_bool y
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index e286b5b..112d9d1 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -440,6 +440,19 @@ config EDAC_ALTERA_SDMMC
Support for error detection and correction on the
Altera SDMMC FIFO Memory for Altera SoCs.
+config EDAC_SIFIVE
+ tristate "Sifive ECC"
+ depends on RISCV
+ help
+ Support for error detection and correction on the SiFive SoCs.
+
+config EDAC_SIFIVE_L2
+ bool "SiFive L2 Cache ECC"
+ depends on EDAC_SIFIVE=y
+ help
+ Support for error detection and correction of the L2 cache
+ memory on SiFive SoCs.
+
config EDAC_SYNOPSYS
tristate "Synopsys DDR Memory Controller"
depends on ARCH_ZYNQ || ARCH_ZYNQMP
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 716096d..b16dce8 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_EDAC_OCTEON_PCI) += octeon_edac-pci.o
obj-$(CONFIG_EDAC_THUNDERX) += thunderx_edac.o
obj-$(CONFIG_EDAC_ALTERA) += altera_edac.o
+obj-$(CONFIG_EDAC_SIFIVE) += sifive_edac.o
obj-$(CONFIG_EDAC_SYNOPSYS) += synopsys_edac.o
obj-$(CONFIG_EDAC_XGENE) += xgene_edac.o
obj-$(CONFIG_EDAC_TI) += ti_edac.o
diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c
new file mode 100644
index 0000000..e11ae6b5
--- /dev/null
+++ b/drivers/edac/sifive_edac.c
@@ -0,0 +1,297 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SiFive EDAC Driver
+ *
+ * Copyright (C) 2018-2019 SiFive, Inc.
+ *
+ */
+#include <linux/edac.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include "edac_module.h"
+
+#define SIFIVE_EDAC_DIRFIX_LOW 0x100
+#define SIFIVE_EDAC_DIRFIX_HIGH 0x104
+#define SIFIVE_EDAC_DIRFIX_COUNT 0x108
+
+#define SIFIVE_EDAC_DATFIX_LOW 0x140
+#define SIFIVE_EDAC_DATFIX_HIGH 0x144
+#define SIFIVE_EDAC_DATFIX_COUNT 0x148
+
+#define SIFIVE_EDAC_DATFAIL_LOW 0x160
+#define SIFIVE_EDAC_DATFAIL_HIGH 0x164
+#define SIFIVE_EDAC_DATFAIL_COUNT 0x168
+
+#define SIFIVE_EDAC_ECCINJECTERR 0x40
+#define SIFIVE_EDAC_CONFIG 0x00
+
+#define SIFIVE_EDAC_MAX_INTR 3
+
+/************************* EDAC Parent Probe *************************/
+
+static const struct of_device_id sifive_edac_device_of_match[];
+
+static const struct of_device_id sifive_edac_of_match[] = {
+ { .compatible = "sifive,ecc-manager" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, sifive_edac_of_match);
+
+static int sifive_edac_probe(struct platform_device *pdev)
+{
+ of_platform_populate(pdev->dev.of_node, sifive_edac_device_of_match,
+ NULL, &pdev->dev);
+ return 0;
+}
+
+static struct platform_driver sifive_edac_driver = {
+ .probe = sifive_edac_probe,
+ .driver = {
+ .name = "ecc_manager",
+ .of_match_table = sifive_edac_of_match,
+ },
+};
+module_platform_driver(sifive_edac_driver);
+
+struct sifive_edac_device_prv {
+ void (*setup)(struct edac_device_ctl_info *dci);
+ irqreturn_t (*ecc_irq_handler)(int irq, void *dev_id);
+ const struct file_operations *inject_fops;
+};
+
+struct sifive_edac_device_dev {
+ void __iomem *base;
+ int irq[SIFIVE_EDAC_MAX_INTR];
+ struct sifive_edac_device_prv *data;
+ char *edac_dev_name;
+};
+
+enum {
+ dir_corr = 0,
+ data_corr,
+ data_uncorr,
+};
+
+static struct dentry *sifive_edac_test;
+
+static ssize_t sifive_edac_l2_write(struct file *file, const char __user *data,
+ size_t count, loff_t *ppos)
+{
+ struct edac_device_ctl_info *dci = file->private_data;
+ struct sifive_edac_device_dev *drvdata = dci->pvt_info;
+ unsigned int val;
+
+ if (kstrtouint_from_user(data, count, 0, &val))
+ return -EINVAL;
+ if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
+ writel(val, drvdata->base + SIFIVE_EDAC_ECCINJECTERR);
+ else
+ return -EINVAL;
+ return count;
+}
+
+static const struct file_operations sifive_edac_l2_fops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .write = sifive_edac_l2_write
+};
+
+static void setup_sifive_debug(struct edac_device_ctl_info *edac_dci,
+ const struct sifive_edac_device_prv *prv)
+{
+ struct sifive_edac_device_dev *drvdata = edac_dci->pvt_info;
+
+ if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
+ return;
+
+ sifive_edac_test = edac_debugfs_create_dir(drvdata->edac_dev_name);
+ if (!sifive_edac_test)
+ return;
+
+ if (!edac_debugfs_create_file("sifive_debug_inject_error", 0200,
+ sifive_edac_test, edac_dci,
+ prv->inject_fops))
+ debugfs_remove_recursive(sifive_edac_test);
+}
+
+static void teardown_sifive_debug(void)
+{
+ debugfs_remove_recursive(sifive_edac_test);
+}
+
+/*
+ * sifive_edac_l2_int_handler - ISR function for l2 cache controller
+ * @irq: Irq Number
+ * @device: Pointer to the edac device controller instance
+ *
+ * This routine is triggered whenever there is ECC error detected
+ *
+ * Return: Always returns IRQ_HANDLED
+ */
+static irqreturn_t sifive_edac_l2_int_handler(int irq, void *device)
+{
+ struct edac_device_ctl_info *dci =
+ (struct edac_device_ctl_info *)device;
+ struct sifive_edac_device_dev *drvdata = dci->pvt_info;
+ u32 regval, add_h, add_l;
+
+ if (irq == drvdata->irq[dir_corr]) {
+ add_h = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_HIGH);
+ add_l = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_LOW);
+ dev_err(dci->dev,
+ "DirError at address 0x%08X.%08X\n", add_h, add_l);
+ regval = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_COUNT);
+ edac_device_handle_ce(dci, 0, 0, "DirECCFix");
+ }
+ if (irq == drvdata->irq[data_corr]) {
+ add_h = readl(drvdata->base + SIFIVE_EDAC_DATFIX_HIGH);
+ add_l = readl(drvdata->base + SIFIVE_EDAC_DATFIX_LOW);
+ dev_err(dci->dev,
+ "DataError at address 0x%08X.%08X\n", add_h, add_l);
+ regval = readl(drvdata->base + SIFIVE_EDAC_DATFIX_COUNT);
+ edac_device_handle_ce(dci, 0, 0, "DatECCFix");
+ }
+ if (irq == drvdata->irq[data_uncorr]) {
+ add_h = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_HIGH);
+ add_l = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_LOW);
+ dev_err(dci->dev,
+ "DataFail at address 0x%08X.%08X\n", add_h, add_l);
+ regval = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_COUNT);
+ edac_device_handle_ue(dci, 0, 0, "DatECCFail");
+ }
+
+ return IRQ_HANDLED;
+}
+
+static void sifive_edac_l2_config_read(struct edac_device_ctl_info *dci)
+{
+ struct sifive_edac_device_dev *drvdata = dci->pvt_info;
+ u32 regval, val;
+
+ regval = readl(drvdata->base + SIFIVE_EDAC_CONFIG);
+ val = regval & 0xFF;
+ dev_info(dci->dev, "No. of Banks in the cache: %d\n", val);
+ val = (regval & 0xFF00) >> 8;
+ dev_info(dci->dev, "No. of ways per bank: %d\n", val);
+ val = (regval & 0xFF0000) >> 16;
+ dev_info(dci->dev, "Sets per bank: %llu\n", (uint64_t)1 << val);
+ val = (regval & 0xFF000000) >> 24;
+ dev_info(dci->dev,
+ "Bytes per cache block: %llu\n", (uint64_t)1 << val);
+}
+
+static const struct sifive_edac_device_prv l2ecc_data = {
+ .setup = sifive_edac_l2_config_read,
+ .inject_fops = &sifive_edac_l2_fops,
+ .ecc_irq_handler = sifive_edac_l2_int_handler,
+};
+
+/*
+ * sifive_edac_device_probe()
+ * This is a generic EDAC device driver that will support
+ * various SiFive memory devices as well as the memories
+ * for other peripherals. Module specific initialization is
+ * done by passing the function index in the device tree.
+ */
+static int sifive_edac_device_probe(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *dci;
+ struct sifive_edac_device_dev *drvdata;
+ int rc, i;
+ struct resource *res;
+ void __iomem *baseaddr;
+ struct device_node *np = pdev->dev.of_node;
+ char *ecc_name = (char *)np->name;
+ static int dev_instance;
+
+ /* Get the data from the platform device */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ baseaddr = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(baseaddr))
+ return PTR_ERR(baseaddr);
+
+ dci = edac_device_alloc_ctl_info(sizeof(*drvdata), ecc_name,
+ 1, ecc_name, 1, 1, NULL, 0,
+ dev_instance++);
+ if (IS_ERR(dci))
+ return PTR_ERR(dci);
+
+ drvdata = dci->pvt_info;
+ drvdata->base = baseaddr;
+ drvdata->edac_dev_name = ecc_name;
+ dci->dev = &pdev->dev;
+ dci->mod_name = "Sifive ECC Manager";
+ dci->ctl_name = dev_name(&pdev->dev);
+ dci->dev_name = dev_name(&pdev->dev);
+
+ /* Get driver specific data for this EDAC device */
+ drvdata->data = of_match_node(sifive_edac_device_of_match, np)->data;
+
+ setup_sifive_debug(dci, drvdata->data);
+
+ if (drvdata->data->setup)
+ drvdata->data->setup(dci);
+
+ for (i = 0; i < SIFIVE_EDAC_MAX_INTR; i++) {
+ drvdata->irq[i] = platform_get_irq(pdev, i);
+ rc = devm_request_irq(&pdev->dev, drvdata->irq[i],
+ sifive_edac_l2_int_handler, 0,
+ dev_name(&pdev->dev), (void *)dci);
+ if (rc) {
+ dev_err(&pdev->dev,
+ "Could not request IRQ %d\n", drvdata->irq[i]);
+ goto del_edac_device;
+ }
+ }
+
+ rc = edac_device_add_device(dci);
+ if (rc) {
+ dev_err(&pdev->dev, "failed to register with EDAC core\n");
+ goto del_edac_device;
+ }
+
+ return rc;
+
+del_edac_device:
+ teardown_sifive_debug();
+ edac_device_del_device(&pdev->dev);
+ edac_device_free_ctl_info(dci);
+
+ return rc;
+}
+
+static int sifive_edac_device_remove(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
+
+ teardown_sifive_debug();
+ edac_device_del_device(&pdev->dev);
+ edac_device_free_ctl_info(dci);
+
+ return 0;
+}
+
+static const struct of_device_id sifive_edac_device_of_match[] = {
+ { .compatible = "sifive,ccache0", .data = &l2ecc_data },
+ { /* end of table */ },
+};
+MODULE_DEVICE_TABLE(of, sifive_edac_device_of_match);
+
+static struct platform_driver sifive_edac_device_driver = {
+ .driver = {
+ .name = "sifive_edac_device",
+ .owner = THIS_MODULE,
+ .of_match_table = sifive_edac_device_of_match,
+ },
+ .probe = sifive_edac_device_probe,
+ .remove = sifive_edac_device_remove,
+};
+
+module_platform_driver(sifive_edac_device_driver);
+
+MODULE_AUTHOR("SiFive Inc.");
+MODULE_DESCRIPTION("SiFive EDAC driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-21 13:33 ` Borislav Petkov
0 siblings, 0 replies; 32+ messages in thread
From: Borislav Petkov @ 2019-03-21 13:33 UTC (permalink / raw)
To: Yash Shah
Cc: linux-riscv, linux-edac, palmer, paul.walmsley, linux-kernel,
robh+dt, mark.rutland, aou, mchehab, devicetree, sachin.ghadi
On Wed, Mar 20, 2019 at 05:22:08PM +0530, Yash Shah wrote:
> This EDAC driver supports:
> - Initial configuration reporting on bootup via debug logs
> - ECC event monitoring and reporting through the EDAC framework
> - ECC event injection
>
> This driver is partially based on pnd2_edac.c and altera_edac.c
>
> Initially L2 Cache controller is added as a subcomponent to
> this EDAC driver.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
...
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index e286b5b..112d9d1 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -440,6 +440,19 @@ config EDAC_ALTERA_SDMMC
> Support for error detection and correction on the
> Altera SDMMC FIFO Memory for Altera SoCs.
>
> +config EDAC_SIFIVE
> + tristate "Sifive ECC"
> + depends on RISCV
> + help
> + Support for error detection and correction on the SiFive SoCs.
> +
> +config EDAC_SIFIVE_L2
That config item is unused. Drop it.
> + bool "SiFive L2 Cache ECC"
> + depends on EDAC_SIFIVE=y
> + help
> + Support for error detection and correction of the L2 cache
> + memory on SiFive SoCs.
> +
> config EDAC_SYNOPSYS
> tristate "Synopsys DDR Memory Controller"
> depends on ARCH_ZYNQ || ARCH_ZYNQMP
...
> +/*
> + * sifive_edac_l2_int_handler - ISR function for l2 cache controller
> + * @irq: Irq Number
> + * @device: Pointer to the edac device controller instance
> + *
> + * This routine is triggered whenever there is ECC error detected
> + *
> + * Return: Always returns IRQ_HANDLED
> + */
> +static irqreturn_t sifive_edac_l2_int_handler(int irq, void *device)
> +{
> + struct edac_device_ctl_info *dci =
> + (struct edac_device_ctl_info *)device;
No ugly linebreaks like that - just let it stick out even if it is > 80
cols long.
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + u32 regval, add_h, add_l;
> +
> + if (irq == drvdata->irq[dir_corr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_LOW);
> + dev_err(dci->dev,
> + "DirError at address 0x%08X.%08X\n", add_h, add_l);
Same here and below.
> + regval = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_COUNT);
> + edac_device_handle_ce(dci, 0, 0, "DirECCFix");
> + }
> + if (irq == drvdata->irq[data_corr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DATFIX_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DATFIX_LOW);
> + dev_err(dci->dev,
> + "DataError at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DATFIX_COUNT);
> + edac_device_handle_ce(dci, 0, 0, "DatECCFix");
> + }
> + if (irq == drvdata->irq[data_uncorr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_LOW);
> + dev_err(dci->dev,
> + "DataFail at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_COUNT);
> + edac_device_handle_ue(dci, 0, 0, "DatECCFail");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void sifive_edac_l2_config_read(struct edac_device_ctl_info *dci)
> +{
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + u32 regval, val;
> +
> + regval = readl(drvdata->base + SIFIVE_EDAC_CONFIG);
> + val = regval & 0xFF;
> + dev_info(dci->dev, "No. of Banks in the cache: %d\n", val);
> + val = (regval & 0xFF00) >> 8;
> + dev_info(dci->dev, "No. of ways per bank: %d\n", val);
> + val = (regval & 0xFF0000) >> 16;
> + dev_info(dci->dev, "Sets per bank: %llu\n", (uint64_t)1 << val);
> + val = (regval & 0xFF000000) >> 24;
> + dev_info(dci->dev,
> + "Bytes per cache block: %llu\n", (uint64_t)1 << val);
> +}
> +
> +static const struct sifive_edac_device_prv l2ecc_data = {
> + .setup = sifive_edac_l2_config_read,
> + .inject_fops = &sifive_edac_l2_fops,
> + .ecc_irq_handler = sifive_edac_l2_int_handler,
> +};
> +
> +/*
> + * sifive_edac_device_probe()
> + * This is a generic EDAC device driver that will support
> + * various SiFive memory devices as well as the memories
> + * for other peripherals. Module specific initialization is
> + * done by passing the function index in the device tree.
This comment doesn't have anything to do with that function but rather
belongs at the top of this file.
> + */
> +static int sifive_edac_device_probe(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci;
> + struct sifive_edac_device_dev *drvdata;
> + int rc, i;
> + struct resource *res;
> + void __iomem *baseaddr;
> + struct device_node *np = pdev->dev.of_node;
> + char *ecc_name = (char *)np->name;
> + static int dev_instance;
Please sort function local variables declaration in a reverse christmas
tree order:
<type_a> longest_variable_name;
<type_b> shorter_var_name;
<type_c> even_shorter;
<type_d> i;
> +
> + /* Get the data from the platform device */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + baseaddr = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(baseaddr))
> + return PTR_ERR(baseaddr);
> +
> + dci = edac_device_alloc_ctl_info(sizeof(*drvdata), ecc_name,
> + 1, ecc_name, 1, 1, NULL, 0,
> + dev_instance++);
> + if (IS_ERR(dci))
> + return PTR_ERR(dci);
> +
> + drvdata = dci->pvt_info;
> + drvdata->base = baseaddr;
> + drvdata->edac_dev_name = ecc_name;
> + dci->dev = &pdev->dev;
> + dci->mod_name = "Sifive ECC Manager";
> + dci->ctl_name = dev_name(&pdev->dev);
> + dci->dev_name = dev_name(&pdev->dev);
> +
> + /* Get driver specific data for this EDAC device */
> + drvdata->data = of_match_node(sifive_edac_device_of_match, np)->data;
> +
> + setup_sifive_debug(dci, drvdata->data);
> +
> + if (drvdata->data->setup)
> + drvdata->data->setup(dci);
> +
> + for (i = 0; i < SIFIVE_EDAC_MAX_INTR; i++) {
> + drvdata->irq[i] = platform_get_irq(pdev, i);
> + rc = devm_request_irq(&pdev->dev, drvdata->irq[i],
> + sifive_edac_l2_int_handler, 0,
> + dev_name(&pdev->dev), (void *)dci);
> + if (rc) {
> + dev_err(&pdev->dev,
> + "Could not request IRQ %d\n", drvdata->irq[i]);
> + goto del_edac_device;
> + }
> + }
> +
> + rc = edac_device_add_device(dci);
> + if (rc) {
> + dev_err(&pdev->dev, "failed to register with EDAC core\n");
> + goto del_edac_device;
> + }
Call setup_sifive_debug() in the success case here so that you don't
have to teardown below.
> +
> + return rc;
> +
> +del_edac_device:
> + teardown_sifive_debug();
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
Those three can be replaced by a call to sifive_edac_device_remove() ?
Btw, you have prefixed your static functions with "sifive_edac_" which
doesn't make much sense and you have long names for no good reason.
> +
> + return rc;
> +}
> +
> +static int sifive_edac_device_remove(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
> +
> + teardown_sifive_debug();
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sifive_edac_device_of_match[] = {
> + { .compatible = "sifive,ccache0", .data = &l2ecc_data },
> + { /* end of table */ },
> +};
> +MODULE_DEVICE_TABLE(of, sifive_edac_device_of_match);
> +
> +static struct platform_driver sifive_edac_device_driver = {
> + .driver = {
> + .name = "sifive_edac_device",
"device"? I think it is clear that it is a device and thus kinda
tautological. "sifive_edac" should be enough...
Last but not least, building this driver with the riscv64 cross compilers from
here:
http://cdn.kernel.org/pub/tools/crosstool/files/bin/
fails like below. With the riscv32 compiler it fails the same.
Provided I'm not doing anything wrong, you should not be sending me
half-baked stuff which doesn't even build.
drivers/edac/sifive_edac.c: In function 'sifive_edac_device_probe':
drivers/edac/sifive_edac.c:231:16: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
drvdata->data = of_match_node(sifive_edac_device_of_match, np)->data;
^
In file included from drivers/edac/sifive_edac.c:10:
drivers/edac/sifive_edac.c: At top level:
./include/linux/module.h:130:42: error: redefinition of '__inittest'
static inline initcall_t __maybe_unused __inittest(void) \
^~~~~~~~~~
./include/linux/device.h:1647:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:293:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_device_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:130:42: note: previous definition of '__inittest' was here
static inline initcall_t __maybe_unused __inittest(void) \
^~~~~~~~~~
./include/linux/device.h:1647:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:57:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:132:6: error: redefinition of 'init_module'
int init_module(void) __copy(initfn) __attribute__((alias(#initfn)));
^~~~~~~~~~~
./include/linux/device.h:1647:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:293:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_device_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:132:6: note: previous definition of 'init_module' was here
int init_module(void) __copy(initfn) __attribute__((alias(#initfn)));
^~~~~~~~~~~
./include/linux/device.h:1647:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:57:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:136:42: error: redefinition of '__exittest'
static inline exitcall_t __maybe_unused __exittest(void) \
^~~~~~~~~~
./include/linux/device.h:1652:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:293:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_device_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:136:42: note: previous definition of '__exittest' was here
static inline exitcall_t __maybe_unused __exittest(void) \
^~~~~~~~~~
./include/linux/device.h:1652:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:57:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:138:7: error: redefinition of 'cleanup_module'
void cleanup_module(void) __copy(exitfn) __attribute__((alias(#exitfn)));
^~~~~~~~~~~~~~
./include/linux/device.h:1652:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:293:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_device_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:138:7: note: previous definition of 'cleanup_module' was here
void cleanup_module(void) __copy(exitfn) __attribute__((alias(#exitfn)));
^~~~~~~~~~~~~~
./include/linux/device.h:1652:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:57:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_driver);
^~~~~~~~~~~~~~~~~~~~~~
make[3]: *** [scripts/Makefile.build:285: drivers/edac/sifive_edac.o] Error 1
make[2]: *** [scripts/Makefile.build:489: drivers/edac] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/home/boris/kernel/linux-2.6/Makefile:1046: drivers] Error 2
make: *** [Makefile:170: sub-make] Error 2
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 32+ messages in thread
* [2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-21 13:33 ` Borislav Petkov
0 siblings, 0 replies; 32+ messages in thread
From: Borislav Petkov @ 2019-03-21 13:33 UTC (permalink / raw)
To: Yash Shah
Cc: linux-riscv, linux-edac, palmer, paul.walmsley, linux-kernel,
robh+dt, mark.rutland, aou, mchehab, devicetree, sachin.ghadi
On Wed, Mar 20, 2019 at 05:22:08PM +0530, Yash Shah wrote:
> This EDAC driver supports:
> - Initial configuration reporting on bootup via debug logs
> - ECC event monitoring and reporting through the EDAC framework
> - ECC event injection
>
> This driver is partially based on pnd2_edac.c and altera_edac.c
>
> Initially L2 Cache controller is added as a subcomponent to
> this EDAC driver.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
...
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index e286b5b..112d9d1 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -440,6 +440,19 @@ config EDAC_ALTERA_SDMMC
> Support for error detection and correction on the
> Altera SDMMC FIFO Memory for Altera SoCs.
>
> +config EDAC_SIFIVE
> + tristate "Sifive ECC"
> + depends on RISCV
> + help
> + Support for error detection and correction on the SiFive SoCs.
> +
> +config EDAC_SIFIVE_L2
That config item is unused. Drop it.
> + bool "SiFive L2 Cache ECC"
> + depends on EDAC_SIFIVE=y
> + help
> + Support for error detection and correction of the L2 cache
> + memory on SiFive SoCs.
> +
> config EDAC_SYNOPSYS
> tristate "Synopsys DDR Memory Controller"
> depends on ARCH_ZYNQ || ARCH_ZYNQMP
...
> +/*
> + * sifive_edac_l2_int_handler - ISR function for l2 cache controller
> + * @irq: Irq Number
> + * @device: Pointer to the edac device controller instance
> + *
> + * This routine is triggered whenever there is ECC error detected
> + *
> + * Return: Always returns IRQ_HANDLED
> + */
> +static irqreturn_t sifive_edac_l2_int_handler(int irq, void *device)
> +{
> + struct edac_device_ctl_info *dci =
> + (struct edac_device_ctl_info *)device;
No ugly linebreaks like that - just let it stick out even if it is > 80
cols long.
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + u32 regval, add_h, add_l;
> +
> + if (irq == drvdata->irq[dir_corr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_LOW);
> + dev_err(dci->dev,
> + "DirError at address 0x%08X.%08X\n", add_h, add_l);
Same here and below.
> + regval = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_COUNT);
> + edac_device_handle_ce(dci, 0, 0, "DirECCFix");
> + }
> + if (irq == drvdata->irq[data_corr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DATFIX_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DATFIX_LOW);
> + dev_err(dci->dev,
> + "DataError at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DATFIX_COUNT);
> + edac_device_handle_ce(dci, 0, 0, "DatECCFix");
> + }
> + if (irq == drvdata->irq[data_uncorr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_LOW);
> + dev_err(dci->dev,
> + "DataFail at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_COUNT);
> + edac_device_handle_ue(dci, 0, 0, "DatECCFail");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void sifive_edac_l2_config_read(struct edac_device_ctl_info *dci)
> +{
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + u32 regval, val;
> +
> + regval = readl(drvdata->base + SIFIVE_EDAC_CONFIG);
> + val = regval & 0xFF;
> + dev_info(dci->dev, "No. of Banks in the cache: %d\n", val);
> + val = (regval & 0xFF00) >> 8;
> + dev_info(dci->dev, "No. of ways per bank: %d\n", val);
> + val = (regval & 0xFF0000) >> 16;
> + dev_info(dci->dev, "Sets per bank: %llu\n", (uint64_t)1 << val);
> + val = (regval & 0xFF000000) >> 24;
> + dev_info(dci->dev,
> + "Bytes per cache block: %llu\n", (uint64_t)1 << val);
> +}
> +
> +static const struct sifive_edac_device_prv l2ecc_data = {
> + .setup = sifive_edac_l2_config_read,
> + .inject_fops = &sifive_edac_l2_fops,
> + .ecc_irq_handler = sifive_edac_l2_int_handler,
> +};
> +
> +/*
> + * sifive_edac_device_probe()
> + * This is a generic EDAC device driver that will support
> + * various SiFive memory devices as well as the memories
> + * for other peripherals. Module specific initialization is
> + * done by passing the function index in the device tree.
This comment doesn't have anything to do with that function but rather
belongs at the top of this file.
> + */
> +static int sifive_edac_device_probe(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci;
> + struct sifive_edac_device_dev *drvdata;
> + int rc, i;
> + struct resource *res;
> + void __iomem *baseaddr;
> + struct device_node *np = pdev->dev.of_node;
> + char *ecc_name = (char *)np->name;
> + static int dev_instance;
Please sort function local variables declaration in a reverse christmas
tree order:
<type_a> longest_variable_name;
<type_b> shorter_var_name;
<type_c> even_shorter;
<type_d> i;
> +
> + /* Get the data from the platform device */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + baseaddr = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(baseaddr))
> + return PTR_ERR(baseaddr);
> +
> + dci = edac_device_alloc_ctl_info(sizeof(*drvdata), ecc_name,
> + 1, ecc_name, 1, 1, NULL, 0,
> + dev_instance++);
> + if (IS_ERR(dci))
> + return PTR_ERR(dci);
> +
> + drvdata = dci->pvt_info;
> + drvdata->base = baseaddr;
> + drvdata->edac_dev_name = ecc_name;
> + dci->dev = &pdev->dev;
> + dci->mod_name = "Sifive ECC Manager";
> + dci->ctl_name = dev_name(&pdev->dev);
> + dci->dev_name = dev_name(&pdev->dev);
> +
> + /* Get driver specific data for this EDAC device */
> + drvdata->data = of_match_node(sifive_edac_device_of_match, np)->data;
> +
> + setup_sifive_debug(dci, drvdata->data);
> +
> + if (drvdata->data->setup)
> + drvdata->data->setup(dci);
> +
> + for (i = 0; i < SIFIVE_EDAC_MAX_INTR; i++) {
> + drvdata->irq[i] = platform_get_irq(pdev, i);
> + rc = devm_request_irq(&pdev->dev, drvdata->irq[i],
> + sifive_edac_l2_int_handler, 0,
> + dev_name(&pdev->dev), (void *)dci);
> + if (rc) {
> + dev_err(&pdev->dev,
> + "Could not request IRQ %d\n", drvdata->irq[i]);
> + goto del_edac_device;
> + }
> + }
> +
> + rc = edac_device_add_device(dci);
> + if (rc) {
> + dev_err(&pdev->dev, "failed to register with EDAC core\n");
> + goto del_edac_device;
> + }
Call setup_sifive_debug() in the success case here so that you don't
have to teardown below.
> +
> + return rc;
> +
> +del_edac_device:
> + teardown_sifive_debug();
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
Those three can be replaced by a call to sifive_edac_device_remove() ?
Btw, you have prefixed your static functions with "sifive_edac_" which
doesn't make much sense and you have long names for no good reason.
> +
> + return rc;
> +}
> +
> +static int sifive_edac_device_remove(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
> +
> + teardown_sifive_debug();
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sifive_edac_device_of_match[] = {
> + { .compatible = "sifive,ccache0", .data = &l2ecc_data },
> + { /* end of table */ },
> +};
> +MODULE_DEVICE_TABLE(of, sifive_edac_device_of_match);
> +
> +static struct platform_driver sifive_edac_device_driver = {
> + .driver = {
> + .name = "sifive_edac_device",
"device"? I think it is clear that it is a device and thus kinda
tautological. "sifive_edac" should be enough...
Last but not least, building this driver with the riscv64 cross compilers from
here:
http://cdn.kernel.org/pub/tools/crosstool/files/bin/
fails like below. With the riscv32 compiler it fails the same.
Provided I'm not doing anything wrong, you should not be sending me
half-baked stuff which doesn't even build.
drivers/edac/sifive_edac.c: In function 'sifive_edac_device_probe':
drivers/edac/sifive_edac.c:231:16: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
drvdata->data = of_match_node(sifive_edac_device_of_match, np)->data;
^
In file included from drivers/edac/sifive_edac.c:10:
drivers/edac/sifive_edac.c: At top level:
./include/linux/module.h:130:42: error: redefinition of '__inittest'
static inline initcall_t __maybe_unused __inittest(void) \
^~~~~~~~~~
./include/linux/device.h:1647:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:293:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_device_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:130:42: note: previous definition of '__inittest' was here
static inline initcall_t __maybe_unused __inittest(void) \
^~~~~~~~~~
./include/linux/device.h:1647:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:57:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:132:6: error: redefinition of 'init_module'
int init_module(void) __copy(initfn) __attribute__((alias(#initfn)));
^~~~~~~~~~~
./include/linux/device.h:1647:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:293:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_device_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:132:6: note: previous definition of 'init_module' was here
int init_module(void) __copy(initfn) __attribute__((alias(#initfn)));
^~~~~~~~~~~
./include/linux/device.h:1647:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:57:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:136:42: error: redefinition of '__exittest'
static inline exitcall_t __maybe_unused __exittest(void) \
^~~~~~~~~~
./include/linux/device.h:1652:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:293:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_device_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:136:42: note: previous definition of '__exittest' was here
static inline exitcall_t __maybe_unused __exittest(void) \
^~~~~~~~~~
./include/linux/device.h:1652:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:57:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:138:7: error: redefinition of 'cleanup_module'
void cleanup_module(void) __copy(exitfn) __attribute__((alias(#exitfn)));
^~~~~~~~~~~~~~
./include/linux/device.h:1652:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:293:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_device_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:138:7: note: previous definition of 'cleanup_module' was here
void cleanup_module(void) __copy(exitfn) __attribute__((alias(#exitfn)));
^~~~~~~~~~~~~~
./include/linux/device.h:1652:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:57:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_driver);
^~~~~~~~~~~~~~~~~~~~~~
make[3]: *** [scripts/Makefile.build:285: drivers/edac/sifive_edac.o] Error 1
make[2]: *** [scripts/Makefile.build:489: drivers/edac] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/home/boris/kernel/linux-2.6/Makefile:1046: drivers] Error 2
make: *** [Makefile:170: sub-make] Error 2
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-21 13:33 ` Borislav Petkov
0 siblings, 0 replies; 32+ messages in thread
From: Borislav Petkov @ 2019-03-21 13:33 UTC (permalink / raw)
To: Yash Shah
Cc: mark.rutland, devicetree, aou, palmer, linux-kernel,
sachin.ghadi, robh+dt, paul.walmsley, linux-riscv, mchehab,
linux-edac
On Wed, Mar 20, 2019 at 05:22:08PM +0530, Yash Shah wrote:
> This EDAC driver supports:
> - Initial configuration reporting on bootup via debug logs
> - ECC event monitoring and reporting through the EDAC framework
> - ECC event injection
>
> This driver is partially based on pnd2_edac.c and altera_edac.c
>
> Initially L2 Cache controller is added as a subcomponent to
> this EDAC driver.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
...
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index e286b5b..112d9d1 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -440,6 +440,19 @@ config EDAC_ALTERA_SDMMC
> Support for error detection and correction on the
> Altera SDMMC FIFO Memory for Altera SoCs.
>
> +config EDAC_SIFIVE
> + tristate "Sifive ECC"
> + depends on RISCV
> + help
> + Support for error detection and correction on the SiFive SoCs.
> +
> +config EDAC_SIFIVE_L2
That config item is unused. Drop it.
> + bool "SiFive L2 Cache ECC"
> + depends on EDAC_SIFIVE=y
> + help
> + Support for error detection and correction of the L2 cache
> + memory on SiFive SoCs.
> +
> config EDAC_SYNOPSYS
> tristate "Synopsys DDR Memory Controller"
> depends on ARCH_ZYNQ || ARCH_ZYNQMP
...
> +/*
> + * sifive_edac_l2_int_handler - ISR function for l2 cache controller
> + * @irq: Irq Number
> + * @device: Pointer to the edac device controller instance
> + *
> + * This routine is triggered whenever there is ECC error detected
> + *
> + * Return: Always returns IRQ_HANDLED
> + */
> +static irqreturn_t sifive_edac_l2_int_handler(int irq, void *device)
> +{
> + struct edac_device_ctl_info *dci =
> + (struct edac_device_ctl_info *)device;
No ugly linebreaks like that - just let it stick out even if it is > 80
cols long.
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + u32 regval, add_h, add_l;
> +
> + if (irq == drvdata->irq[dir_corr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_LOW);
> + dev_err(dci->dev,
> + "DirError at address 0x%08X.%08X\n", add_h, add_l);
Same here and below.
> + regval = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_COUNT);
> + edac_device_handle_ce(dci, 0, 0, "DirECCFix");
> + }
> + if (irq == drvdata->irq[data_corr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DATFIX_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DATFIX_LOW);
> + dev_err(dci->dev,
> + "DataError at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DATFIX_COUNT);
> + edac_device_handle_ce(dci, 0, 0, "DatECCFix");
> + }
> + if (irq == drvdata->irq[data_uncorr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_LOW);
> + dev_err(dci->dev,
> + "DataFail at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_COUNT);
> + edac_device_handle_ue(dci, 0, 0, "DatECCFail");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void sifive_edac_l2_config_read(struct edac_device_ctl_info *dci)
> +{
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + u32 regval, val;
> +
> + regval = readl(drvdata->base + SIFIVE_EDAC_CONFIG);
> + val = regval & 0xFF;
> + dev_info(dci->dev, "No. of Banks in the cache: %d\n", val);
> + val = (regval & 0xFF00) >> 8;
> + dev_info(dci->dev, "No. of ways per bank: %d\n", val);
> + val = (regval & 0xFF0000) >> 16;
> + dev_info(dci->dev, "Sets per bank: %llu\n", (uint64_t)1 << val);
> + val = (regval & 0xFF000000) >> 24;
> + dev_info(dci->dev,
> + "Bytes per cache block: %llu\n", (uint64_t)1 << val);
> +}
> +
> +static const struct sifive_edac_device_prv l2ecc_data = {
> + .setup = sifive_edac_l2_config_read,
> + .inject_fops = &sifive_edac_l2_fops,
> + .ecc_irq_handler = sifive_edac_l2_int_handler,
> +};
> +
> +/*
> + * sifive_edac_device_probe()
> + * This is a generic EDAC device driver that will support
> + * various SiFive memory devices as well as the memories
> + * for other peripherals. Module specific initialization is
> + * done by passing the function index in the device tree.
This comment doesn't have anything to do with that function but rather
belongs at the top of this file.
> + */
> +static int sifive_edac_device_probe(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci;
> + struct sifive_edac_device_dev *drvdata;
> + int rc, i;
> + struct resource *res;
> + void __iomem *baseaddr;
> + struct device_node *np = pdev->dev.of_node;
> + char *ecc_name = (char *)np->name;
> + static int dev_instance;
Please sort function local variables declaration in a reverse christmas
tree order:
<type_a> longest_variable_name;
<type_b> shorter_var_name;
<type_c> even_shorter;
<type_d> i;
> +
> + /* Get the data from the platform device */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + baseaddr = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(baseaddr))
> + return PTR_ERR(baseaddr);
> +
> + dci = edac_device_alloc_ctl_info(sizeof(*drvdata), ecc_name,
> + 1, ecc_name, 1, 1, NULL, 0,
> + dev_instance++);
> + if (IS_ERR(dci))
> + return PTR_ERR(dci);
> +
> + drvdata = dci->pvt_info;
> + drvdata->base = baseaddr;
> + drvdata->edac_dev_name = ecc_name;
> + dci->dev = &pdev->dev;
> + dci->mod_name = "Sifive ECC Manager";
> + dci->ctl_name = dev_name(&pdev->dev);
> + dci->dev_name = dev_name(&pdev->dev);
> +
> + /* Get driver specific data for this EDAC device */
> + drvdata->data = of_match_node(sifive_edac_device_of_match, np)->data;
> +
> + setup_sifive_debug(dci, drvdata->data);
> +
> + if (drvdata->data->setup)
> + drvdata->data->setup(dci);
> +
> + for (i = 0; i < SIFIVE_EDAC_MAX_INTR; i++) {
> + drvdata->irq[i] = platform_get_irq(pdev, i);
> + rc = devm_request_irq(&pdev->dev, drvdata->irq[i],
> + sifive_edac_l2_int_handler, 0,
> + dev_name(&pdev->dev), (void *)dci);
> + if (rc) {
> + dev_err(&pdev->dev,
> + "Could not request IRQ %d\n", drvdata->irq[i]);
> + goto del_edac_device;
> + }
> + }
> +
> + rc = edac_device_add_device(dci);
> + if (rc) {
> + dev_err(&pdev->dev, "failed to register with EDAC core\n");
> + goto del_edac_device;
> + }
Call setup_sifive_debug() in the success case here so that you don't
have to teardown below.
> +
> + return rc;
> +
> +del_edac_device:
> + teardown_sifive_debug();
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
Those three can be replaced by a call to sifive_edac_device_remove() ?
Btw, you have prefixed your static functions with "sifive_edac_" which
doesn't make much sense and you have long names for no good reason.
> +
> + return rc;
> +}
> +
> +static int sifive_edac_device_remove(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
> +
> + teardown_sifive_debug();
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sifive_edac_device_of_match[] = {
> + { .compatible = "sifive,ccache0", .data = &l2ecc_data },
> + { /* end of table */ },
> +};
> +MODULE_DEVICE_TABLE(of, sifive_edac_device_of_match);
> +
> +static struct platform_driver sifive_edac_device_driver = {
> + .driver = {
> + .name = "sifive_edac_device",
"device"? I think it is clear that it is a device and thus kinda
tautological. "sifive_edac" should be enough...
Last but not least, building this driver with the riscv64 cross compilers from
here:
http://cdn.kernel.org/pub/tools/crosstool/files/bin/
fails like below. With the riscv32 compiler it fails the same.
Provided I'm not doing anything wrong, you should not be sending me
half-baked stuff which doesn't even build.
drivers/edac/sifive_edac.c: In function 'sifive_edac_device_probe':
drivers/edac/sifive_edac.c:231:16: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
drvdata->data = of_match_node(sifive_edac_device_of_match, np)->data;
^
In file included from drivers/edac/sifive_edac.c:10:
drivers/edac/sifive_edac.c: At top level:
./include/linux/module.h:130:42: error: redefinition of '__inittest'
static inline initcall_t __maybe_unused __inittest(void) \
^~~~~~~~~~
./include/linux/device.h:1647:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:293:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_device_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:130:42: note: previous definition of '__inittest' was here
static inline initcall_t __maybe_unused __inittest(void) \
^~~~~~~~~~
./include/linux/device.h:1647:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:57:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:132:6: error: redefinition of 'init_module'
int init_module(void) __copy(initfn) __attribute__((alias(#initfn)));
^~~~~~~~~~~
./include/linux/device.h:1647:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:293:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_device_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:132:6: note: previous definition of 'init_module' was here
int init_module(void) __copy(initfn) __attribute__((alias(#initfn)));
^~~~~~~~~~~
./include/linux/device.h:1647:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:57:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:136:42: error: redefinition of '__exittest'
static inline exitcall_t __maybe_unused __exittest(void) \
^~~~~~~~~~
./include/linux/device.h:1652:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:293:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_device_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:136:42: note: previous definition of '__exittest' was here
static inline exitcall_t __maybe_unused __exittest(void) \
^~~~~~~~~~
./include/linux/device.h:1652:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:57:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:138:7: error: redefinition of 'cleanup_module'
void cleanup_module(void) __copy(exitfn) __attribute__((alias(#exitfn)));
^~~~~~~~~~~~~~
./include/linux/device.h:1652:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:293:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_device_driver);
^~~~~~~~~~~~~~~~~~~~~~
./include/linux/module.h:138:7: note: previous definition of 'cleanup_module' was here
void cleanup_module(void) __copy(exitfn) __attribute__((alias(#exitfn)));
^~~~~~~~~~~~~~
./include/linux/device.h:1652:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~~~~~~~~~
./include/linux/platform_device.h:233:2: note: in expansion of macro 'module_driver'
module_driver(__platform_driver, platform_driver_register, \
^~~~~~~~~~~~~
drivers/edac/sifive_edac.c:57:1: note: in expansion of macro 'module_platform_driver'
module_platform_driver(sifive_edac_driver);
^~~~~~~~~~~~~~~~~~~~~~
make[3]: *** [scripts/Makefile.build:285: drivers/edac/sifive_edac.o] Error 1
make[2]: *** [scripts/Makefile.build:489: drivers/edac] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/home/boris/kernel/linux-2.6/Makefile:1046: drivers] Error 2
make: *** [Makefile:170: sub-make] Error 2
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-22 6:00 ` Yash Shah
0 siblings, 0 replies; 32+ messages in thread
From: Yash Shah @ 2019-03-22 6:00 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-riscv, linux-edac, Palmer Dabbelt, Paul Walmsley,
linux-kernel, robh+dt, mark.rutland, aou, mchehab, devicetree,
Sachin Ghadi
On Thu, Mar 21, 2019 at 7:03 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Mar 20, 2019 at 05:22:08PM +0530, Yash Shah wrote:
> > This EDAC driver supports:
> > - Initial configuration reporting on bootup via debug logs
> > - ECC event monitoring and reporting through the EDAC framework
> > - ECC event injection
> >
> > This driver is partially based on pnd2_edac.c and altera_edac.c
> >
> > Initially L2 Cache controller is added as a subcomponent to
> > this EDAC driver.
> >
> > Signed-off-by: Yash Shah <yash.shah@sifive.com>
>
> ...
>
> > diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> > index e286b5b..112d9d1 100644
> > --- a/drivers/edac/Kconfig
> > +++ b/drivers/edac/Kconfig
> > @@ -440,6 +440,19 @@ config EDAC_ALTERA_SDMMC
> > Support for error detection and correction on the
> > Altera SDMMC FIFO Memory for Altera SoCs.
> >
> > +config EDAC_SIFIVE
> > + tristate "Sifive ECC"
> > + depends on RISCV
> > + help
> > + Support for error detection and correction on the SiFive SoCs.
> > +
> > +config EDAC_SIFIVE_L2
>
> That config item is unused. Drop it.
Sure, will drop it.
> > +static irqreturn_t sifive_edac_l2_int_handler(int irq, void *device)
> > +{
> > + struct edac_device_ctl_info *dci =
> > + (struct edac_device_ctl_info *)device;
>
> No ugly linebreaks like that - just let it stick out even if it is > 80
> cols long.
Ok. Will let it stick out.
> > +
> > +/*
> > + * sifive_edac_device_probe()
> > + * This is a generic EDAC device driver that will support
> > + * various SiFive memory devices as well as the memories
> > + * for other peripherals. Module specific initialization is
> > + * done by passing the function index in the device tree.
>
> This comment doesn't have anything to do with that function but rather
> belongs at the top of this file.
Will move it at the top.
>
> > + */
> > +static int sifive_edac_device_probe(struct platform_device *pdev)
> > +{
> > + struct edac_device_ctl_info *dci;
> > + struct sifive_edac_device_dev *drvdata;
> > + int rc, i;
> > + struct resource *res;
> > + void __iomem *baseaddr;
> > + struct device_node *np = pdev->dev.of_node;
> > + char *ecc_name = (char *)np->name;
> > + static int dev_instance;
>
> Please sort function local variables declaration in a reverse christmas
> tree order:
>
> <type_a> longest_variable_name;
> <type_b> shorter_var_name;
> <type_c> even_shorter;
> <type_d> i;
>
Sure, will be done.
> > +
> > + rc = edac_device_add_device(dci);
> > + if (rc) {
> > + dev_err(&pdev->dev, "failed to register with EDAC core\n");
> > + goto del_edac_device;
> > + }
>
> Call setup_sifive_debug() in the success case here so that you don't
> have to teardown below.
Ok.
>
> > +
> > + return rc;
> > +
> > +del_edac_device:
> > + teardown_sifive_debug();
> > + edac_device_del_device(&pdev->dev);
> > + edac_device_free_ctl_info(dci);
>
> Those three can be replaced by a call to sifive_edac_device_remove() ?
Since now there won't be a need to teardown, I will stick with this
(bottom two function calls).
>
> Btw, you have prefixed your static functions with "sifive_edac_" which
> doesn't make much sense and you have long names for no good reason.
Will remove the prefix "sifive_edac_" on static functions wherever feasible.
> > +static struct platform_driver sifive_edac_device_driver = {
> > + .driver = {
> > + .name = "sifive_edac_device",
>
> "device"? I think it is clear that it is a device and thus kinda
> tautological. "sifive_edac" should be enough...
Sure. Will keep it just "sifive_edac"
>
> Last but not least, building this driver with the riscv64 cross compilers from
> here:
>
> http://cdn.kernel.org/pub/tools/crosstool/files/bin/
>
> fails like below. With the riscv32 compiler it fails the same.
>
> Provided I'm not doing anything wrong, you should not be sending me
> half-baked stuff which doesn't even build.
Sorry about that. It fails if configured as 'module'.
I intended this driver to be built-in only hence I never came across
these errors while testing.
I somehow missed it in Kconfig file.
I will make the correction in Kconfig (change 'tristate' to 'bool')
and make sure everything builds fine.
> --
> Regards/Gruss,
> Boris.
Thanks for your comments.
- Yash
>
> ECO tip #101: Trim your mails when you reply.
> --
^ permalink raw reply [flat|nested] 32+ messages in thread
* [2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-22 6:00 ` Yash Shah
0 siblings, 0 replies; 32+ messages in thread
From: Yash Shah @ 2019-03-22 6:00 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-riscv, linux-edac, Palmer Dabbelt, Paul Walmsley,
linux-kernel, robh+dt, mark.rutland, aou, mchehab, devicetree,
Sachin Ghadi
On Thu, Mar 21, 2019 at 7:03 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Mar 20, 2019 at 05:22:08PM +0530, Yash Shah wrote:
> > This EDAC driver supports:
> > - Initial configuration reporting on bootup via debug logs
> > - ECC event monitoring and reporting through the EDAC framework
> > - ECC event injection
> >
> > This driver is partially based on pnd2_edac.c and altera_edac.c
> >
> > Initially L2 Cache controller is added as a subcomponent to
> > this EDAC driver.
> >
> > Signed-off-by: Yash Shah <yash.shah@sifive.com>
>
> ...
>
> > diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> > index e286b5b..112d9d1 100644
> > --- a/drivers/edac/Kconfig
> > +++ b/drivers/edac/Kconfig
> > @@ -440,6 +440,19 @@ config EDAC_ALTERA_SDMMC
> > Support for error detection and correction on the
> > Altera SDMMC FIFO Memory for Altera SoCs.
> >
> > +config EDAC_SIFIVE
> > + tristate "Sifive ECC"
> > + depends on RISCV
> > + help
> > + Support for error detection and correction on the SiFive SoCs.
> > +
> > +config EDAC_SIFIVE_L2
>
> That config item is unused. Drop it.
Sure, will drop it.
> > +static irqreturn_t sifive_edac_l2_int_handler(int irq, void *device)
> > +{
> > + struct edac_device_ctl_info *dci =
> > + (struct edac_device_ctl_info *)device;
>
> No ugly linebreaks like that - just let it stick out even if it is > 80
> cols long.
Ok. Will let it stick out.
> > +
> > +/*
> > + * sifive_edac_device_probe()
> > + * This is a generic EDAC device driver that will support
> > + * various SiFive memory devices as well as the memories
> > + * for other peripherals. Module specific initialization is
> > + * done by passing the function index in the device tree.
>
> This comment doesn't have anything to do with that function but rather
> belongs at the top of this file.
Will move it at the top.
>
> > + */
> > +static int sifive_edac_device_probe(struct platform_device *pdev)
> > +{
> > + struct edac_device_ctl_info *dci;
> > + struct sifive_edac_device_dev *drvdata;
> > + int rc, i;
> > + struct resource *res;
> > + void __iomem *baseaddr;
> > + struct device_node *np = pdev->dev.of_node;
> > + char *ecc_name = (char *)np->name;
> > + static int dev_instance;
>
> Please sort function local variables declaration in a reverse christmas
> tree order:
>
> <type_a> longest_variable_name;
> <type_b> shorter_var_name;
> <type_c> even_shorter;
> <type_d> i;
>
Sure, will be done.
> > +
> > + rc = edac_device_add_device(dci);
> > + if (rc) {
> > + dev_err(&pdev->dev, "failed to register with EDAC core\n");
> > + goto del_edac_device;
> > + }
>
> Call setup_sifive_debug() in the success case here so that you don't
> have to teardown below.
Ok.
>
> > +
> > + return rc;
> > +
> > +del_edac_device:
> > + teardown_sifive_debug();
> > + edac_device_del_device(&pdev->dev);
> > + edac_device_free_ctl_info(dci);
>
> Those three can be replaced by a call to sifive_edac_device_remove() ?
Since now there won't be a need to teardown, I will stick with this
(bottom two function calls).
>
> Btw, you have prefixed your static functions with "sifive_edac_" which
> doesn't make much sense and you have long names for no good reason.
Will remove the prefix "sifive_edac_" on static functions wherever feasible.
> > +static struct platform_driver sifive_edac_device_driver = {
> > + .driver = {
> > + .name = "sifive_edac_device",
>
> "device"? I think it is clear that it is a device and thus kinda
> tautological. "sifive_edac" should be enough...
Sure. Will keep it just "sifive_edac"
>
> Last but not least, building this driver with the riscv64 cross compilers from
> here:
>
> http://cdn.kernel.org/pub/tools/crosstool/files/bin/
>
> fails like below. With the riscv32 compiler it fails the same.
>
> Provided I'm not doing anything wrong, you should not be sending me
> half-baked stuff which doesn't even build.
Sorry about that. It fails if configured as 'module'.
I intended this driver to be built-in only hence I never came across
these errors while testing.
I somehow missed it in Kconfig file.
I will make the correction in Kconfig (change 'tristate' to 'bool')
and make sure everything builds fine.
> --
> Regards/Gruss,
> Boris.
Thanks for your comments.
- Yash
>
> ECO tip #101: Trim your mails when you reply.
> --
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-22 6:00 ` Yash Shah
0 siblings, 0 replies; 32+ messages in thread
From: Yash Shah @ 2019-03-22 6:00 UTC (permalink / raw)
To: Borislav Petkov
Cc: mark.rutland, devicetree, aou, Palmer Dabbelt, linux-kernel,
Sachin Ghadi, robh+dt, Paul Walmsley, linux-riscv, mchehab,
linux-edac
On Thu, Mar 21, 2019 at 7:03 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Mar 20, 2019 at 05:22:08PM +0530, Yash Shah wrote:
> > This EDAC driver supports:
> > - Initial configuration reporting on bootup via debug logs
> > - ECC event monitoring and reporting through the EDAC framework
> > - ECC event injection
> >
> > This driver is partially based on pnd2_edac.c and altera_edac.c
> >
> > Initially L2 Cache controller is added as a subcomponent to
> > this EDAC driver.
> >
> > Signed-off-by: Yash Shah <yash.shah@sifive.com>
>
> ...
>
> > diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> > index e286b5b..112d9d1 100644
> > --- a/drivers/edac/Kconfig
> > +++ b/drivers/edac/Kconfig
> > @@ -440,6 +440,19 @@ config EDAC_ALTERA_SDMMC
> > Support for error detection and correction on the
> > Altera SDMMC FIFO Memory for Altera SoCs.
> >
> > +config EDAC_SIFIVE
> > + tristate "Sifive ECC"
> > + depends on RISCV
> > + help
> > + Support for error detection and correction on the SiFive SoCs.
> > +
> > +config EDAC_SIFIVE_L2
>
> That config item is unused. Drop it.
Sure, will drop it.
> > +static irqreturn_t sifive_edac_l2_int_handler(int irq, void *device)
> > +{
> > + struct edac_device_ctl_info *dci =
> > + (struct edac_device_ctl_info *)device;
>
> No ugly linebreaks like that - just let it stick out even if it is > 80
> cols long.
Ok. Will let it stick out.
> > +
> > +/*
> > + * sifive_edac_device_probe()
> > + * This is a generic EDAC device driver that will support
> > + * various SiFive memory devices as well as the memories
> > + * for other peripherals. Module specific initialization is
> > + * done by passing the function index in the device tree.
>
> This comment doesn't have anything to do with that function but rather
> belongs at the top of this file.
Will move it at the top.
>
> > + */
> > +static int sifive_edac_device_probe(struct platform_device *pdev)
> > +{
> > + struct edac_device_ctl_info *dci;
> > + struct sifive_edac_device_dev *drvdata;
> > + int rc, i;
> > + struct resource *res;
> > + void __iomem *baseaddr;
> > + struct device_node *np = pdev->dev.of_node;
> > + char *ecc_name = (char *)np->name;
> > + static int dev_instance;
>
> Please sort function local variables declaration in a reverse christmas
> tree order:
>
> <type_a> longest_variable_name;
> <type_b> shorter_var_name;
> <type_c> even_shorter;
> <type_d> i;
>
Sure, will be done.
> > +
> > + rc = edac_device_add_device(dci);
> > + if (rc) {
> > + dev_err(&pdev->dev, "failed to register with EDAC core\n");
> > + goto del_edac_device;
> > + }
>
> Call setup_sifive_debug() in the success case here so that you don't
> have to teardown below.
Ok.
>
> > +
> > + return rc;
> > +
> > +del_edac_device:
> > + teardown_sifive_debug();
> > + edac_device_del_device(&pdev->dev);
> > + edac_device_free_ctl_info(dci);
>
> Those three can be replaced by a call to sifive_edac_device_remove() ?
Since now there won't be a need to teardown, I will stick with this
(bottom two function calls).
>
> Btw, you have prefixed your static functions with "sifive_edac_" which
> doesn't make much sense and you have long names for no good reason.
Will remove the prefix "sifive_edac_" on static functions wherever feasible.
> > +static struct platform_driver sifive_edac_device_driver = {
> > + .driver = {
> > + .name = "sifive_edac_device",
>
> "device"? I think it is clear that it is a device and thus kinda
> tautological. "sifive_edac" should be enough...
Sure. Will keep it just "sifive_edac"
>
> Last but not least, building this driver with the riscv64 cross compilers from
> here:
>
> http://cdn.kernel.org/pub/tools/crosstool/files/bin/
>
> fails like below. With the riscv32 compiler it fails the same.
>
> Provided I'm not doing anything wrong, you should not be sending me
> half-baked stuff which doesn't even build.
Sorry about that. It fails if configured as 'module'.
I intended this driver to be built-in only hence I never came across
these errors while testing.
I somehow missed it in Kconfig file.
I will make the correction in Kconfig (change 'tristate' to 'bool')
and make sure everything builds fine.
> --
> Regards/Gruss,
> Boris.
Thanks for your comments.
- Yash
>
> ECO tip #101: Trim your mails when you reply.
> --
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] edac: sifive: Add DT documentation for SiFive EDAC driver and subcomponent
@ 2019-03-25 0:20 ` Paul Walmsley
0 siblings, 0 replies; 32+ messages in thread
From: Paul Walmsley @ 2019-03-25 0:20 UTC (permalink / raw)
To: Yash Shah
Cc: linux-riscv, linux-edac, palmer, paul.walmsley, linux-kernel,
robh+dt, mark.rutland, aou, bp, mchehab, devicetree,
sachin.ghadi
Hi Yash,
On Wed, 20 Mar 2019, Yash Shah wrote:
> DT documentation for EDAC driver added.
> DT documentation for subcomponent L2 cache controller also added.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> ---
> .../devicetree/bindings/edac/sifive-edac.txt | 40 ++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/edac/sifive-edac.txt
>
> diff --git a/Documentation/devicetree/bindings/edac/sifive-edac.txt b/Documentation/devicetree/bindings/edac/sifive-edac.txt
> new file mode 100644
> index 0000000..c0e3ac7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/edac/sifive-edac.txt
> @@ -0,0 +1,40 @@
> +SiFive ECC Manager
> +This driver uses the EDAC framework to implement the SiFive ECC Manager.
> +
> +Required Properties:
> +- compatible : Should be "sifive,ecc-manager"
As you've probably seen, this is being discussed in a separate thread with
Borislav, but it would be ideal if we could avoid adding DT nodes for
non-existent hardware. Let's see what the outcome of that other thread
will be.
> +L2 Cache ECC
> +Required Properties:
> +- compatible: Should be "sifive,<chip>-ccache" and "sifive,ccache<version>".
We should only use chip-specific compatible strings for this IP block,
like "sifive,fu540-c000-ccache". Let's not use "sifive,ccache*" here for
now.
> + Supported compatible strings are:
> + "sifive,fu540-c000-ccache" for the SiFive cache controller v0 as integrated
> + onto the SiFive FU540 chip, and "sifive,ccache0" for the SiFive
> + cache controller v0 IP block with no chip integration tweaks.
Same comment here.
> + Please refer to sifive-blocks-ip-versioning.txt for details
> +- interrupt-parent: Must be core interrupt controller
> +- interrupts: Must contain 3 entries (DirError, DataError and DataFail signals
> +- reg: Physical base address and size of L2 cache controller registers map
> + A second range can indicate L2 Loosely Integrated Memory
> +
> +Example:
> +
> +eccmgr: eccmgr {
> + compatible = "sifive,ecc-manager";
See above
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + l2-ecc@2010000 {
> + compatible = "sifive,fu540-c000-ccache", "sifive,ccache0";
And as above
> + interrupt-parent = <&plic0>;
> + interrupts = <1 2 3>;
> + reg = <0x0 0x2010000 0x0 0x1000 0x0 0x8000000 0x0 0x2000000>;
> + };
> +};
- Paul
^ permalink raw reply [flat|nested] 32+ messages in thread
* [1/2] edac: sifive: Add DT documentation for SiFive EDAC driver and subcomponent
@ 2019-03-25 0:20 ` Paul Walmsley
0 siblings, 0 replies; 32+ messages in thread
From: Paul Walmsley @ 2019-03-25 0:20 UTC (permalink / raw)
To: Yash Shah
Cc: linux-riscv, linux-edac, palmer, paul.walmsley, linux-kernel,
robh+dt, mark.rutland, aou, bp, mchehab, devicetree,
sachin.ghadi
Hi Yash,
On Wed, 20 Mar 2019, Yash Shah wrote:
> DT documentation for EDAC driver added.
> DT documentation for subcomponent L2 cache controller also added.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> ---
> .../devicetree/bindings/edac/sifive-edac.txt | 40 ++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/edac/sifive-edac.txt
>
> diff --git a/Documentation/devicetree/bindings/edac/sifive-edac.txt b/Documentation/devicetree/bindings/edac/sifive-edac.txt
> new file mode 100644
> index 0000000..c0e3ac7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/edac/sifive-edac.txt
> @@ -0,0 +1,40 @@
> +SiFive ECC Manager
> +This driver uses the EDAC framework to implement the SiFive ECC Manager.
> +
> +Required Properties:
> +- compatible : Should be "sifive,ecc-manager"
As you've probably seen, this is being discussed in a separate thread with
Borislav, but it would be ideal if we could avoid adding DT nodes for
non-existent hardware. Let's see what the outcome of that other thread
will be.
> +L2 Cache ECC
> +Required Properties:
> +- compatible: Should be "sifive,<chip>-ccache" and "sifive,ccache<version>".
We should only use chip-specific compatible strings for this IP block,
like "sifive,fu540-c000-ccache". Let's not use "sifive,ccache*" here for
now.
> + Supported compatible strings are:
> + "sifive,fu540-c000-ccache" for the SiFive cache controller v0 as integrated
> + onto the SiFive FU540 chip, and "sifive,ccache0" for the SiFive
> + cache controller v0 IP block with no chip integration tweaks.
Same comment here.
> + Please refer to sifive-blocks-ip-versioning.txt for details
> +- interrupt-parent: Must be core interrupt controller
> +- interrupts: Must contain 3 entries (DirError, DataError and DataFail signals
> +- reg: Physical base address and size of L2 cache controller registers map
> + A second range can indicate L2 Loosely Integrated Memory
> +
> +Example:
> +
> +eccmgr: eccmgr {
> + compatible = "sifive,ecc-manager";
See above
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + l2-ecc@2010000 {
> + compatible = "sifive,fu540-c000-ccache", "sifive,ccache0";
And as above
> + interrupt-parent = <&plic0>;
> + interrupts = <1 2 3>;
> + reg = <0x0 0x2010000 0x0 0x1000 0x0 0x8000000 0x0 0x2000000>;
> + };
> +};
- Paul
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] edac: sifive: Add DT documentation for SiFive EDAC driver and subcomponent
@ 2019-03-25 0:20 ` Paul Walmsley
0 siblings, 0 replies; 32+ messages in thread
From: Paul Walmsley @ 2019-03-25 0:20 UTC (permalink / raw)
To: Yash Shah
Cc: mark.rutland, devicetree, aou, palmer, linux-kernel,
sachin.ghadi, robh+dt, bp, paul.walmsley, linux-riscv, mchehab,
linux-edac
Hi Yash,
On Wed, 20 Mar 2019, Yash Shah wrote:
> DT documentation for EDAC driver added.
> DT documentation for subcomponent L2 cache controller also added.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> ---
> .../devicetree/bindings/edac/sifive-edac.txt | 40 ++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/edac/sifive-edac.txt
>
> diff --git a/Documentation/devicetree/bindings/edac/sifive-edac.txt b/Documentation/devicetree/bindings/edac/sifive-edac.txt
> new file mode 100644
> index 0000000..c0e3ac7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/edac/sifive-edac.txt
> @@ -0,0 +1,40 @@
> +SiFive ECC Manager
> +This driver uses the EDAC framework to implement the SiFive ECC Manager.
> +
> +Required Properties:
> +- compatible : Should be "sifive,ecc-manager"
As you've probably seen, this is being discussed in a separate thread with
Borislav, but it would be ideal if we could avoid adding DT nodes for
non-existent hardware. Let's see what the outcome of that other thread
will be.
> +L2 Cache ECC
> +Required Properties:
> +- compatible: Should be "sifive,<chip>-ccache" and "sifive,ccache<version>".
We should only use chip-specific compatible strings for this IP block,
like "sifive,fu540-c000-ccache". Let's not use "sifive,ccache*" here for
now.
> + Supported compatible strings are:
> + "sifive,fu540-c000-ccache" for the SiFive cache controller v0 as integrated
> + onto the SiFive FU540 chip, and "sifive,ccache0" for the SiFive
> + cache controller v0 IP block with no chip integration tweaks.
Same comment here.
> + Please refer to sifive-blocks-ip-versioning.txt for details
> +- interrupt-parent: Must be core interrupt controller
> +- interrupts: Must contain 3 entries (DirError, DataError and DataFail signals
> +- reg: Physical base address and size of L2 cache controller registers map
> + A second range can indicate L2 Loosely Integrated Memory
> +
> +Example:
> +
> +eccmgr: eccmgr {
> + compatible = "sifive,ecc-manager";
See above
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + l2-ecc@2010000 {
> + compatible = "sifive,fu540-c000-ccache", "sifive,ccache0";
And as above
> + interrupt-parent = <&plic0>;
> + interrupts = <1 2 3>;
> + reg = <0x0 0x2010000 0x0 0x1000 0x0 0x8000000 0x0 0x2000000>;
> + };
> +};
- Paul
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-25 0:23 ` Paul Walmsley
0 siblings, 0 replies; 32+ messages in thread
From: Paul Walmsley @ 2019-03-25 0:23 UTC (permalink / raw)
To: Yash Shah
Cc: linux-riscv, linux-edac, palmer, paul.walmsley, linux-kernel,
robh+dt, mark.rutland, aou, bp, mchehab, devicetree,
sachin.ghadi
Hi Yash,
Just a few brief comments here:
On Wed, 20 Mar 2019, Yash Shah wrote:
> This EDAC driver supports:
> - Initial configuration reporting on bootup via debug logs
> - ECC event monitoring and reporting through the EDAC framework
> - ECC event injection
>
It's probably worth mentioning here, as you did in the subject line, that
this is for the FU540-C000 platform.
> This driver is partially based on pnd2_edac.c and altera_edac.c
>
> Initially L2 Cache controller is added as a subcomponent to
> this EDAC driver.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> ---
> arch/riscv/Kconfig | 1 +
> drivers/edac/Kconfig | 13 ++
> drivers/edac/Makefile | 1 +
> drivers/edac/sifive_edac.c | 297 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 312 insertions(+)
> create mode 100644 drivers/edac/sifive_edac.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 515fc3c..fede4b6 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -49,6 +49,7 @@ config RISCV
> select RISCV_TIMER
> select GENERIC_IRQ_MULTI_HANDLER
> select ARCH_HAS_PTE_SPECIAL
> + select EDAC_SUPPORT
>
> config MMU
> def_bool y
This part of the patch either needs to get an ack from Palmer, or should
be split into a final, separate patch that Palmer can merge separately.
That way it will avoid unexpected merge conflicts if anything that Palmer
merges changes this file.
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index e286b5b..112d9d1 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -440,6 +440,19 @@ config EDAC_ALTERA_SDMMC
> Support for error detection and correction on the
> Altera SDMMC FIFO Memory for Altera SoCs.
>
> +config EDAC_SIFIVE
> + tristate "Sifive ECC"
> + depends on RISCV
> + help
> + Support for error detection and correction on the SiFive SoCs.
> +
> +config EDAC_SIFIVE_L2
> + bool "SiFive L2 Cache ECC"
> + depends on EDAC_SIFIVE=y
> + help
> + Support for error detection and correction of the L2 cache
> + memory on SiFive SoCs.
> +
> config EDAC_SYNOPSYS
> tristate "Synopsys DDR Memory Controller"
> depends on ARCH_ZYNQ || ARCH_ZYNQMP
> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
> index 716096d..b16dce8 100644
> --- a/drivers/edac/Makefile
> +++ b/drivers/edac/Makefile
> @@ -74,6 +74,7 @@ obj-$(CONFIG_EDAC_OCTEON_PCI) += octeon_edac-pci.o
> obj-$(CONFIG_EDAC_THUNDERX) += thunderx_edac.o
>
> obj-$(CONFIG_EDAC_ALTERA) += altera_edac.o
> +obj-$(CONFIG_EDAC_SIFIVE) += sifive_edac.o
> obj-$(CONFIG_EDAC_SYNOPSYS) += synopsys_edac.o
> obj-$(CONFIG_EDAC_XGENE) += xgene_edac.o
> obj-$(CONFIG_EDAC_TI) += ti_edac.o
> diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c
> new file mode 100644
> index 0000000..e11ae6b5
> --- /dev/null
> +++ b/drivers/edac/sifive_edac.c
> @@ -0,0 +1,297 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SiFive EDAC Driver
> + *
> + * Copyright (C) 2018-2019 SiFive, Inc.
> + *
> + */
> +#include <linux/edac.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include "edac_module.h"
> +
> +#define SIFIVE_EDAC_DIRFIX_LOW 0x100
> +#define SIFIVE_EDAC_DIRFIX_HIGH 0x104
> +#define SIFIVE_EDAC_DIRFIX_COUNT 0x108
> +
> +#define SIFIVE_EDAC_DATFIX_LOW 0x140
> +#define SIFIVE_EDAC_DATFIX_HIGH 0x144
> +#define SIFIVE_EDAC_DATFIX_COUNT 0x148
> +
> +#define SIFIVE_EDAC_DATFAIL_LOW 0x160
> +#define SIFIVE_EDAC_DATFAIL_HIGH 0x164
> +#define SIFIVE_EDAC_DATFAIL_COUNT 0x168
> +
> +#define SIFIVE_EDAC_ECCINJECTERR 0x40
> +#define SIFIVE_EDAC_CONFIG 0x00
> +
> +#define SIFIVE_EDAC_MAX_INTR 3
> +
> +/************************* EDAC Parent Probe *************************/
> +
> +static const struct of_device_id sifive_edac_device_of_match[];
> +
> +static const struct of_device_id sifive_edac_of_match[] = {
> + { .compatible = "sifive,ecc-manager" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, sifive_edac_of_match);
> +
> +static int sifive_edac_probe(struct platform_device *pdev)
> +{
> + of_platform_populate(pdev->dev.of_node, sifive_edac_device_of_match,
> + NULL, &pdev->dev);
> + return 0;
> +}
> +
> +static struct platform_driver sifive_edac_driver = {
> + .probe = sifive_edac_probe,
> + .driver = {
> + .name = "ecc_manager",
As mentioned before we don't have an ECC manager IP block, so we probably
should figure out a different approach here, if possible.
> + .of_match_table = sifive_edac_of_match,
> + },
> +};
> +module_platform_driver(sifive_edac_driver);
> +
> +struct sifive_edac_device_prv {
> + void (*setup)(struct edac_device_ctl_info *dci);
> + irqreturn_t (*ecc_irq_handler)(int irq, void *dev_id);
> + const struct file_operations *inject_fops;
> +};
> +
> +struct sifive_edac_device_dev {
> + void __iomem *base;
> + int irq[SIFIVE_EDAC_MAX_INTR];
> + struct sifive_edac_device_prv *data;
> + char *edac_dev_name;
> +};
> +
> +enum {
> + dir_corr = 0,
> + data_corr,
> + data_uncorr,
> +};
Best to put these in all caps, per coding-style.rst:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n727
> +
> +static struct dentry *sifive_edac_test;
> +
> +static ssize_t sifive_edac_l2_write(struct file *file, const char __user *data,
> + size_t count, loff_t *ppos)
> +{
> + struct edac_device_ctl_info *dci = file->private_data;
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + unsigned int val;
> +
> + if (kstrtouint_from_user(data, count, 0, &val))
> + return -EINVAL;
> + if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
> + writel(val, drvdata->base + SIFIVE_EDAC_ECCINJECTERR);
> + else
> + return -EINVAL;
> + return count;
> +}
> +
> +static const struct file_operations sifive_edac_l2_fops = {
> + .owner = THIS_MODULE,
> + .open = simple_open,
> + .write = sifive_edac_l2_write
> +};
> +
> +static void setup_sifive_debug(struct edac_device_ctl_info *edac_dci,
> + const struct sifive_edac_device_prv *prv)
> +{
> + struct sifive_edac_device_dev *drvdata = edac_dci->pvt_info;
> +
> + if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
> + return;
Can all of these debugfs functions be wrapped with an #if ... #endif such
that, if CONFIG_EDAC_DEBUG is not set, they will all be stripped out by
the preprocessor?
> +
> + sifive_edac_test = edac_debugfs_create_dir(drvdata->edac_dev_name);
> + if (!sifive_edac_test)
> + return;
> +
> + if (!edac_debugfs_create_file("sifive_debug_inject_error", 0200,
> + sifive_edac_test, edac_dci,
> + prv->inject_fops))
> + debugfs_remove_recursive(sifive_edac_test);
> +}
> +
> +static void teardown_sifive_debug(void)
> +{
> + debugfs_remove_recursive(sifive_edac_test);
> +}
> +
> +/*
> + * sifive_edac_l2_int_handler - ISR function for l2 cache controller
> + * @irq: Irq Number
> + * @device: Pointer to the edac device controller instance
> + *
> + * This routine is triggered whenever there is ECC error detected
> + *
> + * Return: Always returns IRQ_HANDLED
> + */
> +static irqreturn_t sifive_edac_l2_int_handler(int irq, void *device)
> +{
> + struct edac_device_ctl_info *dci =
> + (struct edac_device_ctl_info *)device;
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + u32 regval, add_h, add_l;
> +
> + if (irq == drvdata->irq[dir_corr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_LOW);
> + dev_err(dci->dev,
> + "DirError at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_COUNT);
> + edac_device_handle_ce(dci, 0, 0, "DirECCFix");
> + }
> + if (irq == drvdata->irq[data_corr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DATFIX_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DATFIX_LOW);
> + dev_err(dci->dev,
> + "DataError at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DATFIX_COUNT);
> + edac_device_handle_ce(dci, 0, 0, "DatECCFix");
> + }
> + if (irq == drvdata->irq[data_uncorr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_LOW);
> + dev_err(dci->dev,
> + "DataFail at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_COUNT);
> + edac_device_handle_ue(dci, 0, 0, "DatECCFail");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void sifive_edac_l2_config_read(struct edac_device_ctl_info *dci)
> +{
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + u32 regval, val;
> +
> + regval = readl(drvdata->base + SIFIVE_EDAC_CONFIG);
> + val = regval & 0xFF;
> + dev_info(dci->dev, "No. of Banks in the cache: %d\n", val);
> + val = (regval & 0xFF00) >> 8;
> + dev_info(dci->dev, "No. of ways per bank: %d\n", val);
> + val = (regval & 0xFF0000) >> 16;
> + dev_info(dci->dev, "Sets per bank: %llu\n", (uint64_t)1 << val);
> + val = (regval & 0xFF000000) >> 24;
> + dev_info(dci->dev,
> + "Bytes per cache block: %llu\n", (uint64_t)1 << val);
> +}
> +
> +static const struct sifive_edac_device_prv l2ecc_data = {
> + .setup = sifive_edac_l2_config_read,
> + .inject_fops = &sifive_edac_l2_fops,
> + .ecc_irq_handler = sifive_edac_l2_int_handler,
> +};
> +
> +/*
> + * sifive_edac_device_probe()
> + * This is a generic EDAC device driver that will support
> + * various SiFive memory devices as well as the memories
> + * for other peripherals. Module specific initialization is
> + * done by passing the function index in the device tree.
> + */
> +static int sifive_edac_device_probe(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci;
> + struct sifive_edac_device_dev *drvdata;
> + int rc, i;
> + struct resource *res;
> + void __iomem *baseaddr;
> + struct device_node *np = pdev->dev.of_node;
> + char *ecc_name = (char *)np->name;
> + static int dev_instance;
> +
> + /* Get the data from the platform device */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + baseaddr = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(baseaddr))
> + return PTR_ERR(baseaddr);
> +
> + dci = edac_device_alloc_ctl_info(sizeof(*drvdata), ecc_name,
> + 1, ecc_name, 1, 1, NULL, 0,
> + dev_instance++);
> + if (IS_ERR(dci))
> + return PTR_ERR(dci);
> +
> + drvdata = dci->pvt_info;
> + drvdata->base = baseaddr;
> + drvdata->edac_dev_name = ecc_name;
> + dci->dev = &pdev->dev;
> + dci->mod_name = "Sifive ECC Manager";
> + dci->ctl_name = dev_name(&pdev->dev);
> + dci->dev_name = dev_name(&pdev->dev);
> +
> + /* Get driver specific data for this EDAC device */
> + drvdata->data = of_match_node(sifive_edac_device_of_match, np)->data;
> +
> + setup_sifive_debug(dci, drvdata->data);
> +
> + if (drvdata->data->setup)
> + drvdata->data->setup(dci);
> +
> + for (i = 0; i < SIFIVE_EDAC_MAX_INTR; i++) {
> + drvdata->irq[i] = platform_get_irq(pdev, i);
> + rc = devm_request_irq(&pdev->dev, drvdata->irq[i],
> + sifive_edac_l2_int_handler, 0,
> + dev_name(&pdev->dev), (void *)dci);
> + if (rc) {
> + dev_err(&pdev->dev,
> + "Could not request IRQ %d\n", drvdata->irq[i]);
> + goto del_edac_device;
> + }
> + }
> +
> + rc = edac_device_add_device(dci);
> + if (rc) {
> + dev_err(&pdev->dev, "failed to register with EDAC core\n");
> + goto del_edac_device;
> + }
> +
> + return rc;
> +
> +del_edac_device:
> + teardown_sifive_debug();
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
> +
> + return rc;
> +}
> +
> +static int sifive_edac_device_remove(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
> +
> + teardown_sifive_debug();
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sifive_edac_device_of_match[] = {
> + { .compatible = "sifive,ccache0", .data = &l2ecc_data },
This match string should be "sifive,fu540-c000-ccache" if
the intention is to probe against the L2 controller.
> + { /* end of table */ },
> +};
> +MODULE_DEVICE_TABLE(of, sifive_edac_device_of_match);
> +
> +static struct platform_driver sifive_edac_device_driver = {
> + .driver = {
> + .name = "sifive_edac_device",
> + .owner = THIS_MODULE,
> + .of_match_table = sifive_edac_device_of_match,
> + },
> + .probe = sifive_edac_device_probe,
> + .remove = sifive_edac_device_remove,
> +};
> +
> +module_platform_driver(sifive_edac_device_driver);
> +
> +MODULE_AUTHOR("SiFive Inc.");
> +MODULE_DESCRIPTION("SiFive EDAC driver");
> +MODULE_LICENSE("GPL v2");
- Paul
^ permalink raw reply [flat|nested] 32+ messages in thread
* [2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-25 0:23 ` Paul Walmsley
0 siblings, 0 replies; 32+ messages in thread
From: Paul Walmsley @ 2019-03-25 0:23 UTC (permalink / raw)
To: Yash Shah
Cc: linux-riscv, linux-edac, palmer, paul.walmsley, linux-kernel,
robh+dt, mark.rutland, aou, bp, mchehab, devicetree,
sachin.ghadi
Hi Yash,
Just a few brief comments here:
On Wed, 20 Mar 2019, Yash Shah wrote:
> This EDAC driver supports:
> - Initial configuration reporting on bootup via debug logs
> - ECC event monitoring and reporting through the EDAC framework
> - ECC event injection
>
It's probably worth mentioning here, as you did in the subject line, that
this is for the FU540-C000 platform.
> This driver is partially based on pnd2_edac.c and altera_edac.c
>
> Initially L2 Cache controller is added as a subcomponent to
> this EDAC driver.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> ---
> arch/riscv/Kconfig | 1 +
> drivers/edac/Kconfig | 13 ++
> drivers/edac/Makefile | 1 +
> drivers/edac/sifive_edac.c | 297 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 312 insertions(+)
> create mode 100644 drivers/edac/sifive_edac.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 515fc3c..fede4b6 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -49,6 +49,7 @@ config RISCV
> select RISCV_TIMER
> select GENERIC_IRQ_MULTI_HANDLER
> select ARCH_HAS_PTE_SPECIAL
> + select EDAC_SUPPORT
>
> config MMU
> def_bool y
This part of the patch either needs to get an ack from Palmer, or should
be split into a final, separate patch that Palmer can merge separately.
That way it will avoid unexpected merge conflicts if anything that Palmer
merges changes this file.
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index e286b5b..112d9d1 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -440,6 +440,19 @@ config EDAC_ALTERA_SDMMC
> Support for error detection and correction on the
> Altera SDMMC FIFO Memory for Altera SoCs.
>
> +config EDAC_SIFIVE
> + tristate "Sifive ECC"
> + depends on RISCV
> + help
> + Support for error detection and correction on the SiFive SoCs.
> +
> +config EDAC_SIFIVE_L2
> + bool "SiFive L2 Cache ECC"
> + depends on EDAC_SIFIVE=y
> + help
> + Support for error detection and correction of the L2 cache
> + memory on SiFive SoCs.
> +
> config EDAC_SYNOPSYS
> tristate "Synopsys DDR Memory Controller"
> depends on ARCH_ZYNQ || ARCH_ZYNQMP
> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
> index 716096d..b16dce8 100644
> --- a/drivers/edac/Makefile
> +++ b/drivers/edac/Makefile
> @@ -74,6 +74,7 @@ obj-$(CONFIG_EDAC_OCTEON_PCI) += octeon_edac-pci.o
> obj-$(CONFIG_EDAC_THUNDERX) += thunderx_edac.o
>
> obj-$(CONFIG_EDAC_ALTERA) += altera_edac.o
> +obj-$(CONFIG_EDAC_SIFIVE) += sifive_edac.o
> obj-$(CONFIG_EDAC_SYNOPSYS) += synopsys_edac.o
> obj-$(CONFIG_EDAC_XGENE) += xgene_edac.o
> obj-$(CONFIG_EDAC_TI) += ti_edac.o
> diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c
> new file mode 100644
> index 0000000..e11ae6b5
> --- /dev/null
> +++ b/drivers/edac/sifive_edac.c
> @@ -0,0 +1,297 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SiFive EDAC Driver
> + *
> + * Copyright (C) 2018-2019 SiFive, Inc.
> + *
> + */
> +#include <linux/edac.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include "edac_module.h"
> +
> +#define SIFIVE_EDAC_DIRFIX_LOW 0x100
> +#define SIFIVE_EDAC_DIRFIX_HIGH 0x104
> +#define SIFIVE_EDAC_DIRFIX_COUNT 0x108
> +
> +#define SIFIVE_EDAC_DATFIX_LOW 0x140
> +#define SIFIVE_EDAC_DATFIX_HIGH 0x144
> +#define SIFIVE_EDAC_DATFIX_COUNT 0x148
> +
> +#define SIFIVE_EDAC_DATFAIL_LOW 0x160
> +#define SIFIVE_EDAC_DATFAIL_HIGH 0x164
> +#define SIFIVE_EDAC_DATFAIL_COUNT 0x168
> +
> +#define SIFIVE_EDAC_ECCINJECTERR 0x40
> +#define SIFIVE_EDAC_CONFIG 0x00
> +
> +#define SIFIVE_EDAC_MAX_INTR 3
> +
> +/************************* EDAC Parent Probe *************************/
> +
> +static const struct of_device_id sifive_edac_device_of_match[];
> +
> +static const struct of_device_id sifive_edac_of_match[] = {
> + { .compatible = "sifive,ecc-manager" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, sifive_edac_of_match);
> +
> +static int sifive_edac_probe(struct platform_device *pdev)
> +{
> + of_platform_populate(pdev->dev.of_node, sifive_edac_device_of_match,
> + NULL, &pdev->dev);
> + return 0;
> +}
> +
> +static struct platform_driver sifive_edac_driver = {
> + .probe = sifive_edac_probe,
> + .driver = {
> + .name = "ecc_manager",
As mentioned before we don't have an ECC manager IP block, so we probably
should figure out a different approach here, if possible.
> + .of_match_table = sifive_edac_of_match,
> + },
> +};
> +module_platform_driver(sifive_edac_driver);
> +
> +struct sifive_edac_device_prv {
> + void (*setup)(struct edac_device_ctl_info *dci);
> + irqreturn_t (*ecc_irq_handler)(int irq, void *dev_id);
> + const struct file_operations *inject_fops;
> +};
> +
> +struct sifive_edac_device_dev {
> + void __iomem *base;
> + int irq[SIFIVE_EDAC_MAX_INTR];
> + struct sifive_edac_device_prv *data;
> + char *edac_dev_name;
> +};
> +
> +enum {
> + dir_corr = 0,
> + data_corr,
> + data_uncorr,
> +};
Best to put these in all caps, per coding-style.rst:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n727
> +
> +static struct dentry *sifive_edac_test;
> +
> +static ssize_t sifive_edac_l2_write(struct file *file, const char __user *data,
> + size_t count, loff_t *ppos)
> +{
> + struct edac_device_ctl_info *dci = file->private_data;
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + unsigned int val;
> +
> + if (kstrtouint_from_user(data, count, 0, &val))
> + return -EINVAL;
> + if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
> + writel(val, drvdata->base + SIFIVE_EDAC_ECCINJECTERR);
> + else
> + return -EINVAL;
> + return count;
> +}
> +
> +static const struct file_operations sifive_edac_l2_fops = {
> + .owner = THIS_MODULE,
> + .open = simple_open,
> + .write = sifive_edac_l2_write
> +};
> +
> +static void setup_sifive_debug(struct edac_device_ctl_info *edac_dci,
> + const struct sifive_edac_device_prv *prv)
> +{
> + struct sifive_edac_device_dev *drvdata = edac_dci->pvt_info;
> +
> + if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
> + return;
Can all of these debugfs functions be wrapped with an #if ... #endif such
that, if CONFIG_EDAC_DEBUG is not set, they will all be stripped out by
the preprocessor?
> +
> + sifive_edac_test = edac_debugfs_create_dir(drvdata->edac_dev_name);
> + if (!sifive_edac_test)
> + return;
> +
> + if (!edac_debugfs_create_file("sifive_debug_inject_error", 0200,
> + sifive_edac_test, edac_dci,
> + prv->inject_fops))
> + debugfs_remove_recursive(sifive_edac_test);
> +}
> +
> +static void teardown_sifive_debug(void)
> +{
> + debugfs_remove_recursive(sifive_edac_test);
> +}
> +
> +/*
> + * sifive_edac_l2_int_handler - ISR function for l2 cache controller
> + * @irq: Irq Number
> + * @device: Pointer to the edac device controller instance
> + *
> + * This routine is triggered whenever there is ECC error detected
> + *
> + * Return: Always returns IRQ_HANDLED
> + */
> +static irqreturn_t sifive_edac_l2_int_handler(int irq, void *device)
> +{
> + struct edac_device_ctl_info *dci =
> + (struct edac_device_ctl_info *)device;
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + u32 regval, add_h, add_l;
> +
> + if (irq == drvdata->irq[dir_corr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_LOW);
> + dev_err(dci->dev,
> + "DirError at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_COUNT);
> + edac_device_handle_ce(dci, 0, 0, "DirECCFix");
> + }
> + if (irq == drvdata->irq[data_corr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DATFIX_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DATFIX_LOW);
> + dev_err(dci->dev,
> + "DataError at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DATFIX_COUNT);
> + edac_device_handle_ce(dci, 0, 0, "DatECCFix");
> + }
> + if (irq == drvdata->irq[data_uncorr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_LOW);
> + dev_err(dci->dev,
> + "DataFail at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_COUNT);
> + edac_device_handle_ue(dci, 0, 0, "DatECCFail");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void sifive_edac_l2_config_read(struct edac_device_ctl_info *dci)
> +{
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + u32 regval, val;
> +
> + regval = readl(drvdata->base + SIFIVE_EDAC_CONFIG);
> + val = regval & 0xFF;
> + dev_info(dci->dev, "No. of Banks in the cache: %d\n", val);
> + val = (regval & 0xFF00) >> 8;
> + dev_info(dci->dev, "No. of ways per bank: %d\n", val);
> + val = (regval & 0xFF0000) >> 16;
> + dev_info(dci->dev, "Sets per bank: %llu\n", (uint64_t)1 << val);
> + val = (regval & 0xFF000000) >> 24;
> + dev_info(dci->dev,
> + "Bytes per cache block: %llu\n", (uint64_t)1 << val);
> +}
> +
> +static const struct sifive_edac_device_prv l2ecc_data = {
> + .setup = sifive_edac_l2_config_read,
> + .inject_fops = &sifive_edac_l2_fops,
> + .ecc_irq_handler = sifive_edac_l2_int_handler,
> +};
> +
> +/*
> + * sifive_edac_device_probe()
> + * This is a generic EDAC device driver that will support
> + * various SiFive memory devices as well as the memories
> + * for other peripherals. Module specific initialization is
> + * done by passing the function index in the device tree.
> + */
> +static int sifive_edac_device_probe(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci;
> + struct sifive_edac_device_dev *drvdata;
> + int rc, i;
> + struct resource *res;
> + void __iomem *baseaddr;
> + struct device_node *np = pdev->dev.of_node;
> + char *ecc_name = (char *)np->name;
> + static int dev_instance;
> +
> + /* Get the data from the platform device */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + baseaddr = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(baseaddr))
> + return PTR_ERR(baseaddr);
> +
> + dci = edac_device_alloc_ctl_info(sizeof(*drvdata), ecc_name,
> + 1, ecc_name, 1, 1, NULL, 0,
> + dev_instance++);
> + if (IS_ERR(dci))
> + return PTR_ERR(dci);
> +
> + drvdata = dci->pvt_info;
> + drvdata->base = baseaddr;
> + drvdata->edac_dev_name = ecc_name;
> + dci->dev = &pdev->dev;
> + dci->mod_name = "Sifive ECC Manager";
> + dci->ctl_name = dev_name(&pdev->dev);
> + dci->dev_name = dev_name(&pdev->dev);
> +
> + /* Get driver specific data for this EDAC device */
> + drvdata->data = of_match_node(sifive_edac_device_of_match, np)->data;
> +
> + setup_sifive_debug(dci, drvdata->data);
> +
> + if (drvdata->data->setup)
> + drvdata->data->setup(dci);
> +
> + for (i = 0; i < SIFIVE_EDAC_MAX_INTR; i++) {
> + drvdata->irq[i] = platform_get_irq(pdev, i);
> + rc = devm_request_irq(&pdev->dev, drvdata->irq[i],
> + sifive_edac_l2_int_handler, 0,
> + dev_name(&pdev->dev), (void *)dci);
> + if (rc) {
> + dev_err(&pdev->dev,
> + "Could not request IRQ %d\n", drvdata->irq[i]);
> + goto del_edac_device;
> + }
> + }
> +
> + rc = edac_device_add_device(dci);
> + if (rc) {
> + dev_err(&pdev->dev, "failed to register with EDAC core\n");
> + goto del_edac_device;
> + }
> +
> + return rc;
> +
> +del_edac_device:
> + teardown_sifive_debug();
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
> +
> + return rc;
> +}
> +
> +static int sifive_edac_device_remove(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
> +
> + teardown_sifive_debug();
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sifive_edac_device_of_match[] = {
> + { .compatible = "sifive,ccache0", .data = &l2ecc_data },
This match string should be "sifive,fu540-c000-ccache" if
the intention is to probe against the L2 controller.
> + { /* end of table */ },
> +};
> +MODULE_DEVICE_TABLE(of, sifive_edac_device_of_match);
> +
> +static struct platform_driver sifive_edac_device_driver = {
> + .driver = {
> + .name = "sifive_edac_device",
> + .owner = THIS_MODULE,
> + .of_match_table = sifive_edac_device_of_match,
> + },
> + .probe = sifive_edac_device_probe,
> + .remove = sifive_edac_device_remove,
> +};
> +
> +module_platform_driver(sifive_edac_device_driver);
> +
> +MODULE_AUTHOR("SiFive Inc.");
> +MODULE_DESCRIPTION("SiFive EDAC driver");
> +MODULE_LICENSE("GPL v2");
- Paul
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-25 0:23 ` Paul Walmsley
0 siblings, 0 replies; 32+ messages in thread
From: Paul Walmsley @ 2019-03-25 0:23 UTC (permalink / raw)
To: Yash Shah
Cc: mark.rutland, devicetree, aou, palmer, linux-kernel,
sachin.ghadi, robh+dt, bp, paul.walmsley, linux-riscv, mchehab,
linux-edac
Hi Yash,
Just a few brief comments here:
On Wed, 20 Mar 2019, Yash Shah wrote:
> This EDAC driver supports:
> - Initial configuration reporting on bootup via debug logs
> - ECC event monitoring and reporting through the EDAC framework
> - ECC event injection
>
It's probably worth mentioning here, as you did in the subject line, that
this is for the FU540-C000 platform.
> This driver is partially based on pnd2_edac.c and altera_edac.c
>
> Initially L2 Cache controller is added as a subcomponent to
> this EDAC driver.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> ---
> arch/riscv/Kconfig | 1 +
> drivers/edac/Kconfig | 13 ++
> drivers/edac/Makefile | 1 +
> drivers/edac/sifive_edac.c | 297 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 312 insertions(+)
> create mode 100644 drivers/edac/sifive_edac.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 515fc3c..fede4b6 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -49,6 +49,7 @@ config RISCV
> select RISCV_TIMER
> select GENERIC_IRQ_MULTI_HANDLER
> select ARCH_HAS_PTE_SPECIAL
> + select EDAC_SUPPORT
>
> config MMU
> def_bool y
This part of the patch either needs to get an ack from Palmer, or should
be split into a final, separate patch that Palmer can merge separately.
That way it will avoid unexpected merge conflicts if anything that Palmer
merges changes this file.
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index e286b5b..112d9d1 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -440,6 +440,19 @@ config EDAC_ALTERA_SDMMC
> Support for error detection and correction on the
> Altera SDMMC FIFO Memory for Altera SoCs.
>
> +config EDAC_SIFIVE
> + tristate "Sifive ECC"
> + depends on RISCV
> + help
> + Support for error detection and correction on the SiFive SoCs.
> +
> +config EDAC_SIFIVE_L2
> + bool "SiFive L2 Cache ECC"
> + depends on EDAC_SIFIVE=y
> + help
> + Support for error detection and correction of the L2 cache
> + memory on SiFive SoCs.
> +
> config EDAC_SYNOPSYS
> tristate "Synopsys DDR Memory Controller"
> depends on ARCH_ZYNQ || ARCH_ZYNQMP
> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
> index 716096d..b16dce8 100644
> --- a/drivers/edac/Makefile
> +++ b/drivers/edac/Makefile
> @@ -74,6 +74,7 @@ obj-$(CONFIG_EDAC_OCTEON_PCI) += octeon_edac-pci.o
> obj-$(CONFIG_EDAC_THUNDERX) += thunderx_edac.o
>
> obj-$(CONFIG_EDAC_ALTERA) += altera_edac.o
> +obj-$(CONFIG_EDAC_SIFIVE) += sifive_edac.o
> obj-$(CONFIG_EDAC_SYNOPSYS) += synopsys_edac.o
> obj-$(CONFIG_EDAC_XGENE) += xgene_edac.o
> obj-$(CONFIG_EDAC_TI) += ti_edac.o
> diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c
> new file mode 100644
> index 0000000..e11ae6b5
> --- /dev/null
> +++ b/drivers/edac/sifive_edac.c
> @@ -0,0 +1,297 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SiFive EDAC Driver
> + *
> + * Copyright (C) 2018-2019 SiFive, Inc.
> + *
> + */
> +#include <linux/edac.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include "edac_module.h"
> +
> +#define SIFIVE_EDAC_DIRFIX_LOW 0x100
> +#define SIFIVE_EDAC_DIRFIX_HIGH 0x104
> +#define SIFIVE_EDAC_DIRFIX_COUNT 0x108
> +
> +#define SIFIVE_EDAC_DATFIX_LOW 0x140
> +#define SIFIVE_EDAC_DATFIX_HIGH 0x144
> +#define SIFIVE_EDAC_DATFIX_COUNT 0x148
> +
> +#define SIFIVE_EDAC_DATFAIL_LOW 0x160
> +#define SIFIVE_EDAC_DATFAIL_HIGH 0x164
> +#define SIFIVE_EDAC_DATFAIL_COUNT 0x168
> +
> +#define SIFIVE_EDAC_ECCINJECTERR 0x40
> +#define SIFIVE_EDAC_CONFIG 0x00
> +
> +#define SIFIVE_EDAC_MAX_INTR 3
> +
> +/************************* EDAC Parent Probe *************************/
> +
> +static const struct of_device_id sifive_edac_device_of_match[];
> +
> +static const struct of_device_id sifive_edac_of_match[] = {
> + { .compatible = "sifive,ecc-manager" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, sifive_edac_of_match);
> +
> +static int sifive_edac_probe(struct platform_device *pdev)
> +{
> + of_platform_populate(pdev->dev.of_node, sifive_edac_device_of_match,
> + NULL, &pdev->dev);
> + return 0;
> +}
> +
> +static struct platform_driver sifive_edac_driver = {
> + .probe = sifive_edac_probe,
> + .driver = {
> + .name = "ecc_manager",
As mentioned before we don't have an ECC manager IP block, so we probably
should figure out a different approach here, if possible.
> + .of_match_table = sifive_edac_of_match,
> + },
> +};
> +module_platform_driver(sifive_edac_driver);
> +
> +struct sifive_edac_device_prv {
> + void (*setup)(struct edac_device_ctl_info *dci);
> + irqreturn_t (*ecc_irq_handler)(int irq, void *dev_id);
> + const struct file_operations *inject_fops;
> +};
> +
> +struct sifive_edac_device_dev {
> + void __iomem *base;
> + int irq[SIFIVE_EDAC_MAX_INTR];
> + struct sifive_edac_device_prv *data;
> + char *edac_dev_name;
> +};
> +
> +enum {
> + dir_corr = 0,
> + data_corr,
> + data_uncorr,
> +};
Best to put these in all caps, per coding-style.rst:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n727
> +
> +static struct dentry *sifive_edac_test;
> +
> +static ssize_t sifive_edac_l2_write(struct file *file, const char __user *data,
> + size_t count, loff_t *ppos)
> +{
> + struct edac_device_ctl_info *dci = file->private_data;
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + unsigned int val;
> +
> + if (kstrtouint_from_user(data, count, 0, &val))
> + return -EINVAL;
> + if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
> + writel(val, drvdata->base + SIFIVE_EDAC_ECCINJECTERR);
> + else
> + return -EINVAL;
> + return count;
> +}
> +
> +static const struct file_operations sifive_edac_l2_fops = {
> + .owner = THIS_MODULE,
> + .open = simple_open,
> + .write = sifive_edac_l2_write
> +};
> +
> +static void setup_sifive_debug(struct edac_device_ctl_info *edac_dci,
> + const struct sifive_edac_device_prv *prv)
> +{
> + struct sifive_edac_device_dev *drvdata = edac_dci->pvt_info;
> +
> + if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
> + return;
Can all of these debugfs functions be wrapped with an #if ... #endif such
that, if CONFIG_EDAC_DEBUG is not set, they will all be stripped out by
the preprocessor?
> +
> + sifive_edac_test = edac_debugfs_create_dir(drvdata->edac_dev_name);
> + if (!sifive_edac_test)
> + return;
> +
> + if (!edac_debugfs_create_file("sifive_debug_inject_error", 0200,
> + sifive_edac_test, edac_dci,
> + prv->inject_fops))
> + debugfs_remove_recursive(sifive_edac_test);
> +}
> +
> +static void teardown_sifive_debug(void)
> +{
> + debugfs_remove_recursive(sifive_edac_test);
> +}
> +
> +/*
> + * sifive_edac_l2_int_handler - ISR function for l2 cache controller
> + * @irq: Irq Number
> + * @device: Pointer to the edac device controller instance
> + *
> + * This routine is triggered whenever there is ECC error detected
> + *
> + * Return: Always returns IRQ_HANDLED
> + */
> +static irqreturn_t sifive_edac_l2_int_handler(int irq, void *device)
> +{
> + struct edac_device_ctl_info *dci =
> + (struct edac_device_ctl_info *)device;
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + u32 regval, add_h, add_l;
> +
> + if (irq == drvdata->irq[dir_corr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_LOW);
> + dev_err(dci->dev,
> + "DirError at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DIRFIX_COUNT);
> + edac_device_handle_ce(dci, 0, 0, "DirECCFix");
> + }
> + if (irq == drvdata->irq[data_corr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DATFIX_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DATFIX_LOW);
> + dev_err(dci->dev,
> + "DataError at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DATFIX_COUNT);
> + edac_device_handle_ce(dci, 0, 0, "DatECCFix");
> + }
> + if (irq == drvdata->irq[data_uncorr]) {
> + add_h = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_HIGH);
> + add_l = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_LOW);
> + dev_err(dci->dev,
> + "DataFail at address 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(drvdata->base + SIFIVE_EDAC_DATFAIL_COUNT);
> + edac_device_handle_ue(dci, 0, 0, "DatECCFail");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void sifive_edac_l2_config_read(struct edac_device_ctl_info *dci)
> +{
> + struct sifive_edac_device_dev *drvdata = dci->pvt_info;
> + u32 regval, val;
> +
> + regval = readl(drvdata->base + SIFIVE_EDAC_CONFIG);
> + val = regval & 0xFF;
> + dev_info(dci->dev, "No. of Banks in the cache: %d\n", val);
> + val = (regval & 0xFF00) >> 8;
> + dev_info(dci->dev, "No. of ways per bank: %d\n", val);
> + val = (regval & 0xFF0000) >> 16;
> + dev_info(dci->dev, "Sets per bank: %llu\n", (uint64_t)1 << val);
> + val = (regval & 0xFF000000) >> 24;
> + dev_info(dci->dev,
> + "Bytes per cache block: %llu\n", (uint64_t)1 << val);
> +}
> +
> +static const struct sifive_edac_device_prv l2ecc_data = {
> + .setup = sifive_edac_l2_config_read,
> + .inject_fops = &sifive_edac_l2_fops,
> + .ecc_irq_handler = sifive_edac_l2_int_handler,
> +};
> +
> +/*
> + * sifive_edac_device_probe()
> + * This is a generic EDAC device driver that will support
> + * various SiFive memory devices as well as the memories
> + * for other peripherals. Module specific initialization is
> + * done by passing the function index in the device tree.
> + */
> +static int sifive_edac_device_probe(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci;
> + struct sifive_edac_device_dev *drvdata;
> + int rc, i;
> + struct resource *res;
> + void __iomem *baseaddr;
> + struct device_node *np = pdev->dev.of_node;
> + char *ecc_name = (char *)np->name;
> + static int dev_instance;
> +
> + /* Get the data from the platform device */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + baseaddr = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(baseaddr))
> + return PTR_ERR(baseaddr);
> +
> + dci = edac_device_alloc_ctl_info(sizeof(*drvdata), ecc_name,
> + 1, ecc_name, 1, 1, NULL, 0,
> + dev_instance++);
> + if (IS_ERR(dci))
> + return PTR_ERR(dci);
> +
> + drvdata = dci->pvt_info;
> + drvdata->base = baseaddr;
> + drvdata->edac_dev_name = ecc_name;
> + dci->dev = &pdev->dev;
> + dci->mod_name = "Sifive ECC Manager";
> + dci->ctl_name = dev_name(&pdev->dev);
> + dci->dev_name = dev_name(&pdev->dev);
> +
> + /* Get driver specific data for this EDAC device */
> + drvdata->data = of_match_node(sifive_edac_device_of_match, np)->data;
> +
> + setup_sifive_debug(dci, drvdata->data);
> +
> + if (drvdata->data->setup)
> + drvdata->data->setup(dci);
> +
> + for (i = 0; i < SIFIVE_EDAC_MAX_INTR; i++) {
> + drvdata->irq[i] = platform_get_irq(pdev, i);
> + rc = devm_request_irq(&pdev->dev, drvdata->irq[i],
> + sifive_edac_l2_int_handler, 0,
> + dev_name(&pdev->dev), (void *)dci);
> + if (rc) {
> + dev_err(&pdev->dev,
> + "Could not request IRQ %d\n", drvdata->irq[i]);
> + goto del_edac_device;
> + }
> + }
> +
> + rc = edac_device_add_device(dci);
> + if (rc) {
> + dev_err(&pdev->dev, "failed to register with EDAC core\n");
> + goto del_edac_device;
> + }
> +
> + return rc;
> +
> +del_edac_device:
> + teardown_sifive_debug();
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
> +
> + return rc;
> +}
> +
> +static int sifive_edac_device_remove(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
> +
> + teardown_sifive_debug();
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sifive_edac_device_of_match[] = {
> + { .compatible = "sifive,ccache0", .data = &l2ecc_data },
This match string should be "sifive,fu540-c000-ccache" if
the intention is to probe against the L2 controller.
> + { /* end of table */ },
> +};
> +MODULE_DEVICE_TABLE(of, sifive_edac_device_of_match);
> +
> +static struct platform_driver sifive_edac_device_driver = {
> + .driver = {
> + .name = "sifive_edac_device",
> + .owner = THIS_MODULE,
> + .of_match_table = sifive_edac_device_of_match,
> + },
> + .probe = sifive_edac_device_probe,
> + .remove = sifive_edac_device_remove,
> +};
> +
> +module_platform_driver(sifive_edac_device_driver);
> +
> +MODULE_AUTHOR("SiFive Inc.");
> +MODULE_DESCRIPTION("SiFive EDAC driver");
> +MODULE_LICENSE("GPL v2");
- Paul
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-25 6:57 ` Borislav Petkov
0 siblings, 0 replies; 32+ messages in thread
From: Borislav Petkov @ 2019-03-25 6:57 UTC (permalink / raw)
To: Paul Walmsley
Cc: Yash Shah, linux-riscv, linux-edac, palmer, linux-kernel,
robh+dt, mark.rutland, aou, mchehab, devicetree, sachin.ghadi
On Sun, Mar 24, 2019 at 05:23:27PM -0700, Paul Walmsley wrote:
> > + if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
> > + return;
>
> Can all of these debugfs functions be wrapped with an #if ... #endif such
> that, if CONFIG_EDAC_DEBUG is not set, they will all be stripped out by
> the preprocessor?
Why would you make the code more ugly with ifdeffery?
Do you have any serious code size constraints so that you absolutely
need to remove a couple of KBs?
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-25 6:57 ` Borislav Petkov
0 siblings, 0 replies; 32+ messages in thread
From: Borislav Petkov @ 2019-03-25 6:57 UTC (permalink / raw)
To: Paul Walmsley
Cc: Yash Shah, linux-riscv, linux-edac, palmer, linux-kernel,
robh+dt, mark.rutland, aou, mchehab, devicetree, sachin.ghadi
On Sun, Mar 24, 2019 at 05:23:27PM -0700, Paul Walmsley wrote:
> > + if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
> > + return;
>
> Can all of these debugfs functions be wrapped with an #if ... #endif such
> that, if CONFIG_EDAC_DEBUG is not set, they will all be stripped out by
> the preprocessor?
Why would you make the code more ugly with ifdeffery?
Do you have any serious code size constraints so that you absolutely
need to remove a couple of KBs?
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-25 6:57 ` Borislav Petkov
0 siblings, 0 replies; 32+ messages in thread
From: Borislav Petkov @ 2019-03-25 6:57 UTC (permalink / raw)
To: Paul Walmsley
Cc: mark.rutland, devicetree, aou, palmer, linux-kernel,
sachin.ghadi, Yash Shah, robh+dt, linux-riscv, mchehab,
linux-edac
On Sun, Mar 24, 2019 at 05:23:27PM -0700, Paul Walmsley wrote:
> > + if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
> > + return;
>
> Can all of these debugfs functions be wrapped with an #if ... #endif such
> that, if CONFIG_EDAC_DEBUG is not set, they will all be stripped out by
> the preprocessor?
Why would you make the code more ugly with ifdeffery?
Do you have any serious code size constraints so that you absolutely
need to remove a couple of KBs?
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-25 21:26 ` Paul Walmsley
0 siblings, 0 replies; 32+ messages in thread
From: Paul Walmsley @ 2019-03-25 21:26 UTC (permalink / raw)
To: Borislav Petkov
Cc: Paul Walmsley, Yash Shah, linux-riscv, linux-edac, palmer,
linux-kernel, robh+dt, mark.rutland, aou, mchehab, devicetree,
sachin.ghadi
On Mon, 25 Mar 2019, Borislav Petkov wrote:
> On Sun, Mar 24, 2019 at 05:23:27PM -0700, Paul Walmsley wrote:
> > > + if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
> > > + return;
> >
> > Can all of these debugfs functions be wrapped with an #if ... #endif such
> > that, if CONFIG_EDAC_DEBUG is not set, they will all be stripped out by
> > the preprocessor?
>
> Why would you make the code more ugly with ifdeffery?
>
> Do you have any serious code size constraints so that you absolutely
> need to remove a couple of KBs?
We'll definitely take the RAM savings that a few #ifdefs will deliver to
us. They add up. We're selling chips for embedded use cases, not just
big-iron x86 systems.
Other EDAC drivers have far more #ifdef lines than the single set that I'm
proposing, so I don't understand why you're singling this driver out for
criticism. Consider:
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_ETHERNET
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_NAND
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_DMA
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_USB
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_QSPI
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDMMC
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_ETHERNET
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_NAND
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_DMA
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_USB
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_QSPI
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDMMC
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDRAM
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
- Paul
^ permalink raw reply [flat|nested] 32+ messages in thread
* [2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-25 21:26 ` Paul Walmsley
0 siblings, 0 replies; 32+ messages in thread
From: Paul Walmsley @ 2019-03-25 21:26 UTC (permalink / raw)
To: Borislav Petkov
Cc: Paul Walmsley, Yash Shah, linux-riscv, linux-edac, palmer,
linux-kernel, robh+dt, mark.rutland, aou, mchehab, devicetree,
sachin.ghadi
On Mon, 25 Mar 2019, Borislav Petkov wrote:
> On Sun, Mar 24, 2019 at 05:23:27PM -0700, Paul Walmsley wrote:
> > > + if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
> > > + return;
> >
> > Can all of these debugfs functions be wrapped with an #if ... #endif such
> > that, if CONFIG_EDAC_DEBUG is not set, they will all be stripped out by
> > the preprocessor?
>
> Why would you make the code more ugly with ifdeffery?
>
> Do you have any serious code size constraints so that you absolutely
> need to remove a couple of KBs?
We'll definitely take the RAM savings that a few #ifdefs will deliver to
us. They add up. We're selling chips for embedded use cases, not just
big-iron x86 systems.
Other EDAC drivers have far more #ifdef lines than the single set that I'm
proposing, so I don't understand why you're singling this driver out for
criticism. Consider:
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_ETHERNET
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_NAND
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_DMA
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_USB
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_QSPI
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDMMC
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_ETHERNET
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_NAND
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_DMA
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_USB
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_QSPI
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDMMC
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDRAM
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
- Paul
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-25 21:26 ` Paul Walmsley
0 siblings, 0 replies; 32+ messages in thread
From: Paul Walmsley @ 2019-03-25 21:26 UTC (permalink / raw)
To: Borislav Petkov
Cc: mark.rutland, devicetree, aou, palmer, linux-kernel,
sachin.ghadi, Yash Shah, robh+dt, Paul Walmsley, linux-riscv,
mchehab, linux-edac
On Mon, 25 Mar 2019, Borislav Petkov wrote:
> On Sun, Mar 24, 2019 at 05:23:27PM -0700, Paul Walmsley wrote:
> > > + if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
> > > + return;
> >
> > Can all of these debugfs functions be wrapped with an #if ... #endif such
> > that, if CONFIG_EDAC_DEBUG is not set, they will all be stripped out by
> > the preprocessor?
>
> Why would you make the code more ugly with ifdeffery?
>
> Do you have any serious code size constraints so that you absolutely
> need to remove a couple of KBs?
We'll definitely take the RAM savings that a few #ifdefs will deliver to
us. They add up. We're selling chips for embedded use cases, not just
big-iron x86 systems.
Other EDAC drivers have far more #ifdef lines than the single set that I'm
proposing, so I don't understand why you're singling this driver out for
criticism. Consider:
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_ETHERNET
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_NAND
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_DMA
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_USB
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_QSPI
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDMMC
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_ETHERNET
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_NAND
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_DMA
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_USB
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_QSPI
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDMMC
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDRAM
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
- Paul
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-25 21:50 ` Borislav Petkov
0 siblings, 0 replies; 32+ messages in thread
From: Borislav Petkov @ 2019-03-25 21:50 UTC (permalink / raw)
To: Paul Walmsley
Cc: Yash Shah, linux-riscv, linux-edac, palmer, linux-kernel,
robh+dt, mark.rutland, aou, mchehab, devicetree, sachin.ghadi
On Mon, Mar 25, 2019 at 02:26:52PM -0700, Paul Walmsley wrote:
> We'll definitely take the RAM savings that a few #ifdefs will deliver to
> us. They add up. We're selling chips for embedded use cases, not just
> big-iron x86 systems.
Fair enough.
Btw, while we're at it, this driver would need a MAINTAINERS entry so
that you guys can get CCed on bugs.
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-25 21:50 ` Borislav Petkov
0 siblings, 0 replies; 32+ messages in thread
From: Borislav Petkov @ 2019-03-25 21:50 UTC (permalink / raw)
To: Paul Walmsley
Cc: Yash Shah, linux-riscv, linux-edac, palmer, linux-kernel,
robh+dt, mark.rutland, aou, mchehab, devicetree, sachin.ghadi
On Mon, Mar 25, 2019 at 02:26:52PM -0700, Paul Walmsley wrote:
> We'll definitely take the RAM savings that a few #ifdefs will deliver to
> us. They add up. We're selling chips for embedded use cases, not just
> big-iron x86 systems.
Fair enough.
Btw, while we're at it, this driver would need a MAINTAINERS entry so
that you guys can get CCed on bugs.
Thx.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-25 21:50 ` Borislav Petkov
0 siblings, 0 replies; 32+ messages in thread
From: Borislav Petkov @ 2019-03-25 21:50 UTC (permalink / raw)
To: Paul Walmsley
Cc: mark.rutland, devicetree, aou, palmer, linux-kernel,
sachin.ghadi, Yash Shah, robh+dt, linux-riscv, mchehab,
linux-edac
On Mon, Mar 25, 2019 at 02:26:52PM -0700, Paul Walmsley wrote:
> We'll definitely take the RAM savings that a few #ifdefs will deliver to
> us. They add up. We're selling chips for embedded use cases, not just
> big-iron x86 systems.
Fair enough.
Btw, while we're at it, this driver would need a MAINTAINERS entry so
that you guys can get CCed on bugs.
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-29 19:45 ` Paul Walmsley
0 siblings, 0 replies; 32+ messages in thread
From: Paul Walmsley @ 2019-03-29 19:45 UTC (permalink / raw)
To: linux-edac
Cc: linux-riscv, Yash Shah, james.morse, palmer, paul.walmsley,
linux-kernel, robh+dt, mark.rutland, aou, bp, mchehab,
devicetree, sachin.ghadi
On Wed, 20 Mar 2019, Yash Shah wrote:
> This EDAC driver supports:
> - Initial configuration reporting on bootup via debug logs
> - ECC event monitoring and reporting through the EDAC framework
> - ECC event injection
>
> This driver is partially based on pnd2_edac.c and altera_edac.c
>
> Initially L2 Cache controller is added as a subcomponent to
> this EDAC driver.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
Based on the discussion with Borislav, we're going to move the L2 driver
elsewhere in the tree. We'll look into creating a platform-specific EDAC
driver on top of that.
- Paul
^ permalink raw reply [flat|nested] 32+ messages in thread
* [2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-29 19:45 ` Paul Walmsley
0 siblings, 0 replies; 32+ messages in thread
From: Paul Walmsley @ 2019-03-29 19:45 UTC (permalink / raw)
To: linux-edac
Cc: linux-riscv, Yash Shah, james.morse, palmer, paul.walmsley,
linux-kernel, robh+dt, mark.rutland, aou, bp, mchehab,
devicetree, sachin.ghadi
On Wed, 20 Mar 2019, Yash Shah wrote:
> This EDAC driver supports:
> - Initial configuration reporting on bootup via debug logs
> - ECC event monitoring and reporting through the EDAC framework
> - ECC event injection
>
> This driver is partially based on pnd2_edac.c and altera_edac.c
>
> Initially L2 Cache controller is added as a subcomponent to
> this EDAC driver.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
Based on the discussion with Borislav, we're going to move the L2 driver
elsewhere in the tree. We'll look into creating a platform-specific EDAC
driver on top of that.
- Paul
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip
@ 2019-03-29 19:45 ` Paul Walmsley
0 siblings, 0 replies; 32+ messages in thread
From: Paul Walmsley @ 2019-03-29 19:45 UTC (permalink / raw)
To: linux-edac
Cc: mark.rutland, devicetree, aou, palmer, linux-kernel,
sachin.ghadi, Yash Shah, robh+dt, james.morse, paul.walmsley, bp,
linux-riscv, mchehab
On Wed, 20 Mar 2019, Yash Shah wrote:
> This EDAC driver supports:
> - Initial configuration reporting on bootup via debug logs
> - ECC event monitoring and reporting through the EDAC framework
> - ECC event injection
>
> This driver is partially based on pnd2_edac.c and altera_edac.c
>
> Initially L2 Cache controller is added as a subcomponent to
> this EDAC driver.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
Based on the discussion with Borislav, we're going to move the L2 driver
elsewhere in the tree. We'll look into creating a platform-specific EDAC
driver on top of that.
- Paul
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2019-03-29 19:45 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-20 11:52 [PATCH 0/2] EDAC Support for SiFive SoCs Yash Shah
2019-03-20 11:52 ` Yash Shah
2019-03-20 11:52 ` [PATCH 1/2] edac: sifive: Add DT documentation for SiFive EDAC driver and subcomponent Yash Shah
2019-03-20 11:52 ` Yash Shah
2019-03-20 11:52 ` [1/2] " Yash Shah
2019-03-25 0:20 ` [PATCH 1/2] " Paul Walmsley
2019-03-25 0:20 ` Paul Walmsley
2019-03-25 0:20 ` [1/2] " Paul Walmsley
2019-03-20 11:52 ` [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip Yash Shah
2019-03-20 11:52 ` Yash Shah
2019-03-20 11:52 ` [2/2] " Yash Shah
2019-03-21 13:33 ` [PATCH 2/2] " Borislav Petkov
2019-03-21 13:33 ` Borislav Petkov
2019-03-21 13:33 ` [2/2] " Borislav Petkov
2019-03-22 6:00 ` [PATCH 2/2] " Yash Shah
2019-03-22 6:00 ` Yash Shah
2019-03-22 6:00 ` [2/2] " Yash Shah
2019-03-25 0:23 ` [PATCH 2/2] " Paul Walmsley
2019-03-25 0:23 ` Paul Walmsley
2019-03-25 0:23 ` [2/2] " Paul Walmsley
2019-03-25 6:57 ` [PATCH 2/2] " Borislav Petkov
2019-03-25 6:57 ` Borislav Petkov
2019-03-25 6:57 ` [2/2] " Borislav Petkov
2019-03-25 21:26 ` [PATCH 2/2] " Paul Walmsley
2019-03-25 21:26 ` Paul Walmsley
2019-03-25 21:26 ` [2/2] " Paul Walmsley
2019-03-25 21:50 ` [PATCH 2/2] " Borislav Petkov
2019-03-25 21:50 ` Borislav Petkov
2019-03-25 21:50 ` [2/2] " Borislav Petkov
2019-03-29 19:45 ` [PATCH 2/2] " Paul Walmsley
2019-03-29 19:45 ` Paul Walmsley
2019-03-29 19:45 ` [2/2] " Paul Walmsley
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.