Browse Source

usb_host: Add check to prevent submitting already inflight transfers

This commit adds a simple flag/check in the USB Host Library that prevents
users from submitting a transfer that is already inflight.

- A transfer is considered inflight as soon as it is submitted by calling
usb_host_transfer_submit() or usb_host_transfer_submit_control()
- An inflight transfer remains inflight up until right before its callback
is called by one of the USB Host Library handler functions.

Closes https://github.com/espressif/esp-idf/issues/8748
Darian Leung 3 years ago
parent
commit
e17e066828
2 changed files with 24 additions and 4 deletions
  1. 3 2
      components/usb/private_include/usb_private.h
  2. 21 2
      components/usb/usb_host.c

+ 3 - 2
components/usb/private_include/usb_private.h

@@ -38,12 +38,13 @@ _Static_assert(sizeof(usb_transfer_dummy_t) == sizeof(usb_transfer_t), "usb_tran
 
 struct urb_s{
     TAILQ_ENTRY(urb_s) tailq_entry;
-    //HCD handler pointer and variables. Must be initialized to NULL and 0 respectively
+    //HCD Layer: Handler pointer and variables. Must be initialized to NULL and 0 respectively
     void *hcd_ptr;
     uint32_t hcd_var;
-    //Host Driver layer handler
+    //Host Lib Layer:
     void *usb_host_client;  //Currently only used when submitted to shared pipes (i.e., Device default pipes)
     size_t usb_host_header_size; //USB Host may need the data buffer to have a transparent header
+    bool usb_host_inflight; //Debugging variable, used to prevent re-submitting URBs already inflight
     //Public transfer structure. Must be last due to variable length array
     usb_transfer_t transfer;
 };

+ 21 - 2
components/usb/usb_host.c

@@ -591,6 +591,8 @@ static void _handle_pending_ep(client_t *client_obj)
                 //Dequeue all URBs and run their transfer callback
                 urb_t *urb = hcd_urb_dequeue(ep_obj->constant.pipe_hdl);
                 while (urb != NULL) {
+                    //Clear the transfer's inflight flag to indicate the transfer is no longer inflight
+                    urb->usb_host_inflight = false;
                     urb->transfer.callback(&urb->transfer);
                     num_urb_dequeued++;
                     urb = hcd_urb_dequeue(ep_obj->constant.pipe_hdl);
@@ -748,6 +750,8 @@ esp_err_t usb_host_client_handle_events(usb_host_client_handle_t client_hdl, Tic
             TAILQ_REMOVE(&client_obj->dynamic.done_ctrl_xfer_tailq, urb, tailq_entry);
             client_obj->dynamic.num_done_ctrl_xfer--;
             HOST_EXIT_CRITICAL();
+            //Clear the transfer's inflight flag to indicate the transfer is no longer inflight
+            urb->usb_host_inflight = false;
             //Call the transfer's callback
             urb->transfer.callback(&urb->transfer);
             HOST_ENTER_CRITICAL();
@@ -1298,6 +1302,9 @@ esp_err_t usb_host_transfer_submit(usb_transfer_t *transfer)
                               USB_EP_DESC_GET_XFERTYPE(ep_obj->constant.ep_desc),
                               USB_EP_DESC_GET_MPS(ep_obj->constant.ep_desc),
                               transfer->bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_DIR_MASK), ESP_ERR_INVALID_ARG);
+    //Check that we are not submitting a transfer already inflight
+    HOST_CHECK(!urb_obj->usb_host_inflight, ESP_ERR_NOT_FINISHED);
+    urb_obj->usb_host_inflight = true;
     HOST_ENTER_CRITICAL();
     ep_obj->dynamic.num_urb_inflight++;
     HOST_EXIT_CRITICAL();
@@ -1317,6 +1324,7 @@ hcd_err:
     HOST_ENTER_CRITICAL();
     ep_obj->dynamic.num_urb_inflight--;
     HOST_EXIT_CRITICAL();
+    urb_obj->usb_host_inflight = false;
 err:
     return ret;
 }
@@ -1324,6 +1332,7 @@ err:
 esp_err_t usb_host_transfer_submit_control(usb_host_client_handle_t client_hdl, usb_transfer_t *transfer)
 {
     HOST_CHECK(client_hdl != NULL && transfer != NULL, ESP_ERR_INVALID_ARG);
+
     //Check that control transfer is valid
     HOST_CHECK(transfer->device_handle != NULL, ESP_ERR_INVALID_ARG);   //Target device must be set
     usb_device_handle_t dev_hdl = transfer->device_handle;
@@ -1333,8 +1342,18 @@ esp_err_t usb_host_transfer_submit_control(usb_host_client_handle_t client_hdl,
     HOST_CHECK(transfer_check(transfer, USB_TRANSFER_TYPE_CTRL, dev_info.bMaxPacketSize0, xfer_is_in), ESP_ERR_INVALID_ARG);
     //Control transfers must be targeted at EP 0
     HOST_CHECK((transfer->bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_NUM_MASK) == 0, ESP_ERR_INVALID_ARG);
-    //Save client handle into URB
+
     urb_t *urb_obj = __containerof(transfer, urb_t, transfer);
+    //Check that we are not submitting a transfer already inflight
+    HOST_CHECK(!urb_obj->usb_host_inflight, ESP_ERR_NOT_FINISHED);
+    urb_obj->usb_host_inflight = true;
+    //Save client handle into URB
     urb_obj->usb_host_client = (void *)client_hdl;
-    return usbh_dev_submit_ctrl_urb(dev_hdl, urb_obj);
+
+    esp_err_t ret;
+    ret = usbh_dev_submit_ctrl_urb(dev_hdl, urb_obj);
+    if (ret != ESP_OK) {
+        urb_obj->usb_host_inflight = false;
+    }
+    return ret;
 }