netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Matt Carlson" <mcarlson@broadcom.com>
To: "Felix Radensky" <felix@embedded-sol.com>
Cc: netdev@vger.kernel.org, "Michael Chan" <mchan@broadcom.com>,
	andy@greyhouse.net
Subject: [PATCH net-2.6 V2] tg3: Fix phylib locking strategy
Date: Mon, 5 Oct 2009 20:55:29 -0700	[thread overview]
Message-ID: <1254801692.18507@xw6200> (raw)

O.K.  Here is the latest version.  Felix, can you verify your problem
is solved with this patch?

---

Felix Radensky noted that chip resets were generating stack trace dumps.
This is because the driver is attempting to acquire the mdio bus mutex
while holding the tp->lock spinlock.  The fix is to change the code such
that every phy access takes the tp->lock spinlock instead.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
---
 drivers/net/tg3.c |   41 ++++++++++++++---------------------------
 drivers/net/tg3.h |    1 -
 2 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index f09bc5d..ba5d3fe 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -902,11 +902,12 @@ static int tg3_mdio_read(struct mii_bus *bp, int mii_id, int reg)
 	struct tg3 *tp = bp->priv;
 	u32 val;
 
-	if (tp->tg3_flags3 & TG3_FLG3_MDIOBUS_PAUSED)
-		return -EAGAIN;
+	spin_lock_bh(&tp->lock);
 
 	if (tg3_readphy(tp, reg, &val))
-		return -EIO;
+		val = -EIO;
+
+	spin_unlock_bh(&tp->lock);
 
 	return val;
 }
@@ -914,14 +915,16 @@ static int tg3_mdio_read(struct mii_bus *bp, int mii_id, int reg)
 static int tg3_mdio_write(struct mii_bus *bp, int mii_id, int reg, u16 val)
 {
 	struct tg3 *tp = bp->priv;
+	u32 ret = 0;
 
-	if (tp->tg3_flags3 & TG3_FLG3_MDIOBUS_PAUSED)
-		return -EAGAIN;
+	spin_lock_bh(&tp->lock);
 
 	if (tg3_writephy(tp, reg, val))
-		return -EIO;
+		ret = -EIO;
 
-	return 0;
+	spin_unlock_bh(&tp->lock);
+
+	return ret;
 }
 
 static int tg3_mdio_reset(struct mii_bus *bp)
@@ -1011,12 +1014,6 @@ static void tg3_mdio_config_5785(struct tg3 *tp)
 
 static void tg3_mdio_start(struct tg3 *tp)
 {
-	if (tp->tg3_flags3 & TG3_FLG3_MDIOBUS_INITED) {
-		mutex_lock(&tp->mdio_bus->mdio_lock);
-		tp->tg3_flags3 &= ~TG3_FLG3_MDIOBUS_PAUSED;
-		mutex_unlock(&tp->mdio_bus->mdio_lock);
-	}
-
 	tp->mi_mode &= ~MAC_MI_MODE_AUTO_POLL;
 	tw32_f(MAC_MI_MODE, tp->mi_mode);
 	udelay(80);
@@ -1041,15 +1038,6 @@ static void tg3_mdio_start(struct tg3 *tp)
 		tg3_mdio_config_5785(tp);
 }
 
-static void tg3_mdio_stop(struct tg3 *tp)
-{
-	if (tp->tg3_flags3 & TG3_FLG3_MDIOBUS_INITED) {
-		mutex_lock(&tp->mdio_bus->mdio_lock);
-		tp->tg3_flags3 |= TG3_FLG3_MDIOBUS_PAUSED;
-		mutex_unlock(&tp->mdio_bus->mdio_lock);
-	}
-}
-
 static int tg3_mdio_init(struct tg3 *tp)
 {
 	int i;
@@ -1141,7 +1129,6 @@ static void tg3_mdio_fini(struct tg3 *tp)
 		tp->tg3_flags3 &= ~TG3_FLG3_MDIOBUS_INITED;
 		mdiobus_unregister(tp->mdio_bus);
 		mdiobus_free(tp->mdio_bus);
-		tp->tg3_flags3 &= ~TG3_FLG3_MDIOBUS_PAUSED;
 	}
 }
 
@@ -1363,7 +1350,7 @@ static void tg3_adjust_link(struct net_device *dev)
 	struct tg3 *tp = netdev_priv(dev);
 	struct phy_device *phydev = tp->mdio_bus->phy_map[PHY_ADDR];
 
-	spin_lock(&tp->lock);
+	spin_lock_bh(&tp->lock);
 
 	mac_mode = tp->mac_mode & ~(MAC_MODE_PORT_MODE_MASK |
 				    MAC_MODE_HALF_DUPLEX);
@@ -1431,7 +1418,7 @@ static void tg3_adjust_link(struct net_device *dev)
 	tp->link_config.active_speed = phydev->speed;
 	tp->link_config.active_duplex = phydev->duplex;
 
-	spin_unlock(&tp->lock);
+	spin_unlock_bh(&tp->lock);
 
 	if (linkmesg)
 		tg3_link_report(tp);
@@ -6392,8 +6379,6 @@ static int tg3_chip_reset(struct tg3 *tp)
 
 	tg3_nvram_lock(tp);
 
-	tg3_mdio_stop(tp);
-
 	tg3_ape_lock(tp, TG3_APE_LOCK_GRC);
 
 	/* No matching tg3_nvram_unlock() after this because
@@ -8698,6 +8683,8 @@ static int tg3_close(struct net_device *dev)
 
 	del_timer_sync(&tp->timer);
 
+	tg3_phy_stop(tp);
+
 	tg3_full_lock(tp, 1);
 #if 0
 	tg3_dump_state(tp);
diff --git a/drivers/net/tg3.h b/drivers/net/tg3.h
index 524691c..bab7940 100644
--- a/drivers/net/tg3.h
+++ b/drivers/net/tg3.h
@@ -2748,7 +2748,6 @@ struct tg3 {
 #define TG3_FLG3_5701_DMA_BUG		0x00000008
 #define TG3_FLG3_USE_PHYLIB		0x00000010
 #define TG3_FLG3_MDIOBUS_INITED		0x00000020
-#define TG3_FLG3_MDIOBUS_PAUSED		0x00000040
 #define TG3_FLG3_PHY_CONNECTED		0x00000080
 #define TG3_FLG3_RGMII_STD_IBND_DISABLE	0x00000100
 #define TG3_FLG3_RGMII_EXT_IBND_RX_EN	0x00000200
-- 
1.6.4.4



             reply	other threads:[~2009-10-06  4:02 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-06  3:55 Matt Carlson [this message]
2009-10-06  5:45 ` [PATCH net-2.6 V2] tg3: Fix phylib locking strategy Felix Radensky
2009-10-07 10:41 ` David Miller
2009-11-16 20:53 ` Felix Radensky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1254801692.18507@xw6200 \
    --to=mcarlson@broadcom.com \
    --cc=andy@greyhouse.net \
    --cc=felix@embedded-sol.com \
    --cc=mchan@broadcom.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).