netdev - Re: [PATCH][RESEND] PHY: Add the phy_device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20071204073847.GA881@avionic-tr-basi.adnet.avionic-design.de>
Date:	Tue, 4 Dec 2007 08:38:47 +0100
From:	Thierry Reding <thierry.reding@...onic-design.de>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Jeff Garzik <jeff@...zik.org>, netdev@...r.kernel.org,
	David Miller <davem@...emloft.net>,
	Anton Vorontsov <avorontsov@...mvista.com>,
	Andy Fleming <afleming@...escale.com>
Subject: Re: [PATCH][RESEND] PHY: Add the phy_device_release device method.

* Andrew Morton wrote:
> On Mon, 3 Dec 2007 09:35:11 +0100 Thierry Reding <thierry.reding@...onic-design.de> wrote:
> 
> > In cases where more than a single PHY is found on the MDIO bus, the kernel
> > will print a warning that this method is missing for each PHY device that
> > is not attached to a networking device.
> > 
> > Signed-off-by: Thierry Reding <thierry.reding@...onic-design.de>
> > ---
> >  drivers/net/phy/mdio_bus.c |   19 ++++++++++++++++++-
> >  1 files changed, 18 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> > index fc2f0e6..cb7fb47 100644
> > --- a/drivers/net/phy/mdio_bus.c
> > +++ b/drivers/net/phy/mdio_bus.c
> > @@ -36,6 +36,23 @@
> >  #include <asm/uaccess.h>
> >  
> >  /**
> > + * phy_device_release - free a phy_device structure when all users of it are
> > + * 	finished.
> > + *
> > + * @dev: device that's been disconnected
> > + *
> > + * Will be called only by the device core when all users of this phy_device
> > + * are done.
> > + */
> > +static void phy_device_release(struct device *dev)
> > +{
> > +	struct phy_device *phy;
> > +
> > +	phy = to_phy_device(dev);
> > +	kfree(phy);
> > +}
> > +
> > +/**
> >   * mdiobus_register - bring up all the PHYs on a given bus and attach them to bus
> >   * @bus: target mii_bus
> >   *
> > @@ -83,6 +100,7 @@ int mdiobus_register(struct mii_bus *bus)
> >  		if (phydev) {
> >  			phydev->irq = bus->irq[i];
> >  
> > +			phydev->dev.release = phy_device_release;
> >  			phydev->dev.parent = bus->dev;
> >  			phydev->dev.bus = &mdio_bus_type;
> >  			snprintf(phydev->dev.bus_id, BUS_ID_SIZE, PHY_ID_FMT, bus->id, i);
> > @@ -112,7 +130,6 @@ void mdiobus_unregister(struct mii_bus *bus)
> >  	for (i = 0; i < PHY_MAX_ADDR; i++) {
> >  		if (bus->phy_map[i]) {
> >  			device_unregister(&bus->phy_map[i]->dev);
> > -			kfree(bus->phy_map[i]);
> >  		}
> >  	}
> >  }
> > 
> 
> I've been sitting on
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.24-rc3/2.6.24-rc3-mm2/broken-out/phy-implement-release-function.patch
> for a few weeks.  For some reason I have it in my "nacked netdev patches"
> section but I think that was a mistake and it has not (yet ;)) been nacked.
> 
> Anyway, Anton's patch looks somewhat different from yours.  Please compare
> notes.

FWIW, I like Anton's patch better, especially since it plugs a possible
memory leak. I'm not sure it's useful or necessary to export the
phy_device_free symbol, though.

The only other difference that I can see is that Anton's patch sets the
release function in a different place, when the device is created, whereas my
patch only sets it before registering the device. Both should be identical
since the release function won't be needed unless the device is registered.

Thierry


Download attachment "signature.asc" of type "application/pgp-signature" (190 bytes)