Subject: Re: mt76: dealing with disconnections, -ENODEV and -EPROTO

From: Michał Pecio <[email-masked]>
Date: Fri, 20 Dec 2024 00:27:59 +0100

> >  		ret = usb_control_msg(udev, pipe, req, req_type, val,
> >  				      offset, buf, len, MT_VEND_REQ_TOUT_MS);
> > -		if (ret == -ENODEV)
> > +		if (ret == -ENODEV || ret == -EPROTO)
> >  			set_bit(MT76_REMOVED, &dev->phy.state);
> > -		if (ret >= 0 || ret == -ENODEV)
> > +		if (ret >= 0 || ret == -ENODEV || ret == -EPROTO)
> >  			return ret;
> >  		usleep_range(5000, 10000);
> 
> How do other drivers handle this?
> Can -EPROTO happen in other cases, not only unplugging, which this
> patch would break?

Yes, -EPROTO may be a transient error, although they are relatively
rare as some retries are done before the URB fails. This patch will
only break things if things work in the first place, i.e. the driver
has sensible retry policies, the hardware doesn't get confused, etc.

Note that -EPROTO is not guaranteed in this case, see
Documentation/driver-api/usb/error-codes.rst

I know that xHCI gives -EPROTO and it looks like EHCI does too (IDK
if this is reliable), but I just checked that OHCI gives -ETIME.

I don't have this hardware, but I played with some other wired/WiFi
dongles and observed similar problems of varying severity.

It looks to me like USB core doesn't actually return -ENODEV on
disconnected devices, or at least doesn't react to disconnection
until .probe() returns, I am not yet sure which of those.

And drivers don't seem to expect -EPROTO or -ETIME to be fatal.

And maybe they should, if they wouldn't be able to recover from it
anyway, and I know that there are drivers which can't. But I think
this USB subsystem behavior is suboptimal too.

Regards,
Michal