All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Winiarska, Iwona" <iwona.winiarska@intel.com>
To: "linux@roeck-us.net" <linux@roeck-us.net>,
	"Luck, Tony" <tony.luck@intel.com>, "bp@alien8.de" <bp@alien8.de>,
	"jdelvare@suse.com" <jdelvare@suse.com>
Cc: "patches@lists.linux.dev" <patches@lists.linux.dev>,
	"linux-hwmon@vger.kernel.org" <linux-hwmon@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 55/71] hwmon: (peci/cputemp) Switch to new Intel CPU model defines
Date: Fri, 26 Apr 2024 01:07:33 +0000	[thread overview]
Message-ID: <2fe8fce0d553eed5c27c51596f5bce780bbd9395.camel@intel.com> (raw)
In-Reply-To: <Ziq2AMcJUyikTtxK@agluck-desk3>

On Thu, 2024-04-25 at 12:58 -0700, Tony Luck wrote:
> > Is drivers/peci/device.c: peci_get_cpu_id() specific to Intel CPUs? It seems
> > to retrieve a value in the format of CPUID(EAX=1).EAX that would apply to
> > both Intel and AMD. Yet I don't see any checking of vendor id before looking
> > at the model id.
> 
> Some web wearching indicates that PECI is an Intel specific protocol, so
> no need for vendor checks.

Correct.

> 
> I've re-worked the peci patches (combining the core and hwmon bits) to
> switch from separate "family" and "model" fields and checks to using
> the same x86_vfm combined value.
> 
> The peci patch is included below. But you can see it in context in:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/aegl/linux.git
> new_families_v5_wip
> 
> The x86 build of these bits works. But I don't have cross-compile
> enviroment to check non-X86 builds. Also no way to test.

Compile-tested for now, but it looks good to me in general.
One small comment - the kerneldoc for struct peci_device_id in
drivers/peci/internal.h also needs to be updated.

Thanks
-Iwona

