On Mon, 17 Jun 2013, Sarah Sharp wrote:
Correct me if I'm wrong here. The original idea was to switch
everything over to xHCI during some early stage (like when the xHCI
controller is first passed to the pci-quirks.c code) and switch back to
EHCI at shutdown. As a refinement, you want to switch over to xHCI
again following system resume, in case the BIOS messed things up. Is
there anything else?
Yes, that was the idea.
Why does the old code need to be changed? The only problem I see is
that it may be a little too elaborate. For example, why bother trying
to do the switch when the EHCI controller resumes? Won't everything
work okay if you wait until the xHCI controller resumes?
The code needs to be changed because we don't want to keep adding new
PCI IDs for all the new Intel hosts.
Okay. That's simple enough; just remove the product ID checks.
Mathias also didn't like that we
inefficiently walked the PCI bus, looking for the xHCI host in the EHCI
resume routine, so he stored the xHCI pointer. Perhaps this would be
more clear if these two goals were broken up into two patches?
Why not just remove all that stuff from the EHCI resume routine? It
would be a lot simpler. See below.
Why does the code check so hard to see whether or not there are any
switchable ports? Why not just always try to set the switch on any
Intel controller?
Not all ports may be exposed on the platform (because there are
different skews). If we don't check which ports to switchover, some
BIOSes freak out on either shutdown, or suspend. I don't remember
which. So we need to pay attention to the port switchover masks.
Sorry, I wasn't clear. Yes, by all means, do check the switchover
masks. But there's no need to check the product ID; checking just the
vendor ID should be good enough.
Why is it necessary to search through all the PCI devices? Why is it
necessary to store any sort of list of switchable controllers?
We don't know which host will resume from suspend first.
Assume we have a broken BIOS that switches ports back to EHCI on resume.
We don't want the USB core to notice EHCI connects and start servicing
them before xHCI resumes. Therefore, we need to have both the xHCI and
the EHCI resume functions attempt to switch the ports over to xHCI.
The USB core won't notice anything, because the hub driver doesn't
start running again until after all the devices (including host
controllers) have been resumed. In other words, the khubd thread is
freezable. (This is not by some random chance; it was an explicit
decision that we don't want the hub driver mucking things up while
we're in the middle of a big transition like system suspend or resume.
You can rely on it.)
So it won't matter if a device gets switched over to EHCI for a while.
When the xHCI controller resumes, the device will get switched back to
it, and usbcore will see only that there was a disconnect/reconnect
during suspend.
That means for EHCI, we either need to search the PCI tree for the xHCI
host on resume (because that's where the port switchover mechanism
registers are), or we need to store the xHCI PCI host pointer somewhere.
Mathias chose to store the pointer.
The reasoning above shows that there's no need either to search the
entire PCI tree or to store any host controller pointers.
Why should hot-unplug be an issue? Or multiple xHCI controllers?
This is not a problem right now, I'm perhaps being paranoid. :) For all
current Intel systems, AFAIK, there will only be one xHCI host
controller, and it will be a part of the PCH, so you won't see any PCI
hot unplug events unless someone removes the entire PCH (which I don't
know if we support?).
Even if someone in the future decides to put an Intel xHCI controller
on a PCIe card, there shouldn't be any trouble if you handle things as
decribed above. Agreed?
