[RFC 0/2] arm64: dts: socfpga: clarify MMIO vs non-MMIO nodes

[RFC 0/2] arm64: dts: socfpga: clarify MMIO vs non-MMIO nodes

[Dinh Nguyen]

Hi DTS maintainers,

This letter will try to explain this comment[1] regarding the eccmgr node
not having an addressable range, but it's children does.

I am only including the bindings and DTS patches for this RFC to
hopefully not confuse things more.

The eccmgr node is an interrupt aggregator / irqchip, not a standalone
register block. Its INTMASK and INTSTAT registers live inside the
System Manager. The driver reaches them through the altr,sysmgr-syscon
phandle (syscon/regmap), so the manager itself has no reg property and
no unit address. In DT terms it is not an MMIO device.

The children (ocram-ecc, usb-ecc, emac-*-ecc, sdmmc-ecc, l2-ecc,
dma-ecc, ...) are MMIO devices. Each peripheral has its own ECC
controller block at a unique SoC address. The driver maps that block
with of_address_to_resource() / ioremap, so those nodes have a reg
property and a unit address (for example ocram-ecc@ff8cc000).

The parent still has #address-cells, #size-cells, and ranges so those
child addresses can be translated to CPU physical addresses (identity
map). ranges describes the children's address space. It does not give
the parent its own MMIO window.

sdramedac is the exception among the children: SDRAM ECC registers
live in the SDR controller, accessed via altr,sdr-syscon, so that
child also has no reg.

The children stay under eccmgr because they are interrupt clients of
the manager's irqchip, and the A10/S10/Agilex driver walks the
manager's child nodes from probe. One shared IRQ domain, many
per-RAM ECC blocks.

I hope that explains the layout of the EDAC controller on SoCFPGA
platforms. I welcome any feedback.

Thanks,
Dinh

[1] https://lore.kernel.org/all/20260724-fragrant-baboon-of-temperance-c5cdc0@quoll/

Dinh Nguyen (2):
  dt-bindings: edac: altera: agilex5: document new edac support
  arm64: dts: socfpga: agilex5: add support for the ECC manager

 .../edac/altr,socfpga-ecc-manager.yaml        | 85 +++++++++++++------
 .../arm64/boot/dts/intel/socfpga_agilex5.dtsi | 69 +++++++++++++++
 2 files changed, 130 insertions(+), 24 deletions(-)

-- 
2.42.0.411.g813d9a9188

[RFC PATCH 1/2] dt-bindings: edac: altera: agilex5: document new edac support

[Dinh Nguyen]

The Agilex5 EDAC module supports ECC on a few additional peripherals. Along
with support for ocram, ethernet and USB, the Agilex5 EDAC adds support for
SDM QSPI, CRAM SEU and IO96B.

The Agilex5 EDAC also exposes up to 7 named interrupts on the manager
node. global_sbe and global_dbe are aggregate single/double-bit error
interrupts that fan-out through the manager's IRQ domain to the child
peripheral ECCs. The remaining named interrupts are routed directly from
dedicated controllers (IO96B memory controllers and Secure Device Manager)
to the GIC.

Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
v2: add if/then to enforce maxItem of 2 interrupts for older platforms,
    and 7 for Agilex5
    add if/then to enforce interrupt-names as required property for
    Agilex5
    enum altr,socfpga-s10-ecc-manager and altr,socfpga-agilex5-ecc-manager
---
 .../edac/altr,socfpga-ecc-manager.yaml        | 85 +++++++++++++------
 1 file changed, 61 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml b/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml
index 136e8fccd429a..16940b49471cd 100644
--- a/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml
+++ b/Documentation/devicetree/bindings/edac/altr,socfpga-ecc-manager.yaml
@@ -20,7 +20,9 @@ properties:
   compatible:
     oneOf:
       - items:
-          - const: altr,socfpga-s10-ecc-manager
+          - enum:
+              - altr,socfpga-s10-ecc-manager
+              - altr,socfpga-agilex5-ecc-manager
           - const: altr,socfpga-a10-ecc-manager
       - const: altr,socfpga-a10-ecc-manager
       - const: altr,socfpga-ecc-manager
@@ -33,7 +35,25 @@ properties:
 
   interrupts:
     minItems: 1
-    maxItems: 2
+    items:
+      - description: Global single-bit error
+      - description: Global double-bit error
+      - description: IO96B0 DRAM controller error
+      - description: IO96B1 DRAM controller error
+      - description: SDM QSPI single-bit error
+      - description: SDM QSPI double-bit error
+      - description: SDM single-event
+
+  interrupt-names:
+    minItems: 1
+    items:
+      - const: global_sbe
+      - const: global_dbe
+      - const: io96b0
+      - const: io96b1
+      - const: sdm_qspi_sbe
+      - const: sdm_qspi_dbe
+      - const: sdm_seu
 
   interrupt-controller: true
 
@@ -235,28 +255,45 @@ patternProperties:
       - interrupts
       - altr,ecc-parent
 
