Re: [PATCHv2 07/11] EDAC, altera: Add status offset & masks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Boris,

On 03/07/2016 01:43 PM, tthayer@xxxxxxxxxxxxxxxxxxxxx wrote:
From: Thor Thayer <tthayer@xxxxxxxxxxxxxxxxxxxxx>

In preparation for the Arria10 peripheral ECCs, the IRQ
status needs to be determined because the IRQs are shared.
The IRQ status register is read to determine if the IRQ
was for this ECC peripheral. Cyclone5 and Arria5 have
dedicated IRQs so the confirmation mechanism is not
required and the mask is set to 0.

Signed-off-by: Thor Thayer <tthayer@xxxxxxxxxxxxxxxxxxxxx>
---
v2: Split large patch into smaller patches. Determine
     if IRQ matches this ECC peripheral before handling it.
---
  drivers/edac/altera_edac.c |   41 +++++++++++++++++++++++++++++++----------
  drivers/edac/altera_edac.h |    3 +++
  2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index fd73a77..11b7291 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -556,19 +556,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, clear_addr);
-		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, clear_addr);
-		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);
  	}


While working on subsequent ECC components to upstream, I realized that the above is not an optimal solution for Arria10.

The Arria10 is significantly different from the Cyclone5/Arria5 and therefore should be it's own implementation.

Please disregard this patch series. I'll redo the series with a different IRQ implementation that is cleaner - it will be closer to the Xgene driver.

Sorry for the noise.

Thor


@@ -882,6 +895,10 @@ const struct edac_device_prv_data ocramecc_data = {
  	.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,
@@ -957,7 +974,11 @@ 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_L2_ECC_REG_OFFSET,
+	.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,
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index b262f74..43e0dae 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -226,6 +226,9 @@ struct edac_device_prv_data {
  	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);

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux