public inbox for linux-edac@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Add EDAC support for Agilex5 SoCFPGA
@ 2025-10-28  9:22 niravkumarlaxmidas.rabara
  2025-10-28  9:22 ` [PATCH 1/6] dt-bindings: edac: altera: Document additional ECC instances niravkumarlaxmidas.rabara
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: niravkumarlaxmidas.rabara @ 2025-10-28  9:22 UTC (permalink / raw)
  To: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, bp, tony.luck
  Cc: linux-edac, devicetree, linux-kernel, Niravkumar L Rabara

From: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>

This patch series extends the EDAC support for the Agilex5 SoCFPGA.

It introduces the ECC manager (eccmgr) as a logical parent node that
manages multiple ECC hardware instances for USB, Ethernet, OCRAM, IO96B
memory controller, Secure Device Manager (SDM) QSPI, and Configuration RAM
(CRAM) Single Event Upseet(SEU).

The series includes device tree bindings update, DTS update, and EDAC
driver changes for interrupt handling, error detection and reporting
through the EDAC framework.

Niravkumar L Rabara (6):
  dt-bindings: edac: altera: Document additional ECC instances
  arm64: dts: agilex5: Add ECC manager and submodule nodes
  EDAC/altera: Add DBE interrupt handling for Agilex5
  EDAC/altera: Add IO96B ECC support for Agilex5 SoCFPGA
  EDAC/altera: Add support for CRAM SEU error handling on SoCFPGA
  EDAC: altera: Add ECC support for SDM QSPI on Agilex5

 .../edac/altr,socfpga-ecc-manager.yaml        |  77 +-
 .../arm64/boot/dts/intel/socfpga_agilex5.dtsi |  98 +++
 drivers/edac/Kconfig                          |  33 +
 drivers/edac/altera_edac.c                    | 670 ++++++++++++++++--
 drivers/edac/altera_edac.h                    |  56 ++
 include/linux/firmware/intel/stratix10-smc.h  |  55 ++
 6 files changed, 948 insertions(+), 41 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/6] dt-bindings: edac: altera: Document additional ECC instances
  2025-10-28  9:22 [PATCH 0/6] Add EDAC support for Agilex5 SoCFPGA niravkumarlaxmidas.rabara
@ 2025-10-28  9:22 ` niravkumarlaxmidas.rabara
  2025-10-29  6:50   ` Krzysztof Kozlowski
  2025-10-28  9:22 ` [PATCH 2/6] arm64: dts: agilex5: Add ECC manager and submodule nodes niravkumarlaxmidas.rabara
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: niravkumarlaxmidas.rabara @ 2025-10-28  9:22 UTC (permalink / raw)
  To: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, bp, tony.luck
  Cc: linux-edac, devicetree, linux-kernel, Niravkumar L Rabara

From: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>

Add support for Secure Device Manager(SDM) QSPI ECC, IO96B memory
controller ECC and Configuration RAM(CRAM) Single Event Upset(SEU).

Add interrupt-names property and increase interrupts maxItems from 2 to 7
to accommodate additional interrupts.

Signed-off-by: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
---
 .../edac/altr,socfpga-ecc-manager.yaml        | 77 ++++++++++++++++++-
 1 file changed, 76 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml b/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml
index 3d787dea0f14..5e0c08a15ab9 100644
--- a/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml
+++ b/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml
@@ -33,7 +33,13 @@ properties:
 
   interrupts:
     minItems: 1
-    maxItems: 2
+    maxItems: 7
+
+  interrupt-names:
+    items:
+      enum: [global_sbe, global_dbe, io96b0, io96b1, sdm_qspi_sbe, sdm_qspi_dbe, sdm_seu]
+    minItems: 1
+    maxItems: 7
 
   interrupt-controller: true
 
@@ -70,6 +76,41 @@ properties:
       - interrupts
       - altr,sdr-syscon
 
+  cram-seu:
+    type: object
+    additionalProperties: false
+
+    properties:
+      compatible:
+        items:
+          - const: altr,socfpga-cram-seu
+
+      reg:
+        maxItems: 1
+
+      altr,seu-safe-inject-ce-msb:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: MSB of error injection command for Correctable Error
+
+      altr,seu-safe-inject-ce-lsb:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: LSB of error injection command for Correctable Error
+
+      altr,seu-safe-inject-ue-msb:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: MSB of error injection command for Uncorrectable Error
+
+      altr,seu-safe-inject-ue-lsb:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: LSB of error injection command for Uncorrectable Error
+
+    required:
+      - compatible
+      - altr,seu-safe-inject-ce-msb
+      - altr,seu-safe-inject-ce-lsb
+      - altr,seu-safe-inject-ue-msb
+      - altr,seu-safe-inject-ue-lsb
+
 patternProperties:
   "^ocram-ecc@[a-f0-9]+$":
     type: object
@@ -191,6 +232,40 @@ patternProperties:
       - interrupts
       - altr,ecc-parent
 
+  "^sdm-qspi-ecc@[a-f0-9]+$":
+    type: object
+    additionalProperties: false
+
+    properties:
+      compatible:
+        items:
+          - const: altr,socfpga-sdm-qspi-ecc
+
+      reg:
+        maxItems: 1
+
+    required:
+      - compatible
+      - reg
+
+  "^io96b[0-9]-ecc@[a-f0-9]+$":
+    type: object
+    additionalProperties: false
+
+    properties:
+      compatible:
+        items:
+          - enum:
+              - altr,socfpga-io96b0-ecc
+              - altr,socfpga-io96b1-ecc
+
+      reg:
+        maxItems: 1
+
+    required:
+      - compatible
+      - reg
+
   "^l2-ecc@[a-f0-9]+$":
     type: object
     additionalProperties: false
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/6] arm64: dts: agilex5: Add ECC manager and submodule nodes
  2025-10-28  9:22 [PATCH 0/6] Add EDAC support for Agilex5 SoCFPGA niravkumarlaxmidas.rabara
  2025-10-28  9:22 ` [PATCH 1/6] dt-bindings: edac: altera: Document additional ECC instances niravkumarlaxmidas.rabara
@ 2025-10-28  9:22 ` niravkumarlaxmidas.rabara
  2025-10-29  6:51   ` Krzysztof Kozlowski
  2025-10-28  9:22 ` [PATCH 3/6] EDAC/altera: Add DBE interrupt handling for Agilex5 niravkumarlaxmidas.rabara
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: niravkumarlaxmidas.rabara @ 2025-10-28  9:22 UTC (permalink / raw)
  To: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, bp, tony.luck
  Cc: linux-edac, devicetree, linux-kernel, Niravkumar L Rabara

From: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>

Add the ECC manager (eccmgr) node and its associated ECC submodules to the
Agilex5 SoCFPGA device tree. The eccmgr node serves as a logical parent to
group various ECC hardware instances, including those for USB, Ethernet,
OCRAM, IO96B memory controllers, Secure Device Manager (SDM) QSPI, and
Configuration RAM (CRAM) Single Event Upset (SEU) subsystems.

Signed-off-by: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
---
 .../arm64/boot/dts/intel/socfpga_agilex5.dtsi | 98 +++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
index 04e99cd7e74b..5ea7a506d3d2 100644
--- a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
+++ b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
@@ -428,6 +428,104 @@ usb0: usb@10b00000 {
 			status = "disabled";
 		};
 
+		eccmgr {
+			compatible = "altr,socfpga-a10-ecc-manager";
+			altr,sysmgr-syscon = <&sysmgr>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 241 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 95 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 120 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "global_sbe", "global_dbe", "io96b0" , "io96b1",
+					  "sdm_qspi_sbe", "sdm_qspi_dbe", "sdm_seu";
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			ranges;
+
+			ocram-ecc@108cc000 {
+				compatible = "altr,socfpga-a10-ocram-ecc";
+				reg = <0x108cc000 0x100>;
+				interrupts = <1 IRQ_TYPE_LEVEL_HIGH>, <33 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+			usb0-ecc@108c4000 {
+				compatible = "altr,socfpga-usb-ecc";
+				reg = <0x108c4000 0x100>;
+				altr,ecc-parent = <&usb0>;
+				interrupts = <2 IRQ_TYPE_LEVEL_HIGH>, <34 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+			emac0-rx-ecc@108c0000 {
+				compatible = "altr,socfpga-eth-mac-ecc";
+				reg = <0x108c0000 0x100>;
+				altr,ecc-parent = <&gmac0>;
+				interrupts = <4 IRQ_TYPE_LEVEL_HIGH>, <38 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+			emac0-tx-ecc@108c0400 {
+				compatible = "altr,socfpga-eth-mac-ecc";
+				reg = <0x108c0400 0x100>;
+				altr,ecc-parent = <&gmac0>;
+				interrupts = <5 IRQ_TYPE_LEVEL_HIGH>, <37 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+			emac1-rx-ecc@108c0800 {
+				compatible = "altr,socfpga-eth-mac-ecc";
+				reg = <0x108c0800 0x100>;
+				altr,ecc-parent = <&gmac1>;
+				interrupts = <6 IRQ_TYPE_LEVEL_HIGH>, <38 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+			emac1-tx-ecc@108c0c00 {
+				compatible = "altr,socfpga-eth-mac-ecc";
+				reg = <0x108c0c00 0x100>;
+				altr,ecc-parent = <&gmac1>;
+				interrupts = <7 IRQ_TYPE_LEVEL_HIGH>, <39 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+			emac2-rx-ecc@108c1000 {
+				compatible = "altr,socfpga-eth-mac-ecc";
+				reg = <0x108c1000 0x100>;
+				altr,ecc-parent = <&gmac2>;
+				interrupts = <8 IRQ_TYPE_LEVEL_HIGH>, <40 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+			emac2-tx-ecc@108c1400 {
+				compatible = "altr,socfpga-eth-mac-ecc";
+				reg = <0x108c1400 0x100>;
+				altr,ecc-parent = <&gmac2>;
+				interrupts = <9 IRQ_TYPE_LEVEL_HIGH>, <41 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+			io96b0-ecc@18400000 {
+				compatible = "altr,socfpga-io96b0-ecc";
+				reg = <0x18400000 0x1000>;
+			};
+
+			io96b1-ecc@18800000 {
+				compatible = "altr,socfpga-io96b1-ecc";
+				reg = <0x18800000 0x1000>;
+				status = "disabled";
+			};
+
+			sdm-qspi-ecc@10a22000 {
+				compatible = "altr,socfpga-sdm-qspi-ecc";
+				reg = <0x10a22000 0x100>;
+			};
+
+			cram-seu {
+				compatible = "altr,socfpga-cram-seu";
+				altr,seu-safe-inject-ce-msb = <0x0>;
+				altr,seu-safe-inject-ce-lsb = <0x30000>;
+				altr,seu-safe-inject-ue-msb = <0x20>;
+				altr,seu-safe-inject-ue-lsb = <0x30001>;
+			};
+		};
+
 		watchdog0: watchdog@10d00200 {
 			compatible = "snps,dw-wdt";
 			reg = <0x10d00200 0x100>;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/6] EDAC/altera: Add DBE interrupt handling for Agilex5
  2025-10-28  9:22 [PATCH 0/6] Add EDAC support for Agilex5 SoCFPGA niravkumarlaxmidas.rabara
  2025-10-28  9:22 ` [PATCH 1/6] dt-bindings: edac: altera: Document additional ECC instances niravkumarlaxmidas.rabara
  2025-10-28  9:22 ` [PATCH 2/6] arm64: dts: agilex5: Add ECC manager and submodule nodes niravkumarlaxmidas.rabara
@ 2025-10-28  9:22 ` niravkumarlaxmidas.rabara
  2025-10-28  9:22 ` [PATCH 4/6] EDAC/altera: Add IO96B ECC support for Agilex5 SoCFPGA niravkumarlaxmidas.rabara
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: niravkumarlaxmidas.rabara @ 2025-10-28  9:22 UTC (permalink / raw)
  To: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, bp, tony.luck
  Cc: linux-edac, devicetree, linux-kernel, Niravkumar L Rabara

From: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>

Agilex5 SoCFPGA uses a dedicated interrupt for Double Bit Error (DBE)
reporting, unlike other 64-bit SoCFPGA platforms (Agilex7 and Stratix10)
which signal DBE events via Asynchronous SError interrupts.

Add Agilex5-specific handling to differentiate between these platforms and
correctly process DBE interrupt.

Signed-off-by: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
---
 drivers/edac/altera_edac.c | 42 ++++++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 103b2c2eba2a..ee3270bf75e6 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -1966,12 +1966,32 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 	}
 
 #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;
+	if (of_machine_is_compatible("intel,socfpga-agilex5")) {
+		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_ONESHOT | IRQF_TRIGGER_HIGH,
+				      ecc_name, altdev);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE,
+				    "No DBERR IRQ resource\n");
+			goto err_release_group1;
+		}
+	} else {
+		/* 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);
@@ -2145,6 +2165,16 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
 					 altr_edac_a10_irq_handler,
 					 edac);
 
+	if (of_machine_is_compatible("intel,socfpga-agilex5")) {
+		edac->db_irq = platform_get_irq_byname(pdev, "global_dbe");
+		if (edac->db_irq < 0)
+			return edac->db_irq;
+
+		irq_set_chained_handler_and_data(edac->db_irq,
+						 altr_edac_a10_irq_handler,
+						 edac);
+	}
+
 #ifdef CONFIG_64BIT
 	{
 		int dberror, err_addr;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/6] EDAC/altera: Add IO96B ECC support for Agilex5 SoCFPGA
  2025-10-28  9:22 [PATCH 0/6] Add EDAC support for Agilex5 SoCFPGA niravkumarlaxmidas.rabara
                   ` (2 preceding siblings ...)
  2025-10-28  9:22 ` [PATCH 3/6] EDAC/altera: Add DBE interrupt handling for Agilex5 niravkumarlaxmidas.rabara
@ 2025-10-28  9:22 ` niravkumarlaxmidas.rabara
  2025-10-30 14:30   ` Borislav Petkov
  2025-11-04  3:46   ` kernel test robot
  2025-10-28  9:22 ` [PATCH 5/6] EDAC/altera: Add support for CRAM SEU error handling on SoCFPGA niravkumarlaxmidas.rabara
  2025-10-28  9:22 ` [PATCH 6/6] EDAC: altera: Add ECC support for SDM QSPI on Agilex5 niravkumarlaxmidas.rabara
  5 siblings, 2 replies; 16+ messages in thread
From: niravkumarlaxmidas.rabara @ 2025-10-28  9:22 UTC (permalink / raw)
  To: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, bp, tony.luck
  Cc: linux-edac, devicetree, linux-kernel, Niravkumar L Rabara

From: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>

Add EDAC driver support for the two IO96B memory controllers present in
Agilex5 SoCFPGA.

The IO96B controller provides dedicated mailbox registers for status
and control. ECC error injection is handled via Secure Monitor Calls (SMC).

Signed-off-by: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
---
 drivers/edac/Kconfig                         |  10 +
 drivers/edac/altera_edac.c                   | 305 ++++++++++++++++---
 drivers/edac/altera_edac.h                   |  42 +++
 include/linux/firmware/intel/stratix10-smc.h |  18 ++
 4 files changed, 326 insertions(+), 49 deletions(-)

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 39352b9b7a7e..33a9fccde2fe 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -410,6 +410,16 @@ config EDAC_ALTERA_SDRAM
 	  preloader must initialize the SDRAM before loading
 	  the kernel.
 
+config EDAC_ALTERA_IO96B
+	bool "Altera I096B ECC"
+	depends on EDAC_ALTERA=y && ARM64
+	help
+	  Support for SERR and DERR detection and correction on the
+	  IO96B memory controller interface for Altera SoCFPGA.
+
+	  I096B memory controller provides dedicated mailbox registers
+	  for error injection and error information.
+
 config EDAC_ALTERA_L2C
 	bool "Altera L2 Cache ECC"
 	depends on EDAC_ALTERA=y && CACHE_L2X0
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index ee3270bf75e6..a82c3b01be1a 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -656,6 +656,19 @@ static const struct file_operations altr_edac_a10_device_inject_fops __maybe_unu
 	.llseek = generic_file_llseek,
 };
 
+#ifdef CONFIG_EDAC_ALTERA_IO96B
+static ssize_t __maybe_unused
+altr_edac_io96b_device_trig(struct file *file, const char __user *user_buf,
+			    size_t count, loff_t *ppos);
+
+static const struct file_operations
+altr_edac_io96b_inject_fops __maybe_unused = {
+	.open = simple_open,
+	.write = altr_edac_io96b_device_trig,
+	.llseek = generic_file_llseek,
+};
+#endif
+
 static ssize_t __maybe_unused
 altr_edac_a10_device_trig2(struct file *file, const char __user *user_buf,
 			   size_t count, loff_t *ppos);
@@ -1121,6 +1134,33 @@ static const struct edac_device_prv_data s10_sdramecc_data = {
 };
 #endif /* CONFIG_EDAC_ALTERA_SDRAM */
 
+/************************IO96B EDAC *************************************/
+
+#ifdef CONFIG_EDAC_ALTERA_IO96B
+static DEFINE_MUTEX(io96b_mb_mutex);
+
+static int altr_agilex5_io96b_ecc_init(struct altr_edac_device_dev *device)
+{
+	u32 ecc_status;
+
+	ecc_status = readl(device->base + IO96B_ECC_ENABLE_INFO_OFST);
+	ecc_status &= GENMASK(1, 0);
+
+	if (!ecc_status) {
+		edac_printk(KERN_ERR, EDAC_DEVICE,
+			    "%s: No ECC present or ECC disabled.\n",
+			    device->edac_dev_name);
+		return -ENODEV;
+	}
+	return 0;
+}
+
+static const struct edac_device_prv_data agilex5_io96b_data = {
+	.setup = altr_agilex5_io96b_ecc_init,
+	.inject_fops = &altr_edac_io96b_inject_fops,
+};
+#endif /* CONFIG_EDAC_ALTERA_IO96B */
+
 /*********************** OCRAM EDAC Device Functions *********************/
 
 #ifdef CONFIG_EDAC_ALTERA_OCRAM
@@ -1717,11 +1757,62 @@ static const struct of_device_id altr_edac_a10_device_of_match[] = {
 #endif
 #ifdef CONFIG_EDAC_ALTERA_SDRAM
 	{ .compatible = "altr,sdram-edac-s10", .data = &s10_sdramecc_data },
+#endif
+#ifdef CONFIG_EDAC_ALTERA_IO96B
+	{ .compatible = "altr,socfpga-io96b0-ecc", .data = &agilex5_io96b_data },
+	{ .compatible = "altr,socfpga-io96b1-ecc", .data = &agilex5_io96b_data },
 #endif
 	{},
 };
 MODULE_DEVICE_TABLE(of, altr_edac_a10_device_of_match);
 
+/*
+ * The IO96B EDAC Device Functions differ from the rest of the
+ * ECC peripherals.
+ */
+
+#ifdef CONFIG_EDAC_ALTERA_IO96B
+static ssize_t __maybe_unused
+altr_edac_io96b_device_trig(struct file *file, const char __user *user_buf,
+			    size_t count, loff_t *ppos)
+{
+	struct edac_device_ctl_info *edac_dci = file->private_data;
+	struct altr_edac_device_dev *drvdata = edac_dci->pvt_info;
+	u8 trig_type;
+	u32 val;
+	struct arm_smccc_res result;
+
+	if (!user_buf || get_user(trig_type, user_buf))
+		return -EFAULT;
+
+	mutex_lock(&io96b_mb_mutex);
+	if (readl(drvdata->base + IO96B_CMD_RESP_STATUS_OFST))
+		writel(0, drvdata->base + IO96B_CMD_RESP_STATUS_OFST);
+
+	arm_smccc_smc(INTEL_SIP_SMC_IO96B_INJECT_ECC_ERR,
+		      (trig_type == ALTR_UE_TRIGGER_CHAR) ?
+		      IO96B_DBE_SYNDROME : IO96B_SBE_SYNDROME,
+		      IO96B_CMD_TRIG_ECC_ENJECT_OP, 0, 0, 0, 0, 0, &result);
+
+	writel(IO06B_ECC_SCRUB_INTERVAL, drvdata->base + IO96B_CMD_PARAM_0_OFST);
+	writel(IO06B_ECC_SCRUB_LEN, drvdata->base + IO96B_CMD_PARAM_1_OFST);
+	writel(IO06B_ECC_SCRUB_FULL_MEM, drvdata->base + IO96B_CMD_PARAM_2_OFST);
+	writel(IO96B_CMD_ECC_SCRUB_MODE_0, drvdata->base + IO96B_CMD_REQ_OFST);
+
+	if (readl_relaxed_poll_timeout(drvdata->base + IO96B_ECC_SCRUB_STAT0_OFST,
+				       val, !(val & IO96B_ECC_SCRUB_COMPLETE),
+				       IO96B_ECC_SCRUB_POLL_US,
+				       IO96B_ECC_SCRUB_TIMEOUT))
+		edac_printk(KERN_ALERT, EDAC_DEVICE,
+			    "IO96B ECC Scrubing timeout - Try again.\n");
+
+	writel(0, drvdata->base + IO96B_CMD_RESP_STATUS_OFST);
+	mutex_unlock(&io96b_mb_mutex);
+
+	return count;
+}
+#endif
+
 /*
  * The Arria10 EDAC Device Functions differ from the Cyclone5/Arria5
  * because 2 IRQs are shared among the all ECC peripherals. The ECC
@@ -1819,6 +1910,70 @@ altr_edac_a10_device_trig2(struct file *file, const char __user *user_buf,
 	return count;
 }
 
+static irqreturn_t io96b_irq_handler(int irq, void *dev_id)
+{
+	struct altr_edac_device_dev *dci = dev_id;
+	u32 err_word0;
+	u32 err_word1;
+	u32 cnt = 0;
+	u32 ecc_error_status;
+	u16 err_queue_overflow;
+	u16 err_count = 0;
+	bool dbe = false;
+	enum io96b_error_type error_type;
+	u32 err_queue = IO96B_ECC_ERR_ENTRIES_OFST;
+
+	ecc_error_status = readl(dci->base + IO96B_ECC_ERR_REG_OFST);
+	err_queue_overflow = ecc_error_status & GENMASK(31, 16);
+	err_count = ecc_error_status & GENMASK(15, 0);
+
+	if (!err_queue_overflow) {
+		while (cnt < err_count) {
+			err_word0 = readl(dci->base + err_queue);
+			err_word1 = readl(dci->base + (err_queue + 4));
+
+			error_type = (err_word0 & GENMASK(9, 6)) >> 6;
+			if (error_type == ECC_SINGLE_DBE || error_type == ECC_MULTI_DBE ||
+			    error_type == ECC_WRITE_LINK_DBE ||
+			    error_type == ECC_READ_LINK_DBE ||
+			    error_type == ECC_READ_LINK_RMW_DBE) {
+				edac_printk(KERN_ERR, EDAC_DEVICE,
+					    "%s: DBE: word0:0x%08X, word1:0x%08X\n",
+					    dci->edac_dev_name, err_word0, err_word1);
+				dbe = true;
+			} else {
+				edac_printk(KERN_ERR, EDAC_DEVICE,
+					    "%s: SBE: word0:0x%08X, word1:0x%08X\n",
+					    dci->edac_dev_name, err_word0, err_word1);
+				edac_device_handle_ce(dci->edac_dev, 0, 0,
+						      dci->edac_dev_name);
+			}
+			cnt++;
+			err_queue += 8;
+		}
+		if (dbe)
+			panic("\nEDAC:IO96B[Uncorrectable errors]\n");
+	} else {
+		err_queue_overflow = (err_word0 & GENMASK(9, 6)) >> 6;
+		if (error_type == ECC_SINGLE_DBE || error_type == ECC_MULTI_DBE ||
+		    error_type == ECC_WRITE_LINK_DBE ||
+		    error_type == ECC_READ_LINK_DBE ||
+		    error_type == ECC_READ_LINK_RMW_DBE) {
+			panic("\nEDAC: UE: %s: word0:0x%08X, word1:0x%08X\n",
+			      dci->edac_dev_name, err_word0, err_word1);
+		} else {
+			edac_printk(KERN_ERR, EDAC_DEVICE,
+				    "%s: Buffer Overflow SBE:0x%08X\n",
+				    dci->edac_dev_name, err_queue_overflow);
+			edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);
+		}
+	}
+
+	//Clear Queue
+	writel(IO96B_ECC_ERROR_QUEUE_CLEAR, dci->base + IO96B_CMD_REQ_OFST);
+	return IRQ_HANDLED;
+}
+
 static void altr_edac_a10_irq_handler(struct irq_desc *desc)
 {
 	int dberr, bit, sm_offset, irq_status;
@@ -1885,6 +2040,8 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 	struct resource res;
 	int edac_idx;
 	int rc = 0;
+	bool io96b0_ecc = false;
+	bool io96b1_ecc = false;
 	const struct edac_device_prv_data *prv;
 	/* Get matching node and check for valid result */
 	const struct of_device_id *pdev_id =
@@ -1902,11 +2059,15 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 
 	if (!devres_open_group(edac->dev, altr_edac_a10_device_add, GFP_KERNEL))
 		return -ENOMEM;
-
-	if (of_device_is_compatible(np, "altr,sdram-edac-s10"))
+	if (of_device_is_compatible(np, "altr,socfpga-io96b0-ecc")) {
+		io96b0_ecc = true;
+	} else if (of_device_is_compatible(np, "altr,socfpga-io96b1-ecc")) {
+		io96b1_ecc = true;
+	} else if (of_device_is_compatible(np, "altr,sdram-edac-s10")) {
 		rc = get_s10_sdram_edac_resource(np, &res);
-	else
+	} else {
 		rc = of_address_to_resource(np, 0, &res);
+	}
 
 	if (rc < 0) {
 		edac_printk(KERN_ERR, EDAC_DEVICE,
@@ -1938,10 +2099,22 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 	dci->mod_name = ecc_name;
 	dci->dev_name = ecc_name;
 
-	altdev->base = devm_ioremap_resource(edac->dev, &res);
-	if (IS_ERR(altdev->base)) {
-		rc = PTR_ERR(altdev->base);
-		goto err_release_group1;
+	if (io96b0_ecc || io96b1_ecc) {
+		rc = of_address_to_resource(np, 0, &res);
+		if (rc)
+			goto err_release_group1;
+
+		altdev->base = ioremap(res.start, resource_size(&res));
+		if (IS_ERR(altdev->base)) {
+			rc = PTR_ERR(altdev->base);
+			goto err_release_group1;
+		}
+	} else {
+		altdev->base = devm_ioremap_resource(edac->dev, &res);
+		if (IS_ERR(altdev->base)) {
+			rc = PTR_ERR(altdev->base);
+			goto err_release_group1;
+		}
 	}
 
 	/* Check specific dependencies for the module */
@@ -1951,26 +2124,70 @@ 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_ONESHOT | IRQF_TRIGGER_HIGH,
-			      ecc_name, altdev);
-	if (rc) {
-		edac_printk(KERN_ERR, EDAC_DEVICE, "No SBERR IRQ resource\n");
-		goto err_release_group1;
-	}
+	if (io96b0_ecc) {
+		altdev->io96b0_irq = altdev->edac->io96b0_irq;
+		rc = devm_request_threaded_irq(edac->dev, altdev->io96b0_irq, NULL,
+					       io96b_irq_handler, IRQF_ONESHOT,
+					       ecc_name, altdev);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "No IO96B0 IRQ resource\n");
+			goto err_release_group1;
+		}
+	} else if (io96b1_ecc) {
+		altdev->io96b1_irq = altdev->edac->io96b1_irq;
+		rc = devm_request_threaded_irq(edac->dev, altdev->io96b1_irq, NULL,
+					       io96b_irq_handler, IRQF_ONESHOT,
+					       ecc_name, altdev);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "No IO96B1 IRQ resource\n");
+			goto err_release_group1;
+		}
+	} else {
+		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_ONESHOT | IRQF_TRIGGER_HIGH, ecc_name, altdev);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "No SBERR IRQ resource\n");
+			goto err_release_group1;
+		}
 
 #ifdef CONFIG_64BIT
-	if (of_machine_is_compatible("intel,socfpga-agilex5")) {
+		if (of_machine_is_compatible("intel,socfpga-agilex5")) {
+			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_ONESHOT | IRQF_TRIGGER_HIGH,
+					      ecc_name, altdev);
+			if (rc) {
+				edac_printk(KERN_ERR, EDAC_DEVICE,
+					    "No DBERR IRQ resource\n");
+				goto err_release_group1;
+			}
+		} else {
+			/* 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");
+			edac_printk(KERN_ERR, EDAC_DEVICE, "Error allocating DBIRQ\n");
 			rc = -ENODEV;
 			goto err_release_group1;
 		}
@@ -1979,35 +2196,11 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 				      IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
 				      ecc_name, altdev);
 		if (rc) {
-			edac_printk(KERN_ERR, EDAC_DEVICE,
-				    "No DBERR IRQ resource\n");
+			edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
 			goto err_release_group1;
 		}
-	} else {
-		/* 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_ONESHOT | 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) {
@@ -2198,7 +2391,21 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
 			regmap_write(edac->ecc_mgr_map,
 				     S10_SYSMGR_UE_ADDR_OFST, 0);
 		}
+
+#ifdef CONFIG_EDAC_ALTERA_IO96B
+		edac->io96b0_irq = platform_get_irq_byname(pdev, "io96b0");
+		if (edac->io96b0_irq < 0) {
+			dev_err(&pdev->dev, "No io96b0 IRQ resource\n");
+			return edac->io96b0_irq;
+		}
+		edac->io96b1_irq = platform_get_irq_byname(pdev, "io96b1");
+		if (edac->io96b1_irq < 0) {
+			dev_err(&pdev->dev, "No io96b1 IRQ resource\n");
+			return edac->io96b1_irq;
+		}
+#endif
 	}
+
 #else
 	edac->db_irq = platform_get_irq(pdev, 1);
 	if (edac->db_irq < 0)
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index 7248d24c4908..a2c8b80eefa8 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -352,6 +352,44 @@ struct altr_sdram_mc_data {
 #define ECC_READ_EOVR                     0x2
 #define ECC_READ_EDOVR                    0x3
 
+/************ IO96B ECC defines *******/
+#define IO96B_ECC_ENABLE_INFO_OFST		0x240
+#define IO96B_ECC_SCRUB_STAT0_OFST		0x244
+#define IO96B_ECC_ERR_REG_OFST			0x300
+#define IO96B_ECC_ERR_ENTRIES_OFST		0x310
+
+#define IO96B_CMD_RESP_STATUS_OFST		0x45C
+#define IO96B_CMD_RESP_DATA_0_OFST		0x458
+#define IO96B_CMD_RESP_DATA_1_OFST		0x454
+#define IO96B_CMD_RESP_DATA_2_OFST		0x450
+#define IO96B_CMD_REQ_OFST			0x43C
+#define IO96B_CMD_PARAM_0_OFST			0x438
+#define IO96B_CMD_PARAM_1_OFST			0x434
+#define IO96B_CMD_PARAM_2_OFST			0x430
+
+#define IO96B_CMD_TRIG_ECC_ENJECT_OP		0x20040109
+#define IO96B_CMD_ECC_SCRUB_MODE_0		0x20040202
+#define IO96B_ECC_ERROR_QUEUE_CLEAR		0x20040110
+
+#define IO06B_ECC_SCRUB_INTERVAL		0x14
+#define IO06B_ECC_SCRUB_LEN			0x100
+#define IO06B_ECC_SCRUB_FULL_MEM		0x1
+
+#define IO96B_SBE_SYNDROME			0xF4
+#define IO96B_DBE_SYNDROME			0xFF
+
+#define IO96B_ECC_SCRUB_TIMEOUT		400000
+#define IO96B_ECC_SCRUB_POLL_US		500
+#define IO96B_ECC_SCRUB_COMPLETE		BIT(1)
+
+enum io96b_error_type {
+	ECC_SINGLE_DBE = 2,
+	ECC_MULTI_DBE = 3,
+	ECC_WRITE_LINK_DBE = 0xa,
+	ECC_READ_LINK_DBE = 0xc,
+	ECC_READ_LINK_RMW_DBE
+};
+
 struct altr_edac_device_dev;
 
 struct edac_device_prv_data {
@@ -384,6 +422,8 @@ struct altr_edac_device_dev {
 	struct edac_device_ctl_info *edac_dev;
 	struct device ddev;
 	int edac_idx;
+	int io96b0_irq;
+	int io96b1_irq;
 };
 
 struct altr_arria10_edac {
@@ -395,6 +435,8 @@ struct altr_arria10_edac {
 	struct irq_chip		irq_chip;
 	struct list_head	a10_ecc_devices;
 	struct notifier_block	panic_notifier;
+	int io96b0_irq;
+	int io96b1_irq;
 };
 
 #endif	/* #ifndef _ALTERA_EDAC_H */
diff --git a/include/linux/firmware/intel/stratix10-smc.h b/include/linux/firmware/intel/stratix10-smc.h
index ee80ca4bb0d0..283597022e61 100644
--- a/include/linux/firmware/intel/stratix10-smc.h
+++ b/include/linux/firmware/intel/stratix10-smc.h
@@ -620,4 +620,22 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
 #define INTEL_SIP_SMC_FCS_GET_PROVISION_DATA \
 	INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FCS_GET_PROVISION_DATA)
 
+/**
+ * Request INTEL_SIP_SMC_IO96B_INJECT_ECC_ERR
+ * Sync call to inject IO96B ECC Error.
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_FUNCID_IO96B_INJECT_ECC_ERR
+ * a1 IO96B error syndrome
+ * a2 I096B mailbox command
+ *
+ * Return status:
+ * a0 INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_STATUS_NOT_SUPPORTED or
+ *    INTEL_SIP_SMC_STATUS_ERROR
+ * a1-a3 Not used
+ */
+#define INTEL_SIP_SMC_FUNCID_IO96B_INJECT_ECC_ERR 156
+#define INTEL_SIP_SMC_IO96B_INJECT_ECC_ERR \
+		INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_IO96B_INJECT_ECC_ERR)
+
 #endif
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/6] EDAC/altera: Add support for CRAM SEU error handling on SoCFPGA
  2025-10-28  9:22 [PATCH 0/6] Add EDAC support for Agilex5 SoCFPGA niravkumarlaxmidas.rabara
                   ` (3 preceding siblings ...)
  2025-10-28  9:22 ` [PATCH 4/6] EDAC/altera: Add IO96B ECC support for Agilex5 SoCFPGA niravkumarlaxmidas.rabara
@ 2025-10-28  9:22 ` niravkumarlaxmidas.rabara
  2025-10-29  6:52   ` Krzysztof Kozlowski
  2025-10-28  9:22 ` [PATCH 6/6] EDAC: altera: Add ECC support for SDM QSPI on Agilex5 niravkumarlaxmidas.rabara
  5 siblings, 1 reply; 16+ messages in thread
From: niravkumarlaxmidas.rabara @ 2025-10-28  9:22 UTC (permalink / raw)
  To: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, bp, tony.luck
  Cc: linux-edac, devicetree, linux-kernel, Niravkumar L Rabara

From: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>

Add new EDAC driver support for detecting and handling Single Event Upset
(SEU) errors in the FPGA Configuration RAM (CRAM) on Altera SoCFPGA
devices.

The Secure Device Manager (SDM) is responsible for detecting correctable
and uncorrectable SEU errors and notifies the CPU through a dedicated
interrupt. Upon receiving the interrupt, the driver invokes an SMC call
to the ARM Trusted Firmware (ATF) to query the error status.
The ATF, in turn, communicates with the SDM via the mailbox interface to
retrieve the error details and returns to the driver.

Signed-off-by: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
---
 drivers/edac/Kconfig                         |  12 ++
 drivers/edac/altera_edac.c                   | 178 +++++++++++++++++++
 drivers/edac/altera_edac.h                   |   9 +
 include/linux/firmware/intel/stratix10-smc.h |  37 ++++
 4 files changed, 236 insertions(+)

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 33a9fccde2fe..701b15e73a39 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -477,6 +477,18 @@ config EDAC_ALTERA_SDMMC
 	  Support for error detection and correction on the
 	  Altera SDMMC FIFO Memory for Altera SoCs.
 
+config EDAC_ALTERA_CRAM_SEU
+	bool "Altera CRAM SEU"
+	depends on EDAC_ALTERA=y && 64BIT
+	help
+	  Support for error detection and correction on Altera SoCs for
+	  FPGA Configuration RAM(CRAM) Single Event Upset(SEU).
+	  The SEU errors caused by radiation or other transient events are
+	  monitored by the Secure Device Manager (SDM), which notifies the
+	  CPU through a dedicated interrupt.
+	  This driver uses an SMC interface to query the error status and
+	  report events to the EDAC framework.
+
 config EDAC_SIFIVE
 	bool "Sifive platform EDAC driver"
 	depends on EDAC=y && SIFIVE_CCACHE
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index a82c3b01be1a..ac2151c625a2 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -656,6 +656,19 @@ static const struct file_operations altr_edac_a10_device_inject_fops __maybe_unu
 	.llseek = generic_file_llseek,
 };
 
+#if IS_ENABLED(CONFIG_EDAC_ALTERA_CRAM_SEU)
+static ssize_t __maybe_unused
+altr_edac_seu_trig(struct file *file, const char __user *user_buf,
+		   size_t count, loff_t *ppos);
+
+static const struct file_operations
+altr_edac_cram_inject_fops __maybe_unused = {
+	.open = simple_open,
+	.write = altr_edac_seu_trig,
+	.llseek = generic_file_llseek,
+};
+#endif
+
 #ifdef CONFIG_EDAC_ALTERA_IO96B
 static ssize_t __maybe_unused
 altr_edac_io96b_device_trig(struct file *file, const char __user *user_buf,
@@ -1492,6 +1505,56 @@ static const struct edac_device_prv_data a10_usbecc_data = {
 
 #endif	/* CONFIG_EDAC_ALTERA_USB */
 
+#if IS_ENABLED(CONFIG_EDAC_ALTERA_CRAM_SEU)
+static irqreturn_t seu_irq_handler(int irq, void *dev_id)
+{
+	struct altr_edac_device_dev *dci = dev_id;
+	struct arm_smccc_res result;
+
+	arm_smccc_smc(INTEL_SIP_SMC_SEU_ERR_STATUS, 0,
+		      0, 0, 0, 0, 0, 0, &result);
+
+	if ((u32)result.a0) {
+		edac_printk(KERN_ERR, EDAC_DEVICE,
+			    "SEU %s: Count=0x%X, SecAddr=0x%X, ErrData=0x%X\n",
+			    ((u32)result.a2 & BIT(28)) == 0 ? "UE" : "CE",
+			    (u32)result.a0, (u32)result.a1, (u32)result.a2);
+
+		if ((u32)result.a2 & BIT(28))
+			edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);
+		else
+			edac_device_handle_ue(dci->edac_dev, 0, 0, dci->edac_dev_name);
+	}
+	return IRQ_HANDLED;
+}
+
+static ssize_t __maybe_unused
+altr_edac_seu_trig(struct file *file, const char __user *user_buf,
+		   size_t count, loff_t *ppos)
+{
+	struct edac_device_ctl_info *edac_dci = file->private_data;
+	struct altr_edac_device_dev *dev = edac_dci->pvt_info;
+	u8 trig_type;
+	struct arm_smccc_res result;
+
+	if (!user_buf || get_user(trig_type, user_buf))
+		return -EFAULT;
+
+	if (trig_type == ALTR_UE_TRIGGER_CHAR)
+		arm_smccc_smc(INTEL_SIP_SMC_SAFE_INJECT_SEU_ERR,
+			      ((u64)dev->seu.ue_msb << 32) |
+			      dev->seu.ue_lsb,
+			      2, 0, 0, 0, 0, 0, &result);
+	else
+		arm_smccc_smc(INTEL_SIP_SMC_SAFE_INJECT_SEU_ERR,
+			      ((u64)dev->seu.ce_msb << 32) |
+			      dev->seu.ce_lsb, 2, 0, 0, 0,
+			      0, 0, &result);
+
+	return count;
+}
+#endif
+
 /********************** QSPI Device Functions **********************/
 
 #ifdef CONFIG_EDAC_ALTERA_QSPI
@@ -2031,6 +2094,117 @@ static int get_s10_sdram_edac_resource(struct device_node *np,
 	return ret;
 }
 
+#if IS_ENABLED(CONFIG_EDAC_ALTERA_CRAM_SEU)
+static int altr_edac_seu_device_add(struct altr_arria10_edac *edac,
+				    struct platform_device *pdev, struct device_node *dev_node)
+{
+	struct edac_device_ctl_info *dci;
+	struct altr_edac_device_dev *altdev;
+	char *ecc_name = kstrdup(dev_node->name, GFP_KERNEL);
+	int edac_idx;
+	int seu_irq;
+	int rc = 0;
+
+	seu_irq = platform_get_irq_byname(pdev, "sdm_seu");
+	if (seu_irq < 0) {
+		dev_warn(&pdev->dev, "no %s IRQ defined\n", "sdm_seu");
+		return 0;
+	}
+
+	edac_idx = edac_device_alloc_index();
+	dci = edac_device_alloc_ctl_info(sizeof(*altdev), ecc_name,
+					 1, ecc_name, 1, 0, edac_idx);
+	if (!dci) {
+		edac_printk(KERN_ERR, EDAC_DEVICE,
+			    "%s: Unable to allocate EDAC device\n", ecc_name);
+		rc = -ENOMEM;
+		goto err_release_group;
+	}
+
+	altdev = dci->pvt_info;
+	dci->dev = edac->dev;
+	altdev->edac_dev_name = ecc_name;
+	altdev->edac_idx = edac_idx;
+	altdev->edac = edac;
+	altdev->edac_dev = dci;
+	altdev->ddev = *edac->dev;
+	dci->dev = &altdev->ddev;
+	dci->ctl_name = "Altera ECC Manager";
+	dci->mod_name = ecc_name;
+	dci->dev_name = ecc_name;
+
+	rc = of_property_read_u32(dev_node, "altr,seu-safe-inject-ce-msb",
+				  &altdev->seu.ce_msb);
+	if (rc) {
+		edac_printk(KERN_ERR, EDAC_DEVICE,
+			    "Missing - altr,seu-safe-inject-ce-msb\n");
+		return -EINVAL;
+	}
+
+	rc = of_property_read_u32(dev_node, "altr,seu-safe-inject-ce-lsb",
+				  &altdev->seu.ce_lsb);
+	if (rc) {
+		edac_printk(KERN_ERR, EDAC_DEVICE,
+			    "Missing - altr,seu-safe-inject-ce-lsb\n");
+		return -EINVAL;
+	}
+
+	rc = of_property_read_u32(dev_node, "altr,seu-safe-inject-ue-msb",
+				  &altdev->seu.ue_msb);
+	if (rc) {
+		edac_printk(KERN_ERR, EDAC_DEVICE,
+			    "Missing - altr,seu-safe-inject-ue-msb\n");
+		return -EINVAL;
+	}
+
+	rc = of_property_read_u32(dev_node, "altr,seu-safe-inject-ue-lsb",
+				  &altdev->seu.ue_lsb);
+	if (rc) {
+		edac_printk(KERN_ERR, EDAC_DEVICE,
+			    "Missing - altr,seu-safe-inject-ue-lsb\n");
+		return -EINVAL;
+	}
+
+	altdev->seu_irq = seu_irq;
+	rc = devm_request_threaded_irq(edac->dev, altdev->seu_irq, NULL,
+				       seu_irq_handler,	IRQF_ONESHOT,
+				       ecc_name, altdev);
+	if (rc) {
+		edac_printk(KERN_ERR, EDAC_DEVICE, "No SEU 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");
+		rc = -ENOMEM;
+		goto err_release_group1;
+	}
+
+	if (IS_ENABLED(CONFIG_EDAC_DEBUG)) {
+		altdev->debugfs_dir = edac_debugfs_create_dir(ecc_name);
+		if (!altdev->debugfs_dir) {
+			rc = -EBUSY;
+			goto err_release_group1;
+		}
+
+		if (!edac_debugfs_create_file("altr_trigger", 0200,
+					      altdev->debugfs_dir, dci,
+					      &altr_edac_cram_inject_fops))
+			debugfs_remove_recursive(altdev->debugfs_dir);
+	}
+	return 0;
+
+err_release_group1:
+	edac_device_free_ctl_info(dci);
+err_release_group:
+	edac_printk(KERN_ERR, EDAC_DEVICE,
+		    "%s:Error setting up EDAC device: %d\n", ecc_name, rc);
+
+	return rc;
+}
+#endif
+
 static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 				    struct device_node *np)
 {
@@ -2421,6 +2595,10 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
 
 		if (of_match_node(altr_edac_a10_device_of_match, child))
 			altr_edac_a10_device_add(edac, child);
+#if IS_ENABLED(CONFIG_EDAC_ALTERA_CRAM_SEU)
+		else if (of_device_is_compatible(child, "altr,socfpga-cram-seu"))
+			altr_edac_seu_device_add(edac, pdev, child);
+#endif
 
 #ifdef CONFIG_EDAC_ALTERA_SDRAM
 		else if (of_device_is_compatible(child, "altr,sdram-edac-a10"))
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index a2c8b80eefa8..8b475dc692e1 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -410,6 +410,13 @@ struct edac_device_prv_data {
 	bool panic;
 };
 
+struct altr_seu {
+	u32 ce_msb;
+	u32 ce_lsb;
+	u32 ue_msb;
+	u32 ue_lsb;
+};
+
 struct altr_edac_device_dev {
 	struct list_head next;
 	void __iomem *base;
@@ -424,6 +431,8 @@ struct altr_edac_device_dev {
 	int edac_idx;
 	int io96b0_irq;
 	int io96b1_irq;
+	int seu_irq;
+	struct altr_seu seu;
 };
 
 struct altr_arria10_edac {
diff --git a/include/linux/firmware/intel/stratix10-smc.h b/include/linux/firmware/intel/stratix10-smc.h
index 283597022e61..87e13683776f 100644
--- a/include/linux/firmware/intel/stratix10-smc.h
+++ b/include/linux/firmware/intel/stratix10-smc.h
@@ -620,6 +620,43 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
 #define INTEL_SIP_SMC_FCS_GET_PROVISION_DATA \
 	INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FCS_GET_PROVISION_DATA)
 
+/**
+ * Request INTEL_SIP_SMC_SEU_ERR_STATUS
+ * Sync call to get previous Double Bit ECC error information.
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_SEU_ERR_STATUS
+ * a1-7 not used
+ *
+ * Return status:
+ * a0 INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_STATUS_NOT_SUPPORTED or
+ *    INTEL_SIP_SMC_STATUS_ERROR
+ * a1 error count of response data
+ * a2 sector address of response data
+ * a3 error information
+ */
+#define INTEL_SIP_SMC_FUNCID_SEU_ERR_STATUS 153
+#define INTEL_SIP_SMC_SEU_ERR_STATUS \
+		INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_SEU_ERR_STATUS)
+
+/**
+ * Request INTEL_SIP_SMC_SAFE_INJECT_SEU_ERR
+ * Sync call to inject SEU Error.
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_FUNCID_SAFE_INJECT_SEU_ERR
+ * a1 Number of words
+ * a2-7 not used
+ *
+ * Return status:
+ * a0 INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_STATUS_NOT_SUPPORTED or
+ *    INTEL_SIP_SMC_STATUS_ERROR
+ * a1-a3 Not used
+ */
+#define INTEL_SIP_SMC_FUNCID_SAFE_INJECT_SEU_ERR 154
+#define INTEL_SIP_SMC_SAFE_INJECT_SEU_ERR \
+		INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_SAFE_INJECT_SEU_ERR)
+
 /**
  * Request INTEL_SIP_SMC_IO96B_INJECT_ECC_ERR
  * Sync call to inject IO96B ECC Error.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/6] EDAC: altera: Add ECC support for SDM QSPI on Agilex5
  2025-10-28  9:22 [PATCH 0/6] Add EDAC support for Agilex5 SoCFPGA niravkumarlaxmidas.rabara
                   ` (4 preceding siblings ...)
  2025-10-28  9:22 ` [PATCH 5/6] EDAC/altera: Add support for CRAM SEU error handling on SoCFPGA niravkumarlaxmidas.rabara
@ 2025-10-28  9:22 ` niravkumarlaxmidas.rabara
  2025-10-29  6:52   ` Krzysztof Kozlowski
  5 siblings, 1 reply; 16+ messages in thread
From: niravkumarlaxmidas.rabara @ 2025-10-28  9:22 UTC (permalink / raw)
  To: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, bp, tony.luck
  Cc: linux-edac, devicetree, linux-kernel, Niravkumar L Rabara

From: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>

On Agilex5 SoCFPGA, the Secure Device Manager(SDM) manages ECC protection
for the QSPI. Since SDM operates in secure mode, all register accesses
must go through ARM Trusted Firmware (ATF) using Secure Monitor Calls
(SMC).
This driver uses the SMC interface to initialize ECC, handle correctable
and uncorrectable error interrupts, and support error injection using
debugfs.

Signed-off-by: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
---
 drivers/edac/Kconfig       |  11 +++
 drivers/edac/altera_edac.c | 177 ++++++++++++++++++++++++++++++++++++-
 drivers/edac/altera_edac.h |   5 ++
 3 files changed, 192 insertions(+), 1 deletion(-)

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 701b15e73a39..439b823a6549 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -470,6 +470,17 @@ config EDAC_ALTERA_QSPI
 	  Support for error detection and correction on the
 	  Altera QSPI FIFO Memory for Altera SoCs.
 
+config EDAC_ALTERA_SDM_QSPI
+	bool "Altera SDM QSPI FIFO ECC"
+	depends on EDAC_ALTERA=y && ARM64 && SPI_CADENCE_QUADSPI
+	help
+	  Support for error detection and correction on the
+	  Secure Device Manager (SDM) QSPI FIFO Memory that HPS
+	  access on Agilex5 onwards platform.
+
+	  SDM QSPI ECC is always in secure mode, so access to register
+	  is through ATF using ARM Secure Monitor Call(SMC).
+
 config EDAC_ALTERA_SDMMC
 	bool "Altera SDMMC FIFO ECC"
 	depends on EDAC_ALTERA=y && MMC_DW
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index ac2151c625a2..2f2755ab2c45 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -682,6 +682,19 @@ altr_edac_io96b_inject_fops __maybe_unused = {
 };
 #endif
 
+#if IS_ENABLED(CONFIG_EDAC_ALTERA_SDM_QSPI)
+static ssize_t __maybe_unused
+altr_edac_sdm_qspi_device_trig(struct file *file, const char __user *user_buf,
+			       size_t count, loff_t *ppos);
+
+static const struct file_operations
+altr_edac_sdm_qspi_device_inject_fops __maybe_unused = {
+	.open = simple_open,
+	.write = altr_edac_sdm_qspi_device_trig,
+	.llseek = generic_file_llseek,
+};
+#endif
+
 static ssize_t __maybe_unused
 altr_edac_a10_device_trig2(struct file *file, const char __user *user_buf,
 			   size_t count, loff_t *ppos);
@@ -1585,6 +1598,104 @@ static const struct edac_device_prv_data a10_qspiecc_data = {
 
 #endif	/* CONFIG_EDAC_ALTERA_QSPI */
 
+#if IS_ENABLED(CONFIG_EDAC_ALTERA_SDM_QSPI)
+
+static ssize_t __maybe_unused
+altr_edac_sdm_qspi_device_trig(struct file *file, const char __user *user_buf,
+			       size_t count, loff_t *ppos)
+{
+	struct edac_device_ctl_info *edac_dci = file->private_data;
+	struct altr_edac_device_dev *drvdata = edac_dci->pvt_info;
+	unsigned long flags;
+	u8 trig_type;
+	struct arm_smccc_res result;
+
+	if (!user_buf || get_user(trig_type, user_buf))
+		return -EFAULT;
+
+	local_irq_save(flags);
+	if (trig_type == ALTR_UE_TRIGGER_CHAR)
+		arm_smccc_smc(INTEL_SIP_SMC_REG_WRITE,
+			      drvdata->sdm_qspi_addr + ALTR_A10_ECC_INTTEST_OFST,
+			      ALTR_A10_ECC_TDERRA, 0, 0, 0, 0, 0, &result);
+	else
+		arm_smccc_smc(INTEL_SIP_SMC_REG_WRITE,
+			      drvdata->sdm_qspi_addr + ALTR_A10_ECC_INTTEST_OFST,
+			      ALTR_A10_ECC_TSERRA, 0, 0, 0, 0, 0, &result);
+
+	/* Ensure the interrupt test bits are set */
+	wmb();
+	local_irq_restore(flags);
+
+	return count;
+}
+
+static int __init socfpga_init_sdm_qspi_ecc(struct altr_edac_device_dev *device)
+{
+	struct arm_smccc_res result;
+	u32 read_reg;
+	int limit = ALTR_A10_ECC_INIT_WATCHDOG_10US;
+
+	/* Disable ECC */
+	arm_smccc_smc(INTEL_SIP_SMC_REG_WRITE,
+		      device->sdm_qspi_addr + ALTR_A10_ECC_ERRINTENR_OFST,
+		      ALTR_A10_ECC_SERRINTEN, 0, 0, 0, 0, 0, &result);
+
+	arm_smccc_smc(INTEL_SIP_SMC_REG_READ,
+		      device->sdm_qspi_addr + ALTR_A10_ECC_CTRL_OFST,
+		      0, 0, 0, 0, 0, 0, &result);
+	read_reg = (unsigned int)result.a1 & 0x00;
+
+	arm_smccc_smc(INTEL_SIP_SMC_REG_WRITE,
+		      device->sdm_qspi_addr + ALTR_A10_ECC_CTRL_OFST,
+		      read_reg, 0, 0, 0, 0, 0, &result);
+
+	/* Ensure all writes complete */
+	wmb();
+	arm_smccc_smc(INTEL_SIP_SMC_REG_READ,
+		      device->sdm_qspi_addr + ALTR_A10_ECC_CTRL_OFST,
+		      0, 0, 0, 0, 0, 0, &result);
+	read_reg = (unsigned int)result.a1 | ALTR_A10_ECC_INITA;
+
+	arm_smccc_smc(INTEL_SIP_SMC_REG_WRITE,
+		      device->sdm_qspi_addr + ALTR_A10_ECC_CTRL_OFST,
+		      read_reg, 0, 0, 0, 0, 0, &result);
+
+	while (limit--) {
+		arm_smccc_smc(INTEL_SIP_SMC_REG_READ,
+			      device->sdm_qspi_addr + ALTR_A10_ECC_INITSTAT_OFST,
+			      0, 0, 0, 0, 0, 0, &result);
+
+		if ((unsigned int)result.a1 & ALTR_A10_ECC_INITCOMPLETEA)
+			break;
+		udelay(1);
+	}
+	if (limit <= 0)
+		return -EBUSY;
+
+	/* Enable ECC */
+	arm_smccc_smc(INTEL_SIP_SMC_REG_READ,
+		      device->sdm_qspi_addr + ALTR_A10_ECC_CTRL_OFST,
+		      0, 0, 0, 0, 0, 0, &result);
+	read_reg = (unsigned int)result.a1 | ALTR_A10_ECC_SERRINTEN;
+
+	arm_smccc_smc(INTEL_SIP_SMC_REG_WRITE,
+		      device->sdm_qspi_addr + ALTR_A10_ECC_CTRL_OFST,
+		      read_reg, 0, 0, 0, 0, 0, &result);
+
+	arm_smccc_smc(INTEL_SIP_SMC_REG_WRITE,
+		      device->sdm_qspi_addr + ALTR_A10_ECC_ERRINTEN_OFST,
+		      ALTR_A10_ECC_SERRINTEN, 0, 0, 0, 0, 0, &result);
+	return 0;
+}
+
+static const struct edac_device_prv_data a10_sdmqspiecc_data = {
+	.setup = socfpga_init_sdm_qspi_ecc,
+	.inject_fops = &altr_edac_sdm_qspi_device_inject_fops,
+};
+
+#endif	/* CONFIG_EDAC_ALTERA_SDM_QSPI */
+
 /********************* SDMMC Device Functions **********************/
 
 #ifdef CONFIG_EDAC_ALTERA_SDMMC
@@ -1815,6 +1926,10 @@ static const struct of_device_id altr_edac_a10_device_of_match[] = {
 #ifdef CONFIG_EDAC_ALTERA_QSPI
 	{ .compatible = "altr,socfpga-qspi-ecc", .data = &a10_qspiecc_data },
 #endif
+#if IS_ENABLED(CONFIG_EDAC_ALTERA_SDM_QSPI)
+	{ .compatible = "altr,socfpga-sdm-qspi-ecc",
+	  .data = &a10_sdmqspiecc_data },
+#endif
 #ifdef CONFIG_EDAC_ALTERA_SDMMC
 	{ .compatible = "altr,socfpga-sdmmc-ecc", .data = &a10_sdmmcecca_data },
 #endif
@@ -2037,6 +2152,25 @@ static irqreturn_t io96b_irq_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t sdm_qspi_irq_handler(int irq, void *dev_id)
+{
+	struct altr_edac_device_dev *dci = dev_id;
+	struct arm_smccc_res result;
+
+	if (irq == dci->sdm_qspi_sb_irq) {
+		arm_smccc_smc(INTEL_SIP_SMC_REG_WRITE,
+			      dci->sdm_qspi_addr + ALTR_A10_ECC_INTSTAT_OFST,
+			      ALTR_A10_ECC_SERRPENA, 0, 0, 0, 0, 0, &result);
+		edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);
+	} else {
+		arm_smccc_smc(INTEL_SIP_SMC_REG_WRITE,
+			      dci->sdm_qspi_addr + ALTR_A10_ECC_INTSTAT_OFST,
+			      ALTR_A10_ECC_DERRPENA, 0, 0, 0, 0, 0, &result);
+		edac_device_handle_ue(dci->edac_dev, 0, 0, dci->edac_dev_name);
+	}
+	return IRQ_HANDLED;
+}
+
 static void altr_edac_a10_irq_handler(struct irq_desc *desc)
 {
 	int dberr, bit, sm_offset, irq_status;
@@ -2214,6 +2348,7 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 	struct resource res;
 	int edac_idx;
 	int rc = 0;
+	bool sdm_qspi_ecc = false;
 	bool io96b0_ecc = false;
 	bool io96b1_ecc = false;
 	const struct edac_device_prv_data *prv;
@@ -2237,6 +2372,8 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 		io96b0_ecc = true;
 	} else if (of_device_is_compatible(np, "altr,socfpga-io96b1-ecc")) {
 		io96b1_ecc = true;
+	} else if (of_device_is_compatible(np, "altr,socfpga-sdm-qspi-ecc")) {
+		sdm_qspi_ecc = true;
 	} else if (of_device_is_compatible(np, "altr,sdram-edac-s10")) {
 		rc = get_s10_sdram_edac_resource(np, &res);
 	} else {
@@ -2283,6 +2420,13 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 			rc = PTR_ERR(altdev->base);
 			goto err_release_group1;
 		}
+	} else if (sdm_qspi_ecc) {
+		altdev->sdm_qspi_addr =
+				(u32)of_translate_address(np,
+							  of_get_address(np,
+									 0,
+									 NULL,
+									 NULL));
 	} else {
 		altdev->base = devm_ioremap_resource(edac->dev, &res);
 		if (IS_ERR(altdev->base)) {
@@ -2298,7 +2442,24 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 			goto err_release_group1;
 	}
 
-	if (io96b0_ecc) {
+	if (sdm_qspi_ecc) {
+		altdev->sdm_qspi_sb_irq = altdev->edac->sdm_qspi_sb_irq;
+		rc = devm_request_threaded_irq(edac->dev, altdev->sdm_qspi_sb_irq, NULL,
+					       sdm_qspi_irq_handler, IRQF_ONESHOT,
+					       ecc_name, altdev);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "No SDM QSPI SBE IRQ resource\n");
+			goto err_release_group1;
+		}
+		altdev->sdm_qspi_db_irq = altdev->edac->sdm_qspi_db_irq;
+		rc = devm_request_threaded_irq(edac->dev, altdev->sdm_qspi_db_irq, NULL,
+					       sdm_qspi_irq_handler, IRQF_ONESHOT,
+					       ecc_name, altdev);
+		if (rc) {
+			edac_printk(KERN_ERR, EDAC_DEVICE, "No SDM QSPI DBE IRQ resource\n");
+			goto err_release_group1;
+		}
+	} else if (io96b0_ecc) {
 		altdev->io96b0_irq = altdev->edac->io96b0_irq;
 		rc = devm_request_threaded_irq(edac->dev, altdev->io96b0_irq, NULL,
 					       io96b_irq_handler, IRQF_ONESHOT,
@@ -2578,6 +2739,20 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
 			return edac->io96b1_irq;
 		}
 #endif
+
+#if IS_ENABLED(CONFIG_EDAC_ALTERA_SDM_QSPI)
+		edac->sdm_qspi_sb_irq = platform_get_irq_byname(pdev, "sdm_qspi_sbe");
+		if (edac->sdm_qspi_sb_irq < 0) {
+			dev_err(&pdev->dev, "no %s IRQ defined\n", "sdm_qspi_sbe");
+			return edac->sdm_qspi_sb_irq;
+		}
+
+		edac->sdm_qspi_db_irq = platform_get_irq_byname(pdev, "sdm_qspi_dbe");
+		if (edac->sdm_qspi_db_irq < 0) {
+			dev_err(&pdev->dev, "no %s IRQ defined\n", "sdm_qspi_dbe");
+			return edac->sdm_qspi_db_irq;
+		}
+#endif
 	}
 
 #else
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index 8b475dc692e1..e537304522fb 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -431,6 +431,9 @@ struct altr_edac_device_dev {
 	int edac_idx;
 	int io96b0_irq;
 	int io96b1_irq;
+	int sdm_qspi_sb_irq;
+	int sdm_qspi_db_irq;
+	u32 sdm_qspi_addr;
 	int seu_irq;
 	struct altr_seu seu;
 };
@@ -446,6 +449,8 @@ struct altr_arria10_edac {
 	struct notifier_block	panic_notifier;
 	int io96b0_irq;
 	int io96b1_irq;
+	int sdm_qspi_sb_irq;
+	int sdm_qspi_db_irq;
 };
 
 #endif	/* #ifndef _ALTERA_EDAC_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/6] dt-bindings: edac: altera: Document additional ECC instances
  2025-10-28  9:22 ` [PATCH 1/6] dt-bindings: edac: altera: Document additional ECC instances niravkumarlaxmidas.rabara
@ 2025-10-29  6:50   ` Krzysztof Kozlowski
  2025-10-31  8:01     ` Niravkumar L Rabara
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-29  6:50 UTC (permalink / raw)
  To: niravkumarlaxmidas.rabara
  Cc: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, bp, tony.luck,
	linux-edac, devicetree, linux-kernel

On Tue, Oct 28, 2025 at 05:22:27PM +0800, niravkumarlaxmidas.rabara@altera.com wrote:
> From: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
> 
> Add support for Secure Device Manager(SDM) QSPI ECC, IO96B memory
> controller ECC and Configuration RAM(CRAM) Single Event Upset(SEU).
> 
> Add interrupt-names property and increase interrupts maxItems from 2 to 7
> to accommodate additional interrupts.
> 
> Signed-off-by: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
> ---
>  .../edac/altr,socfpga-ecc-manager.yaml        | 77 ++++++++++++++++++-
>  1 file changed, 76 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml b/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml
> index 3d787dea0f14..5e0c08a15ab9 100644
> --- a/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml
> +++ b/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml
> @@ -33,7 +33,13 @@ properties:
>  
>    interrupts:
>      minItems: 1
> -    maxItems: 2
> +    maxItems: 7

No, list the interrupts instead. Your commit msg must clearly explain
why exception of not-fixed length/entries is justified.

See writing bindings.

> +
> +  interrupt-names:
> +    items:
> +      enum: [global_sbe, global_dbe, io96b0, io96b1, sdm_qspi_sbe, sdm_qspi_dbe, sdm_seu]

Nope, list the items instead. Please do not come up with some custom
syntax.

> +    minItems: 1
> +    maxItems: 7
>  
>    interrupt-controller: true
>  
> @@ -70,6 +76,41 @@ properties:
>        - interrupts
>        - altr,sdr-syscon
>  
> +  cram-seu:

Missing description, so difficult to say what is here.
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
If you cannot find a name matching your device, please check in kernel
sources for similar cases or you can grow the spec (via pull request to
DT spec repo).

> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      compatible:
> +        items:
> +          - const: altr,socfpga-cram-seu

Why do you need compatible?

> +
> +      reg:
> +        maxItems: 1

So you created child node only for reg? No, fold it into parent.

You also forgot to update the example.

> +
> +      altr,seu-safe-inject-ce-msb:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: MSB of error injection command for Correctable Error
> +
> +      altr,seu-safe-inject-ce-lsb:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: LSB of error injection command for Correctable Error
> +
> +      altr,seu-safe-inject-ue-msb:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: MSB of error injection command for Uncorrectable Error
> +
> +      altr,seu-safe-inject-ue-lsb:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: LSB of error injection command for Uncorrectable Error

How are these board-level properties?

> +
> +    required:
> +      - compatible
> +      - altr,seu-safe-inject-ce-msb
> +      - altr,seu-safe-inject-ce-lsb
> +      - altr,seu-safe-inject-ue-msb
> +      - altr,seu-safe-inject-ue-lsb
> +
>  patternProperties:
>    "^ocram-ecc@[a-f0-9]+$":
>      type: object
> @@ -191,6 +232,40 @@ patternProperties:
>        - interrupts
>        - altr,ecc-parent
>  
> +  "^sdm-qspi-ecc@[a-f0-9]+$":
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      compatible:
> +        items:
> +          - const: altr,socfpga-sdm-qspi-ecc

No, drop.

> +
> +      reg:
> +        maxItems: 1
> +
> +    required:
> +      - compatible
> +      - reg

No point for empty children. One reg is not justification for having a
child.

> +
> +  "^io96b[0-9]-ecc@[a-f0-9]+$":

You need to stop coming with random node names. Nothing explains why you
need children, why these are not part of parent node.

> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      compatible:
> +        items:
> +          - enum:
> +              - altr,socfpga-io96b0-ecc
> +              - altr,socfpga-io96b1-ecc

Plus all your compatibles have WRONG format. See writing bindings and
numerouse presentations - you always must use SoC specific compatible.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/6] arm64: dts: agilex5: Add ECC manager and submodule nodes
  2025-10-28  9:22 ` [PATCH 2/6] arm64: dts: agilex5: Add ECC manager and submodule nodes niravkumarlaxmidas.rabara
@ 2025-10-29  6:51   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-29  6:51 UTC (permalink / raw)
  To: niravkumarlaxmidas.rabara
  Cc: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, bp, tony.luck,
	linux-edac, devicetree, linux-kernel

On Tue, Oct 28, 2025 at 05:22:28PM +0800, niravkumarlaxmidas.rabara@altera.com wrote:
> From: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
> 
> Add the ECC manager (eccmgr) node and its associated ECC submodules to the
> Agilex5 SoCFPGA device tree. The eccmgr node serves as a logical parent to
> group various ECC hardware instances, including those for USB, Ethernet,
> OCRAM, IO96B memory controllers, Secure Device Manager (SDM) QSPI, and
> Configuration RAM (CRAM) Single Event Upset (SEU) subsystems.
> 
> Signed-off-by: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
> ---
>  .../arm64/boot/dts/intel/socfpga_agilex5.dtsi | 98 +++++++++++++++++++
>  1 file changed, 98 insertions(+)

DTS cannot be the second patch. Organize your patchset correctly, see
submitting patches.


> 
> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> index 04e99cd7e74b..5ea7a506d3d2 100644
> --- a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> @@ -428,6 +428,104 @@ usb0: usb@10b00000 {
>  			status = "disabled";
>  		};
>  
> +		eccmgr {
> +			compatible = "altr,socfpga-a10-ecc-manager";
> +			altr,sysmgr-syscon = <&sysmgr>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 241 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 95 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 120 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "global_sbe", "global_dbe", "io96b0" , "io96b1",
> +					  "sdm_qspi_sbe", "sdm_qspi_dbe", "sdm_seu";
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			ranges;
> +
> +			ocram-ecc@108cc000 {
> +				compatible = "altr,socfpga-a10-ocram-ecc";
> +				reg = <0x108cc000 0x100>;
> +				interrupts = <1 IRQ_TYPE_LEVEL_HIGH>, <33 IRQ_TYPE_LEVEL_HIGH>;
> +			};
> +
> +			usb0-ecc@108c4000 {
> +				compatible = "altr,socfpga-usb-ecc";
> +				reg = <0x108c4000 0x100>;
> +				altr,ecc-parent = <&usb0>;
> +				interrupts = <2 IRQ_TYPE_LEVEL_HIGH>, <34 IRQ_TYPE_LEVEL_HIGH>;
> +			};
> +
> +			emac0-rx-ecc@108c0000 {
> +				compatible = "altr,socfpga-eth-mac-ecc";
> +				reg = <0x108c0000 0x100>;
> +				altr,ecc-parent = <&gmac0>;
> +				interrupts = <4 IRQ_TYPE_LEVEL_HIGH>, <38 IRQ_TYPE_LEVEL_HIGH>;
> +			};
> +
> +			emac0-tx-ecc@108c0400 {
> +				compatible = "altr,socfpga-eth-mac-ecc";
> +				reg = <0x108c0400 0x100>;
> +				altr,ecc-parent = <&gmac0>;
> +				interrupts = <5 IRQ_TYPE_LEVEL_HIGH>, <37 IRQ_TYPE_LEVEL_HIGH>;
> +			};
> +
> +			emac1-rx-ecc@108c0800 {
> +				compatible = "altr,socfpga-eth-mac-ecc";
> +				reg = <0x108c0800 0x100>;
> +				altr,ecc-parent = <&gmac1>;
> +				interrupts = <6 IRQ_TYPE_LEVEL_HIGH>, <38 IRQ_TYPE_LEVEL_HIGH>;
> +			};
> +
> +			emac1-tx-ecc@108c0c00 {
> +				compatible = "altr,socfpga-eth-mac-ecc";
> +				reg = <0x108c0c00 0x100>;
> +				altr,ecc-parent = <&gmac1>;
> +				interrupts = <7 IRQ_TYPE_LEVEL_HIGH>, <39 IRQ_TYPE_LEVEL_HIGH>;
> +			};
> +
> +			emac2-rx-ecc@108c1000 {
> +				compatible = "altr,socfpga-eth-mac-ecc";
> +				reg = <0x108c1000 0x100>;
> +				altr,ecc-parent = <&gmac2>;
> +				interrupts = <8 IRQ_TYPE_LEVEL_HIGH>, <40 IRQ_TYPE_LEVEL_HIGH>;
> +			};
> +
> +			emac2-tx-ecc@108c1400 {
> +				compatible = "altr,socfpga-eth-mac-ecc";
> +				reg = <0x108c1400 0x100>;
> +				altr,ecc-parent = <&gmac2>;
> +				interrupts = <9 IRQ_TYPE_LEVEL_HIGH>, <41 IRQ_TYPE_LEVEL_HIGH>;
> +			};
> +
> +			io96b0-ecc@18400000 {
> +				compatible = "altr,socfpga-io96b0-ecc";
> +				reg = <0x18400000 0x1000>;

Could not express more: NAK. Completely pointless node with pointless
name.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/6] EDAC: altera: Add ECC support for SDM QSPI on Agilex5
  2025-10-28  9:22 ` [PATCH 6/6] EDAC: altera: Add ECC support for SDM QSPI on Agilex5 niravkumarlaxmidas.rabara
