Re: [PATCH] e1000e: Use deferrable timer for watchdog

Parag Warudkar wrote:

On Dec 20, 2007 12:05 PM, Kok, Auke [off-list ref] wrote:

quoted

I can't even apply this patch and the e1000 one... not only is it whitespace
damaged it is also not properly formatted as patch at all. If you want me to take
these patches seriously, then please fix the formatting issues.

Sigh - I use Pine, follow Documents/email-clients.txt for the
recommended settings and obviously the pathces are not generated with
whitespace damage at my end as I test those before sending out.

So although I hate to see this happen there is nothing at this moment
that I can do - except for attaching the patch instead of inlining it.
Since they have already been reviewed inline, please see if the
attached patches work for you.

here's what the files in my Maildir spool look like in vim (my vim displays a '»'
char for tabs and a "¶" for EOL):

 76 --- linux-2.6/drivers/net/e1000e/netdev.c»      2007-12-07 10:04:39.00000000
 77 +++ linux-2.6-work/drivers/net/e1000e/netdev.c» 2007-12-18 20:45:59.00000000
 78 @@ -3899,7 +3899,7 @@¶
 79   »     »       goto err_eeprom;¶
 80   »     }¶
 81 ¶
 82 -»      init_timer(&adapter->watchdog_timer);¶
 83 +»      init_timer_deferrable(&adapter->watchdog_timer);¶
 84   »     adapter->watchdog_timer.function = &e1000_watchdog;¶
 85   »     adapter->watchdog_timer.data = (unsigned long) adapter;¶
 86 ¶
 87 --¶

notice that there are two spaces instead of 1. Also there's no line heading the
diff with 'diff a/foo b/foo' which is what throws of stg. And the -p option is
missing.


as for content, the patch looks OK with me. I ran the numbers and allthough there
was a slight average delay in the link up detection time it is negligeable (less
than 0.2sec difference over a bunch of measurements), and I confirmed your
powertop numbers are correct. As for the timer interval, the watchdog may already
be delayed up to 3 seconds safely, this doesn't change that.

I'll forward the patch, Care to make one for e100? plenty of laptops with those
still around! The embedded guys would love it I think.

Thanks,

Auke