Patchwork [4/9] sky2: fix shutdown synchronization

login
register
mail settings
Submitter stephen hemminger
Date June 17, 2009, 5:30 p.m.
Message ID <20090617173139.828049268@vyatta.com>
Download mbox | patch
Permalink /patch/28801/
State Accepted
Delegated to: David Miller
Headers show

Comments

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>
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

--- 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 */