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... >> 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. -- Simon Arlott -- 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
