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

[Qunqin Zhao <zhaoqunqin@loongson.cn>]

在 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.

>