Re: [PATCH] usb: xhci: add AMD PROM21 xHCI hwmon support for temperature monitoring

[Guenter Roeck <linux@roeck-us.net>]

On 5/6/26 06:37, Mathias Nyman wrote:
> Hi
> 
> On 5/6/26 06:29, Jihong Min wrote:
>> AMD PROM21 xHCI controllers expose a temperature byte through a
>> vendor-specific index/data register pair in the xHCI PCI MMIO BAR region.
>> Add hwmon support for this, limited to temperature monitoring, and
>> initialize it from the xHCI PCI driver probe path.
>>
>> The xhci-pci probe and remove paths call the PROM21 hwmon helpers only for
>> AMD 1022:43fd controllers. The hwmon read path selects the temperature
>> register through the vendor index register, reads the raw temperature byte
>> from the data register, and restores the previous index before returning.
>>
>> No public AMD register reference is available for this value. The
>> conversion formula is derived from observed temperature readings:
>>
>>    temp[C] = raw * 0.9066 - 78.624
>>
>> This is not implemented as a standalone driver because the temperature
>> register belongs to the PROM21 xHCI PCI function, is accessed through the
>> xHCI BAR after the host controller is initialized, and should share that
>> device's lifetime and runtime PM. Keep the device-specific PROM21 code in a
>> separate helper called only from xhci-pci, while linking it into xhci-hcd
>> to match the existing xHCI object layout. The xhci-pci remove path
>> invalidates the helper before HCD teardown.
>>
> 
> To me it looks like a sepate device should be created for this, and a new
> driver in drivers/hwmon bind to it.
> 
> Not sure what is the best solution, do we create a parent mfd driver that
> binds to the PCI device, which then creates two child devices, xhci and hwmon.
> Or do we just create some kind of platform device as a child to this xhci
> PCI device.
> 

Auxiliary devices were created specifically for this purpose. The parent driver
would create an auxiliary device in its probe function, which would then be
instantiated from the client driver.

On a side note, the code below looks messy. I would personally prefer something
like the Renesas solution.

Guenter

