public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jihong Min <hurryman2212@icloud.com>
To: Mario Limonciello <mario.limonciello@amd.com>,
	Jihong Min <hurryman2212@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Mathias Nyman <mathias.nyman@intel.com>
Cc: Guenter Roeck <linux@roeck-us.net>,
	Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Basavaraj Natikar <Basavaraj.Natikar@amd.com>,
	linux-usb@vger.kernel.org, linux-hwmon@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] usb: xhci-pci: add generic auxiliary device interface
Date: Thu, 7 May 2026 06:09:45 +0900	[thread overview]
Message-ID: <d5629334-3619-441e-92eb-28845b2cfa57@icloud.com> (raw)
In-Reply-To: <bb047320-594d-46de-9522-1098e784ebc1@amd.com>

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

  reply	other threads:[~2026-05-06 21:09 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d5629334-3619-441e-92eb-28845b2cfa57@icloud.com \
    --to=hurryman2212@icloud.com \
    --cc=Basavaraj.Natikar@amd.com \
    --cc=corbet@lwn.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=hurryman2212@gmail.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=mario.limonciello@amd.com \
    --cc=mathias.nyman@intel.com \
    --cc=skhan@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox