debugprobe icon indicating copy to clipboard operation
debugprobe copied to clipboard

Add MS OS Descriptor 2.0 to load WinUSB automatically on Windows.

Open ciniml opened this issue 4 years ago • 10 comments
trafficstars

This PR adds MS OS Descriptor 2.0 to load WinUSB driver automatically on Windows.

MS OS 2.0 Descriptor spec is here. https://docs.microsoft.com/en-us/windows-hardware/drivers/usbcon/microsoft-os-2-0-descriptors-specification

I've confirmed that this modification works with Windows 10 Pro 20H2, and OpenOCD can access to the picoprobe correctly.

I think this PR solves #15, because after this modification we don't need to install driver with Zadig anymore.

ciniml avatar Jul 15 '21 21:07 ciniml

That page says "Please read the license agreement before continuing." and I'm not familiar enough with legalese to be able to tell if that licence allows Open Source implementations, so pinging @ghollingworth

lurch avatar Jul 15 '21 21:07 lurch

If you want to support Windows 7 then it is better to use the old MS OS 1.0 descriptors. https://docs.microsoft.com/en-us/windows-hardware/drivers/usbcon/microsoft-defined-usb-descriptors

mcuee avatar Jul 19 '21 10:07 mcuee

If you want to support Windows 7…

There is no official support for Windows versions before Windows 10.

aallan avatar Jul 19 '21 10:07 aallan

If you want to support Windows 7…

There is no official support for Windows versions before Windows 10.

I see. In that case, MS OS 2.0 descriptor is better as it better designed.

mcuee avatar Jul 19 '21 11:07 mcuee

I'm happy with that license

ghollingworth avatar Jul 19 '21 12:07 ghollingworth

Hi. Thanks for commenting to the PR.

In this code, are there any problem which blocks to merge this PR? If so, I will fix that.

ciniml avatar Jul 23 '21 21:07 ciniml

Hi. Thanks for commenting to the PR.

In this code, are there any problem which blocks to merge this PR? If so, I will fix that.

can you please match the pre-existing code style. Also despite having linked the MS spec, the descriptor is a little terse. I'm prepared to believe it works, but maybe a comment saying exactly what it is saying would be helpful. Is there anything in there that is specific/unique to picoprobe?

kilograham avatar Oct 25 '21 20:10 kilograham

This pull request appears to be using a device interface GUID of A5DCBF10-6530-11D2-901F-00C04FB951ED, which is GUID_DEVINTERFACE_USB_DEVICE, a special GUID that Microsoft's hub drivers use.

However, the device interface GUID is supposed to be a GUID indicating what type of interface this specific device has. Any application using libusb won't care about the GUID, but there is example code from Microsoft showing how to enumerate all the USB devices and then select USB devices based on their device interface GUID. If you reuse this GUID that Microsoft defined, there is some risk that applications or other drivers might connect to the picoprobe interface and expect that they are talking to a USB hub driver, but they would actually be talking to WinUSB. I recommend generating a new GUID for the picoprobe with uuidgen or guidgen and using that.

In case you are not convinced, here is documentation from Microsoft talking about how to get WinUSB working with an INF file, and it says to generate a device interface GUID using guidgen.exe: https://docs.microsoft.com/en-us/windows-hardware/drivers/usbcon/winusb-installation

The MS OS 2.0 Descriptors allow us to set up the device interface GUID using a USB descriptor instead of using an INF file, but it doesn't change the meaning of the device interface GUID.

DavidEGrayson avatar May 17 '22 01:05 DavidEGrayson

Good point to use an unique GUID.

A good reference is here at libwdi wiki site: https://github.com/pbatard/libwdi/wiki/WCID-Devices

mcuee avatar May 17 '22 03:05 mcuee

Another choice , you can just omit GUID registry definition from the MSOS20-descriptor. WinUSB and libusb both work just fine.

elfmimi avatar May 21 '22 13:05 elfmimi

Superseded by #31, which implements the above (and adds the GUID that Keil specifies for DAP interfaces).

P33M avatar Nov 22 '22 16:11 P33M