From: Thor Thayer <[email protected]>
This patch adds the L2 cache and OCRAM peripherals to the EDAC framework
using the EDAC device framework. The ECC is enabled early in the boot
process in the platform specific code.
v2 changes:
- Split On-Chip RAM ECC platform initialization into separate patch from
L2 ECC platform initialization.
- Fix L2 cache dependency comments.
- Remove OCRAM node from dts and reference prior patch.
v3 changes:
- Move L2 cache & On-Chip RAM EDAC code into altera_edac.c
- Remove SDRAM module compile.
Thor Thayer (5):
arm: socfpga: Enable L2 Cache ECC on startup.
arm: socfpga: Enable OCRAM ECC on startup.
edac: altera: Remove SDRAM module compile
edac: altera: Add Altera L2 Cache and OCRAM EDAC Support
arm: dts: Add Altera L2 Cache and OCRAM EDAC
.../bindings/arm/altera/socfpga-l2-edac.txt | 15 +
.../bindings/arm/altera/socfpga-ocram-edac.txt | 16 +
MAINTAINERS | 2 +
arch/arm/boot/dts/socfpga.dtsi | 15 +-
arch/arm/mach-socfpga/Makefile | 2 +
arch/arm/mach-socfpga/l2_cache.c | 44 ++
arch/arm/mach-socfpga/l2_cache.h | 28 ++
arch/arm/mach-socfpga/ocram.c | 90 ++++
arch/arm/mach-socfpga/ocram.h | 28 ++
arch/arm/mach-socfpga/socfpga.c | 13 +-
drivers/edac/Kconfig | 18 +-
drivers/edac/Makefile | 5 +-
drivers/edac/altera_edac.c | 475 +++++++++++++++++++-
13 files changed, 745 insertions(+), 6 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/altera/socfpga-l2-edac.txt
create mode 100644 Documentation/devicetree/bindings/arm/altera/socfpga-ocram-edac.txt
create mode 100644 arch/arm/mach-socfpga/l2_cache.c
create mode 100644 arch/arm/mach-socfpga/l2_cache.h
create mode 100644 arch/arm/mach-socfpga/ocram.c
create mode 100644 arch/arm/mach-socfpga/ocram.h
--
1.7.9.5
From: Thor Thayer <[email protected]>
This patch enables the ECC for On-Chip RAM on machine
startup. The ECC has to be enabled before data is
is stored in memory otherwise the ECC will fail on
reads.
Signed-off-by: Thor Thayer <[email protected]>
---
v2: Split OCRAM ECC portion separately. Addition of iounmap()
and reorganization of gen_pool_free. Remove defconfig from patch.
v3: No change
---
MAINTAINERS | 1 +
arch/arm/mach-socfpga/Makefile | 1 +
arch/arm/mach-socfpga/ocram.c | 90 +++++++++++++++++++++++++++++++++++++++
arch/arm/mach-socfpga/ocram.h | 28 ++++++++++++
arch/arm/mach-socfpga/socfpga.c | 8 ++++
5 files changed, 128 insertions(+)
create mode 100644 arch/arm/mach-socfpga/ocram.c
create mode 100644 arch/arm/mach-socfpga/ocram.h
diff --git a/MAINTAINERS b/MAINTAINERS
index d0c7752..c6d390e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1408,6 +1408,7 @@ M: Thor Thayer <[email protected]>
S: Maintained
F: drivers/edac/altera_edac.
F: arch/arm/mach-socfpga/l2_cache.*
+F: arch/arm/mach-socfpga/ocram.*
ARM/STI ARCHITECTURE
M: Srinivas Kandagatla <[email protected]>
diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile
index 142609e..1552ca5 100644
--- a/arch/arm/mach-socfpga/Makefile
+++ b/arch/arm/mach-socfpga/Makefile
@@ -5,3 +5,4 @@
obj-y := socfpga.o
obj-$(CONFIG_SMP) += headsmp.o platsmp.o
obj-$(CONFIG_EDAC_ALTERA_L2C) += l2_cache.o
+obj-$(CONFIG_EDAC_ALTERA_OCRAM) += ocram.o
diff --git a/arch/arm/mach-socfpga/ocram.c b/arch/arm/mach-socfpga/ocram.c
new file mode 100644
index 0000000..9136009
--- /dev/null
+++ b/arch/arm/mach-socfpga/ocram.c
@@ -0,0 +1,90 @@
+/*
+ * Copyright Altera Corporation (C) 2014. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/clk-provider.h>
+#include <linux/genalloc.h>
+#include <linux/of_platform.h>
+
+#include "ocram.h"
+
+void socfpga_init_ocram_ecc(void)
+{
+ struct device_node *np;
+ const __be32 *prop;
+ u32 ocr_edac_addr, iram_addr, len;
+ void __iomem *mapped_ocr_edac_addr;
+ size_t size;
+ struct gen_pool *gp;
+
+ np = of_find_compatible_node(NULL, NULL, "altr,ocram-edac");
+ if (!np) {
+ pr_err("SOCFPGA: Unable to find altr,ocram-edac in dtb\n");
+ return;
+ }
+
+ prop = of_get_property(np, "reg", &size);
+ ocr_edac_addr = be32_to_cpup(prop++);
+ len = be32_to_cpup(prop);
+ if (!prop || size < sizeof(*prop)) {
+ pr_err("SOCFPGA: Unable to find OCRAM ECC mapping in dtb\n");
+ return;
+ }
+
+ gp = of_get_named_gen_pool(np, "iram", 0);
+ if (!gp) {
+ pr_err("SOCFPGA: OCRAM cannot find gen pool\n");
+ return;
+ }
+
+ np = of_find_compatible_node(NULL, NULL, "mmio-sram");
+ if (!np) {
+ pr_err("SOCFPGA: Unable to find mmio-sram in dtb\n");
+ return;
+ }
+ /* Determine the OCRAM address and size */
+ prop = of_get_property(np, "reg", &size);
+ iram_addr = be32_to_cpup(prop++);
+ len = be32_to_cpup(prop);
+
+ if (!prop || size < sizeof(*prop)) {
+ pr_err("SOCFPGA: Unable to find OCRAM mapping in dtb\n");
+ return;
+ }
+
+ iram_addr = gen_pool_alloc(gp, len);
+ if (iram_addr == 0) {
+ pr_err("SOCFPGA: cannot alloc from gen pool\n");
+ return;
+ }
+
+ memset((void *)iram_addr, 0, len);
+
+ gen_pool_free(gp, iram_addr, len);
+
+ mapped_ocr_edac_addr = ioremap(ocr_edac_addr, 4);
+ if (!mapped_ocr_edac_addr) {
+ pr_err("SOCFPGA: Unable to map OCRAM ecc regs.\n");
+ return;
+ }
+
+ /* Clear any pending OCRAM ECC interrupts, then enable ECC */
+ writel(0x18, mapped_ocr_edac_addr);
+ writel(0x19, mapped_ocr_edac_addr);
+
+ iounmap(mapped_ocr_edac_addr);
+
+ pr_debug("SOCFPGA: Success Initializing OCRAM\n");
+}
+
diff --git a/arch/arm/mach-socfpga/ocram.h b/arch/arm/mach-socfpga/ocram.h
new file mode 100644
index 0000000..f93cf84
--- /dev/null
+++ b/arch/arm/mach-socfpga/ocram.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright Altera Corporation (C) 2014. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef MACH_SOCFPGA_OCRAM_H
+#define MACH_SOCFPGA_OCRAM_H
+
+#ifdef CONFIG_EDAC_ALTERA_OCRAM
+void socfpga_init_ocram_ecc(void);
+#else
+inline void socfpga_init_ocram_ecc(void)
+{
+}
+#endif
+
+#endif /* #ifndef MACH_SOCFPGA_OCRAM_H */
diff --git a/arch/arm/mach-socfpga/socfpga.c b/arch/arm/mach-socfpga/socfpga.c
index af6413a..fb41aca 100644
--- a/arch/arm/mach-socfpga/socfpga.c
+++ b/arch/arm/mach-socfpga/socfpga.c
@@ -101,6 +101,13 @@ static void socfpga_cyclone5_restart(enum reboot_mode mode, const char *cmd)
writel(temp, rst_manager_base_addr + SOCFPGA_RSTMGR_CTRL);
}
+static void __init socfpga_cyclone5_init(void)
+{
+ of_platform_populate(NULL, of_default_bus_match_table,
+ NULL, NULL);
+ socfpga_init_ocram_ecc();
+}
+
static const char *altera_dt_match[] = {
"altr,socfpga",
NULL
@@ -112,6 +119,7 @@ DT_MACHINE_START(SOCFPGA, "Altera SOCFPGA")
.smp = smp_ops(socfpga_smp_ops),
.map_io = socfpga_map_io,
.init_irq = socfpga_init_irq,
+ .init_machine = socfpga_cyclone5_init,
.restart = socfpga_cyclone5_restart,
.dt_compat = altera_dt_match,
MACHINE_END
--
1.7.9.5
From: Thor Thayer <[email protected]>
The SDRAM EDAC requires SDRAM configuration/initialization before
SDRAM is accessed (in the preloader). Having a module compile is
not desired so force to be built into kernel.
Signed-off-by: Thor Thayer <[email protected]>
---
v3: Added in this version as a separate patch.
---
drivers/edac/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 7072c28..1719975 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -377,8 +377,8 @@ config EDAC_OCTEON_PCI
Cavium Octeon family of SOCs.
config EDAC_ALTERA_MC
- tristate "Altera SDRAM Memory Controller EDAC"
- depends on EDAC_MM_EDAC && ARCH_SOCFPGA
+ bool "Altera SDRAM Memory Controller EDAC"
+ depends on EDAC_MM_EDAC=y && ARCH_SOCFPGA
help
Support for error detection and correction on the
Altera SDRAM memory controller. Note that the
--
1.7.9.5
From: Thor Thayer <[email protected]>
Adding L2 Cache and On-Chip RAM EDAC support for the
Altera SoCs using the EDAC device model. The SDRAM
controller is using the Memory Controller model. All
Altera EDAC functions live in altera_edac.c.
Signed-off-by: Thor Thayer <[email protected]>
---
v2: Fix L2 dependency comments.
v3: Move OCRAM and L2 cache EDAC functions into altera_edac.c
instead of separate files.
---
drivers/edac/Kconfig | 14 ++
drivers/edac/Makefile | 5 +-
drivers/edac/altera_edac.c | 475 +++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 492 insertions(+), 2 deletions(-)
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 1719975..b145a52 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -385,4 +385,18 @@ config EDAC_ALTERA_MC
preloader must initialize the SDRAM before loading
the kernel.
+config EDAC_ALTERA_L2C
+ bool "Altera L2 Cache EDAC"
+ depends on EDAC_MM_EDAC=y && ARCH_SOCFPGA && CACHE_L2X0
+ help
+ Support for error detection and correction on the
+ Altera L2 cache Memory for Altera SoCs.
+
+config EDAC_ALTERA_OCRAM
+ bool "Altera On-Chip RAM EDAC"
+ depends on EDAC_MM_EDAC=y && ARCH_SOCFPGA && SRAM && GENERIC_ALLOCATOR
+ help
+ Support for error detection and correction on the
+ Altera On-Chip RAM Memory for Altera SoCs.
+
endif # EDAC
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 359aa49..fa8aebc 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -66,4 +66,7 @@ obj-$(CONFIG_EDAC_OCTEON_L2C) += octeon_edac-l2c.o
obj-$(CONFIG_EDAC_OCTEON_LMC) += octeon_edac-lmc.o
obj-$(CONFIG_EDAC_OCTEON_PCI) += octeon_edac-pci.o
-obj-$(CONFIG_EDAC_ALTERA_MC) += altera_edac.o
+alt_edac-y := altera_edac.o
+obj-$(CONFIG_EDAC_ALTERA_MC) += alt_edac.o
+obj-$(CONFIG_EDAC_ALTERA_L2C) += alt_edac.o
+obj-$(CONFIG_EDAC_ALTERA_OCRAM) += alt_edac.o
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 3c4929f..1ee8d94 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -17,8 +17,10 @@
* Adapted from the highbank_mc_edac driver.
*/
+#include <asm/cacheflush.h>
#include <linux/ctype.h>
#include <linux/edac.h>
+#include <linux/genalloc.h>
#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/mfd/syscon.h>
@@ -33,6 +35,7 @@
#define EDAC_MOD_STR "altera_edac"
#define EDAC_VERSION "1"
+#define EDAC_DEVICE "DEVICE"
/* SDRAM Controller CtrlCfg Register */
#define CTLCFG_OFST 0x00
@@ -107,6 +110,30 @@ struct altr_sdram_mc_data {
struct regmap *mc_vbase;
};
+/************************** EDAC Device Defines **************************/
+
+/* OCRAM ECC Management Group Defines */
+#define ALTR_MAN_GRP_OCRAM_ECC_OFFSET 0x04
+#define ALTR_OCR_ECC_EN_MASK 0x00000001
+#define ALTR_OCR_ECC_INJS_MASK 0x00000002
+#define ALTR_OCR_ECC_INJD_MASK 0x00000004
+#define ALTR_OCR_ECC_SERR_MASK 0x00000008
+#define ALTR_OCR_ECC_DERR_MASK 0x00000010
+
+/* MPU L2 Register Defines */
+#define ALTR_MPUL2_CONTROL_OFFSET 0x100
+#define ALTR_MPUL2_CTL_CACHE_EN_MASK 0x00000001
+
+/* L2 ECC Management Group Defines */
+#define ALTR_MAN_GRP_L2_ECC_OFFSET 0x00
+#define ALTR_L2_ECC_EN_MASK 0x00000001
+#define ALTR_L2_ECC_INJS_MASK 0x00000002
+#define ALTR_L2_ECC_INJD_MASK 0x00000004
+
+/*********************** EDAC Memory Controller Functions ****************/
+
+/* The SDRAM controller uses the EDAC Memory Controller framework. */
+
static irqreturn_t altr_sdram_mc_err_handler(int irq, void *dev_id)
{
struct mem_ctl_info *mci = dev_id;
@@ -405,6 +432,452 @@ static struct platform_driver altr_sdram_edac_driver = {
module_platform_driver(altr_sdram_edac_driver);
+/************************* EDAC Device Functions *************************/
+
+/*
+ * EDAC Device Functions (shared between various IPs).
+ * The discrete memories use the EDAC Device framework. The probe
+ * and error handling functions are very similar between memories
+ * so they are shared. The memory allocation and free for EDAC trigger
+ * testing are different for each memory.
+ */
+
+const struct edac_device_prv_data ocramecc_data;
+const struct edac_device_prv_data l2ecc_data;
+
+struct edac_device_prv_data {
+ int (*setup)(struct platform_device *pdev, void __iomem *base);
+ int ce_clear_mask;
+ int ue_clear_mask;
+#ifdef CONFIG_EDAC_DEBUG
+ struct edac_dev_sysfs_attribute *eccmgr_sysfs_attr;
+ void * (*alloc_mem)(size_t size, void **other);
+ void (*free_mem)(void *p, size_t size, void *other);
+ int ecc_enable_mask;
+ int ce_set_mask;
+ int ue_set_mask;
+ int trig_alloc_sz;
+#endif
+};
+
+struct altr_edac_device_dev {
+ void __iomem *base;
+ int sb_irq;
+ int db_irq;
+ const struct edac_device_prv_data *data;
+ char *edac_dev_name;
+};
+
+static irqreturn_t altr_edac_device_handler(int irq, void *dev_id)
+{
+ struct edac_device_ctl_info *dci = dev_id;
+ struct altr_edac_device_dev *drvdata = dci->pvt_info;
+ const struct edac_device_prv_data *priv = drvdata->data;
+
+ if (irq == drvdata->sb_irq) {
+ if (priv->ce_clear_mask)
+ writel(priv->ce_clear_mask, drvdata->base);
+ edac_device_handle_ce(dci, 0, 0, drvdata->edac_dev_name);
+ }
+ if (irq == drvdata->db_irq) {
+ if (priv->ue_clear_mask)
+ writel(priv->ue_clear_mask, drvdata->base);
+ edac_device_handle_ue(dci, 0, 0, drvdata->edac_dev_name);
+ panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
+ }
+
+ return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_EDAC_DEBUG
+ssize_t altr_edac_device_trig(struct edac_device_ctl_info *edac_dci,
+ const char *buffer, size_t count)
+{
+ u32 *ptemp, i, error_mask;
+ int result = 0;
+ unsigned long flags;
+ struct altr_edac_device_dev *drvdata = edac_dci->pvt_info;
+ const struct edac_device_prv_data *priv = drvdata->data;
+ void *generic_ptr = edac_dci->dev;
+
+ if (!priv->alloc_mem)
+ return -ENOMEM;
+
+ /* Note that generic_ptr is initialized to the device * but in
+ * some init_functions, this is overridden and returns data */
+ ptemp = priv->alloc_mem(priv->trig_alloc_sz, &generic_ptr);
+ if (!ptemp) {
+ edac_printk(KERN_ERR, EDAC_DEVICE,
+ "Inject: Buffer Allocation error\n");
+ return -ENOMEM;
+ }
+
+ if (count == 3)
+ error_mask = priv->ue_set_mask;
+ else
+ error_mask = priv->ce_set_mask;
+
+ edac_printk(KERN_ALERT, EDAC_DEVICE,
+ "Trigger Error Mask (0x%X)\n", error_mask);
+
+ local_irq_save(flags);
+ /* write ECC corrupted data out. */
+ for (i = 0; i < (priv->trig_alloc_sz/sizeof(*ptemp)); i++) {
+ /* Read data so we're in the correct state */
+ rmb();
+ if (ACCESS_ONCE(ptemp[i]))
+ result = -1;
+ /* Toggle Error bit (it is latched), leave ECC enabled */
+ writel(error_mask, drvdata->base);
+ writel(priv->ecc_enable_mask, drvdata->base);
+ ptemp[i] = i;
+ }
+ /* Ensure it has been written out */
+ wmb();
+ local_irq_restore(flags);
+
+ if (result)
+ edac_printk(KERN_ERR, EDAC_DEVICE, "Mem Not Cleared\n");
+
+ /* Read out written data. ECC error caused here */
+ for (i = 0; i < 16; i++)
+ if (ACCESS_ONCE(ptemp[i]) != i)
+ edac_printk(KERN_ERR, EDAC_DEVICE,
+ "Mem Doesn't match\n");
+
+ if (priv->free_mem)
+ priv->free_mem(ptemp, priv->trig_alloc_sz, generic_ptr);
+
+ return count;
+}
+
+static void altr_set_sysfs_attr(struct edac_device_ctl_info *edac_dci,
+ const struct edac_device_prv_data *priv)
+{
+ struct edac_dev_sysfs_attribute *ecc_attr = priv->eccmgr_sysfs_attr;
+
+ if (ecc_attr) {
+ edac_dci->sysfs_attributes = ecc_attr;
+ edac_printk(KERN_ERR, EDAC_DEVICE, "Set SysFS trigger\n");
+ }
+}
+#else
+static void altr_set_sysfs_attr(struct edac_device_ctl_info *edac_dci,
+ const struct edac_device_prv_data *priv)
+{}
+#endif /* #ifdef CONFIG_EDAC_DEBUG */
+
+static const struct of_device_id altr_edac_device_of_match[] = {
+#ifdef CONFIG_EDAC_ALTERA_L2C
+ { .compatible = "altr,l2-edac", .data = (void *)&l2ecc_data },
+#endif
+#ifdef CONFIG_EDAC_ALTERA_OCRAM
+ { .compatible = "altr,ocram-edac", .data = (void *)&ocramecc_data },
+#endif
+ {},
+};
+MODULE_DEVICE_TABLE(of, altr_edac_device_of_match);
+
+/*
+ * altr_edac_device_probe()
+ * This is a generic EDAC device driver that will support
+ * various Altera memory devices such as the L2 cache ECC and
+ * OCRAM ECC as well as the memories for other peripherals.
+ * Module specific initialization is done by passing the
+ * function index in the device tree.
+ */
+static int altr_edac_device_probe(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *dci;
+ struct altr_edac_device_dev *drvdata;
+ struct resource *r;
+ int res = 0;
+ struct device_node *np = pdev->dev.of_node;
+ char *ecc_name = (char *)np->name;
+ static int dev_instance;
+
+ if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL))
+ return -ENOMEM;
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!r) {
+ edac_printk(KERN_ERR, EDAC_DEVICE,
+ "Unable to get mem resource\n");
+ return -ENODEV;
+ }
+
+ if (!devm_request_mem_region(&pdev->dev, r->start, resource_size(r),
+ dev_name(&pdev->dev))) {
+ edac_printk(KERN_ERR, EDAC_DEVICE,
+ "%s:Error requesting mem region\n", ecc_name);
+ return -EBUSY;
+ }
+
+ dci = edac_device_alloc_ctl_info(sizeof(*drvdata), ecc_name,
+ 1, ecc_name, 1, 0, NULL, 0,
+ dev_instance++);
+
+ if (!dci)
+ return -ENOMEM;
+
+ drvdata = dci->pvt_info;
+ dci->dev = &pdev->dev;
+ platform_set_drvdata(pdev, dci);
+ drvdata->edac_dev_name = ecc_name;
+
+ drvdata->base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
+ if (!drvdata->base) {
+ edac_printk(KERN_ERR, EDAC_DEVICE,
+ "%s:Unable to map regs\n", ecc_name);
+ return -ENOMEM;
+ }
+
+ /* Get driver specific data for this EDAC device */
+ drvdata->data = of_match_node(altr_edac_device_of_match, np)->data;
+
+ /* Check specific dependencies for the module */
+ if (drvdata->data->setup) {
+ res = drvdata->data->setup(pdev, drvdata->base);
+ if (res < 0)
+ goto err;
+ }
+
+ drvdata->sb_irq = platform_get_irq(pdev, 0);
+ res = devm_request_irq(&pdev->dev, drvdata->sb_irq,
+ altr_edac_device_handler,
+ 0, dev_name(&pdev->dev), dci);
+ if (res < 0)
+ goto err;
+
+ drvdata->db_irq = platform_get_irq(pdev, 1);
+ res = devm_request_irq(&pdev->dev, drvdata->db_irq,
+ altr_edac_device_handler,
+ 0, dev_name(&pdev->dev), dci);
+ if (res < 0)
+ goto err;
+
+ dci->mod_name = "EDAC_DEVICE";
+ dci->dev_name = drvdata->edac_dev_name;
+
+ altr_set_sysfs_attr(dci, drvdata->data);
+
+ if (edac_device_add_device(dci))
+ goto err;
+
+ devres_close_group(&pdev->dev, NULL);
+
+ return 0;
+err:
+ devres_release_group(&pdev->dev, NULL);
+ edac_device_free_ctl_info(dci);
+
+ return res;
+}
+
+static int altr_edac_device_remove(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
+
+ edac_device_del_device(&pdev->dev);
+ edac_device_free_ctl_info(dci);
+
+ return 0;
+}
+
+static struct platform_driver altr_edac_device_driver = {
+ .probe = altr_edac_device_probe,
+ .remove = altr_edac_device_remove,
+ .driver = {
+ .name = "altr_edac_device",
+ .of_match_table = altr_edac_device_of_match,
+ },
+};
+module_platform_driver(altr_edac_device_driver);
+
+/*********************** OCRAM EDAC Device Functions *********************/
+
+#ifdef CONFIG_EDAC_ALTERA_OCRAM
+
+#ifdef CONFIG_EDAC_DEBUG
+static void *ocram_alloc_mem(size_t size, void **other)
+{
+ struct device_node *np;
+ struct gen_pool *gp;
+ void *sram_addr;
+
+ np = of_find_compatible_node(NULL, NULL, "altr,ocram-edac");
+ if (!np)
+ return NULL;
+
+ gp = of_get_named_gen_pool(np, "iram", 0);
+ if (!gp)
+ return NULL;
+ *other = gp;
+
+ sram_addr = (void *)gen_pool_alloc(gp, size);
+ if (!sram_addr)
+ return NULL;
+
+ memset(sram_addr, 0, size);
+ wmb(); /* Ensure data is written out */
+
+ return sram_addr;
+}
+
+static void ocram_free_mem(void *p, size_t size, void *other)
+{
+ gen_pool_free((struct gen_pool *)other, (u32)p, size);
+}
+
+static struct edac_dev_sysfs_attribute altr_ocr_sysfs_attributes[] = {
+ {
+ .attr = { .name = "altr_ocram_trigger",
+ .mode = (S_IRUGO | S_IWUSR) },
+ .show = NULL,
+ .store = altr_edac_device_trig
+ },
+ {
+ .attr = {.name = NULL }
+ }
+};
+#endif /* #ifdef CONFIG_EDAC_DEBUG */
+
+/*
+ * altr_ocram_dependencies()
+ * Test for OCRAM cache ECC dependencies upon entry because
+ * platform specific startup should have initialized the
+ * On-Chip RAM memory and enabled the ECC.
+ * Can't turn on ECC here because accessing un-initialized
+ * memory will cause CE/UE errors possibly causing an ABORT.
+ */
+static int altr_ocram_dependencies(struct platform_device *pdev,
+ void __iomem *base)
+{
+ u32 control;
+
+ control = readl(base) & ALTR_OCR_ECC_EN_MASK;
+ if (!control) {
+ dev_err(&pdev->dev,
+ "OCRAM: No ECC present or ECC disabled.\n");
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+const struct edac_device_prv_data ocramecc_data = {
+ .setup = altr_ocram_dependencies,
+ .ce_clear_mask = (ALTR_OCR_ECC_EN_MASK | ALTR_OCR_ECC_SERR_MASK),
+ .ue_clear_mask = (ALTR_OCR_ECC_EN_MASK | ALTR_OCR_ECC_DERR_MASK),
+#ifdef CONFIG_EDAC_DEBUG
+ .eccmgr_sysfs_attr = altr_ocr_sysfs_attributes,
+ .alloc_mem = ocram_alloc_mem,
+ .free_mem = ocram_free_mem,
+ .ecc_enable_mask = ALTR_OCR_ECC_EN_MASK,
+ .ce_set_mask = (ALTR_OCR_ECC_EN_MASK | ALTR_OCR_ECC_INJS_MASK),
+ .ue_set_mask = (ALTR_OCR_ECC_EN_MASK | ALTR_OCR_ECC_INJD_MASK),
+ .trig_alloc_sz = (32 * sizeof(u32)),
+#endif
+};
+
+#endif /* #ifdef CONFIG_EDAC_ALTERA_OCRAM */
+
+/********************* L2 Cache EDAC Device Functions ********************/
+
+#ifdef CONFIG_EDAC_ALTERA_L2C
+
+#ifdef CONFIG_EDAC_DEBUG
+static void *l2_alloc_mem(size_t size, void **other)
+{
+ struct device *dev = *other;
+ void *ptemp = devm_kzalloc(dev, size, GFP_KERNEL);
+
+ if (!ptemp)
+ return NULL;
+
+ /* Make sure everything is written out */
+ wmb();
+ flush_cache_all();
+
+ return ptemp;
+}
+
+static void l2_free_mem(void *p, size_t size, void *other)
+{
+ struct device *dev = other;
+
+ if (dev && p)
+ devm_kfree(dev, p);
+}
+
+static struct edac_dev_sysfs_attribute altr_l2_sysfs_attributes[] = {
+ {
+ .attr = { .name = "altr_l2_trigger",
+ .mode = (S_IRUGO | S_IWUSR) },
+ .show = NULL,
+ .store = altr_edac_device_trig
+ },
+ {
+ .attr = {.name = NULL }
+ }
+};
+#endif /* #ifdef CONFIG_EDAC_DEBUG */
+
+/*
+ * altr_l2_dependencies()
+ * Test for L2 cache ECC dependencies upon entry because
+ * platform specific startup should have initialized the L2
+ * memory and enabled the ECC.
+ * Can't turn on ECC here because accessing un-initialized
+ * memory will cause CE/UE errors possibly causing an ABORT.
+ * Bail if ECC is not on.
+ * Test For 1) L2 ECC is enabled and 2) L2 Cache is enabled.
+ */
+static int altr_l2_dependencies(struct platform_device *pdev,
+ void __iomem *base)
+{
+ u32 control;
+ struct regmap *l2_vbase;
+
+ control = readl(base) & ALTR_L2_ECC_EN_MASK;
+ if (!control) {
+ dev_err(&pdev->dev, "L2: No ECC present, or ECC disabled\n");
+ return -ENODEV;
+ }
+
+ l2_vbase = syscon_regmap_lookup_by_compatible("arm,pl310-cache");
+ if (IS_ERR(l2_vbase)) {
+ dev_err(&pdev->dev,
+ "L2 ECC:regmap for arm,pl310-cache lookup failed.\n");
+ return -ENODEV;
+ }
+
+ regmap_read(l2_vbase, ALTR_MPUL2_CONTROL_OFFSET, &control);
+ if (!(control & ALTR_MPUL2_CTL_CACHE_EN_MASK)) {
+ dev_err(&pdev->dev, "L2: Cache disabled\n");
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+const struct edac_device_prv_data l2ecc_data = {
+ .setup = altr_l2_dependencies,
+ .ce_clear_mask = 0,
+ .ue_clear_mask = 0,
+#ifdef CONFIG_EDAC_DEBUG
+ .eccmgr_sysfs_attr = altr_l2_sysfs_attributes,
+ .alloc_mem = l2_alloc_mem,
+ .free_mem = l2_free_mem,
+ .ecc_enable_mask = ALTR_L2_ECC_EN_MASK,
+ .ce_set_mask = (ALTR_L2_ECC_EN_MASK | ALTR_L2_ECC_INJS_MASK),
+ .ue_set_mask = (ALTR_L2_ECC_EN_MASK | ALTR_L2_ECC_INJD_MASK),
+ .trig_alloc_sz = 4 * 1024,
+#endif
+};
+
+#endif /* #ifdef CONFIG_EDAC_ALTERA_L2C */
+
MODULE_LICENSE("GPL v2");
MODULE_AUTHOR("Thor Thayer");
-MODULE_DESCRIPTION("EDAC Driver for Altera SDRAM Controller");
+MODULE_DESCRIPTION("EDAC Driver for Altera Memories");
--
1.7.9.5
From: Thor Thayer <[email protected]>
Adding the device tree entries and bindings needed to support
the Altera L2 cache and On-Chip RAM EDAC. This patch relies upon
an earlier patch to declare and setup On-chip RAM properly.
http://www.spinics.net/lists/devicetree/msg51117.html
Signed-off-by: Thor Thayer <[email protected]>
---
v2: Remove OCRAM declaration and reference prior patch.
v3: No Change
---
.../bindings/arm/altera/socfpga-l2-edac.txt | 15 +++++++++++++++
.../bindings/arm/altera/socfpga-ocram-edac.txt | 16 ++++++++++++++++
arch/arm/boot/dts/socfpga.dtsi | 15 ++++++++++++++-
3 files changed, 45 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/arm/altera/socfpga-l2-edac.txt
create mode 100644 Documentation/devicetree/bindings/arm/altera/socfpga-ocram-edac.txt
diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-l2-edac.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-l2-edac.txt
new file mode 100644
index 0000000..35b19e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/altera/socfpga-l2-edac.txt
@@ -0,0 +1,15 @@
+Altera SoCFPGA L2 cache Error Detection and Correction [EDAC]
+
+Required Properties:
+- compatible : Should be "altr,l2-edac"
+- reg : Address and size for ECC error interrupt clear registers.
+- interrupts : Should be single bit error interrupt, then double bit error
+ interrupt. Note the rising edge type.
+
+Example:
+
+ l2edac@ffd08140 {
+ compatible = "altr,l2-edac";
+ reg = <0xffd08140 0x4>;
+ interrupts = <0 36 1>, <0 37 1>;
+ };
diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-ocram-edac.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-ocram-edac.txt
new file mode 100644
index 0000000..31ab205
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/altera/socfpga-ocram-edac.txt
@@ -0,0 +1,16 @@
+Altera SoCFPGA On-Chip RAM Error Detection and Correction [EDAC]
+
+OCRAM ECC Required Properties:
+- compatible : Should be "altr,ocram-edac"
+- reg : Address and size for ECC error interrupt clear registers.
+- iram : phandle to On-Chip RAM definition.
+- interrupts : Should be single bit error interrupt, then double bit error
+ interrupt. Note the rising edge type.
+
+Example:
+ ocramedac@ffd08144 {
+ compatible = "altr,ocram-edac";
+ reg = <0xffd08144 0x4>;
+ iram = <&ocram>;
+ interrupts = <0 178 1>, <0 179 1>;
+ };
diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 6af96ed..32c63a3 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -618,8 +618,21 @@
interrupts = <0 39 4>;
};
+ l2edac@ffd08140 {
+ compatible = "altr,l2-edac";
+ reg = <0xffd08140 0x4>;
+ interrupts = <0 36 1>, <0 37 1>;
+ };
+
+ ocramedac@ffd08144 {
+ compatible = "altr,ocram-edac";
+ reg = <0xffd08144 0x4>;
+ iram = <&ocram>;
+ interrupts = <0 178 1>, <0 179 1>;
+ };
+
L2: l2-cache@fffef000 {
- compatible = "arm,pl310-cache";
+ compatible = "arm,pl310-cache", "syscon";
reg = <0xfffef000 0x1000>;
interrupts = <0 38 0x04>;
cache-unified;
--
1.7.9.5
From: Thor Thayer <[email protected]>
This patch enables the ECC for L2 cache on machine
startup. The ECC has to be enabled before data is
is stored in memory otherwise the ECC will fail on
reads.
Signed-off-by: Thor Thayer <[email protected]>
---
v2: Split OCRAM initialization into separate patch.
v3: No change
---
MAINTAINERS | 1 +
arch/arm/mach-socfpga/Makefile | 1 +
arch/arm/mach-socfpga/l2_cache.c | 44 ++++++++++++++++++++++++++++++++++++++
arch/arm/mach-socfpga/l2_cache.h | 28 ++++++++++++++++++++++++
arch/arm/mach-socfpga/socfpga.c | 5 ++++-
5 files changed, 78 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/mach-socfpga/l2_cache.c
create mode 100644 arch/arm/mach-socfpga/l2_cache.h
diff --git a/MAINTAINERS b/MAINTAINERS
index ee1bc5b..d0c7752 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1407,6 +1407,7 @@ ARM/SOCFPGA EDAC SUPPORT
M: Thor Thayer <[email protected]>
S: Maintained
F: drivers/edac/altera_edac.
+F: arch/arm/mach-socfpga/l2_cache.*
ARM/STI ARCHITECTURE
M: Srinivas Kandagatla <[email protected]>
diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile
index 6dd7a93..142609e 100644
--- a/arch/arm/mach-socfpga/Makefile
+++ b/arch/arm/mach-socfpga/Makefile
@@ -4,3 +4,4 @@
obj-y := socfpga.o
obj-$(CONFIG_SMP) += headsmp.o platsmp.o
+obj-$(CONFIG_EDAC_ALTERA_L2C) += l2_cache.o
diff --git a/arch/arm/mach-socfpga/l2_cache.c b/arch/arm/mach-socfpga/l2_cache.c
new file mode 100644
index 0000000..8e109f3
--- /dev/null
+++ b/arch/arm/mach-socfpga/l2_cache.c
@@ -0,0 +1,44 @@
+/*
+ * Copyright Altera Corporation (C) 2014. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/clk-provider.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+
+#include "l2_cache.h"
+
+void socfpga_init_l2_ecc(void)
+{
+ struct device_node *np;
+ void __iomem *mapped_l2_edac_addr;
+
+ np = of_find_compatible_node(NULL, NULL, "altr,l2-edac");
+ if (!np) {
+ pr_err("SOCFPGA: Unable to find altr,l2-edac in dtb\n");
+ return;
+ }
+
+ mapped_l2_edac_addr = of_iomap(np, 0);
+ if (!mapped_l2_edac_addr) {
+ pr_err("SOCFPGA: Unable to find L2 ECC mapping in dtb\n");
+ return;
+ }
+
+ /* Enable ECC */
+ writel(0x01, mapped_l2_edac_addr);
+
+ pr_debug("SOCFPGA: Success Initializing L2 cache ECC\n");
+}
+
diff --git a/arch/arm/mach-socfpga/l2_cache.h b/arch/arm/mach-socfpga/l2_cache.h
new file mode 100644
index 0000000..58e140d
--- /dev/null
+++ b/arch/arm/mach-socfpga/l2_cache.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright Altera Corporation (C) 2014. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef MACH_SOCFPGA_L2_CACHE_H
+#define MACH_SOCFPGA_L2_CACHE_H
+
+#ifdef CONFIG_EDAC_ALTERA_L2C
+void socfpga_init_l2_ecc(void);
+#else
+inline void socfpga_init_l2_ecc(void)
+{
+}
+#endif
+
+#endif /* #ifndef MACH_SOCFPGA_L2_CACHE_H */
diff --git a/arch/arm/mach-socfpga/socfpga.c b/arch/arm/mach-socfpga/socfpga.c
index adbf383..af6413a 100644
--- a/arch/arm/mach-socfpga/socfpga.c
+++ b/arch/arm/mach-socfpga/socfpga.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012 Altera Corporation
+ * Copyright (C) 2012;2014 Altera Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -25,6 +25,8 @@
#include <asm/mach/map.h>
#include "core.h"
+#include "l2_cache.h"
+#include "ocram.h"
void __iomem *socfpga_scu_base_addr = ((void __iomem *)(SOCFPGA_SCU_VIRT_BASE));
void __iomem *sys_manager_base_addr;
@@ -83,6 +85,7 @@ static void __init socfpga_init_irq(void)
{
irqchip_init();
socfpga_sysmgr_init();
+ socfpga_init_l2_ecc();
}
static void socfpga_cyclone5_restart(enum reboot_mode mode, const char *cmd)
--
1.7.9.5
On Thu, Oct 30, 2014 at 10:32:10AM -0500, [email protected] wrote:
> From: Thor Thayer <[email protected]>
>
> Adding L2 Cache and On-Chip RAM EDAC support for the
> Altera SoCs using the EDAC device model. The SDRAM
> controller is using the Memory Controller model. All
> Altera EDAC functions live in altera_edac.c.
>
> Signed-off-by: Thor Thayer <[email protected]>
> ---
> v2: Fix L2 dependency comments.
>
> v3: Move OCRAM and L2 cache EDAC functions into altera_edac.c
> instead of separate files.
> ---
> drivers/edac/Kconfig | 14 ++
> drivers/edac/Makefile | 5 +-
> drivers/edac/altera_edac.c | 475 +++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 492 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 1719975..b145a52 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -385,4 +385,18 @@ config EDAC_ALTERA_MC
> preloader must initialize the SDRAM before loading
> the kernel.
>
> +config EDAC_ALTERA_L2C
> + bool "Altera L2 Cache EDAC"
> + depends on EDAC_MM_EDAC=y && ARCH_SOCFPGA && CACHE_L2X0
> + help
> + Support for error detection and correction on the
> + Altera L2 cache Memory for Altera SoCs.
> +
> +config EDAC_ALTERA_OCRAM
> + bool "Altera On-Chip RAM EDAC"
> + depends on EDAC_MM_EDAC=y && ARCH_SOCFPGA && SRAM && GENERIC_ALLOCATOR
> + help
> + Support for error detection and correction on the
> + Altera On-Chip RAM Memory for Altera SoCs.
Why are those separate config items? Why not switch on EDAC_ALTERA_MC
and get it all?
We can always split them out later, if needed but I don't think
we should burden the user with unnecessary questions if it is not
absolutely necessary.
> +
> endif # EDAC
> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
> index 359aa49..fa8aebc 100644
> --- a/drivers/edac/Makefile
> +++ b/drivers/edac/Makefile
> @@ -66,4 +66,7 @@ obj-$(CONFIG_EDAC_OCTEON_L2C) += octeon_edac-l2c.o
> obj-$(CONFIG_EDAC_OCTEON_LMC) += octeon_edac-lmc.o
> obj-$(CONFIG_EDAC_OCTEON_PCI) += octeon_edac-pci.o
>
> -obj-$(CONFIG_EDAC_ALTERA_MC) += altera_edac.o
> +alt_edac-y := altera_edac.o
> +obj-$(CONFIG_EDAC_ALTERA_MC) += alt_edac.o
> +obj-$(CONFIG_EDAC_ALTERA_L2C) += alt_edac.o
> +obj-$(CONFIG_EDAC_ALTERA_OCRAM) += alt_edac.o
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index 3c4929f..1ee8d94 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c
> @@ -17,8 +17,10 @@
> * Adapted from the highbank_mc_edac driver.
> */
>
> +#include <asm/cacheflush.h>
> #include <linux/ctype.h>
> #include <linux/edac.h>
> +#include <linux/genalloc.h>
> #include <linux/interrupt.h>
> #include <linux/kernel.h>
> #include <linux/mfd/syscon.h>
> @@ -33,6 +35,7 @@
>
> #define EDAC_MOD_STR "altera_edac"
> #define EDAC_VERSION "1"
> +#define EDAC_DEVICE "DEVICE"
That name is not very descriptive.
> /* SDRAM Controller CtrlCfg Register */
> #define CTLCFG_OFST 0x00
> @@ -107,6 +110,30 @@ struct altr_sdram_mc_data {
> struct regmap *mc_vbase;
> };
>
> +/************************** EDAC Device Defines **************************/
> +
> +/* OCRAM ECC Management Group Defines */
> +#define ALTR_MAN_GRP_OCRAM_ECC_OFFSET 0x04
> +#define ALTR_OCR_ECC_EN_MASK 0x00000001
> +#define ALTR_OCR_ECC_INJS_MASK 0x00000002
> +#define ALTR_OCR_ECC_INJD_MASK 0x00000004
> +#define ALTR_OCR_ECC_SERR_MASK 0x00000008
> +#define ALTR_OCR_ECC_DERR_MASK 0x00000010
> +
> +/* MPU L2 Register Defines */
> +#define ALTR_MPUL2_CONTROL_OFFSET 0x100
> +#define ALTR_MPUL2_CTL_CACHE_EN_MASK 0x00000001
> +
> +/* L2 ECC Management Group Defines */
> +#define ALTR_MAN_GRP_L2_ECC_OFFSET 0x00
> +#define ALTR_L2_ECC_EN_MASK 0x00000001
> +#define ALTR_L2_ECC_INJS_MASK 0x00000002
> +#define ALTR_L2_ECC_INJD_MASK 0x00000004
You can use BIT() for those.
> +
> +/*********************** EDAC Memory Controller Functions ****************/
> +
> +/* The SDRAM controller uses the EDAC Memory Controller framework. */
> +
> static irqreturn_t altr_sdram_mc_err_handler(int irq, void *dev_id)
> {
> struct mem_ctl_info *mci = dev_id;
> @@ -405,6 +432,452 @@ static struct platform_driver altr_sdram_edac_driver = {
>
> module_platform_driver(altr_sdram_edac_driver);
>
> +/************************* EDAC Device Functions *************************/
> +
> +/*
> + * EDAC Device Functions (shared between various IPs).
> + * The discrete memories use the EDAC Device framework. The probe
> + * and error handling functions are very similar between memories
> + * so they are shared. The memory allocation and free for EDAC trigger
> + * testing are different for each memory.
> + */
> +
> +const struct edac_device_prv_data ocramecc_data;
> +const struct edac_device_prv_data l2ecc_data;
> +
> +struct edac_device_prv_data {
> + int (*setup)(struct platform_device *pdev, void __iomem *base);
> + int ce_clear_mask;
> + int ue_clear_mask;
> +#ifdef CONFIG_EDAC_DEBUG
> + struct edac_dev_sysfs_attribute *eccmgr_sysfs_attr;
> + void * (*alloc_mem)(size_t size, void **other);
> + void (*free_mem)(void *p, size_t size, void *other);
> + int ecc_enable_mask;
> + int ce_set_mask;
> + int ue_set_mask;
> + int trig_alloc_sz;
> +#endif
> +};
> +
> +struct altr_edac_device_dev {
> + void __iomem *base;
> + int sb_irq;
> + int db_irq;
> + const struct edac_device_prv_data *data;
> + char *edac_dev_name;
> +};
> +
> +static irqreturn_t altr_edac_device_handler(int irq, void *dev_id)
> +{
> + struct edac_device_ctl_info *dci = dev_id;
> + struct altr_edac_device_dev *drvdata = dci->pvt_info;
> + const struct edac_device_prv_data *priv = drvdata->data;
> +
> + if (irq == drvdata->sb_irq) {
> + if (priv->ce_clear_mask)
> + writel(priv->ce_clear_mask, drvdata->base);
> + edac_device_handle_ce(dci, 0, 0, drvdata->edac_dev_name);
> + }
> + if (irq == drvdata->db_irq) {
> + if (priv->ue_clear_mask)
> + writel(priv->ue_clear_mask, drvdata->base);
> + edac_device_handle_ue(dci, 0, 0, drvdata->edac_dev_name);
> + panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +#ifdef CONFIG_EDAC_DEBUG
> +ssize_t altr_edac_device_trig(struct edac_device_ctl_info *edac_dci,
> + const char *buffer, size_t count)
> +{
> + u32 *ptemp, i, error_mask;
> + int result = 0;
> + unsigned long flags;
> + struct altr_edac_device_dev *drvdata = edac_dci->pvt_info;
> + const struct edac_device_prv_data *priv = drvdata->data;
> + void *generic_ptr = edac_dci->dev;
> +
> + if (!priv->alloc_mem)
> + return -ENOMEM;
> +
> + /* Note that generic_ptr is initialized to the device * but in
> + * some init_functions, this is overridden and returns data */
Kernel comment style:
/*
* Blabla.
* More Bla.
*/
> + ptemp = priv->alloc_mem(priv->trig_alloc_sz, &generic_ptr);
> + if (!ptemp) {
> + edac_printk(KERN_ERR, EDAC_DEVICE,
> + "Inject: Buffer Allocation error\n");
> + return -ENOMEM;
> + }
> +
> + if (count == 3)
Magic number 3.
/me needs crystal ball :)
> + error_mask = priv->ue_set_mask;
> + else
> + error_mask = priv->ce_set_mask;
> +
> + edac_printk(KERN_ALERT, EDAC_DEVICE,
> + "Trigger Error Mask (0x%X)\n", error_mask);
> +
> + local_irq_save(flags);
> + /* write ECC corrupted data out. */
> + for (i = 0; i < (priv->trig_alloc_sz/sizeof(*ptemp)); i++) {
> + /* Read data so we're in the correct state */
> + rmb();
> + if (ACCESS_ONCE(ptemp[i]))
> + result = -1;
> + /* Toggle Error bit (it is latched), leave ECC enabled */
> + writel(error_mask, drvdata->base);
> + writel(priv->ecc_enable_mask, drvdata->base);
> + ptemp[i] = i;
> + }
> + /* Ensure it has been written out */
> + wmb();
> + local_irq_restore(flags);
Doesn't the interrupt reenabling on ARM already cause outstanding writes
to commit so that you don't really need the memory barrier?
> +
> + if (result)
> + edac_printk(KERN_ERR, EDAC_DEVICE, "Mem Not Cleared\n");
> +
> + /* Read out written data. ECC error caused here */
> + for (i = 0; i < 16; i++)
Magic number 16. Where's my ball again?! :)
> + if (ACCESS_ONCE(ptemp[i]) != i)
> + edac_printk(KERN_ERR, EDAC_DEVICE,
> + "Mem Doesn't match\n");
I guess injectors should know what this printk means? I can assume
it says something about inject data written doesn't match what we're
reading?
> +
> + if (priv->free_mem)
> + priv->free_mem(ptemp, priv->trig_alloc_sz, generic_ptr);
> +
> + return count;
> +}
> +
> +static void altr_set_sysfs_attr(struct edac_device_ctl_info *edac_dci,
> + const struct edac_device_prv_data *priv)
> +{
> + struct edac_dev_sysfs_attribute *ecc_attr = priv->eccmgr_sysfs_attr;
> +
> + if (ecc_attr) {
> + edac_dci->sysfs_attributes = ecc_attr;
> + edac_printk(KERN_ERR, EDAC_DEVICE, "Set SysFS trigger\n");
Why do we have to say this here? Debugging leftovers maybe?
> + }
> +}
> +#else
> +static void altr_set_sysfs_attr(struct edac_device_ctl_info *edac_dci,
> + const struct edac_device_prv_data *priv)
> +{}
> +#endif /* #ifdef CONFIG_EDAC_DEBUG */
> +
> +static const struct of_device_id altr_edac_device_of_match[] = {
> +#ifdef CONFIG_EDAC_ALTERA_L2C
> + { .compatible = "altr,l2-edac", .data = (void *)&l2ecc_data },
> +#endif
> +#ifdef CONFIG_EDAC_ALTERA_OCRAM
> + { .compatible = "altr,ocram-edac", .data = (void *)&ocramecc_data },
> +#endif
If you enable l2c and ocram by default, you can save yourself the ugly
ifdeffery here too.
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, altr_edac_device_of_match);
> +
> +/*
> + * altr_edac_device_probe()
> + * This is a generic EDAC device driver that will support
> + * various Altera memory devices such as the L2 cache ECC and
> + * OCRAM ECC as well as the memories for other peripherals.
> + * Module specific initialization is done by passing the
> + * function index in the device tree.
> + */
> +static int altr_edac_device_probe(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci;
> + struct altr_edac_device_dev *drvdata;
> + struct resource *r;
> + int res = 0;
> + struct device_node *np = pdev->dev.of_node;
> + char *ecc_name = (char *)np->name;
> + static int dev_instance;
> +
> + if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL))
> + return -ENOMEM;
> +
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!r) {
> + edac_printk(KERN_ERR, EDAC_DEVICE,
> + "Unable to get mem resource\n");
> + return -ENODEV;
> + }
> +
> + if (!devm_request_mem_region(&pdev->dev, r->start, resource_size(r),
> + dev_name(&pdev->dev))) {
> + edac_printk(KERN_ERR, EDAC_DEVICE,
> + "%s:Error requesting mem region\n", ecc_name);
> + return -EBUSY;
> + }
> +
> + dci = edac_device_alloc_ctl_info(sizeof(*drvdata), ecc_name,
> + 1, ecc_name, 1, 0, NULL, 0,
> + dev_instance++);
> +
> + if (!dci)
> + return -ENOMEM;
> +
> + drvdata = dci->pvt_info;
> + dci->dev = &pdev->dev;
> + platform_set_drvdata(pdev, dci);
> + drvdata->edac_dev_name = ecc_name;
> +
> + drvdata->base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
> + if (!drvdata->base) {
> + edac_printk(KERN_ERR, EDAC_DEVICE,
> + "%s:Unable to map regs\n", ecc_name);
> + return -ENOMEM;
This should be goto err, no? To unwind what edac_device_alloc_ctl_info()
allocated.
> + }
> +
> + /* Get driver specific data for this EDAC device */
> + drvdata->data = of_match_node(altr_edac_device_of_match, np)->data;
> +
> + /* Check specific dependencies for the module */
> + if (drvdata->data->setup) {
> + res = drvdata->data->setup(pdev, drvdata->base);
> + if (res < 0)
> + goto err;
> + }
> +
> + drvdata->sb_irq = platform_get_irq(pdev, 0);
> + res = devm_request_irq(&pdev->dev, drvdata->sb_irq,
> + altr_edac_device_handler,
> + 0, dev_name(&pdev->dev), dci);
> + if (res < 0)
> + goto err;
> +
> + drvdata->db_irq = platform_get_irq(pdev, 1);
> + res = devm_request_irq(&pdev->dev, drvdata->db_irq,
> + altr_edac_device_handler,
> + 0, dev_name(&pdev->dev), dci);
> + if (res < 0)
> + goto err;
> +
> + dci->mod_name = "EDAC_DEVICE";
Maybe more descriptive name with "ALTERA" in it?
> + dci->dev_name = drvdata->edac_dev_name;
> +
> + altr_set_sysfs_attr(dci, drvdata->data);
> +
> + if (edac_device_add_device(dci))
> + goto err;
> +
> + devres_close_group(&pdev->dev, NULL);
> +
> + return 0;
> +err:
> + devres_release_group(&pdev->dev, NULL);
> + edac_device_free_ctl_info(dci);
> +
> + return res;
> +}
> +
> +static int altr_edac_device_remove(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
> +
> + edac_device_del_device(&pdev->dev);
> + edac_device_free_ctl_info(dci);
> +
> + return 0;
> +}
> +
> +static struct platform_driver altr_edac_device_driver = {
> + .probe = altr_edac_device_probe,
> + .remove = altr_edac_device_remove,
> + .driver = {
> + .name = "altr_edac_device",
> + .of_match_table = altr_edac_device_of_match,
> + },
> +};
> +module_platform_driver(altr_edac_device_driver);
> +
> +/*********************** OCRAM EDAC Device Functions *********************/
> +
> +#ifdef CONFIG_EDAC_ALTERA_OCRAM
> +
> +#ifdef CONFIG_EDAC_DEBUG
> +static void *ocram_alloc_mem(size_t size, void **other)
> +{
> + struct device_node *np;
> + struct gen_pool *gp;
> + void *sram_addr;
> +
> + np = of_find_compatible_node(NULL, NULL, "altr,ocram-edac");
> + if (!np)
> + return NULL;
> +
> + gp = of_get_named_gen_pool(np, "iram", 0);
> + if (!gp)
> + return NULL;
> + *other = gp;
There's this assignment here to the supplied pointer...
> +
> + sram_addr = (void *)gen_pool_alloc(gp, size);
> + if (!sram_addr)
> + return NULL;
... and when we fail here, caller still gets to play with it. Perhaps do
the assignment after all calls here have succeeded first...?
> +
> + memset(sram_addr, 0, size);
> + wmb(); /* Ensure data is written out */
> +
> + return sram_addr;
> +}
> +
> +static void ocram_free_mem(void *p, size_t size, void *other)
> +{
> + gen_pool_free((struct gen_pool *)other, (u32)p, size);
> +}
> +
> +static struct edac_dev_sysfs_attribute altr_ocr_sysfs_attributes[] = {
> + {
> + .attr = { .name = "altr_ocram_trigger",
> + .mode = (S_IRUGO | S_IWUSR) },
> + .show = NULL,
> + .store = altr_edac_device_trig
> + },
> + {
> + .attr = {.name = NULL }
> + }
> +};
> +#endif /* #ifdef CONFIG_EDAC_DEBUG */
> +
> +/*
> + * altr_ocram_dependencies()
> + * Test for OCRAM cache ECC dependencies upon entry because
> + * platform specific startup should have initialized the
> + * On-Chip RAM memory and enabled the ECC.
> + * Can't turn on ECC here because accessing un-initialized
> + * memory will cause CE/UE errors possibly causing an ABORT.
> + */
> +static int altr_ocram_dependencies(struct platform_device *pdev,
> + void __iomem *base)
> +{
> + u32 control;
> +
> + control = readl(base) & ALTR_OCR_ECC_EN_MASK;
> + if (!control) {
> + dev_err(&pdev->dev,
> + "OCRAM: No ECC present or ECC disabled.\n");
> + return -ENODEV;
> + }
Flip logic to save an indentation level:
if (control)
return 0;
dev_err...
return -ENODEV;
> +
> + return 0;
> +}
> +
> +const struct edac_device_prv_data ocramecc_data = {
> + .setup = altr_ocram_dependencies,
> + .ce_clear_mask = (ALTR_OCR_ECC_EN_MASK | ALTR_OCR_ECC_SERR_MASK),
> + .ue_clear_mask = (ALTR_OCR_ECC_EN_MASK | ALTR_OCR_ECC_DERR_MASK),
> +#ifdef CONFIG_EDAC_DEBUG
> + .eccmgr_sysfs_attr = altr_ocr_sysfs_attributes,
> + .alloc_mem = ocram_alloc_mem,
> + .free_mem = ocram_free_mem,
> + .ecc_enable_mask = ALTR_OCR_ECC_EN_MASK,
> + .ce_set_mask = (ALTR_OCR_ECC_EN_MASK | ALTR_OCR_ECC_INJS_MASK),
> + .ue_set_mask = (ALTR_OCR_ECC_EN_MASK | ALTR_OCR_ECC_INJD_MASK),
> + .trig_alloc_sz = (32 * sizeof(u32)),
> +#endif
> +};
> +
> +#endif /* #ifdef CONFIG_EDAC_ALTERA_OCRAM */
> +
> +/********************* L2 Cache EDAC Device Functions ********************/
> +
> +#ifdef CONFIG_EDAC_ALTERA_L2C
> +
> +#ifdef CONFIG_EDAC_DEBUG
> +static void *l2_alloc_mem(size_t size, void **other)
> +{
> + struct device *dev = *other;
> + void *ptemp = devm_kzalloc(dev, size, GFP_KERNEL);
> +
> + if (!ptemp)
> + return NULL;
> +
> + /* Make sure everything is written out */
> + wmb();
> + flush_cache_all();
> +
> + return ptemp;
> +}
> +
> +static void l2_free_mem(void *p, size_t size, void *other)
> +{
> + struct device *dev = other;
> +
> + if (dev && p)
> + devm_kfree(dev, p);
> +}
> +
> +static struct edac_dev_sysfs_attribute altr_l2_sysfs_attributes[] = {
> + {
> + .attr = { .name = "altr_l2_trigger",
> + .mode = (S_IRUGO | S_IWUSR) },
> + .show = NULL,
> + .store = altr_edac_device_trig
> + },
> + {
> + .attr = {.name = NULL }
> + }
> +};
> +#endif /* #ifdef CONFIG_EDAC_DEBUG */
> +
> +/*
> + * altr_l2_dependencies()
> + * Test for L2 cache ECC dependencies upon entry because
> + * platform specific startup should have initialized the L2
> + * memory and enabled the ECC.
> + * Can't turn on ECC here because accessing un-initialized
> + * memory will cause CE/UE errors possibly causing an ABORT.
> + * Bail if ECC is not on.
> + * Test For 1) L2 ECC is enabled and 2) L2 Cache is enabled.
> + */
> +static int altr_l2_dependencies(struct platform_device *pdev,
> + void __iomem *base)
> +{
> + u32 control;
> + struct regmap *l2_vbase;
> +
> + control = readl(base) & ALTR_L2_ECC_EN_MASK;
> + if (!control) {
> + dev_err(&pdev->dev, "L2: No ECC present, or ECC disabled\n");
> + return -ENODEV;
> + }
> +
> + l2_vbase = syscon_regmap_lookup_by_compatible("arm,pl310-cache");
> + if (IS_ERR(l2_vbase)) {
> + dev_err(&pdev->dev,
> + "L2 ECC:regmap for arm,pl310-cache lookup failed.\n");
> + return -ENODEV;
> + }
> +
> + regmap_read(l2_vbase, ALTR_MPUL2_CONTROL_OFFSET, &control);
> + if (!(control & ALTR_MPUL2_CTL_CACHE_EN_MASK)) {
> + dev_err(&pdev->dev, "L2: Cache disabled\n");
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> +const struct edac_device_prv_data l2ecc_data = {
> + .setup = altr_l2_dependencies,
> + .ce_clear_mask = 0,
> + .ue_clear_mask = 0,
> +#ifdef CONFIG_EDAC_DEBUG
> + .eccmgr_sysfs_attr = altr_l2_sysfs_attributes,
> + .alloc_mem = l2_alloc_mem,
> + .free_mem = l2_free_mem,
> + .ecc_enable_mask = ALTR_L2_ECC_EN_MASK,
> + .ce_set_mask = (ALTR_L2_ECC_EN_MASK | ALTR_L2_ECC_INJS_MASK),
> + .ue_set_mask = (ALTR_L2_ECC_EN_MASK | ALTR_L2_ECC_INJD_MASK),
> + .trig_alloc_sz = 4 * 1024,
> +#endif
> +};
> +
> +#endif /* #ifdef CONFIG_EDAC_ALTERA_L2C */
> +
> MODULE_LICENSE("GPL v2");
> MODULE_AUTHOR("Thor Thayer");
> -MODULE_DESCRIPTION("EDAC Driver for Altera SDRAM Controller");
> +MODULE_DESCRIPTION("EDAC Driver for Altera Memories");
> --
> 1.7.9.5
>
>
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
Hi Boris!
On 11/04/2014 09:12 AM, Borislav Petkov wrote:
> On Thu, Oct 30, 2014 at 10:32:10AM -0500, [email protected] wrote:
>> From: Thor Thayer <[email protected]>
>>
>> Adding L2 Cache and On-Chip RAM EDAC support for the
>> Altera SoCs using the EDAC device model. The SDRAM
>> controller is using the Memory Controller model. All
>> Altera EDAC functions live in altera_edac.c.
>>
>> Signed-off-by: Thor Thayer <[email protected]>
>> ---
>> v2: Fix L2 dependency comments.
>>
>> v3: Move OCRAM and L2 cache EDAC functions into altera_edac.c
>> instead of separate files.
>> ---
>> drivers/edac/Kconfig | 14 ++
>> drivers/edac/Makefile | 5 +-
>> drivers/edac/altera_edac.c | 475 +++++++++++++++++++++++++++++++++++++++++++-
>> 3 files changed, 492 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
>> index 1719975..b145a52 100644
>> --- a/drivers/edac/Kconfig
>> +++ b/drivers/edac/Kconfig
>> @@ -385,4 +385,18 @@ config EDAC_ALTERA_MC
>> preloader must initialize the SDRAM before loading
>> the kernel.
>>
>> +config EDAC_ALTERA_L2C
>> + bool "Altera L2 Cache EDAC"
>> + depends on EDAC_MM_EDAC=y && ARCH_SOCFPGA && CACHE_L2X0
>> + help
>> + Support for error detection and correction on the
>> + Altera L2 cache Memory for Altera SoCs.
>> +
>> +config EDAC_ALTERA_OCRAM
>> + bool "Altera On-Chip RAM EDAC"
>> + depends on EDAC_MM_EDAC=y && ARCH_SOCFPGA && SRAM && GENERIC_ALLOCATOR
>> + help
>> + Support for error detection and correction on the
>> + Altera On-Chip RAM Memory for Altera SoCs.
> Why are those separate config items? Why not switch on EDAC_ALTERA_MC
> and get it all?
>
> We can always split them out later, if needed but I don't think
> we should burden the user with unnecessary questions if it is not
> absolutely necessary.
We want to at least separate L2/OCRAM ECC from the SDRAM ECC because
1) the SDRAM preparation can take almost 2 seconds on boot and some
customers need a faster boot time.
2) the SDRAM has an ECC initialization dependency on the preloader which
is outside the kernel. It is desirable to be able to turn the SDRAM on &
off separately.
You bring up a good point about the L2 and OCRAM being combined though.
If we do want granular control, maybe I should use a submenu? Or isn't
that desirable either?
>> +
>> endif # EDAC
>> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
>> index 359aa49..fa8aebc 100644
>> --- a/drivers/edac/Makefile
>> +++ b/drivers/edac/Makefile
>> @@ -66,4 +66,7 @@ obj-$(CONFIG_EDAC_OCTEON_L2C) += octeon_edac-l2c.o
>> obj-$(CONFIG_EDAC_OCTEON_LMC) += octeon_edac-lmc.o
>> obj-$(CONFIG_EDAC_OCTEON_PCI) += octeon_edac-pci.o
>>
>> -obj-$(CONFIG_EDAC_ALTERA_MC) += altera_edac.o
>> +alt_edac-y := altera_edac.o
>> +obj-$(CONFIG_EDAC_ALTERA_MC) += alt_edac.o
>> +obj-$(CONFIG_EDAC_ALTERA_L2C) += alt_edac.o
>> +obj-$(CONFIG_EDAC_ALTERA_OCRAM) += alt_edac.o
>> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
>> index 3c4929f..1ee8d94 100644
>> --- a/drivers/edac/altera_edac.c
>> +++ b/drivers/edac/altera_edac.c
>> @@ -17,8 +17,10 @@
>> * Adapted from the highbank_mc_edac driver.
>> */
>>
>> +#include <asm/cacheflush.h>
>> #include <linux/ctype.h>
>> #include <linux/edac.h>
>> +#include <linux/genalloc.h>
>> #include <linux/interrupt.h>
>> #include <linux/kernel.h>
>> #include <linux/mfd/syscon.h>
>> @@ -33,6 +35,7 @@
>>
>> #define EDAC_MOD_STR "altera_edac"
>> #define EDAC_VERSION "1"
>> +#define EDAC_DEVICE "DEVICE"
> That name is not very descriptive.
I will change to using "Altera EDAC Device"
>> /* SDRAM Controller CtrlCfg Register */
>> #define CTLCFG_OFST 0x00
>> @@ -107,6 +110,30 @@ struct altr_sdram_mc_data {
>> struct regmap *mc_vbase;
>> };
>>
>> +/************************** EDAC Device Defines **************************/
>> +
>> +/* OCRAM ECC Management Group Defines */
>> +#define ALTR_MAN_GRP_OCRAM_ECC_OFFSET 0x04
>> +#define ALTR_OCR_ECC_EN_MASK 0x00000001
>> +#define ALTR_OCR_ECC_INJS_MASK 0x00000002
>> +#define ALTR_OCR_ECC_INJD_MASK 0x00000004
>> +#define ALTR_OCR_ECC_SERR_MASK 0x00000008
>> +#define ALTR_OCR_ECC_DERR_MASK 0x00000010
>> +
>> +/* MPU L2 Register Defines */
>> +#define ALTR_MPUL2_CONTROL_OFFSET 0x100
>> +#define ALTR_MPUL2_CTL_CACHE_EN_MASK 0x00000001
>> +
>> +/* L2 ECC Management Group Defines */
>> +#define ALTR_MAN_GRP_L2_ECC_OFFSET 0x00
>> +#define ALTR_L2_ECC_EN_MASK 0x00000001
>> +#define ALTR_L2_ECC_INJS_MASK 0x00000002
>> +#define ALTR_L2_ECC_INJD_MASK 0x00000004
> You can use BIT() for those.
Thanks. I will change this.
>> +
>> +/*********************** EDAC Memory Controller Functions ****************/
>> +
>> +/* The SDRAM controller uses the EDAC Memory Controller framework. */
>> +
>> static irqreturn_t altr_sdram_mc_err_handler(int irq, void *dev_id)
>> {
>> struct mem_ctl_info *mci = dev_id;
>> @@ -405,6 +432,452 @@ static struct platform_driver altr_sdram_edac_driver = {
>>
>> module_platform_driver(altr_sdram_edac_driver);
>>
>> +/************************* EDAC Device Functions *************************/
>> +
>> +/*
>> + * EDAC Device Functions (shared between various IPs).
>> + * The discrete memories use the EDAC Device framework. The probe
>> + * and error handling functions are very similar between memories
>> + * so they are shared. The memory allocation and free for EDAC trigger
>> + * testing are different for each memory.
>> + */
>> +
>> +const struct edac_device_prv_data ocramecc_data;
>> +const struct edac_device_prv_data l2ecc_data;
>> +
>> +struct edac_device_prv_data {
>> + int (*setup)(struct platform_device *pdev, void __iomem *base);
>> + int ce_clear_mask;
>> + int ue_clear_mask;
>> +#ifdef CONFIG_EDAC_DEBUG
>> + struct edac_dev_sysfs_attribute *eccmgr_sysfs_attr;
>> + void * (*alloc_mem)(size_t size, void **other);
>> + void (*free_mem)(void *p, size_t size, void *other);
>> + int ecc_enable_mask;
>> + int ce_set_mask;
>> + int ue_set_mask;
>> + int trig_alloc_sz;
>> +#endif
>> +};
>> +
>> +struct altr_edac_device_dev {
>> + void __iomem *base;
>> + int sb_irq;
>> + int db_irq;
>> + const struct edac_device_prv_data *data;
>> + char *edac_dev_name;
>> +};
>> +
>> +static irqreturn_t altr_edac_device_handler(int irq, void *dev_id)
>> +{
>> + struct edac_device_ctl_info *dci = dev_id;
>> + struct altr_edac_device_dev *drvdata = dci->pvt_info;
>> + const struct edac_device_prv_data *priv = drvdata->data;
>> +
>> + if (irq == drvdata->sb_irq) {
>> + if (priv->ce_clear_mask)
>> + writel(priv->ce_clear_mask, drvdata->base);
>> + edac_device_handle_ce(dci, 0, 0, drvdata->edac_dev_name);
>> + }
>> + if (irq == drvdata->db_irq) {
>> + if (priv->ue_clear_mask)
>> + writel(priv->ue_clear_mask, drvdata->base);
>> + edac_device_handle_ue(dci, 0, 0, drvdata->edac_dev_name);
>> + panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
>> + }
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +#ifdef CONFIG_EDAC_DEBUG
>> +ssize_t altr_edac_device_trig(struct edac_device_ctl_info *edac_dci,
>> + const char *buffer, size_t count)
>> +{
>> + u32 *ptemp, i, error_mask;
>> + int result = 0;
>> + unsigned long flags;
>> + struct altr_edac_device_dev *drvdata = edac_dci->pvt_info;
>> + const struct edac_device_prv_data *priv = drvdata->data;
>> + void *generic_ptr = edac_dci->dev;
>> +
>> + if (!priv->alloc_mem)
>> + return -ENOMEM;
>> +
>> + /* Note that generic_ptr is initialized to the device * but in
>> + * some init_functions, this is overridden and returns data */
> Kernel comment style:
>
> /*
> * Blabla.
> * More Bla.
> */
Whoops. Sorry - I should have caught this. Weird that checkpatch.pl
didn't notice...
>> + ptemp = priv->alloc_mem(priv->trig_alloc_sz, &generic_ptr);
>> + if (!ptemp) {
>> + edac_printk(KERN_ERR, EDAC_DEVICE,
>> + "Inject: Buffer Allocation error\n");
>> + return -ENOMEM;
>> + }
>> +
>> + if (count == 3)
> Magic number 3.
>
> /me needs crystal ball :)
Will fix.
>> + error_mask = priv->ue_set_mask;
>> + else
>> + error_mask = priv->ce_set_mask;
>> +
>> + edac_printk(KERN_ALERT, EDAC_DEVICE,
>> + "Trigger Error Mask (0x%X)\n", error_mask);
>> +
>> + local_irq_save(flags);
>> + /* write ECC corrupted data out. */
>> + for (i = 0; i < (priv->trig_alloc_sz/sizeof(*ptemp)); i++) {
>> + /* Read data so we're in the correct state */
>> + rmb();
>> + if (ACCESS_ONCE(ptemp[i]))
>> + result = -1;
>> + /* Toggle Error bit (it is latched), leave ECC enabled */
>> + writel(error_mask, drvdata->base);
>> + writel(priv->ecc_enable_mask, drvdata->base);
>> + ptemp[i] = i;
>> + }
>> + /* Ensure it has been written out */
>> + wmb();
>> + local_irq_restore(flags);
> Doesn't the interrupt reenabling on ARM already cause outstanding writes
> to commit so that you don't really need the memory barrier?
I wasn't aware of this but it would make sense.
When I looked at the inline assembly of the irq_restore(), it didn't
have a memory barrier instruction in it (DSB). It did have the :
"memory" for the clobbered register but I don't think that is the same
thing. I will continue looking into this.
>> +
>> + if (result)
>> + edac_printk(KERN_ERR, EDAC_DEVICE, "Mem Not Cleared\n");
>> +
>> + /* Read out written data. ECC error caused here */
>> + for (i = 0; i < 16; i++)
> Magic number 16. Where's my ball again?! :)
Thanks. I will fix it.
>> + if (ACCESS_ONCE(ptemp[i]) != i)
>> + edac_printk(KERN_ERR, EDAC_DEVICE,
>> + "Mem Doesn't match\n");
> I guess injectors should know what this printk means? I can assume
> it says something about inject data written doesn't match what we're
> reading?
You are correct and I will make the message more descriptive.
>> +
>> + if (priv->free_mem)
>> + priv->free_mem(ptemp, priv->trig_alloc_sz, generic_ptr);
>> +
>> + return count;
>> +}
>> +
>> +static void altr_set_sysfs_attr(struct edac_device_ctl_info *edac_dci,
>> + const struct edac_device_prv_data *priv)
>> +{
>> + struct edac_dev_sysfs_attribute *ecc_attr = priv->eccmgr_sysfs_attr;
>> +
>> + if (ecc_attr) {
>> + edac_dci->sysfs_attributes = ecc_attr;
>> + edac_printk(KERN_ERR, EDAC_DEVICE, "Set SysFS trigger\n");
> Why do we have to say this here? Debugging leftovers maybe?
Yes, thank you. I will remove.
>> + }
>> +}
>> +#else
>> +static void altr_set_sysfs_attr(struct edac_device_ctl_info *edac_dci,
>> + const struct edac_device_prv_data *priv)
>> +{}
>> +#endif /* #ifdef CONFIG_EDAC_DEBUG */
>> +
>> +static const struct of_device_id altr_edac_device_of_match[] = {
>> +#ifdef CONFIG_EDAC_ALTERA_L2C
>> + { .compatible = "altr,l2-edac", .data = (void *)&l2ecc_data },
>> +#endif
>> +#ifdef CONFIG_EDAC_ALTERA_OCRAM
>> + { .compatible = "altr,ocram-edac", .data = (void *)&ocramecc_data },
>> +#endif
> If you enable l2c and ocram by default, you can save yourself the ugly
> ifdeffery here too.
Yes. I just need to confirm the granularity needed to determine if I can
combine these as you suggest.
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, altr_edac_device_of_match);
>> +
>> +/*
>> + * altr_edac_device_probe()
>> + * This is a generic EDAC device driver that will support
>> + * various Altera memory devices such as the L2 cache ECC and
>> + * OCRAM ECC as well as the memories for other peripherals.
>> + * Module specific initialization is done by passing the
>> + * function index in the device tree.
>> + */
>> +static int altr_edac_device_probe(struct platform_device *pdev)
>> +{
>> + struct edac_device_ctl_info *dci;
>> + struct altr_edac_device_dev *drvdata;
>> + struct resource *r;
>> + int res = 0;
>> + struct device_node *np = pdev->dev.of_node;
>> + char *ecc_name = (char *)np->name;
>> + static int dev_instance;
>> +
>> + if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL))
>> + return -ENOMEM;
>> +
>> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!r) {
>> + edac_printk(KERN_ERR, EDAC_DEVICE,
>> + "Unable to get mem resource\n");
>> + return -ENODEV;
>> + }
>> +
>> + if (!devm_request_mem_region(&pdev->dev, r->start, resource_size(r),
>> + dev_name(&pdev->dev))) {
>> + edac_printk(KERN_ERR, EDAC_DEVICE,
>> + "%s:Error requesting mem region\n", ecc_name);
>> + return -EBUSY;
>> + }
>> +
>> + dci = edac_device_alloc_ctl_info(sizeof(*drvdata), ecc_name,
>> + 1, ecc_name, 1, 0, NULL, 0,
>> + dev_instance++);
>> +
>> + if (!dci)
>> + return -ENOMEM;
>> +
>> + drvdata = dci->pvt_info;
>> + dci->dev = &pdev->dev;
>> + platform_set_drvdata(pdev, dci);
>> + drvdata->edac_dev_name = ecc_name;
>> +
>> + drvdata->base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>> + if (!drvdata->base) {
>> + edac_printk(KERN_ERR, EDAC_DEVICE,
>> + "%s:Unable to map regs\n", ecc_name);
>> + return -ENOMEM;
> This should be goto err, no? To unwind what edac_device_alloc_ctl_info()
> allocated.
Yes. Thank you.
>> + }
>> +
>> + /* Get driver specific data for this EDAC device */
>> + drvdata->data = of_match_node(altr_edac_device_of_match, np)->data;
>> +
>> + /* Check specific dependencies for the module */
>> + if (drvdata->data->setup) {
>> + res = drvdata->data->setup(pdev, drvdata->base);
>> + if (res < 0)
>> + goto err;
>> + }
>> +
>> + drvdata->sb_irq = platform_get_irq(pdev, 0);
>> + res = devm_request_irq(&pdev->dev, drvdata->sb_irq,
>> + altr_edac_device_handler,
>> + 0, dev_name(&pdev->dev), dci);
>> + if (res < 0)
>> + goto err;
>> +
>> + drvdata->db_irq = platform_get_irq(pdev, 1);
>> + res = devm_request_irq(&pdev->dev, drvdata->db_irq,
>> + altr_edac_device_handler,
>> + 0, dev_name(&pdev->dev), dci);
>> + if (res < 0)
>> + goto err;
>> +
>> + dci->mod_name = "EDAC_DEVICE";
> Maybe more descriptive name with "ALTERA" in it?
OK. Changing to "Altera EDAC Memory". Still generic since it is shared
with L2 and OCRAM.
>> + dci->dev_name = drvdata->edac_dev_name;
>> +
>> + altr_set_sysfs_attr(dci, drvdata->data);
>> +
>> + if (edac_device_add_device(dci))
>> + goto err;
>> +
>> + devres_close_group(&pdev->dev, NULL);
>> +
>> + return 0;
>> +err:
>> + devres_release_group(&pdev->dev, NULL);
>> + edac_device_free_ctl_info(dci);
>> +
>> + return res;
>> +}
>> +
>> +static int altr_edac_device_remove(struct platform_device *pdev)
>> +{
>> + struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
>> +
>> + edac_device_del_device(&pdev->dev);
>> + edac_device_free_ctl_info(dci);
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver altr_edac_device_driver = {
>> + .probe = altr_edac_device_probe,
>> + .remove = altr_edac_device_remove,
>> + .driver = {
>> + .name = "altr_edac_device",
>> + .of_match_table = altr_edac_device_of_match,
>> + },
>> +};
>> +module_platform_driver(altr_edac_device_driver);
>> +
>> +/*********************** OCRAM EDAC Device Functions *********************/
>> +
>> +#ifdef CONFIG_EDAC_ALTERA_OCRAM
>> +
>> +#ifdef CONFIG_EDAC_DEBUG
>> +static void *ocram_alloc_mem(size_t size, void **other)
>> +{
>> + struct device_node *np;
>> + struct gen_pool *gp;
>> + void *sram_addr;
>> +
>> + np = of_find_compatible_node(NULL, NULL, "altr,ocram-edac");
>> + if (!np)
>> + return NULL;
>> +
>> + gp = of_get_named_gen_pool(np, "iram", 0);
>> + if (!gp)
>> + return NULL;
>> + *other = gp;
> There's this assignment here to the supplied pointer...
>
>> +
>> + sram_addr = (void *)gen_pool_alloc(gp, size);
>> + if (!sram_addr)
>> + return NULL;
> ... and when we fail here, caller still gets to play with it. Perhaps do
> the assignment after all calls here have succeeded first...?
>
The caller tests the return value which is set properly on an error
condition (NULL). However, this is better and I will make the change.
Thank you.
>> +
>> + memset(sram_addr, 0, size);
>> + wmb(); /* Ensure data is written out */
>> +
>> + return sram_addr;
>> +}
>> +
>> +static void ocram_free_mem(void *p, size_t size, void *other)
>> +{
>> + gen_pool_free((struct gen_pool *)other, (u32)p, size);
>> +}
>> +
>> +static struct edac_dev_sysfs_attribute altr_ocr_sysfs_attributes[] = {
>> + {
>> + .attr = { .name = "altr_ocram_trigger",
>> + .mode = (S_IRUGO | S_IWUSR) },
>> + .show = NULL,
>> + .store = altr_edac_device_trig
>> + },
>> + {
>> + .attr = {.name = NULL }
>> + }
>> +};
>> +#endif /* #ifdef CONFIG_EDAC_DEBUG */
>> +
>> +/*
>> + * altr_ocram_dependencies()
>> + * Test for OCRAM cache ECC dependencies upon entry because
>> + * platform specific startup should have initialized the
>> + * On-Chip RAM memory and enabled the ECC.
>> + * Can't turn on ECC here because accessing un-initialized
>> + * memory will cause CE/UE errors possibly causing an ABORT.
>> + */
>> +static int altr_ocram_dependencies(struct platform_device *pdev,
>> + void __iomem *base)
>> +{
>> + u32 control;
>> +
>> + control = readl(base) & ALTR_OCR_ECC_EN_MASK;
>> + if (!control) {
>> + dev_err(&pdev->dev,
>> + "OCRAM: No ECC present or ECC disabled.\n");
>> + return -ENODEV;
>> + }
> Flip logic to save an indentation level:
>
> if (control)
> return 0;
>
> dev_err...
> return -ENODEV;
Thank you for reviewing. I will make the change.
>> +
>> + return 0;
>> +}
>> +
>> +const struct edac_device_prv_data ocramecc_data = {
>> + .setup = altr_ocram_dependencies,
>> + .ce_clear_mask = (ALTR_OCR_ECC_EN_MASK | ALTR_OCR_ECC_SERR_MASK),
>> + .ue_clear_mask = (ALTR_OCR_ECC_EN_MASK | ALTR_OCR_ECC_DERR_MASK),
>> +#ifdef CONFIG_EDAC_DEBUG
>> + .eccmgr_sysfs_attr = altr_ocr_sysfs_attributes,
>> + .alloc_mem = ocram_alloc_mem,
>> + .free_mem = ocram_free_mem,
>> + .ecc_enable_mask = ALTR_OCR_ECC_EN_MASK,
>> + .ce_set_mask = (ALTR_OCR_ECC_EN_MASK | ALTR_OCR_ECC_INJS_MASK),
>> + .ue_set_mask = (ALTR_OCR_ECC_EN_MASK | ALTR_OCR_ECC_INJD_MASK),
>> + .trig_alloc_sz = (32 * sizeof(u32)),
>> +#endif
>> +};
>> +
>> +#endif /* #ifdef CONFIG_EDAC_ALTERA_OCRAM */
>> +
>> +/********************* L2 Cache EDAC Device Functions ********************/
>> +
>> +#ifdef CONFIG_EDAC_ALTERA_L2C
>> +
>> +#ifdef CONFIG_EDAC_DEBUG
>> +static void *l2_alloc_mem(size_t size, void **other)
>> +{
>> + struct device *dev = *other;
>> + void *ptemp = devm_kzalloc(dev, size, GFP_KERNEL);
>> +
>> + if (!ptemp)
>> + return NULL;
>> +
>> + /* Make sure everything is written out */
>> + wmb();
>> + flush_cache_all();
>> +
>> + return ptemp;
>> +}
>> +
>> +static void l2_free_mem(void *p, size_t size, void *other)
>> +{
>> + struct device *dev = other;
>> +
>> + if (dev && p)
>> + devm_kfree(dev, p);
>> +}
>> +
>> +static struct edac_dev_sysfs_attribute altr_l2_sysfs_attributes[] = {
>> + {
>> + .attr = { .name = "altr_l2_trigger",
>> + .mode = (S_IRUGO | S_IWUSR) },
>> + .show = NULL,
>> + .store = altr_edac_device_trig
>> + },
>> + {
>> + .attr = {.name = NULL }
>> + }
>> +};
>> +#endif /* #ifdef CONFIG_EDAC_DEBUG */
>> +
>> +/*
>> + * altr_l2_dependencies()
>> + * Test for L2 cache ECC dependencies upon entry because
>> + * platform specific startup should have initialized the L2
>> + * memory and enabled the ECC.
>> + * Can't turn on ECC here because accessing un-initialized
>> + * memory will cause CE/UE errors possibly causing an ABORT.
>> + * Bail if ECC is not on.
>> + * Test For 1) L2 ECC is enabled and 2) L2 Cache is enabled.
>> + */
>> +static int altr_l2_dependencies(struct platform_device *pdev,
>> + void __iomem *base)
>> +{
>> + u32 control;
>> + struct regmap *l2_vbase;
>> +
>> + control = readl(base) & ALTR_L2_ECC_EN_MASK;
>> + if (!control) {
>> + dev_err(&pdev->dev, "L2: No ECC present, or ECC disabled\n");
>> + return -ENODEV;
>> + }
>> +
>> + l2_vbase = syscon_regmap_lookup_by_compatible("arm,pl310-cache");
>> + if (IS_ERR(l2_vbase)) {
>> + dev_err(&pdev->dev,
>> + "L2 ECC:regmap for arm,pl310-cache lookup failed.\n");
>> + return -ENODEV;
>> + }
>> +
>> + regmap_read(l2_vbase, ALTR_MPUL2_CONTROL_OFFSET, &control);
>> + if (!(control & ALTR_MPUL2_CTL_CACHE_EN_MASK)) {
>> + dev_err(&pdev->dev, "L2: Cache disabled\n");
>> + return -ENODEV;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +const struct edac_device_prv_data l2ecc_data = {
>> + .setup = altr_l2_dependencies,
>> + .ce_clear_mask = 0,
>> + .ue_clear_mask = 0,
>> +#ifdef CONFIG_EDAC_DEBUG
>> + .eccmgr_sysfs_attr = altr_l2_sysfs_attributes,
>> + .alloc_mem = l2_alloc_mem,
>> + .free_mem = l2_free_mem,
>> + .ecc_enable_mask = ALTR_L2_ECC_EN_MASK,
>> + .ce_set_mask = (ALTR_L2_ECC_EN_MASK | ALTR_L2_ECC_INJS_MASK),
>> + .ue_set_mask = (ALTR_L2_ECC_EN_MASK | ALTR_L2_ECC_INJD_MASK),
>> + .trig_alloc_sz = 4 * 1024,
>> +#endif
>> +};
>> +
>> +#endif /* #ifdef CONFIG_EDAC_ALTERA_L2C */
>> +
>> MODULE_LICENSE("GPL v2");
>> MODULE_AUTHOR("Thor Thayer");
>> -MODULE_DESCRIPTION("EDAC Driver for Altera SDRAM Controller");
>> +MODULE_DESCRIPTION("EDAC Driver for Altera Memories");
>> --
>> 1.7.9.5
>>
>>
Hi Thor,
On Tue, Nov 04, 2014 at 04:57:44PM -0600, Thor Thayer wrote:
> We want to at least separate L2/OCRAM ECC from the SDRAM ECC because
> 1) the SDRAM preparation can take almost 2 seconds on boot and some
> customers need a faster boot time.
> 2) the SDRAM has an ECC initialization dependency on the preloader which is
> outside the kernel. It is desirable to be able to turn the SDRAM on & off
> separately.
Well, now that I asked and you gave valid reasons for the split,
you should keep them split the way they are. But please do add that
explanation to the commit message so that it is clear to people why
there is a split.
> You bring up a good point about the L2 and OCRAM being combined though.
>
> If we do want granular control, maybe I should use a submenu? Or isn't that
> desirable either?
Well, what do you think would be easier/faster for a user configuring? A
separate menu where you have to do a couple of key presses just to enter
it or simply a subtree in Kconfig with all the options together. I think
the "depends" gives you that already...
Ok, once you've worked in the suggested changes, you're good to go,
at least for the EDAC bits. Let me know how you want to handle this,
whether I should pick up the whole thing or I should ack the EDAC parts.
This patchset should go together, in any case, and so I don't care
whoever picks it up.
Thanks.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
Hi Boris,
On 11/06/2014 10:31 AM, Borislav Petkov wrote:
> Hi Thor,
>
> On Tue, Nov 04, 2014 at 04:57:44PM -0600, Thor Thayer wrote:
>> We want to at least separate L2/OCRAM ECC from the SDRAM ECC because
>> 1) the SDRAM preparation can take almost 2 seconds on boot and some
>> customers need a faster boot time.
>> 2) the SDRAM has an ECC initialization dependency on the preloader which is
>> outside the kernel. It is desirable to be able to turn the SDRAM on & off
>> separately.
>
> Well, now that I asked and you gave valid reasons for the split,
> you should keep them split the way they are. But please do add that
> explanation to the commit message so that it is clear to people why
> there is a split.
>
>> You bring up a good point about the L2 and OCRAM being combined though.
>>
>> If we do want granular control, maybe I should use a submenu? Or isn't that
>> desirable either?
>
> Well, what do you think would be easier/faster for a user configuring? A
> separate menu where you have to do a couple of key presses just to enter
> it or simply a subtree in Kconfig with all the options together. I think
> the "depends" gives you that already...
>
> Ok, once you've worked in the suggested changes, you're good to go,
> at least for the EDAC bits. Let me know how you want to handle this,
> whether I should pick up the whole thing or I should ack the EDAC parts.
> This patchset should go together, in any case, and so I don't care
> whoever picks it up.
>
If it's okay, can you please pick up this series, once everything is
cleaned up? I just checked to make sure that there aren't any merge
conflicts in the DTS files in this series against DTS patches that I
have queue up for 3.19, and there aren't.
Thanks,
Dinh
On Fri, Nov 07, 2014 at 10:31:20AM -0600, Dinh Nguyen wrote:
> If it's okay, can you please pick up this series, once everything is
> cleaned up? I just checked to make sure that there aren't any merge
> conflicts in the DTS files in this series against DTS patches that I
> have queue up for 3.19, and there aren't.
Sure, but I'd need ACKs from you/the relevant maintainers for 1/5, 2/5
and 5/5 patches.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--