On Wed, 10 Jul 2013, Roger Quadros wrote:
> Some platforms e.g. ehci-omap can generate an interrupt
> (i.e. remote wakeup) even when the controller is suspended i.e.
> HW_ACCESSIBLE is cleared.
>
> Introduce a flag "has_wakeup_irq" in struct usb_hcd to indicate
> such cases.
>
> We tackle this case by disabling the IRQ, scheduling a
> hub resume and enabling back the IRQ after the controller has
> resumed. This ensures that the IRQ handler runs only after the
> controller is accessible.
> @@ -2248,7 +2253,21 @@ irqreturn_t usb_hcd_irq (int irq, void *__hcd)
> */
> local_irq_save(flags);
>
> - if (unlikely(HCD_DEAD(hcd) || !HCD_HW_ACCESSIBLE(hcd)))
> + if (unlikely(HCD_DEAD(hcd)))
> + rc = IRQ_NONE;
> + else if (unlikely(!HCD_HW_ACCESSIBLE(hcd) && hcd->has_wakeup_irq)) {
> + /*
> + * We got a wakeup interrupt while the controller was
> + * suspending or suspended. We can't handle it now, so
> + * disable the IRQ and resume the root hub (and hence
> + * the controller too).
> + */
> + disable_irq_nosync(hcd->irq);
> + set_bit(HCD_FLAG_IRQ_DISABLED, &hcd->flags);
> + usb_hcd_resume_root_hub(hcd);
> +
> + rc = IRQ_HANDLED;
> + } else if (unlikely(!HCD_HW_ACCESSIBLE(hcd)))
> rc = IRQ_NONE;
In the interest of minimizing the amount of work needed in the most
common case, this should be written as:
else if (unlikely(!HCD_HW_ACCESSIBLE(hcd))) {
if (hcd->has_wakeup_irq) {
...
} else
rc = IRQ_NONE;
> else if (hcd->driver->irq(hcd) == IRQ_NONE)
> rc = IRQ_NONE;
Apart from that, the patch is okay. When you rearrange the logic, you
can add my Acked-by.
Alan Stern
--
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