@ 2025-10-29  6:52   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-29  6:52 UTC (permalink / raw)
  To: niravkumarlaxmidas.rabara
  Cc: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, bp, tony.luck,
	linux-edac, devicetree, linux-kernel

On Tue, Oct 28, 2025 at 05:22:32PM +0800, niravkumarlaxmidas.rabara@altera.com wrote:
> From: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
> 
> On Agilex5 SoCFPGA, the Secure Device Manager(SDM) manages ECC protection
> for the QSPI. Since SDM operates in secure mode, all register accesses
> must go through ARM Trusted Firmware (ATF) using Secure Monitor Calls
> (SMC).
> This driver uses the SMC interface to initialize ECC, handle correctable
> and uncorrectable error interrupts, and support error injection using
> debugfs.
> 
> Signed-off-by: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>


How in one patchset you cannot get same subject prefix right?

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/6] EDAC/altera: Add support for CRAM SEU error handling on SoCFPGA
  2025-10-28  9:22 ` [PATCH 5/6] EDAC/altera: Add support for CRAM SEU error handling on SoCFPGA niravkumarlaxmidas.rabara
@ 2025-10-29  6:52   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-29  6:52 UTC (permalink / raw)
  To: niravkumarlaxmidas.rabara
  Cc: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, bp, tony.luck,
	linux-edac, devicetree, linux-kernel

On Tue, Oct 28, 2025 at 05:22:31PM +0800, niravkumarlaxmidas.rabara@altera.com wrote:
> From: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
> 
> Add new EDAC driver support for detecting and handling Single Event Upset
> (SEU) errors in the FPGA Configuration RAM (CRAM) on Altera SoCFPGA
> devices.
> 
> The Secure Device Manager (SDM) is responsible for detecting correctable
> and uncorrectable SEU errors and notifies the CPU through a dedicated
> interrupt. Upon receiving the interrupt, the driver invokes an SMC call
> to the ARM Trusted Firmware (ATF) to query the error status.
> The ATF, in turn, communicates with the SDM via the mailbox interface to
> retrieve the error details and returns to the driver.
> 
> Signed-off-by: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
> ---
>  drivers/edac/Kconfig                         |  12 ++
>  drivers/edac/altera_edac.c                   | 178 +++++++++++++++++++
>  drivers/edac/altera_edac.h                   |   9 +
>  include/linux/firmware/intel/stratix10-smc.h |  37 ++++
>  4 files changed, 236 insertions(+)
> 
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 33a9fccde2fe..701b15e73a39 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -477,6 +477,18 @@ config EDAC_ALTERA_SDMMC
>  	  Support for error detection and correction on the
>  	  Altera SDMMC FIFO Memory for Altera SoCs.
>  
> +config EDAC_ALTERA_CRAM_SEU
> +	bool "Altera CRAM SEU"
> +	depends on EDAC_ALTERA=y && 64BIT
> +	help
> +	  Support for error detection and correction on Altera SoCs for
> +	  FPGA Configuration RAM(CRAM) Single Event Upset(SEU).
> +	  The SEU errors caused by radiation or other transient events are
> +	  monitored by the Secure Device Manager (SDM), which notifies the
> +	  CPU through a dedicated interrupt.
> +	  This driver uses an SMC interface to query the error status and
> +	  report events to the EDAC framework.
> +
>  config EDAC_SIFIVE
>  	bool "Sifive platform EDAC driver"
>  	depends on EDAC=y && SIFIVE_CCACHE
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index a82c3b01be1a..ac2151c625a2 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c
> @@ -656,6 +656,19 @@ static const struct file_operations altr_edac_a10_device_inject_fops __maybe_unu
>  	.llseek = generic_file_llseek,
>  };
>  
> +#if IS_ENABLED(CONFIG_EDAC_ALTERA_CRAM_SEU)
> +static ssize_t __maybe_unused
> +altr_edac_seu_trig(struct file *file, const char __user *user_buf,
> +		   size_t count, loff_t *ppos);
> +
> +static const struct file_operations
> +altr_edac_cram_inject_fops __maybe_unused = {
> +	.open = simple_open,
> +	.write = altr_edac_seu_trig,
> +	.llseek = generic_file_llseek,
> +};
> +#endif
> +
>  #ifdef CONFIG_EDAC_ALTERA_IO96B
>  static ssize_t __maybe_unused
>  altr_edac_io96b_device_trig(struct file *file, const char __user *user_buf,
> @@ -1492,6 +1505,56 @@ static const struct edac_device_prv_data a10_usbecc_data = {
>  
>  #endif	/* CONFIG_EDAC_ALTERA_USB */
>  
> +#if IS_ENABLED(CONFIG_EDAC_ALTERA_CRAM_SEU)
> +static irqreturn_t seu_irq_handler(int irq, void *dev_id)
> +{
> +	struct altr_edac_device_dev *dci = dev_id;
> +	struct arm_smccc_res result;
> +
> +	arm_smccc_smc(INTEL_SIP_SMC_SEU_ERR_STATUS, 0,
> +		      0, 0, 0, 0, 0, 0, &result);
> +
> +	if ((u32)result.a0) {
> +		edac_printk(KERN_ERR, EDAC_DEVICE,
> +			    "SEU %s: Count=0x%X, SecAddr=0x%X, ErrData=0x%X\n",
> +			    ((u32)result.a2 & BIT(28)) == 0 ? "UE" : "CE",
> +			    (u32)result.a0, (u32)result.a1, (u32)result.a2);
> +
> +		if ((u32)result.a2 & BIT(28))
> +			edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);
> +		else
> +			edac_device_handle_ue(dci->edac_dev, 0, 0, dci->edac_dev_name);
> +	}
> +	return IRQ_HANDLED;
> +}
> +
> +static ssize_t __maybe_unused
> +altr_edac_seu_trig(struct file *file, const char __user *user_buf,
> +		   size_t count, loff_t *ppos)
> +{
> +	struct edac_device_ctl_info *edac_dci = file->private_data;
> +	struct altr_edac_device_dev *dev = edac_dci->pvt_info;
> +	u8 trig_type;
> +	struct arm_smccc_res result;
> +
> +	if (!user_buf || get_user(trig_type, user_buf))
> +		return -EFAULT;
> +
> +	if (trig_type == ALTR_UE_TRIGGER_CHAR)
> +		arm_smccc_smc(INTEL_SIP_SMC_SAFE_INJECT_SEU_ERR,
> +			      ((u64)dev->seu.ue_msb << 32) |
> +			      dev->seu.ue_lsb,
> +			      2, 0, 0, 0, 0, 0, &result);
> +	else
> +		arm_smccc_smc(INTEL_SIP_SMC_SAFE_INJECT_SEU_ERR,
> +			      ((u64)dev->seu.ce_msb << 32) |
> +			      dev->seu.ce_lsb, 2, 0, 0, 0,
> +			      0, 0, &result);
> +
> +	return count;
> +}
> +#endif
> +
>  /********************** QSPI Device Functions **********************/
>  
>  #ifdef CONFIG_EDAC_ALTERA_QSPI
> @@ -2031,6 +2094,117 @@ static int get_s10_sdram_edac_resource(struct device_node *np,
>  	return ret;
>  }
>  
> +#if IS_ENABLED(CONFIG_EDAC_ALTERA_CRAM_SEU)
> +static int altr_edac_seu_device_add(struct altr_arria10_edac *edac,
> +				    struct platform_device *pdev, struct device_node *dev_node)
> +{
> +	struct edac_device_ctl_info *dci;
> +	struct altr_edac_device_dev *altdev;
> +	char *ecc_name = kstrdup(dev_node->name, GFP_KERNEL);
> +	int edac_idx;
> +	int seu_irq;
> +	int rc = 0;
> +
> +	seu_irq = platform_get_irq_byname(pdev, "sdm_seu");
> +	if (seu_irq < 0) {
> +		dev_warn(&pdev->dev, "no %s IRQ defined\n", "sdm_seu");
> +		return 0;
> +	}
> +
> +	edac_idx = edac_device_alloc_index();
> +	dci = edac_device_alloc_ctl_info(sizeof(*altdev), ecc_name,
> +					 1, ecc_name, 1, 0, edac_idx);
> +	if (!dci) {
> +		edac_printk(KERN_ERR, EDAC_DEVICE,
> +			    "%s: Unable to allocate EDAC device\n", ecc_name);

NAK, you never print errors on ENOMEM.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/6] EDAC/altera: Add IO96B ECC support for Agilex5 SoCFPGA
  2025-10-28  9:22 ` [PATCH 4/6] EDAC/altera: Add IO96B ECC support for Agilex5 SoCFPGA niravkumarlaxmidas.rabara
