| Message ID | 20081107092450.GG32552@xi.wantstofly.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sat, Nov 8, 2008 at 12:41 AM, Lennert Buytenhek <buytenh@wantstofly.org> wrote: > On Sat, Nov 08, 2008 at 12:36:08AM +0800, Bryan Wu wrote: > >> >> I got a regression since I update our blackfin kernel to the lastest >> >> 2.6.28-rc2 which includes your phylib patches. >> >> Please find the tracker here: >> >> https://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_id=141&tracker_item_id=4595 >> >> >> >> Briefly, the calling chain is: >> >> in bfin_mac_remove(), mdiobus_unregister() -> >> >> device_unregister(&bus->dev); -> put_device() -> kobject_put() -> >> >> kref_put(&kobj->kref, kobject_release); -> kobject_release() -> >> >> kobject_cleanup() -> mdiobus_release(). >> >> So in mdiobus_unregister() bus->state = MDIOBUS_UNREGISTERED; but in >> >> mdiobus_release(), this BUG_ON will fire: >> >> BUG_ON(bus->state != MDIOBUS_RELEASED); >> >> >> >> How about this patch? >> >> >> >> --- >> >> Index: drivers/net/phy/mdio_bus.c >> >> =================================================================== >> >> --- drivers/net/phy/mdio_bus.c (revision 5575) >> >> +++ drivers/net/phy/mdio_bus.c (working copy) >> >> @@ -134,13 +134,16 @@ >> >> int i; >> >> >> >> BUG_ON(bus->state != MDIOBUS_REGISTERED); >> >> - bus->state = MDIOBUS_UNREGISTERED; >> >> >> >> + bus->state = MDIOBUS_RELEASED; >> >> + >> >> device_unregister(&bus->dev); >> >> for (i = 0; i < PHY_MAX_ADDR; i++) { >> >> if (bus->phy_map[i]) >> >> device_unregister(&bus->phy_map[i]->dev); >> >> } >> >> + >> >> + bus->state = MDIOBUS_UNREGISTERED; >> >> } >> >> EXPORT_SYMBOL(mdiobus_unregister); >> >> --- >> > >> > Actually, what I think should be done is this. Can you try that? >> > >> > >> > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c >> > index d0ed1ef..536bda1 100644 >> > --- a/drivers/net/phy/mdio_bus.c >> > +++ b/drivers/net/phy/mdio_bus.c >> > @@ -136,7 +136,7 @@ void mdiobus_unregister(struct mii_bus *bus) >> > BUG_ON(bus->state != MDIOBUS_REGISTERED); >> > bus->state = MDIOBUS_UNREGISTERED; >> > >> > - device_unregister(&bus->dev); >> > + device_del(&bus->dev); >> > for (i = 0; i < PHY_MAX_ADDR; i++) { >> > if (bus->phy_map[i]) >> > device_unregister(&bus->phy_map[i]->dev); >> > -- >> >> Right, that's the right way. Our tester confirmed that this bug was >> fixed with this patch. >> Need I send a patch or you patched this youself? > > Can you please confirm on netdev@ that that patch fixes it and then > I'll submit it with your Tested-by, is that okay with you? > Sorry for just clicking reply not reply-all. Tested-by is OK for me definitely, -:)) -Bryan -- 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
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index d0ed1ef..536bda1 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -136,7 +136,7 @@ void mdiobus_unregister(struct mii_bus *bus) BUG_ON(bus->state != MDIOBUS_REGISTERED); bus->state = MDIOBUS_UNREGISTERED; - device_unregister(&bus->dev); + device_del(&bus->dev); for (i = 0; i < PHY_MAX_ADDR; i++) { if (bus->phy_map[i]) device_unregister(&bus->phy_map[i]->dev);