net: Assorted fixes

net: Assorted fixes

[Anton Vorontsov]

On Sun, Dec 27, 2009 at 09:57:36PM -0800, David Miller wrote:
> Actually I'm reverting both patches:
> 
> drivers/net/phy/mdio_bus.c:268: warning: ‘mdio_bus_phy_may_suspend’ defined but not used
> 
> Please resubmit these two patches with this warning fixed.
> 
> CONFIG_PM is not always enabled :-)

Ugh, sorry about that, and thanks for catching!

Here are the amended patches + one more fix.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

[PATCH 1/4] phylib: Fix deadlock on resume

[Anton Vorontsov]

Sometimes kernel hangs on resume with the following trace:

 ucc_geth e0102000.ucc: resume
 INFO: task bash:1764 blocked for more than 120 seconds.
 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
 bash          D 0fecf43c     0  1764   1763 0x00000000
 Call Trace:
 [cf9a7c10] [c0012868] ret_from_except+0x0/0x14 (unreliable)
 --- Exception: cf9a7ce0 at __switch_to+0x4c/0x6c
     LR = 0xcf9a7cc0
 [cf9a7cd0] [c0008c14] __switch_to+0x4c/0x6c (unreliable)
 [cf9a7ce0] [c028bcfc] schedule+0x158/0x260
 [cf9a7d10] [c028c720] __mutex_lock_slowpath+0x80/0xd8
 [cf9a7d40] [c01cf388] phy_stop+0x20/0x70
 [cf9a7d50] [c01d514c] ugeth_resume+0x6c/0x13c
 [...]

Here is why.

On suspend:

- PM core starts suspending devices, ucc_geth_suspend gets called;

- ucc_geth calls phy_stop() on suspend. Note that phy_stop() is
  mostly asynchronous so it doesn't block ucc_geth's suspend routine,
  it just sets PHY_HALTED state and disables PHY's interrupts;

- Suddenly the state machine gets scheduled, it grabs the phydev->lock
  mutex and tries to process the PHY_HALTED state, so it calls
  phydev->adjust_link(phydev->attached_dev). In ucc_geth case
  adjust_link() calls msleep(), which reschedules the code flow back to
  PM core, which now finishes suspend and so we end up sleeping with
  phydev->lock mutex held.

On resume:

- PM core starts resuming devices (notice that nobody rescheduled
  the state machine yet, so the mutex is still held), the core calls
  ucc_geth's resume routine;

- ucc_geth_resume restarts the PHY with phy_stop()/phy_start()
  sequence, and the phy_*() calls are trying to grab the phydev->lock
  mutex. Here comes the deadlock.

This patch fixes the issue by stopping the state machine on suspend
and starting it again on resume.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/net/phy/mdio_bus.c |   24 ++++++++++++++++++++++--
 1 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index bd4e8d7..49252d3 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -303,8 +303,18 @@ static int mdio_bus_suspend(struct device * dev, pm_message_t state)
 	struct phy_driver *phydrv = to_phy_driver(dev->driver);
 	struct phy_device *phydev = to_phy_device(dev);
 
+	/*
+	 * We must stop the state machine manually, otherwise it stops out of
+	 * control, possibly with the phydev->lock held. Upon resume, netdev
+	 * may call phy routines that try to grab the same lock, and that may
+	 * lead to a deadlock.
+	 */
+	if (phydev->attached_dev)
+		phy_stop_machine(phydev);
+
 	if (!mdio_bus_phy_may_suspend(phydev))
 		return 0;
+
 	return phydrv->suspend(phydev);
 }
 
@@ -312,10 +322,20 @@ static int mdio_bus_resume(struct device * dev)
 {
 	struct phy_driver *phydrv = to_phy_driver(dev->driver);
 	struct phy_device *phydev = to_phy_device(dev);
+	int ret;
 
 	if (!mdio_bus_phy_may_suspend(phydev))
-		return 0;
-	return phydrv->resume(phydev);
+		goto no_resume;
+
+	ret = phydrv->resume(phydev);
+	if (ret < 0)
+		return ret;
+
+no_resume:
+	if (phydev->attached_dev)
+		phy_start_machine(phydev, NULL);
+
+	return 0;
 }
 
 struct bus_type mdio_bus_type = {
-- 
1.6.5.7

[PATCH 2/4] phylib: Properly reinitialize PHYs after hibernation

[Anton Vorontsov]

Since hibernation assumes power loss, we should fully reinitialize
PHYs (including platform fixups), as if PHYs were just attached.

This patch factors phy_init_hw() out of phy_attach_direct(), then
converts mdio_bus to dev_pm_ops and adds an appropriate restore()
callback.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/net/phy/mdio_bus.c   |   50 ++++++++++++++++++++++++++++++++++++------
 drivers/net/phy/phy_device.c |   30 ++++++++++++------------
 include/linux/phy.h          |    1 +
 3 files changed, 59 insertions(+), 22 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 49252d3..e17b702 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -264,6 +264,8 @@ static int mdio_bus_match(struct device *dev, struct device_driver *drv)
 		(phydev->phy_id & phydrv->phy_id_mask));
 }
 
