[PATCH 0/5] Set Arria10 ECC Manager IRQ Controller

[PATCH 0/5] Set Arria10 ECC Manager IRQ Controller

[tthayer]

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

The Arria10 IRQs for each peripheral ECC block funnel into 2 IRQs
[1 for single bit errors (SBERR) and 1 for double bit errors (DBERR)]
which are better handled by the IRQ controller and IRQ domain
framework than the IRQ handler in the current implementation.

The IRQ numbers (hwirq) in each peripheral ECC block (currently L2
and OCRAM) device tree node cannot be parsed using of_ functions
in the current implementation because these functions attach the
IRQ to an IRQ domain.

This patch set adds the IRQ controller/IRQ domain framework but
requires some device tree and binding changes as a result.

Thor Thayer (5):
  Documentation: dt: socfpga: Add interrupt-controller to ecc-manager
  EDAC, altera: ECC Manager IRQ controller support
  EDAC, altera: Handle Arria10 SDRAM child node.
  ARM: dts: Arria10 ECC Manager IRQ controller changes
  ARM: dts: Move Arria10 SDRAM as child of ECC Manager

 .../bindings/arm/altera/socfpga-eccmgr.txt         |   14 +-
 arch/arm/boot/dts/socfpga_arria10.dtsi             |   19 +-
 drivers/edac/altera_edac.c                         |  182 +++++++++++++++-----
 drivers/edac/altera_edac.h                         |    5 +-
 4 files changed, 168 insertions(+), 52 deletions(-)

-- 
1.7.9.5

[PATCH 1/5] Documentation: dt: socfpga: Add interrupt-controller to ecc-manager

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

Designate the ECC Manager as an interrupt controller and add child
interrupts.

Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
---
 .../bindings/arm/altera/socfpga-eccmgr.txt         |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt
index 5a6b160..15eb0df 100644
--- a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt
+++ b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt
@@ -61,7 +61,9 @@ Required Properties:
 - #address-cells: must be 1
 - #size-cells: must be 1
 - interrupts : Should be single bit error interrupt, then double bit error
-	interrupt. Note the rising edge type.
+	interrupt.
+- interrupt-controller : boolean indicator that ECC Manager is an interrupt controller
+- #interrupt-cells : must be set to 2.
 - ranges : standard definition, should translate from local addresses
 
 Subcomponents:
@@ -70,11 +72,15 @@ L2 Cache ECC
 Required Properties:
 - compatible : Should be "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, in this order.
 
 On-Chip RAM ECC
 Required Properties:
 - compatible : Should be "altr,socfpga-a10-ocram-ecc"
 - reg        : Address and size for ECC block registers.
+- interrupts : Should be single bit error interrupt, then double bit error
+	interrupt, in this order.
 
 Example:
 
@@ -85,15 +91,21 @@ Example:
 		#size-cells = <1>;
 		interrupts = <0 2 IRQ_TYPE_LEVEL_HIGH>,
 			     <0 0 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
 		ranges;
 
 		l2-ecc@ffd06010 {
 			compatible = "altr,socfpga-a10-l2-ecc";
 			reg = <0xffd06010 0x4>;
+			interrupts = <0 IRQ_TYPE_LEVEL_HIGH>,
+				     <32 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
 		ocram-ecc@ff8c3000 {
 			compatible = "altr,socfpga-a10-ocram-ecc";
 			reg = <0xff8c3000 0x90>;
+			interrupts = <1 IRQ_TYPE_LEVEL_HIGH>,
+				     <33 IRQ_TYPE_LEVEL_HIGH> ;
 		};
 	};
-- 
1.7.9.5

--
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

Re: [PATCH 1/5] Documentation: dt: socfpga: Add interrupt-controller to ecc-manager

[Rob Herring]

On Wed, May 25, 2016 at 11:29:39AM -0500, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> Designate the ECC Manager as an interrupt controller and add child
> interrupts.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  .../bindings/arm/altera/socfpga-eccmgr.txt         |   14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)

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

[PATCH 3/5] EDAC, altera: Handle Arria10 SDRAM child node.

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

Separate the device match arrays for each platform to prevent
CycloneV matches when calling of_platform_populate() on the
Arria10 ECC manager node.
If the SDRAM is a child node of ECC manager, call probe function
via of_platform_populate().

Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
---
 drivers/edac/altera_edac.c |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 3eb73bc..dfd5d1b 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -688,11 +688,9 @@ 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 = &l2ecc_data },
-	{ .compatible = "altr,socfpga-a10-l2-ecc", .data = &a10_l2ecc_data },
 #endif
 #ifdef CONFIG_EDAC_ALTERA_OCRAM
 	{ .compatible = "altr,socfpga-ocram-ecc", .data = &ocramecc_data },
-	{ .compatible = "altr,socfpga-a10-ocram-ecc", .data = &a10_ocramecc_data },
 #endif
 	{},
 };
@@ -1054,6 +1052,17 @@ const struct edac_device_prv_data a10_l2ecc_data = {
 #endif	/* CONFIG_EDAC_ALTERA_L2C */
 
 /********************* Arria10 EDAC Device Functions *************************/
+static const struct of_device_id altr_edac_a10_device_of_match[] = {
+#ifdef CONFIG_EDAC_ALTERA_L2C
+	{ .compatible = "altr,socfpga-a10-l2-ecc", .data = &a10_l2ecc_data },
+#endif
+#ifdef CONFIG_EDAC_ALTERA_OCRAM
+	{ .compatible = "altr,socfpga-a10-ocram-ecc",
+	  .data = &a10_ocramecc_data },
+#endif
+	{},
+};
+MODULE_DEVICE_TABLE(of, altr_edac_a10_device_of_match);
 
 /*
  * The Arria10 EDAC Device Functions differ from the Cyclone5/Arria5
@@ -1123,7 +1132,7 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 	const struct edac_device_prv_data *prv;
 	/* Get matching node and check for valid result */
 	const struct of_device_id *pdev_id =
-		of_match_node(altr_edac_device_of_match, np);
+		of_match_node(altr_edac_a10_device_of_match, np);
 	if (IS_ERR_OR_NULL(pdev_id))
 		return -ENODEV;
 
@@ -1324,6 +1333,11 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
 		else if (of_device_is_compatible(child,
 						 "altr,socfpga-a10-ocram-ecc"))
 			altr_edac_a10_device_add(edac, child);
+		else if (of_device_is_compatible(child,
+						 "altr,sdram-edac-a10"))
+			of_platform_populate(pdev->dev.of_node,
+					     altr_sdram_ctrl_of_match,
+					     NULL, &pdev->dev);
 	}
 
 	return 0;
-- 
1.7.9.5

--
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 2/5] EDAC, altera: ECC Manager IRQ controller support

[tthayer]

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

