Hi,

> My patch may seem like a huge change, but in essence the change is
> pretty simple.
> 
> In my patch, the interrupt handler code will check whether the interrupted
> code is holding the swfw semaphore. If it is held, the watchdog function
> is deferred until swfw semaphore is released.
> The modification is for the interrupted code which is holding the
> semaphore, and the interrupt handler, so they are both directly related
> to this problem.
> 
> I will try to add some comments to my code to make it more readable.

I have rebased this patch for 2.6.20-rc5 and added some comments to this
patch to make it more readable.

Does this patch have problems? Or do you have any other ideas?

--
  Kenzo Iwami ([EMAIL PROTECTED])

Signed-off-by: Kenzo Iwami <[EMAIL PROTECTED]>

diff -urpN linux-2.6.20-rc5_org/drivers/net/e1000/e1000_hw.c 
linux-2.6.20-rc5_fix/drivers/net/e1000/e1000_hw.c
--- linux-2.6.20-rc5_org/drivers/net/e1000/e1000_hw.c   2007-01-13 
03:54:26.000000000 +0900
+++ linux-2.6.20-rc5_fix/drivers/net/e1000/e1000_hw.c   2007-01-18 
18:04:04.000000000 +0900
@@ -35,6 +35,7 @@

 static int32_t e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask);
 static void e1000_swfw_sync_release(struct e1000_hw *hw, uint16_t mask);
+static void e1000_check_watchdog_deferred(struct e1000_hw *hw);
 static int32_t e1000_read_kmrn_reg(struct e1000_hw *hw, uint32_t reg_addr, 
uint16_t *data);
 static int32_t e1000_write_kmrn_reg(struct e1000_hw *hw, uint32_t reg_addr, 
uint16_t data);
 static int32_t e1000_get_software_semaphore(struct e1000_hw *hw);
@@ -3396,6 +3397,29 @@ e1000_shift_in_mdi_bits(struct e1000_hw
     return data;
 }

