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-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
