| Message ID | 20220228161354.54923-7-nchatrad@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | EDAC/amd64: move platform specific routines to pvt->ops | expand |
On Mon, Feb 28, 2022 at 09:43:46PM +0530, Naveen Krishna Chatradhi wrote: > From: Muralidhara M K <muralimk@amd.com> > > Add function pointer for get_mc_regs() in pvt->ops and assign > family specific get_mc_regs() definitions appropriately. > Please include the "why". > Signed-off-by: Muralidhara M K <muralimk@amd.com> > Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com> > --- > This patch is created by splitting the 5/12th patch in series > [v7 5/12] https://patchwork.kernel.org/project/linux-edac/patch/20220203174942.31630-6-nchatrad@amd.com/ > > drivers/edac/amd64_edac.c | 77 +++++++++++++++++++++------------------ > drivers/edac/amd64_edac.h | 1 + > 2 files changed, 43 insertions(+), 35 deletions(-) > > diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c > index 69c33eb17e4f..713ffe763e64 100644 > --- a/drivers/edac/amd64_edac.c > +++ b/drivers/edac/amd64_edac.c > @@ -3214,6 +3214,27 @@ static void f17_determine_ecc_sym_sz(struct amd64_pvt *pvt) > } > } > > +static void read_top_mem_registers(struct amd64_pvt *pvt) > +{ > + u64 msr_val; > + > + /* > + * Retrieve TOP_MEM and TOP_MEM2; no masking off of reserved bits since > + * those are Read-As-Zero. > + */ > + rdmsrl(MSR_K8_TOP_MEM1, pvt->top_mem); > + edac_dbg(0, " TOP_MEM: 0x%016llx\n", pvt->top_mem); > + > + /* Check first whether TOP_MEM2 is enabled: */ > + rdmsrl(MSR_AMD64_SYSCFG, msr_val); > + if (msr_val & BIT(21)) { > + rdmsrl(MSR_K8_TOP_MEM2, pvt->top_mem2); > + edac_dbg(0, " TOP_MEM2: 0x%016llx\n", pvt->top_mem2); > + } else { > + edac_dbg(0, " TOP_MEM2 disabled\n"); > + } These two values are not used by any code within this module. They are only used in debug print statements and debug sysfs entries. I think this code should just be removed. An expert user who wants to know TOM and TOM2 can use another method, like msr-tools, rather than recompile a kernel with CONFIG_EDAC_DEBUG, etc. > +} > + > /* > * Retrieve the hardware registers of the memory controller. > */ > @@ -3235,6 +3256,8 @@ static void __read_mc_regs_df(struct amd64_pvt *pvt) > amd_smn_read(nid, umc_base + UMCCH_ECC_CTRL, &umc->ecc_ctrl); > amd_smn_read(nid, umc_base + UMCCH_UMC_CAP_HI, &umc->umc_cap_hi); > } > + > + amd64_read_pci_cfg(pvt->F0, DF_DHAR, &pvt->dhar); "dhar" is not used by any code for Zen-based systems. I think this line can be dropped. Reading "dhar" should still be preserved for legacy systems. This is also the only use of PCI F0. So all the F0 IDs can be removed too. I have a patch for this as part of some general code clean up. Let's include that with this set also. I think removing TOM/TOM2 code can be included too. > } > > /* > @@ -3244,30 +3267,8 @@ static void __read_mc_regs_df(struct amd64_pvt *pvt) > static void read_mc_regs(struct amd64_pvt *pvt) > { > unsigned int range; > - u64 msr_val; > > - /* > - * Retrieve TOP_MEM and TOP_MEM2; no masking off of reserved bits since > - * those are Read-As-Zero. > - */ > - rdmsrl(MSR_K8_TOP_MEM1, pvt->top_mem); > - edac_dbg(0, " TOP_MEM: 0x%016llx\n", pvt->top_mem); > - > - /* Check first whether TOP_MEM2 is enabled: */ > - rdmsrl(MSR_AMD64_SYSCFG, msr_val); > - if (msr_val & BIT(21)) { > - rdmsrl(MSR_K8_TOP_MEM2, pvt->top_mem2); > - edac_dbg(0, " TOP_MEM2: 0x%016llx\n", pvt->top_mem2); > - } else { > - edac_dbg(0, " TOP_MEM2 disabled\n"); > - } > - > - if (pvt->umc) { > - __read_mc_regs_df(pvt); > - amd64_read_pci_cfg(pvt->F0, DF_DHAR, &pvt->dhar); > - > - goto skip; > - } > + read_top_mem_registers(pvt); > > amd64_read_pci_cfg(pvt->F3, NBCAP, &pvt->nbcap); > > @@ -3308,16 +3309,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->get_base_mask(pvt); > - > - pvt->ops->determine_memory_type(pvt); > - edac_dbg(1, " DIMM type: %s\n", edac_mem_types[pvt->dram_type]); > - > - pvt->ops->determine_ecc_sym_sz(pvt); > } > > /* > @@ -3792,6 +3783,7 @@ static int per_family_init(struct amd64_pvt *pvt) > pvt->ops->prep_chip_selects = k8_prep_chip_selects; > pvt->ops->determine_memory_type = f1x_determine_memory_type; > pvt->ops->determine_ecc_sym_sz = f1x_determine_ecc_sym_sz; > + pvt->ops->get_mc_regs = read_mc_regs; The function names should be more consistent: either both get or read. The read_mc_regs() function is used for systems with DCTs (i.e. legacy). This can be included in the name. > break; > > case 0x10: > @@ -3805,6 +3797,7 @@ static int per_family_init(struct amd64_pvt *pvt) > pvt->ops->prep_chip_selects = default_prep_chip_selects; > pvt->ops->determine_memory_type = f1x_determine_memory_type; > pvt->ops->determine_ecc_sym_sz = f1x_determine_ecc_sym_sz; > + pvt->ops->get_mc_regs = read_mc_regs; > break; > > case 0x15: > @@ -3834,6 +3827,7 @@ static int per_family_init(struct amd64_pvt *pvt) > pvt->ops->get_base_mask = read_dct_base_mask; > pvt->ops->determine_memory_type = f1x_determine_memory_type; > pvt->ops->determine_ecc_sym_sz = f1x_determine_ecc_sym_sz; > + pvt->ops->get_mc_regs = read_mc_regs; > break; > > case 0x16: > @@ -3853,6 +3847,7 @@ static int per_family_init(struct amd64_pvt *pvt) > pvt->ops->prep_chip_selects = default_prep_chip_selects; > pvt->ops->determine_memory_type = f1x_determine_memory_type; > pvt->ops->determine_ecc_sym_sz = f1x_determine_ecc_sym_sz; > + pvt->ops->get_mc_regs = read_mc_regs; > break; > > case 0x17: > @@ -3886,6 +3881,7 @@ static int per_family_init(struct amd64_pvt *pvt) > pvt->ops->prep_chip_selects = f17_prep_chip_selects; > pvt->ops->determine_memory_type = f17_determine_memory_type; > pvt->ops->determine_ecc_sym_sz = f17_determine_ecc_sym_sz; > + pvt->ops->get_mc_regs = __read_mc_regs_df; The underscore prefix can be removed, since this is no longer a helper function. Also, the "df" suffix can be removed when changing the name. Maybe something like this: pvt->ops->read_mc_regs() <--- This reads memory controller registers. read_dct_regs() <--- Used for DRAM Controllers (DCTs). read_umc_regs() <--- Used for Unified Memory Controllers (UMCs). > > if (pvt->fam == 0x18) { > pvt->ctl_name = "F18h"; > @@ -3925,6 +3921,7 @@ static int per_family_init(struct amd64_pvt *pvt) > pvt->ops->prep_chip_selects = f17_prep_chip_selects; > pvt->ops->determine_memory_type = f17_determine_memory_type; > pvt->ops->determine_ecc_sym_sz = f17_determine_ecc_sym_sz; > + pvt->ops->get_mc_regs = __read_mc_regs_df; > break; > > default: > @@ -3935,7 +3932,8 @@ static int per_family_init(struct amd64_pvt *pvt) > /* ops required for all the families */ > if (!pvt->ops->early_channel_count || !pvt->ops->dbam_to_cs || > !pvt->ops->get_base_mask || !pvt->ops->prep_chip_selects || > - !pvt->ops->determine_memory_type || !pvt->ops->determine_ecc_sym_sz) { > + !pvt->ops->determine_memory_type || !pvt->ops->determine_ecc_sym_sz || > + !pvt->ops->get_mc_regs) { > edac_dbg(1, "Common helper routines not defined.\n"); > return -EFAULT; > } > @@ -3972,7 +3970,16 @@ static int hw_info_get(struct amd64_pvt *pvt) > if (ret) > return ret; > > - read_mc_regs(pvt); > + pvt->ops->get_mc_regs(pvt); > + > + pvt->ops->prep_chip_selects(pvt); > + > + pvt->ops->get_base_mask(pvt); > + > + pvt->ops->determine_memory_type(pvt); > + edac_dbg(1, " DIMM type: %s\n", edac_mem_types[pvt->dram_type]); This line should be included in determine_memory_type(). It should be called for each PVT on legacy systems and for each UMC on current systems. Thanks, Yazen
Hi Yazen, On 3/28/2022 9:38 PM, Yazen Ghannam wrote: > On Mon, Feb 28, 2022 at 09:43:46PM +0530, Naveen Krishna Chatradhi wrote: >> From: Muralidhara M K <muralimk@amd.com> >> >> Add function pointer for get_mc_regs() in pvt->ops and assign >> family specific get_mc_regs() definitions appropriately. >> > Please include the "why". Sure > >> Signed-off-by: Muralidhara M K <muralimk@amd.com> >> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com> >> --- >> This patch is created by splitting the 5/12th patch in series >> [v7 5/12] https://patchwork.kernel.org/project/linux-edac/patch/20220203174942.31630-6-nchatrad@amd.com/ >> >> drivers/edac/amd64_edac.c | 77 +++++++++++++++++++++------------------ >> drivers/edac/amd64_edac.h | 1 + >> 2 files changed, 43 insertions(+), 35 deletions(-) >> >> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c >> index 69c33eb17e4f..713ffe763e64 100644 >> --- a/drivers/edac/amd64_edac.c >> +++ b/drivers/edac/amd64_edac.c >> @@ -3214,6 +3214,27 @@ static void f17_determine_ecc_sym_sz(struct amd64_pvt *pvt) >> } >> } >> >> +static void read_top_mem_registers(struct amd64_pvt *pvt) >> +{ >> + u64 msr_val; >> + >> + /* >> + * Retrieve TOP_MEM and TOP_MEM2; no masking off of reserved bits since >> + * those are Read-As-Zero. >> + */ >> + rdmsrl(MSR_K8_TOP_MEM1, pvt->top_mem); >> + edac_dbg(0, " TOP_MEM: 0x%016llx\n", pvt->top_mem); >> + >> + /* Check first whether TOP_MEM2 is enabled: */ >> + rdmsrl(MSR_AMD64_SYSCFG, msr_val); >> + if (msr_val & BIT(21)) { >> + rdmsrl(MSR_K8_TOP_MEM2, pvt->top_mem2); >> + edac_dbg(0, " TOP_MEM2: 0x%016llx\n", pvt->top_mem2); >> + } else { >> + edac_dbg(0, " TOP_MEM2 disabled\n"); >> + } > These two values are not used by any code within this module. They are only > used in debug print statements and debug sysfs entries. I think this code > should just be removed. An expert user who wants to know TOM and TOM2 can use > another method, like msr-tools, rather than recompile a kernel with > CONFIG_EDAC_DEBUG, etc. Make sense, do you think some users have developed scripts to parse the EDAC debug logs ? > >> +} >> + >> /* >> * Retrieve the hardware registers of the memory controller. >> */ >> @@ -3235,6 +3256,8 @@ static void __read_mc_regs_df(struct amd64_pvt *pvt) >> amd_smn_read(nid, umc_base + UMCCH_ECC_CTRL, &umc->ecc_ctrl); >> amd_smn_read(nid, umc_base + UMCCH_UMC_CAP_HI, &umc->umc_cap_hi); >> } >> + >> + amd64_read_pci_cfg(pvt->F0, DF_DHAR, &pvt->dhar); > "dhar" is not used by any code for Zen-based systems. I think this line can be > dropped. Reading "dhar" should still be preserved for legacy systems. > > This is also the only use of PCI F0. So all the F0 IDs can be removed too. I > have a patch for this as part of some general code clean up. Let's include > that with this set also. I think removing TOM/TOM2 code can be included too. Sure, we can work on this. > >> } >> >> /* >> @@ -3244,30 +3267,8 @@ static void __read_mc_regs_df(struct amd64_pvt *pvt) >> static void read_mc_regs(struct amd64_pvt *pvt) >> { >> unsigned int range; >> - u64 msr_val; >> >> - /* >> - * Retrieve TOP_MEM and TOP_MEM2; no masking off of reserved bits since >> - * those are Read-As-Zero. >> - */ >> - rdmsrl(MSR_K8_TOP_MEM1, pvt->top_mem); >> - edac_dbg(0, " TOP_MEM: 0x%016llx\n", pvt->top_mem); >> - >> - /* Check first whether TOP_MEM2 is enabled: */ >> - rdmsrl(MSR_AMD64_SYSCFG, msr_val); >> - if (msr_val & BIT(21)) { >> - rdmsrl(MSR_K8_TOP_MEM2, pvt->top_mem2); >> - edac_dbg(0, " TOP_MEM2: 0x%016llx\n", pvt->top_mem2); >> - } else { >> - edac_dbg(0, " TOP_MEM2 disabled\n"); >> - } >> - >> - if (pvt->umc) { >> - __read_mc_regs_df(pvt); >> - amd64_read_pci_cfg(pvt->F0, DF_DHAR, &pvt->dhar); >> - >> - goto skip; >> - } >> + read_top_mem_registers(pvt); >> >> amd64_read_pci_cfg(pvt->F3, NBCAP, &pvt->nbcap); >> >> @@ -3308,16 +3309,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->get_base_mask(pvt); >> - >> - pvt->ops->determine_memory_type(pvt); >> - edac_dbg(1, " DIMM type: %s\n", edac_mem_types[pvt->dram_type]); >> - >> - pvt->ops->determine_ecc_sym_sz(pvt); >> } >> >> /* >> @@ -3792,6 +3783,7 @@ static int per_family_init(struct amd64_pvt *pvt) >> pvt->ops->prep_chip_selects = k8_prep_chip_selects; >> pvt->ops->determine_memory_type = f1x_determine_memory_type; >> pvt->ops->determine_ecc_sym_sz = f1x_determine_ecc_sym_sz; >> + pvt->ops->get_mc_regs = read_mc_regs; > The function names should be more consistent: either both get or read. > > The read_mc_regs() function is used for systems with DCTs (i.e. legacy). This > can be included in the name. sure > >> break; >> >> case 0x10: >> @@ -3805,6 +3797,7 @@ static int per_family_init(struct amd64_pvt *pvt) >> pvt->ops->prep_chip_selects = default_prep_chip_selects; >> pvt->ops->determine_memory_type = f1x_determine_memory_type; >> pvt->ops->determine_ecc_sym_sz = f1x_determine_ecc_sym_sz; >> + pvt->ops->get_mc_regs = read_mc_regs; >> break; >> >> case 0x15: >> @@ -3834,6 +3827,7 @@ static int per_family_init(struct amd64_pvt *pvt) >> pvt->ops->get_base_mask = read_dct_base_mask; >> pvt->ops->determine_memory_type = f1x_determine_memory_type; >> pvt->ops->determine_ecc_sym_sz = f1x_determine_ecc_sym_sz; >> + pvt->ops->get_mc_regs = read_mc_regs; >> break; >> >> case 0x16: >> @@ -3853,6 +3847,7 @@ static int per_family_init(struct amd64_pvt *pvt) >> pvt->ops->prep_chip_selects = default_prep_chip_selects; >> pvt->ops->determine_memory_type = f1x_determine_memory_type; >> pvt->ops->determine_ecc_sym_sz = f1x_determine_ecc_sym_sz; >> + pvt->ops->get_mc_regs = read_mc_regs; >> break; >> >> case 0x17: >> @@ -3886,6 +3881,7 @@ static int per_family_init(struct amd64_pvt *pvt) >> pvt->ops->prep_chip_selects = f17_prep_chip_selects; >> pvt->ops->determine_memory_type = f17_determine_memory_type; >> pvt->ops->determine_ecc_sym_sz = f17_determine_ecc_sym_sz; >> + pvt->ops->get_mc_regs = __read_mc_regs_df; > The underscore prefix can be removed, since this is no longer a helper > function. Also, the "df" suffix can be removed when changing the name. > > Maybe something like this: > > pvt->ops->read_mc_regs() <--- This reads memory controller registers. > > read_dct_regs() <--- Used for DRAM Controllers (DCTs). > > read_umc_regs() <--- Used for Unified Memory Controllers (UMCs). sure > >> >> if (pvt->fam == 0x18) { >> pvt->ctl_name = "F18h"; >> @@ -3925,6 +3921,7 @@ static int per_family_init(struct amd64_pvt *pvt) >> pvt->ops->prep_chip_selects = f17_prep_chip_selects; >> pvt->ops->determine_memory_type = f17_determine_memory_type; >> pvt->ops->determine_ecc_sym_sz = f17_determine_ecc_sym_sz; >> + pvt->ops->get_mc_regs = __read_mc_regs_df; >> break; >> >> default: >> @@ -3935,7 +3932,8 @@ static int per_family_init(struct amd64_pvt *pvt) >> /* ops required for all the families */ >> if (!pvt->ops->early_channel_count || !pvt->ops->dbam_to_cs || >> !pvt->ops->get_base_mask || !pvt->ops->prep_chip_selects || >> - !pvt->ops->determine_memory_type || !pvt->ops->determine_ecc_sym_sz) { >> + !pvt->ops->determine_memory_type || !pvt->ops->determine_ecc_sym_sz || >> + !pvt->ops->get_mc_regs) { >> edac_dbg(1, "Common helper routines not defined.\n"); >> return -EFAULT; >> } >> @@ -3972,7 +3970,16 @@ static int hw_info_get(struct amd64_pvt *pvt) >> if (ret) >> return ret; >> >> - read_mc_regs(pvt); >> + pvt->ops->get_mc_regs(pvt); >> + >> + pvt->ops->prep_chip_selects(pvt); >> + >> + pvt->ops->get_base_mask(pvt); >> + >> + pvt->ops->determine_memory_type(pvt); >> + edac_dbg(1, " DIMM type: %s\n", edac_mem_types[pvt->dram_type]); > This line should be included in determine_memory_type(). It should be called > for each PVT on legacy systems and for each UMC on current systems. sure Regards, Naveenk > > Thanks, > Yazen
On Thu, Mar 31, 2022 at 05:49:49PM +0530, Chatradhi, Naveen Krishna wrote: ... > > > +static void read_top_mem_registers(struct amd64_pvt *pvt) > > > +{ > > > + u64 msr_val; > > > + > > > + /* > > > + * Retrieve TOP_MEM and TOP_MEM2; no masking off of reserved bits since > > > + * those are Read-As-Zero. > > > + */ > > > + rdmsrl(MSR_K8_TOP_MEM1, pvt->top_mem); > > > + edac_dbg(0, " TOP_MEM: 0x%016llx\n", pvt->top_mem); > > > + > > > + /* Check first whether TOP_MEM2 is enabled: */ > > > + rdmsrl(MSR_AMD64_SYSCFG, msr_val); > > > + if (msr_val & BIT(21)) { > > > + rdmsrl(MSR_K8_TOP_MEM2, pvt->top_mem2); > > > + edac_dbg(0, " TOP_MEM2: 0x%016llx\n", pvt->top_mem2); > > > + } else { > > > + edac_dbg(0, " TOP_MEM2 disabled\n"); > > > + } > > These two values are not used by any code within this module. They are only > > used in debug print statements and debug sysfs entries. I think this code > > should just be removed. An expert user who wants to know TOM and TOM2 can use > > another method, like msr-tools, rather than recompile a kernel with > > CONFIG_EDAC_DEBUG, etc. > Make sense, do you think some users have developed scripts to parse the EDAC > debug logs ? I'm not aware of any users of this. But this isn't a robust interface like an ABI. Thanks, Yazen
On Mon, Apr 04, 2022 at 06:19:36PM +0000, Yazen Ghannam wrote: > I'm not aware of any users of this. But this isn't a robust interface like an > ABI. Yes, kernel printk message formatting is not an ABI.
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c index 69c33eb17e4f..713ffe763e64 100644 --- a/drivers/edac/amd64_edac.c +++ b/drivers/edac/amd64_edac.c @@ -3214,6 +3214,27 @@ static void f17_determine_ecc_sym_sz(struct amd64_pvt *pvt) } } +static void read_top_mem_registers(struct amd64_pvt *pvt) +{ + u64 msr_val; + + /* + * Retrieve TOP_MEM and TOP_MEM2; no masking off of reserved bits since + * those are Read-As-Zero. + */ + rdmsrl(MSR_K8_TOP_MEM1, pvt->top_mem); + edac_dbg(0, " TOP_MEM: 0x%016llx\n", pvt->top_mem); + + /* Check first whether TOP_MEM2 is enabled: */ + rdmsrl(MSR_AMD64_SYSCFG, msr_val); + if (msr_val & BIT(21)) { + rdmsrl(MSR_K8_TOP_MEM2, pvt->top_mem2); + edac_dbg(0, " TOP_MEM2: 0x%016llx\n", pvt->top_mem2); + } else { + edac_dbg(0, " TOP_MEM2 disabled\n"); + } +} + /* * Retrieve the hardware registers of the memory controller. */ @@ -3235,6 +3256,8 @@ static void __read_mc_regs_df(struct amd64_pvt *pvt) amd_smn_read(nid, umc_base + UMCCH_ECC_CTRL, &umc->ecc_ctrl); amd_smn_read(nid, umc_base + UMCCH_UMC_CAP_HI, &umc->umc_cap_hi); } + + amd64_read_pci_cfg(pvt->F0, DF_DHAR, &pvt->dhar); } /* @@ -3244,30 +3267,8 @@ static void __read_mc_regs_df(struct amd64_pvt *pvt) static void read_mc_regs(struct amd64_pvt *pvt) { unsigned int range; - u64 msr_val; - /* - * Retrieve TOP_MEM and TOP_MEM2; no masking off of reserved bits since - * those are Read-As-Zero. - */ - rdmsrl(MSR_K8_TOP_MEM1, pvt->top_mem); - edac_dbg(0, " TOP_MEM: 0x%016llx\n", pvt->top_mem); - - /* Check first whether TOP_MEM2 is enabled: */ - rdmsrl(MSR_AMD64_SYSCFG, msr_val); - if (msr_val & BIT(21)) { - rdmsrl(MSR_K8_TOP_MEM2, pvt->top_mem2); - edac_dbg(0, " TOP_MEM2: 0x%016llx\n", pvt->top_mem2); - } else { - edac_dbg(0, " TOP_MEM2 disabled\n"); - } - - if (pvt->umc) { - __read_mc_regs_df(pvt); - amd64_read_pci_cfg(pvt->F0, DF_DHAR, &pvt->dhar); - - goto skip; - } + read_top_mem_registers(pvt); amd64_read_pci_cfg(pvt->F3, NBCAP, &pvt->nbcap); @@ -3308,16 +3309,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->get_base_mask(pvt); - - pvt->ops->determine_memory_type(pvt); - edac_dbg(1, " DIMM type: %s\n", edac_mem_types[pvt->dram_type]); - - pvt->ops->determine_ecc_sym_sz(pvt); } /* @@ -3792,6 +3783,7 @@ static int per_family_init(struct amd64_pvt *pvt) pvt->ops->prep_chip_selects = k8_prep_chip_selects; pvt->ops->determine_memory_type = f1x_determine_memory_type; pvt->ops->determine_ecc_sym_sz = f1x_determine_ecc_sym_sz; + pvt->ops->get_mc_regs = read_mc_regs; break; case 0x10: @@ -3805,6 +3797,7 @@ static int per_family_init(struct amd64_pvt *pvt) pvt->ops->prep_chip_selects = default_prep_chip_selects; pvt->ops->determine_memory_type = f1x_determine_memory_type; pvt->ops->determine_ecc_sym_sz = f1x_determine_ecc_sym_sz; + pvt->ops->get_mc_regs = read_mc_regs; break; case 0x15: @@ -3834,6 +3827,7 @@ static int per_family_init(struct amd64_pvt *pvt) pvt->ops->get_base_mask = read_dct_base_mask; pvt->ops->determine_memory_type = f1x_determine_memory_type; pvt->ops->determine_ecc_sym_sz = f1x_determine_ecc_sym_sz; + pvt->ops->get_mc_regs = read_mc_regs; break; case 0x16: @@ -3853,6 +3847,7 @@ static int per_family_init(struct amd64_pvt *pvt) pvt->ops->prep_chip_selects = default_prep_chip_selects; pvt->ops->determine_memory_type = f1x_determine_memory_type; pvt->ops->determine_ecc_sym_sz = f1x_determine_ecc_sym_sz; + pvt->ops->get_mc_regs = read_mc_regs; break; case 0x17: @@ -3886,6 +3881,7 @@ static int per_family_init(struct amd64_pvt *pvt) pvt->ops->prep_chip_selects = f17_prep_chip_selects; pvt->ops->determine_memory_type = f17_determine_memory_type; pvt->ops->determine_ecc_sym_sz = f17_determine_ecc_sym_sz; + pvt->ops->get_mc_regs = __read_mc_regs_df; if (pvt->fam == 0x18) { pvt->ctl_name = "F18h"; @@ -3925,6 +3921,7 @@ static int per_family_init(struct amd64_pvt *pvt) pvt->ops->prep_chip_selects = f17_prep_chip_selects; pvt->ops->determine_memory_type = f17_determine_memory_type; pvt->ops->determine_ecc_sym_sz = f17_determine_ecc_sym_sz; + pvt->ops->get_mc_regs = __read_mc_regs_df; break; default: @@ -3935,7 +3932,8 @@ static int per_family_init(struct amd64_pvt *pvt) /* ops required for all the families */ if (!pvt->ops->early_channel_count || !pvt->ops->dbam_to_cs || !pvt->ops->get_base_mask || !pvt->ops->prep_chip_selects || - !pvt->ops->determine_memory_type || !pvt->ops->determine_ecc_sym_sz) { + !pvt->ops->determine_memory_type || !pvt->ops->determine_ecc_sym_sz || + !pvt->ops->get_mc_regs) { edac_dbg(1, "Common helper routines not defined.\n"); return -EFAULT; } @@ -3972,7 +3970,16 @@ static int hw_info_get(struct amd64_pvt *pvt) if (ret) return ret; - read_mc_regs(pvt); + pvt->ops->get_mc_regs(pvt); + + pvt->ops->prep_chip_selects(pvt); + + pvt->ops->get_base_mask(pvt); + + pvt->ops->determine_memory_type(pvt); + edac_dbg(1, " DIMM type: %s\n", edac_mem_types[pvt->dram_type]); + + pvt->ops->determine_ecc_sym_sz(pvt); return 0; } diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h index f6769148d8b7..1b6df33bb0a8 100644 --- a/drivers/edac/amd64_edac.h +++ b/drivers/edac/amd64_edac.h @@ -468,6 +468,7 @@ struct low_ops { void (*prep_chip_selects)(struct amd64_pvt *pvt); void (*determine_memory_type)(struct amd64_pvt *pvt); void (*determine_ecc_sym_sz)(struct amd64_pvt *pvt); + void (*get_mc_regs)(struct amd64_pvt *pvt); }; int __amd64_read_pci_cfg_dword(struct pci_dev *pdev, int offset,