* net: Assorted fixes
@ 2009-12-30 18:21 Anton Vorontsov
2009-12-30 18:23 ` [PATCH 1/4] phylib: Fix deadlock on resume Anton Vorontsov
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Anton Vorontsov @ 2009-12-30 18:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, afleming, linuxppc-dev
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] phylib: Fix deadlock on resume
2009-12-30 18:21 net: Assorted fixes Anton Vorontsov
@ 2009-12-30 18:23 ` Anton Vorontsov
2009-12-30 18:23 ` [PATCH 2/4] phylib: Properly reinitialize PHYs after hibernation Anton Vorontsov
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Anton Vorontsov @ 2009-12-30 18:23 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Andy Fleming, linuxppc-dev
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] phylib: Properly reinitialize PHYs after hibernation
2009-12-30 18:21 net: Assorted fixes Anton Vorontsov
2009-12-30 18:23 ` [PATCH 1/4] phylib: Fix deadlock on resume Anton Vorontsov
@ 2009-12-30 18:23 ` Anton Vorontsov
2009-12-30 18:23 ` [PATCH 3/4] ucc_geth: Fix netdev watchdog triggering on suspend Anton Vorontsov
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Anton Vorontsov @ 2009-12-30 18:23 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Andy Fleming, linuxppc-dev
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] ucc_geth: Fix netdev watchdog triggering on suspend
2009-12-30 18:21 net: Assorted fixes Anton Vorontsov
2009-12-30 18:23 ` [PATCH 1/4] phylib: Fix deadlock on resume Anton Vorontsov
2009-12-30 18:23 ` [PATCH 2/4] phylib: Properly reinitialize PHYs after hibernation Anton Vorontsov
@ 2009-12-30 18:23 ` Anton Vorontsov
2009-12-30 18:23 ` [PATCH 4/4] fsl_pq_mdio: Fix iomem unmapping for non-eTSEC2.0 controllers Anton Vorontsov
2009-12-31 6:04 ` net: Assorted fixes David Miller
4 siblings, 0 replies; 6+ messages in thread
From: Anton Vorontsov @ 2009-12-30 18:23 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Andy Fleming, linuxppc-dev
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] fsl_pq_mdio: Fix iomem unmapping for non-eTSEC2.0 controllers
2009-12-30 18:21 net: Assorted fixes Anton Vorontsov
` (2 preceding siblings ...)
2009-12-30 18:23 ` [PATCH 3/4] ucc_geth: Fix netdev watchdog triggering on suspend Anton Vorontsov
@ 2009-12-30 18:23 ` Anton Vorontsov
2009-12-31 6:04 ` net: Assorted fixes David Miller
4 siblings, 0 replies; 6+ messages in thread
From: Anton Vorontsov @ 2009-12-30 18:23 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Andy Fleming, linuxppc-dev
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: net: Assorted fixes
2009-12-30 18:21 net: Assorted fixes Anton Vorontsov
` (3 preceding siblings ...)
2009-12-30 18:23 ` [PATCH 4/4] fsl_pq_mdio: Fix iomem unmapping for non-eTSEC2.0 controllers Anton Vorontsov
@ 2009-12-31 6:04 ` David Miller
4 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2009-12-31 6:04 UTC (permalink / raw)
To: avorontsov; +Cc: netdev, afleming, linuxppc-dev
RnJvbTogQW50b24gVm9yb250c292IDxhdm9yb250c292QHJ1Lm12aXN0YS5jb20+DQpEYXRlOiBX
ZWQsIDMwIERlYyAyMDA5IDIxOjIxOjQ2ICswMzAwDQoNCj4gT24gU3VuLCBEZWMgMjcsIDIwMDkg
YXQgMDk6NTc6MzZQTSAtMDgwMCwgRGF2aWQgTWlsbGVyIHdyb3RlOg0KPj4gQWN0dWFsbHkgSSdt
IHJldmVydGluZyBib3RoIHBhdGNoZXM6DQo+PiANCj4+IGRyaXZlcnMvbmV0L3BoeS9tZGlvX2J1
cy5jOjI2ODogd2FybmluZzogoW1kaW9fYnVzX3BoeV9tYXlfc3VzcGVuZKIgZGVmaW5lZCBidXQg
bm90IHVzZWQNCj4+IA0KPj4gUGxlYXNlIHJlc3VibWl0IHRoZXNlIHR3byBwYXRjaGVzIHdpdGgg
dGhpcyB3YXJuaW5nIGZpeGVkLg0KPj4gDQo+PiBDT05GSUdfUE0gaXMgbm90IGFsd2F5cyBlbmFi
bGVkIDotKQ0KPiANCj4gVWdoLCBzb3JyeSBhYm91dCB0aGF0LCBhbmQgdGhhbmtzIGZvciBjYXRj
aGluZyENCj4gDQo+IEhlcmUgYXJlIHRoZSBhbWVuZGVkIHBhdGNoZXMgKyBvbmUgbW9yZSBmaXgu
DQoNCkFsbCBhcHBsaWVkLCB0aGFua3MuDQo=
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-12-31 6:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-12-30 18:21 net: Assorted fixes Anton Vorontsov
2009-12-30 18:23 ` [PATCH 1/4] phylib: Fix deadlock on resume Anton Vorontsov
2009-12-30 18:23 ` [PATCH 2/4] phylib: Properly reinitialize PHYs after hibernation Anton Vorontsov
2009-12-30 18:23 ` [PATCH 3/4] ucc_geth: Fix netdev watchdog triggering on suspend Anton Vorontsov
2009-12-30 18:23 ` [PATCH 4/4] fsl_pq_mdio: Fix iomem unmapping for non-eTSEC2.0 controllers Anton Vorontsov
2009-12-31 6:04 ` net: Assorted fixes David Miller
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).