On Thu, Jan 23, 2014 at 02:07:36PM +0100, Gerd Hoffmann wrote:
On Do, 2014-01-23 at 12:19 +0100, Gerd Hoffmann wrote:
On Mi, 2014-01-22 at 17:52 -0500, Kevin O'Connor wrote:
Hi Gerd,
Have you had a chance to review the XHCI branch I've been testing? https://github.com/KevinOConnor/seabios/commits/xhci-testing
I'll finally have a look later today.
Looks all sane, and survived a quick test with qemu.
3 - In the original code xhci_hub_disconnect() free'd the xhci slot for a device, but this doesn't look right to me as there can be more than one slot for a given port (in the case of a hub). The testing branch above never free's slots now - do you have any thoughts on how slot allocation/free'ing should be done?
I think the original code is correct. Slots are for devices, not root hub ports. Of course the slot for the usb hub must stay active in case seabios handles devices connected to the hub, but I'm sure this is the case as seabios doesn't disconnect these hubs.
It doesn't hurt to not free slots though ...
And it helps simplifying the data structures (kill xhci_device) and handle 64bit xhci data structures.
4 - In the testing branch above, I added a patch to make hubs work (see "xhci: Hack to get hubs working."). This further overloads xhci_alloc_pipe() though. Do you think that is okay or do you think there is a better way to catch hub registration so that the xhci controller knows about it?
Need to look at the patch.
Looks good to me. Maybe factor out the device init (hub hook, set address, ...) into a separate function to streamline the code a bit.
Thanks for reviewing.
I got the Haswell based Acer c720 to enable the xhci controller. All my low speed and high speed devices seem to work okay (including via a hub). However, I'm seeing a failure on a superspeed flash drive when in the superspeed port:
XHCI port #10: 0x000a1203, powered, enabled, pls 0, speed 4 [Super] xhci_update_pipe: reconf ctl endpoint pkt size: 8 -> 9 WARNING - Timeout at xhci_event_wait:394! xhci_control: control xfer failed (cc -1)
I'm going to see if I can track that down, and I'll look to respin the patch series after that.
-Kevin