Just debugged the following:
ehci_setup() happens. Starts a thread to probe its ports... ohci_setup() happens. Starts a thread to probe its ports... OHCI probes port zero, decides there's nothing there. EHCI probes port zero, decides there's a USB1 device there and gives it away to OHCI... which is never going to see it.
What's wrong here? I've "fixed" it with a wait_threads() right before the ohci_setup() in usb.c and now I can see my keyboard, but that's definitely not the right answer.
Do we have any kind of hotplug support for USB devices? If not, should we at least have a kind of "manual" hotplug notification for when EHCI actively gives a port away to the USB1 controller?
(How) is this supposed to work?
On Do, 2013-12-05 at 14:03 +0000, David Woodhouse wrote:
Just debugged the following:
ehci_setup() happens. Starts a thread to probe its ports... ohci_setup() happens. Starts a thread to probe its ports... OHCI probes port zero, decides there's nothing there. EHCI probes port zero, decides there's a USB1 device there and gives it away to OHCI... which is never going to see it.
What's wrong here? I've "fixed" it with a wait_threads() right before the ohci_setup() in usb.c and now I can see my keyboard, but that's definitely not the right answer.
I think not running configure_ehci() as thread should work too and is a bit less agressive than wait_threads(). Waits only for the ehci port probe threads finish (see usb_enumerate() func), not all threads.
Do we have any kind of hotplug support for USB devices?
No.
cheers, Gerd
On Thu, 2013-12-05 at 16:11 +0100, Gerd Hoffmann wrote:
I think not running configure_ehci() as thread should work too and is a bit less agressive than wait_threads(). Waits only for the ehci port probe threads finish (see usb_enumerate() func), not all threads.
Yeah. Is EHCI always initialised before OHCI/UHCI though? I'm staring at the code in usb_setup() and it seems to be attempting that, but I'm not 100% sure if that's what it's actually doing.
And what about XHCI? Does that have similar interactions?
On Do, 2013-12-05 at 15:20 +0000, David Woodhouse wrote:
On Thu, 2013-12-05 at 16:11 +0100, Gerd Hoffmann wrote:
I think not running configure_ehci() as thread should work too and is a bit less agressive than wait_threads(). Waits only for the ehci port probe threads finish (see usb_enumerate() func), not all threads.
Yeah. Is EHCI always initialised before OHCI/UHCI though? I'm staring at the code in usb_setup() and it seems to be attempting that, but I'm not 100% sure if that's what it's actually doing.
Yes, order is ehci first.
And what about XHCI? Does that have similar interactions?
No, xhci handles all usb speeds itself.
cheers, Gerd
On Thu, Dec 05, 2013 at 02:03:23PM +0000, David Woodhouse wrote:
Just debugged the following:
ehci_setup() happens. Starts a thread to probe its ports... ohci_setup() happens. Starts a thread to probe its ports... OHCI probes port zero, decides there's nothing there. EHCI probes port zero, decides there's a USB1 device there and gives it away to OHCI... which is never going to see it.
What is supposed to happen, is that usb_setup() sees that there is an ehci device and only runs ehci_setup() on the whole device. ohci_setup is not supposed to run from usb_setup() for that device at all. Then, if the ehci code finds a legacy device, it runs ohci_setup() from ehci_note_port() after it has configured all the legacy devices into legacy mode.
This certainly used to work, I'll take a look.
-Kevin
On Thu, Dec 05, 2013 at 10:47:21AM -0500, Kevin O'Connor wrote:
On Thu, Dec 05, 2013 at 02:03:23PM +0000, David Woodhouse wrote:
Just debugged the following:
ehci_setup() happens. Starts a thread to probe its ports... ohci_setup() happens. Starts a thread to probe its ports... OHCI probes port zero, decides there's nothing there. EHCI probes port zero, decides there's a USB1 device there and gives it away to OHCI... which is never going to see it.
What is supposed to happen, is that usb_setup() sees that there is an ehci device and only runs ehci_setup() on the whole device. ohci_setup is not supposed to run from usb_setup() for that device at all. Then, if the ehci code finds a legacy device, it runs ohci_setup() from ehci_note_port() after it has configured all the legacy devices into legacy mode.
This certainly used to work, I'll take a look.
I reviewed the code and booted my e350m1 board with an OHCI keyboard and I can't reproduce your issue. Can you post the logs (with debug level 8) of the failure you are seeing?
-Kevin
On Thu, 2013-12-05 at 11:59 -0500, Kevin O'Connor wrote:
I reviewed the code and booted my e350m1 board with an OHCI keyboard and I can't reproduce your issue. Can you post the logs (with debug level 8) of the failure you are seeing?
With some debugging of my own added. Note that ohci_setup() is called from usb_setup(). Not from ehci_note_port(). And that this happens:
|00fe1000| ohci_hub_detect port 0: sts 0
Before this does:
|00fe3000| port 0 has a full speed device |00fe3000| ehci_hub_reset returns -1
I think this is because the EHCI device has a lower PCI function (0:14.3) than its corresponding OHCI device (0:14.4)? Trying to make sense of the loop in usb_setup(), I think it only really works if the EHCI device comes *after* the OHCI device?
[dwmw2@shinybook Quark_EDKII_0_8_0_RC3_1]$ grep -A103 'init usb' ~/git/clantonusb5.cap | sed 's/ *$//' init usb ehci_setup() _malloc zone=0x00fefe8f handle=ffffffff size=72 align=10 ret=0x00fe5e60 (detail=0x00fe5eb0) EHCI init on dev 00:14.3 (regs=0x9000d010) _malloc zone=0x00fefe8f handle=ffffffff size=4096 align=1000 ret=0x00fe4000 (detail=0x00fe5e30) /00fe4000\ Start thread |00fe4000| _malloc zone=0x00fefe9b handle=ffffffff size=4096 align=1000 ret=0x00fff000 (detail=0x00fe5e00) |00fe4000| _malloc zone=0x00fefe9b handle=ffffffff size=68 align=80 ret=0x00ffef80 (detail=0x00fe5dd0) |00fe4000| _malloc zone=0x00fefe9b handle=ffffffff size=68 align=80 ret=0x00ffef00 (detail=0x00fe5da0) |00fe4000| _malloc zone=0x00fefe8f handle=ffffffff size=28 align=10 ret=0x00fe5d50 (detail=0x00fe5d70) |00fe4000| _malloc zone=0x00fefe8f handle=ffffffff size=4096 align=1000 ret=0x00fe3000 (detail=0x00fe5d20) /00fe3000\ Start thread ohci_setup() called from usb_setup ohci_setup() _malloc zone=0x00fefe8f handle=ffffffff size=24 align=10 ret=0x00fe5cd0 (detail=0x00fe5cf0) OHCI init on dev 00:14.4 (regs=0x9000c000) _malloc zone=0x00fefe8f handle=ffffffff size=4096 align=1000 ret=0x00fe2000 (detail=0x00fe5ca0) /00fe2000\ Start thread |00fe2000| _malloc zone=0x00fefe9b handle=ffffffff size=256 align=100 ret=0x00ffee00 (detail=0x00fe5c70) |00fe2000| _malloc zone=0x00fefe9b handle=ffffffff size=16 align=10 ret=0x00ffeff0 (detail=0x00fe5c40) |00fe4000| _malloc zone=0x00fefe8f handle=ffffffff size=28 align=10 ret=0x00fe5bf0 (detail=0x00fe5c10) |00fe4000| _malloc zone=0x00fefe8f handle=ffffffff size=4096 align=1000 ret=0x00fe1000 (detail=0x00fe5bc0) /00fe1000\ Start thread init ps2port _malloc zone=0x00fefe8f handle=ffffffff size=4096 align=1000 ret=0x00fe0000 (detail=0x00fe5b90) /00fe0000\ Start thread |00fe0000| i8042_flush |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| i8042 flushed ff (status=ff) |00fe0000| WARNING - Timeout at i8042_flush:71! |00fe2000| _free 0x00fe0000 (detail=0x00fe5b90) \00fe0000/ End thread |00fe1000| ehci_hub_detect 1 not found one |00fe1000| _free 0x00fe5bf0 (detail=0x00fe5c10) |00fe3000| _free 0x00fe1000 (detail=0x00fe5bc0) \00fe1000/ End thread |00fe3000| ehci_hub_detect 0 found one init lpt Found 0 lpt ports init serial Found 0 serial ports init floppy drives init hard drives init ahci Found floppy of size 1474560 _malloc zone=0x00fefe97 handle=ffffffff size=36 align=10 ret=0x000f5720 (detail=0x00fe5c10) Mapping SPI floppy at addr 0xff800000 _malloc zone=0x00fefe8f handle=ffffffff size=24 align=10 ret=0x00fe5bc0 (detail=0x00fe5be0) Registering bootable: Ramdisk [SPI] (type:1 prio:0 data:f5720) init megasas handle_08 handle_hwpic1 irq=1 |00fe2000| _malloc zone=0x00fefe8f handle=ffffffff size=28 align=10 ret=0x00fe5b70 (detail=0x00fe5b90) |00fe2000| _malloc zone=0x00fefe8f handle=ffffffff size=4096 align=1000 ret=0x00fe1000 (detail=0x00fe5b40) /00fe1000\ Start thread |00fe1000| ohci_hub_detect port 0: sts 0 |00fe1000| _free 0x00fe5b70 (detail=0x00fe5b90) |00fe4000| _free 0x00fe1000 (detail=0x00fe5b40) \00fe1000/ End thread |00fe3000| port 0 has a full speed device |00fe3000| ehci_hub_reset returns -1 |00fe3000| port 0 speed -1 |00fe3000| _free 0x00fe5d50 (detail=0x00fe5d70) _free 0x00fe3000 (detail=0x00fe5d20) \00fe3000/ End thread handle_08 |00fe2000| _malloc zone=0x00fefe8f handle=ffffffff size=28 align=10 ret=0x00fe5d50 (detail=0x00fe5d70) |00fe2000| _malloc zone=0x00fefe8f handle=ffffffff size=4096 align=1000 ret=0x00fe3000 (detail=0x00fe5d20) /00fe3000\ Start thread |00fe3000| ohci_hub_detect port 1: sts 0 |00fe3000| _free 0x00fe5d50 (detail=0x00fe5d70) |00fe4000| _free 0x00fe3000 (detail=0x00fe5d20) \00fe3000/ End thread |00fe4000| check_ehci_ports found 0 |00fe4000| ehci_free_pipes 0x00fe5e60 |00fe2000| check_ohci_ports() found 0 devices |00fe2000| ohci_free_pipes 0x00fe5cd0 handle_08 |00fe2000| _free 0x00ffee00 (detail=0x00fe5c70) |00fe2000| _free 0x00ffeff0 (detail=0x00fe5c40) |00fe4000| _free 0x00fe2000 (detail=0x00fe5ca0) \00fe2000/ End thread |00fe4000| _free 0x00fff000 (detail=0x00fe5e00) |00fe4000| _free 0x00ffef80 (detail=0x00fe5dd0) |00fe4000| _free 0x00ffef00 (detail=0x00fe5da0) |00fe4000| _free 0x00fe5e60 (detail=0x00fe5eb0) _free 0x00fe4000 (detail=0x00fe5e30) \00fe4000/ End thread All threads complete. enter handle_16:
On Thu, 2013-12-05 at 21:26 +0000, David Woodhouse wrote:
I think this is because the EHCI device has a lower PCI function (0:14.3) than its corresponding OHCI device (0:14.4)?
It is indeed. And reading the EHCI spec, I see that the companion controllers *must* have a lower function number than the EHCI. This appears to be a hardware bug. Will chase internally, but may need a quirk for this.
On Thu, Dec 05, 2013 at 09:44:25PM +0000, David Woodhouse wrote:
On Thu, 2013-12-05 at 21:26 +0000, David Woodhouse wrote:
I think this is because the EHCI device has a lower PCI function (0:14.3) than its corresponding OHCI device (0:14.4)?
It is indeed. And reading the EHCI spec, I see that the companion controllers *must* have a lower function number than the EHCI. This appears to be a hardware bug. Will chase internally, but may need a quirk for this.
Right.
It occurred to me that we could actually simplify the code by initializing all the ehci controllers and then init all the ohci/uhci controllers. The current complex interactions between the PCI walking and EHCI setup is really hard to understand. I put together a sample patch - see attached and at:
https://github.com/KevinOConnor/seabios/tree/testing
I've only lightly tested it. I think something like this would have to wait until after v1.7.4.
-Kevin
On Thu, 2013-12-05 at 18:59 -0500, Kevin O'Connor wrote:
It occurred to me that we could actually simplify the code by initializing all the ehci controllers and then init all the ohci/uhci controllers. The current complex interactions between the PCI walking and EHCI setup is really hard to understand. I put together a sample patch - see attached and at:
That works here, and I think I prefer it to what I'd come up with:
diff --git a/src/hw/usb-ehci.c b/src/hw/usb-ehci.c index b495d6c..e800322 100644 --- a/src/hw/usb-ehci.c +++ b/src/hw/usb-ehci.c @@ -371,6 +371,9 @@ ehci_setup(struct pci_device *pci, int busid, struct pci_device *comppci) cntl->companion[count++] = comppci; else if (pci_classprog(comppci) == PCI_CLASS_SERIAL_USB_OHCI) cntl->companion[count++] = comppci; + // Intel Quark quirk. Precisely one OHCI comes *after* the EHCI. + if (comppci->bdf > pci->bdf) + break; comppci = container_of(comppci->node.next, struct pci_device, node); }
diff --git a/src/hw/usb.c b/src/hw/usb.c index 8fe741f..9c09e42 100644 --- a/src/hw/usb.c +++ b/src/hw/usb.c @@ -461,12 +461,28 @@ usb_setup(void) for (;;) { if (pci_classprog(ehcipci) == PCI_CLASS_SERIAL_USB_EHCI) { // Found an ehci controller. + + // Intel Quark got EHCI and OHCI the wrong way round :( + if (pci->vendor == PCI_VENDOR_ID_INTEL && + pci->device == 0x0939) { + struct pci_device *nextpci; + nextpci = container_of_or_null(ehcipci->node.next, + struct pci_device, node); + if (nextpci && nextpci->vendor == PCI_VENDOR_ID_INTEL && + nextpci->device == 0x093a && + pci_bdf_to_busdev(nextpci->bdf) == + pci_bdf_to_busdev(pci->bdf)) { + pci = nextpci; + found++; + } + } int ret = ehci_setup(ehcipci, count++, pci); if (ret) // Error break; count += found; - pci = ehcipci; + if (pci->bdf < ehcipci->bdf) + pci = ehcipci; break; } if (ehcipci->class == PCI_CLASS_SERIAL_USB) @@ -479,6 +495,9 @@ usb_setup(void) break; } } + if (pci->bdf > ehcipci->bdf) + // Quark OHCI. It's been handled. + continue;
if (pci_classprog(pci) == PCI_CLASS_SERIAL_USB_UHCI) uhci_setup(pci, count++);
-
David Woodhouse -
Gerd Hoffmann -
Kevin O'Connor