hathach / tinyusb

An open source cross-platform USB stack for embedded system
https://www.tinyusb.org
MIT License
4.66k stars 997 forks source link

tuh_edpt_abort_xfer should call user callback #2682

Open Ryzee119 opened 1 week ago

Ryzee119 commented 1 week ago

Related area

Usb Host raw api CFG_TUH_API_EDPT_XFER

Hardware specification

OHCI

Is your feature request related to a problem?

Im using the raw api with an RTOS and CFG_TUH_API_EDPT_XFER enabled and this is probably the simplest example that illustrates the issue

const uint8_t my_dev_addr = 0x01;

void my_transfer_cb (tuh_xfer_t *transfer)
{
    osal_semaphore_t *semaphore = (osal_semaphore_t *)transfer->user_data;
    osal_semaphore_post(*semaphore, false);
}

void readpipe(osal_semaphore_t semaphore) {

  tuh_xfer_t transfer = {
          .daddr = my_dev_addr,
          .ep_addr = 0x81,
          .buffer = Buffer,
          .buflen = BufferLength,
          .complete_cb = my_transfer_cb,
          .user_data = (uintptr_t)&semaphore,
      };

  tuh_edpt_xfer(&transfer);
  osal_semaphore_wait(semaphore, OSAL_TIMEOUT_WAIT_FOREVER);

  // If device is unplugged here it will hang forever, as my_transfer_cb() is probably never called (race if transfer fails before device is cleared)
}

void tuh_umount_cb (uint8_t dev_addr) {
  if (dev_addr == my_dev_addr) {
    // Make sure pending transfers are stopped
    tuh_edpt_abort_xfer(dev_addr, 0x81); // Would be good if this would call the endpoint callback
  }
}

Describe the solution you'd like

tuh_edpt_abort_xfer() should probably call the registered callback with a XFER_RESULT_ABORTED status or similar?

This is an issue when using the raw API mainly. Proposed patch below:

If this looks okay, im happy to test it a bit more and raise a PR

typedef enum {
   XFER_RESULT_SUCCESS = 0,
   XFER_RESULT_FAILED,
   XFER_RESULT_STALLED,
   XFER_RESULT_TIMEOUT,
   XFER_RESULT_INVALID,
+  XFER_RESULT_ABORTED
} xfer_result_t;

bool tuh_edpt_abort_xfer(uint8_t daddr, uint8_t ep_addr) {
  usbh_device_t* dev = get_device(daddr);
  TU_VERIFY(dev);

  TU_LOG_USBH("[%u] Aborted transfer on EP %02X\r\n", daddr, ep_addr);

  uint8_t const epnum = tu_edpt_number(ep_addr);
  uint8_t const dir   = tu_edpt_dir(ep_addr);

  if ( epnum == 0 ) {
    // control transfer: only 1 control at a time, check if we are aborting the current one
    TU_VERIFY(daddr == _ctrl_xfer.daddr && _ctrl_xfer.stage != CONTROL_STAGE_IDLE);
    TU_VERIFY(hcd_edpt_abort_xfer(dev->rhport, daddr, ep_addr));
    // reset control transfer state to idle
-    _set_control_xfer_stage(CONTROL_STAGE_IDLE);
+    _control_xfer_complete(daddr, XFER_RESULT_ABORTED);
  } else {
    // non-control skip if not busy
    TU_VERIFY(dev->ep_status[epnum][dir].busy);
    TU_VERIFY(hcd_edpt_abort_xfer(dev->rhport, daddr, ep_addr));
    // mark as ready and release endpoint if transfer is aborted
    dev->ep_status[epnum][dir].busy = false;
    tu_edpt_release(&dev->ep_status[epnum][dir], _usbh_mutex);
+
+    #if CFG_TUH_API_EDPT_XFER
+    tuh_xfer_cb_t const complete_cb = dev->ep_callback[epnum][dir].complete_cb;
+    uintptr_t const user_data = dev->ep_callback[epnum][dir].user_data;
+    if ( complete_cb ) {
+      tuh_xfer_t xfer = {
+          .daddr       = daddr,
+          .ep_addr     = ep_addr,
+          .result      = XFER_RESULT_ABORTED,
+          .actual_len  = 0,
+          .buflen      = 0,
+          .buffer      = NULL,
+          .complete_cb = complete_cb,
+          .user_data   = user_data,
+      };
+      complete_cb(&xfer);
+    }
+    #endif
  }

I have checked existing issues, dicussion and documentation