[PATCH v6 0/4] EDAC/loongson: Improvements for Loongson EDAC driver

[PATCH v6 0/4] EDAC/loongson: Improvements for Loongson EDAC driver

[Qunqin Zhao]

From: Qunqin Zhao <zhaoqunqin@loongson.cn>

This series contains four patches for the Loongson EDAC driver:

Patch 1: Rename driver-local symbols to remove the "edac_" prefix.

Patch 2: Encode node and MC info into mc_idx so that on multi-node
systems, sysfs entries uniquely identify each controller.

Patch 3: Get DIMM size from ACPI _DSM method instead of using hardcoded
values, enabling correct display of memory size in sysfs.

Patch 4: Add CS4-7 timeout filter for cross-node access on 3C6000
multi-node systems to prevent false CE error reports.

Changes in v6:

- Rename check(), probe() and remove() to loongson_check(),
  loongson_probe() and loongson_remove() respectively.

Changes in v5:

- Add a new preparatory patch (Patch 1) that removes the "edac_" prefix
  from driver-local definitions: rename edac_check(), edac_probe() and
  edac_remove() to check(), probe() and remove(), respectively, and
  rename struct loongson_edac_pvt, loongson_edac_driver and
  loongson_edac_acpi_match to loongson_pvt, loongson_driver and
  loongson_acpi_match, respectively (Boris)
- Reorder the variable declarations in probe() so that int ret is
  declared last, following the EDAC tree reverse-fir-tree convention
  (Boris)

Changes in v4:

- int mcs_per_node; -> u32 mcs_per_node;
- int valid_cs_bits; -> u32 valid_cs_bits;

Changes in v3:
Patch 2 "Encode node and MC info into mc_idx":
- Drop mc_idx_u32 temporary variable; pass &mci->mc_idx directly
- Rename device property mc-per-node -> mcs-per-node

Patch 3 "Get DIMM size from ACPI _DSM method":
- Fix subject: get -> Get
- No code changes

Patch 4 "Add CS4-7 timeout filter for cross-node access":
- No changes

Changes in v2:

Patch 2 "Encode node and MC info into mc_idx":
- Use char other_detail[64] = {0} and drop the else branch
- Put snprintf() on one line
- Initialize mc_idx_valid = true and drop the else branch
- Validate mc-per-node firmware property is non-zero to prevent
  division by zero
- Wrap pvt_init() function definition to satisfy 100-column limit

Patch 3 "get DIMM size from ACPI _DSM method":
- No changes; rebased on top of updated patch 2.

Patch 4 "Add CS4-7 timeout filter for cross-node access":
- Assign valid_cs_bits before calling read_ecc() in pvt_init() to
  prevent using uninitialized data when establishing the baseline
  error count
- Add bounds check for valid_cs_bits < 64 to guard against undefined
  behavior when shifting a 64-bit value by 64 bits
- Rename device property valid_cs_bits -> valid-cs-bits to match
  standard DT/ACPI _DSD hyphen convention

v4: https://lore.kernel.org/all/20260902084827.2652-1-zhaoqunqin@loongson.cn/
v3: https://lore.kernel.org/all/20260812064139.1231-1-zhaoqunqin@loongson.cn/
v2: https://lore.kernel.org/all/20260805085414.1482-1-zhaoqunqin@loongson.cn/
v1: https://lore.kernel.org/all/20260730063035.230686-1-zhaoqunqin@loongson.cn/

Qunqin Zhao (2):
  EDAC/loongson: Rename driver-local symbols
  EDAC/loongson: Add CS4-7 timeout filter for cross-node access

Wang Jinwei (2):
  EDAC/loongson: Encode node and MC info into mc_idx
  EDAC/loongson: Get DIMM size from ACPI _DSM method

 drivers/edac/loongson_edac.c | 114 +++++++++++++++++++++++++++--------
 1 file changed, 90 insertions(+), 24 deletions(-)


base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d
-- 
2.47.2

[PATCH v6 1/4] EDAC/loongson: Rename driver-local symbols

[Qunqin Zhao]

From: Qunqin Zhao <zhaoqunqin@loongson.cn>

Remove the "edac_" prefix from driver-local definitions to keep the
edac_ namespace reserved for EDAC core functions.

Rename edac_check(), edac_probe() and edac_remove() to
loongson_check(), loongson_probe() and loongson_remove(), respectively,
and rename struct loongson_edac_pvt, loongson_edac_driver and
loongson_edac_acpi_match to loongson_pvt, loongson_driver and
loongson_acpi_match, respectively. The shorter names are unambiguous
since they are all file-local.

Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn>
---
 drivers/edac/loongson_edac.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c
index 3ed4d34a33..5dfa41e877 100644
--- a/drivers/edac/loongson_edac.c
+++ b/drivers/edac/loongson_edac.c
@@ -13,7 +13,7 @@
 
 #define ECC_CS_COUNT_REG	0x18
 
