On Sat, 29 Feb 2020, Noralf Trønnes wrote:
> >> +static void mud_irq_urb_completion(struct urb *urb)
> >> +{
> >> + struct device *dev = &urb->dev->dev;
> >> + int ret;
> >> +
> >> + mdebug("%s: actual_length=%u\n", __func__, urb->actual_length);
> >> +
> >> + switch (urb->status) {
> >> + case 0:
> >> + mud_irq_queue(urb);
> >> + break;
> >> + case -EPROTO: /* FIXME: verify: dwc2 reports this on disconnect */
> >
> > What does this mean? Why can't you fix it now?
>
> I don't know if this is a dwc2 driver problem or if EPROTO is a valid
> disconnect error. I haven't seen it in other gadget drivers, so I need
Note: This is not a gadget driver. You should be looking in device
drivers.
> to look more into this or even better if someone from USB can answer this.
See Documentation/driver-api/usb/error-codes.rst. In short, -EPROTO is
one of several status codes you may get when an URB fails because the
device was disconnected.
> >> + case -ECONNRESET:
> >> + case -ENOENT:
> >> + case -ESHUTDOWN:
> >> + dev_dbg(dev, "irq urb shutting down with status: %d\n", urb->status);
> >
> > s/irq/IRQ/ in all comments and prints.
> >
> > Same with URB?
> >
> >> + return;
> >> + default:
> >> + dev_dbg(dev, "irq urb failure with status: %d\n", urb->status);
> >> + break;
> >
> > So it's failed, but you're going to attempt to submit it anyway?
>
> Yes, I don't know the reason why it failed, it might succeed the next
> time. But this is also something that someone with real life experience
> with USB failures could weigh in on. Maybe I should send a reset request
> so the device can reset its state machine, I don't know.
USB connections are usually pretty reliable. Sometimes there are
transient errors, but they are relatively rare. No one would criticize
a driver for giving up the first time it gets an error (especially if
there was an easy way to reset it) -- but people will get annoyed if a
ton of error messages shows up on the console whenever they unplug the
device.
In general, the overall design of the driver seems to be reasonable.
I can't judge the interfaces with other subsystems or the other aspects
of their design, but the USB part is okay. (I haven't gone through it
in detail.)
Alan Stern
