[PATCH 0/4] Cleanups in amd64_edac

[PATCH 0/4] Cleanups in amd64_edac

[Avadhut Naik]

This patchset undertakes some cleanups in the amd64_edac module.

The first patch removes explicit assignment of the ctl_name string and
instead generates it at runtime using scnprintf.

The second patch removes the NUM_CONTROLLERS macro and instead uses the
max_mcs variable to determine the size of chipselect array.

The third patch sets zn_regs_v2 flag for all SOCs in Family 1Ah and later.

The fourth patch initiates deprecation of the legacy EDAC sysfs interface.

NOTE:
These cleanups were earlier submitted as part of the set adding EDAC
support for AMD's newer Family 1Ah-based SOCs.[1] However, since the
support patches were critical to enabling EDAC on newer AMD SOCs and
since these cleanups were still under review, the set was split up.

[1]: https://lore.kernel.org/all/20250909185748.1621098-1-avadhut.naik@amd.com/

Avadhut Naik (4):
  EDAC/amd64: Generate ctl_name string at runtime
  EDAC/amd64: Remove NUM_CONTROLLERS macro
  EDAC/amd64: Set zn_regs_v2 flag for all AMD Family 1Ah-based SOCs
  EDAC/mc_sysfs: Begin deprecating legacy sysfs EDAC interface

 drivers/edac/Kconfig         |  2 +-
 drivers/edac/amd64_edac.c    | 69 +++++++++---------------------------
 drivers/edac/amd64_edac.h    |  9 ++---
 drivers/edac/edac_mc.h       |  5 +++
 drivers/edac/edac_mc_sysfs.c | 16 +++++++++
 5 files changed, 44 insertions(+), 57 deletions(-)


base-commit: 5c4a065eebd70227b51bb2c8d06e9e4c1b313b05
-- 
2.43.0

[PATCH 1/4] EDAC/amd64: Generate ctl_name string at runtime

[Avadhut Naik]

Currently, the ctl_name string is statically assigned based on the family
and model of the SOC when the amd64_edac module is loaded.

The same, however, is not exactly needed as the string can be generated
and assigned at runtime through scnprintf().

Remove all static assignments and generate the string at runtime. Also,
cleanup the switch cases which became defunct and consolidate identical
cases.

Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
---
 drivers/edac/amd64_edac.c | 56 +++++++--------------------------------
 drivers/edac/amd64_edac.h |  4 ++-
 2 files changed, 13 insertions(+), 47 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 2f6ab783bf20..886ad075d222 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -3766,6 +3766,7 @@ static int per_family_init(struct amd64_pvt *pvt)
 	pvt->stepping	= boot_cpu_data.x86_stepping;
 	pvt->model	= boot_cpu_data.x86_model;
 	pvt->fam	= boot_cpu_data.x86;
+	char *tmp_name = NULL;
 	pvt->max_mcs	= 2;
 
 	/*
@@ -3779,7 +3780,7 @@ static int per_family_init(struct amd64_pvt *pvt)
 
 	switch (pvt->fam) {
 	case 0xf:
-		pvt->ctl_name				= (pvt->ext_model >= K8_REV_F) ?
+		tmp_name				= (pvt->ext_model >= K8_REV_F) ?
 							  "K8 revF or later" : "K8 revE or earlier";
 		pvt->f1_id				= PCI_DEVICE_ID_AMD_K8_NB_ADDRMAP;
 		pvt->f2_id				= PCI_DEVICE_ID_AMD_K8_NB_MEMCTL;
@@ -3788,7 +3789,6 @@ static int per_family_init(struct amd64_pvt *pvt)
 		break;
 
 	case 0x10:
-		pvt->ctl_name				= "F10h";
 		pvt->f1_id				= PCI_DEVICE_ID_AMD_10H_NB_MAP;
 		pvt->f2_id				= PCI_DEVICE_ID_AMD_10H_NB_DRAM;
 		pvt->ops->dbam_to_cs			= f10_dbam_to_chip_select;
@@ -3797,12 +3797,10 @@ static int per_family_init(struct amd64_pvt *pvt)
 	case 0x15:
 		switch (pvt->model) {
 		case 0x30:
-			pvt->ctl_name			= "F15h_M30h";
 			pvt->f1_id			= PCI_DEVICE_ID_AMD_15H_M30H_NB_F1;
 			pvt->f2_id			= PCI_DEVICE_ID_AMD_15H_M30H_NB_F2;
 			break;
 		case 0x60:
-			pvt->ctl_name			= "F15h_M60h";
 			pvt->f1_id			= PCI_DEVICE_ID_AMD_15H_M60H_NB_F1;
 			pvt->f2_id			= PCI_DEVICE_ID_AMD_15H_M60H_NB_F2;
 			pvt->ops->dbam_to_cs		= f15_m60h_dbam_to_chip_select;
@@ -3811,7 +3809,6 @@ static int per_family_init(struct amd64_pvt *pvt)
 			/* Richland is only client */
 			return -ENODEV;
 		default:
-			pvt->ctl_name			= "F15h";
 			pvt->f1_id			= PCI_DEVICE_ID_AMD_15H_NB_F1;
 			pvt->f2_id			= PCI_DEVICE_ID_AMD_15H_NB_F2;
 			pvt->ops->dbam_to_cs		= f15_dbam_to_chip_select;
