RE: [PATCH v3 7/8] EDAC/amd64: Support Asymmetric Dual-Rank DIMMs

Ghannam, Yazen
2019-08-23T13:28:33Z

> -----Original Message-----
> From: linux-edac-owner@vger.kernel.org <linux-edac-owner@vger.kernel.org> On Behalf Of Borislav Petkov
> Sent: Friday, August 23, 2019 6:26 AM
> To: Ghannam, Yazen <Yazen.Ghannam@amd.com>
> Cc: linux-edac@vger.kernel.org; linux-kernel@vger.kernel.org
> 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 <yazen.ghannam@amd.com>
> >
> > 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 <yazen.ghannam@amd.com>
> > ---
> > Link:
> > https://lkml.kernel.org/r/20190709215643.171078-8-Yazen.Ghannam@amd.com
> >
> > 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