Messages in this thread | | | Date | Mon, 31 Jan 2022 22:43:37 +0000 | From | Yazen Ghannam <> | Subject | Re: [PATCH v3 2/2] EDAC/amd64: Add new register offset support and related changes |
| |
On Fri, Jan 21, 2022 at 01:25:21PM +0100, Borislav Petkov wrote: > On Tue, Dec 28, 2021 at 08:06:15PM +0000, Yazen Ghannam wrote: > > diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c > > index 4db92c77276f..a299c361a904 100644 > > --- a/drivers/edac/amd64_edac.c > > +++ b/drivers/edac/amd64_edac.c > > @@ -15,6 +15,31 @@ static struct msr __percpu *msrs; > > > > static struct amd64_family_type *fam_type; > > > > +/* Family flag helpers */ > > +static inline u64 get_addr_cfg(void) > > +{ > > + if (fam_type->flags.zn_regs_v2) > > + return UMCCH_ADDR_CFG_DDR5; > > + > > + return UMCCH_ADDR_CFG; > > +} > > + > > +static inline u64 get_addr_mask_sec(void) > > +{ > > + if (fam_type->flags.zn_regs_v2) > > + return UMCCH_ADDR_MASK_SEC_DDR5; > > + > > + return UMCCH_ADDR_MASK_SEC; > > +} > > + > > +static inline u64 get_dimm_cfg(void) > > +{ > > + if (fam_type->flags.zn_regs_v2) > > + return UMCCH_DIMM_CFG_DDR5; > > + > > + return UMCCH_DIMM_CFG; > > +} > > Yeah, you can do it either this way and have a lot of small functions > or you can do what I did with mca_msr_reg() which is a single mapping > function you then use everywhere. > > Your call. >
Yes, I'll do this.
> > + > > /* Per-node stuff */ > > static struct ecc_settings **ecc_stngs; > > > > @@ -1429,8 +1454,10 @@ static void __dump_misc_regs_df(struct amd64_pvt *pvt) > > edac_dbg(1, "UMC%d x16 DIMMs present: %s\n", > > i, (umc->dimm_cfg & BIT(7)) ? "yes" : "no"); > > > > - if (pvt->dram_type == MEM_LRDDR4) { > > - amd_smn_read(pvt->mc_node_id, umc_base + UMCCH_ADDR_CFG, &tmp); > > + if (pvt->dram_type == MEM_LRDDR4 || pvt->dram_type == MEM_LRDDR5) { > > This still keeps the ->dram_type per pvt, which is per memory controller > in amd64_edac nomenclature. > > But AFAIR, we said last time that the DRAM type is per UMC now, as you > do in the previous patch. > > Which means, you either have to test umc->dimm_cfg to get the DRAM type > here or push ->dram_type into the umc struct... >
Yep, you're right. I'll cache the dram_type in the umc struct.
Thanks, Yazen
| |