On Tue, Mar 15, 2022 at 06:44:03AM +0100, Oleksij Rempel wrote:

On Mon, Mar 14, 2022 at 08:14:36PM +0100, Andrew Lunn wrote:

quoted

On Mon, Mar 14, 2022 at 07:42:34PM +0100, Lukas Wunner wrote:

quoted

On Thu, Mar 10, 2022 at 12:38:20PM +0100, Oleksij Rempel wrote:

quoted

On Thu, Mar 10, 2022 at 12:25:08PM +0100, Oliver Neukum wrote:

quoted

I got bug reports that 2c9d6c2b871d ("usbnet: run unbind() before
unregister_netdev()") is causing regressions.
Rather than simply reverting it,
it seems to me that the call needs to be split. One in the old place
and one in the place you moved it to.

I disagree.  The commit message claims that the change is necessary
because phy_disconnect() fails if called with
phydev->attached_dev == NULL.

The only place i see which sets phydev->attached_dev is
phy_attach_direct(). So if phydev->attached_dev is NULL, the PHY has
not been attached, and hence there is no need to call
phy_disconnect().

phydev->attached_dev is not NULL.

Right, I was mistaken, sorry.

It was linked to unregistered/freed
netdev. This is why my patch changing the order to call phy_disconnect()
first and then unregister_netdev().

Unregistered yes, but freed no.  Here's the order before 2c9d6c2b871d:

  usbnet_disconnect()
    unregister_netdev()
    ax88772_unbind()
      phy_disconnect()
    free_netdev()

Is it illegal to disconnect a PHY from an unregistered, but not yet freed
net_device?

Oleksij, the commit message of 2c9d6c2b871d says that disconnecting the
PHY "fails" in that situation.  Please elaborate what the failure looked
like.  Did you get a stacktrace?

Thanks,

Lukas