Alan Stern <[email protected]> writes:
> On Thu, 13 Feb 2014, Bjørn Mork wrote:
>> Alan Stern <[email protected]> writes:
>>
>> >> Schould we request to fix that in
>> >> user space, or can we remove this warning like on below patch (call
>> >> CLEAR_HALT ioctl, does not seems to do any harm except printing a
>> >> warning) ?
>> >
>> > Doing an endpoint reset while URBs are queued is a bug, because the
>> > results are not defined. It should be fixed in the userspace program.
>>
>> Maybe the usbfs driver should catch bugs like this and log a message
>> pointing to the buggy userspace "driver" instead of blindly forwarding
>> the clear halt? That would make it more obvious that this is not a
>> kernel bug.
>
> You mean something like this?
Yes, that looks nice. And maybe even reject the command to avoid the
warning from the hcd?
Bjørn
>
> drivers/usb/core/devio.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> Index: usb-3.14/drivers/usb/core/devio.c
> ===================================================================
> --- usb-3.14.orig/drivers/usb/core/devio.c
> +++ usb-3.14/drivers/usb/core/devio.c
> @@ -1043,6 +1043,20 @@ static int proc_bulk(struct dev_state *p
> return ret;
> }
>
> +static void check_reset_of_active_ep(struct usb_device *udev,
> + unsigned int epnum, char *ioctl_name)
> +{
> + struct usb_host_endpoint **eps;
> + struct usb_host_endpoint *ep;
> +
> + eps = (epnum & USB_DIR_IN) ? udev->ep_in : udev->ep_out;
> + ep = eps[epnum & 0x0f];
> + if (ep && !list_empty(&ep->urb_list))
> + pr_warn("Process %d (%s) called USBDEVFS_%s for active endpoint
> %s ep %02x\n",
> + task_pid_nr(current), current->comm,
> + ioctl_name, dev_name(&udev->dev), epnum);
> +}
> +
> static int proc_resetep(struct dev_state *ps, void __user *arg)
> {
> unsigned int ep;
> @@ -1056,6 +1070,7 @@ static int proc_resetep(struct dev_state
> ret = checkintf(ps, ret);
> if (ret)
> return ret;
> + check_reset_of_active_ep(ps->dev, ep, "RESETEP");
> usb_reset_endpoint(ps->dev, ep);
> return 0;
> }
> @@ -1074,6 +1089,7 @@ static int proc_clearhalt(struct dev_sta
> ret = checkintf(ps, ret);
> if (ret)
> return ret;
> + check_reset_of_active_ep(ps->dev, ep, "CLEAR_HALT");
> if (ep & USB_DIR_IN)
> pipe = usb_rcvbulkpipe(ps->dev, ep & 0x7f);
> else
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
