enhance msc device driver

tinyusb/class/msc/msc_device.c

@@ -159,9 +159,9 @@ tusb_error_t mscd_control_request_st(uint8_t rhport, tusb_control_request_t cons
 
 tusb_error_t mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, tusb_event_t event, uint32_t xferred_bytes)
 {
-  mscd_interface_t* const p_msc = &_mscd_itf;
-  msc_cbw_t*        const p_cbw = &p_msc->cbw;
-  msc_csw_t*        const p_csw = &p_msc->csw;
+  mscd_interface_t* p_msc = &_mscd_itf;
+  msc_cbw_t const * p_cbw = &p_msc->cbw;
+  msc_csw_t       * p_csw = &p_msc->csw;
 
   VERIFY( (ep_addr == p_msc->ep_out) || (ep_addr == p_msc->ep_in), TUSB_ERROR_INVALID_PARA);
 
@@ -198,8 +198,11 @@ tusb_error_t mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, tusb_event_t event, u
 
         if ( p_cbw->xfer_bytes == 0)
         {
-          p_csw->status = tud_msc_scsi_cb(rhport, p_cbw->lun, p_cbw->command, NULL, &p_msc->data_len) ? MSC_CSW_STATUS_PASSED : MSC_CSW_STATUS_FAILED;
-          p_msc->stage  = MSC_STAGE_STATUS;
+          p_msc->data_len = tud_msc_scsi_cb(rhport, p_cbw->lun, p_cbw->command, NULL, 0);
+          p_csw->status   = (p_msc->data_len == 0) ? MSC_CSW_STATUS_PASSED : MSC_CSW_STATUS_FAILED;
+          p_msc->stage    = MSC_STAGE_STATUS;
+
+          TU_ASSERT( p_msc->data_len == 0, TUSB_ERROR_INVALID_PARA);
         }
         else if ( !BIT_TEST_(p_cbw->dir, 7) )
         {
@@ -208,7 +211,8 @@ tusb_error_t mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, tusb_event_t event, u
         }
         else
         {
-          p_csw->status = tud_msc_scsi_cb(rhport, p_cbw->lun, p_cbw->command, _mscd_buf, &p_msc->data_len) ? MSC_CSW_STATUS_PASSED : MSC_CSW_STATUS_FAILED;
+          p_msc->data_len = tud_msc_scsi_cb(rhport, p_cbw->lun, p_cbw->command, _mscd_buf, &p_msc->data_len);
+          p_csw->status   = (p_msc->data_len >= 0) ? MSC_CSW_STATUS_PASSED : MSC_CSW_STATUS_FAILED;
 
           TU_ASSERT( p_cbw->xfer_bytes >= p_msc->data_len, TUSB_ERROR_INVALID_PARA ); // cannot return more than host expect
 
@@ -244,7 +248,7 @@ tusb_error_t mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, tusb_event_t event, u
         }
         else
         {
-          p_csw->status = tud_msc_scsi_cb(rhport, p_cbw->lun, p_cbw->command, _mscd_buf, &p_msc->data_len) ? MSC_CSW_STATUS_PASSED : MSC_CSW_STATUS_FAILED;
+          p_csw->status = (tud_msc_scsi_cb(rhport, p_cbw->lun, p_cbw->command, _mscd_buf, p_msc->data_len) >= 0 ) ? MSC_CSW_STATUS_PASSED : MSC_CSW_STATUS_FAILED;
         }
       }
 