To better support child devices, the ECC manager needs to be
implemented as an IRQ controller.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 drivers/edac/altera_edac.c |  162 +++++++++++++++++++++++++++++++++-----------
 drivers/edac/altera_edac.h |    5 +-
 2 files changed, 125 insertions(+), 42 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 5b4d223..3eb73bc 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -22,9 +22,11 @@
 #include <linux/edac.h>
 #include <linux/genalloc.h>
 #include <linux/interrupt.h>
+#include <linux/irqchip/chained_irq.h>
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/of_address.h>
+#include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -882,22 +884,27 @@ static void ocram_free_mem(void *p, size_t size, void *other)
 	gen_pool_free((struct gen_pool *)other, (u32)p, size);
 }
 
-static irqreturn_t altr_edac_a10_ecc_irq(struct altr_edac_device_dev *dci,
-					 bool sberr)
+static irqreturn_t altr_edac_a10_ecc_irq(int irq, void *dev_id)
 {
+	irqreturn_t ret_value = IRQ_NONE;
+	struct altr_edac_device_dev *dci = dev_id;
 	void __iomem  *base = dci->base;
 
-	if (sberr) {
+	if (irq == dci->sb_irq) {
+		ret_value = IRQ_HANDLED;
 		writel(ALTR_A10_ECC_SERRPENA,
 		       base + ALTR_A10_ECC_INTSTAT_OFST);
 		edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);
-	} else {
+	} else if (irq == dci->db_irq) {
+		ret_value = IRQ_HANDLED;
 		writel(ALTR_A10_ECC_DERRPENA,
 		       base + ALTR_A10_ECC_INTSTAT_OFST);
 		edac_device_handle_ue(dci->edac_dev, 0, 0, dci->edac_dev_name);
 		panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
+	} else {
+		WARN_ON(1);
 	}
-	return IRQ_HANDLED;
+	return ret_value;
 }
 
 const struct edac_device_prv_data ocramecc_data = {
@@ -988,22 +995,28 @@ static int altr_l2_check_deps(struct altr_edac_device_dev *device)
 	return -ENODEV;
 }
 
-static irqreturn_t altr_edac_a10_l2_irq(struct altr_edac_device_dev *dci,
-					bool sberr)
+static irqreturn_t altr_edac_a10_l2_irq(int irq, void *dev_id)
 {
-	if (sberr) {
+	irqreturn_t ret_value = IRQ_NONE;
+	struct altr_edac_device_dev *dci = dev_id;
+
+	if (irq == dci->sb_irq) {
+		ret_value = IRQ_HANDLED;
 		regmap_write(dci->edac->ecc_mgr_map,
 			     A10_SYSGMR_MPU_CLEAR_L2_ECC_OFST,
 			     A10_SYSGMR_MPU_CLEAR_L2_ECC_SB);
 		edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);
-	} else {
+	} else if (irq == dci->db_irq) {
+		ret_value = IRQ_HANDLED;
 		regmap_write(dci->edac->ecc_mgr_map,
 			     A10_SYSGMR_MPU_CLEAR_L2_ECC_OFST,
 			     A10_SYSGMR_MPU_CLEAR_L2_ECC_MB);
 		edac_device_handle_ue(dci->edac_dev, 0, 0, dci->edac_dev_name);
 		panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
+	} else {
+		WARN_ON(1);
 	}
-	return IRQ_HANDLED;
+	return ret_value;
 }
 
 const struct edac_device_prv_data l2ecc_data = {
@@ -1075,28 +1088,27 @@ static ssize_t altr_edac_a10_device_trig(struct file *file,
 	return count;
 }
 
-static irqreturn_t altr_edac_a10_irq_handler(int irq, void *dev_id)
+static void altr_edac_a10_irq_handler(struct irq_desc *desc)
 {
-	irqreturn_t rc = IRQ_NONE;
-	struct altr_arria10_edac *edac = dev_id;
-	struct altr_edac_device_dev *dci;
-	int irq_status;
-	bool sberr = (irq == edac->sb_irq) ? 1 : 0;
-	int sm_offset = sberr ? A10_SYSMGR_ECC_INTSTAT_SERR_OFST :
-				A10_SYSMGR_ECC_INTSTAT_DERR_OFST;
+	int dberr, bit, sm_offset, irq_status;
+	struct altr_arria10_edac *edac = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	int irq = irq_desc_get_irq(desc);
+
+	chained_irq_enter(chip, desc);
+	dberr = (irq == edac->db_irq) ? 1 : 0;
+	sm_offset = dberr ? A10_SYSMGR_ECC_INTSTAT_DERR_OFST :
+			    A10_SYSMGR_ECC_INTSTAT_SERR_OFST;
 
 	regmap_read(edac->ecc_mgr_map, sm_offset, &irq_status);
 
-	if ((irq != edac->sb_irq) && (irq != edac->db_irq)) {
-		WARN_ON(1);
-	} else {
-		list_for_each_entry(dci, &edac->a10_ecc_devices, next) {
-			if (irq_status & dci->data->irq_status_mask)
-				rc = dci->data->ecc_irq_handler(dci, sberr);
-		}
+	for_each_set_bit(bit, (unsigned long *)&irq_status, 32) {
+		irq = irq_linear_revmap(edac->domain, dberr * 32 + bit);
+		if (irq)
+			generic_handle_irq(irq);
 	}
 
-	return rc;
+	chained_irq_exit(chip, desc);
 }
 
 static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
@@ -1168,6 +1180,34 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 			goto err_release_group1;
 	}
 
+	altdev->sb_irq = irq_of_parse_and_map(np, 0);
+	if (!altdev->sb_irq) {
+		edac_printk(KERN_ERR, EDAC_DEVICE, "Error allocating SBIRQ\n");
+		rc = -ENODEV;
+		goto err_release_group1;
+	}
+	rc = devm_request_irq(edac->dev, altdev->sb_irq,
+			      prv->ecc_irq_handler,
+			      IRQF_SHARED, ecc_name, altdev);
+	if (rc) {
+		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
+		goto err_release_group1;
+	}
+
+	altdev->db_irq = irq_of_parse_and_map(np, 1);
+	if (!altdev->db_irq) {
+		edac_printk(KERN_ERR, EDAC_DEVICE, "Error allocating DBIRQ\n");
+		rc = -ENODEV;
+		goto err_release_group1;
+	}
+	rc = devm_request_irq(edac->dev, altdev->db_irq,
+			      prv->ecc_irq_handler,
+			      IRQF_SHARED, ecc_name, altdev);
+	if (rc) {
+		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
+		goto err_release_group1;
+	}
+
 	rc = edac_device_add_device(dci);
 	if (rc) {
 		dev_err(edac->dev, "edac_device_add_device failed\n");
@@ -1186,7 +1226,6 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 err_release_group1:
 	edac_device_free_ctl_info(dci);
 err_release_group:
-	edac_printk(KERN_ALERT, EDAC_DEVICE, "%s: %d\n", __func__, __LINE__);
 	devres_release_group(edac->dev, NULL);
 	edac_printk(KERN_ERR, EDAC_DEVICE,
 		    "%s:Error setting up EDAC device: %d\n", ecc_name, rc);
@@ -1194,11 +1233,43 @@ err_release_group:
 	return rc;
 }
 
+static void a10_eccmgr_irq_mask(struct irq_data *d)
+{
+	struct altr_arria10_edac *edac = irq_data_get_irq_chip_data(d);
+
+	regmap_write(edac->ecc_mgr_map,	A10_SYSMGR_ECC_INTMASK_SET_OFST,
+		     BIT(d->hwirq));
+}
+
+static void a10_eccmgr_irq_unmask(struct irq_data *d)
+{
+	struct altr_arria10_edac *edac = irq_data_get_irq_chip_data(d);
+
+	regmap_write(edac->ecc_mgr_map,	A10_SYSMGR_ECC_INTMASK_CLR_OFST,
+		     BIT(d->hwirq));
+}
+
+static int a10_eccmgr_irqdomain_map(struct irq_domain *d, unsigned int irq,
+				    irq_hw_number_t hwirq)
+{
+	struct altr_arria10_edac *edac = d->host_data;
+
+	irq_set_chip_and_handler(irq, &edac->irq_chip, handle_simple_irq);
+	irq_set_chip_data(irq, edac);
+	irq_set_noprobe(irq);
+
+	return 0;
+}
+
+struct irq_domain_ops a10_eccmgr_ic_ops = {
+	.map = a10_eccmgr_irqdomain_map,
+	.xlate = irq_domain_xlate_twocell,
+};
+
 static int altr_edac_a10_probe(struct platform_device *pdev)
 {
 	struct altr_arria10_edac *edac;
 	struct device_node *child;
-	int rc;
 
 	edac = devm_kzalloc(&pdev->dev, sizeof(*edac), GFP_KERNEL);
 	if (!edac)
@@ -1216,23 +1287,34 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
 		return PTR_ERR(edac->ecc_mgr_map);
 	}
 
+	edac->irq_chip.name = pdev->dev.of_node->name;
+	edac->irq_chip.irq_mask = a10_eccmgr_irq_mask;
+	edac->irq_chip.irq_unmask = a10_eccmgr_irq_unmask;
+	edac->domain = irq_domain_add_linear(pdev->dev.of_node, 64,
+					     &a10_eccmgr_ic_ops, edac);
+	if (!edac->domain) {
+		dev_err(&pdev->dev, "Error adding IRQ domain\n");
+		return -ENOMEM;
+	}
+
 	edac->sb_irq = platform_get_irq(pdev, 0);
-	rc = devm_request_irq(&pdev->dev, edac->sb_irq,
-			      altr_edac_a10_irq_handler,
-			      IRQF_SHARED, dev_name(&pdev->dev), edac);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "No SBERR IRQ resource\n");
-		return rc;
+	if (edac->sb_irq < 0) {
+		dev_err(&pdev->dev, "No SBERR IRQ resource\n");
+		return edac->sb_irq;
 	}
 
+	irq_set_chained_handler_and_data(edac->sb_irq,
+					 altr_edac_a10_irq_handler,
+					 edac);
+
 	edac->db_irq = platform_get_irq(pdev, 1);
-	rc = devm_request_irq(&pdev->dev, edac->db_irq,
-			      altr_edac_a10_irq_handler,
-			      IRQF_SHARED, dev_name(&pdev->dev), edac);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
-		return rc;
+	if (edac->db_irq < 0) {
+		dev_err(&pdev->dev, "No DBERR IRQ resource\n");
+		return edac->db_irq;
 	}
+	irq_set_chained_handler_and_data(edac->db_irq,
+					 altr_edac_a10_irq_handler,
+					 edac);
 
 	for_each_child_of_node(pdev->dev.of_node, child) {
 		if (!of_device_is_available(child))
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index 42090f3..62b0fa0 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -295,8 +295,7 @@ struct edac_device_prv_data {
 	int ce_set_mask;
 	int ue_set_mask;
 	int set_err_ofst;
-	irqreturn_t (*ecc_irq_handler)(struct altr_edac_device_dev *dci,
-				       bool sb);
+	irqreturn_t (*ecc_irq_handler)(int irq, void *dev_id);
 	int trig_alloc_sz;
 	const struct file_operations *inject_fops;
 };
@@ -320,6 +319,8 @@ struct altr_arria10_edac {
 	struct regmap		*ecc_mgr_map;
 	int sb_irq;
 	int db_irq;
+	struct irq_domain	*domain;
+	struct irq_chip		irq_chip;
 	struct list_head	a10_ecc_devices;
 };
 
-- 
1.7.9.5

Re: [PATCH 2/5] EDAC, altera: ECC Manager IRQ controller support

[Borislav Petkov]

On Wed, May 25, 2016 at 11:29:40AM -0500, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
> From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> 
> To better support child devices, the ECC manager needs to be
> implemented as an IRQ controller.
> 
> Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> ---
>  drivers/edac/altera_edac.c |  162 +++++++++++++++++++++++++++++++++-----------
>  drivers/edac/altera_edac.h |    5 +-
>  2 files changed, 125 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index 5b4d223..3eb73bc 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c
> @@ -22,9 +22,11 @@
>  #include <linux/edac.h>
>  #include <linux/genalloc.h>
>  #include <linux/interrupt.h>
> +#include <linux/irqchip/chained_irq.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/of_address.h>
> +#include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> @@ -882,22 +884,27 @@ static void ocram_free_mem(void *p, size_t size, void *other)
>  	gen_pool_free((struct gen_pool *)other, (u32)p, size);
>  }
>  
> -static irqreturn_t altr_edac_a10_ecc_irq(struct altr_edac_device_dev *dci,
> -					 bool sberr)
> +static irqreturn_t altr_edac_a10_ecc_irq(int irq, void *dev_id)
>  {
> +	irqreturn_t ret_value = IRQ_NONE;

This ret_value looks funny:

> +	struct altr_edac_device_dev *dci = dev_id;
>  	void __iomem  *base = dci->base;
>  
> -	if (sberr) {
> +	if (irq == dci->sb_irq) {
> +		ret_value = IRQ_HANDLED;
>  		writel(ALTR_A10_ECC_SERRPENA,
>  		       base + ALTR_A10_ECC_INTSTAT_OFST);
>  		edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);

You can do
		return IRQ_HANDLED;

here

> -	} else {
> +	} else if (irq == dci->db_irq) {
> +		ret_value = IRQ_HANDLED;
>  		writel(ALTR_A10_ECC_DERRPENA,
>  		       base + ALTR_A10_ECC_INTSTAT_OFST);
>  		edac_device_handle_ue(dci->edac_dev, 0, 0, dci->edac_dev_name);
>  		panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");

No need to return any value after panic so who cares. But you can still do

		return IRQ_HANDLED;

for consistency.

> +	} else {
> +		WARN_ON(1);
>  	}
> -	return IRQ_HANDLED;
> +	return ret_value;

	return IRQ_NONE;



>  }
>  
>  const struct edac_device_prv_data ocramecc_data = {
> @@ -988,22 +995,28 @@ static int altr_l2_check_deps(struct altr_edac_device_dev *device)
>  	return -ENODEV;
>  }
>  
> -static irqreturn_t altr_edac_a10_l2_irq(struct altr_edac_device_dev *dci,
> -					bool sberr)
> +static irqreturn_t altr_edac_a10_l2_irq(int irq, void *dev_id)
>  {
> -	if (sberr) {
> +	irqreturn_t ret_value = IRQ_NONE;
> +	struct altr_edac_device_dev *dci = dev_id;
> +
> +	if (irq == dci->sb_irq) {
> +		ret_value = IRQ_HANDLED;
>  		regmap_write(dci->edac->ecc_mgr_map,
>  			     A10_SYSGMR_MPU_CLEAR_L2_ECC_OFST,
>  			     A10_SYSGMR_MPU_CLEAR_L2_ECC_SB);
>  		edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);
> -	} else {
> +	} else if (irq == dci->db_irq) {
> +		ret_value = IRQ_HANDLED;
>  		regmap_write(dci->edac->ecc_mgr_map,
>  			     A10_SYSGMR_MPU_CLEAR_L2_ECC_OFST,
>  			     A10_SYSGMR_MPU_CLEAR_L2_ECC_MB);
>  		edac_device_handle_ue(dci->edac_dev, 0, 0, dci->edac_dev_name);
>  		panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
> +	} else {
> +		WARN_ON(1);
>  	}
> -	return IRQ_HANDLED;
> +	return ret_value;

Ditto.

>  }
>  
>  const struct edac_device_prv_data l2ecc_data = {
> @@ -1075,28 +1088,27 @@ static ssize_t altr_edac_a10_device_trig(struct file *file,
>  	return count;
>  }
>  
> -static irqreturn_t altr_edac_a10_irq_handler(int irq, void *dev_id)
> +static void altr_edac_a10_irq_handler(struct irq_desc *desc)
>  {
> -	irqreturn_t rc = IRQ_NONE;
> -	struct altr_arria10_edac *edac = dev_id;
> -	struct altr_edac_device_dev *dci;
> -	int irq_status;
> -	bool sberr = (irq == edac->sb_irq) ? 1 : 0;
> -	int sm_offset = sberr ? A10_SYSMGR_ECC_INTSTAT_SERR_OFST :
> -				A10_SYSMGR_ECC_INTSTAT_DERR_OFST;
> +	int dberr, bit, sm_offset, irq_status;
> +	struct altr_arria10_edac *edac = irq_desc_get_handler_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	int irq = irq_desc_get_irq(desc);
> +
> +	chained_irq_enter(chip, desc);
> +	dberr = (irq == edac->db_irq) ? 1 : 0;
> +	sm_offset = dberr ? A10_SYSMGR_ECC_INTSTAT_DERR_OFST :
> +			    A10_SYSMGR_ECC_INTSTAT_SERR_OFST;

Move
	chained_irq_enter()

here, after the assignments.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
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

[PATCHv2 2/5] EDAC, altera: ECC Manager IRQ controller support

[tthayer]

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

To better support child devices, the ECC manager needs to be
implemented as an IRQ controller.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
v2  Update with cleanup/improvements from maintainer.
---
 drivers/edac/altera_edac.c |  161 +++++++++++++++++++++++++++++++++-----------
 drivers/edac/altera_edac.h |    5 +-
 2 files changed, 124 insertions(+), 42 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 5b4d223..573c68d 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -22,9 +22,11 @@
 #include <linux/edac.h>
 #include <linux/genalloc.h>
 #include <linux/interrupt.h>
+#include <linux/irqchip/chained_irq.h>
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/of_address.h>
+#include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -882,22 +884,26 @@ static void ocram_free_mem(void *p, size_t size, void *other)
 	gen_pool_free((struct gen_pool *)other, (u32)p, size);
 }
 
-static irqreturn_t altr_edac_a10_ecc_irq(struct altr_edac_device_dev *dci,
-					 bool sberr)
+static irqreturn_t altr_edac_a10_ecc_irq(int irq, void *dev_id)
 {
+	struct altr_edac_device_dev *dci = dev_id;
 	void __iomem  *base = dci->base;
 
-	if (sberr) {
+	if (irq == dci->sb_irq) {
 		writel(ALTR_A10_ECC_SERRPENA,
 		       base + ALTR_A10_ECC_INTSTAT_OFST);
 		edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);
-	} else {
+		return IRQ_HANDLED;
+	} else if (irq == dci->db_irq) {
 		writel(ALTR_A10_ECC_DERRPENA,
 		       base + ALTR_A10_ECC_INTSTAT_OFST);
 		edac_device_handle_ue(dci->edac_dev, 0, 0, dci->edac_dev_name);
 		panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
+		return IRQ_HANDLED;
+	} else {
+		WARN_ON(1);
 	}
-	return IRQ_HANDLED;
+	return IRQ_NONE;
 }
 
 const struct edac_device_prv_data ocramecc_data = {
@@ -988,22 +994,27 @@ static int altr_l2_check_deps(struct altr_edac_device_dev *device)
 	return -ENODEV;
 }
 
-static irqreturn_t altr_edac_a10_l2_irq(struct altr_edac_device_dev *dci,
-					bool sberr)
+static irqreturn_t altr_edac_a10_l2_irq(int irq, void *dev_id)
 {
-	if (sberr) {
+	struct altr_edac_device_dev *dci = dev_id;
+
+	if (irq == dci->sb_irq) {
 		regmap_write(dci->edac->ecc_mgr_map,
 			     A10_SYSGMR_MPU_CLEAR_L2_ECC_OFST,
 			     A10_SYSGMR_MPU_CLEAR_L2_ECC_SB);
 		edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);
-	} else {
+		return IRQ_HANDLED;
+	} else if (irq == dci->db_irq) {
 		regmap_write(dci->edac->ecc_mgr_map,
 			     A10_SYSGMR_MPU_CLEAR_L2_ECC_OFST,
 			     A10_SYSGMR_MPU_CLEAR_L2_ECC_MB);
 		edac_device_handle_ue(dci->edac_dev, 0, 0, dci->edac_dev_name);
 		panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
+		return IRQ_HANDLED;
+	} else {
+		WARN_ON(1);
 	}
-	return IRQ_HANDLED;
+	return IRQ_NONE;
 }
 
 const struct edac_device_prv_data l2ecc_data = {
@@ -1075,28 +1086,28 @@ static ssize_t altr_edac_a10_device_trig(struct file *file,
 	return count;
 }
 
-static irqreturn_t altr_edac_a10_irq_handler(int irq, void *dev_id)
+static void altr_edac_a10_irq_handler(struct irq_desc *desc)
 {
-	irqreturn_t rc = IRQ_NONE;
-	struct altr_arria10_edac *edac = dev_id;
-	struct altr_edac_device_dev *dci;
-	int irq_status;
-	bool sberr = (irq == edac->sb_irq) ? 1 : 0;
-	int sm_offset = sberr ? A10_SYSMGR_ECC_INTSTAT_SERR_OFST :
-				A10_SYSMGR_ECC_INTSTAT_DERR_OFST;
+	int dberr, bit, sm_offset, irq_status;
+	struct altr_arria10_edac *edac = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	int irq = irq_desc_get_irq(desc);
+
+	dberr = (irq == edac->db_irq) ? 1 : 0;
+	sm_offset = dberr ? A10_SYSMGR_ECC_INTSTAT_DERR_OFST :
+			    A10_SYSMGR_ECC_INTSTAT_SERR_OFST;
+
+	chained_irq_enter(chip, desc);
 
 	regmap_read(edac->ecc_mgr_map, sm_offset, &irq_status);
 
-	if ((irq != edac->sb_irq) && (irq != edac->db_irq)) {
-		WARN_ON(1);
-	} else {
-		list_for_each_entry(dci, &edac->a10_ecc_devices, next) {
-			if (irq_status & dci->data->irq_status_mask)
-				rc = dci->data->ecc_irq_handler(dci, sberr);
-		}
+	for_each_set_bit(bit, (unsigned long *)&irq_status, 32) {
+		irq = irq_linear_revmap(edac->domain, dberr * 32 + bit);
+		if (irq)
+			generic_handle_irq(irq);
 	}
 
-	return rc;
+	chained_irq_exit(chip, desc);
 }
 
 static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
@@ -1168,6 +1179,34 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 			goto err_release_group1;
 	}
 
+	altdev->sb_irq = irq_of_parse_and_map(np, 0);
+	if (!altdev->sb_irq) {
+		edac_printk(KERN_ERR, EDAC_DEVICE, "Error allocating SBIRQ\n");
+		rc = -ENODEV;
+		goto err_release_group1;
+	}
+	rc = devm_request_irq(edac->dev, altdev->sb_irq,
+			      prv->ecc_irq_handler,
+			      IRQF_SHARED, ecc_name, altdev);
+	if (rc) {
+		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
+		goto err_release_group1;
+	}
+
+	altdev->db_irq = irq_of_parse_and_map(np, 1);
+	if (!altdev->db_irq) {
+		edac_printk(KERN_ERR, EDAC_DEVICE, "Error allocating DBIRQ\n");
+		rc = -ENODEV;
+		goto err_release_group1;
+	}
+	rc = devm_request_irq(edac->dev, altdev->db_irq,
+			      prv->ecc_irq_handler,
+			      IRQF_SHARED, ecc_name, altdev);
+	if (rc) {
+		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
+		goto err_release_group1;
+	}
+
 	rc = edac_device_add_device(dci);
 	if (rc) {
 		dev_err(edac->dev, "edac_device_add_device failed\n");
@@ -1186,7 +1225,6 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 err_release_group1:
 	edac_device_free_ctl_info(dci);
 err_release_group:
-	edac_printk(KERN_ALERT, EDAC_DEVICE, "%s: %d\n", __func__, __LINE__);
 	devres_release_group(edac->dev, NULL);
 	edac_printk(KERN_ERR, EDAC_DEVICE,
 		    "%s:Error setting up EDAC device: %d\n", ecc_name, rc);
@@ -1194,11 +1232,43 @@ err_release_group:
 	return rc;
 }
 
+static void a10_eccmgr_irq_mask(struct irq_data *d)
+{
+	struct altr_arria10_edac *edac = irq_data_get_irq_chip_data(d);
+
+	regmap_write(edac->ecc_mgr_map,	A10_SYSMGR_ECC_INTMASK_SET_OFST,
+		     BIT(d->hwirq));
+}
+
+static void a10_eccmgr_irq_unmask(struct irq_data *d)
+{
+	struct altr_arria10_edac *edac = irq_data_get_irq_chip_data(d);
+
+	regmap_write(edac->ecc_mgr_map,	A10_SYSMGR_ECC_INTMASK_CLR_OFST,
+		     BIT(d->hwirq));
+}
+
+static int a10_eccmgr_irqdomain_map(struct irq_domain *d, unsigned int irq,
+				    irq_hw_number_t hwirq)
+{
+	struct altr_arria10_edac *edac = d->host_data;
+
+	irq_set_chip_and_handler(irq, &edac->irq_chip, handle_simple_irq);
+	irq_set_chip_data(irq, edac);
+	irq_set_noprobe(irq);
+
+	return 0;
+}
+
+struct irq_domain_ops a10_eccmgr_ic_ops = {
+	.map = a10_eccmgr_irqdomain_map,
+	.xlate = irq_domain_xlate_twocell,
+};
+
 static int altr_edac_a10_probe(struct platform_device *pdev)
 {
 	struct altr_arria10_edac *edac;
 	struct device_node *child;
-	int rc;
 
 	edac = devm_kzalloc(&pdev->dev, sizeof(*edac), GFP_KERNEL);
 	if (!edac)
@@ -1216,23 +1286,34 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
 		return PTR_ERR(edac->ecc_mgr_map);
 	}
 
+	edac->irq_chip.name = pdev->dev.of_node->name;
+	edac->irq_chip.irq_mask = a10_eccmgr_irq_mask;
+	edac->irq_chip.irq_unmask = a10_eccmgr_irq_unmask;
+	edac->domain = irq_domain_add_linear(pdev->dev.of_node, 64,
+					     &a10_eccmgr_ic_ops, edac);
+	if (!edac->domain) {
+		dev_err(&pdev->dev, "Error adding IRQ domain\n");
+		return -ENOMEM;
+	}
+
 	edac->sb_irq = platform_get_irq(pdev, 0);
-	rc = devm_request_irq(&pdev->dev, edac->sb_irq,
-			      altr_edac_a10_irq_handler,
-			      IRQF_SHARED, dev_name(&pdev->dev), edac);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "No SBERR IRQ resource\n");
-		return rc;
+	if (edac->sb_irq < 0) {
+		dev_err(&pdev->dev, "No SBERR IRQ resource\n");
+		return edac->sb_irq;
 	}
 
+	irq_set_chained_handler_and_data(edac->sb_irq,
+					 altr_edac_a10_irq_handler,
+					 edac);
+
 	edac->db_irq = platform_get_irq(pdev, 1);
-	rc = devm_request_irq(&pdev->dev, edac->db_irq,
-			      altr_edac_a10_irq_handler,
-			      IRQF_SHARED, dev_name(&pdev->dev), edac);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
-		return rc;
+	if (edac->db_irq < 0) {
+		dev_err(&pdev->dev, "No DBERR IRQ resource\n");
+		return edac->db_irq;
 	}
+	irq_set_chained_handler_and_data(edac->db_irq,
+					 altr_edac_a10_irq_handler,
+					 edac);
 
 	for_each_child_of_node(pdev->dev.of_node, child) {
 		if (!of_device_is_available(child))
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index 42090f3..62b0fa0 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -295,8 +295,7 @@ struct edac_device_prv_data {
 	int ce_set_mask;
 	int ue_set_mask;
 	int set_err_ofst;
-	irqreturn_t (*ecc_irq_handler)(struct altr_edac_device_dev *dci,
-				       bool sb);
+	irqreturn_t (*ecc_irq_handler)(int irq, void *dev_id);
 	int trig_alloc_sz;
 	const struct file_operations *inject_fops;
 };
@@ -320,6 +319,8 @@ struct altr_arria10_edac {
 	struct regmap		*ecc_mgr_map;
 	int sb_irq;
 	int db_irq;
+	struct irq_domain	*domain;
+	struct irq_chip		irq_chip;
 	struct list_head	a10_ecc_devices;
 };
 
-- 
1.7.9.5

Re: [PATCHv2 2/5] EDAC, altera: ECC Manager IRQ controller support

[Borislav Petkov]

On Tue, Jun 07, 2016 at 03:35:57PM -0500, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> To better support child devices, the ECC manager needs to be
> implemented as an IRQ controller.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
> v2  Update with cleanup/improvements from maintainer.
> ---
>  drivers/edac/altera_edac.c |  161 +++++++++++++++++++++++++++++++++-----------
>  drivers/edac/altera_edac.h |    5 +-
>  2 files changed, 124 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index 5b4d223..573c68d 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c
> @@ -22,9 +22,11 @@
>  #include <linux/edac.h>
>  #include <linux/genalloc.h>
>  #include <linux/interrupt.h>
> +#include <linux/irqchip/chained_irq.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/of_address.h>
> +#include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> @@ -882,22 +884,26 @@ static void ocram_free_mem(void *p, size_t size, void *other)
>  	gen_pool_free((struct gen_pool *)other, (u32)p, size);
>  }
>  
> -static irqreturn_t altr_edac_a10_ecc_irq(struct altr_edac_device_dev *dci,
> -					 bool sberr)
> +static irqreturn_t altr_edac_a10_ecc_irq(int irq, void *dev_id)
>  {
> +	struct altr_edac_device_dev *dci = dev_id;
>  	void __iomem  *base = dci->base;
>  
> -	if (sberr) {
> +	if (irq == dci->sb_irq) {
>  		writel(ALTR_A10_ECC_SERRPENA,
>  		       base + ALTR_A10_ECC_INTSTAT_OFST);
>  		edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);
> -	} else {
> +		return IRQ_HANDLED;
> +	} else if (irq == dci->db_irq) {
>  		writel(ALTR_A10_ECC_DERRPENA,
>  		       base + ALTR_A10_ECC_INTSTAT_OFST);
>  		edac_device_handle_ue(dci->edac_dev, 0, 0, dci->edac_dev_name);
>  		panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
> +		return IRQ_HANDLED;
> +	} else {
> +		WARN_ON(1);
>  	}
> -	return IRQ_HANDLED;
> +	return IRQ_NONE;
>  }

Simplified a bit more while applying. Ok?


---
From: Thor Thayer <tthayer@opensource.altera.com>
Date: Tue, 7 Jun 2016 15:35:57 -0500
Subject: [PATCH] EDAC, altera: ECC Manager IRQ controller support

To better support child devices, the ECC manager needs to be
implemented as an IRQ controller.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-edac <linux-edac@vger.kernel.org>
Link: http://lkml.kernel.org/r/1465331757-10227-1-git-send-email-tthayer@opensource.altera.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/edac/altera_edac.c | 167 ++++++++++++++++++++++++++++++++++-----------
 drivers/edac/altera_edac.h |   5 +-
 2 files changed, 130 insertions(+), 42 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index ff74c5b7781a..6f5d586fa0a0 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -22,9 +22,11 @@
 #include <linux/edac.h>
 #include <linux/genalloc.h>
 #include <linux/interrupt.h>
+#include <linux/irqchip/chained_irq.h>
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/of_address.h>
+#include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -880,22 +882,29 @@ static void ocram_free_mem(void *p, size_t size, void *other)
 	gen_pool_free((struct gen_pool *)other, (u32)p, size);
 }
 
