[PATCH] EDAC/altera: use SDMMC compatibles to select A10/S10 IRQ layout

[PATCH] EDAC/altera: use SDMMC compatibles to select A10/S10 IRQ layout

[AZ9tumas]

The SDMMC ECC IRQ layout selection uses CONFIG_64BIT to distinguish
between Arria10 and Stratix10 paths. This is architecture-based,
while the interrupt layout is a hardware property described by DT
compatible strings.

Select the SDMMC IRQ layout via of_device_is_compatible() checks
for altr,socfpga-s10-sdmmc-ecc in both altr_portb_setup() and
altr_edac_a10_device_add().

This keeps the existing behavior for both SoCs while making the
selection mechanism hardware-descriptive.

Signed-off-by: AZ9tumas <rounakdas2025@gmail.com>
---
 drivers/edac/altera_edac.c | 105 +++++++++++++++++++------------------
 1 file changed, 53 insertions(+), 52 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 4edd2088c2db6..161331ef57c01 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -1548,15 +1548,16 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
 
 	/*
 	 * Update the PortB IRQs - A10 has 4, S10 has 2, Index accordingly
-	 *
-	 * FIXME: Instead of ifdefs with different architectures the driver
-	 *        should properly use compatibles.
 	 */
-#ifdef CONFIG_64BIT
-	altdev->sb_irq = irq_of_parse_and_map(np, 1);
-#else
-	altdev->sb_irq = irq_of_parse_and_map(np, 2);
-#endif
+
+	/* Using compatibles to determine the IRQ Index */
+	bool is_s10_sdmmc = of_device_is_compatible(np, "altr,socfpga-s10-sdmmc-ecc");
+
+	if (is_s10_sdmmc)
+		altdev->sb_irq = irq_of_parse_and_map(np, 1);
+	else
+		altdev->sb_irq = irq_of_parse_and_map(np, 2);
+
 	if (!altdev->sb_irq) {
 		edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB SBIRQ alloc\n");
 		rc = -ENODEV;
@@ -1570,29 +1571,29 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
 		goto err_release_group_1;
 	}
 
-#ifdef CONFIG_64BIT
-	/* Use IRQ to determine SError origin instead of assigning IRQ */
-	rc = of_property_read_u32_index(np, "interrupts", 1, &altdev->db_irq);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE,
-			    "Error PortB DBIRQ alloc\n");
-		goto err_release_group_1;
-	}
-#else
-	altdev->db_irq = irq_of_parse_and_map(np, 3);
-	if (!altdev->db_irq) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
-		rc = -ENODEV;
-		goto err_release_group_1;
-	}
-	rc = devm_request_irq(&altdev->ddev, altdev->db_irq,
-			      prv->ecc_irq_handler, IRQF_TRIGGER_HIGH,
-			      ecc_name, altdev);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "PortB DBERR IRQ error\n");
-		goto err_release_group_1;
+	if (is_s10_sdmmc) {
+		/* Use IRQ to determine SError origin instead of assigning IRQ */
+		rc = of_property_read_u32_index(np, "interrupts", 1, &altdev->db_irq);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE,
+					"Error PortB DBIRQ alloc\n");
+			goto err_release_group_1;
+		}
+	} else {
+		altdev->db_irq = irq_of_parse_and_map(np, 3);
+		if (!altdev->db_irq) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
+			rc = -ENODEV;
+			goto err_release_group_1;
+		}
+		rc = devm_request_irq(&altdev->ddev, altdev->db_irq,
+					prv->ecc_irq_handler, IRQF_TRIGGER_HIGH,
+					ecc_name, altdev);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "PortB DBERR IRQ error\n");
+			goto err_release_group_1;
+		}
 	}
-#endif
 
 	rc = edac_device_add_device(dci);
 	if (rc) {
@@ -1974,29 +1975,29 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 		goto err_release_group1;
 	}
 
-#ifdef CONFIG_64BIT
-	/* Use IRQ to determine SError origin instead of assigning IRQ */
-	rc = of_property_read_u32_index(np, "interrupts", 0, &altdev->db_irq);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE,
-			    "Unable to parse DB IRQ index\n");
-		goto err_release_group1;
-	}
-#else
-	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_TRIGGER_HIGH,
-			      ecc_name, altdev);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
-		goto err_release_group1;
+	if (of_device_is_compatible(np, "altr,socfpga-s10-sdmmc-ecc")) {
+		/* Use IRQ to determine SError origin instead of assigning IRQ */
+		rc = of_property_read_u32_index(np, "interrupts", 0, &altdev->db_irq);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE,
+					"Unable to parse DB IRQ index\n");
+			goto err_release_group1;
+		}
+	} else {
+		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_TRIGGER_HIGH,
+					ecc_name, altdev);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
+			goto err_release_group1;
+		}
 	}
-#endif
 
 	rc = edac_device_add_device(dci);
 	if (rc) {
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] EDAC/altera: use SDMMC compatibles to select A10/S10 IRQ layout

[Borislav Petkov]

On June 15, 2026 7:15:08 PM UTC, AZ9tumas <rounakdas2025@gmail.com> wrote:
>The SDMMC ECC IRQ layout selection uses CONFIG_64BIT to distinguish
>between Arria10 and Stratix10 paths. This is architecture-based,
>while the interrupt layout is a hardware property described by DT
>compatible strings.
>
>Select the SDMMC IRQ layout via of_device_is_compatible() checks
>for altr,socfpga-s10-sdmmc-ecc in both altr_portb_setup() and
>altr_edac_a10_device_add().
>
>This keeps the existing behavior for both SoCs while making the
>selection mechanism hardware-descriptive.
>
>Signed-off-by: AZ9tumas <rounakdas2025@gmail.com>

Please use your legal name to sign off on patches.

Thx.

-- 
Small device. Typos and formatting crap

[PATCH v2] EDAC/altera: use SDMMC compatibles to select A10/S10 IRQ layout

[Rounak Das]

The SDMMC ECC IRQ layout selection uses CONFIG_64BIT to distinguish
between Arria10 and Stratix10 paths. This is architecture-based,
while the interrupt layout is a hardware property described by DT
compatible strings.

Select the SDMMC IRQ layout via of_device_is_compatible() checks
for altr,socfpga-s10-sdmmc-ecc in both altr_portb_setup() and
altr_edac_a10_device_add().

This keeps the existing behavior for both SoCs while making the
selection mechanism hardware-descriptive.

Signed-off-by: Rounak Das <rounakdas2025@gmail.com>
---
 drivers/edac/altera_edac.c | 105 +++++++++++++++++++------------------
 1 file changed, 53 insertions(+), 52 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 4edd2088c2db6..161331ef57c01 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -1548,15 +1548,16 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
 
 	/*
 	 * Update the PortB IRQs - A10 has 4, S10 has 2, Index accordingly
-	 *
-	 * FIXME: Instead of ifdefs with different architectures the driver
-	 *        should properly use compatibles.
 	 */
-#ifdef CONFIG_64BIT
-	altdev->sb_irq = irq_of_parse_and_map(np, 1);
-#else
-	altdev->sb_irq = irq_of_parse_and_map(np, 2);
-#endif
+
+	/* Using compatibles to determine the IRQ Index */
+	bool is_s10_sdmmc = of_device_is_compatible(np, "altr,socfpga-s10-sdmmc-ecc");
+
+	if (is_s10_sdmmc)
+		altdev->sb_irq = irq_of_parse_and_map(np, 1);
+	else
+		altdev->sb_irq = irq_of_parse_and_map(np, 2);
+
 	if (!altdev->sb_irq) {
 		edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB SBIRQ alloc\n");
 		rc = -ENODEV;
@@ -1570,29 +1571,29 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
 		goto err_release_group_1;
 	}
 
-#ifdef CONFIG_64BIT
-	/* Use IRQ to determine SError origin instead of assigning IRQ */
-	rc = of_property_read_u32_index(np, "interrupts", 1, &altdev->db_irq);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE,
-			    "Error PortB DBIRQ alloc\n");
-		goto err_release_group_1;
-	}
-#else
-	altdev->db_irq = irq_of_parse_and_map(np, 3);
-	if (!altdev->db_irq) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
-		rc = -ENODEV;
-		goto err_release_group_1;
-	}
-	rc = devm_request_irq(&altdev->ddev, altdev->db_irq,
-			      prv->ecc_irq_handler, IRQF_TRIGGER_HIGH,
-			      ecc_name, altdev);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "PortB DBERR IRQ error\n");
-		goto err_release_group_1;
+	if (is_s10_sdmmc) {
+		/* Use IRQ to determine SError origin instead of assigning IRQ */
+		rc = of_property_read_u32_index(np, "interrupts", 1, &altdev->db_irq);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE,
+					"Error PortB DBIRQ alloc\n");
+			goto err_release_group_1;
+		}
+	} else {
+		altdev->db_irq = irq_of_parse_and_map(np, 3);
+		if (!altdev->db_irq) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
+			rc = -ENODEV;
+			goto err_release_group_1;
+		}
+		rc = devm_request_irq(&altdev->ddev, altdev->db_irq,
+					prv->ecc_irq_handler, IRQF_TRIGGER_HIGH,
+					ecc_name, altdev);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "PortB DBERR IRQ error\n");
+			goto err_release_group_1;
+		}
 	}
-#endif
 
 	rc = edac_device_add_device(dci);
 	if (rc) {
@@ -1974,29 +1975,29 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 		goto err_release_group1;
 	}
 
-#ifdef CONFIG_64BIT
-	/* Use IRQ to determine SError origin instead of assigning IRQ */
-	rc = of_property_read_u32_index(np, "interrupts", 0, &altdev->db_irq);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE,
-			    "Unable to parse DB IRQ index\n");
-		goto err_release_group1;
-	}
-#else
-	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_TRIGGER_HIGH,
-			      ecc_name, altdev);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
-		goto err_release_group1;
+	if (of_device_is_compatible(np, "altr,socfpga-s10-sdmmc-ecc")) {
+		/* Use IRQ to determine SError origin instead of assigning IRQ */
+		rc = of_property_read_u32_index(np, "interrupts", 0, &altdev->db_irq);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE,
+					"Unable to parse DB IRQ index\n");
+			goto err_release_group1;
+		}
+	} else {
+		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_TRIGGER_HIGH,
+					ecc_name, altdev);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
+			goto err_release_group1;
+		}
 	}
-#endif
 
 	rc = edac_device_add_device(dci);
 	if (rc) {
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v2] EDAC/altera: use SDMMC compatibles to select A10/S10 IRQ layout

[Dinh Nguyen]

Hi Rounak,

Thanks you for this patch and attempting to clean this up.

