Linux-EDAC Archive on lore.kernel.org
 help / color / Atom feed
* [PATCHv2] EDAC, altera: Move Stratix10 SDRAM ECC to peripheral
@ 2019-07-12 18:28 thor.thayer
  2019-07-25 12:46 ` James Morse
  0 siblings, 1 reply; 3+ messages in thread
From: thor.thayer @ 2019-07-12 18:28 UTC (permalink / raw)
  To: bp, mchehab, james.morse; +Cc: linux-edac, linux-kernel, Thor Thayer

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

ARM32 SoCFPGAs had separate IRQs for SDRAM. ARM64 SoCFPGAs
send all DBEs to SError so filtering by source is necessary.

The Stratix10 SDRAM ECC is a better match with the generic
Altera peripheral ECC framework because the linked list can
be searched to find the ECC block offset and printout
the DBE Address.

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

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index c2e693e34d43..09a80b53acea 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))
@@ -1898,6 +1922,22 @@ static int validate_parent_available(struct device_node *np)
 	return ret;
 }
 
+static int get_s10_sdram_edac_resource(struct device_node *np,
+				       struct resource *res)
+{
+	struct device_node *parent;
+	int ret;
+
+	parent = of_parse_phandle(np, "altr,sdr-syscon", 0);
+	if (!parent)
+		return -ENODEV;
+
+	ret = of_address_to_resource(parent, 0, res);
+	of_node_put(parent);
+
+	return ret;
+}
+
 static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
 				    struct device_node *np)
 {
@@ -1925,7 +1965,11 @@ 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;
 
-	rc = of_address_to_resource(np, 0, &res);
+	if (of_device_is_compatible(np, "altr,sdram-edac-s10"))
+		rc = get_s10_sdram_edac_resource(np, &res);
+	else
+		rc = of_address_to_resource(np, 0, &res);
+
 	if (rc < 0) {
 		edac_printk(KERN_ERR, EDAC_DEVICE,
 			    "%s: no resource address\n", ecc_name);
@@ -2231,13 +2275,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


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

* Re: [PATCHv2] EDAC, altera: Move Stratix10 SDRAM ECC to peripheral
  2019-07-12 18:28 [PATCHv2] EDAC, altera: Move Stratix10 SDRAM ECC to peripheral thor.thayer
@ 2019-07-25 12:46 ` James Morse
  2019-07-25 23:29   ` Thor Thayer
  0 siblings, 1 reply; 3+ messages in thread
From: James Morse @ 2019-07-25 12:46 UTC (permalink / raw)
  To: thor.thayer; +Cc: bp, mchehab, linux-edac, linux-kernel

Hi Thor,

On 12/07/2019 19:28, thor.thayer@linux.intel.com wrote:
> From: Thor Thayer <thor.thayer@linux.intel.com>
> 
> ARM32 SoCFPGAs had separate IRQs for SDRAM. ARM64 SoCFPGAs
> send all DBEs to SError so filtering by source is necessary.
> 
> The Stratix10 SDRAM ECC is a better match with the generic
> Altera peripheral ECC framework because the linked list can
> be searched to find the ECC block offset and printout
> the DBE Address.


> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index c2e693e34d43..09a80b53acea 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c

> @@ -2231,13 +2275,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"))

I'm just curious: This list looks suspiciously like the altr_edac_a10_device_of_match[]
list. Is there a reason it can't use of_match_device() here?

>  
>  			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);


Acked-by: James Morse <james.morse@arm.com>


Thanks,

James

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

* Re: [PATCHv2] EDAC, altera: Move Stratix10 SDRAM ECC to peripheral
  2019-07-25 12:46 ` James Morse
@ 2019-07-25 23:29   ` Thor Thayer
  0 siblings, 0 replies; 3+ messages in thread
From: Thor Thayer @ 2019-07-25 23:29 UTC (permalink / raw)
  To: James Morse; +Cc: bp, mchehab, linux-edac, linux-kernel

Hi James,

On 7/25/19 7:46 AM, James Morse wrote:
> Hi Thor,
> 
> On 12/07/2019 19:28, thor.thayer@linux.intel.com wrote:
>> From: Thor Thayer <thor.thayer@linux.intel.com>
>>
>> ARM32 SoCFPGAs had separate IRQs for SDRAM. ARM64 SoCFPGAs
>> send all DBEs to SError so filtering by source is necessary.
>>
>> The Stratix10 SDRAM ECC is a better match with the generic
>> Altera peripheral ECC framework because the linked list can
>> be searched to find the ECC block offset and printout
>> the DBE Address.
> 
> 
>> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
>> index c2e693e34d43..09a80b53acea 100644
>> --- a/drivers/edac/altera_edac.c
>> +++ b/drivers/edac/altera_edac.c
> 
>> @@ -2231,13 +2275,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"))
> 
> I'm just curious: This list looks suspiciously like the altr_edac_a10_device_of_match[]
> list. Is there a reason it can't use of_match_device() here?
> 
Good point. Yes, it does look very much like the match list. Although 
of_match_device() doesn't fit with the available parameters (device 
pointer), your question prompted me to look closer and I noticed 
of_match_node() is perfect.

I'll create a version 3 with this change.

Thanks for reviewing!

>>   
>>   			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);
> 
> 
> Acked-by: James Morse <james.morse@arm.com>
> 
> 
> Thanks,
> 
> James
> 


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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-12 18:28 [PATCHv2] EDAC, altera: Move Stratix10 SDRAM ECC to peripheral thor.thayer
2019-07-25 12:46 ` James Morse
2019-07-25 23:29   ` Thor Thayer

Linux-EDAC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-edac/0 linux-edac/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-edac linux-edac/ https://lore.kernel.org/linux-edac \
		linux-edac@vger.kernel.org linux-edac@archiver.kernel.org
	public-inbox-index linux-edac


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-edac


AGPL code for this site: git clone https://public-inbox.org/ public-inbox