-struct loongson_edac_pvt {
+struct loongson_pvt {
 	void __iomem *ecc_base;
 
 	/*
@@ -27,7 +27,7 @@ struct loongson_edac_pvt {
 
 static int read_ecc(struct mem_ctl_info *mci)
 {
-	struct loongson_edac_pvt *pvt = mci->pvt_info;
+	struct loongson_pvt *pvt = mci->pvt_info;
 	u64 ecc;
 	int cs;
 
@@ -41,9 +41,9 @@ static int read_ecc(struct mem_ctl_info *mci)
 	return cs;
 }
 
-static void edac_check(struct mem_ctl_info *mci)
+static void loongson_check(struct mem_ctl_info *mci)
 {
-	struct loongson_edac_pvt *pvt = mci->pvt_info;
+	struct loongson_pvt *pvt = mci->pvt_info;
 	int new, add;
 
 	new = read_ecc(mci);
@@ -74,13 +74,13 @@ static void dimm_config_init(struct mem_ctl_info *mci)
 
 static void pvt_init(struct mem_ctl_info *mci, void __iomem *vbase)
 {
-	struct loongson_edac_pvt *pvt = mci->pvt_info;
+	struct loongson_pvt *pvt = mci->pvt_info;
 
 	pvt->ecc_base = vbase;
 	pvt->last_ce_count = read_ecc(mci);
 }
 
-static int edac_probe(struct platform_device *pdev)
+static int loongson_probe(struct platform_device *pdev)
 {
 	struct edac_mc_layer layers[2];
 	struct mem_ctl_info *mci;
@@ -98,7 +98,7 @@ static int edac_probe(struct platform_device *pdev)
 	layers[1].size = 1;
 	layers[1].is_virt_csrow = true;
 	mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers,
-			    sizeof(struct loongson_edac_pvt));
+			    sizeof(struct loongson_pvt));
 	if (mci == NULL)
 		return -ENOMEM;
 
@@ -112,7 +112,7 @@ static int edac_probe(struct platform_device *pdev)
 	mci->ctl_page_to_phys = NULL;
 	mci->pdev = &pdev->dev;
 	mci->error_desc.grain = 8;
-	mci->edac_check = edac_check;
+	mci->edac_check = loongson_check;
 
 	pvt_init(mci, vbase);
 	dimm_config_init(mci);
@@ -128,7 +128,7 @@ static int edac_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static void edac_remove(struct platform_device *pdev)
+static void loongson_remove(struct platform_device *pdev)
 {
 	struct mem_ctl_info *mci = edac_mc_del_mc(&pdev->dev);
 
@@ -136,21 +136,21 @@ static void edac_remove(struct platform_device *pdev)
 		edac_mc_free(mci);
 }
 
-static const struct acpi_device_id loongson_edac_acpi_match[] = {
+static const struct acpi_device_id loongson_acpi_match[] = {
 	{ .id = "LOON0010" },
 	{ }
 };
-MODULE_DEVICE_TABLE(acpi, loongson_edac_acpi_match);
+MODULE_DEVICE_TABLE(acpi, loongson_acpi_match);
 
-static struct platform_driver loongson_edac_driver = {
-	.probe		= edac_probe,
-	.remove		= edac_remove,
+static struct platform_driver loongson_driver = {
+	.probe		= loongson_probe,
+	.remove		= loongson_remove,
 	.driver		= {
 		.name	= "loongson-mc-edac",
-		.acpi_match_table = loongson_edac_acpi_match,
+		.acpi_match_table = loongson_acpi_match,
 	},
 };
-module_platform_driver(loongson_edac_driver);
+module_platform_driver(loongson_driver);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Zhao Qunqin <zhaoqunqin@loongson.cn>");
-- 
2.47.2

Re: [PATCH v6 1/4] EDAC/loongson: Rename driver-local symbols

[Huacai Chen]

On Thu, Nov 5, 2026 at 10:56 AM Qunqin Zhao <zhaoqunqin@163.com> wrote:
>
> From: Qunqin Zhao <zhaoqunqin@loongson.cn>
>
> Remove the "edac_" prefix from driver-local definitions to keep the
> edac_ namespace reserved for EDAC core functions.
>
> Rename edac_check(), edac_probe() and edac_remove() to
> loongson_check(), loongson_probe() and loongson_remove(), respectively,
> and rename struct loongson_edac_pvt, loongson_edac_driver and
> loongson_edac_acpi_match to loongson_pvt, loongson_driver and
> loongson_acpi_match, respectively. The shorter names are unambiguous
> since they are all file-local.
>
> Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>

> ---
>  drivers/edac/loongson_edac.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c
> index 3ed4d34a33..5dfa41e877 100644
> --- a/drivers/edac/loongson_edac.c
> +++ b/drivers/edac/loongson_edac.c
> @@ -13,7 +13,7 @@
>
>  #define ECC_CS_COUNT_REG       0x18
>
> -struct loongson_edac_pvt {
> +struct loongson_pvt {
>         void __iomem *ecc_base;
>
>         /*
> @@ -27,7 +27,7 @@ struct loongson_edac_pvt {
>
>  static int read_ecc(struct mem_ctl_info *mci)
>  {
> -       struct loongson_edac_pvt *pvt = mci->pvt_info;
> +       struct loongson_pvt *pvt = mci->pvt_info;
>         u64 ecc;
>         int cs;
>
> @@ -41,9 +41,9 @@ static int read_ecc(struct mem_ctl_info *mci)
>         return cs;
>  }
>
> -static void edac_check(struct mem_ctl_info *mci)
> +static void loongson_check(struct mem_ctl_info *mci)
>  {
> -       struct loongson_edac_pvt *pvt = mci->pvt_info;
> +       struct loongson_pvt *pvt = mci->pvt_info;
>         int new, add;
>
>         new = read_ecc(mci);
> @@ -74,13 +74,13 @@ static void dimm_config_init(struct mem_ctl_info *mci)
>
>  static void pvt_init(struct mem_ctl_info *mci, void __iomem *vbase)
>  {
> -       struct loongson_edac_pvt *pvt = mci->pvt_info;
> +       struct loongson_pvt *pvt = mci->pvt_info;
>
>         pvt->ecc_base = vbase;
>         pvt->last_ce_count = read_ecc(mci);
>  }
>
> -static int edac_probe(struct platform_device *pdev)
> +static int loongson_probe(struct platform_device *pdev)
>  {
>         struct edac_mc_layer layers[2];
>         struct mem_ctl_info *mci;
> @@ -98,7 +98,7 @@ static int edac_probe(struct platform_device *pdev)
>         layers[1].size = 1;
>         layers[1].is_virt_csrow = true;
>         mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers,
> -                           sizeof(struct loongson_edac_pvt));
> +                           sizeof(struct loongson_pvt));
>         if (mci == NULL)
>                 return -ENOMEM;
>
> @@ -112,7 +112,7 @@ static int edac_probe(struct platform_device *pdev)
>         mci->ctl_page_to_phys = NULL;
>         mci->pdev = &pdev->dev;
>         mci->error_desc.grain = 8;
> -       mci->edac_check = edac_check;
> +       mci->edac_check = loongson_check;
>
>         pvt_init(mci, vbase);
>         dimm_config_init(mci);
> @@ -128,7 +128,7 @@ static int edac_probe(struct platform_device *pdev)
>         return 0;
>  }
>
> -static void edac_remove(struct platform_device *pdev)
> +static void loongson_remove(struct platform_device *pdev)
>  {
>         struct mem_ctl_info *mci = edac_mc_del_mc(&pdev->dev);
>
> @@ -136,21 +136,21 @@ static void edac_remove(struct platform_device *pdev)
>                 edac_mc_free(mci);
>  }
>
> -static const struct acpi_device_id loongson_edac_acpi_match[] = {
> +static const struct acpi_device_id loongson_acpi_match[] = {
>         { .id = "LOON0010" },
>         { }
>  };
> -MODULE_DEVICE_TABLE(acpi, loongson_edac_acpi_match);
> +MODULE_DEVICE_TABLE(acpi, loongson_acpi_match);
>
> -static struct platform_driver loongson_edac_driver = {
> -       .probe          = edac_probe,
> -       .remove         = edac_remove,
> +static struct platform_driver loongson_driver = {
> +       .probe          = loongson_probe,
> +       .remove         = loongson_remove,
>         .driver         = {
>                 .name   = "loongson-mc-edac",
> -               .acpi_match_table = loongson_edac_acpi_match,
> +               .acpi_match_table = loongson_acpi_match,
>         },
>  };
> -module_platform_driver(loongson_edac_driver);
> +module_platform_driver(loongson_driver);
>
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Zhao Qunqin <zhaoqunqin@loongson.cn>");
> --
> 2.47.2
>

[PATCH v6 2/4] EDAC/loongson: Encode node and MC info into mc_idx

[Qunqin Zhao]

From: Wang Jinwei <wangjinwei@loongson.cn>

On Loongson multi-node systems, memory controllers are distributed across
multiple nodes. Encode node and MC information into mc_idx so that sysfs
entries uniquely identify each controller.

The driver gets mc-idx from ACPI _DSD property if firmware provides it.
If not, it falls back to edac_device_alloc_index() for legacy firmware
compatibility.

The number of memory controllers per node (mcs-per-node) is obtained
from firmware via ACPI _DSD property, with a default of 4 for backward
compatibility. Validate that the firmware-provided value is non-zero to
prevent division by zero. The node and MC IDs are decoded from mc_idx:
    node = mc_idx / mcs_per_node
    mc = mc_idx % mcs_per_node

Cc: Yulong Wang <wangyulong@loongson.cn>
Cc: Dongyan Qian <qiandongyan@loongson.cn>
Cc: Chao Li <lichao@loongson.cn>
Signed-off-by: Wang Jinwei <wangjinwei@loongson.cn>
Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
---
 drivers/edac/loongson_edac.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c
index 5dfa41e877..3c5d29941a 100644
--- a/drivers/edac/loongson_edac.c
+++ b/drivers/edac/loongson_edac.c
@@ -9,6 +9,7 @@
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include "edac_module.h"
 
 #define ECC_CS_COUNT_REG	0x18
@@ -23,6 +24,8 @@ struct loongson_pvt {
 	 * register state.
 	 */
 	int last_ce_count;
+	u32 mcs_per_node;
+	bool mc_idx_valid;
 };
 
 static int read_ecc(struct mem_ctl_info *mci)
@@ -44,7 +47,8 @@ static int read_ecc(struct mem_ctl_info *mci)
 static void loongson_check(struct mem_ctl_info *mci)
 {
 	struct loongson_pvt *pvt = mci->pvt_info;
-	int new, add;
+	char other_detail[64] = {0};
+	int new, add, node, mc;
 
 	new = read_ecc(mci);
 	add = new - pvt->last_ce_count;
@@ -52,8 +56,14 @@ static void loongson_check(struct mem_ctl_info *mci)
 	if (add <= 0)
 		return;
 
+	if (pvt->mc_idx_valid) {
+		node = mci->mc_idx / pvt->mcs_per_node;
+		mc = mci->mc_idx % pvt->mcs_per_node;
+		snprintf(other_detail, sizeof(other_detail), "node:%d mc:%d", node, mc);
+	}
+
 	edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci, add,
-			     0, 0, 0, 0, 0, -1, "error", "");
+			     0, 0, 0, 0, 0, -1, "error", other_detail);
 }
 
 static void dimm_config_init(struct mem_ctl_info *mci)
