ALLOW_PARTIAL_READS pipe policy may not be implemented in a way compatible to WinUSB

[mcuee]

Copies from Sourceforge: https://sourceforge.net/p/libusbk/tickets/2/

Ref: http://libusbx-devel.narkive.com/TKaErHI0/libusb-win32-and-libusbk-support-has-now-been-pushed-to-mainline#post13

https://sourceforge.net/p/libusbx/mailman/message/29736015%20/

[mcuee]

From Pete's post:

I think the issue is the interpretation of ALLOW_PARTIAL_READS from WinUSB and libusbK. If you can tolerate the atrocious new SF code browser interface (and for even more fun, try to copy a line!), [2] and the "if (!mRequestContext->Policies.AllowPartialReads)" section seems to indicate that K will reject a read buffer that is smaller than MaximumPacketSize, unless the ALLOW_PARTIAL_READS policy is set. But we disable this policy by default in X, as per [2], hence the issue.

I have of course now been able to confirm this, by commenting out the disabling of ALLOW_PARTIAL_READS in libusbx, and verifying that it lets K complete the mass storage test where it previously failed.

Now, with regards to where in libusbx or libusbK lies the problem, in xusb, we do call libusb_bulk_transfer() and tell it we expect 36 bytes, which is what K also indicates we receive. And according to what the MSDN has to say on ALLOW_PARTIAL_READS [3], "Disabling ALLOW_PARTIAL_READS causes the read requests to fail whenever the device returns more data (on bulk and interrupt IN endpoints) than the caller requested". Thus, considering that 36 bytes is what K reports, and we're therefore not getting more data than we expect, even with ALLOW_PARTIAL_READS disabled, I think K should let the transfer go through.

Especially I don't think the comparison between stageLength and MaximumPacketSize, that K does to decide what it should do with the ALLOW_PARTIAL_READS flag, is what ALLOW_PARTIAL_READS is really about.

Still, if needed, I don't see a major problem enabling ALLOW_PARTIAL_READS in 1.0.13 (thought we want to check why we decided to disable it in the first place, as some people may rely on it), but if Travis agrees that the current implementation of ALLOW_PARTIAL_READS might be off in K, I'd prefer a fix in there.

Regards,

/Pete

[1] https://github.com/mcuee/libusbk/blob/master/libusbK/src/sys/drv_xfer_bulk.c#L70 [2] https://github.com/libusbx/libusbx/blob/master/libusb/os/windows_usb.c#L2661 [3] http://msdn.microsoft.com/en-us/library/windows/hardware/ff540304%28v=vs.85%29.aspx

[mcuee]

So in the end libusb Windows now has the following codes.

policy = true;
/* ALLOW_PARTIAL_READS must be enabled due to likely libusbK bug. See:
https://sourceforge.net/mailarchive/message.php?msg_id=29736015 */      
if (!WinUSBX[sub_api].SetPipePolicy(winusb_handle, endpoint_address,
        ALLOW_PARTIAL_READS, sizeof(UCHAR), &policy))
        usbi_dbg("failed to enable ALLOW_PARTIAL_READS for endpoint %02X", endpoint_address);

[TravisRo]

I believe I did this for performance/safety reasons. If you submit a URB at the driver level with a buffer less than the maximum packet size and the device returns more data, you will get a "Device Malfunctioned" error. WinUSB always uses a temporary buffer. LibusbK only uses a temporary buffer when "ALLOW_PARTIAL_READS" is true.

We will have to live with this anomaly partially because I think this is how it should work regardless of what Microsoft thinks. :) ..but Mainly because the libusbK driver is more/less discontinued due to Windows 10 signing headaches. Going forward, we will distribute the existing signed driver with newer releases.

[mcuee]

@TravisRo Yes I agree to distribute the existing signed driver with newer releases.

It seems to me we are ready for a release, right?