@ 2025-10-30 14:30   ` Borislav Petkov
  2025-10-31 11:52     ` Niravkumar L Rabara
  2025-11-04  3:46   ` kernel test robot
  1 sibling, 1 reply; 16+ messages in thread
From: Borislav Petkov @ 2025-10-30 14:30 UTC (permalink / raw)
  To: niravkumarlaxmidas.rabara
  Cc: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, tony.luck,
	linux-edac, devicetree, linux-kernel

On Tue, Oct 28, 2025 at 05:22:30PM +0800, niravkumarlaxmidas.rabara@altera.com wrote:
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 39352b9b7a7e..33a9fccde2fe 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -410,6 +410,16 @@ config EDAC_ALTERA_SDRAM
>  	  preloader must initialize the SDRAM before loading
>  	  the kernel.
>  
> +config EDAC_ALTERA_IO96B
> +	bool "Altera I096B ECC"

Is this and the other new Kconfig symbols you're adding absolutely needed?

IOW, why can't the driver simply load on that new hw without needing Kconfig
symbols at all?

What are they really saving?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/6] dt-bindings: edac: altera: Document additional ECC instances
  2025-10-29  6:50   ` Krzysztof Kozlowski
@ 2025-10-31  8:01     ` Niravkumar L Rabara
  0 siblings, 0 replies; 16+ messages in thread
From: Niravkumar L Rabara @ 2025-10-31  8:01 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, bp, tony.luck,
	linux-edac, devicetree, linux-kernel



On 29/10/2025 2:50 pm, Krzysztof Kozlowski wrote:
> On Tue, Oct 28, 2025 at 05:22:27PM +0800,niravkumarlaxmidas.rabara@altera.com wrote:
>> From: Niravkumar L Rabara<niravkumarlaxmidas.rabara@altera.com>
>>
>> Add support for Secure Device Manager(SDM) QSPI ECC, IO96B memory
>> controller ECC and Configuration RAM(CRAM) Single Event Upset(SEU).
>>
>> Add interrupt-names property and increase interrupts maxItems from 2 to 7
>> to accommodate additional interrupts.
>>
>> Signed-off-by: Niravkumar L Rabara<niravkumarlaxmidas.rabara@altera.com>
>> ---
>>   .../edac/altr,socfpga-ecc-manager.yaml        | 77 ++++++++++++++++++-
>>   1 file changed, 76 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml b/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml
>> index 3d787dea0f14..5e0c08a15ab9 100644
>> --- a/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml
>> +++ b/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml
>> @@ -33,7 +33,13 @@ properties:
>>   
>>     interrupts:
>>       minItems: 1
>> -    maxItems: 2
>> +    maxItems: 7
> No, list the interrupts instead. Your commit msg must clearly explain
> why exception of not-fixed length/entries is justified.
> 
> See writing bindings.
> 
>> +
>> +  interrupt-names:
>> +    items:
>> +      enum: [global_sbe, global_dbe, io96b0, io96b1, sdm_qspi_sbe, sdm_qspi_dbe, sdm_seu]
> Nope, list the items instead. Please do not come up with some custom
> syntax.
> 
>> +    minItems: 1
>> +    maxItems: 7
>>   
>>     interrupt-controller: true
>>   
>> @@ -70,6 +76,41 @@ properties:
>>         - interrupts
>>         - altr,sdr-syscon
>>   
>> +  cram-seu:
> Missing description, so difficult to say what is here.
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2- 
> devicetree-basics.html#generic-names-recommendation
> If you cannot find a name matching your device, please check in kernel
> sources for similar cases or you can grow the spec (via pull request to
> DT spec repo).
> 
>> +    type: object
>> +    additionalProperties: false
>> +
>> +    properties:
>> +      compatible:
>> +        items:
>> +          - const: altr,socfpga-cram-seu
> Why do you need compatible?
> 
>> +
>> +      reg:
>> +        maxItems: 1
> So you created child node only for reg? No, fold it into parent.
> 
> You also forgot to update the example.
> 
>> +
>> +      altr,seu-safe-inject-ce-msb:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description: MSB of error injection command for Correctable Error
>> +
>> +      altr,seu-safe-inject-ce-lsb:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description: LSB of error injection command for Correctable Error
>> +
>> +      altr,seu-safe-inject-ue-msb:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description: MSB of error injection command for Uncorrectable Error
>> +
>> +      altr,seu-safe-inject-ue-lsb:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description: LSB of error injection command for Uncorrectable Error
> How are these board-level properties?
> 
>> +
>> +    required:
>> +      - compatible
>> +      - altr,seu-safe-inject-ce-msb
>> +      - altr,seu-safe-inject-ce-lsb
>> +      - altr,seu-safe-inject-ue-msb
>> +      - altr,seu-safe-inject-ue-lsb
>> +
>>   patternProperties:
>>     "^ocram-ecc@[a-f0-9]+$":
>>       type: object
>> @@ -191,6 +232,40 @@ patternProperties:
>>         - interrupts
>>         - altr,ecc-parent
>>   
>> +  "^sdm-qspi-ecc@[a-f0-9]+$":
>> +    type: object
>> +    additionalProperties: false
>> +
>> +    properties:
>> +      compatible:
>> +        items:
>> +          - const: altr,socfpga-sdm-qspi-ecc
> No, drop.
> 
>> +
>> +      reg:
>> +        maxItems: 1
>> +
>> +    required:
>> +      - compatible
>> +      - reg
> No point for empty children. One reg is not justification for having a
> child.
> 
>> +
>> +  "^io96b[0-9]-ecc@[a-f0-9]+$":
> You need to stop coming with random node names. Nothing explains why you
> need children, why these are not part of parent node.
> 
>> +    type: object
>> +    additionalProperties: false
>> +
>> +    properties:
>> +      compatible:
>> +        items:
>> +          - enum:
>> +              - altr,socfpga-io96b0-ecc
>> +              - altr,socfpga-io96b1-ecc
> Plus all your compatibles have WRONG format. See writing bindings and
> numerouse presentations - you always must use SoC specific compatible.
> 
> Best regards,
> Krzysztof


Hi Krzysztof,

Thanks for the review and feedback on the patch series.
I’ll go through your comments and address them in the next patch revision.

Thanks,
Nirav




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/6] EDAC/altera: Add IO96B ECC support for Agilex5 SoCFPGA
  2025-10-30 14:30   ` Borislav Petkov