-static irqreturn_t altr_edac_a10_ecc_irq(struct altr_edac_device_dev *dci,
-					 bool sberr)
+static irqreturn_t altr_edac_a10_ecc_irq(int irq, void *dev_id)
 {
+	struct altr_edac_device_dev *dci = dev_id;
 	void __iomem  *base = dci->base;
 
-	if (sberr) {
+	if (irq == dci->sb_irq) {
 		writel(ALTR_A10_ECC_SERRPENA,
 		       base + ALTR_A10_ECC_INTSTAT_OFST);
 		edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);
-	} else {
+
+		return IRQ_HANDLED;
+	} else if (irq == dci->db_irq) {
 		writel(ALTR_A10_ECC_DERRPENA,
 		       base + ALTR_A10_ECC_INTSTAT_OFST);
 		edac_device_handle_ue(dci->edac_dev, 0, 0, dci->edac_dev_name);
 		panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
+
+		return IRQ_HANDLED;
 	}
-	return IRQ_HANDLED;
+
+	WARN_ON(1);
+
+	return IRQ_NONE;
 }
 
 const struct edac_device_prv_data ocramecc_data = {
@@ -986,22 +995,30 @@ static int altr_l2_check_deps(struct altr_edac_device_dev *device)
 	return -ENODEV;
 }
 
