diff mbox series

[2/2] EDAC, i10nm: Fix i10nm_edac loading failure on some servers

Message ID 20200424185738.7985-3-tony.luck@intel.com (mailing list archive)
State New, archived
Headers show
Series Fix i10nm_edac driver load failure | expand

Commit Message

Tony Luck April 24, 2020, 6:57 p.m. UTC
From: Qiuxu Zhuo <qiuxu.zhuo@intel.com>

It failed to load the i10nm_edac driver on Ice Lake and
Tremont/Jacobsville servers if their CPU stepping >= 4 and failed
on Ice Lake-D servers from stepping 0. The root cause was that for
Ice Lake and Tremont/Jacobsville servers with CPU stepping >=4, the
offset for bus number configuration register was updated from 0xcc
to 0xd0. For Ice Lake-D servers, all the steppings use the updated
0xd0 offset.

Fix the issue by using the appropriate offset for bus number
configuration register according to the CPU model number and stepping.

Reported-by: Jerry Chen <jerry.t.chen@intel.com>
Reported-and-tested-by: Jin Wen <wen.jin@intel.com>
Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 drivers/edac/i10nm_base.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Comments

Borislav Petkov April 27, 2020, 8:40 a.m. UTC | #1
On Fri, Apr 24, 2020 at 11:57:38AM -0700, Tony Luck wrote:

> Subject: Re: [PATCH 2/2] EDAC, i10nm: Fix i10nm_edac loading failure on some

Please use "EDAC/<driver>:" prefix.

And I guess that patch name should be something like:

EDAC/i10nm: Update driver to support different bus number config register offsets

or so.

> From: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> 
> It failed to load the i10nm_edac driver on Ice Lake and

"The i10nm_edac driver failed to load ... "

> Tremont/Jacobsville servers if their CPU stepping >= 4 and failed
> on Ice Lake-D servers from stepping 0. The root cause was that for
> Ice Lake and Tremont/Jacobsville servers with CPU stepping >=4, the
> offset for bus number configuration register was updated from 0xcc
> to 0xd0. For Ice Lake-D servers, all the steppings use the updated
> 0xd0 offset.
> 
> Fix the issue by using the appropriate offset for bus number
> configuration register according to the CPU model number and stepping.
> 
> Reported-by: Jerry Chen <jerry.t.chen@intel.com>
> Reported-and-tested-by: Jin Wen <wen.jin@intel.com>
> Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  drivers/edac/i10nm_base.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/edac/i10nm_base.c b/drivers/edac/i10nm_base.c
> index ba4578c6ef2b..ebb45738c11b 100644
> --- a/drivers/edac/i10nm_base.c
> +++ b/drivers/edac/i10nm_base.c
> @@ -122,16 +122,24 @@ static int i10nm_get_all_munits(void)
>  	return 0;
>  }
>  
> -static struct res_config i10nm_cfg = {
> +/* ATOM_TREMONT_D, ICELAKE_X */

That comment...

> +static struct res_config i10nm_cfg0 = {
>  	.type			= I10NM,
>  	.decs_did		= 0x3452,
>  	.busno_cfg_offset	= 0xcc,
>  };
>  
> +/* ICELAKE_D */

... and that one are kinda redundant...

> +static struct res_config i10nm_cfg1 = {
> +	.type			= I10NM,
> +	.decs_did		= 0x3452,
> +	.busno_cfg_offset	= 0xd0,
> +};
> +
>  static const struct x86_cpu_id i10nm_cpuids[] = {
> -	X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT_D,	&i10nm_cfg),
> -	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X,		&i10nm_cfg),
> -	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D,		&i10nm_cfg),
> +	X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT_D,	&i10nm_cfg0),
> +	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X,		&i10nm_cfg0),
> +	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D,		&i10nm_cfg1),

... since it is written here what what is.

Anyway, just nitpicks. Other than that:

Reviewed-by: Borislav Petkov <bp@suse.de>
diff mbox series

Patch

diff --git a/drivers/edac/i10nm_base.c b/drivers/edac/i10nm_base.c
index ba4578c6ef2b..ebb45738c11b 100644
--- a/drivers/edac/i10nm_base.c
+++ b/drivers/edac/i10nm_base.c
@@ -122,16 +122,24 @@  static int i10nm_get_all_munits(void)
 	return 0;
 }
 
-static struct res_config i10nm_cfg = {
+/* ATOM_TREMONT_D, ICELAKE_X */
+static struct res_config i10nm_cfg0 = {
 	.type			= I10NM,
 	.decs_did		= 0x3452,
 	.busno_cfg_offset	= 0xcc,
 };
 
+/* ICELAKE_D */
+static struct res_config i10nm_cfg1 = {
+	.type			= I10NM,
+	.decs_did		= 0x3452,
+	.busno_cfg_offset	= 0xd0,
+};
+
 static const struct x86_cpu_id i10nm_cpuids[] = {
-	X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT_D,	&i10nm_cfg),
-	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X,		&i10nm_cfg),
-	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D,		&i10nm_cfg),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT_D,	&i10nm_cfg0),
+	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X,		&i10nm_cfg0),
+	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D,		&i10nm_cfg1),
 	{}
 };
 MODULE_DEVICE_TABLE(x86cpu, i10nm_cpuids);
@@ -257,6 +265,10 @@  static int __init i10nm_init(void)
 		return -ENODEV;
 	cfg = (struct res_config *)id->driver_data;
 
+	/* Newer steppings have different offset for ATOM_TREMONT_D/ICELAKE_X */
+	if (boot_cpu_data.x86_stepping >= 4)
+		cfg->busno_cfg_offset = 0xd0;
+
 	rc = skx_get_hi_lo(0x09a2, off, &tolm, &tohm);
 	if (rc)
 		return rc;