@ 2025-10-31 11:52     ` Niravkumar L Rabara
  2025-11-02 19:25       ` Borislav Petkov
  0 siblings, 1 reply; 16+ messages in thread
From: Niravkumar L Rabara @ 2025-10-31 11:52 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, tony.luck,
	linux-edac, devicetree, linux-kernel



On 30/10/2025 10:30 pm, Borislav Petkov wrote:
> On Tue, Oct 28, 2025 at 05:22:30PM +0800,niravkumarlaxmidas.rabara@altera.com wrote:
>> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
>> index 39352b9b7a7e..33a9fccde2fe 100644
>> --- a/drivers/edac/Kconfig
>> +++ b/drivers/edac/Kconfig
>> @@ -410,6 +410,16 @@ config EDAC_ALTERA_SDRAM
>>   	  preloader must initialize the SDRAM before loading
>>   	  the kernel.
>>   
>> +config EDAC_ALTERA_IO96B
>> +	bool "Altera I096B ECC"
> Is this and the other new Kconfig symbols you're adding absolutely needed?
> 
> IOW, why can't the driver simply load on that new hw without needing Kconfig
> symbols at all?
> 
> What are they really saving?
> 
> Thx.
> 
> -- Regards/Gruss, Boris.


Hi Boris,

Thanks for your review.
Your point is absolutely valid — I was initially hesitant to introduce a 
different flow in the common altera_edac.c driver, so I followed the 
existing architecture where each ECC device has its own Kconfig entry 
and corresponding #ifdef blocks in the code.