@@ -3822,12 +3819,10 @@ static int per_family_init(struct amd64_pvt *pvt)
 	case 0x16:
 		switch (pvt->model) {
 		case 0x30:
-			pvt->ctl_name			= "F16h_M30h";
 			pvt->f1_id			= PCI_DEVICE_ID_AMD_16H_M30H_NB_F1;
 			pvt->f2_id			= PCI_DEVICE_ID_AMD_16H_M30H_NB_F2;
 			break;
 		default:
-			pvt->ctl_name			= "F16h";
 			pvt->f1_id			= PCI_DEVICE_ID_AMD_16H_NB_F1;
 			pvt->f2_id			= PCI_DEVICE_ID_AMD_16H_NB_F2;
 			break;
@@ -3836,76 +3831,51 @@ static int per_family_init(struct amd64_pvt *pvt)
 
 	case 0x17:
 		switch (pvt->model) {
-		case 0x10 ... 0x2f:
-			pvt->ctl_name			= "F17h_M10h";
-			break;
 		case 0x30 ... 0x3f:
-			pvt->ctl_name			= "F17h_M30h";
 			pvt->max_mcs			= 8;
 			break;
-		case 0x60 ... 0x6f:
-			pvt->ctl_name			= "F17h_M60h";
-			break;
-		case 0x70 ... 0x7f:
-			pvt->ctl_name			= "F17h_M70h";
-			break;
 		default:
-			pvt->ctl_name			= "F17h";
 			break;
 		}
 		break;
 
 	case 0x18:
-		pvt->ctl_name				= "F18h";
 		break;
 
 	case 0x19:
 		switch (pvt->model) {
 		case 0x00 ... 0x0f:
-			pvt->ctl_name			= "F19h";
 			pvt->max_mcs			= 8;
 			break;
 		case 0x10 ... 0x1f:
-			pvt->ctl_name			= "F19h_M10h";
 			pvt->max_mcs			= 12;
 			pvt->flags.zn_regs_v2		= 1;
 			break;
-		case 0x20 ... 0x2f:
-			pvt->ctl_name			= "F19h_M20h";
-			break;
 		case 0x30 ... 0x3f:
 			if (pvt->F3->device == PCI_DEVICE_ID_AMD_MI200_DF_F3) {
-				pvt->ctl_name		= "MI200";
+				tmp_name			= "MI200";
 				pvt->max_mcs		= 4;
 				pvt->dram_type		= MEM_HBM2;
 				pvt->gpu_umc_base	= 0x50000;
 				pvt->ops		= &gpu_ops;
 			} else {
-				pvt->ctl_name		= "F19h_M30h";
 				pvt->max_mcs		= 8;
 			}
 			break;
-		case 0x50 ... 0x5f:
-			pvt->ctl_name			= "F19h_M50h";
-			break;
 		case 0x60 ... 0x6f:
-			pvt->ctl_name			= "F19h_M60h";
 			pvt->flags.zn_regs_v2		= 1;
 			break;
 		case 0x70 ... 0x7f:
-			pvt->ctl_name			= "F19h_M70h";
 			pvt->max_mcs			= 4;
 			pvt->flags.zn_regs_v2		= 1;
 			break;
 		case 0x90 ... 0x9f:
-			pvt->ctl_name			= "F19h_M90h";
 			pvt->max_mcs			= 4;
 			pvt->dram_type			= MEM_HBM3;
 			pvt->gpu_umc_base		= 0x90000;
 			pvt->ops			= &gpu_ops;
 			break;
 		case 0xa0 ... 0xaf:
-			pvt->ctl_name			= "F19h_MA0h";
 			pvt->max_mcs			= 12;
 			pvt->flags.zn_regs_v2		= 1;
 			break;
@@ -3915,34 +3885,22 @@ static int per_family_init(struct amd64_pvt *pvt)
 	case 0x1A:
 		switch (pvt->model) {
 		case 0x00 ... 0x1f:
-			pvt->ctl_name           = "F1Ah";
 			pvt->max_mcs            = 12;
 			pvt->flags.zn_regs_v2   = 1;
 			break;
 		case 0x40 ... 0x4f:
-			pvt->ctl_name           = "F1Ah_M40h";
 			pvt->flags.zn_regs_v2   = 1;
 			break;
 		case 0x50 ... 0x57:
-			pvt->ctl_name           = "F1Ah_M50h";
+		case 0xc0 ... 0xc7:
 			pvt->max_mcs            = 16;
 			pvt->flags.zn_regs_v2   = 1;
 			break;
 		case 0x90 ... 0x9f:
-			pvt->ctl_name           = "F1Ah_M90h";
-			pvt->max_mcs            = 8;
-			pvt->flags.zn_regs_v2   = 1;
-			break;
 		case 0xa0 ... 0xaf:
-			pvt->ctl_name           = "F1Ah_MA0h";
 			pvt->max_mcs            = 8;
 			pvt->flags.zn_regs_v2   = 1;
 			break;
-		case 0xc0 ... 0xc7:
-			pvt->ctl_name           = "F1Ah_MC0h";
-			pvt->max_mcs            = 16;
-			pvt->flags.zn_regs_v2   = 1;
-			break;
 		}
 		break;
 
@@ -3951,6 +3909,12 @@ static int per_family_init(struct amd64_pvt *pvt)
 		return -ENODEV;
 	}
 
+	if (tmp_name)
+		scnprintf(pvt->ctl_name, sizeof(pvt->ctl_name), tmp_name);
+	else
+		scnprintf(pvt->ctl_name, sizeof(pvt->ctl_name), "F%02Xh_M%02Xh",
+			  pvt->fam, pvt->model);
+
 	return 0;
 }
 
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index d70b8a8d0b09..bb6cf4b1ab77 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -101,6 +101,8 @@
 #define ON true
 #define OFF false
 