> 
> -Tony
> 
> ---
> 
> From 34cc093cbebaf596a52b123c4f8c3ef2df45a240 Mon Sep 17 00:00:00 2001
> From: Tony Luck <tony.luck@intel.com>
> Date: Thu, 25 Apr 2024 12:27:39 -0700
> Subject: [PATCH] peci, hwmon: Switch to new Intel CPU model defines
> 
> Update peci subsystem to use the same vendor-family-model
> combined definition that core x86 code uses.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  include/linux/peci-cpu.h     | 24 ++++++++++++++++++++++++
>  include/linux/peci.h         |  3 +--
>  drivers/peci/internal.h      |  3 +--
>  drivers/hwmon/peci/cputemp.c |  8 ++++----
>  drivers/peci/core.c          |  5 ++---
>  drivers/peci/cpu.c           | 21 +++++++--------------
>  drivers/peci/device.c        |  3 +--
>  7 files changed, 40 insertions(+), 27 deletions(-)
> 
> diff --git a/include/linux/peci-cpu.h b/include/linux/peci-cpu.h
> index ff8ae9c26c80..601cdd086bf6 100644
> --- a/include/linux/peci-cpu.h
> +++ b/include/linux/peci-cpu.h
> @@ -6,6 +6,30 @@
>  
>  #include <linux/types.h>
>  
> +/* Copied from x86 <asm/processor.h> */
> +#define X86_VENDOR_INTEL       0
> +
> +/* Copied from x86 <asm/cpu_device_id.h> */
> +#define VFM_MODEL_BIT	0
> +#define VFM_FAMILY_BIT	8
> +#define VFM_VENDOR_BIT	16
> +#define VFM_RSVD_BIT	24
> +
> +#define	VFM_MODEL_MASK	GENMASK(VFM_FAMILY_BIT - 1, VFM_MODEL_BIT)
> +#define	VFM_FAMILY_MASK	GENMASK(VFM_VENDOR_BIT - 1,
> VFM_FAMILY_BIT)
> +#define	VFM_VENDOR_MASK	GENMASK(VFM_RSVD_BIT - 1,
> VFM_VENDOR_BIT)
> +
> +#define VFM_MODEL(vfm)	(((vfm) & VFM_MODEL_MASK) >> VFM_MODEL_BIT)
> +#define VFM_FAMILY(vfm)	(((vfm) & VFM_FAMILY_MASK) >> VFM_FAMILY_BIT)
> +#define VFM_VENDOR(vfm)	(((vfm) & VFM_VENDOR_MASK) >> VFM_VENDOR_BIT)
> +
> +#define	VFM_MAKE(_vendor, _family, _model) (	\
> +	((_model) << VFM_MODEL_BIT) |		\
> +	((_family) << VFM_FAMILY_BIT) |		\
> +	((_vendor) << VFM_VENDOR_BIT)		\
> +)
> +/* End of copied code */
> +
>  #include "../../arch/x86/include/asm/intel-family.h"
>  
>  #define PECI_PCS_PKG_ID			0  /* Package Identifier Read
> */
> diff --git a/include/linux/peci.h b/include/linux/peci.h
> index 9b3d36aff431..0db8b5dd09d9 100644
> --- a/include/linux/peci.h
> +++ b/include/linux/peci.h
> @@ -74,8 +74,7 @@ static inline struct peci_controller
> *to_peci_controller(void *d)
>  struct peci_device {
>  	struct device dev;
>  	struct {
> -		u16 family;
> -		u8 model;
> +		u32 x86_vfm;
>  		u8 peci_revision;
>  		u8 socket_id;
>  	} info;
> diff --git a/drivers/peci/internal.h b/drivers/peci/internal.h
> index 9d75ea54504c..7ead53e5598f 100644
> --- a/drivers/peci/internal.h
> +++ b/drivers/peci/internal.h
> @@ -71,8 +71,7 @@ struct peci_request *peci_xfer_ep_mmio64_readl(struct
> peci_device *device, u8 ba
>   */
>  struct peci_device_id {
>  	const void *data;
> -	u16 family;
> -	u8 model;
> +	u32 x86_vfm;
>  };
>  
>  extern struct device_type peci_device_type;
> diff --git a/drivers/hwmon/peci/cputemp.c b/drivers/hwmon/peci/cputemp.c
> index a812c15948d9..5a682195b98f 100644
> --- a/drivers/hwmon/peci/cputemp.c
> +++ b/drivers/hwmon/peci/cputemp.c
> @@ -360,10 +360,10 @@ static int init_core_mask(struct peci_cputemp *priv)
>  	int ret;
>  
>  	/* Get the RESOLVED_CORES register value */
> -	switch (peci_dev->info.model) {
> -	case INTEL_FAM6_ICELAKE_X:
> -	case INTEL_FAM6_ICELAKE_D:
> -	case INTEL_FAM6_SAPPHIRERAPIDS_X:
> +	switch (peci_dev->info.x86_vfm) {
> +	case INTEL_ICELAKE_X:
> +	case INTEL_ICELAKE_D:
> +	case INTEL_SAPPHIRERAPIDS_X:
>  		ret = peci_ep_pci_local_read(peci_dev, 0, reg->bus, reg->dev,
>  					     reg->func, reg->offset + 4,
> &data);
>  		if (ret)
> diff --git a/drivers/peci/core.c b/drivers/peci/core.c
> index 0f83a9c6093b..b2d7adf05ba0 100644
> --- a/drivers/peci/core.c
> +++ b/drivers/peci/core.c
> @@ -163,9 +163,8 @@ EXPORT_SYMBOL_NS_GPL(devm_peci_controller_add, PECI);
>  static const struct peci_device_id *
>  peci_bus_match_device_id(const struct peci_device_id *id, struct peci_device
> *device)
>  {
> -	while (id->family != 0) {
> -		if (id->family == device->info.family &&
> -		    id->model == device->info.model)
> +	while (id->x86_vfm != 0) {
> +		if (id->x86_vfm == device->info.x86_vfm)
>  			return id;
>  		id++;
>  	}
> diff --git a/drivers/peci/cpu.c b/drivers/peci/cpu.c
> index bd990acd92b8..152bbd8e717a 100644
> --- a/drivers/peci/cpu.c
> +++ b/drivers/peci/cpu.c
> @@ -294,38 +294,31 @@ peci_cpu_probe(struct peci_device *device, const struct
> peci_device_id *id)
>  
>  static const struct peci_device_id peci_cpu_device_ids[] = {
>  	{ /* Haswell Xeon */
> -		.family	= 6,
> -		.model	= INTEL_FAM6_HASWELL_X,
> +		.x86_vfm = INTEL_HASWELL_X,
>  		.data	= "hsx",
>  	},
>  	{ /* Broadwell Xeon */
> -		.family	= 6,
> -		.model	= INTEL_FAM6_BROADWELL_X,
> +		.x86_vfm = INTEL_BROADWELL_X,
>  		.data	= "bdx",
>  	},
>  	{ /* Broadwell Xeon D */
> -		.family	= 6,
> -		.model	= INTEL_FAM6_BROADWELL_D,
> +		.x86_vfm = INTEL_BROADWELL_D,
>  		.data	= "bdxd",
>  	},
>  	{ /* Skylake Xeon */
> -		.family	= 6,
> -		.model	= INTEL_FAM6_SKYLAKE_X,
> +		.x86_vfm = INTEL_SKYLAKE_X,
>  		.data	= "skx",
>  	},
>  	{ /* Icelake Xeon */
> -		.family	= 6,
> -		.model	= INTEL_FAM6_ICELAKE_X,
> +		.x86_vfm = INTEL_ICELAKE_X,
>  		.data	= "icx",
>  	},
>  	{ /* Icelake Xeon D */
> -		.family	= 6,
> -		.model	= INTEL_FAM6_ICELAKE_D,
> +		.x86_vfm = INTEL_ICELAKE_D,
>  		.data	= "icxd",
>  	},
>  	{ /* Sapphire Rapids Xeon */
> -		.family	= 6,
> -		.model	= INTEL_FAM6_SAPPHIRERAPIDS_X,
> +		.x86_vfm = INTEL_SAPPHIRERAPIDS_X,
>  		.data	= "spr",
>  	},
>  	{ }
> diff --git a/drivers/peci/device.c b/drivers/peci/device.c
> index e6b0bffb14f4..5eb57b503c81 100644
> --- a/drivers/peci/device.c
> +++ b/drivers/peci/device.c
> @@ -100,8 +100,7 @@ static int peci_device_info_init(struct peci_device
> *device)
>  	if (ret)
>  		return ret;
>  
> -	device->info.family = peci_x86_cpu_family(cpu_id);
> -	device->info.model = peci_x86_cpu_model(cpu_id);
> +	device->info.x86_vfm = IFM(peci_x86_cpu_family(cpu_id),
> peci_x86_cpu_model(cpu_id));
>  
>  	ret = peci_get_revision(device, &revision);
>  	if (ret)


  reply	other threads:[~2024-04-26  1:07 UTC|newest]

Thread overview: 109+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-24 18:12 [PATCH v4 00/71] New Intel CPUID families Tony Luck
2024-04-24 18:14 ` [PATCH v4 01/71] tpm: Switch to new Intel CPU model defines Tony Luck
2024-04-25  4:48   ` Jarkko Sakkinen
2024-04-25  4:50     ` Jarkko Sakkinen
2024-04-24 18:14 ` [PATCH v4 02/71] platform/x86/intel/ifs: " Tony Luck
2024-04-29  3:19   ` Kuppuswamy Sathyanarayanan
2024-04-24 18:14 ` [PATCH v4 03/71] KVM: x86/pmu: " Tony Luck
2024-04-24 18:14 ` [PATCH v4 04/71] KVM: VMX: " Tony Luck
2024-04-24 18:14 ` [PATCH v4 05/71] ACPI: LPSS: " Tony Luck
2024-04-24 18:14 ` [PATCH v4 06/71] ACPI: x86: " Tony Luck
2024-04-24 18:14 ` [PATCH v4 07/71] cpufreq: intel_pstate: " Tony Luck
2024-04-24 18:14 ` [PATCH v4 08/71] cpufreq: " Tony Luck
2024-04-24 18:14 ` [PATCH v4 09/71] intel_idle: " Tony Luck
2024-04-24 18:14 ` [PATCH v4 10/71] PCI: PM: " Tony Luck
2024-04-24 18:14 ` [PATCH v4 12/71] powercap: intel_rapl: " Tony Luck
2024-04-24 18:14 ` [PATCH v4 11/71] " Tony Luck
2024-04-24 18:14 ` [PATCH v4 13/71] ASoC: Intel: " Tony Luck
2024-04-24 18:14 ` [PATCH v4 14/71] thermal: intel: intel_tcc_cooling: " Tony Luck
2024-04-24 18:14 ` [PATCH v4 15/71] tools/power/turbostat: " Tony Luck
2024-04-24 18:14 ` [PATCH v4 17/71] crypto: x86/twofish - " Tony Luck
2024-04-24 18:14 ` [PATCH v4 16/71] crypto: x86/poly1305 " Tony Luck
2024-05-31  9:37   ` Herbert Xu
2024-04-24 18:14 ` [PATCH v4 18/71] perf/x86/intel/cstate: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 20/71] perf/x86/intel/pt: " Tony Luck
2024-04-25 15:58   ` Dave Hansen
2024-04-24 18:15 ` [PATCH v4 19/71] perf/x86/lbr: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 21/71] perf/x86/intel/uncore: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 22/71] " Tony Luck
2024-04-24 18:15 ` [PATCH v4 24/71] perf/x86/msr: " Tony Luck
2024-04-25 15:49   ` Dave Hansen
2024-04-25 16:43     ` Luck, Tony
2024-04-25 16:47       ` Dave Hansen
2024-04-25 16:54         ` Luck, Tony
2024-04-24 18:15 ` [PATCH v4 23/71] perf/x86/intel/uncore: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 25/71] x86/apic: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 26/71] x86/aperfmperf: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 27/71] x86/bugs: " Tony Luck
2024-04-24 20:55   ` Josh Poimboeuf
2024-04-25 10:52   ` [tip: x86/cpu] " tip-bot2 for Tony Luck
2024-04-24 18:15 ` [PATCH v4 28/71] " Tony Luck
2024-04-24 21:09   ` Josh Poimboeuf
2024-04-25 10:52   ` [tip: x86/cpu] " tip-bot2 for Tony Luck
2024-04-24 18:15 ` [PATCH v4 29/71] x86/cpu/intel: " Tony Luck
2024-04-25 10:46   ` Borislav Petkov
2024-04-25 16:29     ` Luck, Tony
2024-04-28 18:27       ` Borislav Petkov
2024-04-24 18:15 ` [PATCH v4 30/71] x86/cpu/intel_epb: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 32/71] x86/mce: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 31/71] x86/cpu: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 33/71] x86/mce: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 35/71] x86/microcode/intel: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 34/71] x86/mce: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 37/71] x86/resctrl: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 36/71] " Tony Luck
2024-04-24 18:15 ` [PATCH v4 38/71] x86/cpu/: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 39/71] x86/tsc: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 40/71] x86/tsc_msr: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 41/71] x86/mm: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 42/71] x86/PCI: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 43/71] x86/virt/tdx: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 44/71] perf/x86/intel: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 45/71] x86/platform/intel-mid: " Tony Luck
2024-04-24 19:28   ` Andy Shevchenko
2024-04-24 18:15 ` [PATCH v4 46/71] x86/platform/atom: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 47/71] x86/cpu: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 49/71] EDAC/i10nm: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 48/71] x86/boot: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 50/71] EDAC, pnd2: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 51/71] EDAC/sb_edac: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 52/71] EDAC/skx: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 53/71] extcon: axp288: " Tony Luck
2024-05-08 15:21   ` Chanwoo Choi
2024-04-24 18:15 ` [PATCH v4 54/71] peci: cpu: " Tony Luck
2024-04-24 18:15   ` Tony Luck
2024-04-24 18:15 ` [PATCH v4 55/71] hwmon: (peci/cputemp) " Tony Luck
2024-04-24 20:00   ` Guenter Roeck
2024-04-24 20:25     ` Luck, Tony
2024-04-25 19:58       ` Tony Luck
2024-04-26  1:07         ` Winiarska, Iwona [this message]
2024-04-26 15:56           ` Luck, Tony
2024-04-24 18:15 ` [PATCH v4 56/71] platform/x86: intel_ips: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 57/71] platform/x86/intel/pmc: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 58/71] platform/x86/intel: pmc: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 59/71] platform/x86: intel_scu_wdt: " Tony Luck
2024-04-29  3:19   ` Kuppuswamy Sathyanarayanan
2024-04-24 18:15 ` [PATCH v4 60/71] platform/x86: ISST: " Tony Luck
2024-04-25 12:32   ` srinivas pandruvada
2024-04-25 16:34     ` Luck, Tony
2024-04-24 18:15 ` [PATCH v4 61/71] platform/x86: intel_speed_select_if: " Tony Luck
2024-04-25 12:33   ` srinivas pandruvada
2024-04-24 18:15 ` [PATCH v4 63/71] platform/x86: intel: telemetry: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 62/71] platform/x86: intel_telemetry: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 64/71] platform/x86: intel_turbo_max_3: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 65/71] platform/x86: intel-uncore-freq: " Tony Luck
2024-04-25 12:34   ` srinivas pandruvada
2024-04-24 18:15 ` [PATCH v4 66/71] platform/x86: p2sb: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 67/71] media: atomisp: " Tony Luck
2024-04-24 19:31   ` Andy Shevchenko
2024-04-24 18:15 ` [PATCH v4 68/71] ASoC: Intel: avs: es8336: " Tony Luck
2024-04-24 23:58   ` Mark Brown
2024-04-25  0:12     ` Luck, Tony
2024-04-25  0:58   ` Mark Brown
2024-04-25  7:39   ` Amadeusz Sławiński
2024-04-24 18:15 ` [PATCH v4 69/71] perf/x86/rapl: " Tony Luck
2024-04-24 18:15 ` [PATCH v4 70/71] x86/cpu/vfm: Delete X86_MATCH_INTEL_FAM6_MODEL[_STEPPING]() macros Tony Luck
2024-04-24 18:15 ` [PATCH v4 71/71] x86/cpu/vfm: Delete all the *_FAM6_ CPU #defines Tony Luck
2024-04-24 18:39 ` [PATCH v4 00/71] New Intel CPUID families Dave Hansen
2024-04-24 18:57   ` Luck, Tony
2024-04-29  8:08 ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2fe8fce0d553eed5c27c51596f5bce780bbd9395.camel@intel.com \
    --to=iwona.winiarska@intel.com \
    --cc=bp@alien8.de \
    --cc=jdelvare@suse.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=patches@lists.linux.dev \
    --cc=tony.luck@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.