[PATCH 1/5] EDAC: Altera L2 Kconfig change from select to depends upon.

[PATCH 1/5] EDAC: Altera L2 Kconfig change from select to depends upon.

[tthayer]

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

Force L2 cache dependency instead of forcing selection of
L2 cache.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 drivers/edac/Kconfig |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 37755e6..6ca7474 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -378,12 +378,11 @@ config EDAC_ALTERA
 
 config EDAC_ALTERA_L2C
 	bool "Altera L2 Cache ECC"
-	depends on EDAC_ALTERA=y
-	select CACHE_L2X0
+	depends on EDAC_ALTERA=y && CACHE_L2X0
 	help
 	  Support for error detection and correction on the
 	  Altera L2 cache Memory for Altera SoCs. This option
-	  requires L2 cache so it will force that selection.
+	  requires L2 cache.
 
 config EDAC_ALTERA_OCRAM
 	bool "Altera On-Chip RAM ECC"
-- 
1.7.9.5

[PATCH 2/5] Documentation: dt: socfpga: Add Altera Arri10 L2 cache binding

[tthayer]

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

Add the device tree binding string needed to support the Altera L2
cache on the Arria10 chip.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 .../bindings/arm/altera/socfpga-eccmgr.txt         |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt
index 885f93d..4cea386 100644
--- a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt
+++ b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt
@@ -13,7 +13,8 @@ Subcomponents:
 
 L2 Cache ECC
 Required Properties:
-- compatible : Should be "altr,socfpga-l2-ecc"
+- compatible : Should be "altr,socfpga-l2-ecc" or
+	       "altr,socfpga-a10-l2-ecc"
 - reg : Address and size for ECC error interrupt clear registers.
 - interrupts : Should be single bit error interrupt, then double bit error
 	interrupt. Note the rising edge type.
-- 
1.7.9.5

Re: [PATCH 2/5] Documentation: dt: socfpga: Add Altera Arri10 L2 cache binding

[Rob Herring]

On Tue, Mar 01, 2016 at 10:38:18AM -0600, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> Add the device tree binding string needed to support the Altera L2
> cache on the Arria10 chip.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  .../bindings/arm/altera/socfpga-eccmgr.txt         |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 3/5] EDAC, altera: Addition of Arria10 L2 Cache ECC

[tthayer]

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

Addition of the Arria10 L2 Cache ECC handling. The major
changes affect the L2 ECC registers not being grouped
together. The Arria10 IRQ status needs to be mapped into
a different region. The mapping occurs in the L2 specific
function.
Important changes include:
1) Move private data structure definition to altera_edac.h
2) Move Cyclone5 device defines to altera_edac.h
3) Split IRQ status and ECC enable/control into separate
   memory areas.
4) Add IRQ status mapping in L2 ECC dependency checks
   function.
5) Addition of register offsets in private data structure.
6) Changes to code to use register offset define.
7) Addition of Arria10 L2 cache private data.
8) Add IRQ flags to indicate Exclusive/Shared.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 drivers/edac/altera_edac.c |  172 ++++++++++++++++++++++++++++----------------
 drivers/edac/altera_edac.h |   77 ++++++++++++++++++++
 2 files changed, 186 insertions(+), 63 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 63e4209..7fca74b 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -78,26 +78,6 @@ static const struct altr_sdram_prv_data a10_data = {
 	.ue_set_mask        = A10_DIAGINT_TDERRA_MASK,
 };
 