+static void
+e1000_check_watchdog_deferred(struct e1000_hw *hw)
+{
+    /* If watchdog interrupt was deferred while this process was holding
+     * the swfw semaphore, process the watchdog now.
+     */
+    if (atomic_xchg(&hw->watchdog_deferred, 0)) {
+retry:
+        e1000_do_watchdog(hw);
+    }
+    atomic_dec(&hw->swfw_sem_count);
+
+    /* Check watchdog_deferred once more just in case watchdog interrupt
+     * occurred between atomic_xchg and atomic_dec.
+     * By nature of the watchdog interrupt, we shouldn't go through this
+     * more than once.
+     */
+    if (atomic_xchg(&hw->watchdog_deferred, 0)) {
+        atomic_inc(&hw->swfw_sem_count);
+        goto retry;
+    }
+}
+
 static int32_t
 e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask)
 {
@@ -3413,8 +3437,11 @@ e1000_swfw_sync_acquire(struct e1000_hw
         return e1000_get_hw_eeprom_semaphore(hw);

     while (timeout) {
-            if (e1000_get_hw_eeprom_semaphore(hw))
+            atomic_inc(&hw->swfw_sem_count);
+            if (e1000_get_hw_eeprom_semaphore(hw)) {
+                e1000_check_watchdog_deferred(hw);
                 return -E1000_ERR_SWFW_SYNC;
+            }

             swfw_sync = E1000_READ_REG(hw, SW_FW_SYNC);
             if (!(swfw_sync & (fwmask | swmask))) {
@@ -3424,6 +3451,7 @@ e1000_swfw_sync_acquire(struct e1000_hw
             /* firmware currently using resource (fwmask) */
             /* or other software thread currently using resource (swmask) */
             e1000_put_hw_eeprom_semaphore(hw);
+            e1000_check_watchdog_deferred(hw);
             mdelay(5);
             timeout--;
     }
@@ -3437,6 +3465,8 @@ e1000_swfw_sync_acquire(struct e1000_hw
     E1000_WRITE_REG(hw, SW_FW_SYNC, swfw_sync);

     e1000_put_hw_eeprom_semaphore(hw);
+    e1000_check_watchdog_deferred(hw);
+
     return E1000_SUCCESS;
 }

@@ -3458,6 +3488,7 @@ e1000_swfw_sync_release(struct e1000_hw
         return;
     }

+    atomic_inc(&hw->swfw_sem_count);
     /* if (e1000_get_hw_eeprom_semaphore(hw))
      *    return -E1000_ERR_SWFW_SYNC; */
     while (e1000_get_hw_eeprom_semaphore(hw) != E1000_SUCCESS);
@@ -3468,6 +3499,7 @@ e1000_swfw_sync_release(struct e1000_hw
     E1000_WRITE_REG(hw, SW_FW_SYNC, swfw_sync);

     e1000_put_hw_eeprom_semaphore(hw);
+    e1000_check_watchdog_deferred(hw);
 }

 /*****************************************************************************
diff -urpN linux-2.6.20-rc5_org/drivers/net/e1000/e1000_hw.h 
linux-2.6.20-rc5_fix/drivers/net/e1000/e1000_hw.h
--- linux-2.6.20-rc5_org/drivers/net/e1000/e1000_hw.h   2007-01-13 
03:54:26.000000000 +0900
+++ linux-2.6.20-rc5_fix/drivers/net/e1000/e1000_hw.h   2007-01-18 
18:04:42.000000000 +0900
@@ -306,6 +306,7 @@ typedef enum {
 #define E1000_BYTE_SWAP_WORD(_value) ((((_value) & 0x00ff) << 8) | \
                                      (((_value) & 0xff00) >> 8))

+extern void e1000_do_watchdog(struct e1000_hw *hw);
 /* Function prototypes */
 /* Initialization */
 int32_t e1000_reset_hw(struct e1000_hw *hw);
@@ -1465,6 +1466,8 @@ struct e1000_hw {
        boolean_t               has_manc2h;
        boolean_t               rx_needs_kicking;
        boolean_t               has_smbus;
+       atomic_t                swfw_sem_count;    /* >0 if swfw_sem held on 
ESB2 */
+       atomic_t                watchdog_deferred; /* watchdog deferred on ESB2 
*/
 };


diff -urpN linux-2.6.20-rc5_org/drivers/net/e1000/e1000_main.c 
linux-2.6.20-rc5_fix/drivers/net/e1000/e1000_main.c
--- linux-2.6.20-rc5_org/drivers/net/e1000/e1000_main.c 2007-01-13 
03:54:26.000000000 +0900
+++ linux-2.6.20-rc5_fix/drivers/net/e1000/e1000_main.c 2007-01-18 
18:05:54.000000000 +0900
@@ -152,6 +152,7 @@ static void e1000_clean_rx_ring(struct e
 static void e1000_set_multi(struct net_device *netdev);
 static void e1000_update_phy_info(unsigned long data);
 static void e1000_watchdog(unsigned long data);
+void e1000_do_watchdog(struct e1000_hw *hw);
 static void e1000_82547_tx_fifo_stall(unsigned long data);
 static int e1000_xmit_frame(struct sk_buff *skb, struct net_device *netdev);
 static struct net_device_stats * e1000_get_stats(struct net_device *netdev);
@@ -1313,6 +1314,9 @@ e1000_sw_init(struct e1000_adapter *adap
        hw->tbi_compatibility_en = TRUE;
        hw->adaptive_ifs = TRUE;

+       atomic_set(&hw->swfw_sem_count, 0);
+       atomic_set(&hw->watchdog_deferred, 0);
+
        /* Copper options */

        if (hw->media_type == e1000_media_type_copper) {
@@ -2555,6 +2559,29 @@ static void
 e1000_watchdog(unsigned long data)
 {
        struct e1000_adapter *adapter = (struct e1000_adapter *) data;
+       struct e1000_hw *hw = &adapter->hw;
+
+       if (hw->swfw_sync_present) {
+               /* Check whether the interrupted code is holding the swfw
+                * semaphore. If it is held, defer processing the watchdog
+                * until swfw semaphore is released.
+                */
+               if (atomic_read(&hw->swfw_sem_count))
+                       atomic_set(&hw->watchdog_deferred, 1);
+               else
+                       e1000_do_watchdog(hw);
+       } else {
+               e1000_do_watchdog(hw);
+       }
+
+       /* Reset the timer */
+       mod_timer(&adapter->watchdog_timer, jiffies + 2 * HZ);
+}
+
+void
+e1000_do_watchdog(struct e1000_hw *hw)
+{
+       struct e1000_adapter *adapter = hw->back;
        struct net_device *netdev = adapter->netdev;
        struct e1000_tx_ring *txdr = adapter->tx_ring;
        uint32_t link, tctl;
@@ -2722,9 +2749,6 @@ e1000_watchdog(unsigned long data)
         * reset from the other port. Set the appropriate LAA in RAR[0] */
        if (adapter->hw.mac_type == e1000_82571 && adapter->hw.laa_is_present)
                e1000_rar_set(&adapter->hw, adapter->hw.mac_addr, 0);
-
-       /* Reset the timer */
-       mod_timer(&adapter->watchdog_timer, jiffies + 2 * HZ);
 }

 enum latency_range {

-
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

Reply via email to