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

[Simon Arlott]

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-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/