From e17e0668285f5bdea2a9211e3f456981fa9eb075 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Thu, 20 Oct 2022 00:04:53 +0800 Subject: [PATCH] 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 --- components/usb/private_include/usb_private.h | 5 +++-- components/usb/usb_host.c | 23 ++++++++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/components/usb/private_include/usb_private.h b/components/usb/private_include/usb_private.h index b245970b54..aaa34a1667 100644 --- a/components/usb/private_include/usb_private.h +++ b/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; }; diff --git a/components/usb/usb_host.c b/components/usb/usb_host.c index 2c8531e7c2..ad09f189c1 100644 --- a/components/usb/usb_host.c +++ b/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; }