| Message ID | 1468512408-5156-6-git-send-email-tthayer@opensource.altera.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 14, 2016 at 11:06:43AM -0500, tthayer@opensource.altera.com wrote: > From: Thor Thayer <tthayer@opensource.altera.com> > > Add Altera Arria10 NAND FIFO memory EDAC support. > > Signed-off-by: Thor Thayer <tthayer@opensource.altera.com> > --- > drivers/edac/Kconfig | 7 +++++++ > drivers/edac/altera_edac.c | 34 +++++++++++++++++++++++++++++++++- > 2 files changed, 40 insertions(+), 1 deletion(-) ... > @@ -1589,7 +1619,9 @@ static int altr_edac_a10_probe(struct platform_device *pdev) > else if ((of_device_is_compatible(child, > "altr,socfpga-a10-ocram-ecc")) || > (of_device_is_compatible(child, > - "altr,socfpga-eth-mac-ecc"))) > + "altr,socfpga-eth-mac-ecc")) || > + (of_device_is_compatible(child, > + "altr,socfpga-nand-ecc"))) > altr_edac_a10_device_add(edac, child); > else if (of_device_is_compatible(child, > "altr,sdram-edac-a10")) Can we simplify this loop like this? for_each_child_of_node(pdev->dev.of_node, child) { if (!of_device_is_available(child)) continue; if (of_device_is_compatible(child, "altr,socfpga-a10-l2-ecc") || of_device_is_compatible(child, "altr,socfpga-a10-ocram-ecc") || of_device_is_compatible(child, "altr,socfpga-eth-mac-ecc") || of_device_is_compatible(child, "altr,socfpga-nand-ecc")) altr_edac_a10_device_add(edac, child); 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); } I've merged the first "if" and subsequent "else if" because they all do altr_edac_a10_device_add(edac, child) and added spacing for better readability. Look ok? Or have I fatfingered it?
On 07/27/2016 12:10 PM, Borislav Petkov wrote: > On Thu, Jul 14, 2016 at 11:06:43AM -0500, tthayer@opensource.altera.com wrote: >> From: Thor Thayer <tthayer@opensource.altera.com> >> >> Add Altera Arria10 NAND FIFO memory EDAC support. >> >> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com> >> --- >> drivers/edac/Kconfig | 7 +++++++ >> drivers/edac/altera_edac.c | 34 +++++++++++++++++++++++++++++++++- >> 2 files changed, 40 insertions(+), 1 deletion(-) > > ... > >> @@ -1589,7 +1619,9 @@ static int altr_edac_a10_probe(struct platform_device *pdev) >> else if ((of_device_is_compatible(child, >> "altr,socfpga-a10-ocram-ecc")) || >> (of_device_is_compatible(child, >> - "altr,socfpga-eth-mac-ecc"))) >> + "altr,socfpga-eth-mac-ecc")) || >> + (of_device_is_compatible(child, >> + "altr,socfpga-nand-ecc"))) >> altr_edac_a10_device_add(edac, child); >> else if (of_device_is_compatible(child, >> "altr,sdram-edac-a10")) > > Can we simplify this loop like this? > > for_each_child_of_node(pdev->dev.of_node, child) { > if (!of_device_is_available(child)) > continue; > > if (of_device_is_compatible(child, "altr,socfpga-a10-l2-ecc") || > of_device_is_compatible(child, "altr,socfpga-a10-ocram-ecc") || > of_device_is_compatible(child, "altr,socfpga-eth-mac-ecc") || > of_device_is_compatible(child, "altr,socfpga-nand-ecc")) > > altr_edac_a10_device_add(edac, child); > > 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); > } > > I've merged the first "if" and subsequent "else if" because they all > do altr_edac_a10_device_add(edac, child) and added spacing for better > readability. > > Look ok? > > Or have I fatfingered it? > Yes, that's better. I was trying to stay within the 80 character limit but missed the if/else if improvement. Thanks, Boris! Should I re-submit? Thanks, Thor
On Wed, Jul 27, 2016 at 01:43:04PM -0500, Thor Thayer wrote: > Yes, that's better. I was trying to stay within the 80 character limit but > missed the if/else if improvement. Thanks, Boris! > > Should I re-submit? Nah, no need. I'd only ask you to test the final result once I've pushed it out after the merge window is over. Thanks.
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig index d0c1dab..47378b3 100644 --- a/drivers/edac/Kconfig +++ b/drivers/edac/Kconfig @@ -398,6 +398,13 @@ config EDAC_ALTERA_ETHERNET Support for error detection and correction on the Altera Ethernet FIFO Memory for Altera SoCs. +config EDAC_ALTERA_NAND + bool "Altera NAND FIFO ECC" + depends on EDAC_ALTERA=y && MTD_NAND_DENALI + help + Support for error detection and correction on the + Altera NAND FIFO Memory for Altera SoCs. + config EDAC_SYNOPSYS tristate "Synopsys DDR Memory Controller" depends on EDAC_MM_EDAC && ARCH_ZYNQ diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c index 2398d07..35d87d1 100644 --- a/drivers/edac/altera_edac.c +++ b/drivers/edac/altera_edac.c @@ -1285,6 +1285,33 @@ early_initcall(socfpga_init_ethernet_ecc); #endif /* CONFIG_EDAC_ALTERA_ETHERNET */ +/********************** NAND Device Functions **********************/ + +#ifdef CONFIG_EDAC_ALTERA_NAND + +static const struct edac_device_prv_data a10_nandecc_data = { + .setup = altr_check_ecc_deps, + .ce_clear_mask = ALTR_A10_ECC_SERRPENA, + .ue_clear_mask = ALTR_A10_ECC_DERRPENA, + .dbgfs_name = "altr_trigger", + .ecc_enable_mask = ALTR_A10_COMMON_ECC_EN_CTL, + .ecc_en_ofst = ALTR_A10_ECC_CTRL_OFST, + .ce_set_mask = ALTR_A10_ECC_TSERRA, + .ue_set_mask = ALTR_A10_ECC_TDERRA, + .set_err_ofst = ALTR_A10_ECC_INTTEST_OFST, + .ecc_irq_handler = altr_edac_a10_ecc_irq, + .inject_fops = &altr_edac_a10_device_inject_fops, +}; + +static int __init socfpga_init_nand_ecc(void) +{ + return altr_init_a10_ecc_device_type("altr,socfpga-nand-ecc"); +} + +early_initcall(socfpga_init_nand_ecc); + +#endif /* CONFIG_EDAC_ALTERA_NAND */ + /********************* Arria10 EDAC Device Functions *************************/ static const struct of_device_id altr_edac_a10_device_of_match[] = { #ifdef CONFIG_EDAC_ALTERA_L2C @@ -1298,6 +1325,9 @@ static const struct of_device_id altr_edac_a10_device_of_match[] = { { .compatible = "altr,socfpga-eth-mac-ecc", .data = &a10_enetecc_data }, #endif +#ifdef CONFIG_EDAC_ALTERA_NAND + { .compatible = "altr,socfpga-nand-ecc", .data = &a10_nandecc_data }, +#endif {}, }; MODULE_DEVICE_TABLE(of, altr_edac_a10_device_of_match); @@ -1589,7 +1619,9 @@ static int altr_edac_a10_probe(struct platform_device *pdev) else if ((of_device_is_compatible(child, "altr,socfpga-a10-ocram-ecc")) || (of_device_is_compatible(child, - "altr,socfpga-eth-mac-ecc"))) + "altr,socfpga-eth-mac-ecc")) || + (of_device_is_compatible(child, + "altr,socfpga-nand-ecc"))) altr_edac_a10_device_add(edac, child); else if (of_device_is_compatible(child, "altr,sdram-edac-a10"))