@@ -72,19 +82,25 @@ static void dimm_config_init(struct mem_ctl_info *mci)
 	dimm->grain = 8;
 }
 
-static void pvt_init(struct mem_ctl_info *mci, void __iomem *vbase)
+static void pvt_init(struct mem_ctl_info *mci, void __iomem *vbase,
+		     bool mc_idx_valid, u32 mcs_per_node)
 {
 	struct loongson_pvt *pvt = mci->pvt_info;
 
 	pvt->ecc_base = vbase;
 	pvt->last_ce_count = read_ecc(mci);
+	pvt->mc_idx_valid = mc_idx_valid;
+	pvt->mcs_per_node = mcs_per_node;
 }
 
 static int loongson_probe(struct platform_device *pdev)
 {
 	struct edac_mc_layer layers[2];
 	struct mem_ctl_info *mci;
+	struct device *dev = &pdev->dev;
 	void __iomem *vbase;
+	u32 mcs_per_node;
+	bool mc_idx_valid = true;
 	int ret;
 
 	vbase = devm_platform_ioremap_resource(pdev, 0);
@@ -102,7 +118,6 @@ static int loongson_probe(struct platform_device *pdev)
 	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;
@@ -114,7 +129,15 @@ static int loongson_probe(struct platform_device *pdev)
 	mci->error_desc.grain = 8;
 	mci->edac_check = loongson_check;
 
-	pvt_init(mci, vbase);
+	if (device_property_read_u32(dev, "mc-idx", &mci->mc_idx)) {
+		mci->mc_idx = edac_device_alloc_index();
+		mc_idx_valid = false;
+	}
+
+	if (device_property_read_u32(dev, "mcs-per-node", &mcs_per_node) || mcs_per_node == 0)
+		mcs_per_node = 4;
+
+	pvt_init(mci, vbase, mc_idx_valid, mcs_per_node);
 	dimm_config_init(mci);
 
 	ret = edac_mc_add_mc(mci);
-- 
2.47.2

Re: [PATCH v6 2/4] EDAC/loongson: Encode node and MC info into mc_idx

[Borislav Petkov]

On Thu, Nov 05, 2026 at 10:55:30AM +0800, Qunqin Zhao wrote:
> From: Wang Jinwei <wangjinwei@loongson.cn>
> 
> On Loongson multi-node systems, memory controllers are distributed across
> multiple nodes. Encode node and MC information into mc_idx so that sysfs
> entries uniquely identify each controller.
> 
> The driver gets mc-idx from ACPI _DSD property if firmware provides it.
> If not, it falls back to edac_device_alloc_index() for legacy firmware
> compatibility.

I am still not buying this tinkering so lemme try again.

If you cannot read mc_idx from ACPI, then by lack of options, all your nodes
belong to memory controller 0. There's no other choice.

And you do init it to 0:

        mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers,
			   ^^^

                            sizeof(struct loongson_pvt));

