Add SDRAM ECC support for Stratix10
cover Add SDRAM ECC support for Stratix10
From: Thor Thayer <[email protected]>
The Intel Stratix10 platform is an ARM64 but still has many register definitions that are similar to the Arria10. One significant difference is the Stratix10 hypervisor which requires handling registers that may be shared by guest OSes at a different exception level. Register access is through an ARM SMC call. Currently, SMC handling is implemented in U-Boot.
Thor Thayer (3):
Documentation: dt: socfpga: Add Stratix10 ECC Manager binding
edac: altera: Add support for Stratix10 SDRAM EDAC
arm64: dts: stratix10: add sdram ecc
.../bindings/arm/altera/socfpga-eccmgr.txt | 47 +++ arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 17 + drivers/edac/Kconfig | 2 +- drivers/edac/altera_edac.c | 459 +++++++++++++++++++++ drivers/edac/altera_edac.h | 114 +++++ 5 files changed, 638 insertions(+), 1 deletion(-)
Signature · 1 line
2.7.4
PATCH 1/3 Documentation: dt: socfpga: Add Stratix10 ECC Manager binding
From: Thor Thayer <[email protected]>
Add the device tree bindings needed to support the Stratix10 ECC Manager and SDRAM ECC to the existing bindings.
--- .../bindings/arm/altera/socfpga-eccmgr.txt | 47 ++++++++++++++++++++++ 1 file changed, 47 insertions(+)
Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt +47 -0
@@ -231,3 +231,50 @@ Example: <48 IRQ_TYPE_LEVEL_HIGH>; }; }; + +Stratix10 SoCFPGA ECC Manager +The Stratix10 SoC ECC Manager handles the IRQs for each peripheral +in a shared register similar to the Arria10. However, ECC requires +access to registers that can only be read in EL3 with SMC calls. +Therefore the device tree is slightly different. + +Required Properties: +- compatible : Should be "altr,socfpga-s10-ecc-manager" +- altr,sysgr-syscon : phandle to Stratix10 System Manager Block + containing the ECC manager registers. +- #address-cells: must be 1 +- #size-cells: must be 1 +- interrupts : Should be single bit error interrupt, then double bit error + interrupt. +- interrupt-controller : boolean indicator that ECC Manager is an interrupt controller +- #interrupt-cells : must be set to 2. +- ranges : standard definition, should translate from local addresses + +Subcomponents: + +SDRAM ECC +Required Properties: +- compatible : Should be "altr,sdram-edac-s10" +- reg : Address and size for ECC error interrupt clear registers. +- interrupts : Should be single bit error interrupt, then double bit error + interrupt, in this order. + +Example: + + eccmgr: eccmgr at ffd12000 { + compatible = "altr,socfpga-s10-ecc-manager"; + altr,sysmgr-syscon = <&sysmgr>; + #address-cells = <1>; + #size-cells = <1>; + interrupts = <0 15 4>, <0 95 4>; + interrupt-controller; + #interrupt-cells = <2>; + ranges; + + sdramedac at f8011100 { + compatible = "altr,sdram-edac-s10"; + reg = <0xf8011100 0xC0>; + interrupts = <16 4>, <48 4>; + }; + }; +
Signature · 1 line
2.7.4
Dinh Nguyen
maintainerPATCH 1/3 Documentation: dt: socfpga: Add Stratix10 ECC Manager binding
Quoted 17 lines · thor.thayer at linux.intel.com
From: Thor Thayer <[email protected]>
Add the device tree bindings needed to support the Stratix10 ECC Manager and SDRAM ECC to the existing bindings.
Signed-off-by Thor Thayer <[email protected]>--- .../bindings/arm/altera/socfpga-eccmgr.txt | 47 ++++++++++++++++++++++ 1 file changed, 47 insertions(+)Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt +0
-0@@ -231,3 +231,50 @@ Example: <48 IRQ_TYPE_LEVEL_HIGH>; };
Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt Example: · line 242
Quoted 10 lines
Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt +9
-0}; + +Stratix10 SoCFPGA ECC Manager +The Stratix10 SoC ECC Manager handles the IRQs for each peripheral +in a shared register similar to the Arria10. However, ECC requires +access to registers that can only be read in EL3 with SMC calls. +Therefore the device tree is slightly different. + +Required Properties: +- compatible : Should be "altr,socfpga-s10-ecc-manager"
Altera technically doesn't exist anymore, should this be "intel,stratix10-ecc-manager"?
Dinh
PATCH 1/3 Documentation: dt: socfpga: Add Stratix10 ECC Manager binding
Quoted 25 lines · Dinh Nguyen
Quoted 22 lines · thor.thayer at linux.intel.com
From: Thor Thayer <[email protected]>
Add the device tree bindings needed to support the Stratix10 ECC Manager and SDRAM ECC to the existing bindings.
Signed-off-by Thor Thayer <[email protected]>--- .../bindings/arm/altera/socfpga-eccmgr.txt | 47 ++++++++++++++++++++++ 1 file changed, 47 insertions(+)Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt +4
-0@@ -231,3 +231,50 @@ Example: <48 IRQ_TYPE_LEVEL_HIGH>; }; }; + +Stratix10 SoCFPGA ECC Manager +The Stratix10 SoC ECC Manager handles the IRQs for each peripheral +in a shared register similar to the Arria10. However, ECC requires
Quoted 9 lines
Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt Example: · line 242
Quoted 5 lines
Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt +5
-0+access to registers that can only be read in EL3 with SMC calls. +Therefore the device tree is slightly different. + +Required Properties: +- compatible : Should be "altr,socfpga-s10-ecc-manager"Altera technically doesn't exist anymore, should this be "intel,stratix10-ecc-manager"?
I was trying to be consistent with the older names but I agree with your argument. I will change this. Thanks
Quoted 6 lines
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-edac" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/3 Documentation: dt: socfpga: Add Stratix10 ECC Manager binding
Quoted 38 lines · Thor Thayer
Quoted 28 lines · Dinh Nguyen
Quoted 25 lines · thor.thayer at linux.intel.com
From: Thor Thayer <[email protected]>
Add the device tree bindings needed to support the Stratix10 ECC Manager and SDRAM ECC to the existing bindings.
Signed-off-by Thor Thayer <[email protected]>---
?.../bindings/arm/altera/socfpga-eccmgr.txt???????? | 47
++++++++++++++++++++++
? 1 file changed, 47 insertions(+)(quoted diff) +0
-0
a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txtb/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txtindex4a1714f96bab..fe48ad293a24100644Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt +4
-0@@ -231,3 +231,50 @@ Example: ?????????????????????? <48 IRQ_TYPE_LEVEL_HIGH>; ????????? }; ????? }; + +Stratix10 SoCFPGA ECC Manager +The Stratix10 SoC ECC Manager handles the IRQs for each peripheral +in a shared register similar to the Arria10. However, ECC requiresQuoted 9 lines
Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt Example: · line 242
Quoted 5 lines
Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt +5
-0+access to registers that can only be read in EL3 with SMC calls. +Therefore the device tree is slightly different. + +Required Properties: +- compatible : Should be "altr,socfpga-s10-ecc-manager"Altera technically doesn't exist anymore, should this be "intel,stratix10-ecc-manager"?
Quoted 3 lines
I was trying to be consistent with the older names but I agree with your argument. I will change this. Thanks
After looking at the Stratix10 device tree, there are only "altr," in there. For instance the top node is:
compatible = "altr,socfpga-stratix10";
and even the directory that the device tree is in is named "altera"
so I'll stick with the existing "altr," designation to be consistent.
Thanks,
Thor
Quoted 12 lines
Quoted 6 lines
Dinh
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-edac" in the body of a message to majordomo at vger.kernel.org More majordomo info at? http://vger.kernel.org/majordomo-info.htmlSignature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-edac" in the body of a message to majordomo at vger.kernel.org More majordomo info at? http://vger.kernel.org/majordomo-info.html
Dinh Nguyen
maintainerPATCH 1/3 Documentation: dt: socfpga: Add Stratix10 ECC Manager binding
Quoted 42 lines · Thor Thayer
Quoted 38 lines · Thor Thayer
Quoted 28 lines · Dinh Nguyen
Quoted 25 lines · thor.thayer at linux.intel.com
From: Thor Thayer <[email protected]>
Add the device tree bindings needed to support the Stratix10 ECC Manager and SDRAM ECC to the existing bindings.
Signed-off-by Thor Thayer <[email protected]>---
?.../bindings/arm/altera/socfpga-eccmgr.txt???????? | 47
++++++++++++++++++++++
? 1 file changed, 47 insertions(+)diff --git
a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txtb/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txtindex4a1714f96bab..fe48ad293a24100644Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt +4
-0@@ -231,3 +231,50 @@ Example: ?????????????????????? <48 IRQ_TYPE_LEVEL_HIGH>; ????????? }; ????? }; + +Stratix10 SoCFPGA ECC Manager +The Stratix10 SoC ECC Manager handles the IRQs for each peripheral +in a shared register similar to the Arria10. However, ECC requiresQuoted 9 lines
Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt Example: · line 242
Quoted 5 lines
Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt +5
-0+access to registers that can only be read in EL3 with SMC calls. +Therefore the device tree is slightly different. + +Required Properties: +- compatible : Should be "altr,socfpga-s10-ecc-manager"Altera technically doesn't exist anymore, should this be "intel,stratix10-ecc-manager"?
Quoted 3 lines
I was trying to be consistent with the older names but I agree with your argument. I will change this. Thanks
Quoted 9 lines
After looking at the Stratix10 device tree, there are only "altr," in there. For instance the top node is:
compatible = "altr,socfpga-stratix10";
and even the directory that the device tree is in is named "altera"
so I'll stick with the existing "altr," designation to be consistent.
That's fine and I don't have any strong inclination for 1 way or another. But it's not quite true that there are only "altr" in the dts file.
I did recently switch the clock manager to an intel designation. I upstreamed the original stratix10 DTS file back in 2015, before Altera was purchased by Intel.
My only argument is that if you're adding new support for devices, it should be an intel binding.
Dinh
PATCH 1/3 Documentation: dt: socfpga: Add Stratix10 ECC Manager binding
Quoted 58 lines · Dinh Nguyen
Quoted 42 lines · Thor Thayer
Quoted 38 lines · Thor Thayer
Quoted 28 lines · Dinh Nguyen
Quoted 25 lines · thor.thayer at linux.intel.com
From: Thor Thayer <[email protected]>
Add the device tree bindings needed to support the Stratix10 ECC Manager and SDRAM ECC to the existing bindings.
Signed-off-by Thor Thayer <[email protected]>--- .../bindings/arm/altera/socfpga-eccmgr.txt | 47 ++++++++++++++++++++++ 1 file changed, 47 insertions(+)diff --git
a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txtb/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txtindex4a1714f96bab..fe48ad293a24100644Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt +4
-0@@ -231,3 +231,50 @@ Example: <48 IRQ_TYPE_LEVEL_HIGH>; }; }; + +Stratix10 SoCFPGA ECC Manager +The Stratix10 SoC ECC Manager handles the IRQs for each peripheral +in a shared register similar to the Arria10. However, ECC requiresQuoted 9 lines
Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt Example: · line 242
Quoted 5 lines
Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt +5
-0+access to registers that can only be read in EL3 with SMC calls. +Therefore the device tree is slightly different. + +Required Properties: +- compatible : Should be "altr,socfpga-s10-ecc-manager"Altera technically doesn't exist anymore, should this be "intel,stratix10-ecc-manager"?
Quoted 3 lines
I was trying to be consistent with the older names but I agree with your argument. I will change this. Thanks
Quoted 9 lines
After looking at the Stratix10 device tree, there are only "altr," in there. For instance the top node is:
compatible = "altr,socfpga-stratix10";
and even the directory that the device tree is in is named "altera"
so I'll stick with the existing "altr," designation to be consistent.
That's fine and I don't have any strong inclination for 1 way or another. But it's not quite true that there are only "altr" in the dts file.
Quoted 6 lines
I did recently switch the clock manager to an intel designation. I upstreamed the original stratix10 DTS file back in 2015, before Altera was purchased by Intel.
My only argument is that if you're adding new support for devices, it should be an intel binding.
I think I would make the switch when you change it at the SoC level (for a new SoC). IOW, if the top level compatible is using 'intel', then use that for the device bindings otherwise stick with 'altr' for that SoC.
Rob
PATCH 2/3 edac: altera: Add support for Stratix10 SDRAM EDAC
In reply to thor.thayer at linux.intel.comFrom: Thor Thayer <[email protected]>
Add support for SDRAM ECC on the Stratix10 platform to the EDAC device driver. Although Stratix10 is very similar to the Arria10, hypervisor support for Stratix10 SDRAM ECC requires the use of SMC calls to a higher priority exception level to handle some register reads/writes.
--- drivers/edac/Kconfig | 2 +- drivers/edac/altera_edac.c | 459 +++++++++++++++++++++++++++++++++++++++++++++ drivers/edac/altera_edac.h | 114 +++++++++++ 3 files changed, 574 insertions(+), 1 deletion(-)
drivers/edac/Kconfig +1 -1
@@ -379,7 +379,7 @@ config EDAC_THUNDERX config EDAC_ALTERA bool "Altera SOCFPGA ECC" - depends on EDAC=y && ARCH_SOCFPGA + depends on EDAC=y && (ARCH_SOCFPGA || ARM64) help Support for error detection and correction on the Altera SOCs. This must be selected for SDRAM ECC.
drivers/edac/altera_edac.c +459 -0
493 lines not on this page · raw message
drivers/edac/altera_edac.h +114 -0
148 lines not on this page · raw message
Signature · 1 line
2.7.4
Dinh Nguyen
maintainerPATCH 2/3 edac: altera: Add support for Stratix10 SDRAM EDAC
Quoted 8 lines · thor.thayer at linux.intel.com
From: Thor Thayer <[email protected]>
Add support for SDRAM ECC on the Stratix10 platform to the EDAC device driver. Although Stratix10 is very similar to the Arria10, hypervisor support for Stratix10 SDRAM ECC requires the use of SMC calls to a higher priority exception level to handle some register reads/writes.
Quoted 5 lines
Signed-off-by Thor Thayer <[email protected]>--- drivers/edac/Kconfig | 2 +- drivers/edac/altera_edac.c | 459 +++++++++++++++++++++++++++++++++++++++++++++ drivers/edac/altera_edac.h | 114 +++++++++++
Does it make sense to have the support for this in separate files?
drivers/edac/Kconfig config EDAC_THUNDERX · line 382
Quoted 12 lines
3 files changed, 574 insertions(+), 1 deletion(-)drivers/edac/Kconfig +1
-1@@ -379,7 +379,7 @@ config EDAC_THUNDERX config EDAC_ALTERA bool "Altera SOCFPGA ECC" - depends on EDAC=y && ARCH_SOCFPGA + depends on EDAC=y && (ARCH_SOCFPGA || ARM64)
Should the ARM64 dependency just be ARCH_STRATIX10?
drivers/edac/altera_edac.c line 2
Quoted 10 lines
drivers/edac/Kconfig +0
-0help Support for error detection and correction on the Altera SOCs. This must be selected for SDRAM ECC.drivers/edac/altera_edac.c +1
-0@@ -1,4 +1,5 @@/* +* Copyright (C) 2017-2018, Intel Corporation
If you're updating the license header, would it make sense to convert to SPDX?
Dinh
Borislav Petkov
maintainerPATCH 2/3 edac: altera: Add support for Stratix10 SDRAM EDAC
Quoted 1 line · Dinh Nguyen
Does it make sense to have the support for this in separate files?
No. Per vendor or per uarch/processor type, standalone EDAC drivers are fine. But not per the RAS capability of a functional unit.
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.PATCH 2/3 edac: altera: Add support for Stratix10 SDRAM EDAC
In reply to Dinh NguyenQuoted 27 lines · Dinh Nguyen
Quoted 8 lines · thor.thayer at linux.intel.com
From: Thor Thayer <[email protected]>
Add support for SDRAM ECC on the Stratix10 platform to the EDAC device driver. Although Stratix10 is very similar to the Arria10, hypervisor support for Stratix10 SDRAM ECC requires the use of SMC calls to a higher priority exception level to handle some register reads/writes.
Quoted 5 lines
Signed-off-by Thor Thayer <[email protected]>--- drivers/edac/Kconfig | 2 +- drivers/edac/altera_edac.c | 459 +++++++++++++++++++++++++++++++++++++++++++++ drivers/edac/altera_edac.h | 114 +++++++++++Does it make sense to have the support for this in separate files?
Quoted 8 lines
3 files changed, 574 insertions(+), 1 deletion(-)drivers/edac/Kconfig +0
-0@@ -379,7 +379,7 @@ config EDAC_THUNDERX
Quoted 7 lines
drivers/edac/Kconfig config EDAC_THUNDERX · line 382
Quoted 4 lines
drivers/edac/Kconfig +1
-1config EDAC_ALTERA bool "Altera SOCFPGA ECC" - depends on EDAC=y && ARCH_SOCFPGA + depends on EDAC=y && (ARCH_SOCFPGA || ARM64)Should the ARM64 dependency just be
ARCH_STRATIX10?
Yes, I will update this.
Quoted 5 lines
Quoted 5 lines
drivers/edac/Kconfig +0
-0help Support for error detection and correction on the Altera SOCs. This must be selected for SDRAM ECC.drivers/edac/altera_edac.c +0
-0
Quoted 9 lines
drivers/edac/altera_edac.c line 2
Quoted 5 lines
drivers/edac/altera_edac.c +1
-0@@ -1,4 +1,5 @@/* +* Copyright (C) 2017-2018, Intel Corporation If you're updating the license header, would it make sense to convert to SPDX?
Yes, I can do that.
Quoted 6 lines
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-edac" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 3/3 arm64: dts: stratix10: add sdram ecc
In reply to thor.thayer at linux.intel.comFrom: Thor Thayer <[email protected]>
Add the Stratix10 ECC Manager and SDRAM EDAC nodes to the device tree.
--- arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi +17 -0
@@ -924,6 +924,23 @@ status = "disabled"; }; + eccmgr: eccmgr at ffd12000 { + compatible = "altr,socfpga-s10-ecc-manager"; + altr,sysmgr-syscon = <&sysmgr>; + #address-cells = <1>; + #size-cells = <1>; + interrupts = <0 15 4>, <0 95 4>; + interrupt-controller; + #interrupt-cells = <2>; + ranges; + + sdramedac at f8011100 { + compatible = "altr,sdram-edac-s10"; + reg = <0xf8011100 0xC0>; + interrupts = <16 4>, <48 4>; + }; + }; + qspi: spi at ff8d2000 { compatible = "cdns,qspi-nor"; #address-cells = <1>;
Signature · 1 line
2.7.4