Re: [PATCH] usb: xhci: Restore xhci_pci support for Renesas HCs

Greg Kroah-Hartman
2025-01-27T17:29:55Z

On Mon, Jan 27, 2025 at 03:10:51PM +0100, Michal Pecio wrote:
> Some Renesas HCs require firmware upload to work, this is handled by the
> xhci_pci_renesas driver. Other variants of those chips load firmware from
> onboard flash and can work with xhci_pci alone.
>
> A refactor merged in v6.12 broke the latter configuration and users are
> finding their hardware not working after upgrading and need to enable the
> firmware loader which isn't really necessary on their systems.

Why isn't it necessary? Those chips want firmware to be uploaded, and
if the driver doesn't need it, it should be able to handle it if it is
not present.

> Let xhci_pci work with those chips as before when the firmware loader is
> disabled by kernel configuration.
>
> Fixes: 25f51b76f90f ("xhci-pci: Make xhci-pci-renesas a proper modular driver")
> CC: stable@vger.kernel.org
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219616
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219726
> Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
> ---
> drivers/usb/host/xhci-pci.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 5a5f0b2dba4d..3d08d6527b1d 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -657,17 +657,21 @@ int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id)
> }
> EXPORT_SYMBOL_NS_GPL(xhci_pci_common_probe, "xhci");
>
> +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS)
> static const struct pci_device_id pci_ids_reject[] = {
> /* handled by xhci-pci-renesas */
> { PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0014) },
> { PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0015) },
> { /* end: all zeroes */ }
> };
> +#endif
>
> static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
> {
> +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS)
> if (pci_match_id(pci_ids_reject, dev))
> return -ENODEV;
> +#endif

No, please don't put #ifdef in .c files if at all preventable.

If the firmware isn't there, then the driver should be fixed to keep on
working if it determines loading it is not needed. This type of check
should not be needed in the xhci-pci.c file.

thanks,

greg k-h