cmsis-dap: prevent hidapi to search again for the adapter
authorAntonio Borneo <borneo.antonio@gmail.com>
Mon, 29 Jun 2020 21:22:37 +0000 (23:22 +0200)
committerAntonio Borneo <borneo.antonio@gmail.com>
Mon, 13 Jul 2020 23:39:52 +0000 (00:39 +0100)
The code in cmsis_dap_usb_open() already searches for the right
HID device that corresponds to the adapter. By calling hid_open()
it asks hidapi to re-search the adapter again based on the VID:PID
and the serial string of the adapter it has just found!

Apart from being a run-time overhead, this has an additional
drawback; there are USB adapters built as composite USB devices
that, beside the cmsis-dap HID interface, have other HID
interfaces for other purposes.
A typical example is the NXP LPC-Link2, that over the 5 interfaces
0) cmsis-dap (HID)
1) VCOM-CDC
2) VCOM-CDC
3) LPCSIO (HID)
4) LPC-LINK2 DATA PORT (HID)
has 3 of them of HID class.
The code in cmsis_dap_usb_open() could select the right interface
but then cannot propagate this information to hid_open().

Replace the call to hid_open() with hid_open_path(), passing as
parameter the "unique" path of the HID device already found.

Checking in hidapi source code, the implementation of hid_open()
consists in enumerating the HID devices, scan for the first one
matching VID:PID and serial number, and opening it by calling
hid_open_path(). This analysis highlights that using directly
hid_open_path() should not introduce any regression.

While applying these changes, move hid_init() before enumerating
the HID devices. This has no real consequences because the HID API
is marked as optional but, logically, it should be called before
any other HID API.

Change-Id: I77ec01dca64223ec597f21f188f363366d0049c6
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Suggested-by: Masatoshi Tateishi <tateishim3@gmail.com>
Reviewed-on: http://openocd.zylin.com/5731
Tested-by: jenkins
src/jtag/drivers/cmsis_dap_usb.c

index ee1cb533c050a730a880350b53b793be5287e304..89158368043559385211bbc965e2b6c612f5be71 100644 (file)
@@ -229,14 +229,16 @@ static int cmsis_dap_usb_open(void)
        int i;
        struct hid_device_info *devs, *cur_dev;
        unsigned short target_vid, target_pid;
-       wchar_t *target_serial = NULL;
-
        bool found = false;
-       bool serial_found = false;
 
        target_vid = 0;
        target_pid = 0;
 
+       if (hid_init() != 0) {
+               LOG_ERROR("unable to open HIDAPI");
+               return ERROR_FAIL;
+       }
+
        /*
         * The CMSIS-DAP specification stipulates:
         * "The Product String must contain "CMSIS-DAP" somewhere in the string. This is used by the
@@ -273,7 +275,6 @@ static int cmsis_dap_usb_open(void)
                        /* check serial number matches if given */
                        if (cmsis_dap_serial != NULL) {
                                if ((cur_dev->serial_number != NULL) && wcscmp(cmsis_dap_serial, cur_dev->serial_number) == 0) {
-                                       serial_found = true;
                                        break;
                                }
                        } else
@@ -288,23 +289,16 @@ static int cmsis_dap_usb_open(void)
        if (NULL != cur_dev) {
                target_vid = cur_dev->vendor_id;
                target_pid = cur_dev->product_id;
-               if (serial_found)
-                       target_serial = cmsis_dap_serial;
        }
 
-       hid_free_enumeration(devs);
-
        if (target_vid == 0 && target_pid == 0) {
                LOG_ERROR("unable to find CMSIS-DAP device");
+               hid_free_enumeration(devs);
                return ERROR_FAIL;
        }
 
-       if (hid_init() != 0) {
-               LOG_ERROR("unable to open HIDAPI");
-               return ERROR_FAIL;
-       }
-
-       dev = hid_open(target_vid, target_pid, target_serial);
+       dev = hid_open_path(cur_dev->path);
+       hid_free_enumeration(devs);
 
        if (dev == NULL) {
                LOG_ERROR("unable to open CMSIS-DAP device 0x%x:0x%x", target_vid, target_pid);