Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: 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>,
	Mario Limonciello <mario.limonciello@amd.com>,
	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 v3 1/2] usb: xhci-pci: add generic auxiliary device interface
Date: Thu, 7 May 2026 12:31:24 +0300	[thread overview]
Message-ID: <a5739875-b8a5-4918-8850-fa4b32d5279d@linux.intel.com> (raw)
In-Reply-To: <effa7bd7bef8a8ea28b9e28fe47af6a58e39edf2.1778123510.git.hurryman2212@gmail.com>

On 5/7/26 06:31, 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 PCI ID match table lists AMD Promontory 21 (PROM21) 1022:43fd
> controllers. For matching controllers, xhci-pci creates an auxiliary
> device and stores it in devres so the remove path destroys it before HCD
> teardown.
> 
> Subsystem-specific child drivers can then bind to those devices through
> the auxiliary bus and keep their hardware-specific logic outside xhci-pci.
> 
> 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 | 83 +++++++++++++++++++++++++++++++++++++
>   2 files changed, 93 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..618d6840e108 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>
> @@ -80,6 +82,7 @@
>   #define PCI_DEVICE_ID_AMD_RAVEN_15E1_XHCI		0x15e1
>   #define PCI_DEVICE_ID_AMD_RAVEN2_XHCI			0x15e5
>   #define PCI_DEVICE_ID_AMD_RENOIR_XHCI			0x1639
> +#define PCI_DEVICE_ID_AMD_PROM21_XHCI			0x43fd
>   #define PCI_DEVICE_ID_AMD_PROMONTORYA_4			0x43b9
>   #define PCI_DEVICE_ID_AMD_PROMONTORYA_3			0x43ba
>   #define PCI_DEVICE_ID_AMD_PROMONTORYA_2			0x43bb
> @@ -103,6 +106,80 @@ 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);
>   
> +static const struct pci_device_id pci_ids_have_aux[] = {
> +	{ PCI_DEVICE_DATA(AMD, PROM21_XHCI, "prom21_hwmon") },
> +	{ /* end: all zeroes */ }
> +};
> +
> +struct xhci_pci_aux_devres {
> +	struct auxiliary_device *auxdev;
> +};
> +
> +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;
> +}
> +
> +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;
> +}
> +
>   static const struct xhci_driver_overrides xhci_pci_overrides __initconst = {
>   	.reset = xhci_pci_setup,
>   	.start = xhci_pci_run,
> @@ -677,6 +754,9 @@ 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 (IS_ENABLED(CONFIG_USB_XHCI_PCI_AUXDEV))
> +		xhci_pci_try_add_aux_device(dev);
> +
>   	return 0;

I think this should be turned around so that the vendor specific code calls the common code.
xhci-pci-renesas.c does this nicely.

In your case it would be adding something like a xhci-pci-prom21.c pci driver:

xhci_pci_prom21_probe(struct pci_dev *dev, const struct pci_device_id *id)
{
	crate_auxiliary_device(dev);
	return xhci_pci_common_probe(dev, id);
}

xhci_pci_prom21_remove(struct pci_dev *dev)
{
	destroy_auxiliary_device(dev);
	xhci_pci_remove(dev);
}

static const struct pci_device_id pci_ids[] = {
	{ PCI_DEVICE(YOUR_AMD_PCI_VENDOR_ID, YOUR_PROM21_DEVICE_ID) },
	{ /* end: all zeroes */ }
};
MODULE_DEVICE_TABLE(pci, pci_ids);

static struct pci_driver xhci_prom21_pci_driver = {
	.name =		"xhci-pci-prom21",
	.id_table =	pci_ids,

	.probe =	xhci_pci_prom21_probe,
	.remove =	xhci_pci_prom21_remove,

	.shutdown = 	usb_hcd_pci_shutdown,
	.driver = {
		.pm = pm_ptr(&usb_hcd_pci_pm_ops),
	},
};
module_pci_driver(xhci_prom21_pci_driver);

MODULE_DESCRIPTION("AMD Promontory 21 xHCI PCI Host Controller Driver");
MODULE_IMPORT_NS("xhci");
MODULE_LICENSE("GPL v2");

-Mathias

  parent reply	other threads:[~2026-05-07  9:31 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-07  3:31 [PATCH v3 0/2] AMD Promontory 21 xHCI temperature hwmon support Jihong Min
2026-05-07  3:31 ` [PATCH v3 1/2] usb: xhci-pci: add generic auxiliary device interface Jihong Min
2026-05-07  4:14   ` sashiko-bot
2026-05-07  9:31   ` Mathias Nyman [this message]
2026-05-08  7:04     ` Jihong Min
2026-05-08 13:17       ` Guenter Roeck
2026-05-08 14:22         ` Jihong Min
2026-05-07  3:31 ` [PATCH v3 2/2] hwmon: add AMD Promontory 21 xHCI temperature sensor support Jihong Min
2026-05-07 15:53   ` Guenter Roeck
2026-05-08  5:42     ` Jihong Min
2026-05-08 13:12       ` Guenter Roeck
2026-05-08 14:21         ` Jihong Min
2026-05-08 16:27           ` Mario Limonciello
2026-05-08 16:56             ` 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=a5739875-b8a5-4918-8850-fa4b32d5279d@linux.intel.com \
    --to=mathias.nyman@linux.intel.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