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

[Dave Hansen]

On 2/18/26 11:26, Sohil Mehta wrote:
>> 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.

Thanks, I'll fix it up.

>> 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.

Right now, all the constants are 16 bits long, that's 19:4. If a future
CPU model needs more bits, they'll make all the constants bigger and can
change the comment.

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

I think I just verbatim copied the intel-family.h names and added PECI_.
Are you seeing something different.

BTW, we should probably comment the naming scheme and at least mention
that it should be consistent with the x86 code.