@@ -302,8 +306,8 @@ tusb_error_t mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, tusb_event_t event, u
 
 static void proc_read10_write10(uint8_t rhport, mscd_interface_t* p_msc)
 {
-  msc_cbw_t* const p_cbw = &p_msc->cbw;
-  msc_csw_t* const p_csw = &p_msc->csw;
+  msc_cbw_t const * p_cbw = &p_msc->cbw;
+  msc_csw_t       * p_csw = &p_msc->csw;
 
   // read10 & write10 has the same format
   scsi_read10_t* p_readwrite = (scsi_read10_t*) &p_cbw->command;
@@ -320,22 +324,28 @@ static void proc_read10_write10(uint8_t rhport, mscd_interface_t* p_msc)
   memcpy(&block_count, &p_readwrite->block_count, 2);
   block_count = __be2n_16(block_count);
 
-  uint32_t xfer_bytes = min32_of(sizeof(_mscd_buf), p_cbw->xfer_bytes-p_msc->xferred_len);
+  int32_t xfer_bytes = (int32_t) min32_of(sizeof(_mscd_buf), p_cbw->xfer_bytes-p_msc->xferred_len);
 
   // Write10 callback will be called later when usb transfer complete
   if (SCSI_CMD_READ_10 == p_cbw->command[0])
   {
-    xfer_bytes = tud_msc_read10_cb (rhport, p_cbw->lun, lba, p_msc->xferred_len, _mscd_buf, xfer_bytes);
+    int32_t rd_cnt = tud_msc_read10_cb (rhport, p_cbw->lun, lba, p_msc->xferred_len, _mscd_buf, (uint32_t) xfer_bytes);
   }
 
-  if ( 0 == xfer_bytes  )
+  if ( xfer_bytes < 0 )
   {
-    // xferred_block is zero will cause pipe is stalled & status in CSW set to failed
-    p_csw->data_residue = p_cbw->xfer_bytes;
+    // negative is error -> pipe is stalled & status in CSW set to failed
+    p_csw->data_residue = p_cbw->xfer_bytes - p_msc->xferred_len;
     p_csw->status       = MSC_CSW_STATUS_FAILED;
 
     dcd_edpt_stall(rhport, ep_data);
-  }else
+  }
+  else if ( xfer_bytes == 0 )
+  {
+    // zero is not ready -> try again later by simulate an transfer complete
+    dcd_xfer_complete(rhport, ep_data, 0, true);
+  }
+  else
   {
     TU_ASSERT( dcd_edpt_xfer(rhport, ep_data, _mscd_buf, xfer_bytes), );
   }

tinyusb/class/msc/msc_device.h

@@ -65,11 +65,18 @@
  * \param[in]		lun         Targeted Logical Unit
  *                          Must be accessible by USB controller (see \ref CFG_TUSB_ATTR_USBRAM)
  * \param[in]		lba         Starting Logical Block Address to be read
- * \param[in]		block_count Number of requested block
- * \param[out]	pp_buffer   Pointer to buffer which application need to update with the response data's address.
+ * \param[in]		offset      Byte offset from LBA for reading
+ * \param[out]	buffer      Buffer which application need to update with the response data.
+ * \param[in]   bufsize     Max bytes can be copied. Application must not return more than this value.
  *
- * \retval      non-zero    Actual number of block that application processed, must be less than or equal to \a \b block_count.
- * \retval      zero        Indicate error in retrieving data from application. Tinyusb device stack will \b STALL the corresponding
+ * \retval      positive    Actual bytes returned, must not be more than \a \b bufsize.
+ *                          If value less than bufsize is returned, Tinyusb will transfer this amount and afterwards
+ *                          invoke this callback again with adjusted offset.
+ *
+ * \retval      zero        Indicate application is not ready yet to response e.g disk I/O is not complete.
+ *                          Tinyusb will invoke this callback with the same params again some time later.
+ *
+ * \retval      negative    Indicate error reading disk I/O. Tinyusb will \b STALL the corresponding
  *                          endpoint and return failed status in command status wrapper phase.
  *
  * \note        Host can request dozens of Kbytes in one command e.g \a \b block_count = 128, it is insufficient to require
@@ -78,18 +85,24 @@
  *              \n\n Although this callback is called by tinyusb device task (non-isr context), however as all the classes share
  *              the same task (to save resource), any delay in this callback will cause delay in response on other classes.
  */
-uint32_t tud_msc_read10_cb (uint8_t rhport, uint8_t lun, uint32_t lba, uint32_t offset, void* buffer, uint32_t bufsize);
+int32_t tud_msc_read10_cb (uint8_t rhport, uint8_t lun, uint32_t lba, uint32_t offset, void* buffer, uint32_t bufsize);
 
 /** \brief 			Callback invoked when received \ref SCSI_CMD_WRITE_10 command
  * \param[in]		rhport	    Root hub port
  * \param[in]		lun         Targeted Logical Unit
  *                          Must be accessible by USB controller (see \ref CFG_TUSB_ATTR_USBRAM)
  * \param[in]		lba         Starting Logical Block Address to be write
- * \param[in]		block_count Number of requested block
- * \param[out]	pp_buffer   Pointer to buffer which application need to update with the address to hold data from host
+ * \param[out]	buffer      Buffer which holds written data.
+ * \param[in]   bufsize     Max bytes in buffer. Application must not return more than this value.
+ *
+ * \retval      positive    Actual bytes written, must not be more than \a \b bufsize.
+ *                          If value less than bufsize is returned, Tinyusb will trim off this amount and
+ *                          invoke this callback again with adjusted offset some time later.
+ *
+ * \retval      zero        Indicate application is not ready yet e.g disk I/O is not complete.
+ *                          Tinyusb will invoke this callback with the same params again some time later.
  *
- * \retval      non-zero    Actual number of block that application can receive and must be less than or equal to \a \b block_count.
- * \retval      zero        Indicate error in retrieving data from application. Tinyusb device stack will \b STALL the corresponding
+ * \retval      negative    Indicate error writing disk I/O. Tinyusb will \b STALL the corresponding
  *                          endpoint and return failed status in command status wrapper phase.
  *
  * \note        Host can request dozens of Kbytes in one command e.g \a \b block_count = 128, it is insufficient to require
@@ -98,21 +111,23 @@ uint32_t tud_msc_read10_cb (uint8_t rhport, uint8_t lun, uint32_t lba, uint32_t
  *              \n\n Although this callback is called by tinyusb device task (non-isr context), however as all the classes share
  *              the same task (to save resource), any delay in this callback will cause delay in response on other classes.
  */
-uint32_t tud_msc_write10_cb (uint8_t rhport, uint8_t lun, uint32_t lba, uint32_t offset, void* buffer, uint32_t bufsize);
+int32_t tud_msc_write10_cb (uint8_t rhport, uint8_t lun, uint32_t lba, uint32_t offset, void* buffer, uint32_t bufsize);
 
 
-/** \brief 			  Callback invoked when received an SCSI command other than \ref SCSI_CMD_WRITE_10 and \ref SCSI_CMD_READ_10
- * \param[in]		  rhport	    Root hub port
- * \param[in]		  lun         Targeted Logical Unit
- * \param[in]		  scsi_cmd    SCSI command contents, application should examine this command block to know which command host requested
- * \param[out]    buffer      Pointer to buffer which application need to update with the address to transfer data with host.
- *                            The buffer address can be anywhere since the stack will copy its contents to a internal USB-accessible buffer.
- * \param[in,out] p_len       Expected length from host, Application could update to actual data, but could not larger than original value.
+/** \brief 			Callback invoked when received an SCSI command other than \ref SCSI_CMD_WRITE_10 and \ref SCSI_CMD_READ_10
+ * \param[in]		rhport	    Root hub port
+ * \param[in]		lun         Targeted Logical Unit
+ * \param[in]		scsi_cmd    SCSI command contents, application should examine this command block to know which command host requested
+ * \param[out]  buffer      Buffer for SCSI Data Stage.
+ *                            - For INPUT: application must fill this with response.
+ *                            - For OUTPUT it holds the Data from host
+ * \param[in]   bufsize     Buffer's length.
  *
- * \retval        true if successful, false otherwise. Tinyusb will then \b STALL the corresponding endpoint and return
- *                failed status in command status wrapper stage.
+ * \retval      negative    Indicate error e.g accessing disk I/O. Tinyusb will \b STALL the corresponding
+ *                          endpoint and return failed status in command status wrapper phase.
+ * \retval      otherwise   Actual bytes processed, must not be more than \a \b bufsize. Can be zero for no-data command.
  */
-bool tud_msc_scsi_cb (uint8_t rhport, uint8_t lun, uint8_t scsi_cmd[16], void* buffer, uint16_t* p_len);
+int32_t tud_msc_scsi_cb (uint8_t rhport, uint8_t lun, uint8_t scsi_cmd[16], void* buffer, uint16_t bufsize);
 
 /*------------- Optional callbacks : Could be used by application to free up resources -------------*/
 ATTR_WEAK void tud_msc_read10_complete_cb(uint8_t rhport, uint8_t lun);

tinyusb/osal/osal_freeRTOS.h

@@ -100,6 +100,8 @@ static inline void osal_queue_receive (osal_queue_t const queue_hdl, void *p_dat
 
 static inline bool osal_queue_send(osal_queue_t const queue_hdl, void const * data)
 {
+  // queue send used by msc device for retrying read10/write10
+  verify_breakpoint();
   return xQueueSendToFrontFromISR(queue_hdl, data, NULL);
 }