diff mbox

[net-next,4/8] net: dsa: mv88e6xxx: do not increment bus refcount

Message ID 20160609004456.5441-5-vivien.didelot@savoirfairelinux.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Vivien Didelot June 9, 2016, 12:44 a.m. UTC
The MDIO device probe and remove functions are respectively incrementing
and decrementing the bus refcount themselves. Since these bus level
actions are out of the device scope, remove them.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 3 ---
 1 file changed, 3 deletions(-)

Comments

Andrew Lunn June 9, 2016, 2:36 a.m. UTC | #1
On Wed, Jun 08, 2016 at 08:44:52PM -0400, Vivien Didelot wrote:
> The MDIO device probe and remove functions are respectively incrementing
> and decrementing the bus refcount themselves. Since these bus level
> actions are out of the device scope, remove them.

I agree with the patch. But have you checked the mdio layer is doing
the right thing? If not, we should fix that first.

    Andrew
Vivien Didelot June 10, 2016, 7:59 p.m. UTC | #2
Hi,

Andrew Lunn <andrew@lunn.ch> writes:

> On Wed, Jun 08, 2016 at 08:44:52PM -0400, Vivien Didelot wrote:
>> The MDIO device probe and remove functions are respectively incrementing
>> and decrementing the bus refcount themselves. Since these bus level
>> actions are out of the device scope, remove them.
>
> I agree with the patch. But have you checked the mdio layer is doing
> the right thing? If not, we should fix that first.

So I added some printing after incrementing/decrementing the refcount in
get_device/put_device to track &ps->bus->dev, which name is "0.1".

Regardless having this patch or not, the refcount of the 0.1 mii_bus
device is 5 before loading the mv88e6xxx module on vf610-zii-dev-rev-b.

Below is a portion of dmesg:

    [    8.921647] get_device: 400d1000.etherne refcount: 4
    [    8.926225] get_device: 0.1 refcount: 2
    [    8.929561] get_device: mdio-mux refcount: 5
    [    8.934076] get_device: 0.1 refcount: 3
    [    8.937446] get_device: 0.1 refcount: 4
    [    8.940792] put_device: 0.1 refcount: 3
    [    8.944181] libphy: mdio_mux: probed
    [    8.947885] mdio_bus 0.1:00: mdio_device_register
    [    8.952649] get_device: 0.1:00 refcount: 2
    [    8.956283] get_device: 0.1 refcount: 4
    [    8.959838] get_device: 0.1:00 refcount: 3
    [    8.963991] get_device: 0.1:00 refcount: 4
    [    8.967598] put_device: 0.1:00 refcount: 3
    [    8.971298] get_device: 0.2 refcount: 2
    [    8.974687] get_device: mdio-mux refcount: 7

So it seems like of_ is managing the bus refcount on events such as a
new child node (0.1:00).

Thanks,

        Vivien
Andrew Lunn June 10, 2016, 8:06 p.m. UTC | #3
On Fri, Jun 10, 2016 at 03:59:22PM -0400, Vivien Didelot wrote:
> Hi,
> 
> Andrew Lunn <andrew@lunn.ch> writes:
> 
> > On Wed, Jun 08, 2016 at 08:44:52PM -0400, Vivien Didelot wrote:
> >> The MDIO device probe and remove functions are respectively incrementing
> >> and decrementing the bus refcount themselves. Since these bus level
> >> actions are out of the device scope, remove them.
> >
> > I agree with the patch. But have you checked the mdio layer is doing
> > the right thing? If not, we should fix that first.
> 
> So I added some printing after incrementing/decrementing the refcount in
> get_device/put_device to track &ps->bus->dev, which name is "0.1".
> 
> Regardless having this patch or not, the refcount of the 0.1 mii_bus
> device is 5 before loading the mv88e6xxx module on vf610-zii-dev-rev-b.
> 
> Below is a portion of dmesg:
> 
>     [    8.921647] get_device: 400d1000.etherne refcount: 4
>     [    8.926225] get_device: 0.1 refcount: 2
>     [    8.929561] get_device: mdio-mux refcount: 5
>     [    8.934076] get_device: 0.1 refcount: 3
>     [    8.937446] get_device: 0.1 refcount: 4
>     [    8.940792] put_device: 0.1 refcount: 3
>     [    8.944181] libphy: mdio_mux: probed
>     [    8.947885] mdio_bus 0.1:00: mdio_device_register
>     [    8.952649] get_device: 0.1:00 refcount: 2
>     [    8.956283] get_device: 0.1 refcount: 4
>     [    8.959838] get_device: 0.1:00 refcount: 3
>     [    8.963991] get_device: 0.1:00 refcount: 4
>     [    8.967598] put_device: 0.1:00 refcount: 3
>     [    8.971298] get_device: 0.2 refcount: 2
>     [    8.974687] get_device: mdio-mux refcount: 7
> 
> So it seems like of_ is managing the bus refcount on events such as a
> new child node (0.1:00).

Great, thanks for looking at this.

Acked-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
diff mbox

Patch

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index cfa30ae..02b0af7 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3710,8 +3710,6 @@  int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	ps->sw_addr = mdiodev->addr;
 	mutex_init(&ps->smi_mutex);
 
-	get_device(&ps->bus->dev);
-
 	ds->drv = &mv88e6xxx_switch_driver;
 
 	id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
@@ -3765,7 +3763,6 @@  static void mv88e6xxx_remove(struct mdio_device *mdiodev)
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
 	dsa_unregister_switch(ds);
-	put_device(&ps->bus->dev);
 
 	mv88e6xxx_mdio_unregister(ps);
 }