+#define MAX_CTL_NAMELEN	20
+
 /*
  * PCI-defined configuration space registers
  */
@@ -362,7 +364,7 @@ struct amd64_pvt {
 	/* x4, x8, or x16 syndromes in use */
 	u8 ecc_sym_sz;
 
-	const char *ctl_name;
+	char ctl_name[MAX_CTL_NAMELEN];
 	u16 f1_id, f2_id;
 	/* Maximum number of memory controllers per die/node. */
 	u8 max_mcs;
-- 
2.43.0

Re: [PATCH 1/4] EDAC/amd64: Generate ctl_name string at runtime

[Borislav Petkov]

On Mon, Oct 13, 2025 at 05:30:40PM +0000, Avadhut Naik wrote:
> +#define MAX_CTL_NAMELEN	20

Any particular reason for the 20?

AFAICT, the longest string is "K8 revE or earlier" which is 18 so 19 with the
'\0'...

-- 
Regards/Gruss,
    Boris.

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

[PATCH 1/4] EDAC/amd64: Generate ctl_name string at runtime

[Naik, Avadhut]

On 10/20/2025 08:47, Borislav Petkov wrote:
> On Mon, Oct 13, 2025 at 05:30:40PM +0000, Avadhut Naik wrote:
>> +#define MAX_CTL_NAMELEN	20
> 
> Any particular reason for the 20?
> 
> AFAICT, the longest string is "K8 revE or earlier" which is 18 so 19 with the
> '\0'...
> 
No reason as such. Will change it to 19.
Do you have any other feedback on this set?

-- 
Thanks,
Avadhut Naik

Re: [PATCH 1/4] EDAC/amd64: Generate ctl_name string at runtime

[Borislav Petkov]

On Mon, Oct 20, 2025 at 01:28:05PM -0500, Naik, Avadhut wrote:
> No reason as such. Will change it to 19.
> Do you have any other feedback on this set?

I'll let you know.

Thx.

-- 
Regards/Gruss,
    Boris.

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

[PATCH 2/4] EDAC/amd64: Remove NUM_CONTROLLERS macro

[Avadhut Naik]

Currently, the NUM_CONTROLLERS macro is only used to statically allocate
the csels array of struct chip_select in struct amd64_pvt.

The size of this array, however, will never exceed the number of UMCs on
the SOC. Since, max_mcs variable in struct amd64_pvt already stores the
number of UMCs on the SOC, the macro can be removed and the static array
can be dynamically allocated instead.

The max_mcs variable and the csels array are used for legacy systems too.
These systems have a max of 2 controllers (DCTs). Since the default value
of max_mcs, set in per_family_init(), is 2, these legacy system are also
covered by this change.

Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
---
 drivers/edac/amd64_edac.c | 5 +++++
 drivers/edac/amd64_edac.h | 5 ++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 886ad075d222..2391f3469961 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -3732,6 +3732,7 @@ static void hw_info_put(struct amd64_pvt *pvt)
 	pci_dev_put(pvt->F1);
 	pci_dev_put(pvt->F2);
 	kfree(pvt->umc);
+	kfree(pvt->csels);
 }
 
 static struct low_ops umc_ops = {
@@ -3915,6 +3916,10 @@ static int per_family_init(struct amd64_pvt *pvt)
 		scnprintf(pvt->ctl_name, sizeof(pvt->ctl_name), "F%02Xh_M%02Xh",
 			  pvt->fam, pvt->model);
 
+	pvt->csels = kcalloc(pvt->max_mcs, sizeof(*pvt->csels), GFP_KERNEL);
+	if (!pvt->csels)
+		return -ENOMEM;
+
 	return 0;
 }
 
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index bb6cf4b1ab77..5f61631c8a7d 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -96,7 +96,6 @@
 /* Hardware limit on ChipSelect rows per MC and processors per system */
 #define NUM_CHIPSELECTS			8
 #define DRAM_RANGES			8
-#define NUM_CONTROLLERS			16
 
 #define ON true
 #define OFF false
@@ -347,8 +346,8 @@ struct amd64_pvt {
 	u32 dbam0;		/* DRAM Base Address Mapping reg for DCT0 */
 	u32 dbam1;		/* DRAM Base Address Mapping reg for DCT1 */
 
-	/* one for each DCT/UMC */
-	struct chip_select csels[NUM_CONTROLLERS];
+	/* Allocate one for each DCT/UMC */
+	struct chip_select *csels;
 
 	/* DRAM base and limit pairs F1x[78,70,68,60,58,50,48,40] */
 	struct dram_range ranges[DRAM_RANGES];
-- 
2.43.0

Re: [PATCH 2/4] EDAC/amd64: Remove NUM_CONTROLLERS macro

[Borislav Petkov]

On Mon, Oct 13, 2025 at 05:30:41PM +0000, Avadhut Naik wrote:
> Currently, the NUM_CONTROLLERS macro is only used to statically allocate
> the csels array of struct chip_select in struct amd64_pvt.

"... is used to limit the amount of memory controllers available per node."

You don't need to explain the code - think big picture.

> The size of this array, however, will never exceed the number of UMCs on
> the SOC.

Not on the SOC - the thing is per node instance.

> Since, max_mcs variable in struct amd64_pvt already stores the
> number of UMCs on the SOC, the macro can be removed and the static array

Please describe your changes in imperative mood.

Also, pls read section "2) Describe your changes" in
Documentation/process/submitting-patches.rst for more details.

> can be dynamically allocated instead.
> 
> The max_mcs variable and the csels array are used for legacy systems too.
> These systems have a max of 2 controllers (DCTs). Since the default value

DCTs are DRAM controllers. Do not confuse the reader.

> of max_mcs, set in per_family_init(), is 2, these legacy system are also
> covered by this change.

...

> @@ -347,8 +346,8 @@ struct amd64_pvt {
>  	u32 dbam0;		/* DRAM Base Address Mapping reg for DCT0 */
>  	u32 dbam1;		/* DRAM Base Address Mapping reg for DCT1 */
>  
> -	/* one for each DCT/UMC */
> -	struct chip_select csels[NUM_CONTROLLERS];
> +	/* Allocate one for each DCT/UMC */

You're not allocating here anything. Just explain what this variable
represents - IOW, the comment was fine.

> +	struct chip_select *csels;
>  
>  	/* DRAM base and limit pairs F1x[78,70,68,60,58,50,48,40] */
>  	struct dram_range ranges[DRAM_RANGES];
> -- 
> 2.43.0
> 

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 2/4] EDAC/amd64: Remove NUM_CONTROLLERS macro

[Naik, Avadhut]

On 10/21/2025 05:44, Borislav Petkov wrote:
> On Mon, Oct 13, 2025 at 05:30:41PM +0000, Avadhut Naik wrote:
>> Currently, the NUM_CONTROLLERS macro is only used to statically allocate
>> the csels array of struct chip_select in struct amd64_pvt.
> 
> "... is used to limit the amount of memory controllers available per node."
> 
> You don't need to explain the code - think big picture.
> 
Okay!

>> The size of this array, however, will never exceed the number of UMCs on
>> the SOC.
> 
> Not on the SOC - the thing is per node instance.
>
Will change this!
 
>> Since, max_mcs variable in struct amd64_pvt already stores the
>> number of UMCs on the SOC, the macro can be removed and the static array
> 
> Please describe your changes in imperative mood.
> 
> Also, pls read section "2) Describe your changes" in
> Documentation/process/submitting-patches.rst for more details.
> 
Will do!

>> can be dynamically allocated instead.
>>
>> The max_mcs variable and the csels array are used for legacy systems too.
>> These systems have a max of 2 controllers (DCTs). Since the default value
> 
> DCTs are DRAM controllers. Do not confuse the reader.
> 
Will remove the *DCTs* word altogether.

>> of max_mcs, set in per_family_init(), is 2, these legacy system are also
>> covered by this change.
> 
> ...
> 
>> @@ -347,8 +346,8 @@ struct amd64_pvt {
>>  	u32 dbam0;		/* DRAM Base Address Mapping reg for DCT0 */
>>  	u32 dbam1;		/* DRAM Base Address Mapping reg for DCT1 */
>>  
>> -	/* one for each DCT/UMC */
>> -	struct chip_select csels[NUM_CONTROLLERS];
>> +	/* Allocate one for each DCT/UMC */
> 
> You're not allocating here anything. Just explain what this variable
> represents - IOW, the comment was fine.
> 
Will revert this!

>> +	struct chip_select *csels;
>>  
>>  	/* DRAM base and limit pairs F1x[78,70,68,60,58,50,48,40] */
>>  	struct dram_range ranges[DRAM_RANGES];
>> -- 
>> 2.43.0
>>
> 

-- 
Thanks,
Avadhut Naik

[PATCH 3/4] EDAC/amd64: Set zn_regs_v2 flag for all AMD Family 1Ah-based SOCs

[Avadhut Naik]

The zn_regs_v2 flag should be set for all AMD Family 1Ah-based SOCs.

Set the flag once for all 1Ah-based SOCs and avoid repetetive assignment.

Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
---
 drivers/edac/amd64_edac.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 2391f3469961..832f9675e7b0 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -3779,6 +3779,8 @@ static int per_family_init(struct amd64_pvt *pvt)
 	else
 		pvt->ops = &dct_ops;
 
+	pvt->flags.zn_regs_v2 = (pvt->fam >= 0x1A) ? 1 : 0;
+
 	switch (pvt->fam) {
 	case 0xf:
 		tmp_name				= (pvt->ext_model >= K8_REV_F) ?
@@ -3887,20 +3889,14 @@ static int per_family_init(struct amd64_pvt *pvt)
 		switch (pvt->model) {
 		case 0x00 ... 0x1f:
 			pvt->max_mcs            = 12;
-			pvt->flags.zn_regs_v2   = 1;
-			break;
-		case 0x40 ... 0x4f:
-			pvt->flags.zn_regs_v2   = 1;
 			break;
 		case 0x50 ... 0x57:
 		case 0xc0 ... 0xc7:
 			pvt->max_mcs            = 16;
-			pvt->flags.zn_regs_v2   = 1;
 			break;
 		case 0x90 ... 0x9f:
 		case 0xa0 ... 0xaf:
 			pvt->max_mcs            = 8;
-			pvt->flags.zn_regs_v2   = 1;
 			break;
 		}
 		break;
-- 
2.43.0

Re: [PATCH 3/4] EDAC/amd64: Set zn_regs_v2 flag for all AMD Family 1Ah-based SOCs

[Borislav Petkov]

On Mon, Oct 13, 2025 at 05:30:42PM +0000, Avadhut Naik wrote:
> The zn_regs_v2 flag should be set for all AMD Family 1Ah-based SOCs.
> 
> Set the flag once for all 1Ah-based SOCs and avoid repetetive assignment.

Unknown word [repetetive] in commit message.
Suggestions: ['repetitive', 'repetitively', 'recitative', 'putative']

Please introduce a spellchecker into your patch creation workflow.

> Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
> ---
>  drivers/edac/amd64_edac.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 2391f3469961..832f9675e7b0 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -3779,6 +3779,8 @@ static int per_family_init(struct amd64_pvt *pvt)
>  	else
>  		pvt->ops = &dct_ops;
>  
> +	pvt->flags.zn_regs_v2 = (pvt->fam >= 0x1A) ? 1 : 0;
> +
>  	switch (pvt->fam) {
>  	case 0xf:
>  		tmp_name				= (pvt->ext_model >= K8_REV_F) ?
> @@ -3887,20 +3889,14 @@ static int per_family_init(struct amd64_pvt *pvt)
>  		switch (pvt->model) {
>  		case 0x00 ... 0x1f:
>  			pvt->max_mcs            = 12;
> -			pvt->flags.zn_regs_v2   = 1;
> -			break;
> -		case 0x40 ... 0x4f:
> -			pvt->flags.zn_regs_v2   = 1;
>  			break;
>  		case 0x50 ... 0x57:
>  		case 0xc0 ... 0xc7:
>  			pvt->max_mcs            = 16;
> -			pvt->flags.zn_regs_v2   = 1;
>  			break;
>  		case 0x90 ... 0x9f:
>  		case 0xa0 ... 0xaf:
>  			pvt->max_mcs            = 8;
> -			pvt->flags.zn_regs_v2   = 1;
>  			break;
>  		}

I'm not sure about this: if we hoist this particular assignment up, then
what's the point of the tabellary switch-case where you can see at a quick
glance, all the settings that we do per model...?

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 3/4] EDAC/amd64: Set zn_regs_v2 flag for all AMD Family 1Ah-based SOCs

[Yazen Ghannam]

On Wed, Oct 29, 2025 at 06:10:57PM +0100, Borislav Petkov wrote:
> On Mon, Oct 13, 2025 at 05:30:42PM +0000, Avadhut Naik wrote:

[...]

> > --- a/drivers/edac/amd64_edac.c
> > +++ b/drivers/edac/amd64_edac.c
> > @@ -3779,6 +3779,8 @@ static int per_family_init(struct amd64_pvt *pvt)
> >  	else
> >  		pvt->ops = &dct_ops;
> >  
> > +	pvt->flags.zn_regs_v2 = (pvt->fam >= 0x1A) ? 1 : 0;
> > +
> >  	switch (pvt->fam) {
> >  	case 0xf:
> >  		tmp_name				= (pvt->ext_model >= K8_REV_F) ?
> > @@ -3887,20 +3889,14 @@ static int per_family_init(struct amd64_pvt *pvt)
> >  		switch (pvt->model) {
> >  		case 0x00 ... 0x1f:
> >  			pvt->max_mcs            = 12;
> > -			pvt->flags.zn_regs_v2   = 1;
> > -			break;
> > -		case 0x40 ... 0x4f:
> > -			pvt->flags.zn_regs_v2   = 1;
> >  			break;
> >  		case 0x50 ... 0x57:
> >  		case 0xc0 ... 0xc7:
> >  			pvt->max_mcs            = 16;
> > -			pvt->flags.zn_regs_v2   = 1;
> >  			break;
> >  		case 0x90 ... 0x9f:
> >  		case 0xa0 ... 0xaf:
> >  			pvt->max_mcs            = 8;
> > -			pvt->flags.zn_regs_v2   = 1;
> >  			break;
> >  		}
> 
> I'm not sure about this: if we hoist this particular assignment up, then
> what's the point of the tabellary switch-case where you can see at a quick
> glance, all the settings that we do per model...?
> 

I think we should take any opportunity to get away from family/model
checks.

Yes, we still have one item (max_mcs) left. As a follow up, we can see
if this can be removed also.

Thanks,
Yazen

Re: [PATCH 3/4] EDAC/amd64: Set zn_regs_v2 flag for all AMD Family 1Ah-based SOCs

[Borislav Petkov]

On Thu, Oct 30, 2025 at 09:48:39AM -0400, Yazen Ghannam wrote:
> 
> I think we should take any opportunity to get away from family/model
> checks.

But we're not getting rid of them - we're just partially hoisting the
zn_regs_v2 assignment for some only.

This looks like an unfinished patch to me so what's the point of it even?

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 3/4] EDAC/amd64: Set zn_regs_v2 flag for all AMD Family 1Ah-based SOCs

[Yazen Ghannam]

On Mon, Nov 03, 2025 at 10:18:48PM +0100, Borislav Petkov wrote:
> On Thu, Oct 30, 2025 at 09:48:39AM -0400, Yazen Ghannam wrote:
> > 
> > I think we should take any opportunity to get away from family/model
> > checks.
> 
> But we're not getting rid of them - we're just partially hoisting the
> zn_regs_v2 assignment for some only.
> 
> This looks like an unfinished patch to me so what's the point of it even?
> 

Hmm, okay. I see your point. We can drop this if/until we have more
substantial changes.

Thanks,
Yazen

[PATCH 4/4] EDAC/mc_sysfs: Begin deprecating legacy sysfs EDAC interface

[Avadhut Naik]

The legacy sysfs EDAC interface has been made obsolete more than a decade
ago through the introduction of a new per-DIMM interface.

The legacy interface however, hasn't been removed till date.

Begin deprecating it so that it can eventually be removed by v6.21.

Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
---
 drivers/edac/Kconfig         |  2 +-
 drivers/edac/edac_mc.h       |  5 +++++
 drivers/edac/edac_mc_sysfs.c | 16 ++++++++++++++++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 39352b9b7a7e..fdfeba848114 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -25,7 +25,7 @@ if EDAC
 
 config EDAC_LEGACY_SYSFS
 	bool "EDAC legacy sysfs"
-	default y
+	default n
 	help
 	  Enable the compatibility sysfs nodes.
 	  Use 'Y' if your edac utilities aren't ported to work with the newer
diff --git a/drivers/edac/edac_mc.h b/drivers/edac/edac_mc.h
index 881b00eadf7a..78b49d6906fd 100644
--- a/drivers/edac/edac_mc.h
+++ b/drivers/edac/edac_mc.h
@@ -95,6 +95,11 @@ do {									\
 
 #define to_mci(k) container_of(k, struct mem_ctl_info, dev)
 
+static inline void deprecate_interface(void)
+{
+	pr_warn_once("NOTICE: The legacy EDAC sysfs interface has been deprecated and will be removed by v6.21. Please switch to the new interface!\n");
+}
+
 /**
  * edac_mc_alloc() - Allocate and partially fill a struct &mem_ctl_info.
  *
diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 8689631f1905..8ed4b239b7f1 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -144,6 +144,8 @@ struct dev_ch_attribute {
 static ssize_t csrow_ue_count_show(struct device *dev,
 				   struct device_attribute *mattr, char *data)
 {
+	deprecate_interface();
+
 	struct csrow_info *csrow = to_csrow(dev);
 
 	return sysfs_emit(data, "%u\n", csrow->ue_count);
@@ -152,6 +154,8 @@ static ssize_t csrow_ue_count_show(struct device *dev,
 static ssize_t csrow_ce_count_show(struct device *dev,
 				   struct device_attribute *mattr, char *data)
 {
+	deprecate_interface();
+
 	struct csrow_info *csrow = to_csrow(dev);
 
 	return sysfs_emit(data, "%u\n", csrow->ce_count);
@@ -160,6 +164,8 @@ static ssize_t csrow_ce_count_show(struct device *dev,
 static ssize_t csrow_size_show(struct device *dev,
 			       struct device_attribute *mattr, char *data)
 {
+	deprecate_interface();
+
 	struct csrow_info *csrow = to_csrow(dev);
 	int i;
 	u32 nr_pages = 0;
@@ -172,6 +178,8 @@ static ssize_t csrow_size_show(struct device *dev,
 static ssize_t csrow_mem_type_show(struct device *dev,
 				   struct device_attribute *mattr, char *data)
 {
+	deprecate_interface();
+
 	struct csrow_info *csrow = to_csrow(dev);
 
 	return sysfs_emit(data, "%s\n", edac_mem_types[csrow->channels[0]->dimm->mtype]);
@@ -180,6 +188,8 @@ static ssize_t csrow_mem_type_show(struct device *dev,
 static ssize_t csrow_dev_type_show(struct device *dev,
 				   struct device_attribute *mattr, char *data)
 {
+	deprecate_interface();
+
 	struct csrow_info *csrow = to_csrow(dev);
 
 	return sysfs_emit(data, "%s\n", dev_types[csrow->channels[0]->dimm->dtype]);
@@ -189,6 +199,8 @@ static ssize_t csrow_edac_mode_show(struct device *dev,
 				    struct device_attribute *mattr,
 				    char *data)
 {
+	deprecate_interface();
+
 	struct csrow_info *csrow = to_csrow(dev);
 
 	return sysfs_emit(data, "%s\n", edac_caps[csrow->channels[0]->dimm->edac_mode]);
@@ -199,6 +211,8 @@ static ssize_t channel_dimm_label_show(struct device *dev,
 				       struct device_attribute *mattr,
 				       char *data)
 {
+	deprecate_interface();
+
 	struct csrow_info *csrow = to_csrow(dev);
 	unsigned int chan = to_channel(mattr);
 	struct rank_info *rank = csrow->channels[chan];
@@ -238,6 +252,8 @@ static ssize_t channel_dimm_label_store(struct device *dev,
 static ssize_t channel_ce_count_show(struct device *dev,
 				     struct device_attribute *mattr, char *data)
 {
+	deprecate_interface();
+
 	struct csrow_info *csrow = to_csrow(dev);
 	unsigned int chan = to_channel(mattr);
 	struct rank_info *rank = csrow->channels[chan];
-- 
2.43.0

Re: [PATCH 4/4] EDAC/mc_sysfs: Begin deprecating legacy sysfs EDAC interface

[Borislav Petkov]

+ Greg to tell us whether that would be a proper deprecation strategy.

On Mon, Oct 13, 2025 at 05:30:43PM +0000, Avadhut Naik wrote:
> The legacy sysfs EDAC interface has been made obsolete more than a decade
> ago through the introduction of a new per-DIMM interface.
> 
> The legacy interface however, hasn't been removed till date.
> 
> Begin deprecating it so that it can eventually be removed by v6.21.
> 
> Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
> ---
>  drivers/edac/Kconfig         |  2 +-
>  drivers/edac/edac_mc.h       |  5 +++++
>  drivers/edac/edac_mc_sysfs.c | 16 ++++++++++++++++
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 39352b9b7a7e..fdfeba848114 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -25,7 +25,7 @@ if EDAC
>  
>  config EDAC_LEGACY_SYSFS
>  	bool "EDAC legacy sysfs"

Add "DEPRECATED: EDAC ..."

here.

> -	default y
> +	default n
>  	help
>  	  Enable the compatibility sysfs nodes.
>  	  Use 'Y' if your edac utilities aren't ported to work with the newer
> diff --git a/drivers/edac/edac_mc.h b/drivers/edac/edac_mc.h
> index 881b00eadf7a..78b49d6906fd 100644
> --- a/drivers/edac/edac_mc.h
> +++ b/drivers/edac/edac_mc.h
> @@ -95,6 +95,11 @@ do {									\
>  
>  #define to_mci(k) container_of(k, struct mem_ctl_info, dev)
>  
> +static inline void deprecate_interface(void)
> +{
> +	pr_warn_once("NOTICE: The legacy EDAC sysfs interface has been deprecated and will be removed by v6.21. Please switch to the new interface!\n");
> +}

You don't need to have a function which you replicate everywhere. Simply dump
this notice once...

edac_create_sysfs_mci_device:

	...

#ifdef CONFIG_EDAC_LEGACY_SYSFS

<--- here.

        err = edac_create_csrow_objects(mci);
        if (err < 0)
                goto fail;
#endif

So that it gets issued and hopefully someone sees it.

Then, I'd say around 6.19 we should make those functions return an error
unconditionally and then zap them in 6.21.

That is, if no one comes crawling out of the woodwork with a valid use case.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 4/4] EDAC/mc_sysfs: Begin deprecating legacy sysfs EDAC interface

[Naik, Avadhut]

On 10/29/2025 12:24, Borislav Petkov wrote:
> + Greg to tell us whether that would be a proper deprecation strategy.
> 
> On Mon, Oct 13, 2025 at 05:30:43PM +0000, Avadhut Naik wrote:
>> The legacy sysfs EDAC interface has been made obsolete more than a decade
>> ago through the introduction of a new per-DIMM interface.
>>
>> The legacy interface however, hasn't been removed till date.
>>
>> Begin deprecating it so that it can eventually be removed by v6.21.
>>
>> Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
>> ---
>>  drivers/edac/Kconfig         |  2 +-
>>  drivers/edac/edac_mc.h       |  5 +++++
>>  drivers/edac/edac_mc_sysfs.c | 16 ++++++++++++++++
>>  3 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
>> index 39352b9b7a7e..fdfeba848114 100644
>> --- a/drivers/edac/Kconfig
>> +++ b/drivers/edac/Kconfig
>> @@ -25,7 +25,7 @@ if EDAC
>>  
>>  config EDAC_LEGACY_SYSFS
>>  	bool "EDAC legacy sysfs"
> 
> Add "DEPRECATED: EDAC ..."
> 
> here.
> 
Okay!
>> -	default y
>> +	default n
>>  	help
>>  	  Enable the compatibility sysfs nodes.
>>  	  Use 'Y' if your edac utilities aren't ported to work with the newer
>> diff --git a/drivers/edac/edac_mc.h b/drivers/edac/edac_mc.h
>> index 881b00eadf7a..78b49d6906fd 100644
>> --- a/drivers/edac/edac_mc.h
>> +++ b/drivers/edac/edac_mc.h
>> @@ -95,6 +95,11 @@ do {									\
>>  
>>  #define to_mci(k) container_of(k, struct mem_ctl_info, dev)
>>  
>> +static inline void deprecate_interface(void)
>> +{
>> +	pr_warn_once("NOTICE: The legacy EDAC sysfs interface has been deprecated and will be removed by v6.21. Please switch to the new interface!\n");
>> +}
> 
> You don't need to have a function which you replicate everywhere. Simply dump
> this notice once...
> 
> edac_create_sysfs_mci_device:
> 
> 	...
> 
> #ifdef CONFIG_EDAC_LEGACY_SYSFS
> 
> <--- here.
> 
>         err = edac_create_csrow_objects(mci);
>         if (err < 0)
>                 goto fail;
> #endif
> 
> So that it gets issued and hopefully someone sees it.
> 
> Then, I'd say around 6.19 we should make those functions return an error
> unconditionally and then zap them in 6.21.
> 
> That is, if no one comes crawling out of the woodwork with a valid use case.
> 
> Thx.
> 
Sounds good! Will do!

Just one question:

Wouldn't it be more prudent to have a log of our intentions available when someone
tries to access or read stuff the old interface too?

-- 
Thanks,
Avadhut Naik

Re: [PATCH 4/4] EDAC/mc_sysfs: Begin deprecating legacy sysfs EDAC interface

[Greg KH]

On Wed, Oct 29, 2025 at 06:24:19PM +0100, Borislav Petkov wrote:
> + Greg to tell us whether that would be a proper deprecation strategy.
> 
> On Mon, Oct 13, 2025 at 05:30:43PM +0000, Avadhut Naik wrote:
> > The legacy sysfs EDAC interface has been made obsolete more than a decade
> > ago through the introduction of a new per-DIMM interface.
> > 
> > The legacy interface however, hasn't been removed till date.
> > 
> > Begin deprecating it so that it can eventually be removed by v6.21.
> > 
> > Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
> > ---
> >  drivers/edac/Kconfig         |  2 +-
> >  drivers/edac/edac_mc.h       |  5 +++++
> >  drivers/edac/edac_mc_sysfs.c | 16 ++++++++++++++++
> >  3 files changed, 22 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> > index 39352b9b7a7e..fdfeba848114 100644
> > --- a/drivers/edac/Kconfig
> > +++ b/drivers/edac/Kconfig
> > @@ -25,7 +25,7 @@ if EDAC
> >  
> >  config EDAC_LEGACY_SYSFS
> >  	bool "EDAC legacy sysfs"
> 
> Add "DEPRECATED: EDAC ..."
> 
> here.

Hah, good luck!

> > -	default y
> > +	default n
> >  	help
> >  	  Enable the compatibility sysfs nodes.
> >  	  Use 'Y' if your edac utilities aren't ported to work with the newer
> > diff --git a/drivers/edac/edac_mc.h b/drivers/edac/edac_mc.h
> > index 881b00eadf7a..78b49d6906fd 100644
> > --- a/drivers/edac/edac_mc.h
> > +++ b/drivers/edac/edac_mc.h
> > @@ -95,6 +95,11 @@ do {									\
> >  
> >  #define to_mci(k) container_of(k, struct mem_ctl_info, dev)
> >  
> > +static inline void deprecate_interface(void)
> > +{
> > +	pr_warn_once("NOTICE: The legacy EDAC sysfs interface has been deprecated and will be removed by v6.21. Please switch to the new interface!\n");
> > +}
> 
> You don't need to have a function which you replicate everywhere. Simply dump
> this notice once...
> 
> edac_create_sysfs_mci_device:
> 
> 	...
> 
> #ifdef CONFIG_EDAC_LEGACY_SYSFS
> 
> <--- here.
> 
>         err = edac_create_csrow_objects(mci);
>         if (err < 0)
>                 goto fail;
> #endif
> 
> So that it gets issued and hopefully someone sees it.
> 
> Then, I'd say around 6.19 we should make those functions return an error
> unconditionally and then zap them in 6.21.
> 
> That is, if no one comes crawling out of the woodwork with a valid use case.

No one is going to notice this type of kernel log message.  If you think
that no one is using the sysfs files, delete them now.  Why wait?

sysfs is meant to be such that userspace can handle file removals (i.e.
that value is not present.)  Unfortunately, sometimes this does not
actually happen and user tools do mess up and rely on things.  So either
no one uses the file and it can be removed now, OR you have to leave it
in for "forever".  There's no real chance to remove it later, that's
just postponing the decision.

good luck!

greg k-h

Re: [PATCH 4/4] EDAC/mc_sysfs: Begin deprecating legacy sysfs EDAC interface

[Borislav Petkov]

On Thu, Oct 30, 2025 at 04:01:43PM +0100, Greg KH wrote:
> No one is going to notice this type of kernel log message.  If you think
> that no one is using the sysfs files, delete them now.  Why wait?
> 
> sysfs is meant to be such that userspace can handle file removals (i.e.
> that value is not present.)  Unfortunately, sometimes this does not
> actually happen and user tools do mess up and rely on things.  So either
> no one uses the file and it can be removed now, OR you have to leave it
> in for "forever".  There's no real chance to remove it later, that's
> just postponing the decision.

Lemme try to repeat what you're proposing to make sure we're on the same page:

we simply zap the files and see if anyone complains. If yes, we figure out why
they're using them and if they can't be moved to the new interface, we restore
them and maintain them forever.

Right?

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 4/4] EDAC/mc_sysfs: Begin deprecating legacy sysfs EDAC interface

[Greg KH]

On Thu, Oct 30, 2025 at 08:18:58PM +0100, Borislav Petkov wrote:
> On Thu, Oct 30, 2025 at 04:01:43PM +0100, Greg KH wrote:
> > No one is going to notice this type of kernel log message.  If you think
> > that no one is using the sysfs files, delete them now.  Why wait?
> > 
> > sysfs is meant to be such that userspace can handle file removals (i.e.
> > that value is not present.)  Unfortunately, sometimes this does not
> > actually happen and user tools do mess up and rely on things.  So either
> > no one uses the file and it can be removed now, OR you have to leave it
> > in for "forever".  There's no real chance to remove it later, that's
> > just postponing the decision.
> 
> Lemme try to repeat what you're proposing to make sure we're on the same page:
> 
> we simply zap the files and see if anyone complains. If yes, we figure out why
> they're using them and if they can't be moved to the new interface, we restore
> them and maintain them forever.
> 
> Right?

Yup!

Re: [PATCH 4/4] EDAC/mc_sysfs: Begin deprecating legacy sysfs EDAC interface

[Borislav Petkov]

On Fri, Oct 31, 2025 at 01:38:57PM +0100, Greg KH wrote:
> Yup!

Thanks.

@Avadhut: you can send the next revision now. :)

Thx.


-- 
Regards/Gruss,
    Boris.

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

[PATCH 4/4] EDAC/mc_sysfs: Begin deprecating legacy sysfs EDAC interface

[Naik, Avadhut]

On 10/31/2025 07:54, Borislav Petkov wrote:
> On Fri, Oct 31, 2025 at 01:38:57PM +0100, Greg KH wrote:
>> Yup!
> 
> Thanks.
> 
> @Avadhut: you can send the next revision now. :)
> 
> Thx.
> 
> 
Will do. Will remove the interface and resubmit this set.
-- 
Thanks,
Avadhut Naik