On Thu, Dec 23, 2010 at 05:34:22PM -0500, Alan Stern wrote:
> On Thu, 23 Dec 2010, Sarah Sharp wrote:
>
> > > Then instead of guessing about whether or not pdev->irq should be the
> > > same as hcd->irq, and what it might mean if they are different, just
> > > add a new "don't synchronize irqs" flag to the usb_hcd structure.
> >
> > Like so?
> >
> > diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> > index 3799573..da9d96d 100644
> > --- a/drivers/usb/core/hcd-pci.c
> > +++ b/drivers/usb/core/hcd-pci.c
> > @@ -406,7 +406,12 @@ static int suspend_common(struct device *dev, bool do_wakeup)
> > return retval;
> > }
> >
> > - synchronize_irq(pci_dev->irq);
> > + /* If MSI-X is enabled, the driver will have synchronized all vectors in
> > + * pci_suspend(). If MSI or legacy PCI is enabled, that will be
> > + * synchronized here.
> > + */
> > + if (!HCD_MSIX_ENABLED(hcd))
> > + synchronize_irq(pci_dev->irq);
>
> > diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
> > index 0b6e751..d9a8983 100644
> > --- a/include/linux/usb/hcd.h
> > +++ b/include/linux/usb/hcd.h
> > @@ -99,6 +99,7 @@ struct usb_hcd {
> > #define HCD_FLAG_POLL_RH 2 /* poll for rh status? */
> > #define HCD_FLAG_POLL_PENDING 3 /* status has changed? */
> > #define HCD_FLAG_WAKEUP_PENDING 4 /* root hub is resuming? */
> > +#define HCD_FLAG_MSIX 5 /* driver has MSI-X enabled? */
> >
> > /* The flags can be tested using these macros; they are likely to
> > * be slightly faster than test_bit().
> > @@ -108,6 +109,7 @@ struct usb_hcd {
> > #define HCD_POLL_RH(hcd) ((hcd)->flags & (1U << HCD_FLAG_POLL_RH))
> > #define HCD_POLL_PENDING(hcd) ((hcd)->flags & (1U << HCD_FLAG_POLL_PENDING))
> > #define HCD_WAKEUP_PENDING(hcd) ((hcd)->flags & (1U << HCD_FLAG_WAKEUP_PENDING))
> > +#define HCD_MSIX_ENABLED(hcd) ((hcd)->flags & (1U << HCD_FLAG_MSIX))
> >
> > /* Flags that get set only during HCD registration or removal. */
> > unsigned rh_registered:1;/* is root hub registered? */
>
> Exactly. Except that since this flag gets set only when the driver is
> probed, you can use a regular bitflag (like rh_registered above)
> instead of an atomic bitflag. Right? I assume you don't turn MSI-X
> on and off dynamically while things are running.
I think MSI-X is disabled when xhci_stop() is called, or a host
controller dies. But it's not dynamically turned on and off during
normal operation.
By the way, I'm running into issues with these flags and the split
roothub patches. Most of these flags really apply to the host
controller hardware (e.g. has the hardware seen an interrupt, does the
hardware need to be polled). But now that I've got two roothubs and two
usb_hcd structures, I'm having to set flags in both of them, which makes
the atomicity useless.
The host controller state (hcd->state) is another sorry mess.
"HC_STATE_HALT" and "HC_STATE_RUNNING" really only apply to the host
controller hardware, but "HC_STATE_RESUMING" and "HC_STATE_SUSPENDED"
really apply separately to each roothub bus. So sometimes I need to set
state for both roothubs, and sometimes I don't.
I'll post my updated patches once I get the suspend and resume case
somewhat working, but I wondered if you had any suggestions about
hcd->flags.
Sarah Sharp
--
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
