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.
Beyond a general review, I have a few questions on the XHCI code:
1 - It looks like usb-xhci.c is missing a copyright. Are you okay if I add one that looks like what you have on ahci.c?
Yes.
2 - What is the mdelay(100) in configure_xhci() for? I don't understand the comment there. Also, any delay should probably be an msleep(), so that seabios can continue to bring up other devices. (An mdelay() doesn't let other threads run.)
It gives the xhci controller some time to detect the usb devices.
A real os would just enable port status notification events, then do a scan once (without delay), and depend on the irq firing for ports not ready yet on the initial scan.
The seabios driver has no irq handler so we can't do that, and I've settled for the somewhat hackish delay. And, yes, it should be a msleep.
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.
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.
cheers, Gerd