+#ifdef CONFIG_PM
+
 static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 {
 	struct device_driver *drv = phydev->dev.driver;
@@ -295,10 +297,7 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 	return true;
 }
 
-/* Suspend and resume.  Copied from platform_suspend and
- * platform_resume
- */
-static int mdio_bus_suspend(struct device * dev, pm_message_t state)
+static int mdio_bus_suspend(struct device *dev)
 {
 	struct phy_driver *phydrv = to_phy_driver(dev->driver);
 	struct phy_device *phydev = to_phy_device(dev);
@@ -318,7 +317,7 @@ static int mdio_bus_suspend(struct device * dev, pm_message_t state)
 	return phydrv->suspend(phydev);
 }
 
-static int mdio_bus_resume(struct device * dev)
+static int mdio_bus_resume(struct device *dev)
 {
 	struct phy_driver *phydrv = to_phy_driver(dev->driver);
 	struct phy_device *phydev = to_phy_device(dev);
@@ -338,11 +337,48 @@ no_resume:
 	return 0;
 }
 
+static int mdio_bus_restore(struct device *dev)
+{
+	struct phy_device *phydev = to_phy_device(dev);
+	struct net_device *netdev = phydev->attached_dev;
+	int ret;
+
+	if (!netdev)
+		return 0;
+
+	ret = phy_init_hw(phydev);
+	if (ret < 0)
+		return ret;
+
+	/* The PHY needs to renegotiate. */
+	phydev->link = 0;
+	phydev->state = PHY_UP;
+
+	phy_start_machine(phydev, NULL);
+
+	return 0;
+}
+
+static struct dev_pm_ops mdio_bus_pm_ops = {
+	.suspend = mdio_bus_suspend,
+	.resume = mdio_bus_resume,
+	.freeze = mdio_bus_suspend,
+	.thaw = mdio_bus_resume,
+	.restore = mdio_bus_restore,
+};
+
+#define MDIO_BUS_PM_OPS (&mdio_bus_pm_ops)
+
+#else
+
+#define MDIO_BUS_PM_OPS NULL
+
+#endif /* CONFIG_PM */
+
 struct bus_type mdio_bus_type = {
 	.name		= "mdio_bus",
 	.match		= mdio_bus_match,
-	.suspend	= mdio_bus_suspend,
-	.resume		= mdio_bus_resume,
+	.pm		= MDIO_BUS_PM_OPS,
 };
 EXPORT_SYMBOL(mdio_bus_type);
 
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index b10fedd..8212b2b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -378,6 +378,20 @@ void phy_disconnect(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_disconnect);
 
+int phy_init_hw(struct phy_device *phydev)
+{
+	int ret;
+
+	if (!phydev->drv || !phydev->drv->config_init)
+		return 0;
+
+	ret = phy_scan_fixups(phydev);
+	if (ret < 0)
+		return ret;
+
+	return phydev->drv->config_init(phydev);
+}
+
 /**
  * phy_attach_direct - attach a network device to a given PHY device pointer
  * @dev: network device to attach
@@ -425,21 +439,7 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	/* Do initial configuration here, now that
 	 * we have certain key parameters
 	 * (dev_flags and interface) */
-	if (phydev->drv->config_init) {
-		int err;
-
-		err = phy_scan_fixups(phydev);
-
-		if (err < 0)
-			return err;
-
-		err = phydev->drv->config_init(phydev);
-
-		if (err < 0)
-			return err;
-	}
-
-	return 0;
+	return phy_init_hw(phydev);
 }
 EXPORT_SYMBOL(phy_attach_direct);
 
diff --git a/include/linux/phy.h b/include/linux/phy.h
index b1368b8..7968def 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -447,6 +447,7 @@ struct phy_device* get_phy_device(struct mii_bus *bus, int addr);
 int phy_device_register(struct phy_device *phy);
 int phy_clear_interrupt(struct phy_device *phydev);
 int phy_config_interrupt(struct phy_device *phydev, u32 interrupts);
+int phy_init_hw(struct phy_device *phydev);
 int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 		u32 flags, phy_interface_t interface);
 struct phy_device * phy_attach(struct net_device *dev,
-- 
1.6.5.7

[PATCH 3/4] ucc_geth: Fix netdev watchdog triggering on suspend

[Anton Vorontsov]

Sometimes ucc_geth fails to suspend with the following trace:

 ucc_geth e0103000.ucc: suspend
 ucc_geth e0102000.ucc: suspend
 NETDEV WATCHDOG: eth0 (ucc_geth): transmit queue 0 timed out
 ------------[ cut here ]------------
 Badness at net/sched/sch_generic.c:255
 NIP: c021cb5c LR: c021cb5c CTR: c01ab4b4
 [...]
 NIP [c021cb5c] dev_watchdog+0x298/0x2a8
 LR [c021cb5c] dev_watchdog+0x298/0x2a8
 Call Trace:
 [c0389da0] [c021cb5c] dev_watchdog+0x298/0x2a8 (unreliable)
 [c0389e00] [c0031ed8] run_timer_softirq+0x16c/0x1dc
 [c0389e50] [c002c638] __do_softirq+0xa4/0x11c
 [...]

This patch fixes the issue by properly detaching the device on
suspend, and attaching it back on resume.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/net/ucc_geth.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 41ad2f3..96bdc0b 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3607,6 +3607,7 @@ static int ucc_geth_suspend(struct of_device *ofdev, pm_message_t state)
 	if (!netif_running(ndev))
 		return 0;
 
+	netif_device_detach(ndev);
 	napi_disable(&ugeth->napi);
 
 	/*
@@ -3665,7 +3666,7 @@ static int ucc_geth_resume(struct of_device *ofdev)
 	phy_start(ugeth->phydev);
 
 	napi_enable(&ugeth->napi);
-	netif_start_queue(ndev);
+	netif_device_attach(ndev);
 
 	return 0;
 }
-- 
1.6.5.7

[PATCH 4/4] fsl_pq_mdio: Fix iomem unmapping for non-eTSEC2.0 controllers

[Anton Vorontsov]

We use a rather complicated logic to support eTSEC and eTSEC2.0
registers maps in a single driver. Currently, the code tries to
unmap 'regs', but for non-eTSEC2.0 controllers 'regs' doesn't
point to a mapping start, and this might cause badness on probe
failure or module removal:

 Freescale PowerQUICC MII Bus: probed
 Trying to vfree() nonexistent vm area (e107f000)
 ------------[ cut here ]------------
 Badness at c00a7754 [verbose debug info unavailable]
 NIP: c00a7754 LR: c00a7754 CTR: c02231ec
 [...]
 NIP [c00a7754] __vunmap+0xec/0xf4
 LR [c00a7754] __vunmap+0xec/0xf4
 Call Trace:
 [df827e50] [c00a7754] __vunmap+0xec/0xf4 (unreliable)
 [df827e70] [c001519c] iounmap+0x44/0x54
 [df827e80] [c028b924] fsl_pq_mdio_probe+0x1cc/0x2fc
 [df827eb0] [c02fb9b4] of_platform_device_probe+0x5c/0x84
 [df827ed0] [c0229928] really_probe+0x78/0x1a8
 [df827ef0] [c0229b20] __driver_attach+0xa4/0xa8

Fix this by introducing a proper priv structure (finally!), which
now holds 'regs' and 'map' fields separately.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/net/fsl_pq_mdio.c |   30 +++++++++++++++++++++++-------
 1 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/net/fsl_pq_mdio.c b/drivers/net/fsl_pq_mdio.c
index 25fabb3..d5160ed 100644
--- a/drivers/net/fsl_pq_mdio.c
+++ b/drivers/net/fsl_pq_mdio.c
@@ -46,6 +46,11 @@
 #include "gianfar.h"
 #include "fsl_pq_mdio.h"
 
+struct fsl_pq_mdio_priv {
+	void __iomem *map;
+	struct fsl_pq_mdio __iomem *regs;
+};
+
 /*
  * Write value to the PHY at mii_id at register regnum,
  * on the bus attached to the local interface, which may be different from the
@@ -105,7 +110,9 @@ int fsl_pq_local_mdio_read(struct fsl_pq_mdio __iomem *regs,
 
 static struct fsl_pq_mdio __iomem *fsl_pq_mdio_get_regs(struct mii_bus *bus)
 {
-	return (void __iomem __force *)bus->priv;
+	struct fsl_pq_mdio_priv *priv = bus->priv;
+
+	return priv->regs;
 }
 
 /*
@@ -266,6 +273,7 @@ static int fsl_pq_mdio_probe(struct of_device *ofdev,
 {
 	struct device_node *np = ofdev->node;
 	struct device_node *tbi;
+	struct fsl_pq_mdio_priv *priv;
 	struct fsl_pq_mdio __iomem *regs = NULL;
 	void __iomem *map;
 	u32 __iomem *tbipa;
@@ -274,14 +282,19 @@ static int fsl_pq_mdio_probe(struct of_device *ofdev,
 	u64 addr = 0, size = 0;
 	int err = 0;
 
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
 	new_bus = mdiobus_alloc();
 	if (NULL == new_bus)
-		return -ENOMEM;
+		goto err_free_priv;
 
 	new_bus->name = "Freescale PowerQUICC MII Bus",
 	new_bus->read = &fsl_pq_mdio_read,
 	new_bus->write = &fsl_pq_mdio_write,
 	new_bus->reset = &fsl_pq_mdio_reset,
+	new_bus->priv = priv;
 	fsl_pq_mdio_bus_name(new_bus->id, np);
 
 	/* Set the PHY base address */
@@ -291,6 +304,7 @@ static int fsl_pq_mdio_probe(struct of_device *ofdev,
 		err = -ENOMEM;
 		goto err_free_bus;
 	}
+	priv->map = map;
 
 	if (of_device_is_compatible(np, "fsl,gianfar-mdio") ||
 			of_device_is_compatible(np, "fsl,gianfar-tbi") ||
@@ -298,8 +312,7 @@ static int fsl_pq_mdio_probe(struct of_device *ofdev,
 			of_device_is_compatible(np, "ucc_geth_phy"))
 		map -= offsetof(struct fsl_pq_mdio, miimcfg);
 	regs = map;
-
-	new_bus->priv = (void __force *)regs;
+	priv->regs = regs;
 
 	new_bus->irq = kcalloc(PHY_MAX_ADDR, sizeof(int), GFP_KERNEL);
 
@@ -392,10 +405,11 @@ static int fsl_pq_mdio_probe(struct of_device *ofdev,
 err_free_irqs:
 	kfree(new_bus->irq);
 err_unmap_regs:
-	iounmap(regs);
+	iounmap(priv->map);
 err_free_bus:
 	kfree(new_bus);
-
+err_free_priv:
+	kfree(priv);
 	return err;
 }
 
@@ -404,14 +418,16 @@ static int fsl_pq_mdio_remove(struct of_device *ofdev)
 {
 	struct device *device = &ofdev->dev;
 	struct mii_bus *bus = dev_get_drvdata(device);
+	struct fsl_pq_mdio_priv *priv = bus->priv;
 
 	mdiobus_unregister(bus);
 
 	dev_set_drvdata(device, NULL);
 
-	iounmap(fsl_pq_mdio_get_regs(bus));
+	iounmap(priv->map);
 	bus->priv = NULL;
 	mdiobus_free(bus);
+	kfree(priv);
 
 	return 0;
 }
-- 
1.6.5.7

Re: net: Assorted fixes

[David Miller]

RnJvbTogQW50b24gVm9yb250c292IDxhdm9yb250c292QHJ1Lm12aXN0YS5jb20+DQpEYXRlOiBX
ZWQsIDMwIERlYyAyMDA5IDIxOjIxOjQ2ICswMzAwDQoNCj4gT24gU3VuLCBEZWMgMjcsIDIwMDkg
YXQgMDk6NTc6MzZQTSAtMDgwMCwgRGF2aWQgTWlsbGVyIHdyb3RlOg0KPj4gQWN0dWFsbHkgSSdt
IHJldmVydGluZyBib3RoIHBhdGNoZXM6DQo+PiANCj4+IGRyaXZlcnMvbmV0L3BoeS9tZGlvX2J1
cy5jOjI2ODogd2FybmluZzogoW1kaW9fYnVzX3BoeV9tYXlfc3VzcGVuZKIgZGVmaW5lZCBidXQg
bm90IHVzZWQNCj4+IA0KPj4gUGxlYXNlIHJlc3VibWl0IHRoZXNlIHR3byBwYXRjaGVzIHdpdGgg
dGhpcyB3YXJuaW5nIGZpeGVkLg0KPj4gDQo+PiBDT05GSUdfUE0gaXMgbm90IGFsd2F5cyBlbmFi
bGVkIDotKQ0KPiANCj4gVWdoLCBzb3JyeSBhYm91dCB0aGF0LCBhbmQgdGhhbmtzIGZvciBjYXRj
aGluZyENCj4gDQo+IEhlcmUgYXJlIHRoZSBhbWVuZGVkIHBhdGNoZXMgKyBvbmUgbW9yZSBmaXgu
DQoNCkFsbCBhcHBsaWVkLCB0aGFua3MuDQo=