Re: [PATCH] Rewrite e100_phys_id

[Matthew Wilcox <matthew@wil.cx>]

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 (i=0; i < 4; i++)
-            if (((hw->ledctl_mode2 >> (i * 8)) & 0xFF) ==
-                E1000_LEDCTL_MODE_LED_ON)
-                ledctl_blink |= (E1000_LEDCTL_LED0_BLINK << (i * 8));
-    }
-
-    E1000_WRITE_REG(hw, LEDCTL, ledctl_blink);
-
-    return E1000_SUCCESS;
-}
-
-/******************************************************************************
  * Restores the saved state of the SW controlable LED.
  *
  * hw - Struct containing variables accessed by shared code
  *****************************************************************************/
-int32_t
-e1000_cleanup_led(struct e1000_hw *hw)
+static int32_t e1000_cleanup_led(struct e1000_hw *hw)
 {
     int32_t ret_val = E1000_SUCCESS;
 
@@ -6207,8 +6168,7 @@ e1000_cleanup_led(struct e1000_hw *hw)
  *
  * hw - Struct containing variables accessed by shared code
  *****************************************************************************/
-int32_t
-e1000_led_on(struct e1000_hw *hw)
+static int32_t e1000_led_on(struct e1000_hw *hw)
 {
     uint32_t ctrl = E1000_READ_REG(hw, CTRL);
 
@@ -6258,8 +6218,7 @@ e1000_led_on(struct e1000_hw *hw)
  *
  * hw - Struct containing variables accessed by shared code
  *****************************************************************************/
-int32_t
-e1000_led_off(struct e1000_hw *hw)
+static int32_t e1000_led_off(struct e1000_hw *hw)
 {
     uint32_t ctrl = E1000_READ_REG(hw, CTRL);
 
@@ -6304,6 +6263,69 @@ e1000_led_off(struct e1000_hw *hw)
     return E1000_SUCCESS;
 }
 
+static void e1000_blink_led_auto(struct e1000_hw *hw, unsigned int seconds)
+{
+    int16_t  i;
+    uint32_t ledctl_blink = 0;
+
+    DEBUGFUNC("e1000_blink_led");
+
+    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 (i=0; i < 4; i++)
+            if (((hw->ledctl_mode2 >> (i * 8)) & 0xFF) ==
+                E1000_LEDCTL_MODE_LED_ON)
+                ledctl_blink |= (E1000_LEDCTL_LED0_BLINK << (i * 8));
+    }
+
+    E1000_WRITE_REG(hw, LEDCTL, ledctl_blink);
+
+    msleep_interruptible(seconds * HZ);
+
+    e1000_led_off(hw);
+    e1000_cleanup_led(hw);
+}
+
+static void e1000_blink_led_manually(struct e1000_hw *hw, unsigned int seconds)
+{
+	unsigned int i;
+	e1000_setup_led(hw);
+
+	for (i = 0; i < (seconds * 4); i++) {
+		if (i & 1)
+			e1000_led_off(hw);
+		else
+			e1000_led_on(hw);
+		if (msleep_interruptible(250))
+			break;
+	}
+
+	e1000_led_off(hw);
+	e1000_cleanup_led(hw);
+}
+
+/******************************************************************************
+ * Blink the LED for the given number of seconds. 
+ *
+ * hw - Struct containing variables accessed by shared code
+ * seconds - Length of time to blink LED for
+ *****************************************************************************/
+void e1000_blink_led(struct e1000_hw *hw, unsigned int seconds)
+{
+	if (hw->mac_type < e1000_82571) {
+		e1000_blink_led_manually(hw, seconds);
+	} else if (hw->phy_type == e1000_phy_ife) {
+		e1000_blink_led_manually(hw, seconds);
+	} else {
+		e1000_blink_led_auto(hw, seconds);
+	}
+}
+
 /******************************************************************************
  * Clears all hardware statistics counters.
  *
diff --git a/drivers/net/e1000/e1000_hw.h b/drivers/net/e1000/e1000_hw.h
index 112447f..1efedb7 100644
--- a/drivers/net/e1000/e1000_hw.h
+++ b/drivers/net/e1000/e1000_hw.h
@@ -404,11 +404,7 @@ void e1000_rar_set(struct e1000_hw *hw, 
 void e1000_write_vfta(struct e1000_hw *hw, uint32_t offset, uint32_t value);
 
 /* LED functions */
-int32_t e1000_setup_led(struct e1000_hw *hw);
-int32_t e1000_cleanup_led(struct e1000_hw *hw);
-int32_t e1000_led_on(struct e1000_hw *hw);
-int32_t e1000_led_off(struct e1000_hw *hw);
-int32_t e1000_blink_led_start(struct e1000_hw *hw);
+void e1000_blink_led(struct e1000_hw *hw, unsigned int seconds);
 
 /* Adaptive IFS Functions */