* [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 related [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, other threads:[~2019-07-25 23:27 UTC | newest]
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).