> -----Original Message----- > From: [email protected] <[email protected]> On > Behalf Of Borislav Petkov > Sent: Friday, August 23, 2019 6:26 AM > To: Ghannam, Yazen <[email protected]> > Cc: [email protected]; [email protected] > Subject: Re: [PATCH v3 7/8] EDAC/amd64: Support Asymmetric Dual-Rank DIMMs > > On Thu, Aug 22, 2019 at 12:00:02AM +0000, Ghannam, Yazen wrote: > > From: Yazen Ghannam <[email protected]> > > > > Future AMD systems will support "Asymmetric" Dual-Rank DIMMs. These are > > DIMMs where the ranks are of different sizes. > > > > The even rank will use the Primary Even Chip Select registers and the > > odd rank will use the Secondary Odd Chip Select registers. > > > > Recognize if a Secondary Odd Chip Select is being used. Use the > > Secondary Odd Address Mask when calculating the chip select size. > > > > Signed-off-by: Yazen Ghannam <[email protected]> > > --- > > Link: > > https://lkml.kernel.org/r/[email protected] > > > > v2->v3: > > * Add check of csrow_nr before using secondary mask. > > > > v1->v2: > > * No change. > > > > drivers/edac/amd64_edac.c | 18 +++++++++++++++--- > > 1 file changed, 15 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c > > index 26ce48fcaf00..4d1e6daa7ec4 100644 > > --- a/drivers/edac/amd64_edac.c > > +++ b/drivers/edac/amd64_edac.c > > @@ -790,9 +790,13 @@ static void debug_dump_dramcfg_low(struct amd64_pvt > > *pvt, u32 dclr, int chan) > > > > #define CS_EVEN_PRIMARY BIT(0) > > #define CS_ODD_PRIMARY BIT(1) > > +#define CS_EVEN_SECONDARY BIT(2) > > +#define CS_ODD_SECONDARY BIT(3) > > > > -#define CS_EVEN CS_EVEN_PRIMARY > > -#define CS_ODD CS_ODD_PRIMARY > > +#define CS_EVEN (CS_EVEN_PRIMARY | CS_EVEN_SECONDARY) > > +#define CS_ODD (CS_ODD_PRIMARY | CS_EVEN_SECONDARY) > > That's just my urge to have stuff ballanced but shouldn't that last line be: > > #define CS_ODD (CS_ODD_PRIMARY | CS_ODD_SECONDARY) > > i.e., not have "even" as in CS_EVEN_SECONDARY in there but only "odd"s? :) >
Yes, sorry I missed that. > > +#define csrow_sec_enabled(i, dct, pvt) > > ((pvt)->csels[(dct)].csbases_sec[(i)] & DCSB_CS_ENABLE) > > I moved that to the header, under csrow_enabled(). > Okay, thank you. -Yazen
