* [PATCH v5 0/2] Add EDAC driver for loongson memory controller @ 2024-09-25 2:40 Zhao Qunqin 2024-09-25 2:40 ` [PATCH v5 1/2] dt-bindings: EDAC for ls3a5000 " Zhao Qunqin 2024-09-25 2:40 ` [PATCH v5 2/2] Loongarch: EDAC driver for loongson " Zhao Qunqin 0 siblings, 2 replies; 7+ messages in thread From: Zhao Qunqin @ 2024-09-25 2:40 UTC (permalink / raw) To: robh, krzk+dt, conor+dt, chenhuacai, bp, tony.luck Cc: linux-edac, devicetree, linux-kernel, kernel, james.morse, mchehab, rric, loongarch, Zhao Qunqin Add a simple EDAC driver which report single bit errors (CE) only on loongson platform. Zhao Qunqin (2): dt-bindings: EDAC for ls3a5000 memory controller Loongarch: EDAC driver for loongson memory controller .../edac/loongson,ls3a5000-mc-edac.yaml | 44 +++++ MAINTAINERS | 7 + arch/loongarch/Kconfig | 1 + drivers/edac/Kconfig | 8 + drivers/edac/Makefile | 1 + drivers/edac/loongson_edac.c | 168 ++++++++++++++++++ 6 files changed, 229 insertions(+) create mode 100644 Documentation/devicetree/bindings/edac/loongson,ls3a5000-mc-edac.yaml create mode 100644 drivers/edac/loongson_edac.c base-commit: 61124f42dcaa30f58a8b47a2b69ddb80260677c7 -- 2.43.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 1/2] dt-bindings: EDAC for ls3a5000 memory controller 2024-09-25 2:40 [PATCH v5 0/2] Add EDAC driver for loongson memory controller Zhao Qunqin @ 2024-09-25 2:40 ` Zhao Qunqin 2024-09-25 2:40 ` [PATCH v5 2/2] Loongarch: EDAC driver for loongson " Zhao Qunqin 1 sibling, 0 replies; 7+ messages in thread From: Zhao Qunqin @ 2024-09-25 2:40 UTC (permalink / raw) To: robh, krzk+dt, conor+dt, chenhuacai, bp, tony.luck Cc: linux-edac, devicetree, linux-kernel, kernel, james.morse, mchehab, rric, loongarch, Zhao Qunqin, Krzysztof Kozlowski add device tree bindings for ls3a5000 EDAC driver. Signed-off-by: Zhao Qunqin <zhaoqunqin@loongson.cn> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Changes in v5: - None Changes in v4: - Added tag from Krzysztof (thanks!) Changes in v3: - None Changes in v2: - Fixed the errors raised by Rob - Addressed review comments raised by Krzysztof and Conor .../edac/loongson,ls3a5000-mc-edac.yaml | 44 +++++++++++++++++++ MAINTAINERS | 6 +++ 2 files changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/edac/loongson,ls3a5000-mc-edac.yaml diff --git a/Documentation/devicetree/bindings/edac/loongson,ls3a5000-mc-edac.yaml b/Documentation/devicetree/bindings/edac/loongson,ls3a5000-mc-edac.yaml new file mode 100644 index 000000000..52f10bc94 --- /dev/null +++ b/Documentation/devicetree/bindings/edac/loongson,ls3a5000-mc-edac.yaml @@ -0,0 +1,44 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/edac/loongson,ls3a5000-mc-edac.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Loongson Memory Controller EDAC + +maintainers: + - Zhao Qunqin <zhaoqunqin@loongson.cn> + +description: | + EDAC node is defined to describe on-chip error detection and correction for + Loongson Memory Controller. + +properties: + compatible: + oneOf: + - enum: + - loongson,ls3a5000-mc-edac + - items: + - enum: + - loongson,ls3c5000l-mc-edac + - loongson,ls3c5000-mc-edac + - loongson,ls3d5000-mc-edac + - loongson,ls3a6000-mc-edac + - loongson,ls3c6000-mc-edac + - const: loongson,ls3a5000-mc-edac + + reg: + maxItems: 1 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + memory-controller@1fe00600 { + compatible = "loongson,ls3a5000-mc-edac"; + reg = <0x1fe00600 0x50>; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 42decde38..6cc8cfc8f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13237,6 +13237,12 @@ S: Maintained F: Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml F: drivers/thermal/loongson2_thermal.c +LOONGSON EDAC DRIVER +M: Zhao Qunqin <zhaoqunqin@loongson.cn> +L: linux-edac@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/edac/loongson,ls3a5000-mc-edac.yaml + LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) M: Sathya Prakash <sathya.prakash@broadcom.com> M: Sreekanth Reddy <sreekanth.reddy@broadcom.com> -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v5 2/2] Loongarch: EDAC driver for loongson memory controller 2024-09-25 2:40 [PATCH v5 0/2] Add EDAC driver for loongson memory controller Zhao Qunqin 2024-09-25 2:40 ` [PATCH v5 1/2] dt-bindings: EDAC for ls3a5000 " Zhao Qunqin @ 2024-09-25 2:40 ` Zhao Qunqin 2024-09-25 4:46 ` Huacai Chen 2024-09-25 9:13 ` Jonathan Cameron 1 sibling, 2 replies; 7+ messages in thread From: Zhao Qunqin @ 2024-09-25 2:40 UTC (permalink / raw) To: robh, krzk+dt, conor+dt, chenhuacai, bp, tony.luck Cc: linux-edac, devicetree, linux-kernel, kernel, james.morse, mchehab, rric, loongarch, Zhao Qunqin Reports single bit errors (CE) only. Signed-off-by: Zhao Qunqin <zhaoqunqin@loongson.cn> --- Changes in v5: - Drop the loongson_ prefix from all static functions. - Align function arguments on the opening brace. - Drop useless comments and useless wrapper. Drop side comments. - Reorder variable declarations. Changes in v4: - None Changes in v3: - Addressed review comments raised by Krzysztof and Huacai Changes in v2: - Addressed review comments raised by Krzysztof MAINTAINERS | 1 + arch/loongarch/Kconfig | 1 + drivers/edac/Kconfig | 8 ++ drivers/edac/Makefile | 1 + drivers/edac/loongson_edac.c | 168 +++++++++++++++++++++++++++++++++++ 5 files changed, 179 insertions(+) create mode 100644 drivers/edac/loongson_edac.c diff --git a/MAINTAINERS b/MAINTAINERS index 6cc8cfc8f..5b4526638 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13242,6 +13242,7 @@ M: Zhao Qunqin <zhaoqunqin@loongson.cn> L: linux-edac@vger.kernel.org S: Maintained F: Documentation/devicetree/bindings/edac/loongson,ls3a5000-mc-edac.yaml +F: drivers/edac/loongson_edac.c LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) M: Sathya Prakash <sathya.prakash@broadcom.com> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig index 70f169210..9c135f1a2 100644 --- a/arch/loongarch/Kconfig +++ b/arch/loongarch/Kconfig @@ -181,6 +181,7 @@ config LOONGARCH select PCI_MSI_ARCH_FALLBACKS select PCI_QUIRKS select PERF_USE_VMALLOC + select EDAC_SUPPORT select RTC_LIB select SPARSE_IRQ select SYSCTL_ARCH_UNALIGN_ALLOW diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig index 81af6c344..719bb6ca7 100644 --- a/drivers/edac/Kconfig +++ b/drivers/edac/Kconfig @@ -564,5 +564,13 @@ config EDAC_VERSAL Support injecting both correctable and uncorrectable errors for debugging purposes. +config EDAC_LOONGSON3 + tristate "Loongson-3 Memory Controller" + depends on LOONGARCH || COMPILE_TEST + help + Support for error detection and correction on the Loongson-3 + family memory controller. This driver reports single bit + errors (CE) only. Loongson-3A5000/3C5000/3D5000/3C5000L/3A6000/3C6000 + are compatible. endif # EDAC diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile index faf310eec..e72ca1be4 100644 --- a/drivers/edac/Makefile +++ b/drivers/edac/Makefile @@ -88,3 +88,4 @@ obj-$(CONFIG_EDAC_DMC520) += dmc520_edac.o obj-$(CONFIG_EDAC_NPCM) += npcm_edac.o obj-$(CONFIG_EDAC_ZYNQMP) += zynqmp_edac.o obj-$(CONFIG_EDAC_VERSAL) += versal_edac.o +obj-$(CONFIG_EDAC_LOONGSON3) += loongson_edac.o diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c new file mode 100644 index 000000000..2721dfba5 --- /dev/null +++ b/drivers/edac/loongson_edac.c @@ -0,0 +1,168 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2024 Loongson Technology Corporation Limited. + */ + +#include <linux/edac.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/platform_device.h> + +#include "edac_module.h" + +enum ecc_index { + ECC_SET = 0, + ECC_RESERVED, + ECC_COUNT, + ECC_CS_COUNT, + ECC_CODE, + ECC_ADDR, + ECC_DATA0, + ECC_DATA1, + ECC_DATA2, + ECC_DATA3, +}; + +struct loongson_edac_pvt { + u64 *ecc_base; + int last_ce_count; +}; + +static int read_ecc(struct mem_ctl_info *mci) +{ + struct loongson_edac_pvt *pvt = mci->pvt_info; + u64 ecc; + int cs; + + if (!pvt->ecc_base) + return pvt->last_ce_count; + + ecc = pvt->ecc_base[ECC_CS_COUNT]; + /* cs0 -- cs3 */ + cs = ecc & 0xff; + cs += (ecc >> 8) & 0xff; + cs += (ecc >> 16) & 0xff; + cs += (ecc >> 24) & 0xff; + + return cs; +} + +static void edac_check(struct mem_ctl_info *mci) +{ + struct loongson_edac_pvt *pvt = mci->pvt_info; + int new, add; + + new = read_ecc(mci); + add = new - pvt->last_ce_count; + pvt->last_ce_count = new; + if (add <= 0) + return; + + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci, add, + 0, 0, 0, 0, 0, -1, "error", ""); + edac_mc_printk(mci, KERN_INFO, "add: %d", add); +} + +static int get_dimm_config(struct mem_ctl_info *mci) +{ + struct dimm_info *dimm; + u32 size, npages; + + /* size not used */ + size = -1; + npages = MiB_TO_PAGES(size); + + dimm = edac_get_dimm(mci, 0, 0, 0); + dimm->nr_pages = npages; + snprintf(dimm->label, sizeof(dimm->label), + "MC#%uChannel#%u_DIMM#%u", mci->mc_idx, 0, 0); + dimm->grain = 8; + + return 0; +} + +static void pvt_init(struct mem_ctl_info *mci, u64 *vbase) +{ + struct loongson_edac_pvt *pvt = mci->pvt_info; + + pvt->ecc_base = vbase; + pvt->last_ce_count = read_ecc(mci); +} + +static int edac_probe(struct platform_device *pdev) +{ + struct edac_mc_layer layers[2]; + struct loongson_edac_pvt *pvt; + struct mem_ctl_info *mci; + u64 *vbase; + int ret; + + vbase = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(vbase)) + return PTR_ERR(vbase); + + /* allocate a new MC control structure */ + layers[0].type = EDAC_MC_LAYER_CHANNEL; + layers[0].size = 1; + layers[0].is_virt_csrow = false; + layers[1].type = EDAC_MC_LAYER_SLOT; + layers[1].size = 1; + layers[1].is_virt_csrow = true; + mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*pvt)); + if (mci == NULL) + return -ENOMEM; + + mci->mc_idx = edac_device_alloc_index(); + mci->mtype_cap = MEM_FLAG_RDDR4; + mci->edac_ctl_cap = EDAC_FLAG_NONE; + mci->edac_cap = EDAC_FLAG_NONE; + mci->mod_name = "loongson_edac.c"; + mci->ctl_name = "loongson_edac_ctl"; + mci->dev_name = "loongson_edac_dev"; + mci->ctl_page_to_phys = NULL; + mci->pdev = &pdev->dev; + mci->error_desc.grain = 8; + /* Set the function pointer to an actual operation function */ + mci->edac_check = edac_check; + + pvt_init(mci, vbase); + get_dimm_config(mci); + + ret = edac_mc_add_mc(mci); + if (ret) { + edac_dbg(0, "MC: failed edac_mc_add_mc()\n"); + edac_mc_free(mci); + return ret; + } + edac_op_state = EDAC_OPSTATE_POLL; + + return 0; +} + +static void edac_remove(struct platform_device *pdev) +{ + struct mem_ctl_info *mci = edac_mc_del_mc(&pdev->dev); + + if (mci) + edac_mc_free(mci); +} + +static const struct of_device_id loongson_edac_of_match[] = { + { .compatible = "loongson,ls3a5000-mc-edac", }, + {} +}; +MODULE_DEVICE_TABLE(of, loongson_edac_of_match); + +static struct platform_driver loongson_edac_driver = { + .probe = edac_probe, + .remove = edac_remove, + .driver = { + .name = "loongson-mc-edac", + .of_match_table = loongson_edac_of_match, + }, +}; +module_platform_driver(loongson_edac_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Zhao Qunqin <zhaoqunqin@loongson.cn>"); +MODULE_DESCRIPTION("EDAC driver for loongson memory controller"); -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] Loongarch: EDAC driver for loongson memory controller 2024-09-25 2:40 ` [PATCH v5 2/2] Loongarch: EDAC driver for loongson " Zhao Qunqin @ 2024-09-25 4:46 ` Huacai Chen 2024-09-25 6:17 ` Zhao Qunqin 2024-09-25 9:13 ` Jonathan Cameron 1 sibling, 1 reply; 7+ messages in thread From: Huacai Chen @ 2024-09-25 4:46 UTC (permalink / raw) To: Zhao Qunqin Cc: robh, krzk+dt, conor+dt, bp, tony.luck, linux-edac, devicetree, linux-kernel, kernel, james.morse, mchehab, rric, loongarch Hi, Qunqin, The title should be "EDAC: Add EDAC driver for loongson memory controller" On Wed, Sep 25, 2024 at 10:40 AM Zhao Qunqin <zhaoqunqin@loongson.cn> wrote: > > Reports single bit errors (CE) only. > > Signed-off-by: Zhao Qunqin <zhaoqunqin@loongson.cn> > --- > Changes in v5: > - Drop the loongson_ prefix from all static functions. > - Align function arguments on the opening brace. > - Drop useless comments and useless wrapper. Drop side comments. > - Reorder variable declarations. > > Changes in v4: > - None > > Changes in v3: > - Addressed review comments raised by Krzysztof and Huacai > > Changes in v2: > - Addressed review comments raised by Krzysztof > > MAINTAINERS | 1 + > arch/loongarch/Kconfig | 1 + > drivers/edac/Kconfig | 8 ++ > drivers/edac/Makefile | 1 + > drivers/edac/loongson_edac.c | 168 +++++++++++++++++++++++++++++++++++ > 5 files changed, 179 insertions(+) > create mode 100644 drivers/edac/loongson_edac.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 6cc8cfc8f..5b4526638 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -13242,6 +13242,7 @@ M: Zhao Qunqin <zhaoqunqin@loongson.cn> > L: linux-edac@vger.kernel.org > S: Maintained > F: Documentation/devicetree/bindings/edac/loongson,ls3a5000-mc-edac.yaml > +F: drivers/edac/loongson_edac.c > > LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) > M: Sathya Prakash <sathya.prakash@broadcom.com> > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig > index 70f169210..9c135f1a2 100644 > --- a/arch/loongarch/Kconfig > +++ b/arch/loongarch/Kconfig > @@ -181,6 +181,7 @@ config LOONGARCH > select PCI_MSI_ARCH_FALLBACKS > select PCI_QUIRKS > select PERF_USE_VMALLOC > + select EDAC_SUPPORT > select RTC_LIB > select SPARSE_IRQ > select SYSCTL_ARCH_UNALIGN_ALLOW > diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig > index 81af6c344..719bb6ca7 100644 > --- a/drivers/edac/Kconfig > +++ b/drivers/edac/Kconfig > @@ -564,5 +564,13 @@ config EDAC_VERSAL > Support injecting both correctable and uncorrectable errors > for debugging purposes. > > +config EDAC_LOONGSON3 > + tristate "Loongson-3 Memory Controller" > + depends on LOONGARCH || COMPILE_TEST > + help > + Support for error detection and correction on the Loongson-3 > + family memory controller. This driver reports single bit > + errors (CE) only. Loongson-3A5000/3C5000/3D5000/3C5000L/3A6000/3C6000 > + are compatible. > > endif # EDAC > diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile > index faf310eec..e72ca1be4 100644 > --- a/drivers/edac/Makefile > +++ b/drivers/edac/Makefile > @@ -88,3 +88,4 @@ obj-$(CONFIG_EDAC_DMC520) += dmc520_edac.o > obj-$(CONFIG_EDAC_NPCM) += npcm_edac.o > obj-$(CONFIG_EDAC_ZYNQMP) += zynqmp_edac.o > obj-$(CONFIG_EDAC_VERSAL) += versal_edac.o > +obj-$(CONFIG_EDAC_LOONGSON3) += loongson_edac.o Change the Kconfig name to CONFIG_EDAC_LOONGSON, or change the file name to loongson3_edac.c may be a little better. Huacai > diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c > new file mode 100644 > index 000000000..2721dfba5 > --- /dev/null > +++ b/drivers/edac/loongson_edac.c > @@ -0,0 +1,168 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2024 Loongson Technology Corporation Limited. > + */ > + > +#include <linux/edac.h> > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/platform_device.h> > + > +#include "edac_module.h" > + > +enum ecc_index { > + ECC_SET = 0, > + ECC_RESERVED, > + ECC_COUNT, > + ECC_CS_COUNT, > + ECC_CODE, > + ECC_ADDR, > + ECC_DATA0, > + ECC_DATA1, > + ECC_DATA2, > + ECC_DATA3, > +}; > + > +struct loongson_edac_pvt { > + u64 *ecc_base; > + int last_ce_count; > +}; > + > +static int read_ecc(struct mem_ctl_info *mci) > +{ > + struct loongson_edac_pvt *pvt = mci->pvt_info; > + u64 ecc; > + int cs; > + > + if (!pvt->ecc_base) > + return pvt->last_ce_count; > + > + ecc = pvt->ecc_base[ECC_CS_COUNT]; > + /* cs0 -- cs3 */ > + cs = ecc & 0xff; > + cs += (ecc >> 8) & 0xff; > + cs += (ecc >> 16) & 0xff; > + cs += (ecc >> 24) & 0xff; > + > + return cs; > +} > + > +static void edac_check(struct mem_ctl_info *mci) > +{ > + struct loongson_edac_pvt *pvt = mci->pvt_info; > + int new, add; > + > + new = read_ecc(mci); > + add = new - pvt->last_ce_count; > + pvt->last_ce_count = new; > + if (add <= 0) > + return; > + > + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci, add, > + 0, 0, 0, 0, 0, -1, "error", ""); > + edac_mc_printk(mci, KERN_INFO, "add: %d", add); > +} > + > +static int get_dimm_config(struct mem_ctl_info *mci) > +{ > + struct dimm_info *dimm; > + u32 size, npages; > + > + /* size not used */ > + size = -1; > + npages = MiB_TO_PAGES(size); > + > + dimm = edac_get_dimm(mci, 0, 0, 0); > + dimm->nr_pages = npages; > + snprintf(dimm->label, sizeof(dimm->label), > + "MC#%uChannel#%u_DIMM#%u", mci->mc_idx, 0, 0); > + dimm->grain = 8; > + > + return 0; > +} > + > +static void pvt_init(struct mem_ctl_info *mci, u64 *vbase) > +{ > + struct loongson_edac_pvt *pvt = mci->pvt_info; > + > + pvt->ecc_base = vbase; > + pvt->last_ce_count = read_ecc(mci); > +} > + > +static int edac_probe(struct platform_device *pdev) > +{ > + struct edac_mc_layer layers[2]; > + struct loongson_edac_pvt *pvt; > + struct mem_ctl_info *mci; > + u64 *vbase; > + int ret; > + > + vbase = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(vbase)) > + return PTR_ERR(vbase); > + > + /* allocate a new MC control structure */ > + layers[0].type = EDAC_MC_LAYER_CHANNEL; > + layers[0].size = 1; > + layers[0].is_virt_csrow = false; > + layers[1].type = EDAC_MC_LAYER_SLOT; > + layers[1].size = 1; > + layers[1].is_virt_csrow = true; > + mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*pvt)); > + if (mci == NULL) > + return -ENOMEM; > + > + mci->mc_idx = edac_device_alloc_index(); > + mci->mtype_cap = MEM_FLAG_RDDR4; > + mci->edac_ctl_cap = EDAC_FLAG_NONE; > + mci->edac_cap = EDAC_FLAG_NONE; > + mci->mod_name = "loongson_edac.c"; > + mci->ctl_name = "loongson_edac_ctl"; > + mci->dev_name = "loongson_edac_dev"; > + mci->ctl_page_to_phys = NULL; > + mci->pdev = &pdev->dev; > + mci->error_desc.grain = 8; > + /* Set the function pointer to an actual operation function */ > + mci->edac_check = edac_check; > + > + pvt_init(mci, vbase); > + get_dimm_config(mci); > + > + ret = edac_mc_add_mc(mci); > + if (ret) { > + edac_dbg(0, "MC: failed edac_mc_add_mc()\n"); > + edac_mc_free(mci); > + return ret; > + } > + edac_op_state = EDAC_OPSTATE_POLL; > + > + return 0; > +} > + > +static void edac_remove(struct platform_device *pdev) > +{ > + struct mem_ctl_info *mci = edac_mc_del_mc(&pdev->dev); > + > + if (mci) > + edac_mc_free(mci); > +} > + > +static const struct of_device_id loongson_edac_of_match[] = { > + { .compatible = "loongson,ls3a5000-mc-edac", }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, loongson_edac_of_match); > + > +static struct platform_driver loongson_edac_driver = { > + .probe = edac_probe, > + .remove = edac_remove, > + .driver = { > + .name = "loongson-mc-edac", > + .of_match_table = loongson_edac_of_match, > + }, > +}; > +module_platform_driver(loongson_edac_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Zhao Qunqin <zhaoqunqin@loongson.cn>"); > +MODULE_DESCRIPTION("EDAC driver for loongson memory controller"); > -- > 2.43.0 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] Loongarch: EDAC driver for loongson memory controller 2024-09-25 4:46 ` Huacai Chen @ 2024-09-25 6:17 ` Zhao Qunqin 0 siblings, 0 replies; 7+ messages in thread From: Zhao Qunqin @ 2024-09-25 6:17 UTC (permalink / raw) To: Huacai Chen Cc: robh, krzk+dt, conor+dt, bp, tony.luck, linux-edac, devicetree, linux-kernel, kernel, james.morse, mchehab, rric, loongarch 在 2024/9/25 下午12:46, Huacai Chen 写道: > Hi, Qunqin, > > The title should be "EDAC: Add EDAC driver for loongson memory controller" Will fix > On Wed, Sep 25, 2024 at 10:40 AM Zhao Qunqin <zhaoqunqin@loongson.cn> wrote: >> Reports single bit errors (CE) only. >> >> Signed-off-by: Zhao Qunqin <zhaoqunqin@loongson.cn> >> --- >> Changes in v5: >> - Drop the loongson_ prefix from all static functions. >> - Align function arguments on the opening brace. >> - Drop useless comments and useless wrapper. Drop side comments. >> - Reorder variable declarations. >> >> Changes in v4: >> - None >> >> Changes in v3: >> - Addressed review comments raised by Krzysztof and Huacai >> >> Changes in v2: >> - Addressed review comments raised by Krzysztof >> >> MAINTAINERS | 1 + >> arch/loongarch/Kconfig | 1 + >> drivers/edac/Kconfig | 8 ++ >> drivers/edac/Makefile | 1 + >> drivers/edac/loongson_edac.c | 168 +++++++++++++++++++++++++++++++++++ >> 5 files changed, 179 insertions(+) >> create mode 100644 drivers/edac/loongson_edac.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 6cc8cfc8f..5b4526638 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -13242,6 +13242,7 @@ M: Zhao Qunqin <zhaoqunqin@loongson.cn> >> L: linux-edac@vger.kernel.org >> S: Maintained >> F: Documentation/devicetree/bindings/edac/loongson,ls3a5000-mc-edac.yaml >> +F: drivers/edac/loongson_edac.c >> >> LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) >> M: Sathya Prakash <sathya.prakash@broadcom.com> >> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig >> index 70f169210..9c135f1a2 100644 >> --- a/arch/loongarch/Kconfig >> +++ b/arch/loongarch/Kconfig >> @@ -181,6 +181,7 @@ config LOONGARCH >> select PCI_MSI_ARCH_FALLBACKS >> select PCI_QUIRKS >> select PERF_USE_VMALLOC >> + select EDAC_SUPPORT >> select RTC_LIB >> select SPARSE_IRQ >> select SYSCTL_ARCH_UNALIGN_ALLOW >> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig >> index 81af6c344..719bb6ca7 100644 >> --- a/drivers/edac/Kconfig >> +++ b/drivers/edac/Kconfig >> @@ -564,5 +564,13 @@ config EDAC_VERSAL >> Support injecting both correctable and uncorrectable errors >> for debugging purposes. >> >> +config EDAC_LOONGSON3 >> + tristate "Loongson-3 Memory Controller" >> + depends on LOONGARCH || COMPILE_TEST >> + help >> + Support for error detection and correction on the Loongson-3 >> + family memory controller. This driver reports single bit >> + errors (CE) only. Loongson-3A5000/3C5000/3D5000/3C5000L/3A6000/3C6000 >> + are compatible. >> >> endif # EDAC >> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile >> index faf310eec..e72ca1be4 100644 >> --- a/drivers/edac/Makefile >> +++ b/drivers/edac/Makefile >> @@ -88,3 +88,4 @@ obj-$(CONFIG_EDAC_DMC520) += dmc520_edac.o >> obj-$(CONFIG_EDAC_NPCM) += npcm_edac.o >> obj-$(CONFIG_EDAC_ZYNQMP) += zynqmp_edac.o >> obj-$(CONFIG_EDAC_VERSAL) += versal_edac.o >> +obj-$(CONFIG_EDAC_LOONGSON3) += loongson_edac.o > Change the Kconfig name to CONFIG_EDAC_LOONGSON, or change the file > name to loongson3_edac.c may be a little better. > > Huacai Will fix >> diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c >> new file mode 100644 >> index 000000000..2721dfba5 >> --- /dev/null >> +++ b/drivers/edac/loongson_edac.c >> @@ -0,0 +1,168 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2024 Loongson Technology Corporation Limited. >> + */ >> + >> +#include <linux/edac.h> >> +#include <linux/module.h> >> +#include <linux/init.h> >> +#include <linux/platform_device.h> >> + >> +#include "edac_module.h" >> + >> +enum ecc_index { >> + ECC_SET = 0, >> + ECC_RESERVED, >> + ECC_COUNT, >> + ECC_CS_COUNT, >> + ECC_CODE, >> + ECC_ADDR, >> + ECC_DATA0, >> + ECC_DATA1, >> + ECC_DATA2, >> + ECC_DATA3, >> +}; >> + >> +struct loongson_edac_pvt { >> + u64 *ecc_base; >> + int last_ce_count; >> +}; >> + >> +static int read_ecc(struct mem_ctl_info *mci) >> +{ >> + struct loongson_edac_pvt *pvt = mci->pvt_info; >> + u64 ecc; >> + int cs; >> + >> + if (!pvt->ecc_base) >> + return pvt->last_ce_count; >> + >> + ecc = pvt->ecc_base[ECC_CS_COUNT]; >> + /* cs0 -- cs3 */ >> + cs = ecc & 0xff; >> + cs += (ecc >> 8) & 0xff; >> + cs += (ecc >> 16) & 0xff; >> + cs += (ecc >> 24) & 0xff; >> + >> + return cs; >> +} >> + >> +static void edac_check(struct mem_ctl_info *mci) >> +{ >> + struct loongson_edac_pvt *pvt = mci->pvt_info; >> + int new, add; >> + >> + new = read_ecc(mci); >> + add = new - pvt->last_ce_count; >> + pvt->last_ce_count = new; >> + if (add <= 0) >> + return; >> + >> + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci, add, >> + 0, 0, 0, 0, 0, -1, "error", ""); >> + edac_mc_printk(mci, KERN_INFO, "add: %d", add); >> +} >> + >> +static int get_dimm_config(struct mem_ctl_info *mci) >> +{ >> + struct dimm_info *dimm; >> + u32 size, npages; >> + >> + /* size not used */ >> + size = -1; >> + npages = MiB_TO_PAGES(size); >> + >> + dimm = edac_get_dimm(mci, 0, 0, 0); >> + dimm->nr_pages = npages; >> + snprintf(dimm->label, sizeof(dimm->label), >> + "MC#%uChannel#%u_DIMM#%u", mci->mc_idx, 0, 0); >> + dimm->grain = 8; >> + >> + return 0; >> +} >> + >> +static void pvt_init(struct mem_ctl_info *mci, u64 *vbase) >> +{ >> + struct loongson_edac_pvt *pvt = mci->pvt_info; >> + >> + pvt->ecc_base = vbase; >> + pvt->last_ce_count = read_ecc(mci); >> +} >> + >> +static int edac_probe(struct platform_device *pdev) >> +{ >> + struct edac_mc_layer layers[2]; >> + struct loongson_edac_pvt *pvt; >> + struct mem_ctl_info *mci; >> + u64 *vbase; >> + int ret; >> + >> + vbase = devm_platform_ioremap_resource(pdev, 0); >> + if (IS_ERR(vbase)) >> + return PTR_ERR(vbase); >> + >> + /* allocate a new MC control structure */ >> + layers[0].type = EDAC_MC_LAYER_CHANNEL; >> + layers[0].size = 1; >> + layers[0].is_virt_csrow = false; >> + layers[1].type = EDAC_MC_LAYER_SLOT; >> + layers[1].size = 1; >> + layers[1].is_virt_csrow = true; >> + mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*pvt)); >> + if (mci == NULL) >> + return -ENOMEM; >> + >> + mci->mc_idx = edac_device_alloc_index(); >> + mci->mtype_cap = MEM_FLAG_RDDR4; >> + mci->edac_ctl_cap = EDAC_FLAG_NONE; >> + mci->edac_cap = EDAC_FLAG_NONE; >> + mci->mod_name = "loongson_edac.c"; >> + mci->ctl_name = "loongson_edac_ctl"; >> + mci->dev_name = "loongson_edac_dev"; >> + mci->ctl_page_to_phys = NULL; >> + mci->pdev = &pdev->dev; >> + mci->error_desc.grain = 8; >> + /* Set the function pointer to an actual operation function */ >> + mci->edac_check = edac_check; >> + >> + pvt_init(mci, vbase); >> + get_dimm_config(mci); >> + >> + ret = edac_mc_add_mc(mci); >> + if (ret) { >> + edac_dbg(0, "MC: failed edac_mc_add_mc()\n"); >> + edac_mc_free(mci); >> + return ret; >> + } >> + edac_op_state = EDAC_OPSTATE_POLL; >> + >> + return 0; >> +} >> + >> +static void edac_remove(struct platform_device *pdev) >> +{ >> + struct mem_ctl_info *mci = edac_mc_del_mc(&pdev->dev); >> + >> + if (mci) >> + edac_mc_free(mci); >> +} >> + >> +static const struct of_device_id loongson_edac_of_match[] = { >> + { .compatible = "loongson,ls3a5000-mc-edac", }, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(of, loongson_edac_of_match); >> + >> +static struct platform_driver loongson_edac_driver = { >> + .probe = edac_probe, >> + .remove = edac_remove, >> + .driver = { >> + .name = "loongson-mc-edac", >> + .of_match_table = loongson_edac_of_match, >> + }, >> +}; >> +module_platform_driver(loongson_edac_driver); >> + >> +MODULE_LICENSE("GPL"); >> +MODULE_AUTHOR("Zhao Qunqin <zhaoqunqin@loongson.cn>"); >> +MODULE_DESCRIPTION("EDAC driver for loongson memory controller"); >> -- >> 2.43.0 >> >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] Loongarch: EDAC driver for loongson memory controller 2024-09-25 2:40 ` [PATCH v5 2/2] Loongarch: EDAC driver for loongson " Zhao Qunqin 2024-09-25 4:46 ` Huacai Chen @ 2024-09-25 9:13 ` Jonathan Cameron 2024-09-30 3:24 ` Zhao Qunqin 1 sibling, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2024-09-25 9:13 UTC (permalink / raw) To: Zhao Qunqin Cc: robh, krzk+dt, conor+dt, chenhuacai, bp, tony.luck, linux-edac, devicetree, linux-kernel, kernel, james.morse, mchehab, rric, loongarch On Wed, 25 Sep 2024 10:40:38 +0800 Zhao Qunqin <zhaoqunqin@loongson.cn> wrote: > Reports single bit errors (CE) only. > > Signed-off-by: Zhao Qunqin <zhaoqunqin@loongson.cn> Hi. A few quick comments inline Jonathan > --- > Changes in v5: > - Drop the loongson_ prefix from all static functions. > - Align function arguments on the opening brace. > - Drop useless comments and useless wrapper. Drop side comments. > - Reorder variable declarations. > > Changes in v4: > - None > > Changes in v3: > - Addressed review comments raised by Krzysztof and Huacai > > Changes in v2: > - Addressed review comments raised by Krzysztof > > MAINTAINERS | 1 + > arch/loongarch/Kconfig | 1 + > drivers/edac/Kconfig | 8 ++ > drivers/edac/Makefile | 1 + > drivers/edac/loongson_edac.c | 168 +++++++++++++++++++++++++++++++++++ > 5 files changed, 179 insertions(+) > create mode 100644 drivers/edac/loongson_edac.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 6cc8cfc8f..5b4526638 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -13242,6 +13242,7 @@ M: Zhao Qunqin <zhaoqunqin@loongson.cn> > L: linux-edac@vger.kernel.org > S: Maintained > F: Documentation/devicetree/bindings/edac/loongson,ls3a5000-mc-edac.yaml > +F: drivers/edac/loongson_edac.c > > LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) > M: Sathya Prakash <sathya.prakash@broadcom.com> > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig > index 70f169210..9c135f1a2 100644 > --- a/arch/loongarch/Kconfig > +++ b/arch/loongarch/Kconfig > @@ -181,6 +181,7 @@ config LOONGARCH > select PCI_MSI_ARCH_FALLBACKS > select PCI_QUIRKS > select PERF_USE_VMALLOC > + select EDAC_SUPPORT > select RTC_LIB > select SPARSE_IRQ > select SYSCTL_ARCH_UNALIGN_ALLOW > diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig > index 81af6c344..719bb6ca7 100644 > --- a/drivers/edac/Kconfig > +++ b/drivers/edac/Kconfig > @@ -564,5 +564,13 @@ config EDAC_VERSAL > Support injecting both correctable and uncorrectable errors > for debugging purposes. > > +config EDAC_LOONGSON3 > + tristate "Loongson-3 Memory Controller" > + depends on LOONGARCH || COMPILE_TEST > + help > + Support for error detection and correction on the Loongson-3 > + family memory controller. This driver reports single bit > + errors (CE) only. Loongson-3A5000/3C5000/3D5000/3C5000L/3A6000/3C6000 > + are compatible. > > endif # EDAC > diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile > index faf310eec..e72ca1be4 100644 > --- a/drivers/edac/Makefile > +++ b/drivers/edac/Makefile > @@ -88,3 +88,4 @@ obj-$(CONFIG_EDAC_DMC520) += dmc520_edac.o > obj-$(CONFIG_EDAC_NPCM) += npcm_edac.o > obj-$(CONFIG_EDAC_ZYNQMP) += zynqmp_edac.o > obj-$(CONFIG_EDAC_VERSAL) += versal_edac.o > +obj-$(CONFIG_EDAC_LOONGSON3) += loongson_edac.o > diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c > new file mode 100644 > index 000000000..2721dfba5 > --- /dev/null > +++ b/drivers/edac/loongson_edac.c > @@ -0,0 +1,168 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2024 Loongson Technology Corporation Limited. > + */ > + > +#include <linux/edac.h> > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/platform_device.h> > + > +#include "edac_module.h" > + > +enum ecc_index { > + ECC_SET = 0, > + ECC_RESERVED, > + ECC_COUNT, > + ECC_CS_COUNT, > + ECC_CODE, > + ECC_ADDR, > + ECC_DATA0, > + ECC_DATA1, > + ECC_DATA2, > + ECC_DATA3, > +}; > + > +struct loongson_edac_pvt { > + u64 *ecc_base; > + int last_ce_count; > +}; > + > +static int read_ecc(struct mem_ctl_info *mci) > +{ > + struct loongson_edac_pvt *pvt = mci->pvt_info; > + u64 ecc; > + int cs; > + > + if (!pvt->ecc_base) > + return pvt->last_ce_count; > + > + ecc = pvt->ecc_base[ECC_CS_COUNT]; > + /* cs0 -- cs3 */ > + cs = ecc & 0xff; > + cs += (ecc >> 8) & 0xff; > + cs += (ecc >> 16) & 0xff; > + cs += (ecc >> 24) & 0xff; This smells like an endian swap. swab32() or is this fixing a wrong endian register? In which case b32_to_cpu() > + > + return cs; > +} > + > +static void edac_check(struct mem_ctl_info *mci) > +{ > + struct loongson_edac_pvt *pvt = mci->pvt_info; > + int new, add; > + > + new = read_ecc(mci); > + add = new - pvt->last_ce_count; > + pvt->last_ce_count = new; > + if (add <= 0) This has be a little confused. Either this counter can wrap in which case why drop out here, or it can't in which case does < occur? > + return; > + > + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci, add, > + 0, 0, 0, 0, 0, -1, "error", ""); > + edac_mc_printk(mci, KERN_INFO, "add: %d", add); > +} > + > +static int get_dimm_config(struct mem_ctl_info *mci) > +{ > + struct dimm_info *dimm; > + u32 size, npages; > + > + /* size not used */ > + size = -1; > + npages = MiB_TO_PAGES(size); > + > + dimm = edac_get_dimm(mci, 0, 0, 0); > + dimm->nr_pages = npages; > + snprintf(dimm->label, sizeof(dimm->label), > + "MC#%uChannel#%u_DIMM#%u", mci->mc_idx, 0, 0); > + dimm->grain = 8; > + > + return 0; > +} > + > +static void pvt_init(struct mem_ctl_info *mci, u64 *vbase) > +{ > + struct loongson_edac_pvt *pvt = mci->pvt_info; > + > + pvt->ecc_base = vbase; > + pvt->last_ce_count = read_ecc(mci); > +} > + > +static int edac_probe(struct platform_device *pdev) > +{ > + struct edac_mc_layer layers[2]; > + struct loongson_edac_pvt *pvt; > + struct mem_ctl_info *mci; > + u64 *vbase; > + int ret; > + > + vbase = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(vbase)) > + return PTR_ERR(vbase); > + > + /* allocate a new MC control structure */ > + layers[0].type = EDAC_MC_LAYER_CHANNEL; > + layers[0].size = 1; > + layers[0].is_virt_csrow = false; > + layers[1].type = EDAC_MC_LAYER_SLOT; > + layers[1].size = 1; > + layers[1].is_virt_csrow = true; Could move this to a c99 style struct edac_mc_layer layers[2] = { { .type = EDAC_MC_LAYER_CHANNEL, .size = 1, .is_virt_csrow = false, }, { .type = EDAC_MC_LAYER_SLOT, .size = 1, is_virt_csrow = true, } }; Not particularly important though. > + mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*pvt)); > + if (mci == NULL) Probably !mci is sufficient but I'm not sure on local edac style. > + return -ENOMEM; > + > + mci->mc_idx = edac_device_alloc_index(); > + mci->mtype_cap = MEM_FLAG_RDDR4; > + mci->edac_ctl_cap = EDAC_FLAG_NONE; > + mci->edac_cap = EDAC_FLAG_NONE; > + mci->mod_name = "loongson_edac.c"; > + mci->ctl_name = "loongson_edac_ctl"; > + mci->dev_name = "loongson_edac_dev"; > + mci->ctl_page_to_phys = NULL; > + mci->pdev = &pdev->dev; > + mci->error_desc.grain = 8; > + /* Set the function pointer to an actual operation function */ > + mci->edac_check = edac_check; Similar to above, can initialize this structure more cleanly using *mci = (struct mem_ctl_info) { .mc_idx = edac_device_alloc_index, ... }; > + > + pvt_init(mci, vbase); > + get_dimm_config(mci); > + > + ret = edac_mc_add_mc(mci); I'd be tempted to use devm_add_action_or_cleanup() for this and the alloc above, but not common in edac but it is done in al_mc_edac.c if you want an example. > + if (ret) { > + edac_dbg(0, "MC: failed edac_mc_add_mc()\n"); > + edac_mc_free(mci); > + return ret; > + } > + edac_op_state = EDAC_OPSTATE_POLL; > + > + return 0; > +} > + > +static void edac_remove(struct platform_device *pdev) > +{ > + struct mem_ctl_info *mci = edac_mc_del_mc(&pdev->dev); > + > + if (mci) > + edac_mc_free(mci); Very odd if you got to remove and edac_mc_del_mc() failed. Do we need this check? At least some drivers (I checked a few at random) don't check this. > +} > + > +static const struct of_device_id loongson_edac_of_match[] = { > + { .compatible = "loongson,ls3a5000-mc-edac", }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, loongson_edac_of_match); > + > +static struct platform_driver loongson_edac_driver = { > + .probe = edac_probe, > + .remove = edac_remove, > + .driver = { > + .name = "loongson-mc-edac", > + .of_match_table = loongson_edac_of_match, > + }, > +}; > +module_platform_driver(loongson_edac_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Zhao Qunqin <zhaoqunqin@loongson.cn>"); > +MODULE_DESCRIPTION("EDAC driver for loongson memory controller"); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] Loongarch: EDAC driver for loongson memory controller 2024-09-25 9:13 ` Jonathan Cameron @ 2024-09-30 3:24 ` Zhao Qunqin 0 siblings, 0 replies; 7+ messages in thread From: Zhao Qunqin @ 2024-09-30 3:24 UTC (permalink / raw) To: Jonathan Cameron Cc: robh, krzk+dt, conor+dt, chenhuacai, bp, tony.luck, linux-edac, devicetree, linux-kernel, kernel, james.morse, mchehab, rric, loongarch 在 2024/9/25 下午5:13, Jonathan Cameron 写道: > On Wed, 25 Sep 2024 10:40:38 +0800 > Zhao Qunqin <zhaoqunqin@loongson.cn> wrote: > >> Reports single bit errors (CE) only. >> >> Signed-off-by: Zhao Qunqin <zhaoqunqin@loongson.cn> > Hi. A few quick comments inline > > Jonathan > >> --- >> Changes in v5: >> - Drop the loongson_ prefix from all static functions. >> - Align function arguments on the opening brace. >> - Drop useless comments and useless wrapper. Drop side comments. >> - Reorder variable declarations. >> >> Changes in v4: >> - None >> >> Changes in v3: >> - Addressed review comments raised by Krzysztof and Huacai >> >> Changes in v2: >> - Addressed review comments raised by Krzysztof >> >> MAINTAINERS | 1 + >> arch/loongarch/Kconfig | 1 + >> drivers/edac/Kconfig | 8 ++ >> drivers/edac/Makefile | 1 + >> drivers/edac/loongson_edac.c | 168 +++++++++++++++++++++++++++++++++++ >> 5 files changed, 179 insertions(+) >> create mode 100644 drivers/edac/loongson_edac.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 6cc8cfc8f..5b4526638 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -13242,6 +13242,7 @@ M: Zhao Qunqin <zhaoqunqin@loongson.cn> >> L: linux-edac@vger.kernel.org >> S: Maintained >> F: Documentation/devicetree/bindings/edac/loongson,ls3a5000-mc-edac.yaml >> +F: drivers/edac/loongson_edac.c >> >> LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) >> M: Sathya Prakash <sathya.prakash@broadcom.com> >> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig >> index 70f169210..9c135f1a2 100644 >> --- a/arch/loongarch/Kconfig >> +++ b/arch/loongarch/Kconfig >> @@ -181,6 +181,7 @@ config LOONGARCH >> select PCI_MSI_ARCH_FALLBACKS >> select PCI_QUIRKS >> select PERF_USE_VMALLOC >> + select EDAC_SUPPORT >> select RTC_LIB >> select SPARSE_IRQ >> select SYSCTL_ARCH_UNALIGN_ALLOW >> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig >> index 81af6c344..719bb6ca7 100644 >> --- a/drivers/edac/Kconfig >> +++ b/drivers/edac/Kconfig >> @@ -564,5 +564,13 @@ config EDAC_VERSAL >> Support injecting both correctable and uncorrectable errors >> for debugging purposes. >> >> +config EDAC_LOONGSON3 >> + tristate "Loongson-3 Memory Controller" >> + depends on LOONGARCH || COMPILE_TEST >> + help >> + Support for error detection and correction on the Loongson-3 >> + family memory controller. This driver reports single bit >> + errors (CE) only. Loongson-3A5000/3C5000/3D5000/3C5000L/3A6000/3C6000 >> + are compatible. >> >> endif # EDAC >> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile >> index faf310eec..e72ca1be4 100644 >> --- a/drivers/edac/Makefile >> +++ b/drivers/edac/Makefile >> @@ -88,3 +88,4 @@ obj-$(CONFIG_EDAC_DMC520) += dmc520_edac.o >> obj-$(CONFIG_EDAC_NPCM) += npcm_edac.o >> obj-$(CONFIG_EDAC_ZYNQMP) += zynqmp_edac.o >> obj-$(CONFIG_EDAC_VERSAL) += versal_edac.o >> +obj-$(CONFIG_EDAC_LOONGSON3) += loongson_edac.o >> diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c >> new file mode 100644 >> index 000000000..2721dfba5 >> --- /dev/null >> +++ b/drivers/edac/loongson_edac.c >> @@ -0,0 +1,168 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2024 Loongson Technology Corporation Limited. >> + */ >> + >> +#include <linux/edac.h> >> +#include <linux/module.h> >> +#include <linux/init.h> >> +#include <linux/platform_device.h> >> + >> +#include "edac_module.h" >> + >> +enum ecc_index { >> + ECC_SET = 0, >> + ECC_RESERVED, >> + ECC_COUNT, >> + ECC_CS_COUNT, >> + ECC_CODE, >> + ECC_ADDR, >> + ECC_DATA0, >> + ECC_DATA1, >> + ECC_DATA2, >> + ECC_DATA3, >> +}; >> + >> +struct loongson_edac_pvt { >> + u64 *ecc_base; >> + int last_ce_count; >> +}; >> + >> +static int read_ecc(struct mem_ctl_info *mci) >> +{ >> + struct loongson_edac_pvt *pvt = mci->pvt_info; >> + u64 ecc; >> + int cs; >> + >> + if (!pvt->ecc_base) >> + return pvt->last_ce_count; >> + >> + ecc = pvt->ecc_base[ECC_CS_COUNT]; >> + /* cs0 -- cs3 */ >> + cs = ecc & 0xff; >> + cs += (ecc >> 8) & 0xff; >> + cs += (ecc >> 16) & 0xff; >> + cs += (ecc >> 24) & 0xff; > This smells like an endian swap. > swab32() or is this fixing a wrong endian register? > In which case b32_to_cpu() Not an endian swap. Just add up the values of the lowest four bytes. > >> + >> + return cs; >> +} >> + >> +static void edac_check(struct mem_ctl_info *mci) >> +{ >> + struct loongson_edac_pvt *pvt = mci->pvt_info; >> + int new, add; >> + >> + new = read_ecc(mci); >> + add = new - pvt->last_ce_count; >> + pvt->last_ce_count = new; >> + if (add <= 0) > This has be a little confused. Either this counter can > wrap in which case why drop out here, or it can't in which case > does < occur? The ce count of loongson memory controller should only increase. > >> + return; >> + >> + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci, add, >> + 0, 0, 0, 0, 0, -1, "error", ""); >> + edac_mc_printk(mci, KERN_INFO, "add: %d", add); >> +} >> + >> +static int get_dimm_config(struct mem_ctl_info *mci) >> +{ >> + struct dimm_info *dimm; >> + u32 size, npages; >> + >> + /* size not used */ >> + size = -1; >> + npages = MiB_TO_PAGES(size); >> + >> + dimm = edac_get_dimm(mci, 0, 0, 0); >> + dimm->nr_pages = npages; >> + snprintf(dimm->label, sizeof(dimm->label), >> + "MC#%uChannel#%u_DIMM#%u", mci->mc_idx, 0, 0); >> + dimm->grain = 8; >> + >> + return 0; >> +} >> + >> +static void pvt_init(struct mem_ctl_info *mci, u64 *vbase) >> +{ >> + struct loongson_edac_pvt *pvt = mci->pvt_info; >> + >> + pvt->ecc_base = vbase; >> + pvt->last_ce_count = read_ecc(mci); >> +} >> + >> +static int edac_probe(struct platform_device *pdev) >> +{ >> + struct edac_mc_layer layers[2]; >> + struct loongson_edac_pvt *pvt; >> + struct mem_ctl_info *mci; >> + u64 *vbase; >> + int ret; >> + >> + vbase = devm_platform_ioremap_resource(pdev, 0); >> + if (IS_ERR(vbase)) >> + return PTR_ERR(vbase); >> + >> + /* allocate a new MC control structure */ >> + layers[0].type = EDAC_MC_LAYER_CHANNEL; >> + layers[0].size = 1; >> + layers[0].is_virt_csrow = false; >> + layers[1].type = EDAC_MC_LAYER_SLOT; >> + layers[1].size = 1; >> + layers[1].is_virt_csrow = true; > Could move this to a c99 style > > struct edac_mc_layer layers[2] = { > { > .type = EDAC_MC_LAYER_CHANNEL, > .size = 1, > .is_virt_csrow = false, > }, { > .type = EDAC_MC_LAYER_SLOT, > .size = 1, > is_virt_csrow = true, > } > }; > Not particularly important though. > >> + mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*pvt)); >> + if (mci == NULL) > Probably !mci is sufficient but I'm not sure on local edac style. > >> + return -ENOMEM; >> + >> + mci->mc_idx = edac_device_alloc_index(); >> + mci->mtype_cap = MEM_FLAG_RDDR4; >> + mci->edac_ctl_cap = EDAC_FLAG_NONE; >> + mci->edac_cap = EDAC_FLAG_NONE; >> + mci->mod_name = "loongson_edac.c"; >> + mci->ctl_name = "loongson_edac_ctl"; >> + mci->dev_name = "loongson_edac_dev"; >> + mci->ctl_page_to_phys = NULL; >> + mci->pdev = &pdev->dev; >> + mci->error_desc.grain = 8; >> + /* Set the function pointer to an actual operation function */ >> + mci->edac_check = edac_check; > Similar to above, can initialize this structure more cleanly > using > > *mci = (struct mem_ctl_info) { > .mc_idx = edac_device_alloc_index, > ... > }; >> + >> + pvt_init(mci, vbase); >> + get_dimm_config(mci); >> + >> + ret = edac_mc_add_mc(mci); > I'd be tempted to use devm_add_action_or_cleanup() for this and the > alloc above, but not common in edac but it is done in al_mc_edac.c if > you want an example. Your idea is great, but I don't think it's necessary to make things so complicated >> + if (ret) { >> + edac_dbg(0, "MC: failed edac_mc_add_mc()\n"); >> + edac_mc_free(mci); >> + return ret; >> + } >> + edac_op_state = EDAC_OPSTATE_POLL; >> + >> + return 0; >> +} >> + >> +static void edac_remove(struct platform_device *pdev) >> +{ >> + struct mem_ctl_info *mci = edac_mc_del_mc(&pdev->dev); >> + >> + if (mci) >> + edac_mc_free(mci); > Very odd if you got to remove and edac_mc_del_mc() failed. > Do we need this check? At least some drivers (I checked a few > at random) don't check this. Yes, odd if edac_mc_del_mc() failed. Dose it better to add a check just in case edac_mc_del_mc() failed ? Thanks, Zhao Qunqin > >> +} >> + >> +static const struct of_device_id loongson_edac_of_match[] = { >> + { .compatible = "loongson,ls3a5000-mc-edac", }, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(of, loongson_edac_of_match); >> + >> +static struct platform_driver loongson_edac_driver = { >> + .probe = edac_probe, >> + .remove = edac_remove, >> + .driver = { >> + .name = "loongson-mc-edac", >> + .of_match_table = loongson_edac_of_match, >> + }, >> +}; >> +module_platform_driver(loongson_edac_driver); >> + >> +MODULE_LICENSE("GPL"); >> +MODULE_AUTHOR("Zhao Qunqin <zhaoqunqin@loongson.cn>"); >> +MODULE_DESCRIPTION("EDAC driver for loongson memory controller"); ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-09-30 3:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-25 2:40 [PATCH v5 0/2] Add EDAC driver for loongson memory controller Zhao Qunqin 2024-09-25 2:40 ` [PATCH v5 1/2] dt-bindings: EDAC for ls3a5000 " Zhao Qunqin 2024-09-25 2:40 ` [PATCH v5 2/2] Loongarch: EDAC driver for loongson " Zhao Qunqin 2024-09-25 4:46 ` Huacai Chen 2024-09-25 6:17 ` Zhao Qunqin 2024-09-25 9:13 ` Jonathan Cameron 2024-09-30 3:24 ` Zhao Qunqin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox