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

["Naik, Avadhut" <avadnaik@amd.com>]

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