-/************************** EDAC Device Defines **************************/
-
-/* OCRAM ECC Management Group Defines */
-#define ALTR_MAN_GRP_OCRAM_ECC_OFFSET   0x04
-#define ALTR_OCR_ECC_EN                 BIT(0)
-#define ALTR_OCR_ECC_INJS               BIT(1)
-#define ALTR_OCR_ECC_INJD               BIT(2)
-#define ALTR_OCR_ECC_SERR               BIT(3)
-#define ALTR_OCR_ECC_DERR               BIT(4)
-
-/* L2 ECC Management Group Defines */
-#define ALTR_MAN_GRP_L2_ECC_OFFSET      0x00
-#define ALTR_L2_ECC_EN                  BIT(0)
-#define ALTR_L2_ECC_INJS                BIT(1)
-#define ALTR_L2_ECC_INJD                BIT(2)
-
-#define ALTR_UE_TRIGGER_CHAR            'U'   /* Trigger for UE */
-#define ALTR_TRIGGER_READ_WRD_CNT       32    /* Line size x 4 */
-#define ALTR_TRIG_OCRAM_BYTE_SIZE       128   /* Line size x 4 */
-#define ALTR_TRIG_L2C_BYTE_SIZE         4096  /* Full Page */
 
 /*********************** EDAC Memory Controller Functions ****************/
 
@@ -570,28 +550,7 @@ module_platform_driver(altr_edac_driver);
 
 const struct edac_device_prv_data ocramecc_data;
 const struct edac_device_prv_data l2ecc_data;
-
-struct edac_device_prv_data {
-	int (*setup)(struct platform_device *pdev, void __iomem *base);
-	int ce_clear_mask;
-	int ue_clear_mask;
-	char dbgfs_name[20];
-	void * (*alloc_mem)(size_t size, void **other);
-	void (*free_mem)(void *p, size_t size, void *other);
-	int ecc_enable_mask;
-	int ce_set_mask;
-	int ue_set_mask;
-	int trig_alloc_sz;
-};
-
-struct altr_edac_device_dev {
-	void __iomem *base;
-	int sb_irq;
-	int db_irq;
-	const struct edac_device_prv_data *data;
-	struct dentry *debugfs_dir;
-	char *edac_dev_name;
-};
+const struct edac_device_prv_data a10_l2ecc_data;
 
 static irqreturn_t altr_edac_device_handler(int irq, void *dev_id)
 {
@@ -599,18 +558,32 @@ static irqreturn_t altr_edac_device_handler(int irq, void *dev_id)
 	struct edac_device_ctl_info *dci = dev_id;
 	struct altr_edac_device_dev *drvdata = dci->pvt_info;
 	const struct edac_device_prv_data *priv = drvdata->data;
+	void __iomem *status_addr = drvdata->status + priv->err_status_ofst;
+	void __iomem *clear_addr = drvdata->status + priv->clear_err_ofst;
 
+	/*
+	 * CycloneV is directly mapped to a specific IRQ. Arria10
+	 * shares the IRQ with other ECCs so we must match first.
+	 */
 	if (irq == drvdata->sb_irq) {
-		if (priv->ce_clear_mask)
-			writel(priv->ce_clear_mask, drvdata->base);
-		edac_device_handle_ce(dci, 0, 0, drvdata->edac_dev_name);
-		ret_value = IRQ_HANDLED;
+		if (!priv->ce_status_mask ||
+		    (priv->ce_status_mask & readl(status_addr))) {
+			if (priv->ce_clear_mask)
+				writel(priv->ce_clear_mask, clear_addr);
+			edac_device_handle_ce(dci, 0, 0,
+					      drvdata->edac_dev_name);
+			ret_value = IRQ_HANDLED;
+		}
 	} else if (irq == drvdata->db_irq) {
-		if (priv->ue_clear_mask)
-			writel(priv->ue_clear_mask, drvdata->base);
-		edac_device_handle_ue(dci, 0, 0, drvdata->edac_dev_name);
-		panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
-		ret_value = IRQ_HANDLED;
+		if (!priv->ue_status_mask ||
+		    (priv->ue_status_mask & readl(status_addr))) {
+			if (priv->ue_clear_mask)
+				writel(priv->ue_clear_mask, clear_addr);
+			edac_device_handle_ue(dci, 0, 0,
+					      drvdata->edac_dev_name);
+			panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
+			ret_value = IRQ_HANDLED;
+		}
 	} else {
 		WARN_ON(1);
 	}
@@ -665,8 +638,9 @@ static ssize_t altr_edac_device_trig(struct file *file,
 		if (ACCESS_ONCE(ptemp[i]))
 			result = -1;
 		/* Toggle Error bit (it is latched), leave ECC enabled */
-		writel(error_mask, drvdata->base);
-		writel(priv->ecc_enable_mask, drvdata->base);
+		writel(error_mask, (drvdata->base + priv->set_err_ofst));
+		writel(priv->ecc_enable_mask, (drvdata->base +
+					       priv->set_err_ofst));
 		ptemp[i] = i;
 	}
 	/* Ensure it has been written out */
@@ -715,6 +689,8 @@ static void altr_create_edacdev_dbgfs(struct edac_device_ctl_info *edac_dci,
 static const struct of_device_id altr_edac_device_of_match[] = {
 #ifdef CONFIG_EDAC_ALTERA_L2C
 	{ .compatible = "altr,socfpga-l2-ecc", .data = (void *)&l2ecc_data },
+	{ .compatible = "altr,socfpga-a10-l2-ecc",
+	  .data = (void *)&a10_l2ecc_data },
 #endif
 #ifdef CONFIG_EDAC_ALTERA_OCRAM
 	{ .compatible = "altr,socfpga-ocram-ecc",
@@ -784,12 +760,15 @@ static int altr_edac_device_probe(struct platform_device *pdev)
 	if (!drvdata->base)
 		goto fail1;
 
+	/* Except for A10 L2 cache, status reg is within alloced base mem */
+	drvdata->status = drvdata->base;
+
 	/* Get driver specific data for this EDAC device */
 	drvdata->data = of_match_node(altr_edac_device_of_match, np)->data;
 
 	/* Check specific dependencies for the module */
 	if (drvdata->data->setup) {
-		res = drvdata->data->setup(pdev, drvdata->base);
+		res = drvdata->data->setup(pdev, drvdata);
 		if (res)
 			goto fail1;
 	}
@@ -797,14 +776,16 @@ static int altr_edac_device_probe(struct platform_device *pdev)
 	drvdata->sb_irq = platform_get_irq(pdev, 0);
 	res = devm_request_irq(&pdev->dev, drvdata->sb_irq,
 			       altr_edac_device_handler,
-			       0, dev_name(&pdev->dev), dci);
+			       drvdata->data->irq_flags,
+			       dev_name(&pdev->dev), dci);
 	if (res)
 		goto fail1;
 
 	drvdata->db_irq = platform_get_irq(pdev, 1);
 	res = devm_request_irq(&pdev->dev, drvdata->db_irq,
 			       altr_edac_device_handler,
-			       0, dev_name(&pdev->dev), dci);
+			       drvdata->data->irq_flags,
+			       dev_name(&pdev->dev), dci);
 	if (res)
 		goto fail1;
 
@@ -900,9 +881,12 @@ static void ocram_free_mem(void *p, size_t size, void *other)
  *	memory will cause CE/UE errors possibly causing an ABORT.
  */
 static int altr_ocram_check_deps(struct platform_device *pdev,
-				 void __iomem *base)
+				 struct altr_edac_device_dev *drvdata)
 {
-	if (readl(base) & ALTR_OCR_ECC_EN)
+	void __iomem  *base = drvdata->base;
+	const struct edac_device_prv_data *prv = drvdata->data;
+
+	if (readl(base + prv->ecc_en_ofst) & prv->ecc_enable_mask)
 		return 0;
 
 	edac_printk(KERN_ERR, EDAC_DEVICE,
@@ -914,13 +898,21 @@ const struct edac_device_prv_data ocramecc_data = {
 	.setup = altr_ocram_check_deps,
 	.ce_clear_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_SERR),
 	.ue_clear_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_DERR),
+	.clear_err_ofst = ALTR_OCR_ECC_REG_OFFSET,
+	/* Cyclone5 & Arria5 have separate IRQs so status = 0 */
+	.ce_status_mask = 0,
+	.ue_status_mask = 0,
+	.err_status_ofst = 0,
 	.dbgfs_name = "altr_ocram_trigger",
 	.alloc_mem = ocram_alloc_mem,
 	.free_mem = ocram_free_mem,
 	.ecc_enable_mask = ALTR_OCR_ECC_EN,
+	.ecc_en_ofst = ALTR_OCR_ECC_REG_OFFSET,
 	.ce_set_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_INJS),
 	.ue_set_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_INJD),
