Re: [PATCH 2/4] x86/amd_nb: add support for newer PCI topologies

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Sumeet, Srinivas for INT3401 questions below]
[Beginning of thread:
https://lore.kernel.org/linux-pci/20181102181055.130531-1-brian.woods@amd.com/]

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