Re: smsc9511: Register access happens after unregistration

[Andrew Lunn <andrew@lunn.ch>]

On Fri, Mar 04, 2022 at 03:01:28PM -0300, Fabio Estevam wrote:
> On Fri, Mar 4, 2022 at 12:57 PM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > On Fri, Mar 4, 2022 at 11:51 AM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > But why does it return ENODEV? It seems to me, ignoring it is papering
> > > over the cracks. Why cannot we access to the PHY?
> >
> > The -ENODEV is returned by usb_control_msg():
> >
> > __smsc95xx_read_reg: -19
> >          usbnet_read_cmd: -19
> >               usb_control_msg: -19
> 
> I added a WARN_ON() inside usb_control_msg() that triggers when
> usb_control_msg() returns -ENODEV.

https://elixir.bootlin.com/linux/v5.17-rc6/source/include/linux/usb.h#L1126

say:

 * @disconnect: Called when the interface is no longer accessible, usually
 *	because its device has been (or is being) disconnected or the
 *	driver module is being unloaded.

So i guess the USB core has disconnected the device, and is blocking
further control messages. I guess it handles shutdown the same as hot
unplug. So there are a couple things that can be done to make this
better.

Make __smsc95xx_phy_wait_not_busy(), __smsc95xx_mdio_read() and
__smsc95xx_mdio_write() not print a message on -ENODEV. I would
continue printing the warning for other error codes. The phylib and
phy drivers should be O.K. if they get -ENODEV.

Non-USB Ethernet drivers follow one of two patterns:

1) The PHY is connected in probe, started in open, stopped in close,
   and disconnected in remove.

2) The PHY is both connected and started in open, and stopped and
   disconnected in close.

Depending on what userspace you are using, i think it is normal for
userspace to ifdown interfaces during shutdown. So if you make use of
2) the PHY should be disconnected by the time the USB subsystem calls
the USB disconnect callback. However, for hot unplug, there will not
be an ifdown, so the disconnect callback needs to look at state
information and stop and disconnect the PHY if it is currently
connected and started.

	Andrew