-if:
-  properties:
-    compatible:
-      contains:
-        const: altr,socfpga-ecc-manager
-then:
-  required:
-    - compatible
-    - "#address-cells"
-    - "#size-cells"
-    - ranges
-
-else:
-  required:
-    - compatible
-    - "#address-cells"
-    - "#size-cells"
-    - interrupts
-    - interrupt-controller
-    - "#interrupt-cells"
-    - ranges
-    - altr,sysmgr-syscon
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: altr,socfpga-ecc-manager
+    then:
+      required:
+        - compatible
+        - "#address-cells"
+        - "#size-cells"
+        - ranges
+
+    else:
+      required:
+        - compatible
+        - "#address-cells"
+        - "#size-cells"
+        - interrupts
+        - interrupt-controller
+        - "#interrupt-cells"
+        - ranges
+        - altr,sysmgr-syscon
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: altr,socfpga-agilex5-ecc-manager
+    then:
+      properties:
+        interrupts:
+          maxItems: 7
+      required:
+        - interrupt-names
+    else:
+      properties:
+        interrupts:
+          maxItems: 2
 
 additionalProperties: false
 
-- 
2.42.0.411.g813d9a9188

[RFC PATCH 2/2] arm64: dts: socfpga: agilex5: add support for the ECC manager

[Dinh Nguyen]

Add the EDAC device to the base Agilex5 platform. Add OCRAM and Ethernet ECC
device support to the Agilex5 platform.

Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
v2: move eccmgr out of soc node
    s/38/36 for emac0-rx
---
 .../arm64/boot/dts/intel/socfpga_agilex5.dtsi | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
index b06c6d5d60eed..64e8bffd69df7 100644
--- a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
+++ b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
@@ -149,6 +149,75 @@ qspi_clk: qspi-clk {
 		};
 	};
 
+	eccmgr {
+		compatible = "altr,socfpga-agilex5-ecc-manager",
+			     "altr,socfpga-a10-ecc-manager";
+		altr,sysmgr-syscon = <&sysmgr>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		interrupt-parent = <&intc>;
+		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 = <0 0 0 0xffffffff>;
+
+		ocram-ecc@108cc000 {
+			compatible = "altr,socfpga-a10-ocram-ecc";
+			reg = <0x108cc000 0x100>;
+			interrupts = <1 IRQ_TYPE_LEVEL_HIGH>, <33 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>, <36 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>;
+		};
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupt-parent = <&intc>;
-- 
2.42.0.411.g813d9a9188

Re: [RFC PATCH 2/2] arm64: dts: socfpga: agilex5: add support for the ECC manager

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] Bitwise overflow and value truncation when unmasking double-bit error (DBE) interrupts.
--

commit a5d17bd160094d90efa7211d9c744f1d3da55629
Author: Dinh Nguyen <dinguyen@kernel.org>
Subject: arm64: dts: socfpga: agilex5: add support for the ECC manager

This commit adds the EDAC device and ECC manager nodes to the base Agilex5
platform. It introduces OCRAM and Ethernet ECC device support to the device
tree.

> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> index b06c6d5d60eed..64e8bffd69df7 100644
> --- a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
[ ... ]
> +	eccmgr {
> +		compatible = "altr,socfpga-agilex5-ecc-manager",
> +			     "altr,socfpga-a10-ecc-manager";
> +		altr,sysmgr-syscon = <&sysmgr>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		interrupt-parent = <&intc>;
> +		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 = <0 0 0 0xffffffff>;
> +
> +		ocram-ecc@108cc000 {
> +			compatible = "altr,socfpga-a10-ocram-ecc";
> +			reg = <0x108cc000 0x100>;
> +			interrupts = <1 IRQ_TYPE_LEVEL_HIGH>, <33 IRQ_TYPE_LEVEL_HIGH>;
> +		};

[Severity: Medium]
This is a pre-existing issue, but introducing interrupts with values >= 32
triggers a bitwise overflow and truncation bug in the altera_edac driver.

When the EDAC driver sets up DBE interrupts, it calls a10_eccmgr_irq_unmask()
with hwirq >= 32. The BIT(d->hwirq) macro computes 1UL << 33 (and up to 41
for the emac nodes).

drivers/edac/altera_edac.c:a10_eccmgr_irq_unmask() {
    ...
    regmap_write(edac->ecc_mgr_map, A10_SYSMGR_ECC_INTMASK_CLR_OFST,
                 BIT(d->hwirq));
    ...
}

Does this shift result in undefined behavior or truncation? On 64-bit
systems, this yields a 64-bit value which is silently truncated to 0 when
passed to regmap_write()'s 32-bit val parameter. On 32-bit systems, this
shift exceeds the width of unsigned long, typically yielding 0 at runtime.

The masking function a10_eccmgr_irq_mask() shares the exact same bitwise
overflow and truncation issue:

drivers/edac/altera_edac.c:a10_eccmgr_irq_mask() {
    ...
    regmap_write(edac->ecc_mgr_map, A10_SYSMGR_ECC_INTMASK_SET_OFST,
                 BIT(d->hwirq));
    ...
}

While the DBE interrupts may inadvertently remain unmasked because they
share the same hardware mask bit as single-bit errors, the specific
masking logic for DBE appears to fail because the computed mask evaluates
to 0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902092551.488203-1-dinguyen@kernel.org?part=2