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

Naik, Avadhut
2025-10-22T06:07:25Z

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