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/test/usb_host/msc_client_async_seq.c b/components/usb/test/usb_host/msc_client_async_seq.c index d4be014b1c..16ce7bd3c4 100644 --- a/components/usb/test/usb_host/msc_client_async_seq.c +++ b/components/usb/test/usb_host/msc_client_async_seq.c @@ -192,6 +192,8 @@ void msc_client_async_seq_task(void *arg) xfer_out->num_bytes = sizeof(usb_setup_packet_t); xfer_out->bEndpointAddress = 0; TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit_control(msc_obj.client_hdl, xfer_out)); + //Test that an inflight control transfer cannot be resubmitted + TEST_ASSERT_EQUAL(ESP_ERR_NOT_FINISHED, usb_host_transfer_submit_control(msc_obj.client_hdl, xfer_out)); //Next stage set from transfer callback break; } @@ -201,6 +203,8 @@ void msc_client_async_seq_task(void *arg) xfer_out->num_bytes = sizeof(mock_msc_bulk_cbw_t); xfer_out->bEndpointAddress = MOCK_MSC_SCSI_BULK_OUT_EP_ADDR; TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit(xfer_out)); + //Test that an inflight transfer cannot be resubmitted + TEST_ASSERT_EQUAL(ESP_ERR_NOT_FINISHED, usb_host_transfer_submit(xfer_out)); //Next stage set from transfer callback break; } @@ -209,6 +213,8 @@ void msc_client_async_seq_task(void *arg) xfer_in->num_bytes = usb_round_up_to_mps(MOCK_MSC_SCSI_SECTOR_SIZE * msc_obj.test_param.num_sectors_per_xfer, MOCK_MSC_SCSI_BULK_EP_MPS); xfer_in->bEndpointAddress = MOCK_MSC_SCSI_BULK_IN_EP_ADDR; TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit(xfer_in)); + //Test that an inflight transfer cannot be resubmitted + TEST_ASSERT_EQUAL(ESP_ERR_NOT_FINISHED, usb_host_transfer_submit(xfer_in)); //Next stage set from transfer callback break; } @@ -217,6 +223,8 @@ void msc_client_async_seq_task(void *arg) xfer_in->num_bytes = usb_round_up_to_mps(sizeof(mock_msc_bulk_csw_t), MOCK_MSC_SCSI_BULK_EP_MPS); xfer_in->bEndpointAddress = MOCK_MSC_SCSI_BULK_IN_EP_ADDR; TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit(xfer_in)); + //Test that an inflight transfer cannot be resubmitted + TEST_ASSERT_EQUAL(ESP_ERR_NOT_FINISHED, usb_host_transfer_submit(xfer_in)); //Next stage set from transfer callback break; } diff --git a/components/usb/usb_host.c b/components/usb/usb_host.c index 2c8531e7c2..ebbb00b33c 100644 --- a/components/usb/usb_host.c +++ b/components/usb/usb_host.c @@ -438,8 +438,8 @@ assign_err: hub_err: ESP_ERROR_CHECK(usbh_uninstall()); usbh_err: - if (p_host_lib_obj->constant.phy_handle) { - ESP_ERROR_CHECK(usb_del_phy(p_host_lib_obj->constant.phy_handle)); + if (host_lib_obj->constant.phy_handle) { + ESP_ERROR_CHECK(usb_del_phy(host_lib_obj->constant.phy_handle)); } phy_err: alloc_err: @@ -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; } diff --git a/components/usb/usbh.c b/components/usb/usbh.c index 6426e16c8f..aad2f54def 100644 --- a/components/usb/usbh.c +++ b/components/usb/usbh.c @@ -536,7 +536,7 @@ esp_err_t usbh_dev_open(uint8_t dev_addr, usb_device_handle_t *dev_hdl) goto exit; } } - TAILQ_FOREACH(dev_obj, &p_usbh_obj->dynamic.devs_idle_tailq, dynamic.tailq_entry) { + TAILQ_FOREACH(dev_obj, &p_usbh_obj->dynamic.devs_pending_tailq, dynamic.tailq_entry) { if (dev_obj->constant.address == dev_addr) { found_dev_obj = dev_obj; goto exit; @@ -783,7 +783,7 @@ esp_err_t usbh_ep_alloc(usb_device_handle_t dev_hdl, usbh_ep_config_t *ep_config if (is_in && dev_obj->mux_protected.ep_in[addr - 1] == NULL) { //Is an IN EP dev_obj->mux_protected.ep_in[addr - 1] = pipe_hdl; assigned = true; - } else if (dev_obj->mux_protected.ep_out[addr - 1] == NULL) { //Is an OUT EP + } else if (!is_in && dev_obj->mux_protected.ep_out[addr - 1] == NULL) { //Is an OUT EP dev_obj->mux_protected.ep_out[addr - 1] = pipe_hdl; assigned = true; }