RE: [PATCH] EDAC, amd64: Add Family 17h Model 11h support.

["Ghannam, Yazen" <Yazen.Ghannam@amd.com>]

> -----Original Message-----
> From: Michael Jin <mikhail.jin@gmail.com>
> Sent: Friday, August 10, 2018 2:36 PM
> To: Borislav Petkov <bp@suse.de>; Ghannam, Yazen
> <Yazen.Ghannam@amd.com>; Mauro Carvalho Chehab
> <mchehab@kernel.org>
> Cc: linux-edac@vger.kernel.org; linux-kernel@vger.kernel.org; Michael Jin
> <mikhail.jin@gmail.com>
> Subject: [PATCH] EDAC, amd64: Add Family 17h Model 11h support.
> 
> Add support for ECC error decoding for F17h M11h (Great Horned Owl)
> processors.
> 
> Signed-off-by: Michael Jin <mikhail.jin@gmail.com>
> ---
>  drivers/edac/amd64_edac.c | 14 ++++++++++++++
>  drivers/edac/amd64_edac.h |  3 +++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 18aeabb1d5ee..de077c1f5ae3 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -2200,6 +2200,15 @@ static struct amd64_family_type family_types[] = {
>  			.dbam_to_cs		= f17_base_addr_to_cs_size,
>  		}
>  	},
> +	[F17_M11H_CPUS] = {
> +		.ctl_name = "F17h_M11h",
> +		.f0_id = PCI_DEVICE_ID_AMD_17H_M11H_DF_F0,
> +		.f6_id = PCI_DEVICE_ID_AMD_17H_M11H_DF_F6,
> +		.ops = {
> +			.early_channel_count	= f17_early_channel_count,
> +			.dbam_to_cs		= f17_base_addr_to_cs_size,
> +		}
> +	},
>  };
> 
>  /*
> @@ -3188,6 +3197,11 @@ static struct amd64_family_type
> *per_family_init(struct amd64_pvt *pvt)
>  		break;
> 
>  	case 0x17:
> +		if (pvt->model == 0x11) {

I think we can expand this to include more models. Please see comment
below.

There may be some differences between models, but things should generally
work. It's just a matter of whether or not the Platform enables certain things
like DRAM ECC, etc.

Does the amd64_edac_mod module load on your platform with just this patch?

> +			fam_type = &family_types[F17_M11H_CPUS];
> +			pvt->ops = &family_types[F17_M11H_CPUS].ops;
> +			break;
> +		}
>  		fam_type	= &family_types[F17_CPUS];
>  		pvt->ops	= &family_types[F17_CPUS].ops;
>  		break;
> diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
> index 1d4b74e9a037..e50226cd53c6 100644
> --- a/drivers/edac/amd64_edac.h
> +++ b/drivers/edac/amd64_edac.h
> @@ -115,6 +115,8 @@
>  #define PCI_DEVICE_ID_AMD_16H_M30H_NB_F2 0x1582
>  #define PCI_DEVICE_ID_AMD_17H_DF_F0	0x1460
>  #define PCI_DEVICE_ID_AMD_17H_DF_F6	0x1466
> +#define PCI_DEVICE_ID_AMD_17H_M11H_DF_F0 0x15e8
> +#define PCI_DEVICE_ID_AMD_17H_M11H_DF_F6 0x15ee
> 

These IDs are used for Fam17h Models 10h-2Fh. Can you please change
the names here and in the rest of this patch?

The format is to use the first supported model in the name, e.g. M11H -> M10H.

>  /*
>   * Function 1 - Address Map
> @@ -281,6 +283,7 @@ enum amd_families {
>  	F16_CPUS,
>  	F16_M30H_CPUS,
>  	F17_CPUS,
> +	F17_M11H_CPUS,
>  	NUM_FAMILIES,
>  };
> 

Thanks,
Yazen