-static irqreturn_t altr_edac_a10_l2_irq(struct altr_edac_device_dev *dci,
-					bool sberr)
+static irqreturn_t altr_edac_a10_l2_irq(int irq, void *dev_id)
 {
-	if (sberr) {
+	struct altr_edac_device_dev *dci = dev_id;
+
+	if (irq == dci->sb_irq) {
 		regmap_write(dci->edac->ecc_mgr_map,
 			     A10_SYSGMR_MPU_CLEAR_L2_ECC_OFST,
 			     A10_SYSGMR_MPU_CLEAR_L2_ECC_SB);
 		edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);
-	} else {
+
+		return IRQ_HANDLED;
+	} else if (irq == dci->db_irq) {
 		regmap_write(dci->edac->ecc_mgr_map,
 			     A10_SYSGMR_MPU_CLEAR_L2_ECC_OFST,
 			     A10_SYSGMR_MPU_CLEAR_L2_ECC_MB);
 		edac_device_handle_ue(dci->edac_dev, 0, 0, dci->edac_dev_name);
 		panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
+
+		return IRQ_HANDLED;
 	}
-	return IRQ_HANDLED;
+
+	WARN_ON(1);
+
+	return IRQ_NONE;
 }
 
 const struct edac_device_prv_data l2ecc_data = {
@@ -1084,28 +1101,28 @@ static ssize_t altr_edac_a10_device_trig(struct file *file,
 	return count;
 }
 
-static irqreturn_t altr_edac_a10_irq_handler(int irq, void *dev_id)
+static void altr_edac_a10_irq_handler(struct irq_desc *desc)
 {
-	irqreturn_t rc = IRQ_NONE;
-	struct altr_arria10_edac *edac = dev_id;
-	struct altr_edac_device_dev *dci;
-	int irq_status;
-	bool sberr = (irq == edac->sb_irq) ? 1 : 0;
-	int sm_offset = sberr ? A10_SYSMGR_ECC_INTSTAT_SERR_OFST :
-				A10_SYSMGR_ECC_INTSTAT_DERR_OFST;
+	int dberr, bit, sm_offset, irq_status;
+	struct altr_arria10_edac *edac = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	int irq = irq_desc_get_irq(desc);
+
+	dberr = (irq == edac->db_irq) ? 1 : 0;
+	sm_offset = dberr ? A10_SYSMGR_ECC_INTSTAT_DERR_OFST :
+			    A10_SYSMGR_ECC_INTSTAT_SERR_OFST;
+
+	chained_irq_enter(chip, desc);
 
 	regmap_read(edac->ecc_mgr_map, sm_offset, &irq_status);
 
-	if ((irq != edac->sb_irq) && (irq != edac->db_irq)) {
-		WARN_ON(1);
-	} else {
-		list_for_each_entry(dci, &edac->a10_ecc_devices, next) {
-			if (irq_status & dci->data->irq_status_mask)
-				rc = dci->data->ecc_irq_handler(dci, sberr);
-		}
+	for_each_set_bit(bit, (unsigned long *)&irq_status, 32) {
+		irq = irq_linear_revmap(edac->domain, dberr * 32 + bit);
+		if (irq)
+			generic_handle_irq(irq);
 	}
 
-	return rc;
+	chained_irq_exit(chip, desc);
 }
 
 static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
@@ -1177,6 +1194,34 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 			goto err_release_group1;
 	}
 
+	altdev->sb_irq = irq_of_parse_and_map(np, 0);
+	if (!altdev->sb_irq) {
+		edac_printk(KERN_ERR, EDAC_DEVICE, "Error allocating SBIRQ\n");
+		rc = -ENODEV;
+		goto err_release_group1;
+	}
+	rc = devm_request_irq(edac->dev, altdev->sb_irq,
+			      prv->ecc_irq_handler,
+			      IRQF_SHARED, ecc_name, altdev);
+	if (rc) {
+		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
+		goto err_release_group1;
+	}
+
+	altdev->db_irq = irq_of_parse_and_map(np, 1);
+	if (!altdev->db_irq) {
+		edac_printk(KERN_ERR, EDAC_DEVICE, "Error allocating DBIRQ\n");
+		rc = -ENODEV;
+		goto err_release_group1;
+	}
+	rc = devm_request_irq(edac->dev, altdev->db_irq,
+			      prv->ecc_irq_handler,
+			      IRQF_SHARED, ecc_name, altdev);
+	if (rc) {
+		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
+		goto err_release_group1;
+	}
+
 	rc = edac_device_add_device(dci);
 	if (rc) {
 		dev_err(edac->dev, "edac_device_add_device failed\n");
@@ -1195,7 +1240,6 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 err_release_group1:
 	edac_device_free_ctl_info(dci);
 err_release_group:
-	edac_printk(KERN_ALERT, EDAC_DEVICE, "%s: %d\n", __func__, __LINE__);
 	devres_release_group(edac->dev, NULL);
 	edac_printk(KERN_ERR, EDAC_DEVICE,
 		    "%s:Error setting up EDAC device: %d\n", ecc_name, rc);
@@ -1203,11 +1247,43 @@ err_release_group:
 	return rc;
 }
 
+static void a10_eccmgr_irq_mask(struct irq_data *d)
+{
+	struct altr_arria10_edac *edac = irq_data_get_irq_chip_data(d);
+
+	regmap_write(edac->ecc_mgr_map,	A10_SYSMGR_ECC_INTMASK_SET_OFST,
+		     BIT(d->hwirq));
+}
+
+static void a10_eccmgr_irq_unmask(struct irq_data *d)
+{
+	struct altr_arria10_edac *edac = irq_data_get_irq_chip_data(d);
+
+	regmap_write(edac->ecc_mgr_map,	A10_SYSMGR_ECC_INTMASK_CLR_OFST,
+		     BIT(d->hwirq));
+}
+
+static int a10_eccmgr_irqdomain_map(struct irq_domain *d, unsigned int irq,
+				    irq_hw_number_t hwirq)
+{
+	struct altr_arria10_edac *edac = d->host_data;
+
+	irq_set_chip_and_handler(irq, &edac->irq_chip, handle_simple_irq);
+	irq_set_chip_data(irq, edac);
+	irq_set_noprobe(irq);
+
+	return 0;
+}
+
+struct irq_domain_ops a10_eccmgr_ic_ops = {
+	.map = a10_eccmgr_irqdomain_map,
+	.xlate = irq_domain_xlate_twocell,
+};
+
 static int altr_edac_a10_probe(struct platform_device *pdev)
 {
 	struct altr_arria10_edac *edac;
 	struct device_node *child;
-	int rc;
 
 	edac = devm_kzalloc(&pdev->dev, sizeof(*edac), GFP_KERNEL);
 	if (!edac)
@@ -1225,23 +1301,34 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
 		return PTR_ERR(edac->ecc_mgr_map);
 	}
 
+	edac->irq_chip.name = pdev->dev.of_node->name;
+	edac->irq_chip.irq_mask = a10_eccmgr_irq_mask;
+	edac->irq_chip.irq_unmask = a10_eccmgr_irq_unmask;
+	edac->domain = irq_domain_add_linear(pdev->dev.of_node, 64,
+					     &a10_eccmgr_ic_ops, edac);
+	if (!edac->domain) {
+		dev_err(&pdev->dev, "Error adding IRQ domain\n");
+		return -ENOMEM;
+	}
+
 	edac->sb_irq = platform_get_irq(pdev, 0);
-	rc = devm_request_irq(&pdev->dev, edac->sb_irq,
-			      altr_edac_a10_irq_handler,
-			      IRQF_SHARED, dev_name(&pdev->dev), edac);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "No SBERR IRQ resource\n");
-		return rc;
+	if (edac->sb_irq < 0) {
+		dev_err(&pdev->dev, "No SBERR IRQ resource\n");
+		return edac->sb_irq;
 	}
 