+	.set_err_ofst = ALTR_OCR_ECC_REG_OFFSET,
 	.trig_alloc_sz = ALTR_TRIG_OCRAM_BYTE_SIZE,
+	.irq_flags = 0,
 };
 
 #endif	/* CONFIG_EDAC_ALTERA_OCRAM */
@@ -967,27 +959,81 @@ static void l2_free_mem(void *p, size_t size, void *other)
  *	Note that L2 Cache Enable is forced at build time.
  */
 static int altr_l2_check_deps(struct platform_device *pdev,
-			      void __iomem *base)
+			      struct altr_edac_device_dev *drvdata)
 {
-	if (readl(base) & ALTR_L2_ECC_EN)
+	void __iomem  *status_base, *base = drvdata->base;
+	const struct edac_device_prv_data *prv = drvdata->data;
+
+	if ((readl(base + prv->ecc_en_ofst) & prv->ecc_enable_mask) !=
+	     prv->ecc_enable_mask) {
+		edac_printk(KERN_ERR, EDAC_DEVICE,
+			    "L2: No ECC present, or ECC disabled\n");
+		return -ENODEV;
+	}
+
+	if (!of_machine_is_compatible("altr,socfpga-arria10"))
 		return 0;
 
-	edac_printk(KERN_ERR, EDAC_DEVICE,
-		    "L2: No ECC present, or ECC disabled\n");
-	return -ENODEV;
+	/* A10 L2 cache status registers are not contiguous with base */
+	if (!devm_request_mem_region(&pdev->dev, ALTR_A10_L2_ECC_STATUS,
+				     2 * sizeof(u32), dev_name(&pdev->dev))) {
+		edac_printk(KERN_ERR, EDAC_DEVICE,
+			    "Unable to request mem region\n");
+		return -EBUSY;
+	}
+
+	status_base = devm_ioremap(&pdev->dev, ALTR_A10_L2_ECC_STATUS,
+				   2 * sizeof(u32));
+	if (!status_base) {
+		edac_printk(KERN_ERR, EDAC_DEVICE,
+			    "Unable to ioremap L2 status\n");
+		return -ENOMEM;
+	}
+
+	drvdata->status = status_base;
+
+	return 0;
 }
 
 const struct edac_device_prv_data l2ecc_data = {
 	.setup = altr_l2_check_deps,
 	.ce_clear_mask = 0,
 	.ue_clear_mask = 0,
+	.clear_err_ofst = ALTR_MAN_GRP_L2_ECC_OFFSET,
+	/* Cyclone5 & Arria5 have separate IRQs so status = 0 */
+	.ce_status_mask = 0,
+	.ue_status_mask = 0,
+	.err_status_ofst = 0,
 	.dbgfs_name = "altr_l2_trigger",
 	.alloc_mem = l2_alloc_mem,
 	.free_mem = l2_free_mem,
 	.ecc_enable_mask = ALTR_L2_ECC_EN,
+	.ecc_en_ofst = ALTR_MAN_GRP_L2_ECC_OFFSET,
 	.ce_set_mask = (ALTR_L2_ECC_EN | ALTR_L2_ECC_INJS),
 	.ue_set_mask = (ALTR_L2_ECC_EN | ALTR_L2_ECC_INJD),
+	.set_err_ofst = ALTR_MAN_GRP_L2_ECC_OFFSET,
+	.trig_alloc_sz = ALTR_TRIG_L2C_BYTE_SIZE,
+	.irq_flags = 0,
+};
+
+const struct edac_device_prv_data a10_l2ecc_data = {
+	.setup = altr_l2_check_deps,
+	.ce_clear_mask = ALTR_A10_L2_ECC_CE_CLR,
+	.ue_clear_mask = ALTR_A10_L2_ECC_UE_CLR,
+	.clear_err_ofst = ALTR_A10_L2_ECC_CLR_OFFSET,
+	.ce_status_mask = ALTR_A10_L2_ECC_CE_STAT,
+	.ue_status_mask = ALTR_A10_L2_ECC_UE_STAT,
+	.err_status_ofst = ALTR_A10_L2_ECC_STAT_OFFSET,
+	.dbgfs_name = "altr_l2_trigger",
+	.alloc_mem = l2_alloc_mem,
+	.free_mem = l2_free_mem,
+	.ecc_enable_mask = ALTR_A10_L2_ECC_EN_CTL,
+	.ecc_en_ofst = ALTR_A10_L2_ECC_CTL_OFFSET,
+	.ce_set_mask = ALTR_A10_L2_ECC_CE_INJ_MASK,
+	.ue_set_mask = ALTR_A10_L2_ECC_UE_INJ_MASK,
+	.set_err_ofst = ALTR_A10_L2_ECC_INJ_OFFSET,
 	.trig_alloc_sz = ALTR_TRIG_L2C_BYTE_SIZE,
+	.irq_flags = IRQF_SHARED,
 };
 
 #endif	/* CONFIG_EDAC_ALTERA_L2C */
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index 953077d..a028cf9 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -195,4 +195,81 @@ struct altr_sdram_mc_data {
 	const struct altr_sdram_prv_data *data;
 };
 
