Hi,
two things from usbcore's usb_set_inteface() implementation which I'm
somewhat concerned about:
1) After having success with the SetInterface request we do
dev->toggle[0] = 0; /* 9.1.1.5 says to do this */
dev->toggle[1] = 0;
Well, as the comment suggests, according to 9.1.1.5 we have to reset the
toggles - but only "with endpoints in the affected interfaces". The above
code however resets the toggles for _all_ endpoints, regardless whether
they belong to the interface which we have just set or not.
Hence switching the AS on one interface surprisingly resets the toggles
for all other unaffected interfaces - not good IMHO ;-)
I think we need to identify the right endpoints from the interface under
consideration and mask the other endpoints.
2) Recently in one of the 2.4.18-pre series the following modification
went into usb_set_interface():
+ /* 9.4.10 says devices don't need this, if the interface
+ only has one alternate setting */
+ if (iface->num_altsetting == 1) {
+ warn("ignoring set_interface for dev %d, iface %d, alt %d",
+ dev->devnum, interface, alternate);
+ return 0;
+ }
+
if ((ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
USB_REQ_SET_INTERFACE, USB_RECIP_INTERFACE, alternate,
interface, NULL, 0, HZ * 5)) < 0)
The obvious intention is to give kind of help for drivers to deal with
devices which do return request_error (EP0-STALL) for SetInterface request
on interfaces with single AS - as described in 9.4.10. However, I think
this implementation has the potential to break USB standard conformance:
* 9.4.10 _permits_ the device to return request error ("may") - but by no
means _requires_ it to do so. The snippet above however completely
prevents SetInterface request to be ever sent to single-AS interfaces.
Since the SetInterface request basically implies a reset of the affected
endpoints it might be required nevertheless. IMHO the request should (at
least) be sent to the device to give it a chance to act as intended.
* Another point is the toggle reset. If the device comes with full support
for SetInterface we need to reset the toggles, no doubt. If it doesn't,
however - unless I missed something - the USB specs. don't make any
claims whether the toggles should be reseted anyway. I personally tend
to say no, but my feeling is this might be left to implementation. Hence
I really like the driver to see the response from the device so it can
take appropriate action based on particular knowledge f.e. Any approach
which hides the request error from the driver and maps it to success for
single-AS interfaces would obscure this IMHO.
Hence I tend to suggest not to add any special handling for this 9.4.10
optional behavior in usbcore and let the individual drivers handle this.
A simple improved error test in the driver
if (res && (res!=EPIPE || iface->num_altsetting!=1) {
/* treat as error */
}
instead of
if (res) {
/* error */
}
would do the job and having this duplicated in several drivers shouldn't
be worth the trouble, IMHO.
Comments?
Regards,
Martin
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
