diff mbox series

[12/18] EDAC/amd64: Add determine_edac_cap() into pvt->ops

Message ID 20220509145534.44912-13-yazen.ghannam@amd.com (mailing list archive)
State New, archived
Headers show
Series AMD64 EDAC Cleanup and Refactor | expand

Commit Message

Yazen Ghannam May 9, 2022, 2:55 p.m. UTC 
From: Muralidhara M K <muralidhara.mk@amd.com>

GPU Nodes will have different criteria for checking the EDAC
capabilities of a controller. A function pointer should be used rather
than introduce another branching condition.

Prepare for this by adding determine_edac_cap() to pvt->ops and set it
as needed based on currently supported systems.

Use a "umc" prefix for modern systems, since these use Unified Memory
Controllers (UMCs).

Use a "dct" prefix for newly-defined legacy functions, since these
systems use DRAM Controllers (DCTs).

Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
Signed-off-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
[Rebased/reworked patch and reworded commit message]
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
 drivers/edac/amd64_edac.c | 30 ++++++++++++++++++------------
 drivers/edac/amd64_edac.h |  1 +
 2 files changed, 19 insertions(+), 12 deletions(-)

Comments

Borislav Petkov June 20, 2022, 4:21 p.m. UTC | #1 
Back to those...

On Mon, May 09, 2022 at 02:55:28PM +0000, Yazen Ghannam wrote:
> From: Muralidhara M K <muralidhara.mk@amd.com>
> 
> GPU Nodes will have different criteria for checking the EDAC
> capabilities of a controller. A function pointer should be used rather
> than introduce another branching condition.
> 
> Prepare for this by adding determine_edac_cap() to pvt->ops and set it
> as needed based on currently supported systems.
> 
> Use a "umc" prefix for modern systems, since these use Unified Memory
> Controllers (UMCs).
> 
> Use a "dct" prefix for newly-defined legacy functions, since these
> systems use DRAM Controllers (DCTs).

Please refrain from adding those boilerplates to each commit message. Do
it once for the first patch and then no need anymore. It is clear what's
going on.

Thx.
Yazen Ghannam June 22, 2022, 4:10 p.m. UTC | #2 
On Mon, Jun 20, 2022 at 06:21:11PM +0200, Borislav Petkov wrote:
> Back to those...
> 
> On Mon, May 09, 2022 at 02:55:28PM +0000, Yazen Ghannam wrote:
> > From: Muralidhara M K <muralidhara.mk@amd.com>
> > 
> > GPU Nodes will have different criteria for checking the EDAC
> > capabilities of a controller. A function pointer should be used rather
> > than introduce another branching condition.
> > 
> > Prepare for this by adding determine_edac_cap() to pvt->ops and set it
> > as needed based on currently supported systems.
> > 
> > Use a "umc" prefix for modern systems, since these use Unified Memory
> > Controllers (UMCs).
> > 
> > Use a "dct" prefix for newly-defined legacy functions, since these
> > systems use DRAM Controllers (DCTs).
> 
> Please refrain from adding those boilerplates to each commit message. Do
> it once for the first patch and then no need anymore. It is clear what's
> going on.
>

Understood, will do.

Thanks,
Yazen
diff mbox series

Patch

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 136f2454a502..0bc9a3846773 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1261,13 +1261,25 @@  static int get_channel_from_ecc_syndrome(struct mem_ctl_info *, u16);
  * Determine if the DIMMs have ECC enabled. ECC is enabled ONLY if all the DIMMs
  * are ECC capable.
  */
-static unsigned long determine_edac_cap(struct amd64_pvt *pvt)
+static unsigned long dct_determine_edac_cap(struct amd64_pvt *pvt)
 {
 	unsigned long edac_cap = EDAC_FLAG_NONE;
 	u8 bit;
 
-	if (pvt->umc) {
-		u8 i, umc_en_mask = 0, dimm_ecc_en_mask = 0;
+	bit = (pvt->fam > 0xf || pvt->ext_model >= K8_REV_F)
+		? 19
+		: 17;
+
+	if (pvt->dclr0 & BIT(bit))
+		edac_cap = EDAC_FLAG_SECDED;
+
+	return edac_cap;
+}
+
+static unsigned long umc_determine_edac_cap(struct amd64_pvt *pvt)
+{
+	u8 i, umc_en_mask = 0, dimm_ecc_en_mask = 0;
+	unsigned long edac_cap = EDAC_FLAG_NONE;
 
 		for_each_umc(i) {
 			if (!(pvt->umc[i].sdp_ctrl & UMC_SDP_INIT))
@@ -1282,14 +1294,6 @@  static unsigned long determine_edac_cap(struct amd64_pvt *pvt)
 
 		if (umc_en_mask == dimm_ecc_en_mask)
 			edac_cap = EDAC_FLAG_SECDED;
-	} else {
-		bit = (pvt->fam > 0xf || pvt->ext_model >= K8_REV_F)
-			? 19
-			: 17;
-
-		if (pvt->dclr0 & BIT(bit))
-			edac_cap = EDAC_FLAG_SECDED;
-	}
 
 	return edac_cap;
 }
@@ -3740,7 +3744,7 @@  static void setup_mci_misc_attrs(struct mem_ctl_info *mci)
 			mci->edac_ctl_cap |= EDAC_FLAG_S4ECD4ED;
 	}
 
-	mci->edac_cap		= determine_edac_cap(pvt);
+	mci->edac_cap		= pvt->ops->determine_edac_cap(pvt);
 	mci->mod_name		= EDAC_MOD_STR;
 	mci->ctl_name		= pvt->ctl_name;
 	mci->dev_name		= pci_name(pvt->F3);
@@ -3760,6 +3764,7 @@  static struct low_ops umc_ops = {
 	.determine_ecc_sym_sz		= umc_determine_ecc_sym_sz,
 	.read_mc_regs			= umc_read_mc_regs,
 	.ecc_enabled			= umc_ecc_enabled,
+	.determine_edac_cap		= umc_determine_edac_cap,
 };
 
 /* Use Family 16h versions for defaults and adjust as needed below. */
@@ -3773,6 +3778,7 @@  static struct low_ops dct_ops = {
 	.determine_ecc_sym_sz		= dct_determine_ecc_sym_sz,
 	.read_mc_regs			= dct_read_mc_regs,
 	.ecc_enabled			= dct_ecc_enabled,
+	.determine_edac_cap		= dct_determine_edac_cap,
 };
 
 static int per_family_init(struct amd64_pvt *pvt)
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 99b6ffa21ba5..bfe48492a0ba 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -474,6 +474,7 @@  struct low_ops {
 	void (*determine_ecc_sym_sz)(struct amd64_pvt *pvt);
 	void (*read_mc_regs)(struct amd64_pvt *pvt);
 	bool (*ecc_enabled)(struct amd64_pvt *pvt);
+	unsigned long (*determine_edac_cap)(struct amd64_pvt *pvt);
 };
 
 int __amd64_read_pci_cfg_dword(struct pci_dev *pdev, int offset,