+/************************** EDAC Device Defines **************************/
+struct altr_edac_device_dev;
+
+struct edac_device_prv_data {
+	int (*setup)(struct platform_device *pdev,
+		     struct altr_edac_device_dev *drvdata);
+	int ce_clear_mask;
+	int ue_clear_mask;
+	int clear_err_ofst;
+	int ce_status_mask;
+	int ue_status_mask;
+	int err_status_ofst;
+	char dbgfs_name[20];
+	void * (*alloc_mem)(size_t size, void **other);
+	void (*free_mem)(void *p, size_t size, void *other);
+	int ecc_enable_mask;
+	int ecc_en_ofst;
+	int ce_set_mask;
+	int ue_set_mask;
+	int set_err_ofst;
+	int trig_alloc_sz;
+	int irq_flags;
+};
+
+struct altr_edac_device_dev {
+	void __iomem *base;
+	void __iomem *status;
+	int sb_irq;
+	int db_irq;
+	const struct edac_device_prv_data *data;
+	struct dentry *debugfs_dir;
+	char *edac_dev_name;
+};
+
+/***** General Device Trigger Defines *****/
+/* Trigger for UE */
+#define ALTR_UE_TRIGGER_CHAR            'U'
+/* Line size x 4 */
+#define ALTR_TRIGGER_READ_WRD_CNT       32
+#define ALTR_TRIG_OCRAM_BYTE_SIZE       128
+/* Full Page */
+#define ALTR_TRIG_L2C_BYTE_SIZE         4096
+
+/******* Cyclone5 and Arria5 Defines *******/
+/* OCRAM ECC Management Group Defines */
+#define ALTR_MAN_GRP_OCRAM_ECC_OFFSET   0x04
+#define ALTR_OCR_ECC_REG_OFFSET         0x00
+#define ALTR_OCR_ECC_EN                 BIT(0)
+#define ALTR_OCR_ECC_INJS               BIT(1)
+#define ALTR_OCR_ECC_INJD               BIT(2)
+#define ALTR_OCR_ECC_SERR               BIT(3)
+#define ALTR_OCR_ECC_DERR               BIT(4)
+
+/* L2 ECC Management Group Defines */
+#define ALTR_MAN_GRP_L2_ECC_OFFSET      0x00
+#define ALTR_L2_ECC_EN                  BIT(0)
+#define ALTR_L2_ECC_INJS                BIT(1)
+#define ALTR_L2_ECC_INJD                BIT(2)
+
+/************* Arria10 Defines *************/
+/* Arria 10 L2 ECC Management Group Defines */
+#define ALTR_A10_L2_ECC_CTL_OFFSET      0x0
+#define ALTR_A10_L2_ECC_EN_CTL          BIT(0)
+
+#define ALTR_A10_L2_ECC_STATUS          0xFFD060A4
+#define ALTR_A10_L2_ECC_STAT_OFFSET     0x0
+#define ALTR_A10_L2_ECC_CE_STAT         BIT(15)
+#define ALTR_A10_L2_ECC_UE_STAT         BIT(31)
+
+#define ALTR_A10_L2_ECC_CLR_OFFSET      0x4
+#define ALTR_A10_L2_ECC_CE_CLR          BIT(15)
+#define ALTR_A10_L2_ECC_UE_CLR          BIT(31)
+
+#define ALTR_A10_L2_ECC_INJ_OFFSET      ALTR_A10_L2_ECC_CTL_OFFSET
+#define ALTR_A10_L2_ECC_CE_INJ_MASK     0x00000101
+#define ALTR_A10_L2_ECC_UE_INJ_MASK     0x00010101
+
 #endif	/* #ifndef _ALTERA_EDAC_H */
