Add EDAC support for Agilex5 SoCFPGA

[PATCH 1/6] dt-bindings: edac: altera: Document additional ECC instances

Posted by niravkumarlaxmidas.rabara@altera.com 3 months, 2 weeks ago

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

Re: [PATCH 1/6] dt-bindings: edac: altera: Document additional ECC instances

Posted by Krzysztof Kozlowski 3 months, 1 week ago

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

Re: [PATCH 1/6] dt-bindings: edac: altera: Document additional ECC instances

Posted by Niravkumar L Rabara 3 months, 1 week ago


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

[PATCH 1/6] dt-bindings: edac: altera: Document additional ECC instances
[PATCH 2/6] arm64: dts: agilex5: Add ECC manager and submodule nodes
[PATCH 3/6] EDAC/altera: Add DBE interrupt handling for Agilex5
[PATCH 4/6] EDAC/altera: Add IO96B ECC support for Agilex5 SoCFPGA
[PATCH 5/6] EDAC/altera: Add support for CRAM SEU error handling on SoCFPGA
[PATCH 6/6] EDAC: altera: Add ECC support for SDM QSPI on Agilex5