[PATCH 0/3] Stratix10 SDRAM Common EDAC Framework

[PATCH 0/3] Stratix10 SDRAM Common EDAC Framework

[thor.thayer]

From: Thor Thayer <thor.thayer@linux.intel.com>

Use the common Altera EDAC Device Framework for the SDRAM so that
Double Bit Error Addresses can be tracked for SDRAM.
This also simplifies the device tree.

Thor Thayer (3):
  Documentation: dt: edac: Add reg to S10 SDRAM node
  arm64: dts: Stratix10: Include regs in SDRAM ECC node
  EDAC, altera: Use common framework for Stratix10 SDRAM ECC

 .../devicetree/bindings/edac/socfpga-eccmgr.txt    |  4 ++-
 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi  |  9 ++----
 drivers/edac/altera_edac.c                         | 32 ++++++++++++++++++++--
 drivers/edac/altera_edac.h                         | 25 ++++++++++++++++-
 4 files changed, 58 insertions(+), 12 deletions(-)

-- 
2.7.4

[PATCH 1/3] Documentation: dt: edac: Add reg to S10 SDRAM node

[thor.thayer]

From: Thor Thayer <thor.thayer@linux.intel.com>

Include the register offset and size in the Stratix10 SDRAM node
to be consistent with other ECC modules.
Previously had to follow the phandle to get the register size/offset.

Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
---
 Documentation/devicetree/bindings/edac/socfpga-eccmgr.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/edac/socfpga-eccmgr.txt b/Documentation/devicetree/bindings/edac/socfpga-eccmgr.txt
index 8f52206cfd2a..dd6ba6c020a7 100644
--- a/Documentation/devicetree/bindings/edac/socfpga-eccmgr.txt
+++ b/Documentation/devicetree/bindings/edac/socfpga-eccmgr.txt
@@ -256,6 +256,7 @@ Subcomponents:
 SDRAM ECC
 Required Properties:
 - compatible : Should be "altr,sdram-edac-s10"
+- reg        : Address and size for ECC block registers.
 - interrupts : Should be single bit error interrupt.
 
 On-Chip RAM ECC
@@ -313,8 +314,9 @@ Example:
 		#interrupt-cells = <2>;
 		ranges;
 