-- 
1.7.9.5

Re: [PATCH 3/5] EDAC, altera: Addition of Arria10 L2 Cache ECC

[Borislav Petkov]

On Tue, Mar 01, 2016 at 10:38:19AM -0600, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> Addition of the Arria10 L2 Cache ECC handling. The major
> changes affect the L2 ECC registers not being grouped
> together. The Arria10 IRQ status needs to be mapped into
> a different region. The mapping occurs in the L2 specific
> function.
> Important changes include:

> 1) Move private data structure definition to altera_edac.h
> 2) Move Cyclone5 device defines to altera_edac.h

This should be a separate patch.

> 3) Split IRQ status and ECC enable/control into separate
>    memory areas.

Ditto.

> 4) Add IRQ status mapping in L2 ECC dependency checks
>    function.

Ditto...

> 5) Addition of register offsets in private data structure.
> 6) Changes to code to use register offset define.
> 7) Addition of Arria10 L2 cache private data.
> 8) Add IRQ flags to indicate Exclusive/Shared.

Do you see where I'm going with this?

Each patch should countain one logical change: add defines and move
struct, change functionality A, change functionality B, ...

The fact that you have to make a list of 8 important changes should
already give you a hint that it needs to be split.

As always, I'm going to need ACKs for the ARM stuff.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH 3/5] EDAC, altera: Addition of Arria10 L2 Cache ECC

[Thor Thayer]

Hi Boris,

On 03/04/2016 04:38 AM, Borislav Petkov wrote:
> On Tue, Mar 01, 2016 at 10:38:19AM -0600, tthayer@opensource.altera.com wrote:
>> From: Thor Thayer <tthayer@opensource.altera.com>
>>
>> Addition of the Arria10 L2 Cache ECC handling. The major
>> changes affect the L2 ECC registers not being grouped
>> together. The Arria10 IRQ status needs to be mapped into
>> a different region. The mapping occurs in the L2 specific
>> function.
>> Important changes include:
>
>> 1) Move private data structure definition to altera_edac.h
>> 2) Move Cyclone5 device defines to altera_edac.h
>
> This should be a separate patch.
>
>> 3) Split IRQ status and ECC enable/control into separate
>>     memory areas.
>
> Ditto.
>
>> 4) Add IRQ status mapping in L2 ECC dependency checks
>>     function.
>
> Ditto...
>
>> 5) Addition of register offsets in private data structure.
>> 6) Changes to code to use register offset define.
>> 7) Addition of Arria10 L2 cache private data.
>> 8) Add IRQ flags to indicate Exclusive/Shared.
>
> Do you see where I'm going with this?
>
> Each patch should countain one logical change: add defines and move
> struct, change functionality A, change functionality B, ...
>
> The fact that you have to make a list of 8 important changes should
> already give you a hint that it needs to be split.
>
> As always, I'm going to need ACKs for the ARM stuff.
>

