On Wed, Feb 01, 2012 at 02:36:21PM -0500, Alan Stern wrote:
> On Wed, 1 Feb 2012, Sarah Sharp wrote:
>
> > On Tue, Jan 31, 2012 at 01:29:28PM -0500, Alan Stern wrote:
> > > On Tue, 31 Jan 2012, Sarah Sharp wrote:
> > >
> > > > > In fact, all HCDs need to address this sort of problem. It looks like
> > > > > the best answer is to disable IRQ generation at the start of
> > > > > bus_suspend and call synchronize_irq(). As a bonus, the remainder of
> > > > > bus_suspend can run without holding the private lock, since no other
> > > > > threads will enter the driver until the suspend finishes and IRQ
> > > > > generation is re-enabled.
> > > >
> > > > What you described (disabling IRQs, etc) needs to be done in
> > > > xhci_bus_suspend() as well, correct? Or were you thinking it could be
> > > > done in the USB core, since other HCDs also have this problem?
> > >
> > > It can't be done in the core, because the core doesn't know how to tell
> > > the hardware to stop generating IRQs
> >
> > I have to disable IRQs for all the processors before synchronizing the
> > MSI-X vectors, correct? I'm not sure how to do that. (And I did
> > attempt to RTFM, but docbooks no longer builds and
> > Documentation/PCI/MSI-how-to.txt doesn't have anything terribly useful.)
> > If I don't disable IRQs on all processors, synchronize_irq() will ensure
> > that the MSI-X interrupts on different processors finish running, but it
> > doesn't prevent them from running again.
>
> I really don't know anything about MSI or MSI-X. However it seems
> likely that you don't need to disable interrupts on any processors.
> Just tell the xHCI hardware to stop generating interrupts. That's the
> INTE bit in the USBCMD register, right?
Well, it's a moot point now, because looking at the xhci_bus_suspend
method, it has to issue several commands and wait for the host
controller to finish processing them before it can return, if there's a
port that's not suspended:
while (port_index--) {
/* suspend the port if the port is not suspended */
u32 t1, t2;
int slot_id;
t1 = xhci_readl(xhci, port_array[port_index]);
t2 = xhci_port_state_to_neutral(t1);
if ((t1 & PORT_PE) && !(t1 & PORT_PLS_MASK)) {
xhci_dbg(xhci, "port %d not suspended\n", port_index);
slot_id = xhci_find_slot_id_by_port(hcd, xhci,
port_index + 1);
if (slot_id) {
spin_unlock_irqrestore(&xhci->lock, flags);
xhci_stop_device(xhci, slot_id, 1);
spin_lock_irqsave(&xhci->lock, flags);
}
t2 &= ~PORT_PLS_MASK;
t2 |= PORT_LINK_STROBE | XDEV_U3;
set_bit(port_index, &bus_state->bus_suspended);
}
Is there any reason why the USB core would be suspending a bus if all
attached devices were not suspended? I think this shouldn't be an issue
if we're disabling IRQs in xhci_bus_suspend(), and making sure that
we're not in the middle of a resume.
As to your question about the INTE bit, I don't think I really want to
do that. I think I have to stop the host (set the stop bit) to disable
interrupts that way. From a quick search through the xHCI spec, it only
talks about turning that bit on during initialization. I can't find any
text about what the host is supposed to do if that bit gets turned off
while it's running.
The reason I'm concerned about setting the INTE bit while the host is
running is because xhci_bus_suspend is can be called for one portion of
the roothub, while the other is still trying to run. E.g. the USB 2.0
bus is being suspended while the USB 3.0 bus is still active.
However, both Arjan and HPA say that if the device tries to interrupt
while it's MSI-X vector is disabled, there's no guarantee that the xHCI
driver will get the interrupt later. So disabling MSI or MSI-X vectors
doesn't seem to be a good solution.
Maybe I just need to look for the resume bits in the PCI suspend
function (xhci_suspend), and bail out there if they're set?
Sarah
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
