On Tue, Apr 16, 2024 at 11:05:15PM +0000, Luck, Tony wrote: > Guenter, > > Thanks for taking a look at this patch. > > > > + case VFM_MODEL(INTEL_ICELAKE_X): > > > + case VFM_MODEL(INTEL_ICELAKE_D): > > > + case VFM_MODEL(INTEL_SAPPHIRERAPIDS_X): > > > > $ git describe > > v6.9-rc4-31-g96fca68c4fbf > > $ git grep VFM_MODEL > > $ > > > > So I guess this depends on some other patch ? > > That might be worth mentioning, especially since > > > > $ git describe > > next-20240416 > > $ git grep VFM_MODEL > > $ > > > > doesn't find it either. > > > Yes. This depends on parts 1,2,3 of this series. I should have added > that to the "---" meta comment part of these patches that I'm spraying > out to maintainers of random subsystems that use INTEL_FAM6 defines. > > > On top of that, it looks like > > > > #include <asm/cpu_device_id.h> > > > > introduces a dependency on X86 which is not currently the case. > > CONFIG_PECI is typically used on BMCs. Many of those do not use > > Intel CPUs. It does not seem appropriate to make support for PECI > > based hardware monitoring dependent on it running on an Intel CPU. > > It seems that some use (or need to know about) Icelake and Sapphire Rapids. > > How does this code get the value "peci_dev->info.model" used in this switch? > My understanding is that the PECI protocol provides that information. > Given that it is doing some magic just for some recent Intel CPUs, it seems > plausible that when non-family 6 CPUs appear in the market, it might have > to grab both the family and the model to reliably determine what to do? > > Simple options to avoid including <asm/cpu_device_id.h> would be > to either: > > 1) provide a copy of the VFM_MODEL macro here. > > or > > 2) Keep using the old INTEL_FAM6_* #define names, but define those for > the three CPU models peci needs locally instead of getting them from > <asm/intel-family.h>. It looks like there is a somewhat convoluted path to > include that. I see in <linux/peci.cpu.h> > > #include "../../arch/x86/include/asm/intel-family.h" > > > Or maybe some better option? > If the CPU defines and the new macro are to be kept in architecture code, maybe include arch/x86/include/asm/cpu_device_id.h from linux/peci.cpu.h. That would not be worse than today's include of intel-family.h. Guenter
