From: Guenter Roeck <linux@roeck-us.net>
To: Jihong Min <hurryman2212@gmail.com>,
Michal Pecio <michal.pecio@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Mathias Nyman <mathias.nyman@intel.com>,
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,
"Mario Limonciello (AMD)" <superm1@kernel.org>,
Yaroslav Isakov <yaroslav.isakov@gmail.com>
Subject: Re: [PATCH v6 1/2] usb: xhci-pci: add AMD Promontory 21 PCI glue
Date: Mon, 18 May 2026 16:25:46 -0700 [thread overview]
Message-ID: <ceabda3d-47a2-474c-9792-5453acd5a84e@roeck-us.net> (raw)
In-Reply-To: <7ff352be-05d2-4c21-931e-18238172e4d7@gmail.com>
On 5/18/26 16:06, Jihong Min wrote:
>
>
> On 5/19/26 06:37, Michal Pecio wrote:
>> That's true.
>> Making this possible is the whole purpose of "if IS_ENABLED" here:
>
> I re-checked the Kconfig cases, and I think you are right here.
>
> The two cases I was trying to avoid are:
>
> 1. the sensor driver is built as a module, or loads only after the
> initramfs stage, but the PROM21 controller has already been bound by
> the generic xhci-pci driver, so no auxiliary device exists for the
> sensor driver to bind to;
>
> 2. the built-in generic xhci-pci driver rejects the PROM21 controller, but
> xhci-pci-prom21 is only available as a module and is not present during
> initramfs, leaving USB behind that controller unavailable at that
> stage.
>
> Looking at your proposed Kconfig shape again, it handles both cases.
>
> If SENSORS_PROM21_XHCI=n, then no sensor support is requested and
> USB_XHCI_PCI_PROM21 can stay disabled. In that case generic xhci-pci binds
> the controller, which is fine because there is no sensor driver that
> needs an
> auxiliary device.
>
> If SENSORS_PROM21_XHCI=m or y and USB_XHCI_PCI=y, then
> USB_XHCI_PCI_PROM21 follows USB_XHCI_PCI and becomes y. That means the
> PROM21
> glue is available during early boot, creates the auxiliary device, and the
> hwmon driver can still bind later if it is built as a module.
>
> If USB_XHCI_PCI=m, then xhci-pci itself is modular. In that case needing the
> PROM21 glue module in initramfs is not a PROM21-specific built-in/module
> split
> problem; it is the normal requirement for a modular xHCI PCI setup.
>
> So I agree that tying the hidden glue option to whether
> SENSORS_PROM21_XHCI is enabled is reasonable.
>
>> Currently, you have a weird situation where xhci-pci-prom21 always
>> binds on x86 and xhci-pci on other platforms (with the unofficial PCIe
>> card you mentioned), plus the sensor cannot work on other platforms.
>
> Agreed. I also agree that the X86 dependency is only a heuristic and is
> not a
> good restriction for a PCI ID based driver. PROM21 is mainly used on AMD x86
> desktop platforms today, but the unofficial PCIe card example shows that the
> device can exist outside the normal AMD x86 chipset topology.
>
> I do not know whether other PROM21-related functionality is supported on
> non-x86 platforms, but this driver does not need to prevent the xHCI
> temperature sensor path from being built there.
>
>> One could further argue that neither should it care whether some hwmon
>> driver exists at all, or which kernel releases it exists in :)
>
> Right. I think the cleanest result is:
>
> - generic xhci-pci handles PROM21 when no sensor support is requested;
> - xhci-pci-prom21 handles PROM21 only when the sensor path is enabled;
> - the hwmon driver remains the user-visible option.
>
> Unless Guenter or the USB maintainers object, I plan to change the next
> revision in that direction and test the Kconfig combinations locally.
>
Ok with me if you are sure that it works. When you send a new revision,
you might want to also change the error path as suggested.
Thanks,
Guenter
next prev parent reply other threads:[~2026-05-18 23:25 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-17 13:04 [PATCH v6 0/2] AMD Promontory 21 xHCI temperature sensor support Jihong Min
2026-05-17 13:04 ` [PATCH v6 1/2] usb: xhci-pci: add AMD Promontory 21 PCI glue Jihong Min
2026-05-17 21:21 ` Michal Pecio
2026-05-18 10:55 ` Guenter Roeck
2026-05-18 20:34 ` Michal Pecio
2026-05-18 20:30 ` Jihong Min
2026-05-18 21:37 ` Michal Pecio
2026-05-18 23:06 ` Jihong Min
2026-05-18 23:25 ` Guenter Roeck [this message]
2026-05-18 23:07 ` Jihong Min
2026-05-17 13:04 ` [PATCH v6 2/2] hwmon: add AMD Promontory 21 xHCI temperature sensor support Jihong Min
2026-05-18 10:58 ` Guenter Roeck
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=ceabda3d-47a2-474c-9792-5453acd5a84e@roeck-us.net \
--to=linux@roeck-us.net \
--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=mario.limonciello@amd.com \
--cc=mathias.nyman@intel.com \
--cc=michal.pecio@gmail.com \
--cc=skhan@linuxfoundation.org \
--cc=superm1@kernel.org \
--cc=yaroslav.isakov@gmail.com \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).