+	irq_set_chained_handler_and_data(edac->sb_irq,
+					 altr_edac_a10_irq_handler,
+					 edac);
+
 	edac->db_irq = platform_get_irq(pdev, 1);
-	rc = devm_request_irq(&pdev->dev, edac->db_irq,
-			      altr_edac_a10_irq_handler,
-			      IRQF_SHARED, dev_name(&pdev->dev), edac);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
-		return rc;
+	if (edac->db_irq < 0) {
+		dev_err(&pdev->dev, "No DBERR IRQ resource\n");
+		return edac->db_irq;
 	}
+	irq_set_chained_handler_and_data(edac->db_irq,
+					 altr_edac_a10_irq_handler,
+					 edac);
 
 	for_each_child_of_node(pdev->dev.of_node, child) {
 		if (!of_device_is_available(child))
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index 42090f36ba6e..62b0fa010f95 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -295,8 +295,7 @@ struct edac_device_prv_data {
 	int ce_set_mask;
 	int ue_set_mask;
 	int set_err_ofst;
-	irqreturn_t (*ecc_irq_handler)(struct altr_edac_device_dev *dci,
-				       bool sb);
+	irqreturn_t (*ecc_irq_handler)(int irq, void *dev_id);
 	int trig_alloc_sz;
 	const struct file_operations *inject_fops;
 };
@@ -320,6 +319,8 @@ struct altr_arria10_edac {
 	struct regmap		*ecc_mgr_map;
 	int sb_irq;
 	int db_irq;
+	struct irq_domain	*domain;
+	struct irq_chip		irq_chip;
 	struct list_head	a10_ecc_devices;
 };
 
-- 
2.7.3

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCHv2 2/5] EDAC, altera: ECC Manager IRQ controller support

