On Tue, 2018-11-06 at 17:20 -0600, Bjorn Helgaas wrote: > [+cc Sumeet, Srinivas for INT3401 questions below] > [Beginning of thread: > https://lore.kernel.org/linux-pci/20181102181055.130531-1-brian.woods@xxxxxxx/ > ] > > On Tue, Nov 06, 2018 at 11:00:59PM +0100, Borislav Petkov wrote: > > On Tue, Nov 06, 2018 at 03:42:56PM -0600, Bjorn Helgaas wrote: > > > This isn't some complicated new device where the programming > > > model > > > changed on the new CPU. This is a thermometer that was already > > > supported. ACPI provides plenty of functionality that could be > > > used > > > to support this generically, e.g., see drivers/acpi/thermal.c, > > > drivers/thermal/int340x_thermal/processor_thermal_device.c, etc. > > > > Ok, you say ACPI but how do you envision practically doing that? I > > mean, > > this is used by old boxes too - ever since K8. So how do we go and > > add > > ACPI functionality to old boxes? > > > > Or do you mean it should simply be converted to do > > pci_register_driver() > > with a struct pci_driver pointer which has all those PCI device IDs > > in a > > table? I'm looking at the last example > > drivers/thermal/int340x_thermal/processor_thermal_device.c you gave > > above. > > No, there would be no need to change anything for boxes already in > the > field. But for *new* systems, you could make devices or thermal > zones > in the ACPI namespace (they might even already be there for use by > Windows). > > drivers/thermal/int340x_thermal/processor_thermal_device.c claims > either INT3401 ACPI devices or listed PCI devices. To enumerate a driver to get processor temperature and get power properties, we have two methods: - The older Atom processors valleyview and Baytrail had no PCI device for the processor thermal management. There was INT3401 ACPI device to handle this. - The newer processors for core and Atom, has a dedicate PCI device and there is no INT3401 ACPI device anymore. Since OEM systems will have different power properties and thermal trips, there is a companion ACPI device, which provides PPCC and thermal trips and optionally output from another temperature sensor from the default on processor cores. Thanks, Srinivas > It looks like it > tries the platform (INT3401) devices first, and if it finds any, it > ignores any matching PCI devices. This *could* be so it uses INT3401 > devices on new platforms and falls back to the PCI devices otherwise, > although there *are* recent updates that add PCI IDs. > > It looks like INT3401 is Intel-specific since it uses a PPCC method > which isn't defined by the ACPI spec. But AMD could define a similar > PNP ID and have new platforms expose ACPI devices with _TMP methods > that know how to read the sensor on that platform. > > Or maybe even drivers/acpi/thermal.c, which claims every Thermal Zone > (ACPI 6.2, sec 11), would be sufficient. I don't know what the > relationship between hwmon and other thermal stuff, e.g., > Documentation/thermal/sysfs-api.txt is. acpi/thermal.c looks tied > into the drivers/thermal stuff (it registers "thermal_zone" devices), > but not to hwmon. > > > > But maybe there's some real value in the nitty-gritty device- > > > specific > > > code in amd_nb.c. If so, I guess you're stuck with updates like > > > this > > > and negotiating with the distros to do backports and new > > > releases. > > > > Well, even if it is converted to a different registration scheme, > > you > > still need to add new PCI device IDs to the table, no? So *some* > > sort of > > enablement still needs to happen. > > As long as we have a driver that knows how to claim a known PNP ID, > and every new platform exposes a device compatible with that ID, the > driver should just work. > > Bjorn