On 6/16/26 17:17, Rounak Das wrote:
> The SDMMC ECC IRQ layout selection uses CONFIG_64BIT to distinguish
> between Arria10 and Stratix10 paths. This is architecture-based,
> while the interrupt layout is a hardware property described by DT
> compatible strings.
> 
> Select the SDMMC IRQ layout via of_device_is_compatible() checks
> for altr,socfpga-s10-sdmmc-ecc in both altr_portb_setup() and
> altr_edac_a10_device_add().
> 
> This keeps the existing behavior for both SoCs while making the
> selection mechanism hardware-descriptive.
> 
> Signed-off-by: Rounak Das <rounakdas2025@gmail.com>
> ---
>   drivers/edac/altera_edac.c | 105 +++++++++++++++++++------------------
>   1 file changed, 53 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index 4edd2088c2db6..161331ef57c01 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c
> @@ -1548,15 +1548,16 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
>   
>   	/*
>   	 * Update the PortB IRQs - A10 has 4, S10 has 2, Index accordingly
> -	 *
> -	 * FIXME: Instead of ifdefs with different architectures the driver
> -	 *        should properly use compatibles.
>   	 */
> -#ifdef CONFIG_64BIT
> -	altdev->sb_irq = irq_of_parse_and_map(np, 1);
> -#else
> -	altdev->sb_irq = irq_of_parse_and_map(np, 2);
> -#endif
> +
> +	/* Using compatibles to determine the IRQ Index */
> +	bool is_s10_sdmmc = of_device_is_compatible(np, "altr,socfpga-s10-sdmmc-ecc");

How come you've decided to look at the sdmmc-ecc compatible instead of
"altr,socfpga-s10-ecc-manager"?

