jtag/drivers/cmsis_dap: fix usb bulk connection logic
[fw/openocd] / src / jtag / drivers / cmsis_dap_usb_bulk.c
index b82e3dae560b298ce388f6e7e08fd93cf17607f7..0134c0376557808ab0cdfe28797a13c3a9da5b8a 100644 (file)
@@ -81,7 +81,8 @@ static int cmsis_dap_usb_open(struct cmsis_dap *dap, uint16_t vids[], uint16_t p
 
                /* Match VID/PID */
 
-               bool id_match = true; /* match if we don't enter the loop (no filter) */
+               bool id_match = false;
+               bool id_filter = vids[0] || pids[0];
                for (int id = 0; vids[id] || pids[id]; id++) {
                        id_match = !vids[id] || dev_desc.idVendor == vids[id];
                        id_match &= !pids[id] || dev_desc.idProduct == pids[id];
@@ -90,7 +91,7 @@ static int cmsis_dap_usb_open(struct cmsis_dap *dap, uint16_t vids[], uint16_t p
                                break;
                }
 
-               if (!id_match)
+               if (id_filter && !id_match)
                        continue;
 
                /* Don't continue if we asked for a serial number and the device doesn't have one */
@@ -103,7 +104,7 @@ static int cmsis_dap_usb_open(struct cmsis_dap *dap, uint16_t vids[], uint16_t p
                        /* It's to be expected that most USB devices can't be opened
                         * so only report an error if it was explicitly selected
                         */
-                       if (vids[0] || pids[0]) {
+                       if (id_filter) {
                                LOG_ERROR("could not open device 0x%04x:0x%04x: %s",
                                                dev_desc.idVendor, dev_desc.idProduct, libusb_strerror(err));
                        } else {
@@ -115,7 +116,7 @@ static int cmsis_dap_usb_open(struct cmsis_dap *dap, uint16_t vids[], uint16_t p
 
                /* Match serial number */
 
-               bool serial_match = (serial == NULL);
+               bool serial_match = false;
                char dev_serial[256] = {0};
                if (dev_desc.iSerialNumber > 0) {
                        err = libusb_get_string_descriptor_ascii(
@@ -123,35 +124,44 @@ static int cmsis_dap_usb_open(struct cmsis_dap *dap, uint16_t vids[], uint16_t p
                                        (uint8_t *)dev_serial, sizeof(dev_serial));
 
                        if (err < 0) {
-                               LOG_ERROR("could not read serial number for device 0x%04x:0x%04x: %s",
-                                               dev_desc.idVendor, dev_desc.idProduct, libusb_strerror(err));
+                               const char *msg = "could not read serial number for device 0x%04x:0x%04x: %s";
+                               if (serial)
+                                       LOG_WARNING(msg, dev_desc.idVendor, dev_desc.idProduct,
+                                                               libusb_strerror(err));
+                               else
+                                       LOG_DEBUG(msg, dev_desc.idVendor, dev_desc.idProduct,
+                                                               libusb_strerror(err));
                        } else if (serial && strncmp(dev_serial, serial, sizeof(dev_serial)) == 0) {
                                serial_match = true;
                        }
                }
 
-               if (!serial_match) {
+               if (serial && !serial_match) {
                        libusb_close(dev_handle);
                        continue;
                }
 
                /* Find the CMSIS-DAP string in product string */
 
-               bool cmsis_dap_found = false;
+               bool cmsis_dap_in_product_str = false;
                char product_string[256] = {0};
                if (dev_desc.iProduct > 0) {
                        err = libusb_get_string_descriptor_ascii(
                                        dev_handle, dev_desc.iProduct,
                                        (uint8_t *)product_string, sizeof(product_string));
                        if (err < 0) {
-                               LOG_ERROR("could not read product string for device 0x%04x:0x%04x: %s",
+                               LOG_WARNING("could not read product string for device 0x%04x:0x%04x: %s",
                                                dev_desc.idVendor, dev_desc.idProduct, libusb_strerror(err));
                        } else if (strstr(product_string, "CMSIS-DAP")) {
-                               LOG_DEBUG("CMSIS-DAP found in product string");
-                               cmsis_dap_found = true;
+                               LOG_DEBUG("found product string of 0x%04x:0x%04x '%s'",
+                                                 dev_desc.idVendor, dev_desc.idProduct, product_string);
+                               cmsis_dap_in_product_str = true;
                        }
                }
 
+               bool device_identified_reliably = cmsis_dap_in_product_str
+                                                                                       || serial_match || id_match;
+
                /* Find the CMSIS-DAP interface */
 
                for (int config = 0; config < dev_desc.bNumConfigurations; config++) {
@@ -163,7 +173,11 @@ static int cmsis_dap_usb_open(struct cmsis_dap *dap, uint16_t vids[], uint16_t p
                                continue;
                        }
 
+                       LOG_DEBUG("enumerating interfaces of 0x%04x:0x%04x",
+                                         dev_desc.idVendor, dev_desc.idProduct);
                        int config_num = config_desc->bConfigurationValue;
+                       const struct libusb_interface_descriptor *intf_desc_candidate = NULL;
+                       const struct libusb_interface_descriptor *intf_desc_found = NULL;
 
                        for (int interface = 0; interface < config_desc->bNumInterfaces; interface++) {
                                const struct libusb_interface_descriptor *intf_desc = &config_desc->interface[interface].altsetting[0];
@@ -190,98 +204,165 @@ static int cmsis_dap_usb_open(struct cmsis_dap *dap, uint16_t vids[], uint16_t p
                                 *  - Endpoint 3: Bulk In (optional) – used for streaming SWO trace (if enabled with SWO_STREAM).
                                 */
 
-                               if (intf_desc->bNumEndpoints < 2)
+                               /* Search for "CMSIS-DAP" in the interface string */
+                               bool cmsis_dap_in_interface_str = false;
+                               if (intf_desc->iInterface != 0) {
+
+                                       char interface_str[256] = {0};
+
+                                       err = libusb_get_string_descriptor_ascii(
+                                                       dev_handle, intf_desc->iInterface,
+                                                       (uint8_t *)interface_str, sizeof(interface_str));
+                                       if (err < 0) {
+                                               LOG_DEBUG("could not read interface string %d for device 0x%04x:0x%04x: %s",
+                                                                 intf_desc->iInterface,
+                                                                 dev_desc.idVendor, dev_desc.idProduct,
+                                                                 libusb_strerror(err));
+                                       } else if (strstr(interface_str, "CMSIS-DAP")) {
+                                               cmsis_dap_in_interface_str = true;
+                                               LOG_DEBUG("found interface %d string '%s'",
+                                                                 interface_num, interface_str);
+                                       }
+                               }
+
+                               /* Bypass the following check if this interface was explicitly requested. */
+                               if (cmsis_dap_usb_interface == -1) {
+                                       if (!cmsis_dap_in_product_str && !cmsis_dap_in_interface_str)
+                                               continue;
+                               }
+
+                               /* check endpoints */
+                               if (intf_desc->bNumEndpoints < 2) {
+                                       LOG_DEBUG("skipping interface %d, has only %d endpoints",
+                                                         interface_num, intf_desc->bNumEndpoints);
                                        continue;
+                               }
 
                                if ((intf_desc->endpoint[0].bmAttributes & 3) != LIBUSB_TRANSFER_TYPE_BULK ||
-                                               (intf_desc->endpoint[0].bEndpointAddress & 0x80) != LIBUSB_ENDPOINT_OUT)
+                                               (intf_desc->endpoint[0].bEndpointAddress & 0x80) != LIBUSB_ENDPOINT_OUT) {
+                                       LOG_DEBUG("skipping interface %d, endpoint[0] is not bulk out",
+                                                         interface_num);
                                        continue;
+                               }
 
                                if ((intf_desc->endpoint[1].bmAttributes & 3) != LIBUSB_TRANSFER_TYPE_BULK ||
-                                               (intf_desc->endpoint[1].bEndpointAddress & 0x80) != LIBUSB_ENDPOINT_IN)
+                                               (intf_desc->endpoint[1].bEndpointAddress & 0x80) != LIBUSB_ENDPOINT_IN) {
+                                       LOG_DEBUG("skipping interface %d, endpoint[1] is not bulk in",
+                                                         interface_num);
                                        continue;
+                               }
 
-                               /* Bypass the following checks if this interface was explicitly requested. */
-                               if (cmsis_dap_usb_interface == -1) {
-                                       if (intf_desc->bInterfaceClass != LIBUSB_CLASS_VENDOR_SPEC ||
-                                                       intf_desc->bInterfaceSubClass != 0 || intf_desc->bInterfaceProtocol != 0)
+                               /* We can rely on the interface is really CMSIS-DAP if
+                                * - we've seen CMSIS-DAP in the interface string
+                                * - config asked explicitly for an interface number
+                                * - the device has only one interface
+                                * The later two cases should be honored only if we know
+                                * we are on the rigt device */
+                               bool intf_identified_reliably = cmsis_dap_in_interface_str
+                                                       || (device_identified_reliably &&
+                                                                       (cmsis_dap_usb_interface != -1
+                                                                        || config_desc->bNumInterfaces == 1));
+
+                               if (intf_desc->bInterfaceClass != LIBUSB_CLASS_VENDOR_SPEC ||
+                                               intf_desc->bInterfaceSubClass != 0 || intf_desc->bInterfaceProtocol != 0) {
+                                       /* If the interface is reliably identified
+                                        * then we need not insist on setting USB class, subclass and protocol
+                                        * exactly as the specification requires.
+                                        * At least KitProg3 uses class 0 contrary to the specification */
+                                       if (intf_identified_reliably) {
+                                               LOG_WARNING("Using CMSIS-DAPv2 interface %d with wrong class %" PRId8
+                                                                 " subclass %" PRId8 " or protocol %" PRId8,
+                                                                 interface_num,
+                                                                 intf_desc->bInterfaceClass,
+                                                                 intf_desc->bInterfaceSubClass,
+                                                                 intf_desc->bInterfaceProtocol);
+                                       } else {
+                                               LOG_DEBUG("skipping interface %d, class %" PRId8
+                                                                 " subclass %" PRId8 " protocol %" PRId8,
+                                                                 interface_num,
+                                                                 intf_desc->bInterfaceClass,
+                                                                 intf_desc->bInterfaceSubClass,
+                                                                 intf_desc->bInterfaceProtocol);
                                                continue;
 
-                                       /* Search for "CMSIS-DAP" in the interface string */
-                                       if (cmsis_dap_usb_interface != -1 && !cmsis_dap_found) {
-                                               if (intf_desc->iInterface == 0)
-                                                       continue;
-
-                                               char interface_str[256] = {0};
-
-                                               err = libusb_get_string_descriptor_ascii(
-                                                               dev_handle, intf_desc->iInterface,
-                                                               (uint8_t *)interface_str, sizeof(interface_str));
-                                               if (err < 0) {
-                                                       LOG_ERROR("could not read interface string for device 0x%04x:0x%04x: %s",
-                                                                       dev_desc.idVendor, dev_desc.idProduct, libusb_strerror(err));
-                                                       continue;
-                                               } else if (!strstr(interface_str, "CMSIS-DAP")) {
-                                                       continue;
-                                               } else {
-                                                       LOG_DEBUG("CMSIS-DAP found in interface string");
-                                               }
                                        }
                                }
 
-                               int packet_size = intf_desc->endpoint[0].wMaxPacketSize;
-                               int ep_out = intf_desc->endpoint[0].bEndpointAddress;
-                               int ep_in = intf_desc->endpoint[1].bEndpointAddress;
+                               if (intf_identified_reliably) {
+                                       /* That's the one! */
+                                       intf_desc_found = intf_desc;
+                                       break;
+                               }
+
+                               if (!intf_desc_candidate && device_identified_reliably) {
+                                       /* This interface looks suitable for CMSIS-DAP. Store the pointer to it
+                                        * and keep searching for another one with CMSIS-DAP in interface string */
+                                       intf_desc_candidate = intf_desc;
+                               }
+                       }
+
+                       if (!intf_desc_found) {
+                               /* We were not able to identify reliably which interface is CMSIS-DAP.
+                                * Let's use the first suitable if we found one */
+                               intf_desc_found = intf_desc_candidate;
+                       }
 
-                               /* That's the one! */
+                       if (!intf_desc_found) {
                                libusb_free_config_descriptor(config_desc);
-                               libusb_free_device_list(device_list, true);
+                               continue;
+                       }
 
-                               LOG_INFO("Using CMSIS-DAPv2 interface with VID:PID=0x%04x:0x%04x, serial=%s",
-                                               dev_desc.idVendor, dev_desc.idProduct, dev_serial);
+                       /* We've chosen an interface, connect to it */
+                       int interface_num = intf_desc_found->bInterfaceNumber;
+                       int packet_size = intf_desc_found->endpoint[0].wMaxPacketSize;
+                       int ep_out = intf_desc_found->endpoint[0].bEndpointAddress;
+                       int ep_in = intf_desc_found->endpoint[1].bEndpointAddress;
 
-                               int current_config;
-                               err = libusb_get_configuration(dev_handle, &current_config);
-                               if (err) {
-                                       LOG_ERROR("could not find current configuration: %s", libusb_strerror(err));
-                                       libusb_close(dev_handle);
-                                       libusb_exit(ctx);
-                                       return ERROR_FAIL;
-                               }
+                       libusb_free_config_descriptor(config_desc);
+                       libusb_free_device_list(device_list, true);
 
-                               if (config_num != current_config) {
-                                       err = libusb_set_configuration(dev_handle, config_num);
-                                       if (err) {
-                                               LOG_ERROR("could not set configuration: %s", libusb_strerror(err));
-                                               libusb_close(dev_handle);
-                                               libusb_exit(ctx);
-                                               return ERROR_FAIL;
-                                       }
-                               }
+                       LOG_INFO("Using CMSIS-DAPv2 interface with VID:PID=0x%04x:0x%04x, serial=%s",
+                                       dev_desc.idVendor, dev_desc.idProduct, dev_serial);
 
-                               err = libusb_claim_interface(dev_handle, interface_num);
-                               if (err)
-                                       LOG_WARNING("could not claim interface: %s", libusb_strerror(err));
+                       int current_config;
+                       err = libusb_get_configuration(dev_handle, &current_config);
+                       if (err) {
+                               LOG_ERROR("could not find current configuration: %s", libusb_strerror(err));
+                               libusb_close(dev_handle);
+                               libusb_exit(ctx);
+                               return ERROR_FAIL;
+                       }
 
-                               dap->bdata = malloc(sizeof(struct cmsis_dap_backend_data));
-                               if (dap->bdata == NULL) {
-                                       LOG_ERROR("unable to allocate memory");
-                                       libusb_release_interface(dev_handle, interface_num);
+                       if (config_num != current_config) {
+                               err = libusb_set_configuration(dev_handle, config_num);
+                               if (err) {
+                                       LOG_ERROR("could not set configuration: %s", libusb_strerror(err));
                                        libusb_close(dev_handle);
                                        libusb_exit(ctx);
                                        return ERROR_FAIL;
                                }
+                       }
 
-                               dap->packet_size = packet_size + 1; /* "+ 1" for compatibility with the HID backend */
-                               dap->bdata->usb_ctx = ctx;
-                               dap->bdata->dev_handle = dev_handle;
-                               dap->bdata->ep_out = ep_out;
-                               dap->bdata->ep_in = ep_in;
-                               dap->bdata->interface = interface_num;
-                               return ERROR_OK;
+                       err = libusb_claim_interface(dev_handle, interface_num);
+                       if (err)
+                               LOG_WARNING("could not claim interface: %s", libusb_strerror(err));
+
+                       dap->bdata = malloc(sizeof(struct cmsis_dap_backend_data));
+                       if (dap->bdata == NULL) {
+                               LOG_ERROR("unable to allocate memory");
+                               libusb_release_interface(dev_handle, interface_num);
+                               libusb_close(dev_handle);
+                               libusb_exit(ctx);
+                               return ERROR_FAIL;
                        }
 
-                       libusb_free_config_descriptor(config_desc);
+                       dap->packet_size = packet_size + 1; /* "+ 1" for compatibility with the HID backend */
+                       dap->bdata->usb_ctx = ctx;
+                       dap->bdata->dev_handle = dev_handle;
+                       dap->bdata->ep_out = ep_out;
+                       dap->bdata->ep_in = ep_in;
+                       dap->bdata->interface = interface_num;
+                       return ERROR_OK;
                }
 
                libusb_close(dev_handle);