LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Add Altera Arria10 EDAC Support
@ 2015-05-13 21:49 tthayer
  2015-05-13 21:49 ` [PATCH 1/4] edac, altera: Generalize driver to use DT Memory size tthayer
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: tthayer @ 2015-05-13 21:49 UTC (permalink / raw
  To: bp, dougthompson, m.chehab, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, linux, dinguyen, grant.likely
  Cc: devicetree, linux-doc, linux-edac, linux-kernel, linux-arm-kernel,
	tthayer.linux, tthayer

From: Thor Thayer <tthayer@opensource.altera.com>

This series of patches adds support for the Arria10 EDAC. The
SDRAM controller and ECC registers are significantly different
from the CycloneV/ArriaV but common areas could be abstracted.

Thor Thayer (4):
  edac, altera: Generalize driver to use DT Memory size
  edac, altera: Refactor EDAC for Altera CycloneV SoC.
  edac, altera: Addition of Arria10 EDAC
  dts, altera: Arria10 SDRAM EDAC DTS additions.

 .../bindings/arm/altera/socfpga-sdram-edac.txt     |    2 +-
 arch/arm/boot/dts/socfpga_arria10.dtsi             |   11 +
 drivers/edac/altera_edac.c                         |  357 ++++++++++++--------
 drivers/edac/altera_edac.h                         |  201 +++++++++++
 4 files changed, 434 insertions(+), 137 deletions(-)
 create mode 100644 drivers/edac/altera_edac.h

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] edac, altera: Generalize driver to use DT Memory size
  2015-05-13 21:49 [PATCH 0/4] Add Altera Arria10 EDAC Support tthayer
@ 2015-05-13 21:49 ` tthayer
  2015-05-14  3:25   ` Dinh Nguyen
  2015-05-15 11:00   ` Arnd Bergmann
  2015-05-13 21:49 ` [PATCH 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC tthayer
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: tthayer @ 2015-05-13 21:49 UTC (permalink / raw
  To: bp, dougthompson, m.chehab, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, linux, dinguyen, grant.likely
  Cc: devicetree, linux-doc, linux-edac, linux-kernel, linux-arm-kernel,
	tthayer.linux, tthayer

From: Thor Thayer <tthayer@opensource.altera.com>

The Arria10 SOC uses a completely different SDRAM controller from the
earlier CycloneV and ArriaV SoCs. The memory size is calculated in
the bootloader and passed via the device tree. Using this device
tree size is more generic than using the register fields to
calculate the memory size for different SDRAM controllers.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 drivers/edac/altera_edac.c |   53 ++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 3c4929f..e18a205 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -219,36 +219,35 @@ static void altr_sdr_mc_create_debugfs_nodes(struct mem_ctl_info *mci)
 {}
 #endif
 
-/* Get total memory size in bytes */
-static u32 altr_sdram_get_total_mem_size(struct regmap *mc_vbase)
+/* Get total memory size from Open Firmware DTB */
+static unsigned long get_total_mem(void)
 {
-	u32 size, read_reg, row, bank, col, cs, width;
+	struct device_node *np = NULL;
+	const unsigned int *reg, *reg_end;
+	int len, sw, aw;
+	unsigned long start, size, total_mem;
 
-	if (regmap_read(mc_vbase, DRAMADDRW_OFST, &read_reg) < 0)
+	np = of_find_node_by_type(NULL, "memory");
+	if (!np)
 		return 0;
 
-	if (regmap_read(mc_vbase, DRAMIFWIDTH_OFST, &width) < 0)
-		return 0;
-
-	col = (read_reg & DRAMADDRW_COLBIT_MASK) >>
-		DRAMADDRW_COLBIT_SHIFT;
-	row = (read_reg & DRAMADDRW_ROWBIT_MASK) >>
-		DRAMADDRW_ROWBIT_SHIFT;
-	bank = (read_reg & DRAMADDRW_BANKBIT_MASK) >>
-		DRAMADDRW_BANKBIT_SHIFT;
-	cs = (read_reg & DRAMADDRW_CSBIT_MASK) >>
-		DRAMADDRW_CSBIT_SHIFT;
-
-	/* Correct for ECC as its not addressible */
-	if (width == DRAMIFWIDTH_32B_ECC)
-		width = 32;
-	if (width == DRAMIFWIDTH_16B_ECC)
-		width = 16;
-
-	/* calculate the SDRAM size base on this info */
-	size = 1 << (row + bank + col);
-	size = size * cs * (width / 8);
-	return size;
+	aw = of_n_addr_cells(np);
+	sw = of_n_size_cells(np);
+	reg = (const unsigned int *)of_get_property(np, "reg", &len);
+	reg_end = reg + (len / sizeof(u32));
+
+	total_mem = 0;
+	do {
+		start = of_read_number(reg, aw);
+		reg += aw;
+		size = of_read_number(reg, sw);
+		reg += sw;
+		total_mem += size;
+	} while (reg < reg_end);
+
+	of_node_put(np);
+	edac_printk(KERN_ERR, EDAC_MC, "total_mem 0x%lx\n", total_mem);
+	return total_mem;
 }
 
 static int altr_sdram_probe(struct platform_device *pdev)
@@ -280,7 +279,7 @@ static int altr_sdram_probe(struct platform_device *pdev)
 	}
 
 	/* Grab memory size from device tree. */
-	mem_size = altr_sdram_get_total_mem_size(mc_vbase);
+	mem_size = get_total_mem();
 	if (!mem_size) {
 		edac_printk(KERN_ERR, EDAC_MC,
 			    "Unable to calculate memory size\n");
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC.
  2015-05-13 21:49 [PATCH 0/4] Add Altera Arria10 EDAC Support tthayer
  2015-05-13 21:49 ` [PATCH 1/4] edac, altera: Generalize driver to use DT Memory size tthayer
@ 2015-05-13 21:49 ` tthayer
  2015-05-14 20:13   ` Dinh Nguyen
  2015-05-13 21:49 ` [PATCH 3/4] edac, altera: Addition of Arria10 EDAC tthayer
  2015-05-13 21:49 ` [PATCH 4/4] dts, altera: Arria10 SDRAM EDAC DTS additions tthayer
  3 siblings, 1 reply; 13+ messages in thread
From: tthayer @ 2015-05-13 21:49 UTC (permalink / raw
  To: bp, dougthompson, m.chehab, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, linux, dinguyen, grant.likely
  Cc: devicetree, linux-doc, linux-edac, linux-kernel, linux-arm-kernel,
	tthayer.linux, tthayer

From: Thor Thayer <tthayer@opensource.altera.com>

The Arria10 SOC uses a completely different SDRAM controller from the
earlier CycloneV and ArriaV SoCs. This patch abstracts the SDRAM bits
for the CycloneV/ArriaV SoCs in preparation for the Arria10 support.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 drivers/edac/altera_edac.c |  194 ++++++++++++++++++++------------------------
 drivers/edac/altera_edac.h |  116 ++++++++++++++++++++++++++
 2 files changed, 206 insertions(+), 104 deletions(-)
 create mode 100644 drivers/edac/altera_edac.h

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index e18a205..204ad2d 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright Altera Corporation (C) 2014. All rights reserved.
+ *  Copyright Altera Corporation (C) 2014-2015. All rights reserved.
  *  Copyright 2011-2012 Calxeda, Inc.
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -28,111 +28,64 @@
 #include <linux/types.h>
 #include <linux/uaccess.h>
 
+#include "altera_edac.h"
 #include "edac_core.h"
 #include "edac_module.h"
 
 #define EDAC_MOD_STR		"altera_edac"
 #define EDAC_VERSION		"1"
 
-/* SDRAM Controller CtrlCfg Register */
-#define CTLCFG_OFST             0x00
-
-/* SDRAM Controller CtrlCfg Register Bit Masks */
-#define CTLCFG_ECC_EN           0x400
-#define CTLCFG_ECC_CORR_EN      0x800
-#define CTLCFG_GEN_SB_ERR       0x2000
-#define CTLCFG_GEN_DB_ERR       0x4000
-
-#define CTLCFG_ECC_AUTO_EN	(CTLCFG_ECC_EN | \
-				 CTLCFG_ECC_CORR_EN)
-
-/* SDRAM Controller Address Width Register */
-#define DRAMADDRW_OFST          0x2C
-
-/* SDRAM Controller Address Widths Field Register */
-#define DRAMADDRW_COLBIT_MASK   0x001F
-#define DRAMADDRW_COLBIT_SHIFT  0
-#define DRAMADDRW_ROWBIT_MASK   0x03E0
-#define DRAMADDRW_ROWBIT_SHIFT  5
-#define DRAMADDRW_BANKBIT_MASK	0x1C00
-#define DRAMADDRW_BANKBIT_SHIFT 10
-#define DRAMADDRW_CSBIT_MASK	0xE000
-#define DRAMADDRW_CSBIT_SHIFT   13
-
-/* SDRAM Controller Interface Data Width Register */
-#define DRAMIFWIDTH_OFST        0x30
-
-/* SDRAM Controller Interface Data Width Defines */
-#define DRAMIFWIDTH_16B_ECC     24
-#define DRAMIFWIDTH_32B_ECC     40
-
-/* SDRAM Controller DRAM Status Register */
-#define DRAMSTS_OFST            0x38
-
-/* SDRAM Controller DRAM Status Register Bit Masks */
-#define DRAMSTS_SBEERR          0x04
-#define DRAMSTS_DBEERR          0x08
-#define DRAMSTS_CORR_DROP       0x10
-
-/* SDRAM Controller DRAM IRQ Register */
-#define DRAMINTR_OFST           0x3C
-
-/* SDRAM Controller DRAM IRQ Register Bit Masks */
-#define DRAMINTR_INTREN         0x01
-#define DRAMINTR_SBEMASK        0x02
-#define DRAMINTR_DBEMASK        0x04
-#define DRAMINTR_CORRDROPMASK   0x08
-#define DRAMINTR_INTRCLR        0x10
-
-/* SDRAM Controller Single Bit Error Count Register */
-#define SBECOUNT_OFST           0x40
-
-/* SDRAM Controller Single Bit Error Count Register Bit Masks */
-#define SBECOUNT_MASK           0x0F
-
-/* SDRAM Controller Double Bit Error Count Register */
-#define DBECOUNT_OFST           0x44
-
-/* SDRAM Controller Double Bit Error Count Register Bit Masks */
-#define DBECOUNT_MASK           0x0F
-
-/* SDRAM Controller ECC Error Address Register */
-#define ERRADDR_OFST            0x48
-
-/* SDRAM Controller ECC Error Address Register Bit Masks */
-#define ERRADDR_MASK            0xFFFFFFFF
-
-/* Altera SDRAM Memory Controller data */
-struct altr_sdram_mc_data {
-	struct regmap *mc_vbase;
+const struct altr_sdram_prv_data c5_data = {
+	.ecc_ctrl_offset    = CV_CTLCFG_OFST,
+	.ecc_ctl_en_mask    = CV_CTLCFG_ECC_AUTO_EN,
+	.ecc_stat_offset    = CV_DRAMSTS_OFST,
+	.ecc_stat_ce_mask   = CV_DRAMSTS_SBEERR,
+	.ecc_stat_ue_mask   = CV_DRAMSTS_DBEERR,
+	.ecc_saddr_offset   = CV_ERRADDR_OFST,
+	.ecc_cecnt_offset   = CV_SBECOUNT_OFST,
+	.ecc_uecnt_offset   = CV_DBECOUNT_OFST,
+	.ecc_irq_en_offset  = CV_DRAMINTR_OFST,
+	.ecc_irq_en_mask    = CV_DRAMINTR_INTREN,
+	.ecc_irq_clr_offset = CV_DRAMINTR_OFST,
+	.ecc_irq_clr_mask   = (CV_DRAMINTR_INTRCLR | CV_DRAMINTR_INTREN),
+	.ecc_cnt_rst_offset = CV_DRAMINTR_OFST,
+	.ecc_cnt_rst_mask   = CV_DRAMINTR_INTRCLR,
+#ifdef CONFIG_EDAC_DEBUG
+	.ce_ue_trgr_offset  = CV_CTLCFG_OFST,
+	.ce_set_mask        = CV_CTLCFG_GEN_SB_ERR,
+	.ue_set_mask        = CV_CTLCFG_GEN_DB_ERR,
+#endif
 };
 
 static irqreturn_t altr_sdram_mc_err_handler(int irq, void *dev_id)
 {
 	struct mem_ctl_info *mci = dev_id;
 	struct altr_sdram_mc_data *drvdata = mci->pvt_info;
+	const struct altr_sdram_prv_data *priv = drvdata->data;
 	u32 status, err_count, err_addr;
 
 	/* Error Address is shared by both SBE & DBE */
-	regmap_read(drvdata->mc_vbase, ERRADDR_OFST, &err_addr);
+	regmap_read(drvdata->mc_vbase, priv->ecc_saddr_offset, &err_addr);
 
-	regmap_read(drvdata->mc_vbase, DRAMSTS_OFST, &status);
+	regmap_read(drvdata->mc_vbase, priv->ecc_stat_offset, &status);
 
-	if (status & DRAMSTS_DBEERR) {
-		regmap_read(drvdata->mc_vbase, DBECOUNT_OFST, &err_count);
+	if (status & priv->ecc_stat_ue_mask) {
+		regmap_read(drvdata->mc_vbase, priv->ecc_uecnt_offset,
+			    &err_count);
 		panic("\nEDAC: [%d Uncorrectable errors @ 0x%08X]\n",
 		      err_count, err_addr);
 	}
-	if (status & DRAMSTS_SBEERR) {
-		regmap_read(drvdata->mc_vbase, SBECOUNT_OFST, &err_count);
+	if (status & priv->ecc_stat_ce_mask) {
+		regmap_read(drvdata->mc_vbase,  priv->ecc_cecnt_offset,
+			    &err_count);
 		edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci, err_count,
 				     err_addr >> PAGE_SHIFT,
 				     err_addr & ~PAGE_MASK, 0,
 				     0, 0, -1, mci->ctl_name, "");
 	}
 
-	regmap_write(drvdata->mc_vbase,	DRAMINTR_OFST,
-		     (DRAMINTR_INTRCLR | DRAMINTR_INTREN));
+	regmap_write(drvdata->mc_vbase,	priv->ecc_irq_clr_offset,
+		     priv->ecc_irq_clr_mask);
 
 	return IRQ_HANDLED;
 }
@@ -144,6 +97,7 @@ static ssize_t altr_sdr_mc_err_inject_write(struct file *file,
 {
 	struct mem_ctl_info *mci = file->private_data;
 	struct altr_sdram_mc_data *drvdata = mci->pvt_info;
+	const struct altr_sdram_prv_data *priv = drvdata->data;
 	u32 *ptemp;
 	dma_addr_t dma_handle;
 	u32 reg, read_reg;
@@ -156,8 +110,9 @@ static ssize_t altr_sdr_mc_err_inject_write(struct file *file,
 		return -ENOMEM;
 	}
 
-	regmap_read(drvdata->mc_vbase, CTLCFG_OFST, &read_reg);
-	read_reg &= ~(CTLCFG_GEN_SB_ERR | CTLCFG_GEN_DB_ERR);
+	regmap_read(drvdata->mc_vbase, priv->ce_ue_trgr_offset,
+		    &read_reg);
+	read_reg &= ~(priv->ce_set_mask | priv->ue_set_mask);
 
 	/* Error are injected by writing a word while the SBE or DBE
 	 * bit in the CTLCFG register is set. Reading the word will
@@ -166,20 +121,20 @@ static ssize_t altr_sdr_mc_err_inject_write(struct file *file,
 	if (count == 3) {
 		edac_printk(KERN_ALERT, EDAC_MC,
 			    "Inject Double bit error\n");
-		regmap_write(drvdata->mc_vbase, CTLCFG_OFST,
-			     (read_reg | CTLCFG_GEN_DB_ERR));
+		regmap_write(drvdata->mc_vbase, priv->ce_ue_trgr_offset,
+			     (read_reg | priv->ue_set_mask));
 	} else {
 		edac_printk(KERN_ALERT, EDAC_MC,
 			    "Inject Single bit error\n");
-		regmap_write(drvdata->mc_vbase,	CTLCFG_OFST,
-			     (read_reg | CTLCFG_GEN_SB_ERR));
+		regmap_write(drvdata->mc_vbase,	priv->ce_ue_trgr_offset,
+			     (read_reg | priv->ce_set_mask));
 	}
 
 	ptemp[0] = 0x5A5A5A5A;
 	ptemp[1] = 0xA5A5A5A5;
 
 	/* Clear the error injection bits */
-	regmap_write(drvdata->mc_vbase,	CTLCFG_OFST, read_reg);
+	regmap_write(drvdata->mc_vbase,	priv->ce_ue_trgr_offset, read_reg);
 	/* Ensure it has been written out */
 	wmb();
 
@@ -250,18 +205,29 @@ static unsigned long get_total_mem(void)
 	return total_mem;
 }
 
+static const struct of_device_id altr_sdram_ctrl_of_match[] = {
+	{ .compatible = "altr,sdram-edac", .data = (void *)&c5_data},
+	{},
+};
+MODULE_DEVICE_TABLE(of, altr_sdram_ctrl_of_match);
+
 static int altr_sdram_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *id;
 	struct edac_mc_layer layers[2];
 	struct mem_ctl_info *mci;
 	struct altr_sdram_mc_data *drvdata;
+	const struct altr_sdram_prv_data *priv;
 	struct regmap *mc_vbase;
 	struct dimm_info *dimm;
-	u32 read_reg, mem_size;
-	int irq;
-	int res = 0;
+	u32 read_reg;
+	int irq, res = 0;
+	unsigned long mem_size;
+
+	id = of_match_device(altr_sdram_ctrl_of_match, &pdev->dev);
+	if (!id)
+		return -ENODEV;
 
-	/* Validate the SDRAM controller has ECC enabled */
 	/* Grab the register range from the sdr controller in device tree */
 	mc_vbase = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
 						   "altr,sdr-syscon");
@@ -271,8 +237,13 @@ static int altr_sdram_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	if (regmap_read(mc_vbase, CTLCFG_OFST, &read_reg) ||
-	    ((read_reg & CTLCFG_ECC_AUTO_EN) !=	CTLCFG_ECC_AUTO_EN)) {
+	/* Check specific dependencies for the module */
+	priv = of_match_node(altr_sdram_ctrl_of_match,
+			     pdev->dev.of_node)->data;
+
+	/* Validate the SDRAM controller has ECC enabled */
+	if (regmap_read(mc_vbase, priv->ecc_ctrl_offset, &read_reg) ||
+	    ((read_reg & priv->ecc_ctl_en_mask) != priv->ecc_ctl_en_mask)) {
 		edac_printk(KERN_ERR, EDAC_MC,
 			    "No ECC/ECC disabled [0x%08X]\n", read_reg);
 		return -ENODEV;
@@ -286,10 +257,27 @@ static int altr_sdram_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	/* Ensure the SDRAM Interrupt is disabled and cleared */
-	if (regmap_write(mc_vbase, DRAMINTR_OFST, DRAMINTR_INTRCLR)) {
+	/* Ensure the SDRAM Interrupt is disabled */
+	if (regmap_update_bits(mc_vbase, priv->ecc_irq_en_offset,
+			       priv->ecc_irq_en_mask, 0)) {
+		edac_printk(KERN_ERR, EDAC_MC,
+			    "Error disabling SDRAM ECC IRQ\n");
+		return -ENODEV;
+	}
+
+	/* Toggle to clear the SDRAM Error count */
+	if (regmap_update_bits(mc_vbase, priv->ecc_cnt_rst_offset,
+			       priv->ecc_cnt_rst_mask,
+			       priv->ecc_cnt_rst_mask)) {
+		edac_printk(KERN_ERR, EDAC_MC,
+			    "Error clearing SDRAM ECC count\n");
+		return -ENODEV;
+	}
+
+	if (regmap_update_bits(mc_vbase, priv->ecc_cnt_rst_offset,
+			       priv->ecc_cnt_rst_mask, 0)) {
 		edac_printk(KERN_ERR, EDAC_MC,
-			    "Error clearing SDRAM ECC IRQ\n");
+			    "Error clearing SDRAM ECC count\n");
 		return -ENODEV;
 	}
 
@@ -314,9 +302,12 @@ static int altr_sdram_probe(struct platform_device *pdev)
 	mci->pdev = &pdev->dev;
 	drvdata = mci->pvt_info;
 	drvdata->mc_vbase = mc_vbase;
+	drvdata->data = priv;
 	platform_set_drvdata(pdev, mci);
 
 	if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
+		edac_printk(KERN_ERR, EDAC_MC,
+			    "Unable to get managed device resource\n");
 		res = -ENOMEM;
 		goto free;
 	}
@@ -350,8 +341,9 @@ static int altr_sdram_probe(struct platform_device *pdev)
 		goto err2;
 	}
 
-	if (regmap_write(drvdata->mc_vbase, DRAMINTR_OFST,
-			 (DRAMINTR_INTRCLR | DRAMINTR_INTREN))) {
+	/* Infrastructure ready - enable the IRQ */
+	if (regmap_update_bits(drvdata->mc_vbase, priv->ecc_irq_en_offset,
+			       priv->ecc_irq_en_mask, priv->ecc_irq_en_mask)) {
 		edac_mc_printk(mci, KERN_ERR,
 			       "Error enabling SDRAM ECC IRQ\n");
 		res = -ENODEV;
@@ -387,12 +379,6 @@ static int altr_sdram_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id altr_sdram_ctrl_of_match[] = {
-	{ .compatible = "altr,sdram-edac", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, altr_sdram_ctrl_of_match);
-
 static struct platform_driver altr_sdram_edac_driver = {
 	.probe = altr_sdram_probe,
 	.remove = altr_sdram_remove,
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
new file mode 100644
index 0000000..b744d91
--- /dev/null
+++ b/drivers/edac/altera_edac.h
@@ -0,0 +1,116 @@
+/*
+ *
+ * Copyright (C) 2015 Altera Corporation
+ *
+ * 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 _ALTERA_EDAC_H
+#define _ALTERA_EDAC_H
+
+#include <linux/edac.h>
+#include <linux/types.h>
+
+/* SDRAM Controller CtrlCfg Register */
+#define CV_CTLCFG_OFST             0x00
+
+/* SDRAM Controller CtrlCfg Register Bit Masks */
+#define CV_CTLCFG_ECC_EN           0x400
+#define CV_CTLCFG_ECC_CORR_EN      0x800
+#define CV_CTLCFG_GEN_SB_ERR       0x2000
+#define CV_CTLCFG_GEN_DB_ERR       0x4000
+
+#define CV_CTLCFG_ECC_AUTO_EN     (CV_CTLCFG_ECC_EN | \
+				   CV_CTLCFG_ECC_CORR_EN)
+
+/* SDRAM Controller Address Width Register */
+#define CV_DRAMADDRW_OFST          0x2C
+
+/* SDRAM Controller Address Widths Field Register */
+#define DRAMADDRW_COLBIT_MASK      0x001F
+#define DRAMADDRW_COLBIT_SHIFT     0
+#define DRAMADDRW_ROWBIT_MASK      0x03E0
+#define DRAMADDRW_ROWBIT_SHIFT     5
+#define CV_DRAMADDRW_BANKBIT_MASK  0x1C00
+#define CV_DRAMADDRW_BANKBIT_SHIFT 10
+#define CV_DRAMADDRW_CSBIT_MASK    0xE000
+#define CV_DRAMADDRW_CSBIT_SHIFT   13
+
+/* SDRAM Controller Interface Data Width Register */
+#define CV_DRAMIFWIDTH_OFST        0x30
+
+/* SDRAM Controller Interface Data Width Defines */
+#define CV_DRAMIFWIDTH_16B_ECC     24
+#define CV_DRAMIFWIDTH_32B_ECC     40
+
+/* SDRAM Controller DRAM Status Register */
+#define CV_DRAMSTS_OFST            0x38
+
+/* SDRAM Controller DRAM Status Register Bit Masks */
+#define CV_DRAMSTS_SBEERR          0x04
+#define CV_DRAMSTS_DBEERR          0x08
+#define CV_DRAMSTS_CORR_DROP       0x10
+
+/* SDRAM Controller DRAM IRQ Register */
+#define CV_DRAMINTR_OFST           0x3C
+
+/* SDRAM Controller DRAM IRQ Register Bit Masks */
+#define CV_DRAMINTR_INTREN         0x01
+#define CV_DRAMINTR_SBEMASK        0x02
+#define CV_DRAMINTR_DBEMASK        0x04
+#define CV_DRAMINTR_CORRDROPMASK   0x08
+#define CV_DRAMINTR_INTRCLR        0x10
+
+/* SDRAM Controller Single Bit Error Count Register */
+#define CV_SBECOUNT_OFST           0x40
+
+/* SDRAM Controller Double Bit Error Count Register */
+#define CV_DBECOUNT_OFST           0x44
+
+/* SDRAM Controller ECC Error Address Register */
+#define CV_ERRADDR_OFST            0x48
+
+struct altr_sdram_prv_data {
+	int ecc_ctrl_offset;
+	int ecc_ctl_en_mask;
+	int ecc_cecnt_offset;
+	int ecc_uecnt_offset;
+	int ecc_stat_offset;
+	int ecc_stat_ce_mask;
+	int ecc_stat_ue_mask;
+	int ecc_saddr_offset;
+	int ecc_daddr_offset;
+	int ecc_irq_en_offset;
+	int ecc_irq_en_mask;
+	int ecc_irq_clr_offset;
+	int ecc_irq_clr_mask;
+	int ecc_cnt_rst_offset;
+	int ecc_cnt_rst_mask;
+#ifdef CONFIG_EDAC_DEBUG
+	struct edac_dev_sysfs_attribute *eccmgr_sysfs_attr;
+	int ecc_enable_mask;
+	int ce_set_mask;
+	int ue_set_mask;
+	int ce_ue_trgr_offset;
+#endif
+};
+
+/* Altera SDRAM Memory Controller data */
+struct altr_sdram_mc_data {
+	struct regmap *mc_vbase;
+	int sb_irq;
+	int db_irq;
+	const struct altr_sdram_prv_data *data;
+};
+
+#endif	/* #ifndef _ALTERA_EDAC_H */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] edac, altera: Addition of Arria10 EDAC
  2015-05-13 21:49 [PATCH 0/4] Add Altera Arria10 EDAC Support tthayer
  2015-05-13 21:49 ` [PATCH 1/4] edac, altera: Generalize driver to use DT Memory size tthayer
  2015-05-13 21:49 ` [PATCH 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC tthayer
@ 2015-05-13 21:49 ` tthayer
  2015-05-14 20:20   ` Dinh Nguyen
  2015-05-15 10:57   ` Arnd Bergmann
  2015-05-13 21:49 ` [PATCH 4/4] dts, altera: Arria10 SDRAM EDAC DTS additions tthayer
  3 siblings, 2 replies; 13+ messages in thread
From: tthayer @ 2015-05-13 21:49 UTC (permalink / raw
  To: bp, dougthompson, m.chehab, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, linux, dinguyen, grant.likely
  Cc: devicetree, linux-doc, linux-edac, linux-kernel, linux-arm-kernel,
	tthayer.linux, tthayer

From: Thor Thayer <tthayer@opensource.altera.com>

The Arria10 SDRAM and ECC system differs significantly from the
Cyclone5 and Arria5 SoCs. This patch adds support for the Arria10
SoC.
1) IRQ handler needs to support SHARED IRQ
2) Support sberr and dberr address reporting.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 drivers/edac/altera_edac.c |  132 ++++++++++++++++++++++++++++++++++++++------
 drivers/edac/altera_edac.h |   85 ++++++++++++++++++++++++++++
 2 files changed, 201 insertions(+), 16 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 204ad2d..735a180 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -42,6 +42,7 @@ const struct altr_sdram_prv_data c5_data = {
 	.ecc_stat_ce_mask   = CV_DRAMSTS_SBEERR,
 	.ecc_stat_ue_mask   = CV_DRAMSTS_DBEERR,
 	.ecc_saddr_offset   = CV_ERRADDR_OFST,
+	.ecc_daddr_offset   = CV_ERRADDR_OFST,
 	.ecc_cecnt_offset   = CV_SBECOUNT_OFST,
 	.ecc_uecnt_offset   = CV_DBECOUNT_OFST,
 	.ecc_irq_en_offset  = CV_DRAMINTR_OFST,
@@ -57,37 +58,62 @@ const struct altr_sdram_prv_data c5_data = {
 #endif
 };
 
+const struct altr_sdram_prv_data a10_data = {
+	.ecc_ctrl_offset    = A10_ECCCTRL1_OFST,
+	.ecc_ctl_en_mask    = A10_ECCCTRL1_ECC_EN,
+	.ecc_stat_offset    = A10_INTSTAT_OFST,
+	.ecc_stat_ce_mask   = A10_INTSTAT_SBEERR,
+	.ecc_stat_ue_mask   = A10_INTSTAT_DBEERR,
+	.ecc_saddr_offset   = A10_SERRADDR_OFST,
+	.ecc_daddr_offset   = A10_DERRADDR_OFST,
+	.ecc_irq_en_offset  = A10_ERRINTEN_OFST,
+	.ecc_irq_en_mask    = A10_ECC_IRQ_EN_MASK,
+	.ecc_irq_clr_offset = A10_INTSTAT_OFST,
+	.ecc_irq_clr_mask   = (A10_INTSTAT_SBEERR | A10_INTSTAT_DBEERR),
+	.ecc_cnt_rst_offset = A10_ECCCTRL1_OFST,
+	.ecc_cnt_rst_mask   = A10_ECC_CNT_RESET_MASK,
+#ifdef CONFIG_EDAC_DEBUG
+	.ce_ue_trgr_offset  = A10_DIAGINTTEST_OFST,
+	.ce_set_mask        = A10_DIAGINT_TSERRA_MASK,
+	.ue_set_mask        = A10_DIAGINT_TDERRA_MASK,
+#endif
+};
+
 static irqreturn_t altr_sdram_mc_err_handler(int irq, void *dev_id)
 {
 	struct mem_ctl_info *mci = dev_id;
 	struct altr_sdram_mc_data *drvdata = mci->pvt_info;
 	const struct altr_sdram_prv_data *priv = drvdata->data;
-	u32 status, err_count, err_addr;
-
-	/* Error Address is shared by both SBE & DBE */
-	regmap_read(drvdata->mc_vbase, priv->ecc_saddr_offset, &err_addr);
+	u32 status, err_count = 1, err_addr;
 
 	regmap_read(drvdata->mc_vbase, priv->ecc_stat_offset, &status);
 
 	if (status & priv->ecc_stat_ue_mask) {
-		regmap_read(drvdata->mc_vbase, priv->ecc_uecnt_offset,
-			    &err_count);
+		regmap_read(drvdata->mc_vbase, priv->ecc_daddr_offset,
+			    &err_addr);
+		if (priv->ecc_uecnt_offset)
+			regmap_read(drvdata->mc_vbase, priv->ecc_uecnt_offset,
+				    &err_count);
 		panic("\nEDAC: [%d Uncorrectable errors @ 0x%08X]\n",
 		      err_count, err_addr);
 	}
 	if (status & priv->ecc_stat_ce_mask) {
-		regmap_read(drvdata->mc_vbase,  priv->ecc_cecnt_offset,
-			    &err_count);
+		regmap_read(drvdata->mc_vbase, priv->ecc_saddr_offset,
+			    &err_addr);
+		if (priv->ecc_uecnt_offset)
+			regmap_read(drvdata->mc_vbase,  priv->ecc_cecnt_offset,
+				    &err_count);
 		edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci, err_count,
 				     err_addr >> PAGE_SHIFT,
 				     err_addr & ~PAGE_MASK, 0,
 				     0, 0, -1, mci->ctl_name, "");
-	}
-
-	regmap_write(drvdata->mc_vbase,	priv->ecc_irq_clr_offset,
-		     priv->ecc_irq_clr_mask);
+		/* Clear IRQ to resume */
+		regmap_write(drvdata->mc_vbase,	priv->ecc_irq_clr_offset,
+			     priv->ecc_irq_clr_mask);
 
-	return IRQ_HANDLED;
+		return IRQ_HANDLED;
+	}
+	return IRQ_NONE;
 }
 
 #ifdef CONFIG_EDAC_DEBUG
@@ -207,10 +233,58 @@ static unsigned long get_total_mem(void)
 
 static const struct of_device_id altr_sdram_ctrl_of_match[] = {
 	{ .compatible = "altr,sdram-edac", .data = (void *)&c5_data},
+	{ .compatible = "altr,sdram-edac-a10", .data = (void *)&a10_data},
 	{},
 };
 MODULE_DEVICE_TABLE(of, altr_sdram_ctrl_of_match);
 
+static int a10_init(struct regmap *mc_vbase)
+{
+	if (regmap_update_bits(mc_vbase, A10_INTMODE_OFST,
+			       A10_INTMODE_SB_INT, A10_INTMODE_SB_INT)) {
+		edac_printk(KERN_ERR, EDAC_MC,
+			    "Error setting SB IRQ mode\n");
+		return -ENODEV;
+	}
+
+	if (regmap_write(mc_vbase, A10_SERRCNTREG_OFST, 1)) {
+		edac_printk(KERN_ERR, EDAC_MC,
+			    "Error setting trigger count\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int a10_unmask_irq(struct platform_device *pdev, u32 mask)
+{
+	void __iomem  *sm_base;
+
+	if (!devm_request_mem_region(&pdev->dev, A10_SYMAN_INTMASK_CLR,
+				     sizeof(u32), dev_name(&pdev->dev))) {
+		edac_printk(KERN_ERR, EDAC_MC,
+			    "Unable to request mem region\n");
+		return -EBUSY;
+	}
+
+	sm_base = devm_ioremap(&pdev->dev, A10_SYMAN_INTMASK_CLR,
+			       sizeof(u32));
+	if (!sm_base) {
+		edac_printk(KERN_ERR, EDAC_MC,
+			    "Unable to ioremap device\n");
+
+		return -ENOMEM;
+	}
+
+	iowrite32(mask, sm_base);
+
+	devm_iounmap(&pdev->dev, sm_base);
+	devm_release_mem_region(&pdev->dev, A10_SYMAN_INTMASK_CLR,
+				sizeof(u32));
+
+	return 0;
+}
+
 static int altr_sdram_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *id;
@@ -221,8 +295,8 @@ static int altr_sdram_probe(struct platform_device *pdev)
 	struct regmap *mc_vbase;
 	struct dimm_info *dimm;
 	u32 read_reg;
-	int irq, res = 0;
-	unsigned long mem_size;
+	int irq, irq2, res = 0;
+	unsigned long mem_size, irqflags;
 
 	id = of_match_device(altr_sdram_ctrl_of_match, &pdev->dev);
 	if (!id)
@@ -288,6 +362,9 @@ static int altr_sdram_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	/* Arria10 has a 2nd IRQ */
+	irq2 = platform_get_irq(pdev, 1);
+
 	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
 	layers[0].size = 1;
 	layers[0].is_virt_csrow = true;
@@ -332,8 +409,31 @@ static int altr_sdram_probe(struct platform_device *pdev)
 	if (res < 0)
 		goto err;
 
+	/* Only the Arria10 has separate IRQs */
+	if (irq2 > 0) {
+		/* Arria10 specific initialization */
+		res = a10_init(mc_vbase);
+		if (res < 0)
+			goto err2;
+
+		res = a10_unmask_irq(pdev, A10_DDR0_IRQ_MASK);
+		if (res < 0)
+			goto err2;
+
+		res = devm_request_irq(&pdev->dev, irq2,
+				       altr_sdram_mc_err_handler,
+				       IRQF_SHARED, dev_name(&pdev->dev), mci);
+		if (res < 0) {
+			edac_mc_printk(mci, KERN_ERR,
+				       "Unable to request irq %d\n", irq2);
+			res = -ENODEV;
+			goto err2;
+		}
+		irqflags = IRQF_SHARED;
+	}
+
 	res = devm_request_irq(&pdev->dev, irq, altr_sdram_mc_err_handler,
-			       0, dev_name(&pdev->dev), mci);
+			       irqflags, dev_name(&pdev->dev), mci);
 	if (res < 0) {
 		edac_mc_printk(mci, KERN_ERR,
 			       "Unable to request irq %d\n", irq);
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index b744d91..7b64dc7 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -80,6 +80,91 @@
 /* SDRAM Controller ECC Error Address Register */
 #define CV_ERRADDR_OFST            0x48
 
+/*-----------------------------------------*/
+
+/* SDRAM Controller EccCtrl Register */
+#define A10_ECCCTRL1_OFST          0x00
+
+/* SDRAM Controller EccCtrl Register Bit Masks */
+#define A10_ECCCTRL1_ECC_EN        0x001
+#define A10_ECCCTRL1_CNT_RST       0x010
+#define A10_ECCCTRL1_AWB_CNT_RST   0x100
+#define A10_ECC_CNT_RESET_MASK     (A10_ECCCTRL1_CNT_RST | \
+				    A10_ECCCTRL1_AWB_CNT_RST)
+
+/* SDRAM Controller Address Width Register */
+#define CV_DRAMADDRW               0xFFC2502C
+#define A10_DRAMADDRW              0xFFCFA0A8
+
+/* SDRAM Controller Address Widths Field Register */
+#define DRAMADDRW_COLBIT_MASK      0x001F
+#define DRAMADDRW_COLBIT_SHIFT     0
+#define DRAMADDRW_ROWBIT_MASK      0x03E0
+#define DRAMADDRW_ROWBIT_SHIFT     5
+#define CV_DRAMADDRW_BANKBIT_MASK  0x1C00
+#define CV_DRAMADDRW_BANKBIT_SHIFT 10
+#define CV_DRAMADDRW_CSBIT_MASK    0xE000
+#define CV_DRAMADDRW_CSBIT_SHIFT   13
+
+#define A10_DRAMADDRW_BANKBIT_MASK  0x3C00
+#define A10_DRAMADDRW_BANKBIT_SHIFT 10
+#define A10_DRAMADDRW_GRPBIT_MASK   0xC000
+#define A10_DRAMADDRW_GRPBIT_SHIFT  14
+#define A10_DRAMADDRW_CSBIT_MASK    0x70000
+#define A10_DRAMADDRW_CSBIT_SHIFT   16
+
+/* SDRAM Controller Interface Data Width Register */
+#define CV_DRAMIFWIDTH             0xFFC25030
+#define A10_DRAMIFWIDTH            0xFFCFB008
+
+/* SDRAM Controller Interface Data Width Defines */
+#define CV_DRAMIFWIDTH_16B_ECC     24
+#define CV_DRAMIFWIDTH_32B_ECC     40
+
+#define A10_DRAMIFWIDTH_16B        0x0
+#define A10_DRAMIFWIDTH_32B        0x1
+#define A10_DRAMIFWIDTH_64B        0x2
+
+/* SDRAM Controller DRAM IRQ Register */
+#define A10_ERRINTEN_OFST          0x10
+
+/* SDRAM Controller DRAM IRQ Register Bit Masks */
+#define A10_ERRINTEN_SERRINTEN     0x01
+#define A10_ERRINTEN_DERRINTEN     0x02
+#define A10_ECC_IRQ_EN_MASK        (A10_ERRINTEN_SERRINTEN | \
+				    A10_ERRINTEN_DERRINTEN)
+
+/* SDRAM Interrupt Mode Register */
+#define A10_INTMODE_OFST           0x1C
+#define A10_INTMODE_SB_INT         1
+
+/* SDRAM Controller Error Status Register */
+#define A10_INTSTAT_OFST           0x20
+
+/* SDRAM Controller Error Status Register Bit Masks */
+#define A10_INTSTAT_SBEERR         0x01
+#define A10_INTSTAT_DBEERR         0x02
+
+/* SDRAM Controller ECC Error Address Register */
+#define A10_DERRADDR_OFST          0x2C
+#define A10_SERRADDR_OFST          0x30
+
+/* SDRAM Controller ECC Diagnostic Register */
+#define A10_DIAGINTTEST_OFST       0x24
+
+#define A10_DIAGINT_TSERRA_MASK    0x0001
+#define A10_DIAGINT_TDERRA_MASK    0x0100
+
+#define A10_SBERR_IRQ              34
+#define A10_DBERR_IRQ              32
+
+/* SDRAM Single Bit Error Count Compare Set Register */
+#define A10_SERRCNTREG_OFST        0x3C
+
+#define A10_SYMAN_INTMASK_CLR      0xFFD06098
+#define A10_INTMASK_CLR_OFST       0x10
+#define A10_DDR0_IRQ_MASK          BIT(17)
+
 struct altr_sdram_prv_data {
 	int ecc_ctrl_offset;
 	int ecc_ctl_en_mask;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] dts, altera: Arria10 SDRAM EDAC DTS additions.
  2015-05-13 21:49 [PATCH 0/4] Add Altera Arria10 EDAC Support tthayer
                   ` (2 preceding siblings ...)
  2015-05-13 21:49 ` [PATCH 3/4] edac, altera: Addition of Arria10 EDAC tthayer
@ 2015-05-13 21:49 ` tthayer
  2015-05-15 10:55   ` Arnd Bergmann
  3 siblings, 1 reply; 13+ messages in thread
From: tthayer @ 2015-05-13 21:49 UTC (permalink / raw
  To: bp, dougthompson, m.chehab, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, linux, dinguyen, grant.likely
  Cc: devicetree, linux-doc, linux-edac, linux-kernel, linux-arm-kernel,
	tthayer.linux, tthayer

From: Thor Thayer <tthayer@opensource.altera.com>

Support for the Arria10 SDRAM EDAC is added to the device tree.
Update the bindings document for the new match string.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 .../bindings/arm/altera/socfpga-sdram-edac.txt     |    2 +-
 arch/arm/boot/dts/socfpga_arria10.dtsi             |   11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-sdram-edac.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-sdram-edac.txt
index d0ce01d..f5ad0ff 100644
--- a/Documentation/devicetree/bindings/arm/altera/socfpga-sdram-edac.txt
+++ b/Documentation/devicetree/bindings/arm/altera/socfpga-sdram-edac.txt
@@ -2,7 +2,7 @@ Altera SOCFPGA SDRAM Error Detection & Correction [EDAC]
 The EDAC accesses a range of registers in the SDRAM controller.
 
 Required properties:
-- compatible : should contain "altr,sdram-edac";
+- compatible : should contain "altr,sdram-edac" or "altr,sdram-edac-a10"
 - altr,sdr-syscon : phandle of the sdr module
 - interrupts : Should contain the SDRAM ECC IRQ in the
 	appropriate format for the IRQ controller.
diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi b/arch/arm/boot/dts/socfpga_arria10.dtsi
index e121661..70da147 100644
--- a/arch/arm/boot/dts/socfpga_arria10.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
@@ -524,6 +524,17 @@
 			status = "disabled";
 		};
 
+		sdr: sdr@ffc25000 {
+			compatible = "syscon";
+			reg = <0xffcfb100 0x80>;
+		};
+
+		sdramedac {
+			compatible = "altr,sdram-edac-a10";
+			altr,sdr-syscon = <&sdr>;
+			interrupts = <0 2 4>, <0 0 4>;
+		};
+
 		L2: l2-cache@fffff000 {
 			compatible = "arm,pl310-cache";
 			reg = <0xfffff000 0x1000>;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] edac, altera: Generalize driver to use DT Memory size
  2015-05-13 21:49 ` [PATCH 1/4] edac, altera: Generalize driver to use DT Memory size tthayer
@ 2015-05-14  3:25   ` Dinh Nguyen
  2015-05-15 11:00   ` Arnd Bergmann
  1 sibling, 0 replies; 13+ messages in thread
From: Dinh Nguyen @ 2015-05-14  3:25 UTC (permalink / raw
  To: tthayer, bp, dougthompson, m.chehab, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, linux, grant.likely
  Cc: devicetree, linux-doc, linux-edac, linux-kernel, linux-arm-kernel,
	tthayer.linux



On 5/13/15 4:49 PM, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> The Arria10 SOC uses a completely different SDRAM controller from the
> earlier CycloneV and ArriaV SoCs. The memory size is calculated in
> the bootloader and passed via the device tree. Using this device
> tree size is more generic than using the register fields to
> calculate the memory size for different SDRAM controllers.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  drivers/edac/altera_edac.c |   53 ++++++++++++++++++++++----------------------
>  1 file changed, 26 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index 3c4929f..e18a205 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c
> @@ -219,36 +219,35 @@ static void altr_sdr_mc_create_debugfs_nodes(struct mem_ctl_info *mci)
>  {}
>  #endif
>  
> -/* Get total memory size in bytes */
> -static u32 altr_sdram_get_total_mem_size(struct regmap *mc_vbase)
> +/* Get total memory size from Open Firmware DTB */
> +static unsigned long get_total_mem(void)
>  {
> -	u32 size, read_reg, row, bank, col, cs, width;
> +	struct device_node *np = NULL;
> +	const unsigned int *reg, *reg_end;
> +	int len, sw, aw;
> +	unsigned long start, size, total_mem;
>  
> -	if (regmap_read(mc_vbase, DRAMADDRW_OFST, &read_reg) < 0)
> +	np = of_find_node_by_type(NULL, "memory");
> +	if (!np)
>  		return 0;
>  
> -	if (regmap_read(mc_vbase, DRAMIFWIDTH_OFST, &width) < 0)
> -		return 0;
> -
> -	col = (read_reg & DRAMADDRW_COLBIT_MASK) >>
> -		DRAMADDRW_COLBIT_SHIFT;
> -	row = (read_reg & DRAMADDRW_ROWBIT_MASK) >>
> -		DRAMADDRW_ROWBIT_SHIFT;
> -	bank = (read_reg & DRAMADDRW_BANKBIT_MASK) >>
> -		DRAMADDRW_BANKBIT_SHIFT;
> -	cs = (read_reg & DRAMADDRW_CSBIT_MASK) >>
> -		DRAMADDRW_CSBIT_SHIFT;
> -
> -	/* Correct for ECC as its not addressible */
> -	if (width == DRAMIFWIDTH_32B_ECC)
> -		width = 32;
> -	if (width == DRAMIFWIDTH_16B_ECC)
> -		width = 16;
> -
> -	/* calculate the SDRAM size base on this info */
> -	size = 1 << (row + bank + col);
> -	size = size * cs * (width / 8);
> -	return size;
> +	aw = of_n_addr_cells(np);
> +	sw = of_n_size_cells(np);
> +	reg = (const unsigned int *)of_get_property(np, "reg", &len);
> +	reg_end = reg + (len / sizeof(u32));
> +
> +	total_mem = 0;
> +	do {
> +		start = of_read_number(reg, aw);
> +		reg += aw;
> +		size = of_read_number(reg, sw);
> +		reg += sw;
> +		total_mem += size;
> +	} while (reg < reg_end);
> +
> +	of_node_put(np);
> +	edac_printk(KERN_ERR, EDAC_MC, "total_mem 0x%lx\n", total_mem);

Use edac_dbg() here.

Dinh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC.
  2015-05-13 21:49 ` [PATCH 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC tthayer
@ 2015-05-14 20:13   ` Dinh Nguyen
  0 siblings, 0 replies; 13+ messages in thread
From: Dinh Nguyen @ 2015-05-14 20:13 UTC (permalink / raw
  To: tthayer, bp, dougthompson, m.chehab, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, linux, grant.likely
  Cc: devicetree, linux-doc, linux-edac, linux-kernel, linux-arm-kernel,
	tthayer.linux

On 05/13/2015 04:49 PM, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> The Arria10 SOC uses a completely different SDRAM controller from the
> earlier CycloneV and ArriaV SoCs. This patch abstracts the SDRAM bits
> for the CycloneV/ArriaV SoCs in preparation for the Arria10 support.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  drivers/edac/altera_edac.c |  194 ++++++++++++++++++++------------------------
>  drivers/edac/altera_edac.h |  116 ++++++++++++++++++++++++++
>  2 files changed, 206 insertions(+), 104 deletions(-)
>  create mode 100644 drivers/edac/altera_edac.h
> 
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index e18a205..204ad2d 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c
> @@ -1,5 +1,5 @@
>  /*
> - *  Copyright Altera Corporation (C) 2014. All rights reserved.
> + *  Copyright Altera Corporation (C) 2014-2015. All rights reserved.
>   *  Copyright 2011-2012 Calxeda, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify it
> @@ -28,111 +28,64 @@
>  #include <linux/types.h>
>  #include <linux/uaccess.h>
>  
[...]
> -
> -/* Altera SDRAM Memory Controller data */
> -struct altr_sdram_mc_data {
> -	struct regmap *mc_vbase;
> +const struct altr_sdram_prv_data c5_data = {

This should be static.

Dinh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] edac, altera: Addition of Arria10 EDAC
  2015-05-13 21:49 ` [PATCH 3/4] edac, altera: Addition of Arria10 EDAC tthayer
@ 2015-05-14 20:20   ` Dinh Nguyen
  2015-05-14 20:38     ` Thor Thayer
  2015-05-15 10:57   ` Arnd Bergmann
  1 sibling, 1 reply; 13+ messages in thread
From: Dinh Nguyen @ 2015-05-14 20:20 UTC (permalink / raw
  To: tthayer, bp, dougthompson, m.chehab, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, linux, grant.likely
  Cc: devicetree, linux-doc, linux-edac, linux-kernel, linux-arm-kernel,
	tthayer.linux

On 05/13/2015 04:49 PM, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> The Arria10 SDRAM and ECC system differs significantly from the
> Cyclone5 and Arria5 SoCs. This patch adds support for the Arria10
> SoC.
> 1) IRQ handler needs to support SHARED IRQ
> 2) Support sberr and dberr address reporting.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  drivers/edac/altera_edac.c |  132 ++++++++++++++++++++++++++++++++++++++------
>  drivers/edac/altera_edac.h |   85 ++++++++++++++++++++++++++++
>  2 files changed, 201 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index 204ad2d..735a180 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c
> @@ -42,6 +42,7 @@ const struct altr_sdram_prv_data c5_data = {
>  	.ecc_stat_ce_mask   = CV_DRAMSTS_SBEERR,
>  	.ecc_stat_ue_mask   = CV_DRAMSTS_DBEERR,
>  	.ecc_saddr_offset   = CV_ERRADDR_OFST,
> +	.ecc_daddr_offset   = CV_ERRADDR_OFST,
>  	.ecc_cecnt_offset   = CV_SBECOUNT_OFST,
>  	.ecc_uecnt_offset   = CV_DBECOUNT_OFST,
>  	.ecc_irq_en_offset  = CV_DRAMINTR_OFST,
> @@ -57,37 +58,62 @@ const struct altr_sdram_prv_data c5_data = {
>  #endif
>  };
>  
> +const struct altr_sdram_prv_data a10_data = {\

This should be static.

> +	.ecc_ctrl_offset    = A10_ECCCTRL1_OFST,
> +	.ecc_ctl_en_mask    = A10_ECCCTRL1_ECC_EN,
> +	.ecc_stat_offset    = A10_INTSTAT_OFST,
> +	.ecc_stat_ce_mask   = A10_INTSTAT_SBEERR,
> +	.ecc_stat_ue_mask   = A10_INTSTAT_DBEERR,
> +	.ecc_saddr_offset   = A10_SERRADDR_OFST,
> +	.ecc_daddr_offset   = A10_DERRADDR_OFST,
> +	.ecc_irq_en_offset  = A10_ERRINTEN_OFST,
> +	.ecc_irq_en_mask    = A10_ECC_IRQ_EN_MASK,
> +	.ecc_irq_clr_offset = A10_INTSTAT_OFST,
> +	.ecc_irq_clr_mask   = (A10_INTSTAT_SBEERR | A10_INTSTAT_DBEERR),
> +	.ecc_cnt_rst_offset = A10_ECCCTRL1_OFST,
> +	.ecc_cnt_rst_mask   = A10_ECC_CNT_RESET_MASK,
> +#ifdef CONFIG_EDAC_DEBUG
> +	.ce_ue_trgr_offset  = A10_DIAGINTTEST_OFST,
> +	.ce_set_mask        = A10_DIAGINT_TSERRA_MASK,
> +	.ue_set_mask        = A10_DIAGINT_TDERRA_MASK,
> +#endif
> +};
> +
>
<snip>

> +
>  static int altr_sdram_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *id;
> @@ -221,8 +295,8 @@ static int altr_sdram_probe(struct platform_device *pdev)
>  	struct regmap *mc_vbase;
>  	struct dimm_info *dimm;
>  	u32 read_reg;
> -	int irq, res = 0;
> -	unsigned long mem_size;
> +	int irq, irq2, res = 0;
> +	unsigned long mem_size, irqflags;
>  
>  	id = of_match_device(altr_sdram_ctrl_of_match, &pdev->dev);
>  	if (!id)
> @@ -288,6 +362,9 @@ static int altr_sdram_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> +	/* Arria10 has a 2nd IRQ */
> +	irq2 = platform_get_irq(pdev, 1);
> +
>  	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
>  	layers[0].size = 1;
>  	layers[0].is_virt_csrow = true;
> @@ -332,8 +409,31 @@ static int altr_sdram_probe(struct platform_device *pdev)
>  	if (res < 0)
>  		goto err;
>  
> +	/* Only the Arria10 has separate IRQs */
> +	if (irq2 > 0) {
> +		/* Arria10 specific initialization */
> +		res = a10_init(mc_vbase);
> +		if (res < 0)
> +			goto err2;
> +
> +		res = a10_unmask_irq(pdev, A10_DDR0_IRQ_MASK);
> +		if (res < 0)
> +			goto err2;
> +
> +		res = devm_request_irq(&pdev->dev, irq2,
> +				       altr_sdram_mc_err_handler,
> +				       IRQF_SHARED, dev_name(&pdev->dev), mci);
> +		if (res < 0) {
> +			edac_mc_printk(mci, KERN_ERR,
> +				       "Unable to request irq %d\n", irq2);
> +			res = -ENODEV;
> +			goto err2;
> +		}
> +		irqflags = IRQF_SHARED;
> +	}
> +
>  	res = devm_request_irq(&pdev->dev, irq, altr_sdram_mc_err_handler,
> -			       0, dev_name(&pdev->dev), mci);
> +			       irqflags, dev_name(&pdev->dev), mci);

irqflags was never set for the case of !(irq2 > 0).

Dinh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] edac, altera: Addition of Arria10 EDAC
  2015-05-14 20:20   ` Dinh Nguyen
@ 2015-05-14 20:38     ` Thor Thayer
  0 siblings, 0 replies; 13+ messages in thread
From: Thor Thayer @ 2015-05-14 20:38 UTC (permalink / raw
  To: Dinh Nguyen, bp, dougthompson, m.chehab, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, linux, grant.likely
  Cc: devicetree, linux-doc, linux-edac, linux-kernel, linux-arm-kernel,
	tthayer.linux



On 05/14/2015 03:20 PM, Dinh Nguyen wrote:
> On 05/13/2015 04:49 PM, tthayer@opensource.altera.com wrote:
>> From: Thor Thayer <tthayer@opensource.altera.com>
>>
>> The Arria10 SDRAM and ECC system differs significantly from the
>> Cyclone5 and Arria5 SoCs. This patch adds support for the Arria10
>> SoC.
>> 1) IRQ handler needs to support SHARED IRQ
>> 2) Support sberr and dberr address reporting.
>>
>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>> ---
>>   drivers/edac/altera_edac.c |  132 ++++++++++++++++++++++++++++++++++++++------
>>   drivers/edac/altera_edac.h |   85 ++++++++++++++++++++++++++++
>>   2 files changed, 201 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
>> index 204ad2d..735a180 100644
>> --- a/drivers/edac/altera_edac.c
>> +++ b/drivers/edac/altera_edac.c
>> @@ -42,6 +42,7 @@ const struct altr_sdram_prv_data c5_data = {
>>   	.ecc_stat_ce_mask   = CV_DRAMSTS_SBEERR,
>>   	.ecc_stat_ue_mask   = CV_DRAMSTS_DBEERR,
>>   	.ecc_saddr_offset   = CV_ERRADDR_OFST,
>> +	.ecc_daddr_offset   = CV_ERRADDR_OFST,
>>   	.ecc_cecnt_offset   = CV_SBECOUNT_OFST,
>>   	.ecc_uecnt_offset   = CV_DBECOUNT_OFST,
>>   	.ecc_irq_en_offset  = CV_DRAMINTR_OFST,
>> @@ -57,37 +58,62 @@ const struct altr_sdram_prv_data c5_data = {
>>   #endif
>>   };
>>
>> +const struct altr_sdram_prv_data a10_data = {\
>
> This should be static.
>
>> +	.ecc_ctrl_offset    = A10_ECCCTRL1_OFST,
>> +	.ecc_ctl_en_mask    = A10_ECCCTRL1_ECC_EN,
>> +	.ecc_stat_offset    = A10_INTSTAT_OFST,
>> +	.ecc_stat_ce_mask   = A10_INTSTAT_SBEERR,
>> +	.ecc_stat_ue_mask   = A10_INTSTAT_DBEERR,
>> +	.ecc_saddr_offset   = A10_SERRADDR_OFST,
>> +	.ecc_daddr_offset   = A10_DERRADDR_OFST,
>> +	.ecc_irq_en_offset  = A10_ERRINTEN_OFST,
>> +	.ecc_irq_en_mask    = A10_ECC_IRQ_EN_MASK,
>> +	.ecc_irq_clr_offset = A10_INTSTAT_OFST,
>> +	.ecc_irq_clr_mask   = (A10_INTSTAT_SBEERR | A10_INTSTAT_DBEERR),
>> +	.ecc_cnt_rst_offset = A10_ECCCTRL1_OFST,
>> +	.ecc_cnt_rst_mask   = A10_ECC_CNT_RESET_MASK,
>> +#ifdef CONFIG_EDAC_DEBUG
>> +	.ce_ue_trgr_offset  = A10_DIAGINTTEST_OFST,
>> +	.ce_set_mask        = A10_DIAGINT_TSERRA_MASK,
>> +	.ue_set_mask        = A10_DIAGINT_TDERRA_MASK,
>> +#endif
>> +};
>> +
>>
> <snip>
>
>> +
>>   static int altr_sdram_probe(struct platform_device *pdev)
>>   {
>>   	const struct of_device_id *id;
>> @@ -221,8 +295,8 @@ static int altr_sdram_probe(struct platform_device *pdev)
>>   	struct regmap *mc_vbase;
>>   	struct dimm_info *dimm;
>>   	u32 read_reg;
>> -	int irq, res = 0;
>> -	unsigned long mem_size;
>> +	int irq, irq2, res = 0;
>> +	unsigned long mem_size, irqflags;
>>
>>   	id = of_match_device(altr_sdram_ctrl_of_match, &pdev->dev);
>>   	if (!id)
>> @@ -288,6 +362,9 @@ static int altr_sdram_probe(struct platform_device *pdev)
>>   		return -ENODEV;
>>   	}
>>
>> +	/* Arria10 has a 2nd IRQ */
>> +	irq2 = platform_get_irq(pdev, 1);
>> +
>>   	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
>>   	layers[0].size = 1;
>>   	layers[0].is_virt_csrow = true;
>> @@ -332,8 +409,31 @@ static int altr_sdram_probe(struct platform_device *pdev)
>>   	if (res < 0)
>>   		goto err;
>>
>> +	/* Only the Arria10 has separate IRQs */
>> +	if (irq2 > 0) {
>> +		/* Arria10 specific initialization */
>> +		res = a10_init(mc_vbase);
>> +		if (res < 0)
>> +			goto err2;
>> +
>> +		res = a10_unmask_irq(pdev, A10_DDR0_IRQ_MASK);
>> +		if (res < 0)
>> +			goto err2;
>> +
>> +		res = devm_request_irq(&pdev->dev, irq2,
>> +				       altr_sdram_mc_err_handler,
>> +				       IRQF_SHARED, dev_name(&pdev->dev), mci);
>> +		if (res < 0) {
>> +			edac_mc_printk(mci, KERN_ERR,
>> +				       "Unable to request irq %d\n", irq2);
>> +			res = -ENODEV;
>> +			goto err2;
>> +		}
>> +		irqflags = IRQF_SHARED;
>> +	}
>> +
>>   	res = devm_request_irq(&pdev->dev, irq, altr_sdram_mc_err_handler,
>> -			       0, dev_name(&pdev->dev), mci);
>> +			       irqflags, dev_name(&pdev->dev), mci);
>
> irqflags was never set for the case of !(irq2 > 0).
>
> Dinh
>

Correct. So irqflags will be 0 for the CycloneV case.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] dts, altera: Arria10 SDRAM EDAC DTS additions.
  2015-05-13 21:49 ` [PATCH 4/4] dts, altera: Arria10 SDRAM EDAC DTS additions tthayer
@ 2015-05-15 10:55   ` Arnd Bergmann
  2015-05-15 21:01     ` Thor Thayer
  0 siblings, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2015-05-15 10:55 UTC (permalink / raw
  To: linux-arm-kernel
  Cc: tthayer, bp, dougthompson, m.chehab, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, linux, dinguyen,
	grant.likely, devicetree, linux-doc, linux-kernel, tthayer.linux,
	linux-edac

On Wednesday 13 May 2015 16:49:47 tthayer@opensource.altera.com wrote:
> +               sdr: sdr@ffc25000 {
> +                       compatible = "syscon";
> +                       reg = <0xffcfb100 0x80>;
> +               };
> +
> 

A syscon node with just 128 bytes seems very odd. Can you check the
hardware manual to see if this is part of some bigger unit?

	Arnd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] edac, altera: Addition of Arria10 EDAC
  2015-05-13 21:49 ` [PATCH 3/4] edac, altera: Addition of Arria10 EDAC tthayer
  2015-05-14 20:20   ` Dinh Nguyen
@ 2015-05-15 10:57   ` Arnd Bergmann
  1 sibling, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2015-05-15 10:57 UTC (permalink / raw
  To: linux-arm-kernel
  Cc: tthayer, bp, dougthompson, m.chehab, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, linux, dinguyen,
	grant.likely, devicetree, linux-doc, linux-kernel, tthayer.linux,
	linux-edac

On Wednesday 13 May 2015 16:49:46 tthayer@opensource.altera.com wrote:
> +static int a10_unmask_irq(struct platform_device *pdev, u32 mask)
> +{
> +       void __iomem  *sm_base;
> +
> +       if (!devm_request_mem_region(&pdev->dev, A10_SYMAN_INTMASK_CLR,
> +                                    sizeof(u32), dev_name(&pdev->dev))) {
> +               edac_printk(KERN_ERR, EDAC_MC,
> +                           "Unable to request mem region\n");
> +               return -EBUSY;
> +       }
> +
> +       sm_base = devm_ioremap(&pdev->dev, A10_SYMAN_INTMASK_CLR,
> +                              sizeof(u32));
> +       if (!sm_base) {
> +               edac_printk(KERN_ERR, EDAC_MC,
> +                           "Unable to ioremap device\n");
> +
> +               return -ENOMEM;
> +       }
> +
> +       iowrite32(mask, sm_base);
> +
> +       devm_iounmap(&pdev->dev, sm_base);
> +       devm_release_mem_region(&pdev->dev, A10_SYMAN_INTMASK_CLR,
> +                               sizeof(u32));
> +
> +       return 0;
> +}

If you always unmap right away, better use the normal request_mem_region
and ioremap functions rather than their devm counterparts.


>  
> +       /* Only the Arria10 has separate IRQs */
> +       if (irq2 > 0) {
> +               /* Arria10 specific initialization */
> +               res = a10_init(mc_vbase);
> +               if (res < 0)
> +                       goto err2;
> +
> +               res = a10_unmask_irq(pdev, A10_DDR0_IRQ_MASK);
> +               if (res < 0)
> +                       goto err2;
> +
> +               res = devm_request_irq(&pdev->dev, irq2,
> +                                      altr_sdram_mc_err_handler,
> +                                      IRQF_SHARED, dev_name(&pdev->dev), mci);
> +               if (res < 0) {
> +                       edac_mc_printk(mci, KERN_ERR,
> +                                      "Unable to request irq %d\n", irq2);
> +                       res = -ENODEV;
> +                       goto err2;
> +               }
> +               irqflags = IRQF_SHARED;
> +       }
> +
> 

Should the unmask be done after the request?

	Arnd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] edac, altera: Generalize driver to use DT Memory size
  2015-05-13 21:49 ` [PATCH 1/4] edac, altera: Generalize driver to use DT Memory size tthayer
  2015-05-14  3:25   ` Dinh Nguyen
@ 2015-05-15 11:00   ` Arnd Bergmann
  1 sibling, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2015-05-15 11:00 UTC (permalink / raw
  To: linux-arm-kernel
  Cc: tthayer, bp, dougthompson, m.chehab, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, linux, dinguyen,
	grant.likely, devicetree, linux-doc, linux-kernel, tthayer.linux,
	linux-edac

On Wednesday 13 May 2015 16:49:44 tthayer@opensource.altera.com wrote:
> -static u32 altr_sdram_get_total_mem_size(struct regmap *mc_vbase)
> +/* Get total memory size from Open Firmware DTB */
> +static unsigned long get_total_mem(void)
>  {
> -       u32 size, read_reg, row, bank, col, cs, width;
> +       struct device_node *np = NULL;
> +       const unsigned int *reg, *reg_end;
> +       int len, sw, aw;
> +       unsigned long start, size, total_mem;
>  
> -       if (regmap_read(mc_vbase, DRAMADDRW_OFST, &read_reg) < 0)
> +       np = of_find_node_by_type(NULL, "memory");
> +       if (!np)
>                 return 0;

There can be multiple memory nodes, I think you need to have a loop
using for_each_node_by_type.


	Arnd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] dts, altera: Arria10 SDRAM EDAC DTS additions.
  2015-05-15 10:55   ` Arnd Bergmann
@ 2015-05-15 21:01     ` Thor Thayer
  0 siblings, 0 replies; 13+ messages in thread
From: Thor Thayer @ 2015-05-15 21:01 UTC (permalink / raw
  To: Arnd Bergmann, linux-arm-kernel
  Cc: bp, dougthompson, m.chehab, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, linux, dinguyen, grant.likely, devicetree,
	linux-doc, linux-kernel, tthayer.linux, linux-edac

Hi Arnd,

On 05/15/2015 05:55 AM, Arnd Bergmann wrote:
> On Wednesday 13 May 2015 16:49:47 tthayer@opensource.altera.com wrote:
>> +               sdr: sdr@ffc25000 {
>> +                       compatible = "syscon";
>> +                       reg = <0xffcfb100 0x80>;
>> +               };
>> +
>>
>
> A syscon node with just 128 bytes seems very odd. Can you check the
> hardware manual to see if this is part of some bigger unit?
>
> 	Arnd
>

This is an unfortunate legacy of our previous SDRAM controller (in the 
CycloneV) which had ECC registers interspersed with registers other 
drivers needed - thus the use of syscon.

In the Arria10 chip, the ECC registers are in their own partitioned 
group but I kept the syscon to remain consistent with the Device Tree 
bindings from the CycloneV family.

I've implemented your other suggestions. Thank you for reviewing!

Thor

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-05-15 21:00 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-13 21:49 [PATCH 0/4] Add Altera Arria10 EDAC Support tthayer
2015-05-13 21:49 ` [PATCH 1/4] edac, altera: Generalize driver to use DT Memory size tthayer
2015-05-14  3:25   ` Dinh Nguyen
2015-05-15 11:00   ` Arnd Bergmann
2015-05-13 21:49 ` [PATCH 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC tthayer
2015-05-14 20:13   ` Dinh Nguyen
2015-05-13 21:49 ` [PATCH 3/4] edac, altera: Addition of Arria10 EDAC tthayer
2015-05-14 20:20   ` Dinh Nguyen
2015-05-14 20:38     ` Thor Thayer
2015-05-15 10:57   ` Arnd Bergmann
2015-05-13 21:49 ` [PATCH 4/4] dts, altera: Arria10 SDRAM EDAC DTS additions tthayer
2015-05-15 10:55   ` Arnd Bergmann
2015-05-15 21:01     ` Thor Thayer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).