All of lore.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: 21+ 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  4:07 ` sashiko-bot
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-07  9:08         ` Michal Pecio
2026-05-07 13:24           ` Mario Limonciello
2026-05-06 22:41       ` Jihong Min
2026-05-07  8:51         ` Michal Pecio
2026-05-06 22:17     ` Randy Dunlap
2026-05-06 22:52       ` Jihong Min
2026-05-07  3:29         ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.