OK. I'll split up the changes and resubmit. Thanks!

[PATCH 4/5] ARM: socfpga: Enable Arria10 L2 cache ECC on startup

[tthayer]

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

Enable ECC for Arria10 L2 cache on machine startup. The ECC has to be
enabled before data is stored in memory otherwise the ECC will fail
on reads.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 arch/arm/mach-socfpga/l2_cache.c |   42 ++++++++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-socfpga/l2_cache.c b/arch/arm/mach-socfpga/l2_cache.c
index e3907ab..b197218 100644
--- a/arch/arm/mach-socfpga/l2_cache.c
+++ b/arch/arm/mach-socfpga/l2_cache.c
@@ -17,14 +17,31 @@
 #include <linux/of_platform.h>
 #include <linux/of_address.h>
 
+#include "core.h"
+
+/* A10 System Manager ECC interrupt mask control registers */
+#define A10_L2_ECC_CTRL_OFST            0x0
+
+#define A10_SYSMGR_ECC_INTMASK_CLR_OFST 0x98
+#define A10_L2_ECC_INT_CLR_OFST         0xA8
+
+#define A10_MPU_CTRL_L2_ECC_EN          BIT(0)
+#define A10_ECC_INTMASK_CLR_EN          BIT(0)
+#define A10_ECC_INT_CLR                 (BIT(31) | BIT(15))
+
 void socfpga_init_l2_ecc(void)
 {
 	struct device_node *np;
 	void __iomem *mapped_l2_edac_addr;
+	const char *compat = "altr,socfpga-l2-ecc";
 
-	np = of_find_compatible_node(NULL, NULL, "altr,socfpga-l2-ecc");
+	if (of_machine_is_compatible("altr,socfpga-arria10"))
+		compat = "altr,socfpga-a10-l2-ecc";
+
+	/* Find the L2 EDAC device tree node */
+	np = of_find_compatible_node(NULL, NULL, compat);
 	if (!np) {
-		pr_err("Unable to find socfpga-l2-ecc in dtb\n");
+		pr_err("Unable to find %s in dtb\n", compat);
 		return;
 	}
 
@@ -35,7 +52,24 @@ void socfpga_init_l2_ecc(void)
 		return;
 	}
 
-	/* Enable ECC */
-	writel(0x01, mapped_l2_edac_addr);
+	if (of_machine_is_compatible("altr,socfpga-arria10")) {
+		if (!sys_manager_base_addr) {
+			pr_err("System Mananger not mapped for L2 ECC\n");
+			goto exit;
+		}
+		/* Clear any pending IRQs */
+		writel(A10_ECC_INT_CLR, (sys_manager_base_addr +
+					 A10_L2_ECC_INT_CLR_OFST));
+		/* Enable ECC */
+		writel(A10_ECC_INTMASK_CLR_EN, sys_manager_base_addr +
+		       A10_SYSMGR_ECC_INTMASK_CLR_OFST);
+		writel(A10_MPU_CTRL_L2_ECC_EN, mapped_l2_edac_addr +
+		       A10_L2_ECC_CTRL_OFST);
+	} else {
+		/* Enable ECC */
+		writel(0x01, mapped_l2_edac_addr);
+	}
+
+exit:
 	iounmap(mapped_l2_edac_addr);
 }
-- 
1.7.9.5

