On Thu, 17 Apr 2014, Laurent Pinchart wrote:
> Hi Thomas, > > On Wednesday 16 April 2014 12:29:22 Thomas Pugliese wrote: > > On Wed, 16 Apr 2014, Laurent Pinchart wrote: > > > Hi Thomas, > > > > > > (CC'ing the linux-usb mailing list) > > > > > > On Tuesday 15 April 2014 16:45:28 Thomas Pugliese wrote: > > > > On Tue, 15 Apr 2014, Laurent Pinchart wrote: > > > > > Hi Thomas, > > > > > > > > > > Could you please send me a proper revert patch with the above > > > > > description in the commit message and CC Mauro Carvalho Chehab > > > > > <m.che...@samsung.com> ? > > > > > > > > Hi Laurent, > > > > I can submit a patch to revert but I should make a correction first. I > > > > had backported this change to an earlier kernel (2.6.39) which was > > > > before super speed support was added and the regression I described was > > > > based on that kernel. It was actually the addition of super speed > > > > support that broke windows compatible devices. My previous change fixed > > > > spec compliant devices but left windows compatible devices broken. > > > > > > > > Basically, the timeline of changes is this: > > > > > > > > 1. Prior to the addition of super speed support (commit > > > > 6fd90db8df379e215): all WUSB devices were treated as HIGH_SPEED devices. > > > > This is how Windows works so Windows compatible devices would work. For > > > > spec compliant WUSB devices, the max packet size would be incorrectly > > > > calculated which would result in high-bandwidth isoc streams being > > > > unable to find an alt setting that provided enough bandwidth. > > > > > > > > 2. After super speed support: all WUSB devices fell through to the > > > > default case of uvc_endpoint_max_bpi which would mask off the upper bits > > > > of the max packet size. This broke both WUSB spec compliant and non > > > > compliant devices because no endpoint with a large enough bpi would be > > > > found. > > > > > > > > 3. After 79af67e77f86404e77e: Spec compliant devices are fixed but > > > > non-spec compliant (although Windows compatible) devices are broken. > > > > Basically, this is the opposite of how it worked prior to super speed > > > > support. > > > > > > > > Given that, I can submit a patch to revert 79af67e77f86404e77e but that > > > > would go back to having all WUSB devices broken. Alternatively, the > > > > change below will revert the behavior back to scenario 1 where Windows > > > > compatible devices work but strictly spec complaint devices may not. > > > > > > > > I can send a proper patch for whichever scenario you prefer. > > > > > > Thank you for the explanation. > > > > > > Reverting 79af67e77f86404e77e doesn't seem like a very good idea, given > > > that all WUSB devices will be broken. We thus have two options: > > > > > > - leaving the code as-is, with support for spec-compliant WUSB devices but > > > not for microsoft-specific devices > > > > > > - applying the patch below, with support for microsoft-specific USB > > > devices but not for spec-compliant devices > > > > > > This isn't the first time this kind of situation occurs. Microsoft didn't > > > support multiple configurations before Windows 8, making vendors come up > > > with lots of "creative" MS-specific solutions. I consider those devices > > > non USB compliant, and they should not be allowed to use the USB logo, > > > but that would require a strong political move from the USB Implementers > > > Forum which is more or less controlled by Microsoft... Welcome to the USB > > > mafia. > > > > > > Anyway, I have no experience with WUSB devices, so I don't know what's > > > more common in the wild. What would you suggest ? > > > > I think that almost all current devices support the Windows/USB 2.0 format > > rather than stricty follow the WUSB spec. Even the prototype device that > > I initially used to test UVC with Wireless USB has been updated to use the > > USB 2.0 format prior to shipping in order to remain compatible with > > Windows. That being said, these devices are not very common at all in the > > consumer market. They are mostly used in embedded/industrial settings so > > that may factor in as to which direction you want to go. > > > > > Would there be a way to support > > > both categories of devices ? > > > > As you had mentioned previously, it should be possible to support both > > formats by ignoring the endpoint descriptor and looking at the bMaxBurst, > > bOverTheAirInterval and wOverTheAirPacketSize fields in the WUSB endpoint > > companion descriptor. That is a more involved change to the UVC driver > > and also would require changes to USB core to store the WUSB endpoint > > companion descriptor in struct usb_host_endpoint similar to what is done > > for super speed devices. > > It's more complex indeed, but I believe it would be worth it. Any volunteer ? > ;-) In the meantime I'm fine with a patch that reverts to the previous > behaviour. Please include the explanation of the problem in the commit > message. > > -- > Regards, > > Laurent Pinchart > I may make an attempt at the more complete fix once I finish some of the other items in my queue. For clarification, would you like a patch that reverts to the pre-super speed behavior where windows-compatible devices work not but spec compliant devices will not (i.e. treat USB_SPEED_HIGH and USB_SPEED_WIRELESS the same)? Regards, Thomas -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
