Re: [PATCH 1/3] EDAC/loongson: Encode node and MC info into mc_idx

Hi, Qunqin,

On Thu, Jul 30, 2026 at 2:45=E2=80=AFPM Qunqin Zhao <[email protected]=
> wrote:
>
> From: Wang Jinwei <[email protected]>
>
> 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 'mc-idx' 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 (mc-per-node) is obtained
> from firmware via ACPI _DSD property, with a default of 4 for
> backward compatibility. The node and MC IDs are decoded from mc_idx:
>      node =3D mc_idx / mc_per_node
>      mc =3D mc_idx % mc_per_node
>
> For example, with mc-per-node=3D4:
>      mc-idx 0-3   -> node0 mc0-mc3
>      mc-idx 4-7   -> node1 mc0-mc3
>      ...
>      mc-idx 28-31 -> node7 mc0-mc3
>
> The dmesg output looks like this=EF=BC=9A
> Before: (no _DSD mc-idx)
> EDAC MC3: 510 CE error on MC#3Channel#0_DIMM#0 (channel:0 slot:0
> page:0x0 offset:0x0 grain:8 syndrome:0x0)
>
> After: (_DSD mc-idx)
> EDAC MC3: 510 CE error on MC#3Channel#0_DIMM#0 (channel:0 slot:0
> page:0x0 offset:0x0 grain:8 syndrome:0x0 - node:0 mc:3)
>
> Cc: Yulong Wang <[email protected]>
> Cc: Dongyan Qian <[email protected]>
> Cc: Chao Li <[email protected]>
> Signed-off-by: Wang Jinwei <[email protected]>
> Signed-off-by: Qunqin Zhao <[email protected]>
> ---
>  drivers/edac/loongson_edac.c | 38 +++++++++++++++++++++++++++++++-----
>  1 file changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c
> index 38745800ed..f2452e98f5 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_edac_pvt {
>          * register state.
>          */
>         int last_ce_count;
> +       int mc_per_node;
I think mcs_per_node is better than mc_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 edac_check(struct mem_ctl_info *mci)
>  {
>         struct loongson_edac_pvt *pvt =3D mci->pvt_info;
> -       int new, add;
> +       char other_detail[64];
Use char other_detail[64] =3D {0}, then we can drop the 'else' branch below=
.

> +       int new, add, node, mc;
>
>         new =3D read_ecc(mci);
>         add =3D new - pvt->last_ce_count;
> @@ -52,8 +56,17 @@ static void edac_check(struct mem_ctl_info *mci)
>         if (add <=3D 0)
>                 return;
>
> +       if (pvt->mc_idx_valid) {
> +               node =3D mci->mc_idx / pvt->mc_per_node;
> +               mc =3D mci->mc_idx % pvt->mc_per_node;
> +               snprintf(other_detail, sizeof(other_detail),
> +                               "node:%d mc:%d", node, mc);
The snprintf() can be put in oneline.

> +       } else {
> +               other_detail[0] =3D '\0';
> +       }
> +
>         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,20 +85,26 @@ static void dimm_config_init(struct mem_ctl_info *mci=
)
>         dimm->grain =3D 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, int mc_per_node)
The function definition can be put in one line.

>  {
>         struct loongson_edac_pvt *pvt =3D mci->pvt_info;
>
>         pvt->ecc_base =3D vbase;
>         pvt->last_ce_count =3D read_ecc(mci);
> +       pvt->mc_idx_valid =3D mc_idx_valid;
> +       pvt->mc_per_node =3D mc_per_node;
>  }
>
>  static int edac_probe(struct platform_device *pdev)
>  {
>         struct edac_mc_layer layers[2];
>         struct mem_ctl_info *mci;
> +       struct device *dev =3D &pdev->dev;
>         void __iomem *vbase;
> +       u32 mc_per_node;
>         int ret;
> +       bool mc_idx_valid;
Use bool mc_idx_valid =3D true, then we can drop the 'else' branch below.


Huacai

>
>         vbase =3D devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(vbase))
> @@ -102,7 +121,6 @@ static int edac_probe(struct platform_device *pdev)
>         if (mci =3D=3D NULL)
>                 return -ENOMEM;
>
> -       mci->mc_idx =3D edac_device_alloc_index();
>         mci->mtype_cap =3D MEM_FLAG_RDDR4;
>         mci->edac_ctl_cap =3D EDAC_FLAG_NONE;
>         mci->edac_cap =3D EDAC_FLAG_NONE;
> @@ -114,7 +132,17 @@ static int edac_probe(struct platform_device *pdev)
>         mci->error_desc.grain =3D 8;
>         mci->edac_check =3D edac_check;
>
> -       pvt_init(mci, vbase);
> +       if (device_property_read_u32(dev, "mc-idx", &mci->mc_idx)) {
> +               mci->mc_idx =3D edac_device_alloc_index();
> +               mc_idx_valid =3D false;
> +       } else {
> +               mc_idx_valid =3D true;
> +       }
> +
> +       if (device_property_read_u32(dev, "mc-per-node", &mc_per_node))
> +               mc_per_node =3D 4;
> +
> +       pvt_init(mci, vbase, mc_idx_valid, mc_per_node);
>         dimm_config_init(mci);
>
>         ret =3D edac_mc_add_mc(mci);
>
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
> --
> 2.47.2
>
>