lkml.org 
[lkml]   [2026]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH v2 1/2] usb: xhci-pci: add generic auxiliary device interface
From
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
>>

\
 
 \ /
  Last update: 2026-05-06 23:43    [W:0.104 / U:24.984 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and my Meterkast|Read the blog