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

[Borislav Petkov <bp@alien8.de>]

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? :)

> +#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().

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.