-		sdramedac {
+		sdramedac@0xf8011100 {
 			compatible = "altr,sdram-edac-s10";
+			reg = <0xf8011100 0xc0>;
 			interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
-- 
2.7.4

[PATCH 2/3] arm64: dts: Stratix10: Include regs in SDRAM ECC node

[thor.thayer]

From: Thor Thayer <thor.thayer@linux.intel.com>

Include the regs directly in the SDRAM node instead of using
a syscon. The Stratix10 SDRAM ECC registers are partitioned
away from other Sys Manager registers so the syscon is no
longer needed for Stratix10.

Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
---
 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
index 4b0f674df849..a1e9545de3d3 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
@@ -517,11 +517,6 @@
 			status = "disabled";
 		};
 
-		sdr: sdr@f8011100 {
-			compatible = "altr,sdr-ctl", "syscon";
-			reg = <0xf8011100 0xc0>;
-		};
-
 		eccmgr {
 			compatible = "altr,socfpga-s10-ecc-manager",
 				     "altr,socfpga-a10-ecc-manager";
@@ -533,9 +528,9 @@
 			#interrupt-cells = <2>;
 			ranges;
 
-			sdramedac {
+			sdramedac@0xf8011100 {
 				compatible = "altr,sdram-edac-s10";
-				altr,sdr-syscon = <&sdr>;
+				reg = <0xf8011100 0xc0>;
 				interrupts = <16 4>;
 			};
 
-- 
2.7.4

[PATCH 3/3] EDAC, altera: Use common framework for Stratix10 SDRAM ECC

[thor.thayer]

From: Thor Thayer <thor.thayer@linux.intel.com>

The Stratix10 SDRAM ECC is a better match with the devices ECC
framework.
Use the ECC device framework with the linked list to track
SDRAM errors. This simplifies the device tree node as well.

Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
---
 drivers/edac/altera_edac.c | 32 +++++++++++++++++++++++++++++---
 drivers/edac/altera_edac.h | 25 ++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index c2e693e34d43..23e6af7287e4 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -222,7 +222,6 @@ static unsigned long get_total_mem(void)
 static const struct of_device_id altr_sdram_ctrl_of_match[] = {
 	{ .compatible = "altr,sdram-edac", .data = &c5_data},
 	{ .compatible = "altr,sdram-edac-a10", .data = &a10_data},
-	{ .compatible = "altr,sdram-edac-s10", .data = &a10_data},
 	{},
 };
 MODULE_DEVICE_TABLE(of, altr_sdram_ctrl_of_match);
@@ -1170,6 +1169,24 @@ static int __init __maybe_unused altr_init_a10_ecc_device_type(char *compat)
 	return 0;
 }
 
+/*********************** SDRAM EDAC Device Functions *********************/
+
+#ifdef CONFIG_EDAC_ALTERA_SDRAM
+
+static const struct edac_device_prv_data s10_sdramecc_data = {
+	.setup = altr_check_ecc_deps,
+	.ce_clear_mask = ALTR_S10_ECC_SERRPENA,
+	.ue_clear_mask = ALTR_S10_ECC_DERRPENA,
+	.ecc_enable_mask = ALTR_S10_ECC_EN,
+	.ecc_en_ofst = ALTR_S10_ECC_CTRL_SDRAM_OFST,
+	.ce_set_mask = ALTR_S10_ECC_TSERRA,
+	.ue_set_mask = ALTR_S10_ECC_TDERRA,
+	.set_err_ofst = ALTR_S10_ECC_INTTEST_OFST,
+	.ecc_irq_handler = altr_edac_a10_ecc_irq,
+	.inject_fops = &altr_edac_a10_device_inject_fops,
+};
+#endif /* CONFIG_EDAC_ALTERA_SDRAM */
+
 /*********************** OCRAM EDAC Device Functions *********************/
 
 #ifdef CONFIG_EDAC_ALTERA_OCRAM
@@ -1759,6 +1776,9 @@ static const struct of_device_id altr_edac_a10_device_of_match[] = {
 #ifdef CONFIG_EDAC_ALTERA_SDMMC
 	{ .compatible = "altr,socfpga-sdmmc-ecc", .data = &a10_sdmmcecca_data },
 #endif
+#ifdef CONFIG_EDAC_ALTERA_SDRAM
+	{ .compatible = "altr,sdram-edac-s10", .data = &s10_sdramecc_data },
+#endif
 	{},
 };
 MODULE_DEVICE_TABLE(of, altr_edac_a10_device_of_match);
@@ -1889,6 +1909,10 @@ static int validate_parent_available(struct device_node *np)
 	struct device_node *parent;
 	int ret = 0;
 
+	/* SDRAM must be present for Linux (implied parent) */
+	if (of_device_is_compatible(np, "altr,sdram-edac-s10"))
+		return 0;
+
 	/* Ensure parent device is enabled if parent node exists */
 	parent = of_parse_phandle(np, "altr,ecc-parent", 0);
 	if (parent && !of_device_is_available(parent))
@@ -2231,13 +2255,15 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
 		    of_device_is_compatible(child, "altr,socfpga-dma-ecc") ||
 		    of_device_is_compatible(child, "altr,socfpga-usb-ecc") ||
 		    of_device_is_compatible(child, "altr,socfpga-qspi-ecc") ||
+#ifdef CONFIG_EDAC_ALTERA_SDRAM
+		    of_device_is_compatible(child, "altr,sdram-edac-s10") ||
+#endif
 		    of_device_is_compatible(child, "altr,socfpga-sdmmc-ecc"))
 
 			altr_edac_a10_device_add(edac, child);
 
 #ifdef CONFIG_EDAC_ALTERA_SDRAM
-		else if ((of_device_is_compatible(child, "altr,sdram-edac-a10")) ||
-			 (of_device_is_compatible(child, "altr,sdram-edac-s10")))
+		else if (of_device_is_compatible(child, "altr,sdram-edac-a10"))
 			of_platform_populate(pdev->dev.of_node,
 					     altr_sdram_ctrl_of_match,
 					     NULL, &pdev->dev);
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index 55654cc4bcdf..3727e72c8c2e 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -289,6 +289,29 @@ struct altr_sdram_mc_data {
 #define ALTR_A10_ECC_INIT_WATCHDOG_10US      10000
 
 /************* Stratix10 Defines **************/
+#define ALTR_S10_ECC_CTRL_SDRAM_OFST      0x00
+#define ALTR_S10_ECC_EN                   BIT(0)
+
+#define ALTR_S10_ECC_ERRINTEN_OFST        0x10
+#define ALTR_S10_ECC_ERRINTENS_OFST       0x14
+#define ALTR_S10_ECC_ERRINTENR_OFST       0x18
+#define ALTR_S10_ECC_SERRINTEN            BIT(0)
+
+#define ALTR_S10_ECC_INTMODE_OFST         0x1C
+#define ALTR_S10_ECC_INTMODE              BIT(0)
+
+#define ALTR_S10_ECC_INTSTAT_OFST         0x20
+#define ALTR_S10_ECC_SERRPENA             BIT(0)
+#define ALTR_S10_ECC_DERRPENA             BIT(8)
+#define ALTR_S10_ECC_ERRPENA_MASK         (ALTR_S10_ECC_SERRPENA | \
+					   ALTR_S10_ECC_DERRPENA)
+
+#define ALTR_S10_ECC_INTTEST_OFST         0x24
+#define ALTR_S10_ECC_TSERRA               BIT(0)
+#define ALTR_S10_ECC_TDERRA               BIT(8)
+#define ALTR_S10_ECC_TSERRB               BIT(16)
+#define ALTR_S10_ECC_TDERRB               BIT(24)
+
 #define ALTR_S10_DERR_ADDRA_OFST          0x2C
 
 /* Stratix10 ECC Manager Defines */
@@ -300,7 +323,7 @@ struct altr_sdram_mc_data {
 #define S10_SYSMGR_UE_ADDR_OFST           0x224
 
 #define S10_DDR0_IRQ_MASK                 BIT(16)
-#define S10_DBE_IRQ_MASK                  0x3FE
+#define S10_DBE_IRQ_MASK                  0x3FFFE
 
 /* Define ECC Block Offsets for peripherals */
 #define ECC_BLK_ADDRESS_OFST              0x40
-- 
2.7.4

Re: [PATCH 0/3] Stratix10 SDRAM Common EDAC Framework

[Thor Thayer]

On 7/9/19 5:24 PM, thor.thayer@linux.intel.com wrote:
> From: Thor Thayer <thor.thayer@linux.intel.com>
> 
> Use the common Altera EDAC Device Framework for the SDRAM so that
> Double Bit Error Addresses can be tracked for SDRAM.
> This also simplifies the device tree.
> 
> Thor Thayer (3):
>    Documentation: dt: edac: Add reg to S10 SDRAM node
>    arm64: dts: Stratix10: Include regs in SDRAM ECC node
>    EDAC, altera: Use common framework for Stratix10 SDRAM ECC
> 
>   .../devicetree/bindings/edac/socfpga-eccmgr.txt    |  4 ++-
>   arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi  |  9 ++----
>   drivers/edac/altera_edac.c                         | 32 ++++++++++++++++++++--
>   drivers/edac/altera_edac.h                         | 25 ++++++++++++++++-
>   4 files changed, 58 insertions(+), 12 deletions(-)
> 
Please disregard this patchset, there is a simpler solution.

Re: [PATCH 1/3] Documentation: dt: edac: Add reg to S10 SDRAM node

[Thor Thayer]

On 7/9/19 5:24 PM, thor.thayer@linux.intel.com wrote:
> From: Thor Thayer <thor.thayer@linux.intel.com>
> 
> Include the register offset and size in the Stratix10 SDRAM node
> to be consistent with other ECC modules.
> Previously had to follow the phandle to get the register size/offset.
> 
> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
> ---
>   Documentation/devicetree/bindings/edac/socfpga-eccmgr.txt | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/edac/socfpga-eccmgr.txt b/Documentation/devicetree/bindings/edac/socfpga-eccmgr.txt
> index 8f52206cfd2a..dd6ba6c020a7 100644
> --- a/Documentation/devicetree/bindings/edac/socfpga-eccmgr.txt
> +++ b/Documentation/devicetree/bindings/edac/socfpga-eccmgr.txt
> @@ -256,6 +256,7 @@ Subcomponents:
>   SDRAM ECC
>   Required Properties:
>   - compatible : Should be "altr,sdram-edac-s10"
> +- reg        : Address and size for ECC block registers.
>   - interrupts : Should be single bit error interrupt.
>   
>   On-Chip RAM ECC
> @@ -313,8 +314,9 @@ Example:
>   		#interrupt-cells = <2>;
>   		ranges;
>   
> -		sdramedac {
> +		sdramedac@0xf8011100 {
>   			compatible = "altr,sdram-edac-s10";
> +			reg = <0xf8011100 0xc0>;
>   			interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
>   		};
>   
> 
Please disregard this patch, there is a simpler solution that I will 
submit shortly.

Re: [PATCH 2/3] arm64: dts: Stratix10: Include regs in SDRAM ECC node

[Thor Thayer]

On 7/9/19 5:24 PM, thor.thayer@linux.intel.com wrote:
> From: Thor Thayer <thor.thayer@linux.intel.com>
> 
> Include the regs directly in the SDRAM node instead of using
> a syscon. The Stratix10 SDRAM ECC registers are partitioned
> away from other Sys Manager registers so the syscon is no
> longer needed for Stratix10.
> 
> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
> ---
>   arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 9 ++-------
>   1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> index 4b0f674df849..a1e9545de3d3 100644
> --- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> @@ -517,11 +517,6 @@
>   			status = "disabled";
>   		};
>   
> -		sdr: sdr@f8011100 {
> -			compatible = "altr,sdr-ctl", "syscon";
> -			reg = <0xf8011100 0xc0>;
> -		};
> -
>   		eccmgr {
>   			compatible = "altr,socfpga-s10-ecc-manager",
>   				     "altr,socfpga-a10-ecc-manager";
> @@ -533,9 +528,9 @@
>   			#interrupt-cells = <2>;
>   			ranges;
>   
> -			sdramedac {
> +			sdramedac@0xf8011100 {
>   				compatible = "altr,sdram-edac-s10";
> -				altr,sdr-syscon = <&sdr>;
> +				reg = <0xf8011100 0xc0>;
>   				interrupts = <16 4>;
>   			};
>   
> 
Please disregard this patch, there is a simpler solution that I will 
submit shortly.