> +
> +	if (is_s10_sdmmc)
> +		altdev->sb_irq = irq_of_parse_and_map(np, 1);
> +	else
> +		altdev->sb_irq = irq_of_parse_and_map(np, 2);
> +
>   	if (!altdev->sb_irq) {
>   		edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB SBIRQ alloc\n");
>   		rc = -ENODEV;
> @@ -1570,29 +1571,29 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
>   		goto err_release_group_1;
>   	}
>   
> -#ifdef CONFIG_64BIT
> -	/* Use IRQ to determine SError origin instead of assigning IRQ */
> -	rc = of_property_read_u32_index(np, "interrupts", 1, &altdev->db_irq);
> -	if (rc) {
> -		edac_printk(KERN_ERR, EDAC_DEVICE,
> -			    "Error PortB DBIRQ alloc\n");
> -		goto err_release_group_1;
> -	}
> -#else
> -	altdev->db_irq = irq_of_parse_and_map(np, 3);
> -	if (!altdev->db_irq) {
> -		edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
> -		rc = -ENODEV;
> -		goto err_release_group_1;
> -	}
> -	rc = devm_request_irq(&altdev->ddev, altdev->db_irq,
> -			      prv->ecc_irq_handler, IRQF_TRIGGER_HIGH,
> -			      ecc_name, altdev);
> -	if (rc) {
> -		edac_printk(KERN_ERR, EDAC_DEVICE, "PortB DBERR IRQ error\n");
> -		goto err_release_group_1;
> +	if (is_s10_sdmmc) {
> +		/* Use IRQ to determine SError origin instead of assigning IRQ */
> +		rc = of_property_read_u32_index(np, "interrupts", 1, &altdev->db_irq);
> +		if (rc) {
> +			edac_printk(KERN_ERR, EDAC_DEVICE,
> +					"Error PortB DBIRQ alloc\n");
> +			goto err_release_group_1;
> +		}
> +	} else {
> +		altdev->db_irq = irq_of_parse_and_map(np, 3);
> +		if (!altdev->db_irq) {
> +			edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
> +			rc = -ENODEV;
> +			goto err_release_group_1;
> +		}
> +		rc = devm_request_irq(&altdev->ddev, altdev->db_irq,
> +					prv->ecc_irq_handler, IRQF_TRIGGER_HIGH,
> +					ecc_name, altdev);
> +		if (rc) {
> +			edac_printk(KERN_ERR, EDAC_DEVICE, "PortB DBERR IRQ error\n");
> +			goto err_release_group_1;
> +		}
>   	}
> -#endif
>   
>   	rc = edac_device_add_device(dci);
>   	if (rc) {
> @@ -1974,29 +1975,29 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
>   		goto err_release_group1;
>   	}
>   
> -#ifdef CONFIG_64BIT
> -	/* Use IRQ to determine SError origin instead of assigning IRQ */
> -	rc = of_property_read_u32_index(np, "interrupts", 0, &altdev->db_irq);
> -	if (rc) {
> -		edac_printk(KERN_ERR, EDAC_DEVICE,
> -			    "Unable to parse DB IRQ index\n");
> -		goto err_release_group1;
> -	}
> -#else
> -	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_TRIGGER_HIGH,
> -			      ecc_name, altdev);
> -	if (rc) {
> -		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
> -		goto err_release_group1;
> +	if (of_device_is_compatible(np, "altr,socfpga-s10-sdmmc-ecc")) {

I'd like to avoid adding more calls to get compatible in the code if 
possible.

> +		/* Use IRQ to determine SError origin instead of assigning IRQ */
> +		rc = of_property_read_u32_index(np, "interrupts", 0, &altdev->db_irq);
> +		if (rc) {
> +			edac_printk(KERN_ERR, EDAC_DEVICE,
> +					"Unable to parse DB IRQ index\n");
> +			goto err_release_group1;
> +		}
> +	} else {
> +		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_TRIGGER_HIGH,
> +					ecc_name, altdev);
> +		if (rc) {
> +			edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
> +			goto err_release_group1;
> +		}
>   	}
> -#endif
>   
>   	rc = edac_device_add_device(dci);
>   	if (rc) {

Got a few checkpatch warnings:

CHECK: Alignment should match open parenthesis
#84: FILE: drivers/edac/altera_edac.c:1579:
+			edac_printk(KERN_ERR, EDAC_DEVICE,
+					"Error PortB DBIRQ alloc\n");

CHECK: Alignment should match open parenthesis
#95: FILE: drivers/edac/altera_edac.c:1590:
+		rc = devm_request_irq(&altdev->ddev, altdev->db_irq,
+					prv->ecc_irq_handler, IRQF_TRIGGER_HIGH,

CHECK: Alignment should match open parenthesis
#136: FILE: drivers/edac/altera_edac.c:1983:
+			edac_printk(KERN_ERR, EDAC_DEVICE,
+					"Unable to parse DB IRQ index\n");

CHECK: Alignment should match open parenthesis
#147: FILE: drivers/edac/altera_edac.c:1994:
+		rc = devm_request_irq(edac->dev, altdev->db_irq, prv->ecc_irq_handler,
+					IRQF_TRIGGER_HIGH,


Thanks,
Dinh

Re: [PATCH v2] EDAC/altera: use SDMMC compatibles to select A10/S10 IRQ layout

[Rounak Das]

Thank you for the review.

I originally used the altr,socfpga-s10-sdmmc-ecc compatible because in
altr_portb_setup() np is the SD/MMC ECC node (found via
of_find_compatible_node(…, “altr,socfpga-sdmmc-ecc”)) so reusing that
felt natural there. I later realised that shifting to
altr,socfpga-s10-ecc-manager is better especially in scenarios where
np could be any child node.

In v3, the compatible is read on the manager node. To avoid repeated
calls (as you have mentioned), I have decided to store it as a `bool
is_s10` in `struct altr_arria10_edac`

Re: [PATCH v2] EDAC/altera: use SDMMC compatibles to select A10/S10 IRQ layout

[Dinh Nguyen]

On 6/16/26 18:58, Rounak Das wrote:
> Thank you for the review.
> 
> I originally used the altr,socfpga-s10-sdmmc-ecc compatible because in
> altr_portb_setup() np is the SD/MMC ECC node (found via
> of_find_compatible_node(…, “altr,socfpga-sdmmc-ecc”)) so reusing that
> felt natural there. I later realised that shifting to
> altr,socfpga-s10-ecc-manager is better especially in scenarios where
> np could be any child node.
> 
> In v3, the compatible is read on the manager node. To avoid repeated
> calls (as you have mentioned), I have decided to store it as a `bool
> is_s10` in `struct altr_arria10_edac`

Doh...sorry about that. Will retest version 3.

Dinh

[PATCH v3] EDAC/altera: use ECC manager compatible to select A10/S10 IRQ layout

[Rounak Das]

The SDMMC ECC IRQ layout selection uses CONFIG_64BIT to distinguish
between Arria10 and Stratix10 paths. This is architecture-based,
while the interrupt layout is a hardware property described by DT
compatible strings.

Detect the SoC once at probe via the altr,socfpga-s10-ecc-manager
compatible on the ECC manager node, store it under the struct
altr_arria10_edac, and use it in altr_portb_setup() and
altr_edac_a10_device_add() in place of CONFIG_64BIT.

Selecting on the manager compatible keeps the decision SoC-wide and
correct for every ECC child device (OCRAM, USB, EMAC, SD/MMC),
and avoids repeated compatible lookups. Stratix10 and
Agilex both declare altr,socfpga-s10-ecc-manager and Arria10 does not,
so behaviour is unchanged on all three SoCs.

Signed-off-by: Rounak Das <rounakdas2025@gmail.com>
---
v3:
 - Use compatible string altr,socfpga-s10-ecc-manager instead of
   sdmmc-ecc (Dinh).
 - Set it once into struct altr_arria10_edac::is_s10 to
   avoid repeated of_device_is_compatible() calls (Dinh).
 - Fix the checkpatch open-parenthesis alignment.

v2: https://lore.kernel.org/linux-edac/20260616081709.48774-1-rounakdas2025@gmail.com/
 - Use legal name in Signed-off-by (Borislav).

 drivers/edac/altera_edac.c | 106 +++++++++++++++++++------------------
 drivers/edac/altera_edac.h |   1 +
 2 files changed, 55 insertions(+), 52 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 4edd2088c2db6..c728cd474abdf 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -1507,6 +1507,7 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
 	int edac_idx, rc;
 	struct device_node *np;
 	const struct edac_device_prv_data *prv = &a10_sdmmceccb_data;
+	bool is_s10 = device->edac->is_s10;
 
 	rc = altr_check_ecc_deps(device);
 	if (rc)
@@ -1548,15 +1549,14 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
 
 	/*
 	 * Update the PortB IRQs - A10 has 4, S10 has 2, Index accordingly
-	 *
-	 * FIXME: Instead of ifdefs with different architectures the driver
-	 *        should properly use compatibles.
 	 */
-#ifdef CONFIG_64BIT
-	altdev->sb_irq = irq_of_parse_and_map(np, 1);
-#else
-	altdev->sb_irq = irq_of_parse_and_map(np, 2);
-#endif
+
+	/* Using compatibles to determine the IRQ Index */
+	if (is_s10)
+		altdev->sb_irq = irq_of_parse_and_map(np, 1);
+	else
+		altdev->sb_irq = irq_of_parse_and_map(np, 2);
+
 	if (!altdev->sb_irq) {
 		edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB SBIRQ alloc\n");
 		rc = -ENODEV;
@@ -1570,29 +1570,28 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
 		goto err_release_group_1;
 	}
 
-#ifdef CONFIG_64BIT
-	/* Use IRQ to determine SError origin instead of assigning IRQ */
-	rc = of_property_read_u32_index(np, "interrupts", 1, &altdev->db_irq);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE,
-			    "Error PortB DBIRQ alloc\n");
-		goto err_release_group_1;
-	}
-#else
-	altdev->db_irq = irq_of_parse_and_map(np, 3);
-	if (!altdev->db_irq) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
-		rc = -ENODEV;
-		goto err_release_group_1;
-	}
-	rc = devm_request_irq(&altdev->ddev, altdev->db_irq,
-			      prv->ecc_irq_handler, IRQF_TRIGGER_HIGH,
-			      ecc_name, altdev);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "PortB DBERR IRQ error\n");
-		goto err_release_group_1;
+	if (is_s10) {
+		/* Use IRQ to determine SError origin instead of assigning IRQ */
+		rc = of_property_read_u32_index(np, "interrupts", 1, &altdev->db_irq);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
+			goto err_release_group_1;
+		}
+	} else {
+		altdev->db_irq = irq_of_parse_and_map(np, 3);
+		if (!altdev->db_irq) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
+			rc = -ENODEV;
+			goto err_release_group_1;
+		}
+		rc = devm_request_irq(&altdev->ddev, altdev->db_irq,
+				      prv->ecc_irq_handler, IRQF_TRIGGER_HIGH,
+				      ecc_name, altdev);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "PortB DBERR IRQ error\n");
+			goto err_release_group_1;
+		}
 	}
