[coreboot-gerrit] Change in coreboot[master]: lp/drivers/usb: Add quirk for QEMU XHCI root hub

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39839 )

Change subject: lp/drivers/usb: Add quirk for QEMU XHCI root hub
......................................................................

lp/drivers/usb: Add quirk for QEMU XHCI root hub

The QEMU XHCI driver does not implement the Port Change Detect bit
in the USBSTS register.  As a result no devices are attached without
looking at each port individually.

Detect this as a quirk based on the QEMU XHCI controller PCI ID,
and apply it to the root hub quirk list so it can get used by the
generic hub driver to skip this check.

With this change an attached USB mass storage device is detected and
able to boot when supplied to qemu:

-drive if=none,id=usbmsc,format=raw,file=/tmp/disk.img
  -device qemu-xhci,id-xhci
  -device usb-storage,bus=xhci.0,drive=usbmsc

Change-Id: I6689cb1dbb24c93d45f5c5ef040b713925d07588
Signed-off-by: Duncan Laurie 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39839
Reviewed-by: Angel Pons 
Reviewed-by: Nico Huber 
Tested-by: build bot (Jenkins) 
---
M payloads/libpayload/drivers/usb/generic_hub.c
M payloads/libpayload/drivers/usb/quirks.c
M payloads/libpayload/drivers/usb/xhci.c
M payloads/libpayload/include/usb/usb.h
4 files changed, 34 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, but someone else must approve
  Angel Pons: Looks good to me, approved

diff --git a/payloads/libpayload/drivers/usb/generic_hub.c b/payloads/libpayload/drivers/usb/generic_hub.c
index 9d444ee..7263400 100644
--- a/payloads/libpayload/drivers/usb/generic_hub.c
+++ b/payloads/libpayload/drivers/usb/generic_hub.c
@@ -218,9 +218,11 @@
    if (!hub)
    	return;

-	if (hub->ops->hub_status_changed &&
-			hub->ops->hub_status_changed(dev) != 1)
+	if (!(dev->quirks & USB_QUIRK_HUB_NO_USBSTS_PCD) &&
+	    hub->ops->hub_status_changed &&
+	    hub->ops->hub_status_changed(dev) != 1) {
    	return;
+	}

int port;
    for (port = 1; port <= hub->num_ports; ++port) {
diff --git a/payloads/libpayload/drivers/usb/quirks.c b/payloads/libpayload/drivers/usb/quirks.c
index 0a35149..d5be0e6 100644
--- a/payloads/libpayload/drivers/usb/quirks.c
+++ b/payloads/libpayload/drivers/usb/quirks.c
@@ -59,6 +59,30 @@
     */
 };

+#if CONFIG(LP_USB_PCI)
+usb_quirks_t pci_quirks[] = {
+	/* QEMU XHCI root hub does not implement port change detect */
+	{ 0x1b36, 0x000d, USB_QUIRK_HUB_NO_USBSTS_PCD, 0 },
+};
+
+u32 pci_quirk_check(pcidev_t controller)
+{
+	int i;
+	u16 vendor = pci_read_config16(controller, REG_VENDOR_ID);
+	u16 device = pci_read_config16(controller, REG_DEVICE_ID);
+
+	for (i = 0; i < ARRAY_SIZE(pci_quirks); i++) {
+		if ((pci_quirks[i].vendor == vendor) &&
+		    (pci_quirks[i].device == device)) {
+			printf("PCI quirks enabled: %08x\n", pci_quirks[i].quirks);
+			return pci_quirks[i].quirks;
+		}
+	}
+
+	return USB_QUIRK_NONE;
+}
+#endif
+
 u32 usb_quirk_check(u16 vendor, u16 device)
 {
    int i;
diff --git a/payloads/libpayload/drivers/usb/xhci.c b/payloads/libpayload/drivers/usb/xhci.c
index ef1d73f..21af579 100644
--- a/payloads/libpayload/drivers/usb/xhci.c
+++ b/payloads/libpayload/drivers/usb/xhci.c
@@ -314,9 +314,13 @@

controller = xhci_init((unsigned long)reg_addr);
    if (controller) {
+		xhci_t *xhci = controller->instance;
    	controller->pcidev = addr;

xhci_switch_ppt_ports(addr);
+
+		/* Set up any quirks for controller root hub */
+		xhci->roothub->quirks = pci_quirk_check(addr);
    }

return controller;
diff --git a/payloads/libpayload/include/usb/usb.h b/payloads/libpayload/include/usb/usb.h
index 5d27f7c..328e883 100644
--- a/payloads/libpayload/include/usb/usb.h
+++ b/payloads/libpayload/include/usb/usb.h
@@ -318,6 +318,7 @@
 int usb_attach_device(hci_t *controller, int hubaddress, int port,
    	      usb_speed speed);

+u32 pci_quirk_check(pcidev_t controller);
 u32 usb_quirk_check(u16 vendor, u16 device);
 int usb_interface_check(u16 vendor, u16 device);

@@ -330,6 +331,7 @@
 #define USB_QUIRK_MSC_FORCE_TRANS_CBI_I		(1 <<  6)
 #define USB_QUIRK_MSC_NO_TEST_UNIT_READY	(1 <<  7)
 #define USB_QUIRK_MSC_SHORT_INQUIRY		(1 <<  8)
+#define USB_QUIRK_HUB_NO_USBSTS_PCD		(1 <<  9)
 #define USB_QUIRK_TEST				(1 << 31)
 #define USB_QUIRK_NONE				 0