"modprobe asix ; rmmod asix ; modprobe asix" fails with:
sysfs: cannot create duplicate filename \
'/devices/virtual/mdio_bus/usb-003:004'
Issue was originally reported by Anton Lundin on 2022-06-22 14:16 UTC:
https://lore.kernel.org/netdev/[email protected]/T/
Chrome OS team hit the same issue in Feb, 2023 when trying to find
work arounds for other issues with AX88172 devices.
The use of devm_mdiobus_register() with usbnet devices results in the
MDIO data being associated with the USB device. When the asix driver
is unloaded, the USB device continues to exist and the corresponding
"mdiobus_unregister()" is NOT called until the USB device is unplugged
or unauthorized. So the next "modprobe asix" will fail because the MDIO
phy sysfs attributes still exist.
The 'easy' (from a design PoV) fix is to use the non-devm variants of
mdiobus_* functions and explicitly manage this use in the asix_bind
and asix_unbind function calls. I've not explored trying to fix usbnet
initialization so devm_* stuff will work.
Fixes: e532a096be0e5 ("net: usb: asix: ax88772: add phylib support")
Reported-by: Anton Lundin <[email protected]>
Tested-by: Eizan Miyamoto <[email protected]>
Signed-off-by: Grant Grundler <[email protected]>
---
drivers/net/usb/asix_devices.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
V2: moved mdiobus_get_phy() call back into ax88772_init_phy()
Lukas Wunner is entirely correct this patch is much easier
to backport without this patch hunk.
Added "Fixes:" tag per request from Florian Fainelli
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 30e87389aefa1..21845b88a64b9 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -640,8 +640,9 @@ static int asix_resume(struct usb_interface *intf)
static int ax88772_init_mdio(struct usbnet *dev)
{
struct asix_common_private *priv = dev->driver_priv;
+ int ret;
- priv->mdio = devm_mdiobus_alloc(&dev->udev->dev);
+ priv->mdio = mdiobus_alloc();
if (!priv->mdio)
return -ENOMEM;
@@ -653,7 +654,20 @@ static int ax88772_init_mdio(struct usbnet *dev)
snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
dev->udev->bus->busnum, dev->udev->devnum);
- return devm_mdiobus_register(&dev->udev->dev, priv->mdio);
+ ret = mdiobus_register(priv->mdio);
+ if (ret) {
+ netdev_err(dev->net, "Could not register MDIO bus (err %d)\n", ret);
+ mdiobus_free(priv->mdio);
+ priv->mdio = NULL;
+ }
+
+ return ret;
+}
+
+static void ax88772_mdio_unregister(struct asix_common_private *priv)
+{
+ mdiobus_unregister(priv->mdio);
+ mdiobus_free(priv->mdio);
}
static int ax88772_init_phy(struct usbnet *dev)
@@ -664,6 +678,7 @@ static int ax88772_init_phy(struct usbnet *dev)
priv->phydev = mdiobus_get_phy(priv->mdio, priv->phy_addr);
if (!priv->phydev) {
netdev_err(dev->net, "Could not find PHY\n");
+ ax88772_mdio_unregister(priv);
return -ENODEV;
}
@@ -805,6 +820,7 @@ static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
struct asix_common_private *priv = dev->driver_priv;
phy_disconnect(priv->phydev);
+ ax88772_mdio_unregister(priv);
asix_rx_fixup_common_free(dev->driver_priv);
}
--
2.39.2
On Tue, 7 Mar 2023 12:05:01 -0800 Grant Grundler wrote:
> Subject: [PATCHv2 1/2] TEST:net: asix: fix modprobe "sysfs: cannot create duplicate filename"
Why the "TEST:" prefix?
The patch doesn't apply cleanly, it needs to go via this tree:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/
so rebase it onto that, please, and put [PATCH net] in the subject
rather than just [PATCH].
Keep patch 2 locally for about a week (we merge fixes and cleanup
branches once a week around Thu, and the two patches depend on each
other).
Please look thru at least the tl;dr of our doc:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
On Tue, Mar 7, 2023 at 4:47 PM Jakub Kicinski <[email protected]> wrote:
>
> On Tue, 7 Mar 2023 12:05:01 -0800 Grant Grundler wrote:
> > Subject: [PATCHv2 1/2] TEST:net: asix: fix modprobe "sysfs: cannot create duplicate filename"
>
> Why the "TEST:" prefix?
Sorry - that's left over from how I mark the change for testing with
chromeos-5.15 kernel branch:
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/4313619
I should have removed that. I upload the change to gerrit so partners
can easily test the same code (e.g. coworker Eizan who is in
Australia).
If you follow the link above, you can see I'm testing a bunch of
additional backports as well and have additional fields in the commit
message required by chromium.org.
> The patch doesn't apply cleanly, it needs to go via this tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/
> so rebase it onto that, please, and put [PATCH net] in the subject
> rather than just [PATCH].
Ok - thanks! Wil repost v3 against netdev/net.git/ shortly. No problem.
> Keep patch 2 locally for about a week (we merge fixes and cleanup
> branches once a week around Thu, and the two patches depend on each
> other).
Awesome! Sounds good.
> Please look thru at least the tl;dr of our doc:
> https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
Thanks!
Because I've been "randomly" contributing to netdev for 20+ years,
I've not looked for documentation (beyond SubmittingPatches). But I am
quite willing to read and follow it - makes life easier for everyone.
cheers,
grant