Cleanup and add support for AMD Family 1Ah-based SOCs

[PATCH v3 4/5] EDAC/mc_sysfs: Increase legacy channel support to 16

Posted by Avadhut Naik 3 weeks, 2 days ago

Newer AMD systems can support up to 16 channels per EDAC "mc" device.
These are detected by the EDAC module running on the device, and the
current EDAC interface is appropriately enumerated.

The legacy EDAC sysfs interface however, provides device attributes for
channels 0 through 11 only. Consequently, the last four channels, 12
through 15, will not be enumerated and will not be visible through the
legacy sysfs interface.

Add additional device attributes to ensure that all 16 channels, if
present, are enumerated by and visible through the legacy EDAC sysfs
interface.

Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
---
Changes in v2:
Patch introduced.

Changes in v3:
No changes.
---
 drivers/edac/edac_mc_sysfs.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 0f338adf7d93..8689631f1905 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -305,6 +305,14 @@ DEVICE_CHANNEL(ch10_dimm_label, S_IRUGO | S_IWUSR,
 	channel_dimm_label_show, channel_dimm_label_store, 10);
 DEVICE_CHANNEL(ch11_dimm_label, S_IRUGO | S_IWUSR,
 	channel_dimm_label_show, channel_dimm_label_store, 11);
+DEVICE_CHANNEL(ch12_dimm_label, S_IRUGO | S_IWUSR,
+	channel_dimm_label_show, channel_dimm_label_store, 12);
+DEVICE_CHANNEL(ch13_dimm_label, S_IRUGO | S_IWUSR,
+	channel_dimm_label_show, channel_dimm_label_store, 13);
+DEVICE_CHANNEL(ch14_dimm_label, S_IRUGO | S_IWUSR,
+	channel_dimm_label_show, channel_dimm_label_store, 14);
+DEVICE_CHANNEL(ch15_dimm_label, S_IRUGO | S_IWUSR,
+	channel_dimm_label_show, channel_dimm_label_store, 15);
 
 /* Total possible dynamic DIMM Label attribute file table */
 static struct attribute *dynamic_csrow_dimm_attr[] = {
@@ -320,6 +328,10 @@ static struct attribute *dynamic_csrow_dimm_attr[] = {
 	&dev_attr_legacy_ch9_dimm_label.attr.attr,
 	&dev_attr_legacy_ch10_dimm_label.attr.attr,
 	&dev_attr_legacy_ch11_dimm_label.attr.attr,
+	&dev_attr_legacy_ch12_dimm_label.attr.attr,
+	&dev_attr_legacy_ch13_dimm_label.attr.attr,
+	&dev_attr_legacy_ch14_dimm_label.attr.attr,
+	&dev_attr_legacy_ch15_dimm_label.attr.attr,
 	NULL
 };
 
@@ -348,6 +360,14 @@ DEVICE_CHANNEL(ch10_ce_count, S_IRUGO,
 		   channel_ce_count_show, NULL, 10);
 DEVICE_CHANNEL(ch11_ce_count, S_IRUGO,
 		   channel_ce_count_show, NULL, 11);
+DEVICE_CHANNEL(ch12_ce_count, S_IRUGO,
+		   channel_ce_count_show, NULL, 12);
+DEVICE_CHANNEL(ch13_ce_count, S_IRUGO,
+		   channel_ce_count_show, NULL, 13);
+DEVICE_CHANNEL(ch14_ce_count, S_IRUGO,
+		   channel_ce_count_show, NULL, 14);
+DEVICE_CHANNEL(ch15_ce_count, S_IRUGO,
+		   channel_ce_count_show, NULL, 15);
 
 /* Total possible dynamic ce_count attribute file table */
 static struct attribute *dynamic_csrow_ce_count_attr[] = {
@@ -363,6 +383,10 @@ static struct attribute *dynamic_csrow_ce_count_attr[] = {
 	&dev_attr_legacy_ch9_ce_count.attr.attr,
 	&dev_attr_legacy_ch10_ce_count.attr.attr,
 	&dev_attr_legacy_ch11_ce_count.attr.attr,
+	&dev_attr_legacy_ch12_ce_count.attr.attr,
+	&dev_attr_legacy_ch13_ce_count.attr.attr,
+	&dev_attr_legacy_ch14_ce_count.attr.attr,
+	&dev_attr_legacy_ch15_ce_count.attr.attr,
 	NULL
 };
 
-- 
2.43.0

Re: [PATCH v3 4/5] EDAC/mc_sysfs: Increase legacy channel support to 16

Posted by Yazen Ghannam 3 weeks, 1 day ago

On Tue, Sep 09, 2025 at 06:53:13PM +0000, Avadhut Naik wrote:
> Newer AMD systems can support up to 16 channels per EDAC "mc" device.
> These are detected by the EDAC module running on the device, and the
> current EDAC interface is appropriately enumerated.
> 
> The legacy EDAC sysfs interface however, provides device attributes for
> channels 0 through 11 only. Consequently, the last four channels, 12
> through 15, will not be enumerated and will not be visible through the
> legacy sysfs interface.
> 
> Add additional device attributes to ensure that all 16 channels, if
> present, are enumerated by and visible through the legacy EDAC sysfs
> interface.
> 
> Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
> ---
> Changes in v2:
> Patch introduced.
> 
> Changes in v3:
> No changes.
> ---
>  drivers/edac/edac_mc_sysfs.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
> index 0f338adf7d93..8689631f1905 100644
> --- a/drivers/edac/edac_mc_sysfs.c
> +++ b/drivers/edac/edac_mc_sysfs.c
> @@ -305,6 +305,14 @@ DEVICE_CHANNEL(ch10_dimm_label, S_IRUGO | S_IWUSR,
>  	channel_dimm_label_show, channel_dimm_label_store, 10);
>  DEVICE_CHANNEL(ch11_dimm_label, S_IRUGO | S_IWUSR,
>  	channel_dimm_label_show, channel_dimm_label_store, 11);
> +DEVICE_CHANNEL(ch12_dimm_label, S_IRUGO | S_IWUSR,
> +	channel_dimm_label_show, channel_dimm_label_store, 12);
> +DEVICE_CHANNEL(ch13_dimm_label, S_IRUGO | S_IWUSR,
> +	channel_dimm_label_show, channel_dimm_label_store, 13);
> +DEVICE_CHANNEL(ch14_dimm_label, S_IRUGO | S_IWUSR,
> +	channel_dimm_label_show, channel_dimm_label_store, 14);
> +DEVICE_CHANNEL(ch15_dimm_label, S_IRUGO | S_IWUSR,
> +	channel_dimm_label_show, channel_dimm_label_store, 15);
>  
>  /* Total possible dynamic DIMM Label attribute file table */
>  static struct attribute *dynamic_csrow_dimm_attr[] = {
> @@ -320,6 +328,10 @@ static struct attribute *dynamic_csrow_dimm_attr[] = {
>  	&dev_attr_legacy_ch9_dimm_label.attr.attr,
>  	&dev_attr_legacy_ch10_dimm_label.attr.attr,
>  	&dev_attr_legacy_ch11_dimm_label.attr.attr,
> +	&dev_attr_legacy_ch12_dimm_label.attr.attr,
> +	&dev_attr_legacy_ch13_dimm_label.attr.attr,
> +	&dev_attr_legacy_ch14_dimm_label.attr.attr,
> +	&dev_attr_legacy_ch15_dimm_label.attr.attr,
>  	NULL
>  };
>  
> @@ -348,6 +360,14 @@ DEVICE_CHANNEL(ch10_ce_count, S_IRUGO,
>  		   channel_ce_count_show, NULL, 10);
>  DEVICE_CHANNEL(ch11_ce_count, S_IRUGO,
>  		   channel_ce_count_show, NULL, 11);
> +DEVICE_CHANNEL(ch12_ce_count, S_IRUGO,
> +		   channel_ce_count_show, NULL, 12);
> +DEVICE_CHANNEL(ch13_ce_count, S_IRUGO,
> +		   channel_ce_count_show, NULL, 13);
> +DEVICE_CHANNEL(ch14_ce_count, S_IRUGO,
> +		   channel_ce_count_show, NULL, 14);
> +DEVICE_CHANNEL(ch15_ce_count, S_IRUGO,
> +		   channel_ce_count_show, NULL, 15);
>  
>  /* Total possible dynamic ce_count attribute file table */
>  static struct attribute *dynamic_csrow_ce_count_attr[] = {
> @@ -363,6 +383,10 @@ static struct attribute *dynamic_csrow_ce_count_attr[] = {
>  	&dev_attr_legacy_ch9_ce_count.attr.attr,
>  	&dev_attr_legacy_ch10_ce_count.attr.attr,
>  	&dev_attr_legacy_ch11_ce_count.attr.attr,
> +	&dev_attr_legacy_ch12_ce_count.attr.attr,
> +	&dev_attr_legacy_ch13_ce_count.attr.attr,
> +	&dev_attr_legacy_ch14_ce_count.attr.attr,
> +	&dev_attr_legacy_ch15_ce_count.attr.attr,
>  	NULL
>  };
>  
> -- 

There are many checkpatch warnings here.

Maybe it'd be prudent to note this in the commit message?

Something like "checkpatch warnings ignored to maintain coding style"
and maybe "affected lines are deprecated and will be removed", etc.?

Otherwise, I expect there will be some "checkpatch warning" fixes coming
our way.

Thanks,
Yazen

Re: [PATCH v3 4/5] EDAC/mc_sysfs: Increase legacy channel support to 16

Posted by Naik, Avadhut 3 weeks, 1 day ago


On 9/10/2025 10:17, Yazen Ghannam wrote:
> On Tue, Sep 09, 2025 at 06:53:13PM +0000, Avadhut Naik wrote:
>> Newer AMD systems can support up to 16 channels per EDAC "mc" device.
>> These are detected by the EDAC module running on the device, and the
>> current EDAC interface is appropriately enumerated.
>>
>> The legacy EDAC sysfs interface however, provides device attributes for
>> channels 0 through 11 only. Consequently, the last four channels, 12
>> through 15, will not be enumerated and will not be visible through the
>> legacy sysfs interface.
>>
>> Add additional device attributes to ensure that all 16 channels, if
>> present, are enumerated by and visible through the legacy EDAC sysfs
>> interface.
>>
>> Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
>> ---
>> Changes in v2:
>> Patch introduced.
>>
>> Changes in v3:
>> No changes.
>> ---
>>  drivers/edac/edac_mc_sysfs.c | 24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
>> index 0f338adf7d93..8689631f1905 100644
>> --- a/drivers/edac/edac_mc_sysfs.c
>> +++ b/drivers/edac/edac_mc_sysfs.c
>> @@ -305,6 +305,14 @@ DEVICE_CHANNEL(ch10_dimm_label, S_IRUGO | S_IWUSR,
>>  	channel_dimm_label_show, channel_dimm_label_store, 10);
>>  DEVICE_CHANNEL(ch11_dimm_label, S_IRUGO | S_IWUSR,
>>  	channel_dimm_label_show, channel_dimm_label_store, 11);
>> +DEVICE_CHANNEL(ch12_dimm_label, S_IRUGO | S_IWUSR,
>> +	channel_dimm_label_show, channel_dimm_label_store, 12);
>> +DEVICE_CHANNEL(ch13_dimm_label, S_IRUGO | S_IWUSR,
>> +	channel_dimm_label_show, channel_dimm_label_store, 13);
>> +DEVICE_CHANNEL(ch14_dimm_label, S_IRUGO | S_IWUSR,
>> +	channel_dimm_label_show, channel_dimm_label_store, 14);
>> +DEVICE_CHANNEL(ch15_dimm_label, S_IRUGO | S_IWUSR,
>> +	channel_dimm_label_show, channel_dimm_label_store, 15);
>>  
>>  /* Total possible dynamic DIMM Label attribute file table */
>>  static struct attribute *dynamic_csrow_dimm_attr[] = {
>> @@ -320,6 +328,10 @@ static struct attribute *dynamic_csrow_dimm_attr[] = {
>>  	&dev_attr_legacy_ch9_dimm_label.attr.attr,
>>  	&dev_attr_legacy_ch10_dimm_label.attr.attr,
>>  	&dev_attr_legacy_ch11_dimm_label.attr.attr,
>> +	&dev_attr_legacy_ch12_dimm_label.attr.attr,
>> +	&dev_attr_legacy_ch13_dimm_label.attr.attr,
>> +	&dev_attr_legacy_ch14_dimm_label.attr.attr,
>> +	&dev_attr_legacy_ch15_dimm_label.attr.attr,
>>  	NULL
>>  };
>>  
>> @@ -348,6 +360,14 @@ DEVICE_CHANNEL(ch10_ce_count, S_IRUGO,
>>  		   channel_ce_count_show, NULL, 10);
>>  DEVICE_CHANNEL(ch11_ce_count, S_IRUGO,
>>  		   channel_ce_count_show, NULL, 11);
>> +DEVICE_CHANNEL(ch12_ce_count, S_IRUGO,
>> +		   channel_ce_count_show, NULL, 12);
>> +DEVICE_CHANNEL(ch13_ce_count, S_IRUGO,
>> +		   channel_ce_count_show, NULL, 13);
>> +DEVICE_CHANNEL(ch14_ce_count, S_IRUGO,
>> +		   channel_ce_count_show, NULL, 14);
>> +DEVICE_CHANNEL(ch15_ce_count, S_IRUGO,
>> +		   channel_ce_count_show, NULL, 15);
>>  
>>  /* Total possible dynamic ce_count attribute file table */
>>  static struct attribute *dynamic_csrow_ce_count_attr[] = {
>> @@ -363,6 +383,10 @@ static struct attribute *dynamic_csrow_ce_count_attr[] = {
>>  	&dev_attr_legacy_ch9_ce_count.attr.attr,
>>  	&dev_attr_legacy_ch10_ce_count.attr.attr,
>>  	&dev_attr_legacy_ch11_ce_count.attr.attr,
>> +	&dev_attr_legacy_ch12_ce_count.attr.attr,
>> +	&dev_attr_legacy_ch13_ce_count.attr.attr,
>> +	&dev_attr_legacy_ch14_ce_count.attr.attr,
>> +	&dev_attr_legacy_ch15_ce_count.attr.attr,
>>  	NULL
>>  };
>>  
>> -- 
> 
> There are many checkpatch warnings here.
> 
> Maybe it'd be prudent to note this in the commit message?
> 
> Something like "checkpatch warnings ignored to maintain coding style"
> and maybe "affected lines are deprecated and will be removed", etc.?
> 
> Otherwise, I expect there will be some "checkpatch warning" fixes coming
> our way.
> 

Initially, was going to fix the checkpatch warnings. But then noticed that
previous commits to this area had ignored warnings too. So let them be as
is.

In any case, will add about this to the commit message.
> Thanks,
> Yazen

-- 
Thanks,
Avadhut Naik

[PATCH v3 1/5] EDAC/amd64: Generate ctl_name string at runtime
[PATCH v3 2/5] EDAC/amd64: Remove NUM_CONTROLLERS macro
[PATCH v3 3/5] EDAC/amd64: Add support for AMD family 1Ah-based newer models
[PATCH v3 4/5] EDAC/mc_sysfs: Increase legacy channel support to 16
[PATCH v3 5/5] EDAC/mc_sysfs: Begin deprecating legacy sysfs EDAC interface