-#endif
 
 	rc = edac_device_add_device(dci);
 	if (rc) {
@@ -1974,29 +1973,29 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 		goto err_release_group1;
 	}
 
-#ifdef CONFIG_64BIT
-	/* Use IRQ to determine SError origin instead of assigning IRQ */
-	rc = of_property_read_u32_index(np, "interrupts", 0, &altdev->db_irq);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE,
-			    "Unable to parse DB IRQ index\n");
-		goto err_release_group1;
-	}
-#else
-	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_TRIGGER_HIGH,
-			      ecc_name, altdev);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
-		goto err_release_group1;
+	if (edac->is_s10) {
+		/* Use IRQ to determine SError origin instead of assigning IRQ */
+		rc = of_property_read_u32_index(np, "interrupts", 0, &altdev->db_irq);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE,
+				    "Unable to parse DB IRQ index\n");
+			goto err_release_group1;
+		}
+	} else {
+		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_TRIGGER_HIGH,
+				      ecc_name, altdev);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
+			goto err_release_group1;
+		}
 	}
-#endif
 
 	rc = edac_device_add_device(dci);
 	if (rc) {
@@ -2122,6 +2121,9 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, edac);
 	INIT_LIST_HEAD(&edac->a10_ecc_devices);
 
+	edac->is_s10 = of_device_is_compatible(pdev->dev.of_node,
+					       "altr,socfpga-s10-ecc-manager");
+
 	edac->ecc_mgr_map =
 		altr_sysmgr_regmap_lookup_by_phandle(pdev->dev.of_node,
 						     "altr,sysmgr-syscon");
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index f3e84172caa9c..9387056fd65e3 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -394,6 +394,7 @@ struct altr_arria10_edac {
 	struct irq_chip		irq_chip;
 	struct list_head	a10_ecc_devices;
 	struct notifier_block	panic_notifier;
+	bool is_s10;
 };
 
 #endif	/* #ifndef _ALTERA_EDAC_H */
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v3] EDAC/altera: use ECC manager compatible to select A10/S10 IRQ layout

[Dinh Nguyen]

On 6/16/26 09:07, Rounak Das wrote:
> The SDMMC ECC IRQ layout selection uses CONFIG_64BIT to distinguish
> between Arria10 and Stratix10 paths. This is architecture-based,
> while the interrupt layout is a hardware property described by DT
> compatible strings.
> 
> Detect the SoC once at probe via the altr,socfpga-s10-ecc-manager
> compatible on the ECC manager node, store it under the struct
> altr_arria10_edac, and use it in altr_portb_setup() and
> altr_edac_a10_device_add() in place of CONFIG_64BIT.
> 
> Selecting on the manager compatible keeps the decision SoC-wide and
> correct for every ECC child device (OCRAM, USB, EMAC, SD/MMC),
> and avoids repeated compatible lookups. Stratix10 and
> Agilex both declare altr,socfpga-s10-ecc-manager and Arria10 does not,
> so behaviour is unchanged on all three SoCs.
> 
> Signed-off-by: Rounak Das <rounakdas2025@gmail.com>
> ---
> v3:
>   - Use compatible string altr,socfpga-s10-ecc-manager instead of
>     sdmmc-ecc (Dinh).
>   - Set it once into struct altr_arria10_edac::is_s10 to
>     avoid repeated of_device_is_compatible() calls (Dinh).
>   - Fix the checkpatch open-parenthesis alignment.
> 
> v2: https://lore.kernel.org/linux-edac/20260616081709.48774-1-rounakdas2025@gmail.com/
>   - Use legal name in Signed-off-by (Borislav).
> 
>   drivers/edac/altera_edac.c | 106 +++++++++++++++++++------------------
>   drivers/edac/altera_edac.h |   1 +
>   2 files changed, 55 insertions(+), 52 deletions(-)
> 

Were you able to test this on any HW? I tested on an Arria10 and it 
looks good. Currently, I don't have access to a 64-bit platform. so I'd 
like to wait to get access to one and test.

Thanks,
Dinh

Re: [PATCH v3] EDAC/altera: use ECC manager compatible to select A10/S10 IRQ layout

[Rounak Das]

> Were you able to test this on any HW?
> I tested on an Arria10 and it looks good.

I don't have access to either platform, so I wasn't able to test on HW.
Thanks for verifying on Arria10, glad it looks good there.

> Currently, I don't have access to a 64-bit platform. so I'd
> like to wait to get access to one and test.

Alright, sure.

Thanks,
Rounak

Re: [PATCH v3] EDAC/altera: use ECC manager compatible to select A10/S10 IRQ layout

[Dinh Nguyen]

