Re: [PATCH] peci: Remove dependency on x86 CPU variables

[Sohil Mehta <sohil.mehta@intel.com>]

On 2/18/2026 9:03 AM, Dave Hansen wrote:

> 
>  b/drivers/hwmon/peci/cputemp.c |   10 ++++-----
>  b/drivers/peci/core.c          |    4 +--
>  b/drivers/peci/cpu.c           |   16 +++++++--------
>  b/drivers/peci/device.c        |   40 ++++++++++++---------------------------
>  b/drivers/peci/internal.h      |    4 +--
>  b/include/linux/peci-cpu.h     |   42 ++++++++++++++++-------------------------
>  b/include/linux/peci.h         |    2 -
>  7 files changed, 48 insertions(+), 70 deletions(-)
> 

Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>

Everything mostly looks good. A few small suggestions below.


> diff -puN include/linux/peci.h~peci-sanity include/linux/peci.h
> --- a/include/linux/peci.h~peci-sanity	2026-02-18 08:19:42.371396743 -0800
> +++ b/include/linux/peci.h	2026-02-18 08:19:42.388397383 -0800
> @@ -72,7 +72,7 @@ static inline struct peci_controller *to
>  struct peci_device {
>  	struct device dev;
>  	struct {
> -		u32 x86_vfm;
> +		u32 device_id;

There is a kernel-doc comment on top of this struct as well that needs
to reflect the change.

>  		u8 peci_revision;
>  		u8 socket_id;
>  	} info;


> diff -puN include/linux/peci-cpu.h~peci-sanity include/linux/peci-cpu.h
> -#include "../../arch/x86/include/asm/intel-family.h"
> +/*
> + * These are in the format of and match the values of the x86
> + * CPUID.01H:EAX[19:4]. They encode the model and family of

Can we include the extended family bits in the *comment* to say
EAX[27:4]? I expect Family 19 (DMR) will be added soonish, which will
make the comment stale.

> + * the CPU with which the driver is interfacing.
> + *
> + * All driver functionality is common across all CPU steppings
> + * of a given model, so the lower 4 stepping bits are excluded
> + * from these IDs.
> + */
> +#define PECI_INTEL_HASWELL_X		0x306C
> +#define PECI_INTEL_BROADWELL_X		0x406F
> +#define PECI_INTEL_BROADWELL_D		0x5066
> +#define PECI_INTEL_SKYLAKE_X		0x5065
> +#define PECI_INTEL_ICELAKE_X		0x606A
> +#define PECI_INTEL_ICELAKE_D		0x606C
> +#define PECI_INTEL_SAPPHIRERAPIDS_X	0x806F
> +#define PECI_INTEL_EMERALDRAPIDS_X	0xC06F
>  

The _D has been used in Intel official product names such as "XEON D".
AFAIU, The _X notation is specific to intel-family.h. Should that be
explained in the comment above?

Something like:

 *	_X	- regular server parts
 *	_D	- micro server parts


>  #define PECI_PCS_PKG_ID			0  /* Package Identifier Read */
>  #define  PECI_PKG_ID_CPU_ID		0x0000  /* CPUID Info */