[Thor Thayer]

On 06/08/2016 04:00 AM, Borislav Petkov wrote:
> On Tue, Jun 07, 2016 at 03:35:57PM -0500, tthayer@opensource.altera.com wrote:
>> From: Thor Thayer <tthayer@opensource.altera.com>
>>
>> To better support child devices, the ECC manager needs to be
>> implemented as an IRQ controller.
>>
>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>> ---
>> v2  Update with cleanup/improvements from maintainer.
>> ---

<snip>

>> +	} else {
>> +		WARN_ON(1);
>>   	}
>> -	return IRQ_HANDLED;
>> +	return IRQ_NONE;
>>   }
>
> Simplified a bit more while applying. Ok?
>
>
Yes, yours is cleaner, Thanks!

<snip>

> +		return IRQ_HANDLED;
>   	}
> -	return IRQ_HANDLED;
> +
> +	WARN_ON(1);
> +
> +	return IRQ_NONE;
>   }
<snip>

[PATCH 4/5] ARM: dts: Arria10 ECC Manager IRQ controller changes

[tthayer]

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

Changes to support IRQ controller implementation including adding
new property irq-controller to eccmgr and adding IRQ property
to children.

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

diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi b/arch/arm/boot/dts/socfpga_arria10.dtsi
index 27cc497..0901090 100644
--- a/arch/arm/boot/dts/socfpga_arria10.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
@@ -606,16 +606,22 @@
 			#size-cells = <1>;
 			interrupts = <0 2 IRQ_TYPE_LEVEL_HIGH>,
 				     <0 0 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
 			ranges;
 
 			l2-ecc@ffd06010 {
 				compatible = "altr,socfpga-a10-l2-ecc";
 				reg = <0xffd06010 0x4>;
+				interrupts = <0 IRQ_TYPE_LEVEL_HIGH>,
+					     <32 IRQ_TYPE_LEVEL_HIGH>;
 			};
 
 			ocram-ecc@ff8c3000 {
 				compatible = "altr,socfpga-a10-ocram-ecc";
 				reg = <0xff8c3000 0x400>;
+				interrupts = <1 IRQ_TYPE_LEVEL_HIGH>,
+					     <33 IRQ_TYPE_LEVEL_HIGH>;
 			};
 		};
 
