Change in coreboot[master]: lp/drivers/usb: Add quirk for QEMU XHCI root hub
Duncan Laurie has uploaded this change for review. ( 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 <dlaurie@google.com> --- 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, 35 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/39839/1 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..556ce9e 100644 --- a/payloads/libpayload/drivers/usb/quirks.c +++ b/payloads/libpayload/drivers/usb/quirks.c @@ -59,6 +59,31 @@ */ }; +#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 659861f..55ca89f 100644 --- a/payloads/libpayload/drivers/usb/xhci.c +++ b/payloads/libpayload/drivers/usb/xhci.c @@ -316,9 +316,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 -- To view, visit https://review.coreboot.org/c/coreboot/+/39839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6689cb1dbb24c93d45f5c5ef040b713925d07588 Gerrit-Change-Number: 39839 Gerrit-PatchSet: 1 Gerrit-Owner: Duncan Laurie <dlaurie@chromium.org> Gerrit-MessageType: newchange
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39839 ) Change subject: lp/drivers/usb: Add quirk for QEMU XHCI root hub ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/39839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6689cb1dbb24c93d45f5c5ef040b713925d07588 Gerrit-Change-Number: 39839 Gerrit-PatchSet: 1 Gerrit-Owner: Duncan Laurie <dlaurie@chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Wed, 25 Mar 2020 19:04:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39839 ) Change subject: lp/drivers/usb: Add quirk for QEMU XHCI root hub ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/39839/1/payloads/libpayload/drivers... File payloads/libpayload/drivers/usb/quirks.c: https://review.coreboot.org/c/coreboot/+/39839/1/payloads/libpayload/drivers... PS1, Line 78: pci_quirks[i].quirks); Update for 96 character text width? -- To view, visit https://review.coreboot.org/c/coreboot/+/39839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6689cb1dbb24c93d45f5c5ef040b713925d07588 Gerrit-Change-Number: 39839 Gerrit-PatchSet: 1 Gerrit-Owner: Duncan Laurie <dlaurie@chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 25 Mar 2020 20:58:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Hello build bot (Jenkins), Furquan Shaikh, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/39839 to look at the new patch set (#2). 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 <dlaurie@google.com> --- 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(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/39839/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/39839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6689cb1dbb24c93d45f5c5ef040b713925d07588 Gerrit-Change-Number: 39839 Gerrit-PatchSet: 2 Gerrit-Owner: Duncan Laurie <dlaurie@chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39839 ) Change subject: lp/drivers/usb: Add quirk for QEMU XHCI root hub ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/39839/1/payloads/libpayload/drivers... File payloads/libpayload/drivers/usb/quirks.c: https://review.coreboot.org/c/coreboot/+/39839/1/payloads/libpayload/drivers... PS1, Line 78: pci_quirks[i].quirks);
Update for 96 character text width? Done
-- To view, visit https://review.coreboot.org/c/coreboot/+/39839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6689cb1dbb24c93d45f5c5ef040b713925d07588 Gerrit-Change-Number: 39839 Gerrit-PatchSet: 1 Gerrit-Owner: Duncan Laurie <dlaurie@chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 26 Mar 2020 21:06:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39839 ) Change subject: lp/drivers/usb: Add quirk for QEMU XHCI root hub ...................................................................... Patch Set 2: (2 comments) https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... File payloads/libpayload/drivers/usb/generic_hub.c: https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... PS2, Line 222: hub_status_changed Alternatively to the whole quirk infrastructure, could we just not set the pointer? https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... File payloads/libpayload/drivers/usb/quirks.c: https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... PS2, Line 62: #if CONFIG(LP_USB_PCI) Can we leave this to the linker? -- To view, visit https://review.coreboot.org/c/coreboot/+/39839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6689cb1dbb24c93d45f5c5ef040b713925d07588 Gerrit-Change-Number: 39839 Gerrit-PatchSet: 2 Gerrit-Owner: Duncan Laurie <dlaurie@chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 26 Mar 2020 22:36:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39839 ) Change subject: lp/drivers/usb: Add quirk for QEMU XHCI root hub ...................................................................... Patch Set 2: (2 comments) https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... File payloads/libpayload/drivers/usb/generic_hub.c: https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... PS2, Line 222: hub_status_changed
Alternatively to the whole quirk infrastructure, could we just not […] I actually looked at that first, it is a bit messy because the RH is added before the PCI device is hooked in, so at the time xhci_rh_init runs it doesn't know the PCI device. I was going to fix that init path as well but this seemed like it would need a quirk check at some level anyway...
https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... File payloads/libpayload/drivers/usb/quirks.c: https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... PS2, Line 62: #if CONFIG(LP_USB_PCI)
Can we leave this to the linker? Maybe? All the other PCI use is guarded in this directory so I went for consistency.
-- To view, visit https://review.coreboot.org/c/coreboot/+/39839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6689cb1dbb24c93d45f5c5ef040b713925d07588 Gerrit-Change-Number: 39839 Gerrit-PatchSet: 2 Gerrit-Owner: Duncan Laurie <dlaurie@chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 30 Mar 2020 22:14:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39839 ) Change subject: lp/drivers/usb: Add quirk for QEMU XHCI root hub ...................................................................... Patch Set 3: (1 comment) https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... File payloads/libpayload/drivers/usb/generic_hub.c: https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... PS2, Line 222: hub_status_changed
I actually looked at that first, it is a bit messy because the RH is added before the PCI device is […] I don't see that, xhci_pci_init() calls xhci_init() calls xhci_rh_init(). What do I miss?
Beside not providing `.hub_status_changed` we could also let it return 1 for the Qemu implementation. Sorry for squabbling but it feels wrong to add quirk handling to work around an existing interface. If we end up handling it here, we should be honest and name the quirk USB_QUIRK_SPURIOUS_HUB_STATUS_CHANGED_FUNCTION Beside all that, do we actually benefit from having `.hub_status_change`? I probably just added it because there seemed to be a reliable PCD bit... No other generic hub implementation uses it (OHCI could, if anybody would wanted to port it forward... before we abandon it). -- To view, visit https://review.coreboot.org/c/coreboot/+/39839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6689cb1dbb24c93d45f5c5ef040b713925d07588 Gerrit-Change-Number: 39839 Gerrit-PatchSet: 3 Gerrit-Owner: Duncan Laurie <dlaurie@chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 05 Apr 2020 18:29:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Comment-In-Reply-To: Duncan Laurie <dlaurie@chromium.org> Gerrit-MessageType: comment
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39839 ) Change subject: lp/drivers/usb: Add quirk for QEMU XHCI root hub ...................................................................... Patch Set 3: (1 comment) https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... File payloads/libpayload/drivers/usb/generic_hub.c: https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... PS2, Line 222: hub_status_changed
I don't see that, xhci_pci_init() calls xhci_init() calls xhci_rh_init(). […] xhci_pci_init() calls xhci_init() before setting controller->pcidev. Inside xhci_init() is the call to xhci->roothub->init() which is then executing before it has a valid pcidev.
I could try to fix that but currently xhci_init() allocates the structure internally so it can't be filled by the calling function until it returns. I'm not opposed to renaming the quirk, but the current name is accurate. -- To view, visit https://review.coreboot.org/c/coreboot/+/39839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6689cb1dbb24c93d45f5c5ef040b713925d07588 Gerrit-Change-Number: 39839 Gerrit-PatchSet: 3 Gerrit-Owner: Duncan Laurie <dlaurie@chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 14 Apr 2020 16:30:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Comment-In-Reply-To: Duncan Laurie <dlaurie@chromium.org> Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39839 ) Change subject: lp/drivers/usb: Add quirk for QEMU XHCI root hub ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/39839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6689cb1dbb24c93d45f5c5ef040b713925d07588 Gerrit-Change-Number: 39839 Gerrit-PatchSet: 3 Gerrit-Owner: Duncan Laurie <dlaurie@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 23 Apr 2020 22:25:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39839 ) Change subject: lp/drivers/usb: Add quirk for QEMU XHCI root hub ...................................................................... Patch Set 3: Code-Review+1 (1 comment) https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... File payloads/libpayload/drivers/usb/generic_hub.c: https://review.coreboot.org/c/coreboot/+/39839/2/payloads/libpayload/drivers... PS2, Line 222: hub_status_changed
but the current name is accurate.
It is from xHCI perspective. But this is not the xhci driver. To me it looks like driving a nail through all the layers. The quirk infrastructure encourages this, maybe I'll try to get rid of it in the future. -- To view, visit https://review.coreboot.org/c/coreboot/+/39839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6689cb1dbb24c93d45f5c5ef040b713925d07588 Gerrit-Change-Number: 39839 Gerrit-PatchSet: 3 Gerrit-Owner: Duncan Laurie <dlaurie@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sat, 25 Apr 2020 22:53:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Comment-In-Reply-To: Duncan Laurie <dlaurie@chromium.org> Gerrit-MessageType: comment
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 <dlaurie@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/39839 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Nico Huber <nico.h@gmx.de> Tested-by: build bot (Jenkins) <no-reply@coreboot.org> --- 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 -- To view, visit https://review.coreboot.org/c/coreboot/+/39839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6689cb1dbb24c93d45f5c5ef040b713925d07588 Gerrit-Change-Number: 39839 Gerrit-PatchSet: 4 Gerrit-Owner: Duncan Laurie <dlaurie@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: merged
participants (6)
-
Angel Pons (Code Review) -
Duncan Laurie (Code Review)
-
Furquan Shaikh (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)