| Message ID | 1598357182-4226-4-git-send-email-yash.shah@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SiFive DDR controller and EDAC support | expand |
On Tue, 25 Aug 2020 05:06:22 PDT (-0700), yash.shah@sifive.com wrote: > Add Memory controller EDAC support in exisiting SiFive platform EDAC > driver. It registers for notifier events from the SiFive DDR controller > driver for DDR ECC events. > > Signed-off-by: Yash Shah <yash.shah@sifive.com> > --- > drivers/edac/Kconfig | 2 +- > drivers/edac/sifive_edac.c | 117 +++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 118 insertions(+), 1 deletion(-) > > diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig > index 7b6ec30..f8b3b53 100644 > --- a/drivers/edac/Kconfig > +++ b/drivers/edac/Kconfig > @@ -462,7 +462,7 @@ config EDAC_ALTERA_SDMMC > > config EDAC_SIFIVE > bool "Sifive platform EDAC driver" > - depends on EDAC=y && SIFIVE_L2 > + depends on EDAC=y && (SIFIVE_L2 || SIFIVE_DDR) > help > Support for error detection and correction on the SiFive SoCs. > > diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c > index 3a3dcb1..cf032685 100644 > --- a/drivers/edac/sifive_edac.c > +++ b/drivers/edac/sifive_edac.c > @@ -11,14 +11,120 @@ > #include <linux/platform_device.h> > #include "edac_module.h" > #include <soc/sifive/sifive_l2_cache.h> > +#include <soc/sifive/sifive_ddr.h> > > #define DRVNAME "sifive_edac" > +#define SIFIVE_EDAC_MOD_NAME "Sifive ECC Manager" > > struct sifive_edac_priv { > struct notifier_block notifier; > struct edac_device_ctl_info *dci; > }; > > +struct sifive_edac_mc_priv { > + struct notifier_block notifier; > + struct mem_ctl_info *mci; > +}; > + > +/** > + * EDAC MC error callback > + * > + * @event: non-zero if unrecoverable. > + */ > +static > +int ecc_mc_err_event(struct notifier_block *this, unsigned long event, void *ptr) > +{ > + struct sifive_ddr_priv *priv = ptr; > + struct sifive_edac_mc_priv *p; > + > + p = container_of(this, struct sifive_edac_mc_priv, notifier); > + if (event == SIFIVE_DDR_ERR_TYPE_UE) { > + edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, p->mci, > + priv->error_count, priv->page_frame_number, > + priv->offset_in_page, priv->syndrome, > + priv->top_layer, priv->mid_layer, > + priv->low_layer, p->mci->ctl_name, ""); > + } else if (event == SIFIVE_DDR_ERR_TYPE_CE) { > + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, p->mci, > + priv->error_count, priv->page_frame_number, > + priv->offset_in_page, priv->syndrome, > + priv->top_layer, priv->mid_layer, > + priv->low_layer, p->mci->ctl_name, ""); > + } > + > + return NOTIFY_OK; > +} > + > +static int ecc_mc_register(struct platform_device *pdev) > +{ > + struct sifive_edac_mc_priv *p; > + struct edac_mc_layer layers[1]; > + int ret; > + > + p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL); > + if (!p) > + return -ENOMEM; > + > + p->notifier.notifier_call = ecc_mc_err_event; > + platform_set_drvdata(pdev, p); > + > + layers[0].type = EDAC_MC_LAYER_CHIP_SELECT; > + layers[0].size = 1; > + layers[0].is_virt_csrow = true; > + > + p->mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, 0); > + if (!p->mci) { > + dev_err(&pdev->dev, "Failed mem allocation for mc instance\n"); > + return -ENOMEM; > + } > + > + p->mci->pdev = &pdev->dev; > + /* Initialize controller capabilities */ > + p->mci->mtype_cap = MEM_FLAG_DDR4; > + p->mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED; > + p->mci->edac_cap = EDAC_FLAG_SECDED; > + p->mci->scrub_cap = SCRUB_UNKNOWN; > + p->mci->scrub_mode = SCRUB_HW_PROG; > + p->mci->ctl_name = dev_name(&pdev->dev); > + p->mci->dev_name = dev_name(&pdev->dev); > + p->mci->mod_name = SIFIVE_EDAC_MOD_NAME; > + p->mci->ctl_page_to_phys = NULL; > + > + /* Interrupt feature is supported by cadence mc */ > + edac_op_state = EDAC_OPSTATE_INT; > + > + ret = edac_mc_add_mc(p->mci); > + if (ret) { > + edac_printk(KERN_ERR, SIFIVE_EDAC_MOD_NAME, > + "Failed to register with EDAC core\n"); > + goto err; > + } > + > +#ifdef CONFIG_SIFIVE_DDR > + register_sifive_ddr_error_notifier(&p->notifier); > +#endif > + > + return 0; > + > +err: > + edac_mc_free(p->mci); > + > + return -ENXIO; > +} > + > +static int ecc_mc_unregister(struct platform_device *pdev) > +{ > + struct sifive_edac_mc_priv *p = platform_get_drvdata(pdev); > + > +#ifdef CONFIG_SIFIVE_DDR > + unregister_sifive_ddr_error_notifier(&p->notifier); > +#endif > + edac_mc_del_mc(&pdev->dev); > + edac_mc_free(p->mci); > + > + return 0; > +} > + > /** > * EDAC error callback > * > @@ -67,7 +173,9 @@ static int ecc_register(struct platform_device *pdev) > goto err; > } > > +#ifdef CONFIG_SIFIVE_L2 > register_sifive_l2_error_notifier(&p->notifier); > +#endif > > return 0; > > @@ -81,7 +189,9 @@ static int ecc_unregister(struct platform_device *pdev) > { > struct sifive_edac_priv *p = platform_get_drvdata(pdev); > > +#ifdef CONFIG_SIFIVE_L2 > unregister_sifive_l2_error_notifier(&p->notifier); > +#endif > edac_device_del_device(&pdev->dev); > edac_device_free_ctl_info(p->dci); > > @@ -102,12 +212,19 @@ static int __init sifive_edac_init(void) > if (ret) > platform_device_unregister(sifive_pdev); > > + ret = ecc_mc_register(sifive_pdev); > + if (ret) { > + ecc_unregister(sifive_pdev); > + platform_device_unregister(sifive_pdev); > + } > + > return ret; > } > > static void __exit sifive_edac_exit(void) > { > ecc_unregister(sifive_pdev); > + ecc_mc_unregister(sifive_pdev); > platform_device_unregister(sifive_pdev); > } Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
> Subject: Re: [PATCH 3/3] edac: sifive: Add EDAC support for Memory Controller in SiFive SoCs Fix subject prefix: "EDAC/sifive: ..." On Tue, Aug 25, 2020 at 05:36:22PM +0530, Yash Shah wrote: > Add Memory controller EDAC support in exisiting SiFive platform EDAC s/in exisiting/to the/ > driver. It registers for notifier events from the SiFive DDR controller > driver for DDR ECC events. Simplify: "It registers for ECC notifier events from the memory controller." > Signed-off-by: Yash Shah <yash.shah@sifive.com> > --- > drivers/edac/Kconfig | 2 +- > drivers/edac/sifive_edac.c | 117 +++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 118 insertions(+), 1 deletion(-) > > diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig > index 7b6ec30..f8b3b53 100644 > --- a/drivers/edac/Kconfig > +++ b/drivers/edac/Kconfig > @@ -462,7 +462,7 @@ config EDAC_ALTERA_SDMMC > > config EDAC_SIFIVE > bool "Sifive platform EDAC driver" > - depends on EDAC=y && SIFIVE_L2 > + depends on EDAC=y && (SIFIVE_L2 || SIFIVE_DDR) > help > Support for error detection and correction on the SiFive SoCs. > > diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c > index 3a3dcb1..cf032685 100644 > --- a/drivers/edac/sifive_edac.c > +++ b/drivers/edac/sifive_edac.c > @@ -11,14 +11,120 @@ > #include <linux/platform_device.h> > #include "edac_module.h" > #include <soc/sifive/sifive_l2_cache.h> > +#include <soc/sifive/sifive_ddr.h> > > #define DRVNAME "sifive_edac" > +#define SIFIVE_EDAC_MOD_NAME "Sifive ECC Manager" s/SIFIVE_EDAC_MOD_NAME/EDAC_MOD_NAME/g like the other EDAC drivers. ... > +static int ecc_mc_register(struct platform_device *pdev) > +{ > + struct sifive_edac_mc_priv *p; > + struct edac_mc_layer layers[1]; > + int ret; > + > + p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL); > + if (!p) > + return -ENOMEM; > + > + p->notifier.notifier_call = ecc_mc_err_event; > + platform_set_drvdata(pdev, p); > + > + layers[0].type = EDAC_MC_LAYER_CHIP_SELECT; > + layers[0].size = 1; > + layers[0].is_virt_csrow = true; > + > + p->mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, 0); > + if (!p->mci) { > + dev_err(&pdev->dev, "Failed mem allocation for mc instance\n"); > + return -ENOMEM; > + } > + > + p->mci->pdev = &pdev->dev; > + /* Initialize controller capabilities */ > + p->mci->mtype_cap = MEM_FLAG_DDR4; > + p->mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED; > + p->mci->edac_cap = EDAC_FLAG_SECDED; > + p->mci->scrub_cap = SCRUB_UNKNOWN; > + p->mci->scrub_mode = SCRUB_HW_PROG; > + p->mci->ctl_name = dev_name(&pdev->dev); > + p->mci->dev_name = dev_name(&pdev->dev); > + p->mci->mod_name = SIFIVE_EDAC_MOD_NAME; > + p->mci->ctl_page_to_phys = NULL; > + > + /* Interrupt feature is supported by cadence mc */ > + edac_op_state = EDAC_OPSTATE_INT; > + > + ret = edac_mc_add_mc(p->mci); > + if (ret) { > + edac_printk(KERN_ERR, SIFIVE_EDAC_MOD_NAME, > + "Failed to register with EDAC core\n"); > + goto err; > + } > + > +#ifdef CONFIG_SIFIVE_DDR It seems all that ifdeffery can be replaced with if (IS_ENABLED(CONFIG_...)) Thx.
> -----Original Message----- > From: Borislav Petkov <bp@alien8.de> > Sent: 31 August 2020 14:22 > To: Yash Shah <yash.shah@openfive.com> > Cc: robh+dt@kernel.org; palmer@dabbelt.com; Paul Walmsley ( Sifive) > <paul.walmsley@sifive.com>; mchehab@kernel.org; tony.luck@intel.com; > aou@eecs.berkeley.edu; james.morse@arm.com; rrichter@marvell.com; > devicetree@vger.kernel.org; linux-riscv@lists.infradead.org; linux- > kernel@vger.kernel.org; linux-edac@vger.kernel.org; Sachin Ghadi > <sachin.ghadi@openfive.com> > Subject: Re: [PATCH 3/3] edac: sifive: Add EDAC support for Memory > Controller in SiFive SoCs > > [External Email] Do not click links or attachments unless you recognize the > sender and know the content is safe > > > Subject: Re: [PATCH 3/3] edac: sifive: Add EDAC support for Memory > > Controller in SiFive SoCs > > Fix subject prefix: "EDAC/sifive: ..." > > On Tue, Aug 25, 2020 at 05:36:22PM +0530, Yash Shah wrote: > > Add Memory controller EDAC support in exisiting SiFive platform EDAC > > s/in exisiting/to the/ > > > driver. It registers for notifier events from the SiFive DDR > > controller driver for DDR ECC events. > > Simplify: > > "It registers for ECC notifier events from the memory controller." > > > Signed-off-by: Yash Shah <yash.shah@sifive.com> > > --- > > drivers/edac/Kconfig | 2 +- > > drivers/edac/sifive_edac.c | 117 > > +++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 118 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig index > > 7b6ec30..f8b3b53 100644 > > --- a/drivers/edac/Kconfig > > +++ b/drivers/edac/Kconfig > > @@ -462,7 +462,7 @@ config EDAC_ALTERA_SDMMC > > > > config EDAC_SIFIVE > > bool "Sifive platform EDAC driver" > > - depends on EDAC=y && SIFIVE_L2 > > + depends on EDAC=y && (SIFIVE_L2 || SIFIVE_DDR) > > help > > Support for error detection and correction on the SiFive SoCs. > > > > diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c > > index 3a3dcb1..cf032685 100644 > > --- a/drivers/edac/sifive_edac.c > > +++ b/drivers/edac/sifive_edac.c > > @@ -11,14 +11,120 @@ > > #include <linux/platform_device.h> > > #include "edac_module.h" > > #include <soc/sifive/sifive_l2_cache.h> > > +#include <soc/sifive/sifive_ddr.h> > > > > #define DRVNAME "sifive_edac" > > +#define SIFIVE_EDAC_MOD_NAME "Sifive ECC Manager" > > s/SIFIVE_EDAC_MOD_NAME/EDAC_MOD_NAME/g > > like the other EDAC drivers. > Sure, will make all the above suggested textual changes in v2. > ... > > > +static int ecc_mc_register(struct platform_device *pdev) { > > + struct sifive_edac_mc_priv *p; > > + struct edac_mc_layer layers[1]; > > + int ret; > > + > > + p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL); > > + if (!p) > > + return -ENOMEM; > > + > > + p->notifier.notifier_call = ecc_mc_err_event; > > + platform_set_drvdata(pdev, p); > > + > > + layers[0].type = EDAC_MC_LAYER_CHIP_SELECT; > > + layers[0].size = 1; > > + layers[0].is_virt_csrow = true; > > + > > + p->mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, 0); > > + if (!p->mci) { > > + dev_err(&pdev->dev, "Failed mem allocation for mc instance\n"); > > + return -ENOMEM; > > + } > > + > > + p->mci->pdev = &pdev->dev; > > + /* Initialize controller capabilities */ > > + p->mci->mtype_cap = MEM_FLAG_DDR4; > > + p->mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED; > > + p->mci->edac_cap = EDAC_FLAG_SECDED; > > + p->mci->scrub_cap = SCRUB_UNKNOWN; > > + p->mci->scrub_mode = SCRUB_HW_PROG; > > + p->mci->ctl_name = dev_name(&pdev->dev); > > + p->mci->dev_name = dev_name(&pdev->dev); > > + p->mci->mod_name = SIFIVE_EDAC_MOD_NAME; > > + p->mci->ctl_page_to_phys = NULL; > > + > > + /* Interrupt feature is supported by cadence mc */ > > + edac_op_state = EDAC_OPSTATE_INT; > > + > > + ret = edac_mc_add_mc(p->mci); > > + if (ret) { > > + edac_printk(KERN_ERR, SIFIVE_EDAC_MOD_NAME, > > + "Failed to register with EDAC core\n"); > > + goto err; > > + } > > + > > +#ifdef CONFIG_SIFIVE_DDR > > It seems all that ifdeffery can be replaced with > > if (IS_ENABLED(CONFIG_...)) Yes, will replace all the ifdeffery in v2 Thanks for the review. - Yash > > Thx. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig index 7b6ec30..f8b3b53 100644 --- a/drivers/edac/Kconfig +++ b/drivers/edac/Kconfig @@ -462,7 +462,7 @@ config EDAC_ALTERA_SDMMC config EDAC_SIFIVE bool "Sifive platform EDAC driver" - depends on EDAC=y && SIFIVE_L2 + depends on EDAC=y && (SIFIVE_L2 || SIFIVE_DDR) help Support for error detection and correction on the SiFive SoCs. diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c index 3a3dcb1..cf032685 100644 --- a/drivers/edac/sifive_edac.c +++ b/drivers/edac/sifive_edac.c @@ -11,14 +11,120 @@ #include <linux/platform_device.h> #include "edac_module.h" #include <soc/sifive/sifive_l2_cache.h> +#include <soc/sifive/sifive_ddr.h> #define DRVNAME "sifive_edac" +#define SIFIVE_EDAC_MOD_NAME "Sifive ECC Manager" struct sifive_edac_priv { struct notifier_block notifier; struct edac_device_ctl_info *dci; }; +struct sifive_edac_mc_priv { + struct notifier_block notifier; + struct mem_ctl_info *mci; +}; + +/** + * EDAC MC error callback + * + * @event: non-zero if unrecoverable. + */ +static +int ecc_mc_err_event(struct notifier_block *this, unsigned long event, void *ptr) +{ + struct sifive_ddr_priv *priv = ptr; + struct sifive_edac_mc_priv *p; + + p = container_of(this, struct sifive_edac_mc_priv, notifier); + if (event == SIFIVE_DDR_ERR_TYPE_UE) { + edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, p->mci, + priv->error_count, priv->page_frame_number, + priv->offset_in_page, priv->syndrome, + priv->top_layer, priv->mid_layer, + priv->low_layer, p->mci->ctl_name, ""); + } else if (event == SIFIVE_DDR_ERR_TYPE_CE) { + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, p->mci, + priv->error_count, priv->page_frame_number, + priv->offset_in_page, priv->syndrome, + priv->top_layer, priv->mid_layer, + priv->low_layer, p->mci->ctl_name, ""); + } + + return NOTIFY_OK; +} + +static int ecc_mc_register(struct platform_device *pdev) +{ + struct sifive_edac_mc_priv *p; + struct edac_mc_layer layers[1]; + int ret; + + p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL); + if (!p) + return -ENOMEM; + + p->notifier.notifier_call = ecc_mc_err_event; + platform_set_drvdata(pdev, p); + + layers[0].type = EDAC_MC_LAYER_CHIP_SELECT; + layers[0].size = 1; + layers[0].is_virt_csrow = true; + + p->mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, 0); + if (!p->mci) { + dev_err(&pdev->dev, "Failed mem allocation for mc instance\n"); + return -ENOMEM; + } + + p->mci->pdev = &pdev->dev; + /* Initialize controller capabilities */ + p->mci->mtype_cap = MEM_FLAG_DDR4; + p->mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED; + p->mci->edac_cap = EDAC_FLAG_SECDED; + p->mci->scrub_cap = SCRUB_UNKNOWN; + p->mci->scrub_mode = SCRUB_HW_PROG; + p->mci->ctl_name = dev_name(&pdev->dev); + p->mci->dev_name = dev_name(&pdev->dev); + p->mci->mod_name = SIFIVE_EDAC_MOD_NAME; + p->mci->ctl_page_to_phys = NULL; + + /* Interrupt feature is supported by cadence mc */ + edac_op_state = EDAC_OPSTATE_INT; + + ret = edac_mc_add_mc(p->mci); + if (ret) { + edac_printk(KERN_ERR, SIFIVE_EDAC_MOD_NAME, + "Failed to register with EDAC core\n"); + goto err; + } + +#ifdef CONFIG_SIFIVE_DDR + register_sifive_ddr_error_notifier(&p->notifier); +#endif + + return 0; + +err: + edac_mc_free(p->mci); + + return -ENXIO; +} + +static int ecc_mc_unregister(struct platform_device *pdev) +{ + struct sifive_edac_mc_priv *p = platform_get_drvdata(pdev); + +#ifdef CONFIG_SIFIVE_DDR + unregister_sifive_ddr_error_notifier(&p->notifier); +#endif + edac_mc_del_mc(&pdev->dev); + edac_mc_free(p->mci); + + return 0; +} + /** * EDAC error callback * @@ -67,7 +173,9 @@ static int ecc_register(struct platform_device *pdev) goto err; } +#ifdef CONFIG_SIFIVE_L2 register_sifive_l2_error_notifier(&p->notifier); +#endif return 0; @@ -81,7 +189,9 @@ static int ecc_unregister(struct platform_device *pdev) { struct sifive_edac_priv *p = platform_get_drvdata(pdev); +#ifdef CONFIG_SIFIVE_L2 unregister_sifive_l2_error_notifier(&p->notifier); +#endif edac_device_del_device(&pdev->dev); edac_device_free_ctl_info(p->dci); @@ -102,12 +212,19 @@ static int __init sifive_edac_init(void) if (ret) platform_device_unregister(sifive_pdev); + ret = ecc_mc_register(sifive_pdev); + if (ret) { + ecc_unregister(sifive_pdev); + platform_device_unregister(sifive_pdev); + } + return ret; } static void __exit sifive_edac_exit(void) { ecc_unregister(sifive_pdev); + ecc_mc_unregister(sifive_pdev); platform_device_unregister(sifive_pdev); }
Add Memory controller EDAC support in exisiting SiFive platform EDAC driver. It registers for notifier events from the SiFive DDR controller driver for DDR ECC events. Signed-off-by: Yash Shah <yash.shah@sifive.com> --- drivers/edac/Kconfig | 2 +- drivers/edac/sifive_edac.c | 117 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-)