* [PATCH] usb: xhci: add AMD PROM21 xHCI hwmon support for temperature monitoring
@ 2026-05-06 3:29 Jihong Min
2026-05-06 13:37 ` Mathias Nyman
2026-05-06 20:40 ` [PATCH v2 0/2] AMD PROM21 xHCI temperature hwmon support Jihong Min
0 siblings, 2 replies; 16+ messages in thread
From: Jihong Min @ 2026-05-06 3:29 UTC (permalink / raw)
To: Greg Kroah-Hartman, Mathias Nyman
Cc: Guenter Roeck, Mario Limonciello, Basavaraj Natikar, Raju Rangoju,
linux-usb, linux-hwmon, linux-kernel, Jihong Min
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.
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
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] usb: xhci: add AMD PROM21 xHCI hwmon support for temperature monitoring
2026-05-06 3:29 [PATCH] usb: xhci: add AMD PROM21 xHCI hwmon support for temperature monitoring Jihong Min
@ 2026-05-06 13:37 ` Mathias Nyman
2026-05-06 14:26 ` Guenter Roeck
2026-05-06 20:40 ` [PATCH v2 0/2] AMD PROM21 xHCI temperature hwmon support Jihong Min
1 sibling, 1 reply; 16+ messages in thread
From: Mathias Nyman @ 2026-05-06 13:37 UTC (permalink / raw)
To: Jihong Min, Greg Kroah-Hartman, Mathias Nyman
Cc: Guenter Roeck, Mario Limonciello, Basavaraj Natikar, Raju Rangoju,
linux-usb, linux-hwmon, linux-kernel, Linux PCI
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.
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] usb: xhci: add AMD PROM21 xHCI hwmon support for temperature monitoring
2026-05-06 13:37 ` Mathias Nyman
@ 2026-05-06 14:26 ` Guenter Roeck
2026-05-06 20:28 ` Jihong Min
0 siblings, 1 reply; 16+ messages in thread
From: Guenter Roeck @ 2026-05-06 14:26 UTC (permalink / raw)
To: Mathias Nyman, Jihong Min, Greg Kroah-Hartman, Mathias Nyman
Cc: Mario Limonciello, Basavaraj Natikar, Raju Rangoju, linux-usb,
linux-hwmon, linux-kernel, Linux PCI
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] usb: xhci: add AMD PROM21 xHCI hwmon support for temperature monitoring
2026-05-06 14:26 ` Guenter Roeck
@ 2026-05-06 20:28 ` Jihong Min
0 siblings, 0 replies; 16+ messages in thread
From: Jihong Min @ 2026-05-06 20:28 UTC (permalink / raw)
To: Guenter Roeck, Mathias Nyman, Jihong Min, Greg Kroah-Hartman,
Mathias Nyman
Cc: Mario Limonciello, Basavaraj Natikar, Raju Rangoju, linux-usb,
linux-hwmon, linux-kernel, Linux PCI
Hi Mathias, Guenter,
Thank you for the review. I reworked the patch for v2 and addressed the
main points as follows.
> To me it looks like a separate device should be created for this, and a
> new driver in drivers/hwmon bind to it.
Addressed in v2. The PROM21-specific hwmon code is no longer linked into
drivers/usb/host. The xhci-pci side now creates an auxiliary device for
selected controllers, and the PROM21 temperature support is implemented as a
separate driver under drivers/hwmon which binds to that auxiliary device.
The v2 series is split into two patches:
1. usb: xhci-pci: add generic auxiliary device interface
2. hwmon: add initial support for AMD PROM21 xHCI temperature sensor
> 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.
Addressed in v2. xhci-pci creates the auxiliary device only for
controllers matched by an internal PCI ID table, currently only AMD
1022:43fd. It destroys that auxiliary device from the remove path before
HCD teardown. The hwmon driver binds through the auxiliary bus and owns the
hwmon device lifetime.
The hwmon device is registered under the parent PCI function so userspace
still reports it as a PCI adapter, but the auxiliary driver explicitly
unregisters it from the auxiliary remove path before xhci-pci tears down the
HCD.
> On a side note, the code below looks messy. I would personally prefer
> something like the Renesas solution.
I moved the PROM21-specific code out of xhci-pci and out of
drivers/usb/host. xhci-pci now only has the generic auxiliary-device
creation side; the register offsets, conversion formula, hwmon callbacks,
and runtime PM policy are all in drivers/hwmon/prom21-hwmon.c.
I also tried to keep the generic auxiliary-device support in xhci-pci as
small and generic as possible. Similar to the Renesas handling, xhci-pci
uses PCI ID based filtering, and only matching devices get an auxiliary
child device.
> 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.
I tested this on my system. Reading the PROM21 temperature register without
runtime-resuming the parent xHCI PCI function did not return a valid
temperature value while the device was suspended. Reading after runtime
resume returned valid values.
Based on that result, v2 keeps the default behavior of allowing a
temperature read to wake the xHCI PCI device. I also added the
allow_pm_switch module parameter. With allow_pm_switch=N, a hwmon read
does not wake the xHCI PCI device; if the device is suspended, the read
returns -EAGAIN instead. v2 also adds
Documentation/hwmon/prom21-hwmon.rst, which documents this behavior.
> Can this cause a linker error if CONFIG_USB_XHCI_PCI is built-in (y) and
> CONFIG_HWMON is built as a module (m)?
Addressed by the v2 structure. xhci-hcd no longer calls into hwmon APIs or
links PROM21 hwmon code directly. The PROM21 temperature support is now a
standalone hwmon driver using the auxiliary bus, so the xhci-pci side only
creates the auxiliary device and does not reference hwmon symbols.
I also added Documentation/hwmon/prom21-hwmon.rst documenting the
supported device, register offsets, conversion formula, sysfs attributes,
lookup method, and allow_pm_switch behavior.
Sincerely,
Jihong Min
On 5/6/26 23:26, Guenter Roeck wrote:
> 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
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 0/2] AMD PROM21 xHCI temperature hwmon support
2026-05-06 3:29 [PATCH] usb: xhci: add AMD PROM21 xHCI hwmon support for temperature monitoring Jihong Min
2026-05-06 13:37 ` Mathias Nyman
@ 2026-05-06 20:40 ` Jihong Min
2026-05-06 20:40 ` [PATCH v2 1/2] usb: xhci-pci: add generic auxiliary device interface Jihong Min
2026-05-06 20:40 ` [PATCH v2 2/2] hwmon: add initial support for AMD PROM21 xHCI temperature sensor Jihong Min
1 sibling, 2 replies; 16+ messages in thread
From: Jihong Min @ 2026-05-06 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Mathias Nyman
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Mario Limonciello,
Basavaraj Natikar, linux-usb, linux-hwmon, linux-doc, linux-pci,
linux-kernel, Jihong Min
Hi,
This series adds hwmon support for the temperature sensor exposed by AMD
PROM21 xHCI controllers. The temperature value is read through a
vendor-specific index/data register pair in the xHCI PCI MMIO BAR.
v1 implemented this directly under drivers/usb/host. Based on review
feedback, v2 splits the support into a generic xhci-pci auxiliary-device
interface and a standalone hwmon driver under drivers/hwmon. xhci-pci
creates the auxiliary child only for PCI IDs matched by its internal table,
currently only AMD 1022:43fd, and the hwmon driver binds to that child
through the auxiliary bus.
The PROM21 hwmon driver registers the hwmon device under the parent PCI
function so userspace reports it as a PCI adapter. The auxiliary driver
still owns the lifetime and unregisters the hwmon device before xhci-pci
tears down the HCD.
I tested register access while the parent xHCI PCI function was suspended.
The temperature register did not return a valid value without
runtime-resuming the device. v2 therefore keeps the default behavior of
allowing a temperature read to wake the xHCI PCI device, and adds
allow_pm_switch=N for users who prefer reads not to wake the device. In
that mode a read returns -EAGAIN while the device is suspended.
Changes in v2:
- split the original single patch into a two-patch series
- add a generic xhci-pci auxiliary-device interface
- move PROM21 hwmon support to drivers/hwmon as an auxiliary driver
- avoid linking hwmon-specific code into xhci-hcd
- add Documentation/hwmon/prom21-hwmon.rst
- document register offsets, conversion formula, sysfs attributes, and
allow_pm_switch behavior
Sincerely,
Jihong Min
Jihong Min (2):
usb: xhci-pci: add generic auxiliary device interface
hwmon: add initial support for AMD PROM21 xHCI temperature sensor
Documentation/hwmon/index.rst | 1 +
Documentation/hwmon/prom21-hwmon.rst | 84 ++++++++
drivers/hwmon/Kconfig | 11 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/prom21-hwmon.c | 300 +++++++++++++++++++++++++++
drivers/usb/host/Kconfig | 10 +
drivers/usb/host/xhci-pci.c | 114 ++++++++++
7 files changed, 521 insertions(+)
create mode 100644 Documentation/hwmon/prom21-hwmon.rst
create mode 100644 drivers/hwmon/prom21-hwmon.c
--
2.53.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/2] usb: xhci-pci: add generic auxiliary device interface
2026-05-06 20:40 ` [PATCH v2 0/2] AMD PROM21 xHCI temperature hwmon support Jihong Min
@ 2026-05-06 20:40 ` Jihong Min
2026-05-06 20:53 ` Mario Limonciello
2026-05-06 21:15 ` Michal Pecio
2026-05-06 20:40 ` [PATCH v2 2/2] hwmon: add initial support for AMD PROM21 xHCI temperature sensor Jihong Min
1 sibling, 2 replies; 16+ messages in thread
From: Jihong Min @ 2026-05-06 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Mathias Nyman
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Mario Limonciello,
Basavaraj Natikar, linux-usb, linux-hwmon, linux-doc, linux-pci,
linux-kernel, Jihong Min
Some xHCI PCI controllers expose controller-specific functionality that is
not part of generic xHCI operation and is better handled by optional child
drivers in other subsystems. Add a small auxiliary device registration path
for selected xHCI PCI controllers.
The initial table creates an xhci_pci.hwmon auxiliary device for AMD
1022:43fd controllers. Store the created auxiliary device in devres so the
xhci-pci remove path destroys it before HCD teardown. Use a
PCI-domain-qualified id so auxiliary device names remain unique across PCI
domains.
This keeps xhci-pci responsible only for publishing selected controller
functions while allowing subsystem-specific drivers to bind through the
auxiliary bus.
Assisted-by: Codex:gpt-5.5
Signed-off-by: Jihong Min <hurryman2212@gmail.com>
---
drivers/usb/host/Kconfig | 10 ++++
drivers/usb/host/xhci-pci.c | 114 ++++++++++++++++++++++++++++++++++++
2 files changed, 124 insertions(+)
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 0a277a07cf70..e0c2c7ac5c97 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_AUXDEV
+ bool "xHCI PCI auxiliary device support"
+ depends on USB_XHCI_PCI
+ select AUXILIARY_BUS
+ help
+ This enables xHCI PCI support for registering auxiliary devices
+ for selected controllers. It is used by optional child drivers
+ that bind to xHCI PCI controller-specific functionality through
+ the auxiliary bus.
+
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/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 585b2f3117b0..1ab27d2182eb 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -8,6 +8,8 @@
* Some code borrowed from the Linux EHCI driver.
*/
+#include <linux/auxiliary_bus.h>
+#include <linux/device/devres.h>
#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/module.h>
@@ -103,6 +105,114 @@ static int xhci_pci_run(struct usb_hcd *hcd);
static int xhci_pci_update_hub_device(struct usb_hcd *hcd, struct usb_device *hdev,
struct usb_tt *tt, gfp_t mem_flags);
+#if IS_ENABLED(CONFIG_USB_XHCI_PCI_AUXDEV)
+static const struct {
+ struct pci_device_id id;
+ const char *aux_dev_name;
+} pci_ids_have_aux[] = {
+ {
+ .id = { PCI_DEVICE(PCI_VENDOR_ID_AMD, 0x43fd) }, /* PROM21 xHCI */
+ .aux_dev_name = "hwmon",
+ },
+ { /* end: all zeroes */ }
+};
+
+struct xhci_pci_aux_devres {
+ struct auxiliary_device *auxdev;
+};
+
+static bool xhci_pci_aux_match_id(const struct pci_device_id *id,
+ const struct pci_dev *pdev)
+{
+ if (id->vendor != PCI_ANY_ID && id->vendor != pdev->vendor)
+ return false;
+ if (id->device != PCI_ANY_ID && id->device != pdev->device)
+ return false;
+ if (id->subvendor != PCI_ANY_ID &&
+ id->subvendor != pdev->subsystem_vendor)
+ return false;
+ if (id->subdevice != PCI_ANY_ID &&
+ id->subdevice != pdev->subsystem_device)
+ return false;
+
+ return !((id->class ^ pdev->class) & id->class_mask);
+}
+
+static const char *xhci_pci_aux_dev_name(struct pci_dev *pdev)
+{
+ int i;
+
+ for (i = 0; pci_ids_have_aux[i].aux_dev_name; i++) {
+ if (xhci_pci_aux_match_id(&pci_ids_have_aux[i].id, pdev))
+ return pci_ids_have_aux[i].aux_dev_name;
+ }
+
+ return NULL;
+}
+
+static void xhci_pci_aux_devres_release(struct device *dev, void *res)
+{
+ struct xhci_pci_aux_devres *devres = res;
+
+ if (devres->auxdev)
+ auxiliary_device_destroy(devres->auxdev);
+}
+
+static void xhci_pci_try_add_aux_device(struct pci_dev *pdev)
+{
+ struct xhci_pci_aux_devres *devres;
+ struct auxiliary_device *auxdev;
+ const char *aux_dev_name;
+
+ aux_dev_name = xhci_pci_aux_dev_name(pdev);
+ if (!aux_dev_name)
+ return;
+
+ devres = devres_alloc(xhci_pci_aux_devres_release, sizeof(*devres),
+ GFP_KERNEL);
+ if (!devres) {
+ dev_warn(&pdev->dev,
+ "failed to allocate auxiliary device state\n");
+ return;
+ }
+
+ auxdev = auxiliary_device_create(&pdev->dev, KBUILD_MODNAME,
+ aux_dev_name, NULL,
+ (pci_domain_nr(pdev->bus) << 16) |
+ pci_dev_id(pdev));
+ if (!auxdev) {
+ devres_free(devres);
+ dev_warn(&pdev->dev, "failed to add %s auxiliary device\n",
+ aux_dev_name);
+ return;
+ }
+
+ devres->auxdev = auxdev;
+ devres_add(&pdev->dev, devres);
+}
+
+static void xhci_pci_try_remove_aux_device(struct pci_dev *pdev)
+{
+ struct xhci_pci_aux_devres *devres;
+
+ devres = devres_find(&pdev->dev, xhci_pci_aux_devres_release, NULL,
+ NULL);
+ if (!devres || !devres->auxdev)
+ return;
+
+ auxiliary_device_destroy(devres->auxdev);
+ devres->auxdev = NULL;
+}
+#else
+static inline void xhci_pci_try_add_aux_device(struct pci_dev *pdev)
+{
+}
+
+static inline void xhci_pci_try_remove_aux_device(struct pci_dev *pdev)
+{
+}
+#endif
+
static const struct xhci_driver_overrides xhci_pci_overrides __initconst = {
.reset = xhci_pci_setup,
.start = xhci_pci_run,
@@ -677,6 +787,8 @@ 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);
+ xhci_pci_try_add_aux_device(dev);
+
return 0;
put_usb3_hcd:
@@ -713,6 +825,8 @@ void xhci_pci_remove(struct pci_dev *dev)
xhci = hcd_to_xhci(pci_get_drvdata(dev));
set_power_d3 = xhci->quirks & XHCI_SPURIOUS_WAKEUP;
+ xhci_pci_try_remove_aux_device(dev);
+
xhci->xhc_state |= XHCI_STATE_REMOVING;
if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0)
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/2] hwmon: add initial support for AMD PROM21 xHCI temperature sensor
2026-05-06 20:40 ` [PATCH v2 0/2] AMD PROM21 xHCI temperature hwmon support Jihong Min
2026-05-06 20:40 ` [PATCH v2 1/2] usb: xhci-pci: add generic auxiliary device interface Jihong Min
@ 2026-05-06 20:40 ` Jihong Min
2026-05-06 21:33 ` Michal Pecio
2026-05-06 22:17 ` Randy Dunlap
1 sibling, 2 replies; 16+ messages in thread
From: Jihong Min @ 2026-05-06 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Mathias Nyman
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Mario Limonciello,
Basavaraj Natikar, linux-usb, linux-hwmon, linux-doc, linux-pci,
linux-kernel, Jihong Min
AMD PROM21 xHCI controllers expose an 8-bit temperature value
through a vendor-specific index/data register pair in the xHCI PCI
MMIO BAR region. Add an auxiliary hwmon driver for AMD 1022:43fd
controllers and bind it to the xhci_pci.hwmon auxiliary device
created by xhci-pci.
The read path verifies the parent PCI function and uses the
initialized xHCI HCD MMIO mapping. The vendor index register is at
byte offset 0x3000 from the xHCI MMIO BAR base and the vendor data
register is at byte offset 0x3008. The driver writes register selector
0x0001e520 to the index register, reads the raw temperature value from
the low 8 bits of the data register, and restores the previous index
before returning. Expose temp1_input and an xHCI label through hwmon.
Register the hwmon device under the parent PCI function so userspace
reports it as a PCI adapter, while the auxiliary driver still owns the
hwmon lifetime and unregisters it from the auxiliary remove path.
No public AMD reference is available for this value. The conversion
formula is derived from observed temperature readings:
temp[C] = raw * 0.9066 - 78.624
The temperature register did not return a valid value while the xHCI
PCI function was suspended in testing. Keep the existing behavior by
default and allow temperature reads to wake the xHCI PCI device. Add an
allow_pm_switch module parameter so users can disable that behavior;
when disabled, reads do not wake the device and return -EAGAIN if it is
suspended.
Document the supported device, register access, conversion formula,
module parameter, sysfs attributes, and sysfs lookup method.
Assisted-by: Codex:gpt-5.5
Signed-off-by: Jihong Min <hurryman2212@gmail.com>
---
Documentation/hwmon/index.rst | 1 +
Documentation/hwmon/prom21-hwmon.rst | 84 ++++++++
drivers/hwmon/Kconfig | 11 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/prom21-hwmon.c | 300 +++++++++++++++++++++++++++
5 files changed, 397 insertions(+)
create mode 100644 Documentation/hwmon/prom21-hwmon.rst
create mode 100644 drivers/hwmon/prom21-hwmon.c
diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 8b655e5d6b68..0d85b78596cf 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -215,6 +215,7 @@ Hardware Monitoring Kernel Drivers
peci-dimmtemp
pmbus
powerz
+ prom21-hwmon
powr1220
pt5161l
pxe1610
diff --git a/Documentation/hwmon/prom21-hwmon.rst b/Documentation/hwmon/prom21-hwmon.rst
new file mode 100644
index 000000000000..965a50595b47
--- /dev/null
+++ b/Documentation/hwmon/prom21-hwmon.rst
@@ -0,0 +1,84 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver prom21-hwmon
+==========================
+
+Supported chips:
+
+ * AMD PROM21 xHCI
+
+ Prefix: 'prom21_hwmon'
+
+ PCI ID: 1022:43fd
+
+Author:
+
+ - Jihong Min <hurryman2212@gmail.com>
+
+Description
+-----------
+
+This driver exposes the temperature sensor in AMD PROM21 xHCI controllers.
+
+The driver binds to an auxiliary device created by the xHCI PCI driver for
+supported controllers. The sensor value is accessed through a vendor-specific
+index/data register pair in the controller's PCI MMIO BAR.
+
+Since the xHCI controllers are integrated in PROM21, this temperature can also
+be used as a monitor for a temperature close to the AMD chipset temperature.
+
+Register access
+---------------
+
+The temperature value is read through a vendor-specific index/data register
+pair in the xHCI PCI MMIO BAR. The driver uses the following byte offsets from
+the MMIO BAR base:
+
+======================= =====================================================
+0x3000 Vendor index register
+0x3008 Vendor data register
+======================= =====================================================
+
+The driver saves the current vendor index register value, writes the
+temperature selector ``0x0001e520`` to the vendor index register, reads the
+vendor data register, and restores the previous vendor index value before
+returning. The raw temperature value is the low 8 bits of the vendor data
+register value.
+
+No public AMD reference is available for the raw value. The temperature
+conversion formula is derived from observed PROM21 xHCI temperature readings:
+
+ temp[C] = raw * 0.9066 - 78.624
+
+Module parameters
+-----------------
+
+allow_pm_switch: bool
+ Allow temperature reads to wake the xHCI PCI device. This is enabled by
+ default. If disabled, the driver does not wake the xHCI PCI device from a
+ temperature read. It reads the temperature only when the device is not
+ suspended. A read from a suspended device returns ``-EAGAIN``.
+
+Sysfs entries
+-------------
+
+======================= =====================================================
+temp1_input Temperature in millidegrees Celsius
+temp1_label "xHCI"
+======================= =====================================================
+
+The hwmon device name is ``prom21_hwmon``. The sysfs path depends on the hwmon
+device number assigned by the kernel. Userspace can locate the device by
+matching the ``name`` attribute:
+
+.. code-block:: sh
+
+ for hwmon in /sys/class/hwmon/hwmon*; do
+ [ "$(cat "$hwmon/name")" = "prom21_hwmon" ] || continue
+ cat "$hwmon/temp1_label"
+ cat "$hwmon/temp1_input"
+ done
+
+``temp1_input`` reports millidegrees Celsius, so a value of ``50113`` means
+50.113 degrees Celsius. If the raw register value is invalid, ``temp1_input``
+returns ``-ENODATA``.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 14e4cea48acc..da2266807f06 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -940,6 +940,17 @@ config SENSORS_POWERZ
This driver can also be built as a module. If so, the module
will be called powerz.
+config SENSORS_PROM21
+ tristate "AMD PROM21 xHCI temperature sensor"
+ depends on USB_XHCI_PCI
+ select USB_XHCI_PCI_AUXDEV
+ help
+ If you say yes here you get support for the AMD PROM21 xHCI
+ temperature sensor.
+
+ This driver can also be built as a module. If so, the module
+ will be called prom21-hwmon.
+
config SENSORS_POWR1220
tristate "Lattice POWR1220 Power Monitoring"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 4788996aa137..7693ed3b3f72 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -196,6 +196,7 @@ obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
obj-$(CONFIG_SENSORS_POWERZ) += powerz.o
obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o
+obj-$(CONFIG_SENSORS_PROM21) += prom21-hwmon.o
obj-$(CONFIG_SENSORS_PT5161L) += pt5161l.o
obj-$(CONFIG_SENSORS_PWM_FAN) += pwm-fan.o
obj-$(CONFIG_SENSORS_QNAP_MCU_HWMON) += qnap-mcu-hwmon.o
diff --git a/drivers/hwmon/prom21-hwmon.c b/drivers/hwmon/prom21-hwmon.c
new file mode 100644
index 000000000000..179d2f206c7d
--- /dev/null
+++ b/drivers/hwmon/prom21-hwmon.c
@@ -0,0 +1,300 @@
+// 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/auxiliary_bus.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/hwmon.h>
+#include <linux/io.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pci.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+
+#define PCI_DEVICE_ID_AMD_PROM21_XHCI 0x43fd
+
+#define PROM21_INDEX 0x3000
+#define PROM21_DATA 0x3008
+#define PROM21_TEMP_REG 0x0001e520
+
+#define PROM21_HWMON_NAME "prom21_hwmon"
+#define PROM21_TEMP_LABEL "xHCI"
+
+struct prom21_hwmon {
+ struct pci_dev *pdev;
+ struct device *hwmon_dev;
+ void __iomem *regs;
+ bool removing;
+ struct mutex lock; /* protects removing and the index/data registers */
+};
+
+static bool allow_pm_switch = true;
+module_param(allow_pm_switch, bool, 0444);
+MODULE_PARM_DESC(allow_pm_switch,
+ "Allow temperature reads to wake the xHCI PCI device");
+
+static void prom21_hwmon_invalidate(struct prom21_hwmon *hwmon)
+{
+ mutex_lock(&hwmon->lock);
+ hwmon->removing = true;
+ mutex_unlock(&hwmon->lock);
+}
+
+static int prom21_hwmon_pm_get(struct prom21_hwmon *hwmon, bool *pm_ref)
+{
+ struct device *dev = &hwmon->pdev->dev;
+ int ret;
+
+ *pm_ref = false;
+
+ /*
+ * PROM21 temperature register access does not return a valid value while
+ * the parent xHCI PCI function is suspended. By default, resume the xHC
+ * before touching MMIO. If allow_pm_switch=N, do not resume the xHC from
+ * a hwmon sysfs read; only read when runtime PM reports it as active, or
+ * when runtime PM is disabled and the device is not marked as suspended.
+ */
+ if (allow_pm_switch) {
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret < 0)
+ return ret;
+
+ *pm_ref = true;
+ return 0;
+ }
+
+ ret = pm_runtime_get_if_active(dev);
+ if (ret > 0) {
+ *pm_ref = true;
+ return 0;
+ }
+
+ if (!ret || pm_runtime_status_suspended(dev))
+ return -EAGAIN;
+
+ if (ret == -EINVAL)
+ return 0;
+
+ return ret;
+}
+
+/*
+ * This is not a pure MMIO read. The PROM21 vendor data register is selected
+ * by temporarily writing PROM21_TEMP_REG to the vendor index register. Keep
+ * the sequence short and restore the previous index before returning.
+ */
+static int prom21_hwmon_read_temp_raw_restore_index(struct prom21_hwmon *hwmon,
+ u8 *raw)
+{
+ struct device *dev = &hwmon->pdev->dev;
+ bool pm_ref;
+ u32 index;
+ u32 data;
+ int ret;
+
+ /*
+ * The xHCI PCI remove path destroys the auxiliary device 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 = prom21_hwmon_pm_get(hwmon, &pm_ref);
+ if (ret) {
+ mutex_unlock(&hwmon->lock);
+ return ret;
+ }
+
+ index = readl(hwmon->regs + PROM21_INDEX);
+ /* Select the PROM21 temperature register through the vendor index. */
+ writel(PROM21_TEMP_REG, hwmon->regs + PROM21_INDEX);
+ data = readl(hwmon->regs + PROM21_DATA);
+ /* Restore the previous vendor index register value. */
+ writel(index, hwmon->regs + PROM21_INDEX);
+ readl(hwmon->regs + PROM21_INDEX);
+
+ if (pm_ref) {
+ /*
+ * Use autosuspend so repeated sysfs reads do not suspend the
+ * controller immediately after each successful register access.
+ */
+ 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 prom21_hwmon_raw_to_millicelsius(u8 raw)
+{
+ /*
+ * No public AMD 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 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:
+ case hwmon_temp_label:
+ return 0444;
+ default:
+ return 0;
+ }
+}
+
+static int prom21_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ struct prom21_hwmon *hwmon = dev_get_drvdata(dev);
+ u8 raw;
+ int ret;
+
+ if (type != hwmon_temp || attr != hwmon_temp_input || channel)
+ return -EOPNOTSUPP;
+
+ ret = prom21_hwmon_read_temp_raw_restore_index(hwmon, &raw);
+ if (ret)
+ return ret;
+
+ *val = prom21_hwmon_raw_to_millicelsius(raw);
+ return 0;
+}
+
+static int prom21_hwmon_read_string(struct device *dev,
+ enum hwmon_sensor_types type, u32 attr,
+ int channel, const char **str)
+{
+ if (type != hwmon_temp || attr != hwmon_temp_label || channel)
+ return -EOPNOTSUPP;
+
+ *str = PROM21_TEMP_LABEL;
+ return 0;
+}
+
+static const struct hwmon_ops prom21_hwmon_ops = {
+ .is_visible = prom21_hwmon_is_visible,
+ .read = prom21_hwmon_read,
+ .read_string = prom21_hwmon_read_string,
+};
+
+static const struct hwmon_channel_info *const prom21_hwmon_info[] = {
+ HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_LABEL),
+ NULL,
+};
+
+static const struct hwmon_chip_info prom21_hwmon_chip_info = {
+ .ops = &prom21_hwmon_ops,
+ .info = prom21_hwmon_info,
+};
+
+static int prom21_hwmon_probe(struct auxiliary_device *auxdev,
+ const struct auxiliary_device_id *id)
+{
+ struct device *dev = &auxdev->dev;
+ struct device *parent = dev->parent;
+ struct prom21_hwmon *hwmon;
+ struct pci_dev *pdev;
+ struct usb_hcd *hcd;
+ int ret;
+
+ if (!parent || !dev_is_pci(parent))
+ return -ENODEV;
+
+ pdev = to_pci_dev(parent);
+ if (pdev->vendor != PCI_VENDOR_ID_AMD ||
+ pdev->device != PCI_DEVICE_ID_AMD_PROM21_XHCI)
+ return -ENODEV;
+
+ hcd = pci_get_drvdata(pdev);
+ if (!hcd)
+ return dev_err_probe(dev, -ENODEV,
+ "xHCI HCD data unavailable\n");
+
+ if (!hcd->regs || hcd->rsrc_len < PROM21_DATA + sizeof(u32))
+ return dev_err_probe(dev, -ENODEV, "invalid MMIO resource\n");
+
+ hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL);
+ if (!hwmon)
+ return -ENOMEM;
+
+ ret = devm_mutex_init(dev, &hwmon->lock);
+ if (ret)
+ return ret;
+
+ hwmon->pdev = pdev;
+ hwmon->regs = hcd->regs;
+ auxiliary_set_drvdata(auxdev, hwmon);
+
+ /*
+ * Use the PCI function as the hwmon parent so user space reports it as
+ * a PCI adapter. Lifetime is still owned by this auxiliary driver;
+ * remove() unregisters the hwmon device before xhci-pci tears down the
+ * HCD.
+ */
+ hwmon->hwmon_dev =
+ hwmon_device_register_with_info(&pdev->dev, PROM21_HWMON_NAME,
+ hwmon, &prom21_hwmon_chip_info,
+ NULL);
+ if (IS_ERR(hwmon->hwmon_dev))
+ return PTR_ERR(hwmon->hwmon_dev);
+
+ return 0;
+}
+
+static void prom21_hwmon_remove(struct auxiliary_device *auxdev)
+{
+ struct prom21_hwmon *hwmon = auxiliary_get_drvdata(auxdev);
+
+ if (hwmon) {
+ prom21_hwmon_invalidate(hwmon);
+ hwmon_device_unregister(hwmon->hwmon_dev);
+ }
+}
+
+static const struct auxiliary_device_id prom21_hwmon_id_table[] = {
+ { .name = "xhci_pci.hwmon" },
+ {}
+};
+MODULE_DEVICE_TABLE(auxiliary, prom21_hwmon_id_table);
+
+static struct auxiliary_driver prom21_hwmon_driver = {
+ .name = "prom21-hwmon",
+ .probe = prom21_hwmon_probe,
+ .remove = prom21_hwmon_remove,
+ .id_table = prom21_hwmon_id_table,
+};
+module_auxiliary_driver(prom21_hwmon_driver);
+
+MODULE_AUTHOR("Jihong Min <hurryman2212@gmail.com>");
+MODULE_DESCRIPTION("AMD PROM21 xHCI hwmon driver");
+MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] usb: xhci-pci: add generic auxiliary device interface
2026-05-06 20:40 ` [PATCH v2 1/2] usb: xhci-pci: add generic auxiliary device interface Jihong Min
@ 2026-05-06 20:53 ` Mario Limonciello
2026-05-06 21:09 ` Jihong Min
2026-05-06 21:15 ` Michal Pecio
1 sibling, 1 reply; 16+ messages in thread
From: Mario Limonciello @ 2026-05-06 20:53 UTC (permalink / raw)
To: Jihong Min, Greg Kroah-Hartman, Mathias Nyman
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Basavaraj Natikar,
linux-usb, linux-hwmon, linux-doc, linux-pci, linux-kernel
On 5/6/26 15:40, Jihong Min wrote:
> [You don't often get email from hurryman2212@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Some xHCI PCI controllers expose controller-specific functionality that is
> not part of generic xHCI operation and is better handled by optional child
> drivers in other subsystems. Add a small auxiliary device registration path
> for selected xHCI PCI controllers.
>
> The initial table creates an xhci_pci.hwmon auxiliary device for AMD
> 1022:43fd controllers. Store the created auxiliary device in devres so the
> xhci-pci remove path destroys it before HCD teardown. Use a
> PCI-domain-qualified id so auxiliary device names remain unique across PCI
> domains.
>
> This keeps xhci-pci responsible only for publishing selected controller
> functions while allowing subsystem-specific drivers to bind through the
> auxiliary bus.
>
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Jihong Min <hurryman2212@gmail.com>
> ---
> drivers/usb/host/Kconfig | 10 ++++
> drivers/usb/host/xhci-pci.c | 114 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 124 insertions(+)
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 0a277a07cf70..e0c2c7ac5c97 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_AUXDEV
> + bool "xHCI PCI auxiliary device support"
> + depends on USB_XHCI_PCI
> + select AUXILIARY_BUS
> + help
> + This enables xHCI PCI support for registering auxiliary devices
> + for selected controllers. It is used by optional child drivers
> + that bind to xHCI PCI controller-specific functionality through
> + the auxiliary bus.
> +
> 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/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 585b2f3117b0..1ab27d2182eb 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -8,6 +8,8 @@
> * Some code borrowed from the Linux EHCI driver.
> */
>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/device/devres.h>
> #include <linux/pci.h>
> #include <linux/slab.h>
> #include <linux/module.h>
> @@ -103,6 +105,114 @@ static int xhci_pci_run(struct usb_hcd *hcd);
> static int xhci_pci_update_hub_device(struct usb_hcd *hcd, struct usb_device *hdev,
> struct usb_tt *tt, gfp_t mem_flags);
>
> +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_AUXDEV)
> +static const struct {
> + struct pci_device_id id;
> + const char *aux_dev_name;
> +} pci_ids_have_aux[] = {
> + {
> + .id = { PCI_DEVICE(PCI_VENDOR_ID_AMD, 0x43fd) }, /* PROM21 xHCI */
> + .aux_dev_name = "hwmon",
> + },
> + { /* end: all zeroes */ }
> +};
> +
> +struct xhci_pci_aux_devres {
> + struct auxiliary_device *auxdev;
> +};
> +
> +static bool xhci_pci_aux_match_id(const struct pci_device_id *id,
> + const struct pci_dev *pdev)
> +{
> + if (id->vendor != PCI_ANY_ID && id->vendor != pdev->vendor)
> + return false;
> + if (id->device != PCI_ANY_ID && id->device != pdev->device)
> + return false;
> + if (id->subvendor != PCI_ANY_ID &&
> + id->subvendor != pdev->subsystem_vendor)
> + return false;
> + if (id->subdevice != PCI_ANY_ID &&
> + id->subdevice != pdev->subsystem_device)
> + return false;
> +
> + return !((id->class ^ pdev->class) & id->class_mask);
What's wrong with pci_match_id()?
> +}
> +
> +static const char *xhci_pci_aux_dev_name(struct pci_dev *pdev)
> +{
> + int i;
> +
> + for (i = 0; pci_ids_have_aux[i].aux_dev_name; i++) {
> + if (xhci_pci_aux_match_id(&pci_ids_have_aux[i].id, pdev))
> + return pci_ids_have_aux[i].aux_dev_name;
> + }
> +
> + return NULL;
> +}
> +
> +static void xhci_pci_aux_devres_release(struct device *dev, void *res)
> +{
> + struct xhci_pci_aux_devres *devres = res;
> +
> + if (devres->auxdev)
> + auxiliary_device_destroy(devres->auxdev);
> +}
> +
> +static void xhci_pci_try_add_aux_device(struct pci_dev *pdev)
> +{
> + struct xhci_pci_aux_devres *devres;
> + struct auxiliary_device *auxdev;
> + const char *aux_dev_name;
> +
> + aux_dev_name = xhci_pci_aux_dev_name(pdev);
> + if (!aux_dev_name)
> + return;
> +
> + devres = devres_alloc(xhci_pci_aux_devres_release, sizeof(*devres),
> + GFP_KERNEL);
> + if (!devres) {
> + dev_warn(&pdev->dev,
> + "failed to allocate auxiliary device state\n");
> + return;
> + }
> +
> + auxdev = auxiliary_device_create(&pdev->dev, KBUILD_MODNAME,
> + aux_dev_name, NULL,
> + (pci_domain_nr(pdev->bus) << 16) |
> + pci_dev_id(pdev));
> + if (!auxdev) {
> + devres_free(devres);
> + dev_warn(&pdev->dev, "failed to add %s auxiliary device\n",
> + aux_dev_name);
> + return;
> + }
> +
> + devres->auxdev = auxdev;
> + devres_add(&pdev->dev, devres);
> +}
> +
> +static void xhci_pci_try_remove_aux_device(struct pci_dev *pdev)
> +{
> + struct xhci_pci_aux_devres *devres;
> +
> + devres = devres_find(&pdev->dev, xhci_pci_aux_devres_release, NULL,
> + NULL);
> + if (!devres || !devres->auxdev)
> + return;
> +
> + auxiliary_device_destroy(devres->auxdev);
> + devres->auxdev = NULL;
> +}
> +#else
> +static inline void xhci_pci_try_add_aux_device(struct pci_dev *pdev)
> +{
> +}
> +
> +static inline void xhci_pci_try_remove_aux_device(struct pci_dev *pdev)
> +{
> +}
> +#endif
> +
> static const struct xhci_driver_overrides xhci_pci_overrides __initconst = {
> .reset = xhci_pci_setup,
> .start = xhci_pci_run,
> @@ -677,6 +787,8 @@ 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);
>
> + xhci_pci_try_add_aux_device(dev);
> +
> return 0;
>
> put_usb3_hcd:
> @@ -713,6 +825,8 @@ void xhci_pci_remove(struct pci_dev *dev)
> xhci = hcd_to_xhci(pci_get_drvdata(dev));
> set_power_d3 = xhci->quirks & XHCI_SPURIOUS_WAKEUP;
>
> + xhci_pci_try_remove_aux_device(dev);
> +
> xhci->xhc_state |= XHCI_STATE_REMOVING;
>
> if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0)
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] usb: xhci-pci: add generic auxiliary device interface
2026-05-06 20:53 ` Mario Limonciello
@ 2026-05-06 21:09 ` Jihong Min
0 siblings, 0 replies; 16+ messages in thread
From: Jihong Min @ 2026-05-06 21:09 UTC (permalink / raw)
To: Mario Limonciello, Jihong Min, Greg Kroah-Hartman, Mathias Nyman
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Basavaraj Natikar,
linux-usb, linux-hwmon, linux-doc, linux-pci, linux-kernel
I refactored the auxiliary device table to use PCI_DEVICE_DATA(), store the
auxiliary device name in driver_data, and use pci_match_id() inside
xhci_pci_aux_dev_name(). The relevant change is:
#define PCI_DEVICE_ID_AMD_PROM21_XHCI 0x43fd
...
static const struct pci_device_id pci_ids_have_aux[] = {
{ PCI_DEVICE_DATA(AMD, PROM21_XHCI, "hwmon") },
{ /* end: all zeroes */ }
};
...
static const char *xhci_pci_aux_dev_name(struct pci_dev *pdev)
{
const struct pci_device_id *id;
id = pci_match_id(pci_ids_have_aux, pdev);
if (!id)
return NULL;
return (const char *)id->driver_data;
}
I will include this in v3.
Thank you,
Jihong Min
On 5/7/26 05:53, Mario Limonciello wrote:
>
>
> On 5/6/26 15:40, Jihong Min wrote:
>> [You don't often get email from hurryman2212@gmail.com. Learn why
>> this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> Some xHCI PCI controllers expose controller-specific functionality
>> that is
>> not part of generic xHCI operation and is better handled by optional
>> child
>> drivers in other subsystems. Add a small auxiliary device
>> registration path
>> for selected xHCI PCI controllers.
>>
>> The initial table creates an xhci_pci.hwmon auxiliary device for AMD
>> 1022:43fd controllers. Store the created auxiliary device in devres
>> so the
>> xhci-pci remove path destroys it before HCD teardown. Use a
>> PCI-domain-qualified id so auxiliary device names remain unique
>> across PCI
>> domains.
>>
>> This keeps xhci-pci responsible only for publishing selected controller
>> functions while allowing subsystem-specific drivers to bind through the
>> auxiliary bus.
>>
>> Assisted-by: Codex:gpt-5.5
>> Signed-off-by: Jihong Min <hurryman2212@gmail.com>
>> ---
>> drivers/usb/host/Kconfig | 10 ++++
>> drivers/usb/host/xhci-pci.c | 114 ++++++++++++++++++++++++++++++++++++
>> 2 files changed, 124 insertions(+)
>>
>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>> index 0a277a07cf70..e0c2c7ac5c97 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_AUXDEV
>> + bool "xHCI PCI auxiliary device support"
>> + depends on USB_XHCI_PCI
>> + select AUXILIARY_BUS
>> + help
>> + This enables xHCI PCI support for registering auxiliary
>> devices
>> + for selected controllers. It is used by optional child drivers
>> + that bind to xHCI PCI controller-specific functionality
>> through
>> + the auxiliary bus.
>> +
>> 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/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>> index 585b2f3117b0..1ab27d2182eb 100644
>> --- a/drivers/usb/host/xhci-pci.c
>> +++ b/drivers/usb/host/xhci-pci.c
>> @@ -8,6 +8,8 @@
>> * Some code borrowed from the Linux EHCI driver.
>> */
>>
>> +#include <linux/auxiliary_bus.h>
>> +#include <linux/device/devres.h>
>> #include <linux/pci.h>
>> #include <linux/slab.h>
>> #include <linux/module.h>
>> @@ -103,6 +105,114 @@ static int xhci_pci_run(struct usb_hcd *hcd);
>> static int xhci_pci_update_hub_device(struct usb_hcd *hcd, struct
>> usb_device *hdev,
>> struct usb_tt *tt, gfp_t
>> mem_flags);
>>
>> +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_AUXDEV)
>> +static const struct {
>> + struct pci_device_id id;
>> + const char *aux_dev_name;
>> +} pci_ids_have_aux[] = {
>> + {
>> + .id = { PCI_DEVICE(PCI_VENDOR_ID_AMD, 0x43fd) }, /*
>> PROM21 xHCI */
>> + .aux_dev_name = "hwmon",
>> + },
>> + { /* end: all zeroes */ }
>> +};
>> +
>> +struct xhci_pci_aux_devres {
>> + struct auxiliary_device *auxdev;
>> +};
>> +
>> +static bool xhci_pci_aux_match_id(const struct pci_device_id *id,
>> + const struct pci_dev *pdev)
>> +{
>> + if (id->vendor != PCI_ANY_ID && id->vendor != pdev->vendor)
>> + return false;
>> + if (id->device != PCI_ANY_ID && id->device != pdev->device)
>> + return false;
>> + if (id->subvendor != PCI_ANY_ID &&
>> + id->subvendor != pdev->subsystem_vendor)
>> + return false;
>> + if (id->subdevice != PCI_ANY_ID &&
>> + id->subdevice != pdev->subsystem_device)
>> + return false;
>> +
>> + return !((id->class ^ pdev->class) & id->class_mask);
>
> What's wrong with pci_match_id()?
>
>> +}
>> +
>> +static const char *xhci_pci_aux_dev_name(struct pci_dev *pdev)
>> +{
>> + int i;
>> +
>> + for (i = 0; pci_ids_have_aux[i].aux_dev_name; i++) {
>> + if (xhci_pci_aux_match_id(&pci_ids_have_aux[i].id,
>> pdev))
>> + return pci_ids_have_aux[i].aux_dev_name;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static void xhci_pci_aux_devres_release(struct device *dev, void *res)
>> +{
>> + struct xhci_pci_aux_devres *devres = res;
>> +
>> + if (devres->auxdev)
>> + auxiliary_device_destroy(devres->auxdev);
>> +}
>> +
>> +static void xhci_pci_try_add_aux_device(struct pci_dev *pdev)
>> +{
>> + struct xhci_pci_aux_devres *devres;
>> + struct auxiliary_device *auxdev;
>> + const char *aux_dev_name;
>> +
>> + aux_dev_name = xhci_pci_aux_dev_name(pdev);
>> + if (!aux_dev_name)
>> + return;
>> +
>> + devres = devres_alloc(xhci_pci_aux_devres_release,
>> sizeof(*devres),
>> + GFP_KERNEL);
>> + if (!devres) {
>> + dev_warn(&pdev->dev,
>> + "failed to allocate auxiliary device state\n");
>> + return;
>> + }
>> +
>> + auxdev = auxiliary_device_create(&pdev->dev, KBUILD_MODNAME,
>> + aux_dev_name, NULL,
>> + (pci_domain_nr(pdev->bus) << 16) |
>> + pci_dev_id(pdev));
>> + if (!auxdev) {
>> + devres_free(devres);
>> + dev_warn(&pdev->dev, "failed to add %s auxiliary
>> device\n",
>> + aux_dev_name);
>> + return;
>> + }
>> +
>> + devres->auxdev = auxdev;
>> + devres_add(&pdev->dev, devres);
>> +}
>> +
>> +static void xhci_pci_try_remove_aux_device(struct pci_dev *pdev)
>> +{
>> + struct xhci_pci_aux_devres *devres;
>> +
>> + devres = devres_find(&pdev->dev, xhci_pci_aux_devres_release,
>> NULL,
>> + NULL);
>> + if (!devres || !devres->auxdev)
>> + return;
>> +
>> + auxiliary_device_destroy(devres->auxdev);
>> + devres->auxdev = NULL;
>> +}
>> +#else
>> +static inline void xhci_pci_try_add_aux_device(struct pci_dev *pdev)
>> +{
>> +}
>> +
>> +static inline void xhci_pci_try_remove_aux_device(struct pci_dev *pdev)
>> +{
>> +}
>> +#endif
>> +
>> static const struct xhci_driver_overrides xhci_pci_overrides
>> __initconst = {
>> .reset = xhci_pci_setup,
>> .start = xhci_pci_run,
>> @@ -677,6 +787,8 @@ 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);
>>
>> + xhci_pci_try_add_aux_device(dev);
>> +
>> return 0;
>>
>> put_usb3_hcd:
>> @@ -713,6 +825,8 @@ void xhci_pci_remove(struct pci_dev *dev)
>> xhci = hcd_to_xhci(pci_get_drvdata(dev));
>> set_power_d3 = xhci->quirks & XHCI_SPURIOUS_WAKEUP;
>>
>> + xhci_pci_try_remove_aux_device(dev);
>> +
>> xhci->xhc_state |= XHCI_STATE_REMOVING;
>>
>> if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0)
>> --
>> 2.53.0
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] usb: xhci-pci: add generic auxiliary device interface
2026-05-06 20:40 ` [PATCH v2 1/2] usb: xhci-pci: add generic auxiliary device interface Jihong Min
2026-05-06 20:53 ` Mario Limonciello
@ 2026-05-06 21:15 ` Michal Pecio
2026-05-06 21:42 ` Jihong Min
1 sibling, 1 reply; 16+ messages in thread
From: Michal Pecio @ 2026-05-06 21:15 UTC (permalink / raw)
To: Jihong Min
Cc: Greg Kroah-Hartman, Mathias Nyman, Guenter Roeck, Jonathan Corbet,
Shuah Khan, Mario Limonciello, Basavaraj Natikar, linux-usb,
linux-hwmon, linux-doc, linux-pci, linux-kernel
On Thu, 7 May 2026 05:40:33 +0900, Jihong Min wrote:
> Some xHCI PCI controllers expose controller-specific functionality that is
> not part of generic xHCI operation and is better handled by optional child
> drivers in other subsystems. Add a small auxiliary device registration path
> for selected xHCI PCI controllers.
>
> The initial table creates an xhci_pci.hwmon auxiliary device for AMD
> 1022:43fd controllers. Store the created auxiliary device in devres so the
> xhci-pci remove path destroys it before HCD teardown. Use a
> PCI-domain-qualified id so auxiliary device names remain unique across PCI
> domains.
>
> This keeps xhci-pci responsible only for publishing selected controller
> functions while allowing subsystem-specific drivers to bind through the
> auxiliary bus.
>
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Jihong Min <hurryman2212@gmail.com>
> ---
> drivers/usb/host/Kconfig | 10 ++++
> drivers/usb/host/xhci-pci.c | 114 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 124 insertions(+)
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 0a277a07cf70..e0c2c7ac5c97 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_AUXDEV
> + bool "xHCI PCI auxiliary device support"
> + depends on USB_XHCI_PCI
> + select AUXILIARY_BUS
> + help
> + This enables xHCI PCI support for registering auxiliary devices
> + for selected controllers. It is used by optional child drivers
> + that bind to xHCI PCI controller-specific functionality through
> + the auxiliary bus.
> +
> 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/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 585b2f3117b0..1ab27d2182eb 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -8,6 +8,8 @@
> * Some code borrowed from the Linux EHCI driver.
> */
>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/device/devres.h>
> #include <linux/pci.h>
> #include <linux/slab.h>
> #include <linux/module.h>
> @@ -103,6 +105,114 @@ static int xhci_pci_run(struct usb_hcd *hcd);
> static int xhci_pci_update_hub_device(struct usb_hcd *hcd, struct usb_device *hdev,
> struct usb_tt *tt, gfp_t mem_flags);
>
> +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_AUXDEV)
> +static const struct {
> + struct pci_device_id id;
> + const char *aux_dev_name;
> +} pci_ids_have_aux[] = {
> + {
> + .id = { PCI_DEVICE(PCI_VENDOR_ID_AMD, 0x43fd) }, /* PROM21 xHCI */
> + .aux_dev_name = "hwmon",
> + },
> + { /* end: all zeroes */ }
> +};
> +
> +struct xhci_pci_aux_devres {
> + struct auxiliary_device *auxdev;
> +};
> +
> +static bool xhci_pci_aux_match_id(const struct pci_device_id *id,
> + const struct pci_dev *pdev)
> +{
> + if (id->vendor != PCI_ANY_ID && id->vendor != pdev->vendor)
> + return false;
> + if (id->device != PCI_ANY_ID && id->device != pdev->device)
> + return false;
> + if (id->subvendor != PCI_ANY_ID &&
> + id->subvendor != pdev->subsystem_vendor)
> + return false;
> + if (id->subdevice != PCI_ANY_ID &&
> + id->subdevice != pdev->subsystem_device)
> + return false;
> +
> + return !((id->class ^ pdev->class) & id->class_mask);
> +}
> +
> +static const char *xhci_pci_aux_dev_name(struct pci_dev *pdev)
> +{
> + int i;
> +
> + for (i = 0; pci_ids_have_aux[i].aux_dev_name; i++) {
> + if (xhci_pci_aux_match_id(&pci_ids_have_aux[i].id, pdev))
> + return pci_ids_have_aux[i].aux_dev_name;
> + }
> +
> + return NULL;
> +}
> +
> +static void xhci_pci_aux_devres_release(struct device *dev, void *res)
> +{
> + struct xhci_pci_aux_devres *devres = res;
> +
> + if (devres->auxdev)
> + auxiliary_device_destroy(devres->auxdev);
> +}
> +
> +static void xhci_pci_try_add_aux_device(struct pci_dev *pdev)
> +{
> + struct xhci_pci_aux_devres *devres;
> + struct auxiliary_device *auxdev;
> + const char *aux_dev_name;
> +
> + aux_dev_name = xhci_pci_aux_dev_name(pdev);
> + if (!aux_dev_name)
> + return;
> +
> + devres = devres_alloc(xhci_pci_aux_devres_release, sizeof(*devres),
> + GFP_KERNEL);
> + if (!devres) {
> + dev_warn(&pdev->dev,
> + "failed to allocate auxiliary device state\n");
> + return;
> + }
> +
> + auxdev = auxiliary_device_create(&pdev->dev, KBUILD_MODNAME,
> + aux_dev_name, NULL,
> + (pci_domain_nr(pdev->bus) << 16) |
> + pci_dev_id(pdev));
> + if (!auxdev) {
> + devres_free(devres);
> + dev_warn(&pdev->dev, "failed to add %s auxiliary device\n",
> + aux_dev_name);
> + return;
> + }
> +
> + devres->auxdev = auxdev;
> + devres_add(&pdev->dev, devres);
> +}
> +
> +static void xhci_pci_try_remove_aux_device(struct pci_dev *pdev)
> +{
> + struct xhci_pci_aux_devres *devres;
> +
> + devres = devres_find(&pdev->dev, xhci_pci_aux_devres_release, NULL,
> + NULL);
> + if (!devres || !devres->auxdev)
> + return;
> +
> + auxiliary_device_destroy(devres->auxdev);
> + devres->auxdev = NULL;
> +}
> +#else
> +static inline void xhci_pci_try_add_aux_device(struct pci_dev *pdev)
> +{
> +}
> +
> +static inline void xhci_pci_try_remove_aux_device(struct pci_dev *pdev)
> +{
> +}
> +#endif
> +
> static const struct xhci_driver_overrides xhci_pci_overrides __initconst = {
> .reset = xhci_pci_setup,
> .start = xhci_pci_run,
> @@ -677,6 +787,8 @@ 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);
>
> + xhci_pci_try_add_aux_device(dev);
Is it considered acceptable to add
if (IS_ENABLED(CONFIG_USB_XHCI_PCI_AUXDEV))
before this call, remove #ifdef around the definitions of the auxdev
functions and rely on dead code elimination to clean them up?
We already have a similar trick with CONFIG_USB_XHCI_PCI_RENESAS here
and it seemed to be working, though the amount of conditional code is
much lower and so is the potential risk.
The reason for doing it this way was because Greg doesn't like #ifdefs
in .c files, and neither do static analyzers like them.
> +
> return 0;
>
> put_usb3_hcd:
> @@ -713,6 +825,8 @@ void xhci_pci_remove(struct pci_dev *dev)
> xhci = hcd_to_xhci(pci_get_drvdata(dev));
> set_power_d3 = xhci->quirks & XHCI_SPURIOUS_WAKEUP;
>
> + xhci_pci_try_remove_aux_device(dev);
> +
> xhci->xhc_state |= XHCI_STATE_REMOVING;
>
> if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0)
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/2] hwmon: add initial support for AMD PROM21 xHCI temperature sensor
2026-05-06 20:40 ` [PATCH v2 2/2] hwmon: add initial support for AMD PROM21 xHCI temperature sensor Jihong Min
@ 2026-05-06 21:33 ` Michal Pecio
2026-05-06 21:36 ` Mario Limonciello
2026-05-06 22:41 ` Jihong Min
2026-05-06 22:17 ` Randy Dunlap
1 sibling, 2 replies; 16+ messages in thread
From: Michal Pecio @ 2026-05-06 21:33 UTC (permalink / raw)
To: Jihong Min
Cc: Greg Kroah-Hartman, Mathias Nyman, Guenter Roeck, Jonathan Corbet,
Shuah Khan, Mario Limonciello, Basavaraj Natikar, linux-usb,
linux-hwmon, linux-doc, linux-pci, linux-kernel
On Thu, 7 May 2026 05:40:34 +0900, Jihong Min wrote:
> AMD PROM21 xHCI controllers expose an 8-bit temperature value
I think this commit message and certainly the Kconfig help text should
include full name of the chip and perhaps its official marketing names
too, so that people better understand what hardware is supported.
So: "AMD Promontory 21 chipset" and "AM5 6xx/8xx series chipsets", or
whatever they are called by AMD and motherboard vendors.
> through a vendor-specific index/data register pair in the xHCI PCI
> MMIO BAR region. Add an auxiliary hwmon driver for AMD 1022:43fd
> controllers and bind it to the xhci_pci.hwmon auxiliary device
> created by xhci-pci.
>
> The read path verifies the parent PCI function and uses the
> initialized xHCI HCD MMIO mapping. The vendor index register is at
> byte offset 0x3000 from the xHCI MMIO BAR base and the vendor data
> register is at byte offset 0x3008. The driver writes register selector
> 0x0001e520 to the index register, reads the raw temperature value from
> the low 8 bits of the data register, and restores the previous index
> before returning. Expose temp1_input and an xHCI label through hwmon.
>
> Register the hwmon device under the parent PCI function so userspace
> reports it as a PCI adapter, while the auxiliary driver still owns the
> hwmon lifetime and unregisters it from the auxiliary remove path.
>
> No public AMD reference is available for this value.
Is there any documentation of the index/data registers themselves?
Any potential danger that something else (FW? SMM?) uses them too?
> The conversion formula is derived from observed temperature readings:
>
> temp[C] = raw * 0.9066 - 78.624
Could make sense to describe methodology, particularly in case some
people would come and question the formula.
How was actual chip temperature measured?
Was the output compared with any other (Windows?) utilities?
People will be comparing these results and possibly trying to draw
some conclusions, like OMG Linux runs this chip 8°C hotter, should
I demand a full refund of my free Ubuntu download????!!!?
> The temperature register did not return a valid value while the xHCI
> PCI function was suspended in testing. Keep the existing behavior by
> default and allow temperature reads to wake the xHCI PCI device. Add an
> allow_pm_switch module parameter so users can disable that behavior;
> when disabled, reads do not wake the device and return -EAGAIN if it is
> suspended.
Is such behavior useful?
Maybe the driver could just disable runtime PM while it's loaded.
>
> Document the supported device, register access, conversion formula,
> module parameter, sysfs attributes, and sysfs lookup method.
>
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Jihong Min <hurryman2212@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/2] hwmon: add initial support for AMD PROM21 xHCI temperature sensor
2026-05-06 21:33 ` Michal Pecio
@ 2026-05-06 21:36 ` Mario Limonciello
2026-05-06 22:41 ` Jihong Min
1 sibling, 0 replies; 16+ messages in thread
From: Mario Limonciello @ 2026-05-06 21:36 UTC (permalink / raw)
To: Michal Pecio, Jihong Min
Cc: Greg Kroah-Hartman, Mathias Nyman, Guenter Roeck, Jonathan Corbet,
Shuah Khan, Basavaraj Natikar, linux-usb, linux-hwmon, linux-doc,
linux-pci, linux-kernel
On 5/6/26 16:33, Michal Pecio wrote:
> On Thu, 7 May 2026 05:40:34 +0900, Jihong Min wrote:
>> AMD PROM21 xHCI controllers expose an 8-bit temperature value
>
> I think this commit message and certainly the Kconfig help text should
> include full name of the chip and perhaps its official marketing names
> too, so that people better understand what hardware is supported.
>
> So: "AMD Promontory 21 chipset" and "AM5 6xx/8xx series chipsets", or
> whatever they are called by AMD and motherboard vendors.
>
>> through a vendor-specific index/data register pair in the xHCI PCI
>> MMIO BAR region. Add an auxiliary hwmon driver for AMD 1022:43fd
>> controllers and bind it to the xhci_pci.hwmon auxiliary device
>> created by xhci-pci.
>>
>> The read path verifies the parent PCI function and uses the
>> initialized xHCI HCD MMIO mapping. The vendor index register is at
>> byte offset 0x3000 from the xHCI MMIO BAR base and the vendor data
>> register is at byte offset 0x3008. The driver writes register selector
>> 0x0001e520 to the index register, reads the raw temperature value from
>> the low 8 bits of the data register, and restores the previous index
>> before returning. Expose temp1_input and an xHCI label through hwmon.
>>
>> Register the hwmon device under the parent PCI function so userspace
>> reports it as a PCI adapter, while the auxiliary driver still owns the
>> hwmon lifetime and unregisters it from the auxiliary remove path.
>>
>> No public AMD reference is available for this value.
>
> Is there any documentation of the index/data registers themselves?
>
> Any potential danger that something else (FW? SMM?) uses them too?
>
>> The conversion formula is derived from observed temperature readings:
>>
>> temp[C] = raw * 0.9066 - 78.624
>
> Could make sense to describe methodology, particularly in case some
> people would come and question the formula.
>
> How was actual chip temperature measured?
> Was the output compared with any other (Windows?) utilities?
>
> People will be comparing these results and possibly trying to draw
> some conclusions, like OMG Linux runs this chip 8°C hotter, should
> I demand a full refund of my free Ubuntu download????!!!?
>
>> The temperature register did not return a valid value while the xHCI
>> PCI function was suspended in testing. Keep the existing behavior by
>> default and allow temperature reads to wake the xHCI PCI device. Add an
>> allow_pm_switch module parameter so users can disable that behavior;
>> when disabled, reads do not wake the device and return -EAGAIN if it is
>> suspended.
>
> Is such behavior useful?
>
> Maybe the driver could just disable runtime PM while it's loaded.
I'd encourage what we do in amdgpu for dGPUs. The hwmon files will
return an error code (I forget which code) when the device is in runtime
PM when called. Don't explicitly wake it otherwise.
This prevents someone installing a sensor monitoring application and
that application "being the only thing" keeping the dGPU awake. If it's
awake already for other reasons (like being used) then return valid data
to the applications.
>
>>
>> Document the supported device, register access, conversion formula,
>> module parameter, sysfs attributes, and sysfs lookup method.
>>
>> Assisted-by: Codex:gpt-5.5
>> Signed-off-by: Jihong Min <hurryman2212@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] usb: xhci-pci: add generic auxiliary device interface
2026-05-06 21:15 ` Michal Pecio
@ 2026-05-06 21:42 ` Jihong Min
0 siblings, 0 replies; 16+ messages in thread
From: Jihong Min @ 2026-05-06 21:42 UTC (permalink / raw)
To: Michal Pecio, Jihong Min
Cc: Greg Kroah-Hartman, Mathias Nyman, Guenter Roeck, Jonathan Corbet,
Shuah Khan, Mario Limonciello, Basavaraj Natikar, linux-usb,
linux-hwmon, linux-doc, linux-pci, linux-kernel
Thank you.
I've changed the code by removing the preprocessor macros and protecting
the two entry points with `if (IS_ENABLED(CONFIG_USB_XHCI_PCI_AUXDEV))`,
for v3.
Jihong Min
On 5/7/26 06:15, Michal Pecio wrote:
> On Thu, 7 May 2026 05:40:33 +0900, Jihong Min wrote:
>> Some xHCI PCI controllers expose controller-specific functionality that is
>> not part of generic xHCI operation and is better handled by optional child
>> drivers in other subsystems. Add a small auxiliary device registration path
>> for selected xHCI PCI controllers.
>>
>> The initial table creates an xhci_pci.hwmon auxiliary device for AMD
>> 1022:43fd controllers. Store the created auxiliary device in devres so the
>> xhci-pci remove path destroys it before HCD teardown. Use a
>> PCI-domain-qualified id so auxiliary device names remain unique across PCI
>> domains.
>>
>> This keeps xhci-pci responsible only for publishing selected controller
>> functions while allowing subsystem-specific drivers to bind through the
>> auxiliary bus.
>>
>> Assisted-by: Codex:gpt-5.5
>> Signed-off-by: Jihong Min <hurryman2212@gmail.com>
>> ---
>> drivers/usb/host/Kconfig | 10 ++++
>> drivers/usb/host/xhci-pci.c | 114 ++++++++++++++++++++++++++++++++++++
>> 2 files changed, 124 insertions(+)
>>
>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>> index 0a277a07cf70..e0c2c7ac5c97 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_AUXDEV
>> + bool "xHCI PCI auxiliary device support"
>> + depends on USB_XHCI_PCI
>> + select AUXILIARY_BUS
>> + help
>> + This enables xHCI PCI support for registering auxiliary devices
>> + for selected controllers. It is used by optional child drivers
>> + that bind to xHCI PCI controller-specific functionality through
>> + the auxiliary bus.
>> +
>> 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/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>> index 585b2f3117b0..1ab27d2182eb 100644
>> --- a/drivers/usb/host/xhci-pci.c
>> +++ b/drivers/usb/host/xhci-pci.c
>> @@ -8,6 +8,8 @@
>> * Some code borrowed from the Linux EHCI driver.
>> */
>>
>> +#include <linux/auxiliary_bus.h>
>> +#include <linux/device/devres.h>
>> #include <linux/pci.h>
>> #include <linux/slab.h>
>> #include <linux/module.h>
>> @@ -103,6 +105,114 @@ static int xhci_pci_run(struct usb_hcd *hcd);
>> static int xhci_pci_update_hub_device(struct usb_hcd *hcd, struct usb_device *hdev,
>> struct usb_tt *tt, gfp_t mem_flags);
>>
>> +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_AUXDEV)
>> +static const struct {
>> + struct pci_device_id id;
>> + const char *aux_dev_name;
>> +} pci_ids_have_aux[] = {
>> + {
>> + .id = { PCI_DEVICE(PCI_VENDOR_ID_AMD, 0x43fd) }, /* PROM21 xHCI */
>> + .aux_dev_name = "hwmon",
>> + },
>> + { /* end: all zeroes */ }
>> +};
>> +
>> +struct xhci_pci_aux_devres {
>> + struct auxiliary_device *auxdev;
>> +};
>> +
>> +static bool xhci_pci_aux_match_id(const struct pci_device_id *id,
>> + const struct pci_dev *pdev)
>> +{
>> + if (id->vendor != PCI_ANY_ID && id->vendor != pdev->vendor)
>> + return false;
>> + if (id->device != PCI_ANY_ID && id->device != pdev->device)
>> + return false;
>> + if (id->subvendor != PCI_ANY_ID &&
>> + id->subvendor != pdev->subsystem_vendor)
>> + return false;
>> + if (id->subdevice != PCI_ANY_ID &&
>> + id->subdevice != pdev->subsystem_device)
>> + return false;
>> +
>> + return !((id->class ^ pdev->class) & id->class_mask);
>> +}
>> +
>> +static const char *xhci_pci_aux_dev_name(struct pci_dev *pdev)
>> +{
>> + int i;
>> +
>> + for (i = 0; pci_ids_have_aux[i].aux_dev_name; i++) {
>> + if (xhci_pci_aux_match_id(&pci_ids_have_aux[i].id, pdev))
>> + return pci_ids_have_aux[i].aux_dev_name;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static void xhci_pci_aux_devres_release(struct device *dev, void *res)
>> +{
>> + struct xhci_pci_aux_devres *devres = res;
>> +
>> + if (devres->auxdev)
>> + auxiliary_device_destroy(devres->auxdev);
>> +}
>> +
>> +static void xhci_pci_try_add_aux_device(struct pci_dev *pdev)
>> +{
>> + struct xhci_pci_aux_devres *devres;
>> + struct auxiliary_device *auxdev;
>> + const char *aux_dev_name;
>> +
>> + aux_dev_name = xhci_pci_aux_dev_name(pdev);
>> + if (!aux_dev_name)
>> + return;
>> +
>> + devres = devres_alloc(xhci_pci_aux_devres_release, sizeof(*devres),
>> + GFP_KERNEL);
>> + if (!devres) {
>> + dev_warn(&pdev->dev,
>> + "failed to allocate auxiliary device state\n");
>> + return;
>> + }
>> +
>> + auxdev = auxiliary_device_create(&pdev->dev, KBUILD_MODNAME,
>> + aux_dev_name, NULL,
>> + (pci_domain_nr(pdev->bus) << 16) |
>> + pci_dev_id(pdev));
>> + if (!auxdev) {
>> + devres_free(devres);
>> + dev_warn(&pdev->dev, "failed to add %s auxiliary device\n",
>> + aux_dev_name);
>> + return;
>> + }
>> +
>> + devres->auxdev = auxdev;
>> + devres_add(&pdev->dev, devres);
>> +}
>> +
>> +static void xhci_pci_try_remove_aux_device(struct pci_dev *pdev)
>> +{
>> + struct xhci_pci_aux_devres *devres;
>> +
>> + devres = devres_find(&pdev->dev, xhci_pci_aux_devres_release, NULL,
>> + NULL);
>> + if (!devres || !devres->auxdev)
>> + return;
>> +
>> + auxiliary_device_destroy(devres->auxdev);
>> + devres->auxdev = NULL;
>> +}
>> +#else
>> +static inline void xhci_pci_try_add_aux_device(struct pci_dev *pdev)
>> +{
>> +}
>> +
>> +static inline void xhci_pci_try_remove_aux_device(struct pci_dev *pdev)
>> +{
>> +}
>> +#endif
>> +
>> static const struct xhci_driver_overrides xhci_pci_overrides __initconst = {
>> .reset = xhci_pci_setup,
>> .start = xhci_pci_run,
>> @@ -677,6 +787,8 @@ 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);
>>
>> + xhci_pci_try_add_aux_device(dev);
> Is it considered acceptable to add
>
> if (IS_ENABLED(CONFIG_USB_XHCI_PCI_AUXDEV))
>
> before this call, remove #ifdef around the definitions of the auxdev
> functions and rely on dead code elimination to clean them up?
>
> We already have a similar trick with CONFIG_USB_XHCI_PCI_RENESAS here
> and it seemed to be working, though the amount of conditional code is
> much lower and so is the potential risk.
>
> The reason for doing it this way was because Greg doesn't like #ifdefs
> in .c files, and neither do static analyzers like them.
>
>> +
>> return 0;
>>
>> put_usb3_hcd:
>> @@ -713,6 +825,8 @@ void xhci_pci_remove(struct pci_dev *dev)
>> xhci = hcd_to_xhci(pci_get_drvdata(dev));
>> set_power_d3 = xhci->quirks & XHCI_SPURIOUS_WAKEUP;
>>
>> + xhci_pci_try_remove_aux_device(dev);
>> +
>> xhci->xhc_state |= XHCI_STATE_REMOVING;
>>
>> if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0)
>> --
>> 2.53.0
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/2] hwmon: add initial support for AMD PROM21 xHCI temperature sensor
2026-05-06 20:40 ` [PATCH v2 2/2] hwmon: add initial support for AMD PROM21 xHCI temperature sensor Jihong Min
2026-05-06 21:33 ` Michal Pecio
@ 2026-05-06 22:17 ` Randy Dunlap
2026-05-06 22:52 ` Jihong Min
1 sibling, 1 reply; 16+ messages in thread
From: Randy Dunlap @ 2026-05-06 22:17 UTC (permalink / raw)
To: Jihong Min, Greg Kroah-Hartman, Mathias Nyman
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Mario Limonciello,
Basavaraj Natikar, linux-usb, linux-hwmon, linux-doc, linux-pci,
linux-kernel
On 5/6/26 1:40 PM, Jihong Min wrote:
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 8b655e5d6b68..0d85b78596cf 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -215,6 +215,7 @@ Hardware Monitoring Kernel Drivers
> peci-dimmtemp
> pmbus
> powerz
> + prom21-hwmon
> powr1220
> pt5161l
> pxe1610
It sorta looks like these entries are supposed to be maintained in alphabetical
order, but that new entry is not.
--
~Randy
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/2] hwmon: add initial support for AMD PROM21 xHCI temperature sensor
2026-05-06 21:33 ` Michal Pecio
2026-05-06 21:36 ` Mario Limonciello
@ 2026-05-06 22:41 ` Jihong Min
1 sibling, 0 replies; 16+ messages in thread
From: Jihong Min @ 2026-05-06 22:41 UTC (permalink / raw)
To: Michal Pecio, Jihong Min, Mario Limonciello
Cc: Greg Kroah-Hartman, Mathias Nyman, Guenter Roeck, Jonathan Corbet,
Shuah Khan, Basavaraj Natikar, linux-usb, linux-hwmon, linux-doc,
linux-pci, linux-kernel
> I think this commit message and certainly the Kconfig help text should
> include full name of the chip and perhaps its official marketing names
> too, so that people better understand what hardware is supported.
>
> So: "AMD Promontory 21 chipset" and "AM5 6xx/8xx series chipsets", or
> whatever they are called by AMD and motherboard vendors.
Addressed locally for v3. I changed the commit message, Kconfig prompt/help
text, and hwmon documentation to use the full name, "AMD Promontory 21
(PROM21)".
I avoided putting chipset marketing names directly into the Kconfig text and
commit subject because some AMD 6xx/8xx series chipsets can be built
from more
than one PROM21 IP in a daisy-chained configuration, including more than one
PROM21 xHCI controller. I described that relationship in the hwmon
documentation instead.
> Is there any documentation of the index/data registers themselves?
I am not aware of public AMD documentation for the PROM21 vendor index/data
registers or the temperature selector.
For my initial validation on an X870E system with two PROM21 xHCI
controllers,
I passed one PROM21 xHCI controller through to a Windows VM, left USB
traffic
idle, and let a Windows monitoring utility poll the controller temperature.
From the Linux host I monitored that controller's PCI MMIO BAR with
read-only
accesses. The vendor index register repeatedly held the same selector value,
and the adjacent data register exposed a stable low 8-bit value. That value
matched the reported PROM21 xHCI temperature after the conversion formula
below.
> Any potential danger that something else (FW? SMM?) uses them too?
I did not find any other in-kernel user of this PROM21 vendor index/data
register pair or the temperature selector.
The driver only serializes its own accesses. It saves the previous index,
selects the temperature register, reads the data register, restores the
previous index, and keeps the sequence short. This avoids leaving the vendor
index/data pair in a different state for other OS code.
This cannot synchronize against firmware or SMM. I cannot rule out such
access
without AMD documentation or confirmation, so I would appreciate AMD
feedback
on whether this PROM21 vendor index/data register pair is safe for OS
polling.
I also tested concurrent temperature reads while USB traffic was active
on the
same controller, including fio against an external USB storage device and
isochronous USB audio playback/capture. I did not observe USB transfer
errors
or audio glitches in those tests.
> Could make sense to describe methodology, particularly in case some
> people would come and question the formula.
>
> How was actual chip temperature measured?
> Was the output compared with any other (Windows?) utilities?
>
> People will be comparing these results and possibly trying to draw
> some conclusions, like OMG Linux runs this chip 8°C hotter, should
> I demand a full refund of my free Ubuntu download????!!!?
After identifying the register pair, I used the same validation setup to
derive
the conversion formula by comparing the observed raw MMIO register value
against HWiNFO64's reported PROM21 xHCI temperature on Windows.
This worked because the PROM21 xHCI temperature is very stable at idle
and the
sensor appears to have low resolution, so the value usually stays unchanged
long enough to compare both sides reliably.
> Is such behavior useful?
>
> Maybe the driver could just disable runtime PM while it's loaded.
and:
> I'd encourage what we do in amdgpu for dGPUs. The hwmon files will
> return an error code (I forget which code) when the device is in runtime
> PM when called. Don't explicitly wake it otherwise.
>
> This prevents someone installing a sensor monitoring application and
> that application "being the only thing" keeping the dGPU awake. If it's
> awake already for other reasons (like being used) then return valid data
> to the applications.
I changed the local v3 branch to follow the amdgpu-style behavior by
default.
The driver now does not wake the xHCI PCI device for a normal hwmon read. If
the device is already active, the read returns valid data. If the device is
suspended, the read returns -EPERM, matching the error code currently
returned
by amdgpu's pm_runtime_get_if_active() based path. I used -EPERM to
match the
current amdgpu precedent, but I can change the errno if maintainers prefer a
different one for this driver.
I kept the allow_pm_switch module parameter for now, but changed its
default to
false. With the default setting, hwmon reads do not change the device
runtime PM
state. Setting allow_pm_switch=Y explicitly opts into waking the xHCI PCI
device for temperature reads.
Thank you,
Jihong Min
On 5/7/26 06:33, Michal Pecio wrote:
> On Thu, 7 May 2026 05:40:34 +0900, Jihong Min wrote:
>> AMD PROM21 xHCI controllers expose an 8-bit temperature value
> I think this commit message and certainly the Kconfig help text should
> include full name of the chip and perhaps its official marketing names
> too, so that people better understand what hardware is supported.
>
> So: "AMD Promontory 21 chipset" and "AM5 6xx/8xx series chipsets", or
> whatever they are called by AMD and motherboard vendors.
>
>> through a vendor-specific index/data register pair in the xHCI PCI
>> MMIO BAR region. Add an auxiliary hwmon driver for AMD 1022:43fd
>> controllers and bind it to the xhci_pci.hwmon auxiliary device
>> created by xhci-pci.
>>
>> The read path verifies the parent PCI function and uses the
>> initialized xHCI HCD MMIO mapping. The vendor index register is at
>> byte offset 0x3000 from the xHCI MMIO BAR base and the vendor data
>> register is at byte offset 0x3008. The driver writes register selector
>> 0x0001e520 to the index register, reads the raw temperature value from
>> the low 8 bits of the data register, and restores the previous index
>> before returning. Expose temp1_input and an xHCI label through hwmon.
>>
>> Register the hwmon device under the parent PCI function so userspace
>> reports it as a PCI adapter, while the auxiliary driver still owns the
>> hwmon lifetime and unregisters it from the auxiliary remove path.
>>
>> No public AMD reference is available for this value.
> Is there any documentation of the index/data registers themselves?
>
> Any potential danger that something else (FW? SMM?) uses them too?
>
>> The conversion formula is derived from observed temperature readings:
>>
>> temp[C] = raw * 0.9066 - 78.624
> Could make sense to describe methodology, particularly in case some
> people would come and question the formula.
>
> How was actual chip temperature measured?
> Was the output compared with any other (Windows?) utilities?
>
> People will be comparing these results and possibly trying to draw
> some conclusions, like OMG Linux runs this chip 8°C hotter, should
> I demand a full refund of my free Ubuntu download????!!!?
>
>> The temperature register did not return a valid value while the xHCI
>> PCI function was suspended in testing. Keep the existing behavior by
>> default and allow temperature reads to wake the xHCI PCI device. Add an
>> allow_pm_switch module parameter so users can disable that behavior;
>> when disabled, reads do not wake the device and return -EAGAIN if it is
>> suspended.
> Is such behavior useful?
>
> Maybe the driver could just disable runtime PM while it's loaded.
>
>> Document the supported device, register access, conversion formula,
>> module parameter, sysfs attributes, and sysfs lookup method.
>>
>> Assisted-by: Codex:gpt-5.5
>> Signed-off-by: Jihong Min <hurryman2212@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/2] hwmon: add initial support for AMD PROM21 xHCI temperature sensor
2026-05-06 22:17 ` Randy Dunlap
@ 2026-05-06 22:52 ` Jihong Min
0 siblings, 0 replies; 16+ messages in thread
From: Jihong Min @ 2026-05-06 22:52 UTC (permalink / raw)
To: Randy Dunlap, Jihong Min, Greg Kroah-Hartman, Mathias Nyman
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Mario Limonciello,
Basavaraj Natikar, linux-usb, linux-hwmon, linux-doc, linux-pci,
linux-kernel
> It sorta looks like these entries are supposed to be maintained in
> alphabetical order, but that new entry is not.
Yes, you are right. I also noticed that but it seems I pasted it in the
wrong place.
Fixed this locally for v3 by moving prom21-hwmon after powr1220 and next
to pt5161l.
Thank you,
Jihong Min
On 5/7/26 07:17, Randy Dunlap wrote:
>
> On 5/6/26 1:40 PM, Jihong Min wrote:
>> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
>> index 8b655e5d6b68..0d85b78596cf 100644
>> --- a/Documentation/hwmon/index.rst
>> +++ b/Documentation/hwmon/index.rst
>> @@ -215,6 +215,7 @@ Hardware Monitoring Kernel Drivers
>> peci-dimmtemp
>> pmbus
>> powerz
>> + prom21-hwmon
>> powr1220
>> pt5161l
>> pxe1610
> It sorta looks like these entries are supposed to be maintained in alphabetical
> order, but that new entry is not.
>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-05-06 22:52 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-06 3:29 [PATCH] usb: xhci: add AMD PROM21 xHCI hwmon support for temperature monitoring Jihong Min
2026-05-06 13:37 ` Mathias Nyman
2026-05-06 14:26 ` Guenter Roeck
2026-05-06 20:28 ` Jihong Min
2026-05-06 20:40 ` [PATCH v2 0/2] AMD PROM21 xHCI temperature hwmon support Jihong Min
2026-05-06 20:40 ` [PATCH v2 1/2] usb: xhci-pci: add generic auxiliary device interface Jihong Min
2026-05-06 20:53 ` Mario Limonciello
2026-05-06 21:09 ` Jihong Min
2026-05-06 21:15 ` Michal Pecio
2026-05-06 21:42 ` Jihong Min
2026-05-06 20:40 ` [PATCH v2 2/2] hwmon: add initial support for AMD PROM21 xHCI temperature sensor Jihong Min
2026-05-06 21:33 ` Michal Pecio
2026-05-06 21:36 ` Mario Limonciello
2026-05-06 22:41 ` Jihong Min
2026-05-06 22:17 ` Randy Dunlap
2026-05-06 22:52 ` Jihong Min
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox