diff mbox series

[v2] xhci-pci: Make xhci-pci-renesas a proper modular driver

Message ID ZqqfXYRJf7kGaqus@decadent.org.uk (mailing list archive)
State Accepted
Commit 25f51b76f90f10f9bf2fbc05fc51cf685da7ccad
Headers show
Series [v2] xhci-pci: Make xhci-pci-renesas a proper modular driver | expand

Commit Message

Ben Hutchings July 31, 2024, 8:32 p.m. UTC
If CONFIG_USB_XHCI_PCI_RENESAS is enabled, xhci-pci conditionally
calls into the xhci-pci-renesas module, which means both modules must
be loaded to use any xHCI PCI controller.

The MODULE_FIRMWARE declaration in the base xhci-pci module causes
initramfs-tools to check for and warn about missing firmware for the
Renesas xHCI controllers, when any xHCI PCI controller is present.
And because of the previous oddity, simply moving this declaration to
xhci-pci-renesas wouldn't help.

To fix this, reverse the relationship between the modules:

- Remove the quirk for the Renesas xHCIs, and the driver_data
  structure used only for them
- In xhci-pci:
  - Rename xhci_pci_probe() to xhci_pci_common_probe()
  - Export xhci_pci_common_probe() and xhci_pci_remove()
  - Use a new probe function that rejects the Renesas xHCIs and then
    calls the common probe function
- In xhci-pci-renesas:
  - Stop exporting renesas_xhci_check_request_fw()
  - Add a probe function that calls renesas_xhci_check_request_fw()
    followed by xhci_pci_common_probe()
  - Add and register a new pci_driver matching only the Renesas xHCIs
    and using its own probe function, but with other operations the
    same as in xhci-pci
- Make CONFIG_USB_XHCI_PCI_RENESAS depend on CONFIG_USB_XHCI_PCI,
  not the other way around

Finally, move the MODULE_FIRMWARE declaration to xhci-pci-renesas.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
v2:
- Fix definition of xhci_pci_probe() to remain static
- Rebase onto v6.11-rc1

 drivers/usb/host/Kconfig            |  2 +-
 drivers/usb/host/xhci-pci-renesas.c | 48 ++++++++++++++++++++----
 drivers/usb/host/xhci-pci.c         | 57 ++++++++++-------------------
 drivers/usb/host/xhci-pci.h         | 19 +---------
 drivers/usb/host/xhci.h             |  2 +-
 5 files changed, 64 insertions(+), 64 deletions(-)

Comments

Cyril Brulebois July 31, 2024, 10:06 p.m. UTC | #1
Ben Hutchings <ben@decadent.org.uk> (2024-07-31):
> v2:
> - Fix definition of xhci_pci_probe() to remain static
> - Rebase onto v6.11-rc1

Tested-by: Cyril Brulebois <cyril@debamax.com>

On top of 21b136cc63d2a9ddd60d4699552b69c214b32964, starting from a
distribution-oriented .config (Debian's 6.10-1~exp1 arm64 package),
default answers to oldconfig, adding CONFIG_USB_XHCI_PCI_RENESAS=m
afterwards.

On the hardware side: Raspberry Pi CM4 Lite on a CM4 IO Board, with two
(slightly) different Supahub PCIe-to-quad-USB cards (006 and 006S),
everything is good with the firmware available. Otherwise this happens:

[    8.037155] xhci-pci-renesas 0000:01:00.0: failed to load firmware renesas_usb_fw.mem, fallback to ROM
[    8.046687] xhci-pci-renesas 0000:01:00.0: xHCI Host Controller
[    8.052712] xhci-pci-renesas 0000:01:00.0: new USB bus registered, assigned bus number 1
[   27.944117] xhci-pci-renesas 0000:01:00.0: can't setup: -110
[   27.949903] xhci-pci-renesas 0000:01:00.0: USB bus 1 deregistered
[   27.956121] xhci-pci-renesas 0000:01:00.0: init 0000:01:00.0 fail, -110
[   27.962837] xhci-pci-renesas 0000:01:00.0: probe with driver xhci-pci-renesas failed with error -110

(No onboard ROM on those.)

Thanks, Ben!


Cheers,
diff mbox series

Patch

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 4448d0ab06f0..d011d6c753ed 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -40,11 +40,11 @@  config USB_XHCI_DBGCAP
 config USB_XHCI_PCI
 	tristate
 	depends on USB_PCI
-	depends on USB_XHCI_PCI_RENESAS || !USB_XHCI_PCI_RENESAS
 	default y
 
 config USB_XHCI_PCI_RENESAS
 	tristate "Support for additional Renesas xHCI controller with firmware"
+	depends on USB_XHCI_PCI
 	help
 	  Say 'Y' to enable the support for the Renesas xHCI controller with
 	  firmware. Make sure you have the firmware for the device and
diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
index 247cc7c2ce70..30cc5a1380a5 100644
--- a/drivers/usb/host/xhci-pci-renesas.c
+++ b/drivers/usb/host/xhci-pci-renesas.c
@@ -50,6 +50,8 @@ 
 #define RENESAS_RETRY	10000
 #define RENESAS_DELAY	10
 
+#define RENESAS_FW_NAME	"renesas_usb_fw.mem"
+
 static int renesas_fw_download_image(struct pci_dev *dev,
 				     const u32 *fw, size_t step, bool rom)
 {
@@ -573,12 +575,10 @@  static int renesas_load_fw(struct pci_dev *pdev, const struct firmware *fw)
 	return err;
 }
 
-int renesas_xhci_check_request_fw(struct pci_dev *pdev,
-				  const struct pci_device_id *id)
+static int renesas_xhci_check_request_fw(struct pci_dev *pdev,
+					 const struct pci_device_id *id)
 {
-	struct xhci_driver_data *driver_data =
-			(struct xhci_driver_data *)id->driver_data;
-	const char *fw_name = driver_data->firmware;
+	const char fw_name[] = RENESAS_FW_NAME;
 	const struct firmware *fw;
 	bool has_rom;
 	int err;
@@ -625,7 +625,41 @@  int renesas_xhci_check_request_fw(struct pci_dev *pdev,
 	release_firmware(fw);
 	return err;
 }
-EXPORT_SYMBOL_GPL(renesas_xhci_check_request_fw);
 
-MODULE_DESCRIPTION("Support for Renesas xHCI controller with firmware");
+static int
+xhci_pci_renesas_probe(struct pci_dev *dev, const struct pci_device_id *id)
+{
+	int retval;
+
+	retval = renesas_xhci_check_request_fw(dev, id);
+	if (retval)
+		return retval;
+
+	return xhci_pci_common_probe(dev, id);
+}
+
+static const struct pci_device_id pci_ids[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0014) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0015) },
+	{ /* end: all zeroes */ }
+};
+MODULE_DEVICE_TABLE(pci, pci_ids);
+
+static struct pci_driver xhci_renesas_pci_driver = {
+	.name =		"xhci-pci-renesas",
+	.id_table =	pci_ids,
+
+	.probe =	xhci_pci_renesas_probe,
+	.remove =	xhci_pci_remove,
+
+	.shutdown = 	usb_hcd_pci_shutdown,
+	.driver = {
+		.pm = pm_ptr(&usb_hcd_pci_pm_ops),
+	},
+};
+module_pci_driver(xhci_renesas_pci_driver);
+
+MODULE_DESCRIPTION("Renesas xHCI PCI Host Controller Driver");
+MODULE_FIRMWARE(RENESAS_FW_NAME);
+MODULE_IMPORT_NS(xhci);
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index dc1e345ab67e..b5705ed01d83 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -235,15 +235,6 @@  static int xhci_pci_reinit(struct xhci_hcd *xhci, struct pci_dev *pdev)
 static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 {
 	struct pci_dev                  *pdev = to_pci_dev(dev);
-	struct xhci_driver_data         *driver_data;
-	const struct pci_device_id      *id;
-
-	id = pci_match_id(to_pci_driver(pdev->dev.driver)->id_table, pdev);
-
-	if (id && id->driver_data) {
-		driver_data = (struct xhci_driver_data *)id->driver_data;
-		xhci->quirks |= driver_data->quirks;
-	}
 
 	/* Look for vendor-specific quirks */
 	if (pdev->vendor == PCI_VENDOR_ID_FRESCO_LOGIC &&
@@ -572,21 +563,13 @@  static int xhci_pci_update_hub_device(struct usb_hcd *hcd, struct usb_device *hd
  * We need to register our own PCI probe function (instead of the USB core's
  * function) in order to create a second roothub under xHCI.
  */
-static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
+int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	int retval;
 	struct xhci_hcd *xhci;
 	struct usb_hcd *hcd;
-	struct xhci_driver_data *driver_data;
 	struct reset_control *reset;
 
-	driver_data = (struct xhci_driver_data *)id->driver_data;
-	if (driver_data && driver_data->quirks & XHCI_RENESAS_FW_QUIRK) {
-		retval = renesas_xhci_check_request_fw(dev, id);
-		if (retval)
-			return retval;
-	}
-
 	reset = devm_reset_control_get_optional_exclusive(&dev->dev, NULL);
 	if (IS_ERR(reset))
 		return PTR_ERR(reset);
@@ -651,8 +634,24 @@  static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	pm_runtime_put_noidle(&dev->dev);
 	return retval;
 }
+EXPORT_SYMBOL_NS_GPL(xhci_pci_common_probe, xhci);
+
+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 */ }
+};
+
+static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
+{
+	if (pci_match_id(pci_ids_reject, dev))
+		return -ENODEV;
+
+	return xhci_pci_common_probe(dev, id);
+}
 
-static void xhci_pci_remove(struct pci_dev *dev)
+void xhci_pci_remove(struct pci_dev *dev)
 {
 	struct xhci_hcd *xhci;
 
@@ -675,6 +674,7 @@  static void xhci_pci_remove(struct pci_dev *dev)
 
 	usb_hcd_pci_remove(dev);
 }
+EXPORT_SYMBOL_NS_GPL(xhci_pci_remove, xhci);
 
 /*
  * In some Intel xHCI controllers, in order to get D3 working,
@@ -882,19 +882,8 @@  static void xhci_pci_shutdown(struct usb_hcd *hcd)
 
 /*-------------------------------------------------------------------------*/
 
-static const struct xhci_driver_data reneses_data = {
-	.quirks  = XHCI_RENESAS_FW_QUIRK,
-	.firmware = "renesas_usb_fw.mem",
-};
-
 /* PCI driver selection metadata; PCI hotplugging uses this */
 static const struct pci_device_id pci_ids[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0014),
-		.driver_data =  (unsigned long)&reneses_data,
-	},
-	{ PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0015),
-		.driver_data =  (unsigned long)&reneses_data,
-	},
 	/* handle any USB 3.0 xHCI controller */
 	{ PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_XHCI, ~0),
 	},
@@ -902,14 +891,6 @@  static const struct pci_device_id pci_ids[] = {
 };
 MODULE_DEVICE_TABLE(pci, pci_ids);
 
-/*
- * Without CONFIG_USB_XHCI_PCI_RENESAS renesas_xhci_check_request_fw() won't
- * load firmware, so don't encumber the xhci-pci driver with it.
- */
-#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS)
-MODULE_FIRMWARE("renesas_usb_fw.mem");
-#endif
-
 /* pci driver glue; this is a "new style" PCI driver module */
 static struct pci_driver xhci_pci_driver = {
 	.name =		hcd_name,
diff --git a/drivers/usb/host/xhci-pci.h b/drivers/usb/host/xhci-pci.h
index cb9a8f331a44..e87c7d9d76b8 100644
--- a/drivers/usb/host/xhci-pci.h
+++ b/drivers/usb/host/xhci-pci.h
@@ -4,22 +4,7 @@ 
 #ifndef XHCI_PCI_H
 #define XHCI_PCI_H
 
-#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS)
-int renesas_xhci_check_request_fw(struct pci_dev *dev,
-				  const struct pci_device_id *id);
-
-#else
-static int renesas_xhci_check_request_fw(struct pci_dev *dev,
-					 const struct pci_device_id *id)
-{
-	return 0;
-}
-
-#endif
-
-struct xhci_driver_data {
-	u64 quirks;
-	const char *firmware;
-};
+int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id);
+void xhci_pci_remove(struct pci_dev *dev);
 
 #endif
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index ebd0afd59a60..df2c81db21a1 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1616,7 +1616,7 @@  struct xhci_hcd {
 #define XHCI_DEFAULT_PM_RUNTIME_ALLOW	BIT_ULL(33)
 #define XHCI_RESET_PLL_ON_DISCONNECT	BIT_ULL(34)
 #define XHCI_SNPS_BROKEN_SUSPEND    BIT_ULL(35)
-#define XHCI_RENESAS_FW_QUIRK	BIT_ULL(36)
+/* Reserved. It was XHCI_RENESAS_FW_QUIRK */
 #define XHCI_SKIP_PHY_INIT	BIT_ULL(37)
 #define XHCI_DISABLE_SPARSE	BIT_ULL(38)
 #define XHCI_SG_TRB_CACHE_SIZE_QUIRK	BIT_ULL(39)