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
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
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
© 2016 - 2025 Red Hat, Inc.