Re: [PATCH] Rewrite e100_phys_id

2006-11-07 Thread Auke Kok 

Matthew Wilcox wrote:

The motivator for this was to fix the sparse warning:

drivers/net/e100.c:2418:48: warning: cast truncates bits from constant
value (83126e978d4fdf becomes 978d4fdf)
drivers/net/e100.c:2419:37: warning: cast truncates bits from constant
value (83126e978d4fdf becomes 978d4fdf)

Initially, I tried a quick fix, but when it ran into difficulties, I
looked at tg3.c to see how it does it.  I liked their way better, so I
rewrote e100.c to be similar.  It shaves ~700 bytes off the size of the
driver, and a few bytes off the size of struct nic, so I think it's a
win all round.  Tested on the internal interface of an HP Integrity rx2600.


bad news, it's completely hosed. The adapter does some indistinguishable blinking for a 
second, then stops blinking alltogether.


I might revert the code to the old situation. I guess I should have tested it initially 
right away.


I'm not even going to touch the e1000 patch for now ;)

Auke



Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]

diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index a3a08a5..aade1e9 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -556,7 +556,6 @@ struct nic {
struct params params;
struct net_device_stats net_stats;
struct timer_list watchdog;
-   struct timer_list blink_timer;
struct mii_if_info mii;
struct work_struct tx_timeout_task;
enum loopback loopback;
@@ -581,7 +580,6 @@ struct nic {
u32 rx_over_length_errors;
 
 	u8 rev_id;

-   u16 leds;
u16 eeprom_wc;
u16 eeprom[256];
spinlock_t mdio_lock;
@@ -2168,23 +2166,6 @@ err_clean_rx:
return err;
 }
 
-#define MII_LED_CONTROL	0x1B

-static void e100_blink_led(unsigned long data)
-{
-   struct nic *nic = (struct nic *)data;
-   enum led_state {
-   led_on = 0x01,
-   led_off= 0x04,
-   led_on_559 = 0x05,
-   led_on_557 = 0x07,
-   };
-
-   nic-leds = (nic-leds  led_on) ? led_off :
-   (nic-mac  mac_82559_D101M) ? led_on_557 : led_on_559;
-   mdio_write(nic-netdev, nic-mii.phy_id, MII_LED_CONTROL, nic-leds);
-   mod_timer(nic-blink_timer, jiffies + HZ / 4);
-}
-
 static int e100_get_settings(struct net_device *netdev, struct ethtool_cmd 
*cmd)
 {
struct nic *nic = netdev_priv(netdev);
@@ -2411,16 +2392,32 @@ static void e100_diag_test(struct net_de
msleep_interruptible(4 * 1000);
 }
 
+#define MII_LED_CONTROL	0x1B

 static int e100_phys_id(struct net_device *netdev, u32 data)
 {
struct nic *nic = netdev_priv(netdev);
+   int i;
+
+   enum led_state {
+   led_off= 0x04,
+   led_on_559 = 0x05,
+   led_on_557 = 0x07,
+   };
+   u16 leds = led_off;
+
+   if (data == 0)
+   data = 2;
+
+   for (i = 0; i  (data * 2); i++) {
+   leds = (leds == led_off) ?
+   (nic-mac  mac_82559_D101M) ? led_on_557 : led_on_559 :
+   led_off;
+   mdio_write(nic-netdev, nic-mii.phy_id, MII_LED_CONTROL, leds);
+   if (msleep_interruptible(500))
+   break;
+   }
 
-	if(!data || data  (u32)(MAX_SCHEDULE_TIMEOUT / HZ))

-   data = (u32)(MAX_SCHEDULE_TIMEOUT / HZ);
-   mod_timer(nic-blink_timer, jiffies);
-   msleep_interruptible(data * 1000);
-   del_timer_sync(nic-blink_timer);
-   mdio_write(netdev, nic-mii.phy_id, MII_LED_CONTROL, 0);
+   mdio_write(netdev, nic-mii.phy_id, MII_LED_CONTROL, led_off);
 
 	return 0;

 }
@@ -2633,9 +2630,6 @@ #endif
init_timer(nic-watchdog);
nic-watchdog.function = e100_watchdog;
nic-watchdog.data = (unsigned long)nic;
-   init_timer(nic-blink_timer);
-   nic-blink_timer.function = e100_blink_led;
-   nic-blink_timer.data = (unsigned long)nic;
 
 	INIT_WORK(nic-tx_timeout_task,

(void (*)(void *))e100_tx_timeout_task, netdev);

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Rewrite e100_phys_id

2006-11-07 Thread Matthew Wilcox 
On Tue, Nov 07, 2006 at 10:33:14AM -0800, Auke Kok wrote:
 Matthew Wilcox wrote:
 Tested on the internal interface of an HP Integrity rx2600.
 
 bad news, it's completely hosed. The adapter does some indistinguishable 
 blinking for a second, then stops blinking alltogether.

Weird.  I tested it on the only e100 I have access to, and it worked.
I've just reviewed the patch you quoted below, and I don't see what the
problem is.  

I wonder if this is wrong:

 -mdio_write(netdev, nic-mii.phy_id, MII_LED_CONTROL, 0);
 +mdio_write(netdev, nic-mii.phy_id, MII_LED_CONTROL, led_off);

but everything else seems pretty straight-forward.

 I might revert the code to the old situation. I guess I should have tested 
 it initially right away.
 
 I'm not even going to touch the e1000 patch for now ;)
 
 Auke
 
 
 Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]
 
 diff --git a/drivers/net/e100.c b/drivers/net/e100.c
 index a3a08a5..aade1e9 100644
 --- a/drivers/net/e100.c
 +++ b/drivers/net/e100.c
 @@ -556,7 +556,6 @@ struct nic {
  struct params params;
  struct net_device_stats net_stats;
  struct timer_list watchdog;
 -struct timer_list blink_timer;
  struct mii_if_info mii;
  struct work_struct tx_timeout_task;
  enum loopback loopback;
 @@ -581,7 +580,6 @@ struct nic {
  u32 rx_over_length_errors;
  
  u8 rev_id;
 -u16 leds;
  u16 eeprom_wc;
  u16 eeprom[256];
  spinlock_t mdio_lock;
 @@ -2168,23 +2166,6 @@ err_clean_rx:
  return err;
  }
  
 -#define MII_LED_CONTROL 0x1B
 -static void e100_blink_led(unsigned long data)
 -{
 -struct nic *nic = (struct nic *)data;
 -enum led_state {
 -led_on = 0x01,
 -led_off= 0x04,
 -led_on_559 = 0x05,
 -led_on_557 = 0x07,
 -};
 -
 -nic-leds = (nic-leds  led_on) ? led_off :
 -(nic-mac  mac_82559_D101M) ? led_on_557 : led_on_559;
 -mdio_write(nic-netdev, nic-mii.phy_id, MII_LED_CONTROL, nic-leds);
 -mod_timer(nic-blink_timer, jiffies + HZ / 4);
 -}
 -
  static int e100_get_settings(struct net_device *netdev, struct 
  ethtool_cmd *cmd)
  {
  struct nic *nic = netdev_priv(netdev);
 @@ -2411,16 +2392,32 @@ static void e100_diag_test(struct net_de
  msleep_interruptible(4 * 1000);
  }
  
 +#define MII_LED_CONTROL 0x1B
  static int e100_phys_id(struct net_device *netdev, u32 data)
  {
  struct nic *nic = netdev_priv(netdev);
 +int i;
 +
 +enum led_state {
 +led_off= 0x04,
 +led_on_559 = 0x05,
 +led_on_557 = 0x07,
 +};
 +u16 leds = led_off;
 +
 +if (data == 0)
 +data = 2;
 +
 +for (i = 0; i  (data * 2); i++) {
 +leds = (leds == led_off) ?
 +(nic-mac  mac_82559_D101M) ? led_on_557 : 
 led_on_559 :
 +led_off;
 +mdio_write(nic-netdev, nic-mii.phy_id, MII_LED_CONTROL, 
 leds);
 +if (msleep_interruptible(500))
 +break;
 +}
  
 -if(!data || data  (u32)(MAX_SCHEDULE_TIMEOUT / HZ))
 -data = (u32)(MAX_SCHEDULE_TIMEOUT / HZ);
 -mod_timer(nic-blink_timer, jiffies);
 -msleep_interruptible(data * 1000);
 -del_timer_sync(nic-blink_timer);
 -mdio_write(netdev, nic-mii.phy_id, MII_LED_CONTROL, 0);
 +mdio_write(netdev, nic-mii.phy_id, MII_LED_CONTROL, led_off);
  
  return 0;
  }
 @@ -2633,9 +2630,6 @@ #endif
  init_timer(nic-watchdog);
  nic-watchdog.function = e100_watchdog;
  nic-watchdog.data = (unsigned long)nic;
 -init_timer(nic-blink_timer);
 -nic-blink_timer.function = e100_blink_led;
 -nic-blink_timer.data = (unsigned long)nic;
  
  INIT_WORK(nic-tx_timeout_task,
  (void (*)(void *))e100_tx_timeout_task, netdev);
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Rewrite e100_phys_id

2006-11-07 Thread Auke Kok 

Matthew Wilcox wrote:

On Tue, Nov 07, 2006 at 10:33:14AM -0800, Auke Kok wrote:

Matthew Wilcox wrote:

Tested on the internal interface of an HP Integrity rx2600.
bad news, it's completely hosed. The adapter does some indistinguishable 
blinking for a second, then stops blinking alltogether.


Weird.  I tested it on the only e100 I have access to, and it worked.
I've just reviewed the patch you quoted below, and I don't see what the
problem is.  


I don't understand it either, and will dig into this after I get more coffee.

point is that `ethtool -p` now exits immediately after 500ms. it should loop until ^C is 
pressed. Somehow msleep_interruptable is always returning 0 on my platform? very strange.


Auke
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Rewrite e100_phys_id

2006-10-27 Thread Auke Kok 

Matthew Wilcox wrote:

On Thu, Oct 26, 2006 at 01:04:32PM -0700, Auke Kok wrote:
no objections, so I'll ACK it with the notion that I'm going to let our 
labs do some more testing on it with all the latest changes to it.


Thanks, Auke.  Here's the equivalent patch for e1000.  I don't have a
convenient machine to test it on, but it reduces the size of the driver
by 1.5k.


this is a bit (!) more complex than e100, so I'm going to take a bit of time to review 
this patch.


thanks,

Auke





diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index 7ecce43..1e22da6 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -257,9 +257,6 @@ #endif
struct work_struct reset_task;
uint8_t fc_autoneg;
 
-	struct timer_list blink_timer;

-   unsigned long led_status;
-
/* TX */
struct e1000_tx_ring *tx_ring;  /* One per active queue */
unsigned long tx_queue_len;
diff --git a/drivers/net/e1000/e1000_ethtool.c 
b/drivers/net/e1000/e1000_ethtool.c
index 773821e..620afa5 100644
--- a/drivers/net/e1000/e1000_ethtool.c
+++ b/drivers/net/e1000/e1000_ethtool.c
@@ -1819,61 +1819,15 @@ e1000_set_wol(struct net_device *netdev,
return 0;
 }
 
-/* toggle LED 4 times per second = 2 blinks per second */

-#define E1000_ID_INTERVAL  (HZ/4)
-
-/* bit defines for adapter-led_status */
-#define E1000_LED_ON   0
-
-static void
-e1000_led_blink_callback(unsigned long data)
-{
-   struct e1000_adapter *adapter = (struct e1000_adapter *) data;
-
-   if (test_and_change_bit(E1000_LED_ON, adapter-led_status))
-   e1000_led_off(adapter-hw);
-   else
-   e1000_led_on(adapter-hw);
-
-   mod_timer(adapter-blink_timer, jiffies + E1000_ID_INTERVAL);
-}
-
 static int
 e1000_phys_id(struct net_device *netdev, uint32_t data)
 {
struct e1000_adapter *adapter = netdev_priv(netdev);
 
-	if (!data || data  (uint32_t)(MAX_SCHEDULE_TIMEOUT / HZ))

-   data = (uint32_t)(MAX_SCHEDULE_TIMEOUT / HZ);
-
-   if (adapter-hw.mac_type  e1000_82571) {
-   if (!adapter-blink_timer.function) {
-   init_timer(adapter-blink_timer);
-   adapter-blink_timer.function = 
e1000_led_blink_callback;
-   adapter-blink_timer.data = (unsigned long) adapter;
-   }
-   e1000_setup_led(adapter-hw);
-   mod_timer(adapter-blink_timer, jiffies);
-   msleep_interruptible(data * 1000);
-   del_timer_sync(adapter-blink_timer);
-   } else if (adapter-hw.phy_type == e1000_phy_ife) {
-   if (!adapter-blink_timer.function) {
-   init_timer(adapter-blink_timer);
-   adapter-blink_timer.function = 
e1000_led_blink_callback;
-   adapter-blink_timer.data = (unsigned long) adapter;
-   }
-   mod_timer(adapter-blink_timer, jiffies);
-   msleep_interruptible(data * 1000);
-   del_timer_sync(adapter-blink_timer);
-   e1000_write_phy_reg((adapter-hw), 
IFE_PHY_SPECIAL_CONTROL_LED, 0);
-   } else {
-   e1000_blink_led_start(adapter-hw);
-   msleep_interruptible(data * 1000);
-   }
+   if (data == 0)
+   data = 2;
 
-	e1000_led_off(adapter-hw);

-   clear_bit(E1000_LED_ON, adapter-led_status);
-   e1000_cleanup_led(adapter-hw);
+   e1000_blink_led(adapter-hw, data);
 
 	return 0;

 }
diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
index 65077f3..db5e999 100644
--- a/drivers/net/e1000/e1000_hw.c
+++ b/drivers/net/e1000/e1000_hw.c
@@ -6071,7 +6071,7 @@ e1000_id_led_init(struct e1000_hw * hw)
  *
  * hw - Struct containing variables accessed by shared code
  */
-int32_t
+static int32_t
 e1000_setup_led(struct e1000_hw *hw)
 {
 uint32_t ledctl;
@@ -6123,50 +6123,11 @@ e1000_setup_led(struct e1000_hw *hw)
 
 
 /**

- * Used on 82571 and later Si that has LED blink bits.
- * Callers must use their own timer and should have already called
- * e1000_id_led_init()
- * Call e1000_cleanup led() to stop blinking
- *
- * hw - Struct containing variables accessed by shared code
- */
-int32_t
-e1000_blink_led_start(struct e1000_hw *hw)
-{
-int16_t  i;
-uint32_t ledctl_blink = 0;
-
-DEBUGFUNC(e1000_id_led_blink_on);
-
-if (hw-mac_type  e1000_82571) {
-/* Nothing to do */
-return E1000_SUCCESS;
-}
-if (hw-media_type == e1000_media_type_fiber) {
-/* always blink LED0 for PCI-E fiber */
-ledctl_blink = E1000_LEDCTL_LED0_BLINK |
- (E1000_LEDCTL_MODE_LED_ON  
E1000_LEDCTL_LED0_MODE_SHIFT);
-} else {
-

[PATCH] Rewrite e100_phys_id

2006-10-26 Thread Matthew Wilcox 

The motivator for this was to fix the sparse warning:

drivers/net/e100.c:2418:48: warning: cast truncates bits from constant
value (83126e978d4fdf becomes 978d4fdf)
drivers/net/e100.c:2419:37: warning: cast truncates bits from constant
value (83126e978d4fdf becomes 978d4fdf)

Initially, I tried a quick fix, but when it ran into difficulties, I
looked at tg3.c to see how it does it.  I liked their way better, so I
rewrote e100.c to be similar.  It shaves ~700 bytes off the size of the
driver, and a few bytes off the size of struct nic, so I think it's a
win all round.  Tested on the internal interface of an HP Integrity rx2600.

Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]

diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index a3a08a5..aade1e9 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -556,7 +556,6 @@ struct nic {
struct params params;
struct net_device_stats net_stats;
struct timer_list watchdog;
-   struct timer_list blink_timer;
struct mii_if_info mii;
struct work_struct tx_timeout_task;
enum loopback loopback;
@@ -581,7 +580,6 @@ struct nic {
u32 rx_over_length_errors;
 
u8 rev_id;
-   u16 leds;
u16 eeprom_wc;
u16 eeprom[256];
spinlock_t mdio_lock;
@@ -2168,23 +2166,6 @@ err_clean_rx:
return err;
 }
 
-#define MII_LED_CONTROL0x1B
-static void e100_blink_led(unsigned long data)
-{
-   struct nic *nic = (struct nic *)data;
-   enum led_state {
-   led_on = 0x01,
-   led_off= 0x04,
-   led_on_559 = 0x05,
-   led_on_557 = 0x07,
-   };
-
-   nic-leds = (nic-leds  led_on) ? led_off :
-   (nic-mac  mac_82559_D101M) ? led_on_557 : led_on_559;
-   mdio_write(nic-netdev, nic-mii.phy_id, MII_LED_CONTROL, nic-leds);
-   mod_timer(nic-blink_timer, jiffies + HZ / 4);
-}
-
 static int e100_get_settings(struct net_device *netdev, struct ethtool_cmd 
*cmd)
 {
struct nic *nic = netdev_priv(netdev);
@@ -2411,16 +2392,32 @@ static void e100_diag_test(struct net_de
msleep_interruptible(4 * 1000);
 }
 
+#define MII_LED_CONTROL0x1B
 static int e100_phys_id(struct net_device *netdev, u32 data)
 {
struct nic *nic = netdev_priv(netdev);
+   int i;
+
+   enum led_state {
+   led_off= 0x04,
+   led_on_559 = 0x05,
+   led_on_557 = 0x07,
+   };
+   u16 leds = led_off;
+
+   if (data == 0)
+   data = 2;
+
+   for (i = 0; i  (data * 2); i++) {
+   leds = (leds == led_off) ?
+   (nic-mac  mac_82559_D101M) ? led_on_557 : led_on_559 :
+   led_off;
+   mdio_write(nic-netdev, nic-mii.phy_id, MII_LED_CONTROL, leds);
+   if (msleep_interruptible(500))
+   break;
+   }
 
-   if(!data || data  (u32)(MAX_SCHEDULE_TIMEOUT / HZ))
-   data = (u32)(MAX_SCHEDULE_TIMEOUT / HZ);
-   mod_timer(nic-blink_timer, jiffies);
-   msleep_interruptible(data * 1000);
-   del_timer_sync(nic-blink_timer);
-   mdio_write(netdev, nic-mii.phy_id, MII_LED_CONTROL, 0);
+   mdio_write(netdev, nic-mii.phy_id, MII_LED_CONTROL, led_off);
 
return 0;
 }
@@ -2633,9 +2630,6 @@ #endif
init_timer(nic-watchdog);
nic-watchdog.function = e100_watchdog;
nic-watchdog.data = (unsigned long)nic;
-   init_timer(nic-blink_timer);
-   nic-blink_timer.function = e100_blink_led;
-   nic-blink_timer.data = (unsigned long)nic;
 
INIT_WORK(nic-tx_timeout_task,
(void (*)(void *))e100_tx_timeout_task, netdev);
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Rewrite e100_phys_id

2006-10-26 Thread Jeff Garzik 
On Thu, Oct 26, 2006 at 01:11:55PM -0600, Matthew Wilcox wrote:
 
 The motivator for this was to fix the sparse warning:
 
 drivers/net/e100.c:2418:48: warning: cast truncates bits from constant
 value (83126e978d4fdf becomes 978d4fdf)
 drivers/net/e100.c:2419:37: warning: cast truncates bits from constant
 value (83126e978d4fdf becomes 978d4fdf)
 
 Initially, I tried a quick fix, but when it ran into difficulties, I
 looked at tg3.c to see how it does it.  I liked their way better, so I
 rewrote e100.c to be similar.  It shaves ~700 bytes off the size of the
 driver, and a few bytes off the size of struct nic, so I think it's a
 win all round.  Tested on the internal interface of an HP Integrity rx2600.
 
 Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]

Seems sane to me...  I'll pick it up, if Auke doesn't...

Jeff



-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Rewrite e100_phys_id

2006-10-26 Thread Auke Kok 

Jeff Garzik wrote:

On Thu, Oct 26, 2006 at 01:11:55PM -0600, Matthew Wilcox wrote:

The motivator for this was to fix the sparse warning:

drivers/net/e100.c:2418:48: warning: cast truncates bits from constant
value (83126e978d4fdf becomes 978d4fdf)
drivers/net/e100.c:2419:37: warning: cast truncates bits from constant
value (83126e978d4fdf becomes 978d4fdf)

Initially, I tried a quick fix, but when it ran into difficulties, I
looked at tg3.c to see how it does it.  I liked their way better, so I
rewrote e100.c to be similar.  It shaves ~700 bytes off the size of the
driver, and a few bytes off the size of struct nic, so I think it's a
win all round.  Tested on the internal interface of an HP Integrity rx2600.

Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]


Seems sane to me...  I'll pick it up, if Auke doesn't...


no objections, so I'll ACK it with the notion that I'm going to let our labs do some 
more testing on it with all the latest changes to it.


Jeff, I will stack it on the patches I have for 2.6.20 and push those out before the 
weekend.


Cheers,

Auke
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Rewrite e100_phys_id

2006-10-26 Thread Matthew Wilcox 
On Thu, Oct 26, 2006 at 01:04:32PM -0700, Auke Kok wrote:
 no objections, so I'll ACK it with the notion that I'm going to let our 
 labs do some more testing on it with all the latest changes to it.

Thanks, Auke.  Here's the equivalent patch for e1000.  I don't have a
convenient machine to test it on, but it reduces the size of the driver
by 1.5k.

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index 7ecce43..1e22da6 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -257,9 +257,6 @@ #endif
struct work_struct reset_task;
uint8_t fc_autoneg;
 
-   struct timer_list blink_timer;
-   unsigned long led_status;
-
/* TX */
struct e1000_tx_ring *tx_ring;  /* One per active queue */
unsigned long tx_queue_len;
diff --git a/drivers/net/e1000/e1000_ethtool.c 
b/drivers/net/e1000/e1000_ethtool.c
index 773821e..620afa5 100644
--- a/drivers/net/e1000/e1000_ethtool.c
+++ b/drivers/net/e1000/e1000_ethtool.c
@@ -1819,61 +1819,15 @@ e1000_set_wol(struct net_device *netdev,
return 0;
 }
 
-/* toggle LED 4 times per second = 2 blinks per second */
-#define E1000_ID_INTERVAL  (HZ/4)
-
-/* bit defines for adapter-led_status */
-#define E1000_LED_ON   0
-
-static void
-e1000_led_blink_callback(unsigned long data)
-{
-   struct e1000_adapter *adapter = (struct e1000_adapter *) data;
-
-   if (test_and_change_bit(E1000_LED_ON, adapter-led_status))
-   e1000_led_off(adapter-hw);
-   else
-   e1000_led_on(adapter-hw);
-
-   mod_timer(adapter-blink_timer, jiffies + E1000_ID_INTERVAL);
-}
-
 static int
 e1000_phys_id(struct net_device *netdev, uint32_t data)
 {
struct e1000_adapter *adapter = netdev_priv(netdev);
 
-   if (!data || data  (uint32_t)(MAX_SCHEDULE_TIMEOUT / HZ))
-   data = (uint32_t)(MAX_SCHEDULE_TIMEOUT / HZ);
-
-   if (adapter-hw.mac_type  e1000_82571) {
-   if (!adapter-blink_timer.function) {
-   init_timer(adapter-blink_timer);
-   adapter-blink_timer.function = 
e1000_led_blink_callback;
-   adapter-blink_timer.data = (unsigned long) adapter;
-   }
-   e1000_setup_led(adapter-hw);
-   mod_timer(adapter-blink_timer, jiffies);
-   msleep_interruptible(data * 1000);
-   del_timer_sync(adapter-blink_timer);
-   } else if (adapter-hw.phy_type == e1000_phy_ife) {
-   if (!adapter-blink_timer.function) {
-   init_timer(adapter-blink_timer);
-   adapter-blink_timer.function = 
e1000_led_blink_callback;
-   adapter-blink_timer.data = (unsigned long) adapter;
-   }
-   mod_timer(adapter-blink_timer, jiffies);
-   msleep_interruptible(data * 1000);
-   del_timer_sync(adapter-blink_timer);
-   e1000_write_phy_reg((adapter-hw), 
IFE_PHY_SPECIAL_CONTROL_LED, 0);
-   } else {
-   e1000_blink_led_start(adapter-hw);
-   msleep_interruptible(data * 1000);
-   }
+   if (data == 0)
+   data = 2;
 
-   e1000_led_off(adapter-hw);
-   clear_bit(E1000_LED_ON, adapter-led_status);
-   e1000_cleanup_led(adapter-hw);
+   e1000_blink_led(adapter-hw, data);
 
return 0;
 }
diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
index 65077f3..db5e999 100644
--- a/drivers/net/e1000/e1000_hw.c
+++ b/drivers/net/e1000/e1000_hw.c
@@ -6071,7 +6071,7 @@ e1000_id_led_init(struct e1000_hw * hw)
  *
  * hw - Struct containing variables accessed by shared code
  */
-int32_t
+static int32_t
 e1000_setup_led(struct e1000_hw *hw)
 {
 uint32_t ledctl;
@@ -6123,50 +6123,11 @@ e1000_setup_led(struct e1000_hw *hw)
 
 
 /**
- * Used on 82571 and later Si that has LED blink bits.
- * Callers must use their own timer and should have already called
- * e1000_id_led_init()
- * Call e1000_cleanup led() to stop blinking
- *
- * hw - Struct containing variables accessed by shared code
- */
-int32_t
-e1000_blink_led_start(struct e1000_hw *hw)
-{
-int16_t  i;
-uint32_t ledctl_blink = 0;
-
-DEBUGFUNC(e1000_id_led_blink_on);
-
-if (hw-mac_type  e1000_82571) {
-/* Nothing to do */
-return E1000_SUCCESS;
-}
-if (hw-media_type == e1000_media_type_fiber) {
-/* always blink LED0 for PCI-E fiber */
-ledctl_blink = E1000_LEDCTL_LED0_BLINK |
- (E1000_LEDCTL_MODE_LED_ON  
E1000_LEDCTL_LED0_MODE_SHIFT);
-} else {
-/* set the blink bit for each LED that's on (0x0E) in ledctl_mode2 */
-ledctl_blink = hw-ledctl_mode2;
-for