On Wed, 21 Aug 2013, Ming Lei wrote: > This patch introduces ehci_disable_event(), which is applied on > IAA_WATCHDOG and START_UNLINK_INTR timeouts if the two corresponding > events(IAA and intr URB submission) happened, so that we may avoid > unnecessary CPU wakeup by canceling the timer. > > One simple test indicates about 7~8 timer expirations/sec > (20% ~ 30% of totoal timer expirations/sec) can be saved when one asix > network NIC is connected to EHCI without network traffic(the device > responds interrupt poll 7~8 times per second constantly for link status > query). > > Cc: Alan Stern <[email protected]> > Signed-off-by: Ming Lei <[email protected]> > --- > drivers/usb/host/ehci-hcd.c | 12 +----------- > drivers/usb/host/ehci-sched.c | 4 +++- > drivers/usb/host/ehci-timer.c | 16 ++++++++++++++++ > 3 files changed, 20 insertions(+), 12 deletions(-) > > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c > index 73c7299..549da36 100644 > --- a/drivers/usb/host/ehci-hcd.c > +++ b/drivers/usb/host/ehci-hcd.c > @@ -738,17 +738,7 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd) > if (status & STS_IAA) { > > /* Turn off the IAA watchdog */ > - ehci->enabled_hrtimer_events &= ~BIT(EHCI_HRTIMER_IAA_WATCHDOG); > - > - /* > - * Mild optimization: Allow another IAAD to reset the > - * hrtimer, if one occurs before the next expiration. > - * In theory we could always cancel the hrtimer, but > - * tests show that about half the time it will be reset > - * for some other event anyway. > - */ > - if (ehci->next_hrtimer_event == EHCI_HRTIMER_IAA_WATCHDOG) > - ++ehci->next_hrtimer_event; > + ehci_disable_event(ehci, EHCI_HRTIMER_IAA_WATCHDOG); > > /* guard against (alleged) silicon errata */ > if (cmd & CMD_IAAD) > diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c > index 6631089..83be03f 100644 > --- a/drivers/usb/host/ehci-sched.c > +++ b/drivers/usb/host/ehci-sched.c > @@ -610,9 +610,11 @@ static void cancel_unlink_wait_intr(struct ehci_hcd > *ehci, struct ehci_qh *qh) > list_del_init(&qh->unlink_node); > > /* > - * TODO: disable the event of EHCI_HRTIMER_START_UNLINK_INTR for > + * disable the event of EHCI_HRTIMER_START_UNLINK_INTR for > * avoiding unnecessary CPU wakeup > */ > + if (list_empty(&ehci->intr_unlink_wait)) > + ehci_disable_event(ehci, EHCI_HRTIMER_START_UNLINK_INTR); > } > > static void start_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh) > diff --git a/drivers/usb/host/ehci-timer.c b/drivers/usb/host/ehci-timer.c > index 424ac5d..983d063 100644 > --- a/drivers/usb/host/ehci-timer.c > +++ b/drivers/usb/host/ehci-timer.c > @@ -99,6 +99,22 @@ static void ehci_enable_event(struct ehci_hcd *ehci, > unsigned event, > } > } > > +/* Warning: don't call this function from hrtimer handler context */ > +static void ehci_disable_event(struct ehci_hcd *ehci, unsigned event) > +{ > + ehci->enabled_hrtimer_events &= ~(1 << event); > + > + if (!ehci->enabled_hrtimer_events) { > + ehci->next_hrtimer_event = EHCI_HRTIMER_NO_EVENT; > + hrtimer_cancel(&ehci->hrtimer); > + } else if (ehci->next_hrtimer_event == event) { > + ehci->next_hrtimer_event = EHCI_HRTIMER_NO_EVENT; > + ehci_enable_event(ehci, > + ffs(ehci->enabled_hrtimer_events) - 1, > + false); > + } > +} > + > > /* Poll the STS_ASS status bit; see when it agrees with CMD_ASE */ > static void ehci_poll_ASS(struct ehci_hcd *ehci)
Okay. You can add Acked-by: Alan Stern <[email protected]> and submit it as a version-2 patch. By the way, even though it's a little late to ask this... Why did you decide to move only the giveback routine into a tasklet, instead of moving the entire interrupt handler? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
