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

Sohil Mehta
2026-02-18T23:06:53Z

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 */