-- 
1.7.9.5

Re: [PATCH 4/5] ARM: dts: Arria10 ECC Manager IRQ controller changes

[Dinh Nguyen]

On Wed, 25 May 2016, tthayer@opensource.altera.com wrote:

> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> Changes to support IRQ controller implementation including adding
> new property irq-controller to eccmgr and adding IRQ property
> to children.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  arch/arm/boot/dts/socfpga_arria10.dtsi |    6 ++++++
>  1 file changed, 6 insertions(+)
>

Applied! 

Thanks,
Dinh

[PATCH 5/5] ARM: dts: Move Arria10 SDRAM as child of ECC Manager

[tthayer]

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

Changes to support ECC Manager as SDRAM IRQ parent by
1) updating IRQ property values to correct child IRQs
2) moving node under ECC Manager.

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

diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi b/arch/arm/boot/dts/socfpga_arria10.dtsi
index 0901090..605c21e 100644
--- a/arch/arm/boot/dts/socfpga_arria10.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
@@ -568,12 +568,6 @@
 			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>;
@@ -610,6 +604,13 @@
 			#interrupt-cells = <2>;
 			ranges;
 
+			sdramedac {
+				compatible = "altr,sdram-edac-a10";
+				altr,sdr-syscon = <&sdr>;
+				interrupts = <17 IRQ_TYPE_LEVEL_HIGH>,
+					     <49 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
 			l2-ecc@ffd06010 {
 				compatible = "altr,socfpga-a10-l2-ecc";
 				reg = <0xffd06010 0x4>;
-- 
1.7.9.5

Re: [PATCH 5/5] ARM: dts: Move Arria10 SDRAM as child of ECC Manager

[Dinh Nguyen]

On Wed, 25 May 2016, tthayer@opensource.altera.com wrote:

> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> Changes to support ECC Manager as SDRAM IRQ parent by
> 1) updating IRQ property values to correct child IRQs
> 2) moving node under ECC Manager.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  arch/arm/boot/dts/socfpga_arria10.dtsi |   13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>

Applied! 

Thanks,
Dinh