[4/9] sky2: fix shutdown synchronization

Submitted by stephen hemminger on June 17, 2009, 5:30 p.m.

Details

Message ID 20090617173139.828049268@vyatta.com
State Accepted
Delegated to: David Miller
Headers show

Commit Message

stephen hemminger June 17, 2009, 5:30 p.m.
The logic in sky2_down was incorrect. Receiver could report status
after rx_stop was called.

The steps need to be:
   * stop new frames from being transmitted
   * shut off transmit/receive logic
   * synchronize with NAPI to process status info about transmitter
     and receiver

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Comments

David Miller June 18, 2009, 1:50 a.m.
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 17 Jun 2009 10:30:35 -0700

> The logic in sky2_down was incorrect. Receiver could report status
> after rx_stop was called.
> 
> The steps need to be:
>    * stop new frames from being transmitted
>    * shut off transmit/receive logic
>    * synchronize with NAPI to process status info about transmitter
>      and receiver
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike McCormack June 18, 2009, 11:25 p.m.
Hi Steven,

After applying your complete patch series, I'm still getting crashes in 
sky2_poll, and I can make them go away by adding an msleep(1) before 
these lines in sky2_down.

+	synchronize_irq(hw->pdev->irq);
+	napi_synchronize(&hw->napi);
+
  	sky2_phy_power_down(hw, port)

thanks,

Mike
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
stephen hemminger June 18, 2009, 11:41 p.m.
On Fri, 19 Jun 2009 08:25:03 +0900
Mike McCormack <mikem@ring3k.org> wrote:

> 
> Hi Steven,
> 
> After applying your complete patch series, I'm still getting crashes in 
> sky2_poll, and I can make them go away by adding an msleep(1) before 
> these lines in sky2_down.
> 

adding msleep adds delay so interrupt and packets can clear, that is okay,
but would rather have something deterministic?  perhaps it needs to poll
irq status register or napi status.

Patch hide | download patch | download mbox

--- a/drivers/net/sky2.c	2009-06-17 10:29:53.382997969 -0700
+++ b/drivers/net/sky2.c	2009-06-17 10:29:55.361810536 -0700
@@ -1815,8 +1815,6 @@  static int sky2_down(struct net_device *
 	sky2_write32(hw, B0_IMSK, imask);
 	sky2_read32(hw, B0_IMSK);
 
-	synchronize_irq(hw->pdev->irq);
-
 	/* Force flow control off */
 	sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_PAUSE_OFF);
 
@@ -1831,9 +1829,6 @@  static int sky2_down(struct net_device *
 	ctrl &= ~(GM_GPCR_TX_ENA | GM_GPCR_RX_ENA);
 	gma_write16(hw, port, GM_GP_CTRL, ctrl);
 
-	/* Make sure no packets are pending */
-	napi_synchronize(&hw->napi);
-
 	sky2_write8(hw, SK_REG(port, GPHY_CTRL), GPC_RST_SET);
 
 	/* Workaround shared GMAC reset */
@@ -1864,6 +1859,15 @@  static int sky2_down(struct net_device *
 	sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_RST_SET);
 	sky2_write8(hw, SK_REG(port, TX_GMF_CTRL_T), GMF_RST_SET);
 
+	/* Force any delayed status interrrupt and NAPI */
+	sky2_write32(hw, STAT_LEV_TIMER_CNT, 0);
+	sky2_write32(hw, STAT_TX_TIMER_CNT, 0);
+	sky2_write32(hw, STAT_ISR_TIMER_CNT, 0);
+	sky2_read8(hw, STAT_ISR_TIMER_CTRL);
+
+	synchronize_irq(hw->pdev->irq);
+	napi_synchronize(&hw->napi);
+
 	sky2_phy_power_down(hw, port);
 
 	/* turn off LED's */