Re: [PATCH] USB: output an error message when the pipe type doesn'tmatch the endpoint type

[Alan Stern]

On Thu, 2 Sep 2010, Simon Arlott wrote:

> On Wed, September 1, 2010 18:49, Alan Stern wrote:
> > On Wed, 1 Sep 2010, Simon Arlott wrote:
> >> > This is okay with me.  If you're serious about not changing the
> >> > behavior merely because debugging is enabled, you could move this test
> >> > out of the debug-only region and possibly change the dev_err to
> >> > dev_dbg.  However doing so might break some devices that are currently
> >> > working.
> >>
> >> I'd expect that to break potentially many devices, although only cxacru
> >> stopped working for me. The USB API isn't really suitable for adding
> >> this type of check because it allows the drivers to get away with too
> >> much already.
> >
> > Unlike device hardware, drivers can always be changed.  If adding a
> > check will help spot errors, it's probably worthwhile.
> 
> Yes, however the information about the device endpoint types hasn't been
> required in the past for the driver to work. The only way to check is to
> have the hardware available so the error may only show up after a full
> release of the kernel and break drivers that used to work. It could be
> enabled for all -rc kernels...

Which suggests that the best approach is to print the error message 
always, but allow the submission unless CONFIG_USB_DEBUG is set.

> >> usb_clear_halt() takes a pipe when it really wants the endpoint, the
> >> pipe type is ignored.
> >
> > What's wrong with that?  Besides, in the end we shouldn't be using
> > pipes at all; we should always use pointers to struct
> > usb_host_endpoint.
> 
> If a driver was trying to conform to the "don't use the wrong pipe type
> with an endpoint" rule then it may have to check it was using the
> correct pipe when calling usb_clear_halt(), although this is only a
> problem with drivers where the devices sometimes have interrupt instead
> of bulk endpoints.
> 
> Looking at usb_clear_halt(), it doesn't use the direction either... but
> drivers can call it in both directions. Several drivers already do this.

What do you mean?  Look at the first lines of code in usb_clear_halt():

	if (usb_pipein(pipe))
		endp |= USB_DIR_IN;

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/