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);
>