On 6/16/26 19:07, Rounak Das wrote:
> The SDMMC ECC IRQ layout selection uses CONFIG_64BIT to distinguish
> between Arria10 and Stratix10 paths. This is architecture-based,
> while the interrupt layout is a hardware property described by DT
> compatible strings.
> 
> Detect the SoC once at probe via the altr,socfpga-s10-ecc-manager
> compatible on the ECC manager node, store it under the struct
> altr_arria10_edac, and use it in altr_portb_setup() and
> altr_edac_a10_device_add() in place of CONFIG_64BIT.
> 
> Selecting on the manager compatible keeps the decision SoC-wide and
> correct for every ECC child device (OCRAM, USB, EMAC, SD/MMC),
> and avoids repeated compatible lookups. Stratix10 and
> Agilex both declare altr,socfpga-s10-ecc-manager and Arria10 does not,
> so behaviour is unchanged on all three SoCs.
> 
> Signed-off-by: Rounak Das <rounakdas2025@gmail.com>
> ---
> v3:
>   - Use compatible string altr,socfpga-s10-ecc-manager instead of
>     sdmmc-ecc (Dinh).
>   - Set it once into struct altr_arria10_edac::is_s10 to
>     avoid repeated of_device_is_compatible() calls (Dinh).
>   - Fix the checkpatch open-parenthesis alignment.
> 
> v2: https://lore.kernel.org/linux-edac/20260616081709.48774-1-rounakdas2025@gmail.com/
>   - Use legal name in Signed-off-by (Borislav).
> 
>   drivers/edac/altera_edac.c | 106 +++++++++++++++++++------------------
>   drivers/edac/altera_edac.h |   1 +
>   2 files changed, 55 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index 4edd2088c2db6..c728cd474abdf 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c
> @@ -1507,6 +1507,7 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
>   	int edac_idx, rc;
>   	struct device_node *np;
>   	const struct edac_device_prv_data *prv = &a10_sdmmceccb_data;
> +	bool is_s10 = device->edac->is_s10;
>   
>   	rc = altr_check_ecc_deps(device);
>   	if (rc)
> @@ -1548,15 +1549,14 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
>   
>   	/*
>   	 * Update the PortB IRQs - A10 has 4, S10 has 2, Index accordingly
> -	 *
> -	 * FIXME: Instead of ifdefs with different architectures the driver
> -	 *        should properly use compatibles.
>   	 */
> -#ifdef CONFIG_64BIT
> -	altdev->sb_irq = irq_of_parse_and_map(np, 1);
> -#else
> -	altdev->sb_irq = irq_of_parse_and_map(np, 2);
> -#endif
> +
> +	/* Using compatibles to determine the IRQ Index */
> +	if (is_s10)
> +		altdev->sb_irq = irq_of_parse_and_map(np, 1);
> +	else
> +		altdev->sb_irq = irq_of_parse_and_map(np, 2);
> +
>   	if (!altdev->sb_irq) {
>   		edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB SBIRQ alloc\n");
>   		rc = -ENODEV;
> @@ -1570,29 +1570,28 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
>   		goto err_release_group_1;
>   	}
>   
> -#ifdef CONFIG_64BIT
> -	/* Use IRQ to determine SError origin instead of assigning IRQ */
> -	rc = of_property_read_u32_index(np, "interrupts", 1, &altdev->db_irq);
> -	if (rc) {
> -		edac_printk(KERN_ERR, EDAC_DEVICE,
> -			    "Error PortB DBIRQ alloc\n");
> -		goto err_release_group_1;
> -	}
> -#else
> -	altdev->db_irq = irq_of_parse_and_map(np, 3);
> -	if (!altdev->db_irq) {
> -		edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
> -		rc = -ENODEV;
> -		goto err_release_group_1;
> -	}
> -	rc = devm_request_irq(&altdev->ddev, altdev->db_irq,
> -			      prv->ecc_irq_handler, IRQF_TRIGGER_HIGH,
> -			      ecc_name, altdev);
> -	if (rc) {
> -		edac_printk(KERN_ERR, EDAC_DEVICE, "PortB DBERR IRQ error\n");
> -		goto err_release_group_1;
> +	if (is_s10) {
> +		/* Use IRQ to determine SError origin instead of assigning IRQ */
> +		rc = of_property_read_u32_index(np, "interrupts", 1, &altdev->db_irq);
> +		if (rc) {
> +			edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
> +			goto err_release_group_1;
> +		}
> +	} else {
> +		altdev->db_irq = irq_of_parse_and_map(np, 3);
> +		if (!altdev->db_irq) {
> +			edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
> +			rc = -ENODEV;
> +			goto err_release_group_1;
> +		}
> +		rc = devm_request_irq(&altdev->ddev, altdev->db_irq,
> +				      prv->ecc_irq_handler, IRQF_TRIGGER_HIGH,
> +				      ecc_name, altdev);
> +		if (rc) {
> +			edac_printk(KERN_ERR, EDAC_DEVICE, "PortB DBERR IRQ error\n");
> +			goto err_release_group_1;
> +		}
>   	}
> -#endif
>   
>   	rc = edac_device_add_device(dci);
>   	if (rc) {
> @@ -1974,29 +1973,29 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
>   		goto err_release_group1;
>   	}
>   
> -#ifdef CONFIG_64BIT
> -	/* Use IRQ to determine SError origin instead of assigning IRQ */
> -	rc = of_property_read_u32_index(np, "interrupts", 0, &altdev->db_irq);
> -	if (rc) {
> -		edac_printk(KERN_ERR, EDAC_DEVICE,
> -			    "Unable to parse DB IRQ index\n");
> -		goto err_release_group1;
> -	}
> -#else
> -	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_TRIGGER_HIGH,
> -			      ecc_name, altdev);
> -	if (rc) {
> -		edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
> -		goto err_release_group1;
> +	if (edac->is_s10) {
> +		/* Use IRQ to determine SError origin instead of assigning IRQ */
> +		rc = of_property_read_u32_index(np, "interrupts", 0, &altdev->db_irq);
> +		if (rc) {
> +			edac_printk(KERN_ERR, EDAC_DEVICE,
> +				    "Unable to parse DB IRQ index\n");
> +			goto err_release_group1;
> +		}
> +	} else {
> +		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_TRIGGER_HIGH,
> +				      ecc_name, altdev);
> +		if (rc) {
> +			edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
> +			goto err_release_group1;
> +		}
>   	}
> -#endif
>   
>   	rc = edac_device_add_device(dci);
>   	if (rc) {
> @@ -2122,6 +2121,9 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
>   	platform_set_drvdata(pdev, edac);
>   	INIT_LIST_HEAD(&edac->a10_ecc_devices);
>   
> +	edac->is_s10 = of_device_is_compatible(pdev->dev.of_node,
> +					       "altr,socfpga-s10-ecc-manager");
> +


I tested this version on both 32-bit and 64-bit platforms and there were 
no regression. But I'd like to see if we can avoid using 
of_device_is_compatible in runtime code and see if we can just bind to 
the "altr,socfpga-s10-ecc-manager" if possible?

Thanks,
Dinh

Re: [PATCH v3] EDAC/altera: use ECC manager compatible to select A10/S10 IRQ layout

[Rounak Das]

Hi Dinh,
I'm glad to know that it tested clean on both platforms.

To make sure that I understand the approach correctly, do you mean to
carry the A10/S10 distinction in the `altr_edac_a10_of_match[]` table
via the `.data` field?
And then reading it back in probe through `device_get_match_data()`?

If I got it right, I'll send it as v4.

Thanks,
Rounak

On Thu, Jun 25, 2026 at 4:09 PM Dinh Nguyen <dinguyen@kernel.org> wrote:
>
>
>
> On 6/16/26 19:07, Rounak Das wrote:
> > The SDMMC ECC IRQ layout selection uses CONFIG_64BIT to distinguish
> > between Arria10 and Stratix10 paths. This is architecture-based,
> > while the interrupt layout is a hardware property described by DT
> > compatible strings.
> >
> > Detect the SoC once at probe via the altr,socfpga-s10-ecc-manager
> > compatible on the ECC manager node, store it under the struct
> > altr_arria10_edac, and use it in altr_portb_setup() and
> > altr_edac_a10_device_add() in place of CONFIG_64BIT.
> >
> > Selecting on the manager compatible keeps the decision SoC-wide and
> > correct for every ECC child device (OCRAM, USB, EMAC, SD/MMC),
> > and avoids repeated compatible lookups. Stratix10 and
> > Agilex both declare altr,socfpga-s10-ecc-manager and Arria10 does not,
> > so behaviour is unchanged on all three SoCs.
> >
> > Signed-off-by: Rounak Das <rounakdas2025@gmail.com>
> > ---
> > v3:
> >   - Use compatible string altr,socfpga-s10-ecc-manager instead of
> >     sdmmc-ecc (Dinh).
> >   - Set it once into struct altr_arria10_edac::is_s10 to
> >     avoid repeated of_device_is_compatible() calls (Dinh).
> >   - Fix the checkpatch open-parenthesis alignment.
> >
> > v2: https://lore.kernel.org/linux-edac/20260616081709.48774-1-rounakdas2025@gmail.com/
> >   - Use legal name in Signed-off-by (Borislav).
> >
> >   drivers/edac/altera_edac.c | 106 +++++++++++++++++++------------------
> >   drivers/edac/altera_edac.h |   1 +
> >   2 files changed, 55 insertions(+), 52 deletions(-)
> >
> > diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> > index 4edd2088c2db6..c728cd474abdf 100644
> > --- a/drivers/edac/altera_edac.c
> > +++ b/drivers/edac/altera_edac.c
> > @@ -1507,6 +1507,7 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
> >       int edac_idx, rc;
> >       struct device_node *np;
> >       const struct edac_device_prv_data *prv = &a10_sdmmceccb_data;
> > +     bool is_s10 = device->edac->is_s10;
> >
> >       rc = altr_check_ecc_deps(device);
> >       if (rc)
> > @@ -1548,15 +1549,14 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
> >
> >       /*
> >        * Update the PortB IRQs - A10 has 4, S10 has 2, Index accordingly
> > -      *
> > -      * FIXME: Instead of ifdefs with different architectures the driver
> > -      *        should properly use compatibles.
> >        */
> > -#ifdef CONFIG_64BIT
> > -     altdev->sb_irq = irq_of_parse_and_map(np, 1);
> > -#else
> > -     altdev->sb_irq = irq_of_parse_and_map(np, 2);
> > -#endif
> > +
> > +     /* Using compatibles to determine the IRQ Index */
> > +     if (is_s10)
> > +             altdev->sb_irq = irq_of_parse_and_map(np, 1);
> > +     else
> > +             altdev->sb_irq = irq_of_parse_and_map(np, 2);
> > +
> >       if (!altdev->sb_irq) {
> >               edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB SBIRQ alloc\n");
> >               rc = -ENODEV;
> > @@ -1570,29 +1570,28 @@ static int altr_portb_setup(struct altr_edac_device_dev *device)
> >               goto err_release_group_1;
> >       }
> >
> > -#ifdef CONFIG_64BIT
> > -     /* Use IRQ to determine SError origin instead of assigning IRQ */
> > -     rc = of_property_read_u32_index(np, "interrupts", 1, &altdev->db_irq);
> > -     if (rc) {
> > -             edac_printk(KERN_ERR, EDAC_DEVICE,
> > -                         "Error PortB DBIRQ alloc\n");
> > -             goto err_release_group_1;
> > -     }
> > -#else
> > -     altdev->db_irq = irq_of_parse_and_map(np, 3);
> > -     if (!altdev->db_irq) {
> > -             edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
> > -             rc = -ENODEV;
> > -             goto err_release_group_1;
> > -     }
> > -     rc = devm_request_irq(&altdev->ddev, altdev->db_irq,
> > -                           prv->ecc_irq_handler, IRQF_TRIGGER_HIGH,
> > -                           ecc_name, altdev);
> > -     if (rc) {
> > -             edac_printk(KERN_ERR, EDAC_DEVICE, "PortB DBERR IRQ error\n");
> > -             goto err_release_group_1;
> > +     if (is_s10) {
> > +             /* Use IRQ to determine SError origin instead of assigning IRQ */
> > +             rc = of_property_read_u32_index(np, "interrupts", 1, &altdev->db_irq);
> > +             if (rc) {
> > +                     edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
> > +                     goto err_release_group_1;
> > +             }
> > +     } else {
> > +             altdev->db_irq = irq_of_parse_and_map(np, 3);
> > +             if (!altdev->db_irq) {
> > +                     edac_printk(KERN_ERR, EDAC_DEVICE, "Error PortB DBIRQ alloc\n");
> > +                     rc = -ENODEV;
> > +                     goto err_release_group_1;
> > +             }
> > +             rc = devm_request_irq(&altdev->ddev, altdev->db_irq,
> > +                                   prv->ecc_irq_handler, IRQF_TRIGGER_HIGH,
> > +                                   ecc_name, altdev);
> > +             if (rc) {
> > +                     edac_printk(KERN_ERR, EDAC_DEVICE, "PortB DBERR IRQ error\n");
> > +                     goto err_release_group_1;
> > +             }
> >       }
> > -#endif
> >
> >       rc = edac_device_add_device(dci);
> >       if (rc) {
> > @@ -1974,29 +1973,29 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
> >               goto err_release_group1;
> >       }
> >
> > -#ifdef CONFIG_64BIT
> > -     /* Use IRQ to determine SError origin instead of assigning IRQ */
> > -     rc = of_property_read_u32_index(np, "interrupts", 0, &altdev->db_irq);
> > -     if (rc) {
> > -             edac_printk(KERN_ERR, EDAC_DEVICE,
> > -                         "Unable to parse DB IRQ index\n");
> > -             goto err_release_group1;
> > -     }
> > -#else
> > -     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_TRIGGER_HIGH,
> > -                           ecc_name, altdev);
> > -     if (rc) {
> > -             edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
> > -             goto err_release_group1;
> > +     if (edac->is_s10) {
> > +             /* Use IRQ to determine SError origin instead of assigning IRQ */
> > +             rc = of_property_read_u32_index(np, "interrupts", 0, &altdev->db_irq);
> > +             if (rc) {
> > +                     edac_printk(KERN_ERR, EDAC_DEVICE,
> > +                                 "Unable to parse DB IRQ index\n");
> > +                     goto err_release_group1;
> > +             }
> > +     } else {
> > +             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_TRIGGER_HIGH,
> > +                                   ecc_name, altdev);
> > +             if (rc) {
> > +                     edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
> > +                     goto err_release_group1;
> > +             }
> >       }
> > -#endif
> >
> >       rc = edac_device_add_device(dci);
> >       if (rc) {
> > @@ -2122,6 +2121,9 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
> >       platform_set_drvdata(pdev, edac);
> >       INIT_LIST_HEAD(&edac->a10_ecc_devices);
> >
> > +     edac->is_s10 = of_device_is_compatible(pdev->dev.of_node,
> > +                                            "altr,socfpga-s10-ecc-manager");
> > +
>
>
> I tested this version on both 32-bit and 64-bit platforms and there were
> no regression. But I'd like to see if we can avoid using
> of_device_is_compatible in runtime code and see if we can just bind to
> the "altr,socfpga-s10-ecc-manager" if possible?
>
> Thanks,
> Dinh
>

Re: [PATCH v3] EDAC/altera: use ECC manager compatible to select A10/S10 IRQ layout

[Dinh Nguyen]

On 6/26/26 03:22, Rounak Das wrote:
> Hi Dinh,
> I'm glad to know that it tested clean on both platforms.
> 
> To make sure that I understand the approach correctly, do you mean to
> carry the A10/S10 distinction in the `altr_edac_a10_of_match[]` table
> via the `.data` field?
> And then reading it back in probe through `device_get_match_data()`?
> 
> If I got it right, I'll send it as v4.
> 

Yes, that was what I thinking.

Thanks,
Dinh

Re: [PATCH v2] EDAC/altera: use SDMMC compatibles to select A10/S10 IRQ layout

[Rounak Das]

Hi Dinh,

Thanks for testing on the Stratix10 devkit.

This looks like it was tested against v2. The message is threaded
under the v2 patch containing the v2 diff, which uses
`altr,socfpga-s10-sdmmc-ecc` inside `altr_edac_a10_device_add()`. This
only matches the SD/MMC node, causing `sdramedac` and `ocram-ecc` to
fall into the else clause and call `irq_of_parse_and_map(np, 1)`.
Those nodes only have a single interrupt, so it does `rc = -ENODEV`
which matches the log (-19).

v3 was reworked specifically to fix this issue by using
`altr,socfpga-s10-ecc-manager` once at the probe, under
`altr_edac_a10_probe` and storing it as `edac->is_s10`.
v3 link: https://lore.kernel.org/lkml/20260617000711.60804-1-rounakdas2025@gmail.com/

Could you re-test against v3 when you get a chance?

Thanks,
Rounak