and the subsequent property read tries to get it.

So, IOW, your MC index is only valid when it is not 0.

IOW, this ontop of yours. Completely untested ofc.

---

diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c
index 3c5d29941a38..2132264a6d7f 100644
--- a/drivers/edac/loongson_edac.c
+++ b/drivers/edac/loongson_edac.c
@@ -25,7 +25,6 @@ struct loongson_pvt {
 	 */
 	int last_ce_count;
 	u32 mcs_per_node;
-	bool mc_idx_valid;
 };
 
 static int read_ecc(struct mem_ctl_info *mci)
@@ -56,7 +55,7 @@ static void loongson_check(struct mem_ctl_info *mci)
 	if (add <= 0)
 		return;
 
-	if (pvt->mc_idx_valid) {
+	if (pvt->mc_idx) {
 		node = mci->mc_idx / pvt->mcs_per_node;
 		mc = mci->mc_idx % pvt->mcs_per_node;
 		snprintf(other_detail, sizeof(other_detail), "node:%d mc:%d", node, mc);
@@ -82,14 +81,12 @@ static void dimm_config_init(struct mem_ctl_info *mci)
 	dimm->grain = 8;
 }
 
-static void pvt_init(struct mem_ctl_info *mci, void __iomem *vbase,
-		     bool mc_idx_valid, u32 mcs_per_node)
+static void pvt_init(struct mem_ctl_info *mci, void __iomem *vbase, u32 mcs_per_node)
 {
 	struct loongson_pvt *pvt = mci->pvt_info;
 
 	pvt->ecc_base = vbase;
 	pvt->last_ce_count = read_ecc(mci);
-	pvt->mc_idx_valid = mc_idx_valid;
 	pvt->mcs_per_node = mcs_per_node;
 }
 
@@ -100,7 +97,6 @@ static int loongson_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	void __iomem *vbase;
 	u32 mcs_per_node;
-	bool mc_idx_valid = true;
 	int ret;
 
 	vbase = devm_platform_ioremap_resource(pdev, 0);
@@ -129,15 +125,12 @@ static int loongson_probe(struct platform_device *pdev)
 	mci->error_desc.grain = 8;
 	mci->edac_check = loongson_check;
 
-	if (device_property_read_u32(dev, "mc-idx", &mci->mc_idx)) {
-		mci->mc_idx = edac_device_alloc_index();
-		mc_idx_valid = false;
-	}
+	device_property_read_u32(dev, "mc-idx", &mci->mc_idx);
 
 	if (device_property_read_u32(dev, "mcs-per-node", &mcs_per_node) || mcs_per_node == 0)
 		mcs_per_node = 4;
 
-	pvt_init(mci, vbase, mc_idx_valid, mcs_per_node);
+	pvt_init(mci, vbase, mcs_per_node);
 	dimm_config_init(mci);
 
 	ret = edac_mc_add_mc(mci);

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v6 2/4] EDAC/loongson: Encode node and MC info into mc_idx

[Qunqin Zhao]

