[PATCH 0/4] Add Altera Arria10 EDAC Support

[PATCH 0/4] Add Altera Arria10 EDAC Support

[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

[PATCH 1/4] edac, altera: Generalize driver to use DT Memory size

[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

Re: [PATCH 1/4] edac, altera: Generalize driver to use DT Memory size

[Dinh Nguyen]

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

Re: [PATCH 1/4] edac, altera: Generalize driver to use DT Memory size

[Arnd Bergmann]

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

[PATCH 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC.

[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

Re: [PATCH 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC.

[Dinh Nguyen]

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

[PATCH 3/4] edac, altera: Addition of Arria10 EDAC

[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

Re: [PATCH 3/4] edac, altera: Addition of Arria10 EDAC

[Dinh Nguyen]

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

Re: [PATCH 3/4] edac, altera: Addition of Arria10 EDAC

[Thor Thayer]

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.

Re: [PATCH 3/4] edac, altera: Addition of Arria10 EDAC

[Arnd Bergmann]

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

[PATCH 4/4] dts, altera: Arria10 SDRAM EDAC DTS additions.

[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

Re: [PATCH 4/4] dts, altera: Arria10 SDRAM EDAC DTS additions.

[Arnd Bergmann]

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

Re: [PATCH 4/4] dts, altera: Arria10 SDRAM EDAC DTS additions.

[Thor Thayer]

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