Re: [PATCH 4/5] ARM: socfpga: Enable Arria10 L2 cache ECC on startup

[Dinh Nguyen]

On Tue, 1 Mar 2016, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:

> From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> 
> Enable ECC for Arria10 L2 cache on machine startup. The ECC has to be
> enabled before data is stored in memory otherwise the ECC will fail
> on reads.
> 
> Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> ---
>  arch/arm/mach-socfpga/l2_cache.c |   42 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/l2_cache.c b/arch/arm/mach-socfpga/l2_cache.c
> index e3907ab..b197218 100644
> --- a/arch/arm/mach-socfpga/l2_cache.c
> +++ b/arch/arm/mach-socfpga/l2_cache.c
> @@ -17,14 +17,31 @@
>  #include <linux/of_platform.h>
>  #include <linux/of_address.h>
>  
> +#include "core.h"
> +
> +/* A10 System Manager ECC interrupt mask control registers */
> +#define A10_L2_ECC_CTRL_OFST            0x0
> +
> +#define A10_SYSMGR_ECC_INTMASK_CLR_OFST 0x98
> +#define A10_L2_ECC_INT_CLR_OFST         0xA8
> +
> +#define A10_MPU_CTRL_L2_ECC_EN          BIT(0)
> +#define A10_ECC_INTMASK_CLR_EN          BIT(0)
> +#define A10_ECC_INT_CLR                 (BIT(31) | BIT(15))
> +
>  void socfpga_init_l2_ecc(void)
>  {
>  	struct device_node *np;
>  	void __iomem *mapped_l2_edac_addr;
> +	const char *compat = "altr,socfpga-l2-ecc";
>  
> -	np = of_find_compatible_node(NULL, NULL, "altr,socfpga-l2-ecc");
> +	if (of_machine_is_compatible("altr,socfpga-arria10"))
> +		compat = "altr,socfpga-a10-l2-ecc";

The ARM maintainers have made comment to me about about trying to not sprinkle
these of_machine_is_compatible() all over the place. You should make the
decision during the initial probe of the machine. Please look at how the
.restart is differentiate between the 2 platforms.


> +
> +	/* Find the L2 EDAC device tree node */
> +	np = of_find_compatible_node(NULL, NULL, compat);
>  	if (!np) {
> -		pr_err("Unable to find socfpga-l2-ecc in dtb\n");
> +		pr_err("Unable to find %s in dtb\n", compat);
>  		return;
>  	}
>  
> @@ -35,7 +52,24 @@ void socfpga_init_l2_ecc(void)
>  		return;
>  	}
>  
> -	/* Enable ECC */
> -	writel(0x01, mapped_l2_edac_addr);
> +	if (of_machine_is_compatible("altr,socfpga-arria10")) {

Same comment as above here.

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] ARM: dts: Add Altera Arria10 L2 Cache EDAC devicetree entry

[tthayer]

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

Add the device tree entries needed to support the Altera L2
cache EDAC on the Arria10 chip.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 arch/arm/boot/dts/socfpga_arria10.dtsi |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi b/arch/arm/boot/dts/socfpga_arria10.dtsi
index cce9e50..e83e973 100644
--- a/arch/arm/boot/dts/socfpga_arria10.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
@@ -599,6 +599,20 @@
 			reg = <0xffe00000 0x40000>;
 		};
 
+		eccmgr: eccmgr@ffd06090 {
+			compatible = "altr,socfpga-ecc-manager";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			l2-ecc@ffd06000 {
+				compatible = "altr,socfpga-a10-l2-ecc";
+				reg = <0xffd06010 0x4>;
+				interrupts = <0 2 IRQ_TYPE_LEVEL_HIGH>,
+					     <0 0 IRQ_TYPE_LEVEL_HIGH>;
+			};
+		};
+
 		rst: rstmgr@ffd05000 {
 			#reset-cells = <1>;
 			compatible = "altr,rst-mgr";
-- 
1.7.9.5