> Maybe someone on the PCI list would know (added to cc)? I guess it's not the
> first time we have a "multi function" PCI device that has just has one BAR.
> 
> The resources this device needs look simple, only access two registers at
> offset 0x3000 and 0x3008 from mmio base.
> 
> Does accessing those registers depend on xHC state? or is enough that the
> PCI device is enabled and in D0?  i.e. does xHC need to be in "running"
> state?
> I'd like to avoid resuming and restarting xHC every time temperature is read
> from a sysfs file.
> 
> Thanks
> Mathias
> 
> (keeping reset of message as reference for pci list readers)
> 
>> Assisted-by: Codex:gpt-5.5
>> Signed-off-by: Jihong Min <hurryman2212@gmail.com>
>> ---
>>   drivers/usb/host/Kconfig             |  10 ++
>>   drivers/usb/host/Makefile            |   4 +
>>   drivers/usb/host/xhci-pci.c          |   9 +
>>   drivers/usb/host/xhci-prom21-hwmon.c | 241 +++++++++++++++++++++++++++
>>   drivers/usb/host/xhci-prom21-hwmon.h |  26 +++
>>   5 files changed, 290 insertions(+)
>>   create mode 100644 drivers/usb/host/xhci-prom21-hwmon.c
>>   create mode 100644 drivers/usb/host/xhci-prom21-hwmon.h
>>
>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>> index 0a277a07cf70..da41ebc272b0 100644
>> --- a/drivers/usb/host/Kconfig
>> +++ b/drivers/usb/host/Kconfig
>> @@ -42,6 +42,16 @@ config USB_XHCI_PCI
>>       depends on USB_PCI
>>       default y
>> +config USB_XHCI_PCI_PROM21_HWMON
>> +    bool "AMD PROM21 xHCI temperature sensor support"
>> +    depends on USB_XHCI_PCI
>> +    depends on HWMON
>> +    help
>> +      Say Y here to expose the AMD PROM21 xHCI temperature sensor
>> +      through the hwmon subsystem. The sensor is accessed through a
>> +      vendor-specific index/data register pair in the controller's PCI
>> +      MMIO BAR and reports temperature only. If unsure, say N.
>> +
>>   config USB_XHCI_PCI_RENESAS
>>       tristate "Support for additional Renesas xHCI controller with firmware"
>>       depends on USB_XHCI_PCI
>> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
>> index a07e7ba9cd53..22f141cb2af7 100644
>> --- a/drivers/usb/host/Makefile
>> +++ b/drivers/usb/host/Makefile
>> @@ -36,6 +36,10 @@ ifneq ($(CONFIG_USB_XHCI_SIDEBAND),)
>>       xhci-hcd-y        += xhci-sideband.o
>>   endif
>> +ifneq ($(CONFIG_USB_XHCI_PCI_PROM21_HWMON),)
>> +    xhci-hcd-y += xhci-prom21-hwmon.o
>> +endif
>> +
>>   obj-$(CONFIG_USB_PCI)    += pci-quirks.o
>>   obj-$(CONFIG_USB_EHCI_HCD)    += ehci-hcd.o
>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>> index 585b2f3117b0..54713efc931f 100644
>> --- a/drivers/usb/host/xhci-pci.c
>> +++ b/drivers/usb/host/xhci-pci.c
>> @@ -18,6 +18,7 @@
>>   #include "xhci.h"
>>   #include "xhci-trace.h"
>>   #include "xhci-pci.h"
>> +#include "xhci-prom21-hwmon.h"
>>   #define SSIC_PORT_NUM        2
>>   #define SSIC_PORT_CFG2        0x880c
>> @@ -677,6 +678,10 @@ int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id)
>>       if (device_property_read_bool(&dev->dev, "ti,pwron-active-high"))
>>           pci_clear_and_set_config_dword(dev, 0xE0, 0, 1 << 22);
>> +    if (dev->vendor == PCI_VENDOR_ID_AMD &&
>> +        dev->device == PCI_DEVICE_ID_AMD_PROM21_XHCI)
>> +        xhci_prom21_hwmon_init(xhci, dev);
>> +
>>       return 0;
>>   put_usb3_hcd:
>> @@ -713,6 +718,10 @@ void xhci_pci_remove(struct pci_dev *dev)
>>       xhci = hcd_to_xhci(pci_get_drvdata(dev));
>>       set_power_d3 = xhci->quirks & XHCI_SPURIOUS_WAKEUP;
>> +    if (dev->vendor == PCI_VENDOR_ID_AMD &&
>> +        dev->device == PCI_DEVICE_ID_AMD_PROM21_XHCI)
>> +        xhci_try_prom21_hwmon_invalidate(dev);
>> +
>>       xhci->xhc_state |= XHCI_STATE_REMOVING;
>>       if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0)
>> diff --git a/drivers/usb/host/xhci-prom21-hwmon.c b/drivers/usb/host/xhci-prom21-hwmon.c
>> new file mode 100644
>> index 000000000000..5f71e72f4a90
>> --- /dev/null
>> +++ b/drivers/usb/host/xhci-prom21-hwmon.c
>> @@ -0,0 +1,241 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * AMD PROM21 xHCI Hwmon Implementation
>> + * (only temperature monitoring is supported)
>> + *
>> + * This can be effectively used as the alternative chipset temperature monitor.
>> + *
>> + * Copyright (C) 2026 Jihong Min <hurryman2212@gmail.com>
>> + */
>> +
>> +#include <linux/device/devres.h>
>> +#include <linux/err.h>
>> +#include <linux/errno.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/io.h>
>> +#include <linux/math.h>
>> +#include <linux/mutex.h>
>> +#include <linux/pci.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/slab.h>
>> +
>> +#include "xhci.h"
>> +#include "xhci-prom21-hwmon.h"
>> +
>> +#define XHCI_PROM21_INDEX 0x3000
>> +#define XHCI_PROM21_DATA 0x3008
>> +#define XHCI_PROM21_TEMP_REG 0x0001e520
>> +
>> +#define XHCI_PROM21_HWMON_NAME "xhci_prom21"
>> +
>> +struct xhci_prom21_hwmon {
>> +    struct pci_dev *pdev;
>> +    void __iomem *regs;
>> +    bool removing;
>> +    struct mutex lock; /* protects removing and the index/data registers */
>> +};
>> +
>> +struct xhci_prom21_hwmon_devres {
>> +    struct xhci_prom21_hwmon *hwmon;
>> +};
>> +
>> +static void xhci_prom21_hwmon_invalidate(struct xhci_prom21_hwmon *hwmon)
>> +{
>> +    mutex_lock(&hwmon->lock);
>> +    hwmon->removing = true;
>> +    mutex_unlock(&hwmon->lock);
>> +}
>> +
>> +static void xhci_prom21_hwmon_devres_release(struct device *dev, void *res)
>> +{
>> +    struct xhci_prom21_hwmon_devres *devres = res;
>> +
>> +    /*
>> +     * devm hwmon unregister runs after this lookup record is released.
>> +     * Mark the data path closed first so any late sysfs read returns
>> +     * without touching xHCI MMIO.
>> +     */
>> +    xhci_prom21_hwmon_invalidate(devres->hwmon);
>> +}
>> +
>> +/*
>> + * This is not a pure MMIO read. The PROM21 vendor data register is selected
>> + * by temporarily writing XHCI_PROM21_TEMP_REG to the vendor index register.
>> + * Keep the sequence short and restore the previous index before returning.
>> + */
>> +static int
>> +xhci_prom21_read_temp_raw_restore_index(struct xhci_prom21_hwmon *hwmon,
>> +                    u8 *raw)
>> +{
>> +    struct device *dev = &hwmon->pdev->dev;
>> +    u32 index;
>> +    u32 data;
>> +    int ret;
>> +
>> +    /*
>> +     * xhci_try_prom21_hwmon_invalidate() uses the same lock before HCD
>> +     * teardown. Keep runtime PM and MMIO inside the critical section so a
>> +     * sysfs read cannot use the vendor register pair after remove starts.
>> +     */
>> +    mutex_lock(&hwmon->lock);
>> +    if (hwmon->removing) {
>> +        mutex_unlock(&hwmon->lock);
>> +        return -ENODEV;
>> +    }
>> +
>> +    ret = pm_runtime_resume_and_get(dev);
>> +    if (ret < 0) {
>> +        mutex_unlock(&hwmon->lock);
>> +        return ret;
>> +    }
>> +
>> +    index = readl(hwmon->regs + XHCI_PROM21_INDEX);
>> +    /* Select the PROM21 temperature register through the vendor index. */
>> +    writel(XHCI_PROM21_TEMP_REG, hwmon->regs + XHCI_PROM21_INDEX);
>> +    data = readl(hwmon->regs + XHCI_PROM21_DATA);
>> +    /* Restore the previous vendor index register value. */
>> +    writel(index, hwmon->regs + XHCI_PROM21_INDEX);
>> +    readl(hwmon->regs + XHCI_PROM21_INDEX);
>> +
>> +    /* Let xHCI PCI runtime PM coalesce repeated sysfs polling. */
>> +    pm_runtime_mark_last_busy(dev);
>> +    pm_runtime_put_autosuspend(dev);
>> +    mutex_unlock(&hwmon->lock);
>> +
>> +    *raw = data & 0xff;
>> +    if (!*raw || *raw == 0xff)
>> +        return -ENODATA;
>> +
>> +    return 0;
>> +}
>> +
>> +static long xhci_prom21_raw_to_millicelsius(u8 raw)
>> +{
>> +    /*
>> +     * No public AMD register reference is available for this value.
>> +     * The scale was derived from observed PROM21 xHCI temperature readings:
>> +     *  temp[C] = raw * 0.9066 - 78.624
>> +     */
>> +    return DIV_ROUND_CLOSEST(raw * 9066, 10) - 78624;
>> +}
>> +
>> +static umode_t xhci_prom21_hwmon_is_visible(const void *drvdata,
>> +                        enum hwmon_sensor_types type,
>> +                        u32 attr, int channel)
>> +{
>> +    if (type != hwmon_temp || channel)
>> +        return 0;
>> +
>> +    switch (attr) {
>> +    case hwmon_temp_input:
>> +        return 0444;
>> +    default:
>> +        return 0;
>> +    }
>> +}
>> +
>> +static int xhci_prom21_hwmon_read(struct device *dev,
>> +                  enum hwmon_sensor_types type, u32 attr,
>> +                  int channel, long *val)
>> +{
>> +    struct xhci_prom21_hwmon *hwmon = dev_get_drvdata(dev);
>> +    u8 raw;
>> +    int ret;
>> +
>> +    if (type != hwmon_temp || attr != hwmon_temp_input || channel)
>> +        return -EOPNOTSUPP;
>> +
>> +    ret = xhci_prom21_read_temp_raw_restore_index(hwmon, &raw);
>> +    if (ret)
>> +        return ret;
>> +
>> +    *val = xhci_prom21_raw_to_millicelsius(raw);
>> +    return 0;
>> +}
>> +
>> +static const struct hwmon_ops xhci_prom21_hwmon_ops = {
>> +    .is_visible = xhci_prom21_hwmon_is_visible,
>> +    .read = xhci_prom21_hwmon_read,
>> +};
>> +
>> +static const struct hwmon_channel_info *const xhci_prom21_hwmon_info[] = {
>> +    HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
>> +    NULL,
>> +};
>> +
>> +static const struct hwmon_chip_info xhci_prom21_chip_info = {
>> +    .ops = &xhci_prom21_hwmon_ops,
>> +    .info = xhci_prom21_hwmon_info,
>> +};
>> +
>> +void xhci_prom21_hwmon_init(struct xhci_hcd *xhci, struct pci_dev *pdev)
>> +{
>> +    struct xhci_prom21_hwmon_devres *devres;
>> +    struct xhci_prom21_hwmon *hwmon;
>> +    struct usb_hcd *hcd = xhci_to_hcd(xhci);
>> +    struct device *dev = &pdev->dev;
>> +    struct device *hwmon_dev;
>> +
>> +    if (!hcd->regs || hcd->rsrc_len < XHCI_PROM21_DATA + sizeof(u32)) {
>> +        dev_err(dev,
>> +            "AMD PROM21 hwmon unavailable: invalid MMIO resource\n");
>> +        return;
>> +    }
>> +
>> +    hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL);
>> +    if (!hwmon) {
>> +        /* The allocator reports OOM; add PROM21 device context. */
>> +        dev_err(dev, "AMD PROM21 hwmon state unavailable\n");
>> +        return;
>> +    }
>> +
>> +    devres = devres_alloc(xhci_prom21_hwmon_devres_release, sizeof(*devres),
>> +                  GFP_KERNEL);
>> +    if (!devres) {
>> +        dev_err(dev, "AMD PROM21 hwmon devres allocation failed\n");
>> +        return;
>> +    }
>> +
>> +    hwmon->pdev = pdev;
>> +    hwmon->regs = hcd->regs;
>> +    mutex_init(&hwmon->lock);
>> +
>> +    hwmon_dev = devm_hwmon_device_register_with_info(dev,
>> +                             XHCI_PROM21_HWMON_NAME,
>> +                             hwmon,
>> +                             &xhci_prom21_chip_info,
>> +                             NULL);
>> +    if (IS_ERR(hwmon_dev)) {
>> +        devres_free(devres);
>> +        dev_err(dev, "AMD PROM21 hwmon registration failed: %pe\n",
>> +            hwmon_dev);
>> +        return;
>> +    }
>> +
>> +    /*
>> +     * Store a private devres record so the device remove path can find this
>> +     * state without adding PROM21-specific part to xhci-pci.
>> +     */
>> +    devres->hwmon = hwmon;
>> +    devres_add(dev, devres);
>> +}
>> +EXPORT_SYMBOL_GPL(xhci_prom21_hwmon_init);
>> +
>> +void xhci_try_prom21_hwmon_invalidate(struct pci_dev *pdev)
>> +{
>> +    struct xhci_prom21_hwmon_devres *devres;
>> +
>> +    /*
>> +     * This is called for every xHCI PCI device. Devices without PROM21
>> +     * hwmon support simply have no matching helper devres entry.
>> +     */
>> +    devres = devres_find(&pdev->dev, xhci_prom21_hwmon_devres_release, NULL,
>> +                 NULL);
>> +    if (!devres) {
>> +        dev_dbg(&pdev->dev, "AMD PROM21 hwmon state not found\n");
>> +        return;
>> +    }
>> +
>> +    xhci_prom21_hwmon_invalidate(devres->hwmon);
>> +}
>> +EXPORT_SYMBOL_GPL(xhci_try_prom21_hwmon_invalidate);
>> diff --git a/drivers/usb/host/xhci-prom21-hwmon.h b/drivers/usb/host/xhci-prom21-hwmon.h
>> new file mode 100644
>> index 000000000000..eca8db23c0ea
>> --- /dev/null
>> +++ b/drivers/usb/host/xhci-prom21-hwmon.h
>> @@ -0,0 +1,26 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Copyright (C) 2026 Jihong Min <hurryman2212@gmail.com> */
>> +
>> +#ifndef XHCI_PROM21_HWMON_H
>> +#define XHCI_PROM21_HWMON_H
>> +
>> +#define PCI_DEVICE_ID_AMD_PROM21_XHCI 0x43fd
>> +
>> +struct pci_dev;
>> +struct xhci_hcd;
>> +
>> +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_PROM21_HWMON)
>> +void xhci_prom21_hwmon_init(struct xhci_hcd *xhci, struct pci_dev *pdev);
>> +void xhci_try_prom21_hwmon_invalidate(struct pci_dev *pdev);
>> +#else
>> +static inline void xhci_prom21_hwmon_init(struct xhci_hcd *xhci,
>> +                      struct pci_dev *pdev)
>> +{
>> +}
>> +
>> +static inline void xhci_try_prom21_hwmon_invalidate(struct pci_dev *pdev)
>> +{
>> +}
>> +#endif
>> +
>> +#endif
>