在 2026/11/10 14:41, Borislav Petkov 写道:
> On Thu, Nov 05, 2026 at 10:55:30AM +0800, Qunqin Zhao wrote:
>> From: Wang Jinwei <wangjinwei@loongson.cn>
>>
>> On Loongson multi-node systems, memory controllers are distributed across
>> multiple nodes. Encode node and MC information into mc_idx so that sysfs
>> entries uniquely identify each controller.
>>
>> The driver gets mc-idx from ACPI _DSD property if firmware provides it.
>> If not, it falls back to edac_device_alloc_index() for legacy firmware
>> compatibility.
> I am still not buying this tinkering so lemme try again.
>
> If you cannot read mc_idx from ACPI, then by lack of options, all your nodes
> belong to memory controller 0. There's no other choice.
>
> And you do init it to 0:
>
>         mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers,
> 			   ^^^
>
>                             sizeof(struct loongson_pvt));
>
> and the subsequent property read tries to get it.
>
> So, IOW, your MC index is only valid when it is not 0.
>
> IOW, this ontop of yours. Completely untested ofc.
>
> ---
>
> diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c
> index 3c5d29941a38..2132264a6d7f 100644
> --- a/drivers/edac/loongson_edac.c
> +++ b/drivers/edac/loongson_edac.c
> @@ -25,7 +25,6 @@ struct loongson_pvt {
>  	 */
>  	int last_ce_count;
>  	u32 mcs_per_node;
> -	bool mc_idx_valid;
>  };
>  
>  static int read_ecc(struct mem_ctl_info *mci)
> @@ -56,7 +55,7 @@ static void loongson_check(struct mem_ctl_info *mci)
>  	if (add <= 0)
>  		return;
>  
> -	if (pvt->mc_idx_valid) {
> +	if (pvt->mc_idx) {
>  		node = mci->mc_idx / pvt->mcs_per_node;
>  		mc = mci->mc_idx % pvt->mcs_per_node;
>  		snprintf(other_detail, sizeof(other_detail), "node:%d mc:%d", node, mc);
> @@ -82,14 +81,12 @@ static void dimm_config_init(struct mem_ctl_info *mci)
>  	dimm->grain = 8;
>  }
>  
> -static void pvt_init(struct mem_ctl_info *mci, void __iomem *vbase,
> -		     bool mc_idx_valid, u32 mcs_per_node)
> +static void pvt_init(struct mem_ctl_info *mci, void __iomem *vbase, u32 mcs_per_node)
>  {
>  	struct loongson_pvt *pvt = mci->pvt_info;
>  
>  	pvt->ecc_base = vbase;
>  	pvt->last_ce_count = read_ecc(mci);
> -	pvt->mc_idx_valid = mc_idx_valid;
>  	pvt->mcs_per_node = mcs_per_node;
>  }
>  
> @@ -100,7 +97,6 @@ static int loongson_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	void __iomem *vbase;
>  	u32 mcs_per_node;
> -	bool mc_idx_valid = true;
>  	int ret;
>  
>  	vbase = devm_platform_ioremap_resource(pdev, 0);
> @@ -129,15 +125,12 @@ static int loongson_probe(struct platform_device *pdev)
>  	mci->error_desc.grain = 8;
>  	mci->edac_check = loongson_check;
>  
> -	if (device_property_read_u32(dev, "mc-idx", &mci->mc_idx)) {
> -		mci->mc_idx = edac_device_alloc_index();
> -		mc_idx_valid = false;
> -	}
> +	device_property_read_u32(dev, "mc-idx", &mci->mc_idx);
>  
>  	if (device_property_read_u32(dev, "mcs-per-node", &mcs_per_node) || mcs_per_node == 0)
>  		mcs_per_node = 4;
>  
> -	pvt_init(mci, vbase, mc_idx_valid, mcs_per_node);
> +	pvt_init(mci, vbase, mcs_per_node);
>  	dimm_config_init(mci);
>  
>  	ret = edac_mc_add_mc(mci);

"all your nodes belong to memory controller 0"
I don't quite follow this sentence. There are four memory controllers on LS3C6000/S.

On firmware without mc-idx there can still be more than one  memory controller

Each probes with edac_mc_alloc(0, ...),  so if we leave mc_idx at 0 the 

second controller hits the "duplicate  mc_idx" check in add_mc_to_global_list() 

and edac_mc_add_mc() returns  non zero.


Qunqin,

Thanks.

>

Re: [PATCH v6 2/4] EDAC/loongson: Encode node and MC info into mc_idx

[Borislav Petkov]

On Tue, Nov 10, 2026 at 03:47:20PM +0800, Qunqin Zhao wrote:
> "all your nodes belong to memory controller 0"
> I don't quite follow this sentence. There are four memory controllers on LS3C6000/S.
> 
> On firmware without mc-idx there can still be more than one  memory controller

So edac_device_alloc_index() is used to give out indices for other EDAC
devices with edac_device_alloc_ctl_info() but I guess you could use that in
your driver until you don't need to add other devices besides MCs and then
it'll become a mess...

Lemme ask the simplest question first: is there any case where ACPI won't
provide that _DSD property or you're in control of the firmware - not like on
x86 - and this case we're trying to fix is never going to happen anyway...?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v6 2/4] EDAC/loongson: Encode node and MC info into mc_idx

[Qunqin Zhao]

在 2026/11/10 16:03, Borislav Petkov 写道:
> On Tue, Nov 10, 2026 at 03:47:20PM +0800, Qunqin Zhao wrote:
>> "all your nodes belong to memory controller 0"
>> I don't quite follow this sentence. There are four memory controllers on LS3C6000/S.
>>
>> On firmware without mc-idx there can still be more than one  memory controller
> So edac_device_alloc_index() is used to give out indices for other EDAC
> devices with edac_device_alloc_ctl_info() but I guess you could use that in
> your driver until you don't need to add other devices besides MCs and then
> it'll become a mess...
>
> Lemme ask the simplest question first: is there any case where ACPI won't

mc-idx is not guaranteed. There is firmware that does not provide the property.


Qunqin,

Thanks.

> provide that _DSD property or you're in control of the firmware - not like on
> x86 - and this case we're trying to fix is never going to happen anyway...?
>
> Thx.
>

Re: [PATCH v6 2/4] EDAC/loongson: Encode node and MC info into mc_idx

[Borislav Petkov]

On Tue, Nov 10, 2026 at 04:23:29PM +0800, Qunqin Zhao wrote:
> mc-idx is not guaranteed. There is firmware that does not provide the property.

Ok, I guess you can do something like this then. It should work naturally with
the rest of the EDAC MC index handling:

diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c
index 3c5d29941a38..78fa8d87da0b 100644
--- a/drivers/edac/loongson_edac.c
+++ b/drivers/edac/loongson_edac.c
@@ -14,6 +14,9 @@
 
 #define ECC_CS_COUNT_REG	0x18
 
+/* Memory controller index invalid due to inability to read it from ACPI, etc */
+#define MC_IDX_INVALID		BIT(16)
+
 struct loongson_pvt {
 	void __iomem *ecc_base;
 
@@ -25,7 +28,6 @@ struct loongson_pvt {
 	 */
 	int last_ce_count;
 	u32 mcs_per_node;
-	bool mc_idx_valid;
 };
 
 static int read_ecc(struct mem_ctl_info *mci)
@@ -56,7 +58,7 @@ static void loongson_check(struct mem_ctl_info *mci)
 	if (add <= 0)
 		return;
 
-	if (pvt->mc_idx_valid) {
+	if (pvt->mc_idx >= MC_IDX_INVALID) {
 		node = mci->mc_idx / pvt->mcs_per_node;
 		mc = mci->mc_idx % pvt->mcs_per_node;
 		snprintf(other_detail, sizeof(other_detail), "node:%d mc:%d", node, mc);
@@ -82,14 +84,12 @@ static void dimm_config_init(struct mem_ctl_info *mci)
 	dimm->grain = 8;
 }
 
-static void pvt_init(struct mem_ctl_info *mci, void __iomem *vbase,
-		     bool mc_idx_valid, u32 mcs_per_node)
+static void pvt_init(struct mem_ctl_info *mci, void __iomem *vbase, u32 mcs_per_node)
 {
 	struct loongson_pvt *pvt = mci->pvt_info;
 
 	pvt->ecc_base = vbase;
 	pvt->last_ce_count = read_ecc(mci);
-	pvt->mc_idx_valid = mc_idx_valid;
 	pvt->mcs_per_node = mcs_per_node;
 }
 
@@ -100,7 +100,6 @@ static int loongson_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	void __iomem *vbase;
 	u32 mcs_per_node;
-	bool mc_idx_valid = true;
 	int ret;
 
 	vbase = devm_platform_ioremap_resource(pdev, 0);
@@ -129,15 +128,13 @@ static int loongson_probe(struct platform_device *pdev)
 	mci->error_desc.grain = 8;
 	mci->edac_check = loongson_check;
 
-	if (device_property_read_u32(dev, "mc-idx", &mci->mc_idx)) {
-		mci->mc_idx = edac_device_alloc_index();
-		mc_idx_valid = false;
-	}
+	if (device_property_read_u32(dev, "mc-idx", &mci->mc_idx))
+		mci->mc_idx = edac_device_alloc_index() | MC_IDX_INVALID;
 
 	if (device_property_read_u32(dev, "mcs-per-node", &mcs_per_node) || mcs_per_node == 0)
 		mcs_per_node = 4;
 
-	pvt_init(mci, vbase, mc_idx_valid, mcs_per_node);
+	pvt_init(mci, vbase, mcs_per_node);
 	dimm_config_init(mci);
 
 	ret = edac_mc_add_mc(mci);

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v6 2/4] EDAC/loongson: Encode node and MC info into mc_idx

[Qunqin Zhao]

在 2026/11/11 02:31, Borislav Petkov 写道:
> On Tue, Nov 10, 2026 at 04:23:29PM +0800, Qunqin Zhao wrote:
>> mc-idx is not guaranteed. There is firmware that does not provide the property.
> Ok, I guess you can do something like this then. It should work naturally with
> the rest of the EDAC MC index handling:
>
> diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c
> index 3c5d29941a38..78fa8d87da0b 100644
> --- a/drivers/edac/loongson_edac.c
> +++ b/drivers/edac/loongson_edac.c
> @@ -14,6 +14,9 @@
>  
>  #define ECC_CS_COUNT_REG	0x18
>  
> +/* Memory controller index invalid due to inability to read it from ACPI, etc */
> +#define MC_IDX_INVALID		BIT(16)
> +
>  struct loongson_pvt {
>  	void __iomem *ecc_base;
>  
> @@ -25,7 +28,6 @@ struct loongson_pvt {
>  	 */
>  	int last_ce_count;
>  	u32 mcs_per_node;
> -	bool mc_idx_valid;
>  };
>  
>  static int read_ecc(struct mem_ctl_info *mci)
> @@ -56,7 +58,7 @@ static void loongson_check(struct mem_ctl_info *mci)
>  	if (add <= 0)
>  		return;
>  
> -	if (pvt->mc_idx_valid) {
> +	if (pvt->mc_idx >= MC_IDX_INVALID) {
>  		node = mci->mc_idx / pvt->mcs_per_node;
>  		mc = mci->mc_idx % pvt->mcs_per_node;
>  		snprintf(other_detail, sizeof(other_detail), "node:%d mc:%d", node, mc);
> @@ -82,14 +84,12 @@ static void dimm_config_init(struct mem_ctl_info *mci)
>  	dimm->grain = 8;
>  }
>  
> -static void pvt_init(struct mem_ctl_info *mci, void __iomem *vbase,
> -		     bool mc_idx_valid, u32 mcs_per_node)
> +static void pvt_init(struct mem_ctl_info *mci, void __iomem *vbase, u32 mcs_per_node)
>  {
>  	struct loongson_pvt *pvt = mci->pvt_info;
>  
>  	pvt->ecc_base = vbase;
>  	pvt->last_ce_count = read_ecc(mci);
> -	pvt->mc_idx_valid = mc_idx_valid;
>  	pvt->mcs_per_node = mcs_per_node;
>  }
>  
> @@ -100,7 +100,6 @@ static int loongson_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	void __iomem *vbase;
>  	u32 mcs_per_node;
> -	bool mc_idx_valid = true;
>  	int ret;
>  
>  	vbase = devm_platform_ioremap_resource(pdev, 0);
> @@ -129,15 +128,13 @@ static int loongson_probe(struct platform_device *pdev)
>  	mci->error_desc.grain = 8;
>  	mci->edac_check = loongson_check;
>  
> -	if (device_property_read_u32(dev, "mc-idx", &mci->mc_idx)) {
> -		mci->mc_idx = edac_device_alloc_index();
> -		mc_idx_valid = false;
> -	}
> +	if (device_property_read_u32(dev, "mc-idx", &mci->mc_idx))
> +		mci->mc_idx = edac_device_alloc_index() | MC_IDX_INVALID;

Thanks for your patch. I’ve tested it and found a minor issue. 

If we encode the presence of the _DSD property into mc_idx, 

the sysfs representation will become confusing. For example:

loongson@loongson:~$ ls /sys/devices/system/edac/mc/
mc65536  power  subsystem  uevent
^^^^^^


Qunqin,

Thanks.

>  
>  	if (device_property_read_u32(dev, "mcs-per-node", &mcs_per_node) || mcs_per_node == 0)
>  		mcs_per_node = 4;
>  
> -	pvt_init(mci, vbase, mc_idx_valid, mcs_per_node);
> +	pvt_init(mci, vbase, mcs_per_node);
>  	dimm_config_init(mci);
>  
>  	ret = edac_mc_add_mc(mci);
>

Re: [PATCH v6 2/4] EDAC/loongson: Encode node and MC info into mc_idx

[Borislav Petkov]

On November 11, 2026 6:37:39 AM UTC, Qunqin Zhao <zhaoqunqin@loongson.cn> wrote:
>If we encode the presence of the _DSD property into mc_idx, 
>
>the sysfs representation will become confusing. For example:
>
>loongson@loongson:~$ ls /sys/devices/system/edac/mc/
>mc65536  power  subsystem  uevent
>^^^^^^

Does it really matter which wrong numbers you have there? They are wrong no matter which ones you choose. 

-- 
Small device. Typos and formatting crap

Re: [PATCH v6 2/4] EDAC/loongson: Encode node and MC info into mc_idx

[Qunqin Zhao]

在 2026/11/11 16:08, Borislav Petkov 写道:
> On November 11, 2026 6:37:39 AM UTC, Qunqin Zhao <zhaoqunqin@loongson.cn> wrote:
>> If we encode the presence of the _DSD property into mc_idx, 
>>
>> the sysfs representation will become confusing. For example:
>>
>> loongson@loongson:~$ ls /sys/devices/system/edac/mc/
>> mc65536  power  subsystem  uevent
>> ^^^^^^
> Does it really matter which wrong numbers you have there? They are wrong no matter which ones you choose. 

You’re right, it doesn’t matter which wrong numbers we choose. 

I’ll rework this in the next version following your suggestion.

Qunqin,

Thanks.

>

[PATCH v6 3/4] EDAC/loongson: Get DIMM size from ACPI _DSM method

[Qunqin Zhao]

From: Wang Jinwei <wangjinwei@loongson.cn>

Add support to retrieve DIMM size from ACPI _DSM method instead
of using hardcoded values.

1. Add loongson_get_dimm_size_acpi() to call ACPI _DSM method
   to Get DIMM size in MB.

2. Update dimm_config_init() to use the size from ACPI, with
   fallback to default 1 MB if ACPI method fails.

3. Add ACPI UUID and DSM function definitions for EDAC.

The DIMM size is passed from UEFI through ACPI OperationRegion
and _DSM method, enabling correct display of memory size in
/sys/devices/system/edac/mc/mc*/size_mb.

Cc: Dongyan Qian <qiandongyan@loongson.cn>
Cc: Chao Li <lichao@loongson.cn>
Signed-off-by: Wang Jinwei <wangjinwei@loongson.cn>
Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
---
 drivers/edac/loongson_edac.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c
index 3c5d29941a..c19b025004 100644
--- a/drivers/edac/loongson_edac.c
+++ b/drivers/edac/loongson_edac.c
@@ -13,6 +13,8 @@
 #include "edac_module.h"
 
 #define ECC_CS_COUNT_REG	0x18
+#define LOONGSON_EDAC_DSM_UUID	"65d0431b-7eb8-46df-b914-b7d568553140"
+#define LOONGSON_EDAC_DSM_FUNC_GET_DIMM_SIZE	1
 
 struct loongson_pvt {
 	void __iomem *ecc_base;
@@ -66,13 +68,39 @@ static void loongson_check(struct mem_ctl_info *mci)
 			     0, 0, 0, 0, 0, -1, "error", other_detail);
 }
 
+static u32 loongson_get_dimm_size_acpi(struct device *dev)
+{
+	acpi_handle handle = ACPI_HANDLE(dev);
+	union acpi_object *obj;
+	u32 size_mb = 1;
+	guid_t guid;
+
+	if (!handle)
+		return size_mb;
+
+	if (guid_parse(LOONGSON_EDAC_DSM_UUID, &guid))
+		return size_mb;
+
+	obj = acpi_evaluate_dsm(handle, &guid, 0,
+				LOONGSON_EDAC_DSM_FUNC_GET_DIMM_SIZE, NULL);
+	if (!obj)
+		return size_mb;
+
+	if (obj->type == ACPI_TYPE_INTEGER)
+		size_mb = (u32)obj->integer.value;
+
+	ACPI_FREE(obj);
+	return size_mb;
+}
+
 static void dimm_config_init(struct mem_ctl_info *mci)
 {
 	struct dimm_info *dimm;
+	struct device *dev = mci->pdev;
 	u32 size, npages;
 
-	/* size not used */
-	size = -1;
+	size = loongson_get_dimm_size_acpi(dev);
+
 	npages = MiB_TO_PAGES(size);
 
 	dimm = edac_get_dimm(mci, 0, 0, 0);
-- 
2.47.2

[PATCH v6 4/4] EDAC/loongson: Add CS4-7 timeout filter for cross-node access

[Qunqin Zhao]

From: Qunqin Zhao <zhaoqunqin@loongson.cn>

On Loongson 3C6000 multi-node systems, when the EDAC driver running
on one node accesses the memory controller registers of another node,
the read operation may timeout and return random values. This causes
false CE error reports because the random values in CS0-3 (low 32 bits)
are misinterpreted as valid ECC error counts.

For cross-node accesses, check CS4-7 (high 32 bits) of the ECC
count register. If the valid-cs-bits firmware property is set and
the high bits are non-zero, it indicates a timeout or random value,
so skip the report and return the previous count.

To avoid undefined behavior on 8-CS systems where valid_cs_bits=64,
guard the shift with a bounds check (valid_cs_bits < 64).

Signed-off-by: Wusheng <wusheng@loongson.cn>
Signed-off-by: Wang Jinwei <wangjinwei@loongson.cn>
Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
---
 drivers/edac/loongson_edac.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c
index c19b025004..4f79c74cb7 100644
--- a/drivers/edac/loongson_edac.c
+++ b/drivers/edac/loongson_edac.c
@@ -27,6 +27,7 @@ struct loongson_pvt {
 	 */
 	int last_ce_count;
 	u32 mcs_per_node;
+	u32 valid_cs_bits;
 	bool mc_idx_valid;
 };
 
@@ -37,11 +38,21 @@ static int read_ecc(struct mem_ctl_info *mci)
 	int cs;
 
 	ecc = readq(pvt->ecc_base + ECC_CS_COUNT_REG);
-	/* cs0 -- cs3 */
+	/* Discard the read value if any invalid bit is set to 1 */
+	if (pvt->valid_cs_bits < 64 && (ecc >> pvt->valid_cs_bits)) {
+		edac_mc_printk(mci, KERN_DEBUG, "ECC read invalid, skip: 0x%llx\n", ecc);
+		return pvt->last_ce_count;
+	}
+
+	/* cs0 -- cs7 */
 	cs = ecc & 0xff;
 	cs += (ecc >> 8) & 0xff;
 	cs += (ecc >> 16) & 0xff;
 	cs += (ecc >> 24) & 0xff;
+	cs += (ecc >> 32) & 0xff;
+	cs += (ecc >> 40) & 0xff;
+	cs += (ecc >> 48) & 0xff;
+	cs += (ecc >> 56) & 0xff;
 
 	return cs;
 }
@@ -111,14 +122,15 @@ static void dimm_config_init(struct mem_ctl_info *mci)
 }
 
 static void pvt_init(struct mem_ctl_info *mci, void __iomem *vbase,
-		     bool mc_idx_valid, u32 mcs_per_node)
+		     bool mc_idx_valid, u32 mcs_per_node, u32 valid_cs_bits)
 {
 	struct loongson_pvt *pvt = mci->pvt_info;
 
 	pvt->ecc_base = vbase;
-	pvt->last_ce_count = read_ecc(mci);
 	pvt->mc_idx_valid = mc_idx_valid;
 	pvt->mcs_per_node = mcs_per_node;
+	pvt->valid_cs_bits = valid_cs_bits;
+	pvt->last_ce_count = read_ecc(mci);
 }
 
 static int loongson_probe(struct platform_device *pdev)
@@ -127,7 +139,7 @@ static int loongson_probe(struct platform_device *pdev)
 	struct mem_ctl_info *mci;
 	struct device *dev = &pdev->dev;
 	void __iomem *vbase;
-	u32 mcs_per_node;
+	u32 mcs_per_node, valid_cs_bits;
 	bool mc_idx_valid = true;
 	int ret;
 
@@ -165,7 +177,10 @@ static int loongson_probe(struct platform_device *pdev)
 	if (device_property_read_u32(dev, "mcs-per-node", &mcs_per_node) || mcs_per_node == 0)
 		mcs_per_node = 4;
 
-	pvt_init(mci, vbase, mc_idx_valid, mcs_per_node);
+	if (device_property_read_u32(dev, "valid-cs-bits", &valid_cs_bits))
+		valid_cs_bits = 32;
+
+	pvt_init(mci, vbase, mc_idx_valid, mcs_per_node, valid_cs_bits);
 	dimm_config_init(mci);
 
 	ret = edac_mc_add_mc(mci);
-- 
2.47.2