Re: [PATCH v6 1/2] usb: xhci-pci: add AMD Promontory 21 PCI glue

[Michal Pecio <michal.pecio@gmail.com>]

On Tue, 19 May 2026 05:30:47 +0900, Jihong Min wrote:
> On 5/18/26 06:21, Michal Pecio wrote:
>  > Instead of the X86 heuristic, would it be possible to build glue
>  > code if and only if SENSORS_PROM21_XHCI is enabled?
>  >
>  > This seems to work:
>  >
>  >   config SENSORS_PROM21_XHCI
>  >          tristate "AMD Promontory 21 xHCI temperature sensor"
>  > -       depends on USB_XHCI_PCI_PROM21
>  > +       depends on USB_XHCI_PCI
>  >
>  >   config USB_XHCI_PCI_PROM21
>  >          tristate
>  > -       depends on X86
>  >          depends on USB_XHCI_PCI
>  > -       default USB_XHCI_PCI
>  > +       default USB_XHCI_PCI if SENSORS_PROM21_XHCI != 'n'
>  >          select AUXILIARY_BUS
>  >
>  > I don't know if it's the best way, perhaps it would be preferable
>  > for the hwmon driver to select the glue, but then I'm not sure how
>  > to force glue to become 'y' when xhci-pci is 'y'.  
> 
> I think I should keep the current hidden glue option for now.
> 
> The PROM21 PCI glue is part of the PCI binding path for the xHCI
> controller when enabled, while SENSORS_PROM21_XHCI is only the
> optional user-visible hwmon driver. Tying the glue to the hwmon
> option would make the sensor option affect which driver binds the USB
> controller.

That's true.
Making this possible is the whole purpose of "if IS_ENABLED" here:

>  static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  {
>  	if (IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS) &&
>  			pci_match_id(pci_ids_renesas, dev))
>  		return -ENODEV;
>  
> +	if (IS_ENABLED(CONFIG_USB_XHCI_PCI_PROM21) &&
> +	    pci_match_id(pci_ids_prom21, dev))
> +		return -ENODEV;
> +
>  	return xhci_pci_common_probe(dev, id);
>  }

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.

> As Guenter pointed out, that would be too strong; the USB controller
> should not depend on whether the optional hwmon driver is enabled.

One could further argue that neither should it care whether some hwmon
driver exists at all, or which kernel releases it exists in :)

Regards,
Michal