diff mbox series

[10/18] EDAC/amd64: Add read_mc_regs() into pvt->ops

Message ID 20220509145534.44912-11-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 read a different set of memory controller values
compared than existing systems. A function pointer should be used
rather than introduce another branching condition.

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

Reading the TOM/TOM2 registers is preserved in the legacy function and
removed for the modern function. The values don't seem useful for the
EDAC module on modern systems. But they are kept for legacy systems in
case there was a need for them.

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 | 31 +++++++++++++------------------
 drivers/edac/amd64_edac.h |  1 +
 2 files changed, 14 insertions(+), 18 deletions(-)

Comments

Borislav Petkov May 18, 2022, 11:02 a.m. UTC | #1 
On Mon, May 09, 2022 at 02:55:26PM +0000, Yazen Ghannam wrote:
> @@ -3766,6 +3751,7 @@ static struct low_ops umc_ops = {
>  	.read_base_mask			= umc_read_base_mask,
>  	.determine_memory_type		= umc_determine_memory_type,
>  	.determine_ecc_sym_sz		= umc_determine_ecc_sym_sz,
> +	.read_mc_regs			= umc_read_mc_regs,
>  };
>  
>  /* Use Family 16h versions for defaults and adjust as needed below. */
> @@ -3777,6 +3763,7 @@ static struct low_ops dct_ops = {
>  	.read_base_mask			= dct_read_base_mask,
>  	.determine_memory_type		= dct_determine_memory_type,
>  	.determine_ecc_sym_sz		= dct_determine_ecc_sym_sz,
> +	.read_mc_regs			= dct_read_mc_regs,

Aha, good, here you do that split I mentioned earlier.

>  static int per_family_init(struct amd64_pvt *pvt)
> @@ -3938,7 +3925,15 @@ static int hw_info_get(struct amd64_pvt *pvt)
>  	if (ret)
>  		return ret;
>  
> -	read_mc_regs(pvt);
> +	pvt->ops->read_mc_regs(pvt);
> +
> +	pvt->ops->prep_chip_selects(pvt);
> +
> +	pvt->ops->read_base_mask(pvt);
> +
> +	pvt->ops->determine_memory_type(pvt);
> +
> +	pvt->ops->determine_ecc_sym_sz(pvt);
>  

Yeah, no need for the spaces.

Thx.
diff mbox series

Patch

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 2a3205f1205e..1bf1660fe8f3 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -3210,7 +3210,7 @@  static void umc_determine_ecc_sym_sz(struct amd64_pvt *pvt)
 /*
  * Retrieve the hardware registers of the memory controller.
  */
-static void __read_mc_regs_df(struct amd64_pvt *pvt)
+static void umc_read_mc_regs(struct amd64_pvt *pvt)
 {
 	u8 nid = pvt->mc_node_id;
 	struct amd64_umc *umc;
@@ -3234,7 +3234,7 @@  static void __read_mc_regs_df(struct amd64_pvt *pvt)
  * Retrieve the hardware registers of the memory controller (this includes the
  * 'Address Map' and 'Misc' device regs)
  */
-static void read_mc_regs(struct amd64_pvt *pvt)
+static void dct_read_mc_regs(struct amd64_pvt *pvt)
 {
 	unsigned int range;
 	u64 msr_val;
@@ -3255,12 +3255,6 @@  static void read_mc_regs(struct amd64_pvt *pvt)
 		edac_dbg(0, "  TOP_MEM2 disabled\n");
 	}
 
-	if (pvt->umc) {
-		__read_mc_regs_df(pvt);
-
-		goto skip;
-	}
-
 	amd64_read_pci_cfg(pvt->F3, NBCAP, &pvt->nbcap);
 
 	read_dram_ctl_register(pvt);
@@ -3300,15 +3294,6 @@  static void read_mc_regs(struct amd64_pvt *pvt)
 		amd64_read_dct_pci_cfg(pvt, 1, DCLR0, &pvt->dclr1);
 		amd64_read_dct_pci_cfg(pvt, 1, DCHR0, &pvt->dchr1);
 	}
-
-skip:
-	pvt->ops->prep_chip_selects(pvt);
-
-	pvt->ops->read_base_mask(pvt);
-
-	pvt->ops->determine_memory_type(pvt);
-
-	pvt->ops->determine_ecc_sym_sz(pvt);
 }
 
 /*
@@ -3766,6 +3751,7 @@  static struct low_ops umc_ops = {
 	.read_base_mask			= umc_read_base_mask,
 	.determine_memory_type		= umc_determine_memory_type,
 	.determine_ecc_sym_sz		= umc_determine_ecc_sym_sz,
+	.read_mc_regs			= umc_read_mc_regs,
 };
 
 /* Use Family 16h versions for defaults and adjust as needed below. */
@@ -3777,6 +3763,7 @@  static struct low_ops dct_ops = {
 	.read_base_mask			= dct_read_base_mask,
 	.determine_memory_type		= dct_determine_memory_type,
 	.determine_ecc_sym_sz		= dct_determine_ecc_sym_sz,
+	.read_mc_regs			= dct_read_mc_regs,
 };
 
 static int per_family_init(struct amd64_pvt *pvt)
@@ -3938,7 +3925,15 @@  static int hw_info_get(struct amd64_pvt *pvt)
 	if (ret)
 		return ret;
 
-	read_mc_regs(pvt);
+	pvt->ops->read_mc_regs(pvt);
+
+	pvt->ops->prep_chip_selects(pvt);
+
+	pvt->ops->read_base_mask(pvt);
+
+	pvt->ops->determine_memory_type(pvt);
+
+	pvt->ops->determine_ecc_sym_sz(pvt);
 
 	return 0;
 }
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 5e9ff6ea7ebc..25d0dcc5c480 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -472,6 +472,7 @@  struct low_ops {
 	void (*read_base_mask)(struct amd64_pvt *pvt);
 	void (*determine_memory_type)(struct amd64_pvt *pvt);
 	void (*determine_ecc_sym_sz)(struct amd64_pvt *pvt);
+	void (*read_mc_regs)(struct amd64_pvt *pvt);
 };
 
 int __amd64_read_pci_cfg_dword(struct pci_dev *pdev, int offset,