In terms of savings, this approach mainly avoids compiling code based on 
Kconfig selection.

If the preferred approach is to detect and handle the new hardware 
without adding a separate Kconfig symbol, I’ll revise the patch accordingly.

In next version, I also need to address Krzysztof’s review comments 
regarding the introduction of new ECC device names in the DT bindings.

Thanks,
Nirav





^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/6] EDAC/altera: Add IO96B ECC support for Agilex5 SoCFPGA
  2025-10-31 11:52     ` Niravkumar L Rabara
@ 2025-11-02 19:25       ` Borislav Petkov
  0 siblings, 0 replies; 16+ messages in thread
From: Borislav Petkov @ 2025-11-02 19:25 UTC (permalink / raw)
  To: Niravkumar L Rabara
  Cc: dinguyen, matthew.gerlach, robh, krzk+dt, conor+dt, tony.luck,
	linux-edac, devicetree, linux-kernel

On Fri, Oct 31, 2025 at 07:52:41PM +0800, Niravkumar L Rabara wrote:
> In terms of savings, this approach mainly avoids compiling code based on
> Kconfig selection.

That would be the main argument which determines whether those new Kconfig
options are needed. Read: the kernel has waaaay tooo many Kconfig options so
if not absolutely necessary, do not add more pls.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/6] EDAC/altera: Add IO96B ECC support for Agilex5 SoCFPGA
  2025-10-28  9:22 ` [PATCH 4/6] EDAC/altera: Add IO96B ECC support for Agilex5 SoCFPGA niravkumarlaxmidas.rabara
  2025-10-30 14:30   ` Borislav Petkov
