* Re: Fw: [PATCH] Add the phy_device_release device method.
[not found] <20071119.214215.207388094.davem@davemloft.net>
@ 2007-12-01 22:02 ` Jeff Garzik
2007-12-03 8:35 ` [PATCH][RESEND] PHY: " Thierry Reding
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Garzik @ 2007-12-01 22:02 UTC (permalink / raw)
To: Thierry Reding; +Cc: netdev, David Miller, Andrew Morton
Thierry, could you resend this patch to me?
I do not seem to have an apply-able version of this patch anywhere.
The copy DaveM forwarded to me had problems (though the technical
content looks OK)
Thanks,
Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH][RESEND] PHY: Add the phy_device_release device method.
2007-12-01 22:02 ` Fw: [PATCH] Add the phy_device_release device method Jeff Garzik
@ 2007-12-03 8:35 ` Thierry Reding
2007-12-04 5:44 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2007-12-03 8:35 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, David Miller, Andrew Morton
[-- Attachment #1: Type: text/plain, Size: 1639 bytes --]
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@avionic-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]);
}
}
}
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH][RESEND] PHY: Add the phy_device_release device method.
2007-12-03 8:35 ` [PATCH][RESEND] PHY: " Thierry Reding
@ 2007-12-04 5:44 ` Andrew Morton
2007-12-04 7:38 ` Thierry Reding
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2007-12-04 5:44 UTC (permalink / raw)
To: Thierry Reding
Cc: Jeff Garzik, netdev, David Miller, Anton Vorontsov, Andy Fleming
On Mon, 3 Dec 2007 09:35:11 +0100 Thierry Reding <thierry.reding@avionic-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@avionic-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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][RESEND] PHY: Add the phy_device_release device method.
2007-12-04 5:44 ` Andrew Morton
@ 2007-12-04 7:38 ` Thierry Reding
2007-12-04 13:17 ` Anton Vorontsov
0 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2007-12-04 7:38 UTC (permalink / raw)
To: Andrew Morton
Cc: Jeff Garzik, netdev, David Miller, Anton Vorontsov, Andy Fleming
[-- Attachment #1: Type: text/plain, Size: 2888 bytes --]
* Andrew Morton wrote:
> On Mon, 3 Dec 2007 09:35:11 +0100 Thierry Reding <thierry.reding@avionic-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@avionic-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
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][RESEND] PHY: Add the phy_device_release device method.
2007-12-04 7:38 ` Thierry Reding
@ 2007-12-04 13:17 ` Anton Vorontsov
2007-12-04 20:06 ` Jeff Garzik
0 siblings, 1 reply; 6+ messages in thread
From: Anton Vorontsov @ 2007-12-04 13:17 UTC (permalink / raw)
To: Thierry Reding
Cc: Andrew Morton, Jeff Garzik, netdev, David Miller, Andy Fleming
On Tue, Dec 04, 2007 at 08:38:47AM +0100, Thierry Reding wrote:
> * Andrew Morton wrote:
> > On Mon, 3 Dec 2007 09:35:11 +0100 Thierry Reding <thierry.reding@avionic-design.de> wrote:
> >
[...]
> > 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.
Heh, it has been otherwise Acked-by: Andy Fleming, informal(?) phylib
maintainer.
> > 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.
Makes sense, I think. Here is the newer patch, the only difference is
removed EXPORT_SYMBOL(). Because of trivial change, I dared to keep
Andy's Acked-by intact.
- - - -
From: Anton Vorontsov <avorontsov@ru.mvista.com>
Subject: [PATCH] phy: implement release function
Lately I've got this nice badness on mdio bus removal:
Device 'e0103120:06' does not have a release() function, it is broken and must be fixed.
------------[ cut here ]------------
Badness at drivers/base/core.c:107
NIP: c015c1a8 LR: c015c1a8 CTR: c0157488
REGS: c34bdcf0 TRAP: 0700 Not tainted (2.6.23-rc5-g9ebadfbb-dirty)
MSR: 00029032 <EE,ME,IR,DR> CR: 24088422 XER: 00000000
...
[c34bdda0] [c015c1a8] device_release+0x78/0x80 (unreliable)
[c34bddb0] [c01354cc] kobject_cleanup+0x80/0xbc
[c34bddd0] [c01365f0] kref_put+0x54/0x6c
[c34bdde0] [c013543c] kobject_put+0x24/0x34
[c34bddf0] [c015c384] put_device+0x1c/0x2c
[c34bde00] [c0180e84] mdiobus_unregister+0x2c/0x58
...
Though actually there is nothing broken, it just device
subsystem core expects another "pattern" of resource managment.
This patch implement phy device's release function, thus
we're getting rid of this badness.
Also small hidden bug fixed, hope none other introduced. ;-)
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Acked-by: Andy Fleming <afleming@freescale.com>
---
drivers/net/phy/mdio_bus.c | 9 +++++----
drivers/net/phy/phy_device.c | 12 ++++++++++++
include/linux/phy.h | 1 +
3 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index fc2f0e6..c30196d 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -91,9 +91,12 @@ int mdiobus_register(struct mii_bus *bus)
err = device_register(&phydev->dev);
- if (err)
+ if (err) {
printk(KERN_ERR "phy %d failed to register\n",
i);
+ phy_device_free(phydev);
+ phydev = NULL;
+ }
}
bus->phy_map[i] = phydev;
@@ -110,10 +113,8 @@ void mdiobus_unregister(struct mii_bus *bus)
int i;
for (i = 0; i < PHY_MAX_ADDR; i++) {
- if (bus->phy_map[i]) {
+ if (bus->phy_map[i])
device_unregister(&bus->phy_map[i]->dev);
- kfree(bus->phy_map[i]);
- }
}
}
EXPORT_SYMBOL(mdiobus_unregister);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index f6e4848..5b9e175 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -44,6 +44,16 @@ static struct phy_driver genphy_driver;
extern int mdio_bus_init(void);
extern void mdio_bus_exit(void);
+void phy_device_free(struct phy_device *phydev)
+{
+ kfree(phydev);
+}
+
+static void phy_device_release(struct device *dev)
+{
+ phy_device_free(to_phy_device(dev));
+}
+
struct phy_device* phy_device_create(struct mii_bus *bus, int addr, int phy_id)
{
struct phy_device *dev;
@@ -54,6 +64,8 @@ struct phy_device* phy_device_create(struct mii_bus *bus, int addr, int phy_id)
if (NULL == dev)
return (struct phy_device*) PTR_ERR((void*)-ENOMEM);
+ dev->dev.release = phy_device_release;
+
dev->speed = 0;
dev->duplex = -1;
dev->pause = dev->asym_pause = 0;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e10763d..554836e 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -403,6 +403,7 @@ int phy_mii_ioctl(struct phy_device *phydev,
int phy_start_interrupts(struct phy_device *phydev);
void phy_print_status(struct phy_device *phydev);
struct phy_device* phy_device_create(struct mii_bus *bus, int addr, int phy_id);
+void phy_device_free(struct phy_device *phydev);
extern struct bus_type mdio_bus_type;
#endif /* __PHY_H */
--
1.5.2.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH][RESEND] PHY: Add the phy_device_release device method.
2007-12-04 13:17 ` Anton Vorontsov
@ 2007-12-04 20:06 ` Jeff Garzik
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Garzik @ 2007-12-04 20:06 UTC (permalink / raw)
To: avorontsov
Cc: Thierry Reding, Andrew Morton, netdev, David Miller, Andy Fleming
Anton Vorontsov wrote:
> On Tue, Dec 04, 2007 at 08:38:47AM +0100, Thierry Reding wrote:
>> * Andrew Morton wrote:
>>> On Mon, 3 Dec 2007 09:35:11 +0100 Thierry Reding <thierry.reding@avionic-design.de> wrote:
>>>
> [...]
>>> 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.
>
> Heh, it has been otherwise Acked-by: Andy Fleming, informal(?) phylib
> maintainer.
>
>>> 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.
>
> Makes sense, I think. Here is the newer patch, the only difference is
> removed EXPORT_SYMBOL(). Because of trivial change, I dared to keep
> Andy's Acked-by intact.
>
> - - - -
> From: Anton Vorontsov <avorontsov@ru.mvista.com>
> Subject: [PATCH] phy: implement release function
>
> Lately I've got this nice badness on mdio bus removal:
>
> Device 'e0103120:06' does not have a release() function, it is broken and must be fixed.
> ------------[ cut here ]------------
> Badness at drivers/base/core.c:107
> NIP: c015c1a8 LR: c015c1a8 CTR: c0157488
> REGS: c34bdcf0 TRAP: 0700 Not tainted (2.6.23-rc5-g9ebadfbb-dirty)
> MSR: 00029032 <EE,ME,IR,DR> CR: 24088422 XER: 00000000
> ...
> [c34bdda0] [c015c1a8] device_release+0x78/0x80 (unreliable)
> [c34bddb0] [c01354cc] kobject_cleanup+0x80/0xbc
> [c34bddd0] [c01365f0] kref_put+0x54/0x6c
> [c34bdde0] [c013543c] kobject_put+0x24/0x34
> [c34bddf0] [c015c384] put_device+0x1c/0x2c
> [c34bde00] [c0180e84] mdiobus_unregister+0x2c/0x58
> ...
>
> Though actually there is nothing broken, it just device
> subsystem core expects another "pattern" of resource managment.
>
> This patch implement phy device's release function, thus
> we're getting rid of this badness.
>
> Also small hidden bug fixed, hope none other introduced. ;-)
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> Acked-by: Andy Fleming <afleming@freescale.com>
applied #upstream-fixes
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-12-04 20:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20071119.214215.207388094.davem@davemloft.net>
2007-12-01 22:02 ` Fw: [PATCH] Add the phy_device_release device method Jeff Garzik
2007-12-03 8:35 ` [PATCH][RESEND] PHY: " Thierry Reding
2007-12-04 5:44 ` Andrew Morton
2007-12-04 7:38 ` Thierry Reding
2007-12-04 13:17 ` Anton Vorontsov
2007-12-04 20:06 ` Jeff Garzik
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.