Messages in this thread | | | Date | Thu, 2 Oct 2008 16:56:49 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] USB: improve ehci_watchdog's side effect in CPU power management |
| |
On Thu, 25 Sep 2008 17:25:44 +0800 Yi Yang <yi.y.yang@intel.com> wrote:
> ehci_watchdog will wake up CPU very frequently so that CPU > stays at C3 very short, average residence time is about 50 > ms on Aspire One, but we expect it should be about 1 second > or more, so this kind of periodic timer is very bad for power > saving. > > We can't remove this timer because of some bad USB controller > chipset, but at least we should reduce its side effect to as > possible as low. > > This patch can make CPU stay at C3 longer, average residence time > is about twice as long as original. > > Please consider to apply it, thanks > > Signed-off-by: Yi Yang <yi.y.yang@intel.com> > --- > ehci.h | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h > index 5799298..9d530d9 100644 > --- a/drivers/usb/host/ehci.h > +++ b/drivers/usb/host/ehci.h > @@ -181,14 +181,16 @@ timer_action (struct ehci_hcd *ehci, enum ehci_timer_action action) > * the async ring; just the I/O watchdog. Note that if a > * SHRINK were pending, OFF would never be requested. > */ > - if (timer_pending(&ehci->watchdog) > - && ((BIT(TIMER_ASYNC_SHRINK) | BIT(TIMER_ASYNC_OFF)) > - & ehci->actions)) > - return; > + enum ehci_timer_action oldactions = ehci->actions; > > if (!test_and_set_bit (action, &ehci->actions)) { > unsigned long t; > > + if (timer_pending(&ehci->watchdog) > + && ((BIT(TIMER_ASYNC_SHRINK) | BIT(TIMER_ASYNC_OFF)) > + & oldactions)) > + return; > + > switch (action) { > case TIMER_IO_WATCHDOG: > t = EHCI_IO_JIFFIES; > @@ -204,7 +206,7 @@ timer_action (struct ehci_hcd *ehci, enum ehci_timer_action action) > t = DIV_ROUND_UP(EHCI_SHRINK_FRAMES * HZ, 1000) + 1; > break; > } > - mod_timer(&ehci->watchdog, t + jiffies); > + mod_timer(&ehci->watchdog, round_jiffies(t + jiffies)); > } > } >
<looks>
<regrets it>
Why does this:
t = DIV_ROUND_UP(EHCI_SHRINK_FRAMES * HZ, 1000) + 1;
add "1000" to a jiffies value when it doesn't know what HZ is? It'll be adding anywhere from one second up to ten seconds to the timeout interval depending upon compile-time options.
I suspect s/1000/HZ/ would improve things here. Or just delete it - doesn't the subsequent round_jiffies() do the same thing, only better? This code needs help, I suspect.
Also, do we really need to inline this large function into at least five callsites?
| |