@ 2025-11-04  3:46   ` kernel test robot
  1 sibling, 0 replies; 16+ messages in thread
From: kernel test robot @ 2025-11-04  3:46 UTC (permalink / raw)
  To: niravkumarlaxmidas.rabara, dinguyen, matthew.gerlach, robh,
	krzk+dt, conor+dt, bp, tony.luck
  Cc: llvm, oe-kbuild-all, linux-edac, devicetree, linux-kernel,
	Niravkumar L Rabara

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on ras/edac-for-next]
[also build test WARNING on robh/for-next linus/master v6.18-rc4 next-20251103]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/niravkumarlaxmidas-rabara-altera-com/dt-bindings-edac-altera-Document-additional-ECC-instances/20251028-173250
base:   https://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git edac-for-next
patch link:    https://lore.kernel.org/r/20251028092232.773991-5-niravkumarlaxmidas.rabara%40altera.com
patch subject: [PATCH 4/6] EDAC/altera: Add IO96B ECC support for Agilex5 SoCFPGA
config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20251104/202511041157.q0be68K1-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project d2625a438020ad35330cda29c3def102c1687b1b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251104/202511041157.q0be68K1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511041157.q0be68K1-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/edac/altera_edac.c:1957:25: warning: variable 'err_word0' is uninitialized when used here [-Wuninitialized]
    1957 |                 err_queue_overflow = (err_word0 & GENMASK(9, 6)) >> 6;
         |                                       ^~~~~~~~~
   drivers/edac/altera_edac.c:1916:15: note: initialize the variable 'err_word0' to silence this warning
    1916 |         u32 err_word0;
         |                      ^
         |                       = 0
>> drivers/edac/altera_edac.c:1963:41: warning: variable 'err_word1' is uninitialized when used here [-Wuninitialized]
    1963 |                               dci->edac_dev_name, err_word0, err_word1);
         |                                                              ^~~~~~~~~
   drivers/edac/altera_edac.c:1917:15: note: initialize the variable 'err_word1' to silence this warning
    1917 |         u32 err_word1;
         |                      ^
         |                       = 0
>> drivers/edac/altera_edac.c:1958:7: warning: variable 'error_type' is uninitialized when used here [-Wuninitialized]
    1958 |                 if (error_type == ECC_SINGLE_DBE || error_type == ECC_MULTI_DBE ||
         |                     ^~~~~~~~~~
   drivers/edac/altera_edac.c:1923:2: note: variable 'error_type' is declared here
    1923 |         enum io96b_error_type error_type;
         |         ^
   3 warnings generated.


vim +/err_word0 +1957 drivers/edac/altera_edac.c

  1912	
  1913	static irqreturn_t io96b_irq_handler(int irq, void *dev_id)
  1914	{
  1915		struct altr_edac_device_dev *dci = dev_id;
  1916		u32 err_word0;
  1917		u32 err_word1;
  1918		u32 cnt = 0;
  1919		u32 ecc_error_status;
  1920		u16 err_queue_overflow;
  1921		u16 err_count = 0;
  1922		bool dbe = false;
  1923		enum io96b_error_type error_type;
  1924		u32 err_queue = IO96B_ECC_ERR_ENTRIES_OFST;
  1925	
  1926		ecc_error_status = readl(dci->base + IO96B_ECC_ERR_REG_OFST);
  1927		err_queue_overflow = ecc_error_status & GENMASK(31, 16);
  1928		err_count = ecc_error_status & GENMASK(15, 0);
  1929	
  1930		if (!err_queue_overflow) {
  1931			while (cnt < err_count) {
  1932				err_word0 = readl(dci->base + err_queue);
  1933				err_word1 = readl(dci->base + (err_queue + 4));
  1934	
  1935				error_type = (err_word0 & GENMASK(9, 6)) >> 6;
  1936				if (error_type == ECC_SINGLE_DBE || error_type == ECC_MULTI_DBE ||
  1937				    error_type == ECC_WRITE_LINK_DBE ||
  1938				    error_type == ECC_READ_LINK_DBE ||
  1939				    error_type == ECC_READ_LINK_RMW_DBE) {
  1940					edac_printk(KERN_ERR, EDAC_DEVICE,
  1941						    "%s: DBE: word0:0x%08X, word1:0x%08X\n",
  1942						    dci->edac_dev_name, err_word0, err_word1);
  1943					dbe = true;
  1944				} else {
  1945					edac_printk(KERN_ERR, EDAC_DEVICE,
  1946						    "%s: SBE: word0:0x%08X, word1:0x%08X\n",
  1947						    dci->edac_dev_name, err_word0, err_word1);
  1948					edac_device_handle_ce(dci->edac_dev, 0, 0,
  1949							      dci->edac_dev_name);
  1950				}
  1951				cnt++;
  1952				err_queue += 8;
  1953			}
  1954			if (dbe)
  1955				panic("\nEDAC:IO96B[Uncorrectable errors]\n");
  1956		} else {
> 1957			err_queue_overflow = (err_word0 & GENMASK(9, 6)) >> 6;
> 1958			if (error_type == ECC_SINGLE_DBE || error_type == ECC_MULTI_DBE ||
  1959			    error_type == ECC_WRITE_LINK_DBE ||
  1960			    error_type == ECC_READ_LINK_DBE ||
  1961			    error_type == ECC_READ_LINK_RMW_DBE) {
  1962				panic("\nEDAC: UE: %s: word0:0x%08X, word1:0x%08X\n",
> 1963				      dci->edac_dev_name, err_word0, err_word1);
  1964			} else {
  1965				edac_printk(KERN_ERR, EDAC_DEVICE,
  1966					    "%s: Buffer Overflow SBE:0x%08X\n",
  1967					    dci->edac_dev_name, err_queue_overflow);
  1968				edac_device_handle_ce(dci->edac_dev, 0, 0, dci->edac_dev_name);
  1969			}
  1970		}
  1971	
  1972		//Clear Queue
  1973		writel(IO96B_ECC_ERROR_QUEUE_CLEAR, dci->base + IO96B_CMD_REQ_OFST);
  1974		return IRQ_HANDLED;
  1975	}
  1976	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-11-04  3:46 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-28  9:22 [PATCH 0/6] Add EDAC support for Agilex5 SoCFPGA niravkumarlaxmidas.rabara
2025-10-28  9:22 ` [PATCH 1/6] dt-bindings: edac: altera: Document additional ECC instances niravkumarlaxmidas.rabara
2025-10-29  6:50   ` Krzysztof Kozlowski
2025-10-31  8:01     ` Niravkumar L Rabara
2025-10-28  9:22 ` [PATCH 2/6] arm64: dts: agilex5: Add ECC manager and submodule nodes niravkumarlaxmidas.rabara
2025-10-29  6:51   ` Krzysztof Kozlowski
2025-10-28  9:22 ` [PATCH 3/6] EDAC/altera: Add DBE interrupt handling for Agilex5 niravkumarlaxmidas.rabara
2025-10-28  9:22 ` [PATCH 4/6] EDAC/altera: Add IO96B ECC support for Agilex5 SoCFPGA niravkumarlaxmidas.rabara
2025-10-30 14:30   ` Borislav Petkov
2025-10-31 11:52     ` Niravkumar L Rabara
2025-11-02 19:25       ` Borislav Petkov
2025-11-04  3:46   ` kernel test robot
2025-10-28  9:22 ` [PATCH 5/6] EDAC/altera: Add support for CRAM SEU error handling on SoCFPGA niravkumarlaxmidas.rabara
2025-10-29  6:52   ` Krzysztof Kozlowski
2025-10-28  9:22 ` [PATCH 6/6] EDAC: altera: Add ECC support for SDM QSPI on Agilex5 niravkumarlaxmidas.rabara
2025-10-29  6:52   ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox