Skip to content

Commit

Permalink
Merge branch 'feature/usb_host_inflight_transfer_check' into 'master'
Browse files Browse the repository at this point in the history 
USB Host: Fix usbh_dev_open(), Fix usbh_ep_alloc(), Add USB Host Library check to prevent resubmitting inflight transfers

Closes IDFGH-7145

See merge request espressif/esp-idf!20688
  • Loading branch information
@Dazza0
Dazza0 committed Nov 7, 2022
2 parents ed36caa + 4e84212 commit 598dd5f
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 6 deletions.
5 changes: 3 additions & 2 deletions components/usb/private_include/usb_private.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
8 changes: 8 additions & 0 deletions components/usb/test/usb_host/msc_client_async_seq.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
23 changes: 21 additions & 2 deletions components/usb/usb_host.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -1317,13 +1324,15 @@ esp_err_t usb_host_transfer_submit(usb_transfer_t *transfer)
HOST_ENTER_CRITICAL();
ep_obj->dynamic.num_urb_inflight--;
HOST_EXIT_CRITICAL();
urb_obj->usb_host_inflight = false;
err:
return ret;
}

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;
Expand All @@ -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;
}
4 changes: 2 additions & 2 deletions components/usb/usbh.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 598dd5f

Please sign in to comment.