refactor MSC device driver

tinyusb/class/msc/msc.h

@@ -102,7 +102,7 @@ typedef struct ATTR_PACKED
 {
   uint32_t signature   ; ///< Signature that helps identify this data packet as a CBW. The signature field shall contain the value 43425355h (little endian), indicating a CBW.
   uint32_t tag         ; ///< Tag sent by the host. The device shall echo the contents of this field back to the host in the dCSWTagfield of the associated CSW. The dCSWTagpositively associates a CSW with the corresponding CBW.
-  uint32_t xfer_bytes  ; ///< The number of bytes of data that the host expects to transfer on the Bulk-In or Bulk-Out endpoint (as indicated by the Directionbit) during the execution of this command. If this field is zero, the device and the host shall transfer no data between the CBW and the associated CSW, and the device shall ignore the value of the Directionbit in bmCBWFlags.
+  uint32_t xfer_bytes  ; ///< The number of bytes of data that the host expects to transfer on the Bulk-In or Bulk-Out endpoint (as indicated by the Direction bit) during the execution of this command. If this field is zero, the device and the host shall transfer no data between the CBW and the associated CSW, and the device shall ignore the value of the Direction bit in bmCBWFlags.
   uint8_t  dir         ; ///< Bit 7 of this field define transfer direction \n - 0 : Data-Out from host to the device. \n - 1 : Data-In from the device to the host.
   uint8_t  lun         ; ///< The device Logical Unit Number (LUN) to which the command block is being sent. For devices that support multiple LUNs, the host shall place into this field the LUN to which this command block is addressed. Otherwise, the host shall set this field to zero.
   uint8_t  cmd_len     ; ///< The valid length of the CBWCBin bytes. This defines the valid length of the command block. The only legal values are 1 through 16

tinyusb/class/msc/msc_device.c

@@ -51,6 +51,13 @@
 //--------------------------------------------------------------------+
 // MACRO CONSTANT TYPEDEF
 //--------------------------------------------------------------------+
+enum
+{
+  MSC_STAGE_CMD  = 0,
+  MSC_STAGE_DATA,
+  MSC_STAGE_STATUS
+};
+
 typedef struct {
   uint8_t scsi_data[64]; // buffer for scsi's response other than read10 & write10. NOTE should be multiple of 64 to be compatible with lpc11/13u
   ATTR_USB_MIN_ALIGNMENT msc_cbw_t  cbw;
@@ -63,8 +70,9 @@ typedef struct {
 
   uint8_t max_lun;
   uint8_t interface_num;
-
   uint8_t ep_in, ep_out;
+
+  uint8_t stage;
 }mscd_interface_t;
 
 TUSB_CFG_ATTR_USBRAM STATIC_VAR mscd_interface_t mscd_data;
@@ -153,75 +161,109 @@ tusb_error_t mscd_control_request_st(uint8_t port, tusb_control_request_t const
 //--------------------------------------------------------------------+
 // MSCD APPLICATION CALLBACK
 //--------------------------------------------------------------------+
-tusb_error_t mscd_xfer_cb(uint8_t port, uint8_t ep_addr, tusb_event_t event, uint32_t xferred_bytes)
+static bool send_status(uint8_t port, mscd_interface_t* p_msc)
 {
-  static bool is_waiting_read10_write10 = false; // indicate we are transferring data in READ10, WRITE10 command
+  TU_ASSERT( tusb_dcd_edpt_xfer(port, p_msc->ep_in , (uint8_t*) &p_msc->csw, sizeof(msc_csw_t), false) );
 
-  mscd_interface_t *         const p_msc = &mscd_data;
-  msc_cbw_t *  const p_cbw = &p_msc->cbw;
-  msc_csw_t * const p_csw = &p_msc->csw;
+  //------------- Queue the next CBW -------------//
+  TU_ASSERT( tusb_dcd_edpt_xfer(port, p_msc->ep_out, (uint8_t*) &p_msc->cbw, sizeof(msc_cbw_t), true) );
+
+  return true;
+}
+
+tusb_error_t mscd_xfer_cb(uint8_t port, uint8_t ep_addr, tusb_event_t event, uint32_t xferred_bytes)
+{
+  mscd_interface_t* const p_msc = &mscd_data;
+  msc_cbw_t*        const p_cbw = &p_msc->cbw;
+  msc_csw_t*        const p_csw = &p_msc->csw;
 
   VERIFY( (ep_addr == p_msc->ep_out) || (ep_addr == p_msc->ep_in), TUSB_ERROR_INVALID_PARA);
 
-  //------------- new CBW received -------------//
-  if ( !is_waiting_read10_write10 )
+  switch (p_msc->stage)
   {
-//    if ( ep_addr == p_msc->edpt_in  ) return TUSB_ERROR_NONE; // bulk in interrupt for dcd to clean up
-
-    TU_ASSERT( (ep_addr == p_msc->ep_out) &&
-               event == TUSB_EVENT_XFER_COMPLETE                  &&
-               xferred_bytes == sizeof(msc_cbw_t)   &&
-               p_cbw->signature == MSC_CBW_SIGNATURE, TUSB_ERROR_INVALID_PARA );
+    //------------- new CBW received -------------//
+    case MSC_STAGE_CMD:
+      TU_ASSERT( (ep_addr == p_msc->ep_out) &&
+                 event == TUSB_EVENT_XFER_COMPLETE                  &&
+                 xferred_bytes == sizeof(msc_cbw_t)   &&
+                 p_cbw->signature == MSC_CBW_SIGNATURE, TUSB_ERROR_INVALID_PARA );
+
+      p_csw->signature    = MSC_CSW_SIGNATURE;
+      p_csw->tag          = p_cbw->tag;
+      p_csw->data_residue = 0;
+
+      // Valid command -> move to Data Stage
+      p_msc->stage = MSC_STAGE_DATA;
+
+      // If not read10 & write10, invoke application callback
+      if ( (SCSI_CMD_READ_10 == p_cbw->command[0]) || (SCSI_CMD_WRITE_10 == p_cbw->command[0]) )
+      {
+        if ( read10_write10_data_xfer(port, p_msc) )
+        {
+          // read10 & write10 data is complete -> move to Status Stage
+          p_msc->stage = MSC_STAGE_STATUS;
+        }
+      }
+      else
+      {
+        void const *p_buffer = NULL;
+        uint16_t actual_length = (uint16_t) p_cbw->xfer_bytes;
 
-    p_csw->signature    = MSC_CSW_SIGNATURE;
-    p_csw->tag          = p_cbw->tag;
-    p_csw->data_residue = 0;
+        // TODO SCSI data out transfer is not yet supported
+        ASSERT_FALSE( p_cbw->xfer_bytes > 0 && !BIT_TEST_(p_cbw->dir, 7), TUSB_ERROR_NOT_SUPPORTED_YET);
 
-    if ( (SCSI_CMD_READ_10 != p_cbw->command[0]) && (SCSI_CMD_WRITE_10 != p_cbw->command[0]) )
-    {
-      void const *p_buffer = NULL;
-      uint16_t actual_length = (uint16_t) p_cbw->xfer_bytes;
+        p_csw->status = tud_msc_scsi_cb(port, p_cbw->lun, p_cbw->command, &p_buffer, &actual_length);
 
-      // TODO SCSI data out transfer is not yet supported
-      ASSERT_FALSE( p_cbw->xfer_bytes > 0 && !BIT_TEST_(p_cbw->dir, 7), TUSB_ERROR_NOT_SUPPORTED_YET);
+        //------------- Data Phase (non READ10, WRITE10) -------------//
+        if ( p_cbw->xfer_bytes )
+        {
+          ASSERT( p_cbw->xfer_bytes >= actual_length, TUSB_ERROR_INVALID_PARA );
+          ASSERT( sizeof(p_msc->scsi_data) >= actual_length, TUSB_ERROR_NOT_ENOUGH_MEMORY); // needs to increase size for scsi_data
+
+          uint8_t const edpt_data = BIT_TEST_(p_cbw->dir, 7) ? p_msc->ep_in : p_msc->ep_out;
+
+          if ( p_buffer == NULL || actual_length == 0 )
+          {
+            // application does not provide data to response --> possibly unsupported SCSI command
+            tusb_dcd_edpt_stall(port, edpt_data);
+            p_csw->status = MSC_CSW_STATUS_FAILED;
+          }else
+          {
+            memcpy(p_msc->scsi_data, p_buffer, actual_length);
+            TU_ASSERT( tusb_dcd_edpt_queue_xfer(port, edpt_data, p_msc->scsi_data, actual_length), TUSB_ERROR_DCD_EDPT_XFER );
+          }
+        }
 
-      p_csw->status = tud_msc_scsi_cb(port, p_cbw->lun, p_cbw->command, &p_buffer, &actual_length);
+        // consider other SCSI is complete after one DATA transfer
+        p_msc->stage = MSC_STAGE_STATUS;
+      }
+    break;
 
-      //------------- Data Phase (non READ10, WRITE10) -------------//
-      if ( p_cbw->xfer_bytes )
+    case MSC_STAGE_DATA:
+      // Can be executed several times e.g write 8K bytes (several flash write)
+      if ( (SCSI_CMD_READ_10 == p_cbw->command[0]) || (SCSI_CMD_WRITE_10 == p_cbw->command[0]) )
       {
-        ASSERT( p_cbw->xfer_bytes >= actual_length, TUSB_ERROR_INVALID_PARA );
-        ASSERT( sizeof(p_msc->scsi_data) >= actual_length, TUSB_ERROR_NOT_ENOUGH_MEMORY); // needs to increase size for scsi_data
-
-        uint8_t const edpt_data = BIT_TEST_(p_cbw->dir, 7) ? p_msc->ep_in : p_msc->ep_out;
-
-        if ( p_buffer == NULL || actual_length == 0 )
-        { // application does not provide data to response --> possibly unsupported SCSI command
-          tusb_dcd_edpt_stall(port, edpt_data);
-          p_csw->status = MSC_CSW_STATUS_FAILED;
-        }else
+        if ( read10_write10_data_xfer(port, p_msc) )
         {
-          memcpy(p_msc->scsi_data, p_buffer, actual_length);
-          TU_ASSERT( tusb_dcd_edpt_queue_xfer(port, edpt_data, p_msc->scsi_data, actual_length), TUSB_ERROR_DCD_EDPT_XFER );
+          // read10 & write10 data is complete -> move to Status Stage
+          p_msc->stage = MSC_STAGE_STATUS;
         }
       }
-    }
-  }
+    break;
 
-  //------------- Data Phase For READ10 & WRITE10 (can be executed several times) -------------//
-  if ( (SCSI_CMD_READ_10 == p_cbw->command[0]) || (SCSI_CMD_WRITE_10 == p_cbw->command[0]) )
-  {
-    is_waiting_read10_write10 = !read10_write10_data_xfer(port, p_msc);
+    case MSC_STAGE_STATUS: break;
+    default : break;
   }
 
-  //------------- Status Phase -------------//
-  // Either bulk in & out can be stalled in the data phase, dcd must make sure these queued transfer will be resumed after host clear stall
-  if (!is_waiting_read10_write10)
+  if ( p_msc->stage == MSC_STAGE_STATUS )
   {
-    TU_ASSERT( tusb_dcd_edpt_xfer(port, p_msc->ep_in , (uint8_t*) p_csw, sizeof(msc_csw_t), false), TUSB_ERROR_DCD_EDPT_XFER );
+    // Move to default CMD stage after sending status
+    p_msc->stage = MSC_STAGE_CMD;
+
+    TU_ASSERT( tusb_dcd_edpt_xfer(port, p_msc->ep_in , (uint8_t*) &p_msc->csw, sizeof(msc_csw_t), false) );
 
     //------------- Queue the next CBW -------------//
-    TU_ASSERT( tusb_dcd_edpt_xfer(port, p_msc->ep_out, (uint8_t*) p_cbw, sizeof(msc_cbw_t), true), TUSB_ERROR_DCD_EDPT_XFER );
+    TU_ASSERT( tusb_dcd_edpt_xfer(port, p_msc->ep_out, (uint8_t*) &p_msc->cbw, sizeof(msc_cbw_t), true) );
   }
 
   return TUSB_ERROR_NONE;
@@ -230,10 +272,11 @@ tusb_error_t mscd_xfer_cb(uint8_t port, uint8_t ep_addr, tusb_event_t event, uin
 // return true if data phase is complete, false if not yet complete
 static bool read10_write10_data_xfer(uint8_t port, mscd_interface_t* p_msc)
 {
-  msc_cbw_t*  const p_cbw = &p_msc->cbw;
+  msc_cbw_t* const p_cbw = &p_msc->cbw;
   msc_csw_t* const p_csw = &p_msc->csw;
 
-  scsi_read10_t* p_readwrite = (scsi_read10_t*) &p_cbw->command; // read10 & write10 has the same format
+  // read10 & write10 has the same format
+  scsi_read10_t* p_readwrite = (scsi_read10_t*) &p_cbw->command;
 
   uint8_t const ep_addr = BIT_TEST_(p_cbw->dir, 7) ? p_msc->ep_in : p_msc->ep_out;
 
@@ -241,34 +284,43 @@ static bool read10_write10_data_xfer(uint8_t port, mscd_interface_t* p_msc)
   uint16_t const block_count = __be2n_16(p_readwrite->block_count);
   void *p_buffer = NULL;
 
-  uint16_t xferred_block = (SCSI_CMD_READ_10 == p_cbw->command[0]) ? tud_msc_read10_cb (port, p_cbw->lun, &p_buffer, lba, block_count) :
-                                                                     tud_msc_write10_cb(port, p_cbw->lun, &p_buffer, lba, block_count);
-  xferred_block = min16_of(xferred_block, block_count);
+  uint16_t xfer_block;
+
+  if (SCSI_CMD_READ_10 == p_cbw->command[0])
+  {
+    xfer_block = tud_msc_read10_cb (port, p_cbw->lun, &p_buffer, lba, block_count);
+  }else
+  {
+    xfer_block = tud_msc_write10_cb(port, p_cbw->lun, &p_buffer, lba, block_count);
+  }
+
+  xfer_block = min16_of(xfer_block, block_count);
 
-  uint16_t const xferred_byte = xferred_block * (p_cbw->xfer_bytes / block_count);
+  uint16_t const xfer_byte = xfer_block * (p_cbw->xfer_bytes / block_count);
 
-  if ( 0 == xferred_block  )
-  { // xferred_block is zero will cause pipe is stalled & status in CSW set to failed
+  if ( 0 == xfer_block  )
+  {
+    // xferred_block is zero will cause pipe is stalled & status in CSW set to failed
     p_csw->data_residue = p_cbw->xfer_bytes;
     p_csw->status       = MSC_CSW_STATUS_FAILED;
 
     tusb_dcd_edpt_stall(port, ep_addr);
 
     return true;
-  } else if (xferred_block < block_count)
+  } else if (xfer_block < block_count)
   {
-    TU_ASSERT( tusb_dcd_edpt_xfer(port, ep_addr, p_buffer, xferred_byte, true), TUSB_ERROR_DCD_EDPT_XFER );
+    TU_ASSERT( tusb_dcd_edpt_xfer(port, ep_addr, p_buffer, xfer_byte, true), TUSB_ERROR_DCD_EDPT_XFER );
 
     // adjust lba, block_count, xfer_bytes for the next call
-    p_readwrite->lba         = __n2be(lba+xferred_block);
-    p_readwrite->block_count = __n2be_16(block_count - xferred_block);
-    p_cbw->xfer_bytes       -= xferred_byte;
+    p_readwrite->lba         = __n2be(lba+xfer_block);
+    p_readwrite->block_count = __n2be_16(block_count - xfer_block);
+    p_cbw->xfer_bytes       -= xfer_byte;
 
     return false;
   }else
   {
     p_csw->status = MSC_CSW_STATUS_PASSED;
-    TU_ASSERT( tusb_dcd_edpt_queue_xfer(port, ep_addr, p_buffer, xferred_byte), TUSB_ERROR_DCD_EDPT_XFER );
+    TU_ASSERT( tusb_dcd_edpt_queue_xfer(port, ep_addr, p_buffer, xfer_byte), TUSB_ERROR_DCD_EDPT_XFER );
     return true;
   }
 }

tinyusb/device/usbd.c

@@ -258,7 +258,7 @@ static tusb_error_t usbd_main_stk(void)
     STASK_INVOKE( proc_control_request_st(event.port, &event.setup_received), error );
   }else if (USBD_EVENTID_XFER_DONE == event.event_id)
   {
-    // Call class handling function, Class that endpoint not belong to should check and return
+    // Call class handling function. Those doest not own the endpoint should check and return
     for (uint8_t class_code = TUSB_CLASS_AUDIO; class_code < USBD_CLASS_DRIVER_COUNT; class_code++)
     {
       if ( usbd_class_drivers[class_code].xfer_cb )