improving msc device driver

examples/obsolete/device/src/msc_device_app.c

@@ -63,7 +63,7 @@ static scsi_read_capacity10_data_t const mscd_read_capacity10_data =
     .block_size = ENDIAN_BE(DISK_BLOCK_SIZE)
 };
 
-scsi_sense_fixed_data_t mscd_sense_data =
+static scsi_sense_fixed_data_t mscd_sense_data =
 {
     .response_code        = 0x70,
     .sense_key            = 0, // no errors
@@ -99,47 +99,62 @@ void msc_app_umount(uint8_t rhport)
 
 }
 
-msc_csw_status_t tud_msc_scsi_cb (uint8_t rhport, uint8_t lun, uint8_t scsi_cmd[16], void const ** pp_buffer, uint16_t* p_length)
+msc_csw_status_t tud_msc_scsi_cb (uint8_t rhport, uint8_t lun, uint8_t scsi_cmd[16], void* buffer, uint16_t* p_len)
 {
   // read10 & write10 has their own callback and MUST not be handled here
+
+  void* bufptr = NULL;
+  uint16_t buflen = 0;
+
   switch (scsi_cmd[0])
   {
     case SCSI_CMD_INQUIRY:
-      (*pp_buffer) = &mscd_inquiry_data;
-      (*p_length)  = sizeof(scsi_inquiry_data_t);
+      bufptr = &mscd_inquiry_data;
+      buflen = sizeof(scsi_inquiry_data_t);
     break;
 
     case SCSI_CMD_READ_CAPACITY_10:
-      (*pp_buffer) = &mscd_read_capacity10_data;
-      (*p_length)  = sizeof(scsi_read_capacity10_data_t);
+      bufptr = &mscd_read_capacity10_data;
+      buflen = sizeof(scsi_read_capacity10_data_t);
     break;
 
     case SCSI_CMD_REQUEST_SENSE:
-      (*pp_buffer) = &mscd_sense_data;
-      (*p_length)  = sizeof(scsi_sense_fixed_data_t);
+      bufptr = &mscd_sense_data;
+      buflen = sizeof(scsi_sense_fixed_data_t);
     break;
 
     case SCSI_CMD_READ_FORMAT_CAPACITY:
-      (*pp_buffer) = &mscd_format_capacity_data;
-      (*p_length)  = sizeof(scsi_read_format_capacity_data_t);
+      bufptr = &mscd_format_capacity_data;
+      buflen = sizeof(scsi_read_format_capacity_data_t);
     break;
 
     case SCSI_CMD_MODE_SENSE_6:
-      (*pp_buffer) = &msc_dev_mode_para;
-      (*p_length)  = sizeof(msc_dev_mode_para);
+      bufptr = &msc_dev_mode_para;
+      buflen = sizeof(msc_dev_mode_para);
     break;
 
     case SCSI_CMD_TEST_UNIT_READY:
-      (*pp_buffer) = NULL;
-      (*p_length) = 0;
+      bufptr = NULL;
+      buflen= 0;
     break;
 
     case SCSI_CMD_PREVENT_ALLOW_MEDIUM_REMOVAL:
-      (*pp_buffer) = NULL;
-      (*p_length) = 0;
+      bufptr = NULL;
+      buflen= 0;
     break;
 
-    default: return MSC_CSW_STATUS_FAILED;
+    default:
+      (*p_len) = 0;
+      return MSC_CSW_STATUS_FAILED;
+  }
+
+  if ( bufptr && buflen )
+  {
+    // Response len must not larger than expected from host
+    TU_ASSERT( (*p_len) >= buflen, MSC_CSW_STATUS_FAILED);
+
+    memcpy(buffer, bufptr, buflen);
+    (*p_len) = buflen;
   }
 
   //------------- clear sense data if it is not request sense command -------------//

examples/obsolete/device/src/msc_device_app.h

@@ -75,8 +75,6 @@ void msc_app_task(void* param);
 void msc_app_mount(uint8_t rhport);
 void msc_app_umount(uint8_t rhport);
 
-extern scsi_sense_fixed_data_t mscd_sense_data;
-
 #else
 
 #define msc_app_init()

examples/obsolete/device/src/tusb_config.h

@@ -57,8 +57,8 @@
 #define CFG_TUD_ENDOINT0_SIZE     64
 
 //------------- CLASS -------------//
-#define CFG_TUD_HID_KEYBOARD      1
-#define CFG_TUD_HID_MOUSE         1
+#define CFG_TUD_HID_KEYBOARD      0
+#define CFG_TUD_HID_MOUSE         0
 #define CFG_TUD_HID_GENERIC       0 // not supported yet
 #define CFG_TUD_MSC               1
 #define CFG_TUD_CDC               1

tinyusb/class/msc/msc_device.c

@@ -187,7 +187,7 @@ tusb_error_t mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, tusb_event_t event, u
       p_csw->tag          = p_cbw->tag;
       p_csw->data_residue = 0;
 
-      // Valid command -> move to Data Stage
+      /*------------- Parse command and prepare DATA -------------*/
       p_msc->stage       = MSC_STAGE_DATA;
       p_msc->data_len    = p_cbw->xfer_bytes;
       p_msc->xferred_len = 0;
@@ -205,7 +205,8 @@ tusb_error_t mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, tusb_event_t event, u
         // TODO SCSI data out transfer is not yet supported
         TU_ASSERT( !(p_cbw->xfer_bytes > 0 && !BIT_TEST_(p_cbw->dir, 7)), TUSB_ERROR_NOT_SUPPORTED_YET);
 
-        p_csw->status = tud_msc_scsi_cb(rhport, p_cbw->lun, p_cbw->command, &p_buffer, &p_msc->data_len);
+        // Invoke callback
+        p_csw->status = tud_msc_scsi_cb(rhport, p_cbw->lun, p_cbw->command, p_msc->scsi_data, &p_msc->data_len);
 
         if ( p_cbw->xfer_bytes == 0)
         {
@@ -215,22 +216,20 @@ tusb_error_t mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, tusb_event_t event, u
         else
         {
           // Data Phase (non READ10, WRITE10)
-          TU_ASSERT( p_cbw->xfer_bytes >= p_msc->data_len, TUSB_ERROR_INVALID_PARA );
+          TU_ASSERT( p_cbw->xfer_bytes >= p_msc->data_len, TUSB_ERROR_INVALID_PARA ); // cannot return more than host expect
           TU_ASSERT( sizeof(p_msc->scsi_data) >= p_msc->data_len, TUSB_ERROR_NOT_ENOUGH_MEMORY); // needs to increase size for scsi_data
 
-          uint8_t const ep_data = BIT_TEST_(p_cbw->dir, 7) ? p_msc->ep_in : p_msc->ep_out;
+          uint8_t const ep = BIT_TEST_(p_cbw->dir, 7) ? p_msc->ep_in : p_msc->ep_out;
 
-          if ( p_buffer == NULL || p_msc->data_len == 0 )
+          if ( p_msc->data_len )
+          {
+            TU_ASSERT( dcd_edpt_xfer(rhport, ep, p_msc->scsi_data, p_msc->data_len), TUSB_ERROR_DCD_EDPT_XFER );
+          }else
           {
             // application does not provide data to response --> possibly unsupported SCSI command
-            dcd_edpt_stall(rhport, ep_data);
+            dcd_edpt_stall(rhport, ep);
             p_csw->status = MSC_CSW_STATUS_FAILED;
-
             p_msc->stage = MSC_STAGE_STATUS;
-          }else
-          {
-            memcpy(p_msc->scsi_data, p_buffer, p_msc->data_len);
-            TU_ASSERT( dcd_edpt_xfer(rhport, ep_data, p_msc->scsi_data, p_msc->data_len), TUSB_ERROR_DCD_EDPT_XFER );
           }
         }
       }
@@ -284,8 +283,10 @@ static bool read10_write10_data_xfer(uint8_t rhport, mscd_interface_t* p_msc)
 
   uint8_t const ep_data = BIT_TEST_(p_cbw->dir, 7) ? p_msc->ep_in : p_msc->ep_out;
 
+  // LBA and Block count are in Big Endian
   uint32_t lba              = __be2n(p_readwrite->lba);
   uint16_t block_count      = __be2n_16(p_readwrite->block_count);
+
   uint16_t const block_size = p_cbw->xfer_bytes / block_count;
 
   // Adjust lba and block count according to byte transferred so far

tinyusb/class/msc/msc_device.h

@@ -53,15 +53,18 @@
  *  @{ */
 
 //--------------------------------------------------------------------+
-// APPLICATION API
+// APPLICATION API (Multiple Root Hub Ports)
+// Should be used only with MCU that support more than 1 ports
 //--------------------------------------------------------------------+
 
+
+
 //--------------------------------------------------------------------+
 // APPLICATION CALLBACK API
 //--------------------------------------------------------------------+
 
 /** \brief 			Callback that is invoked when tinyusb stack received \ref SCSI_CMD_READ_10 command from host
- * \param[in]		rhport	    USB Controller ID
+ * \param[in]		rhport	    Root hub port
  * \param[in]		lun         Targeted Logical Unit
  * \param[out]	pp_buffer   Pointer to buffer which application need to update with the response data's address.
  *                          Must be accessible by USB controller (see \ref CFG_TUSB_ATTR_USBRAM)
@@ -79,7 +82,7 @@
 uint16_t tud_msc_read10_cb (uint8_t rhport, uint8_t lun, void** pp_buffer, uint32_t lba, uint16_t block_count);
 
 /** \brief 			Callback that is invoked when tinyusb stack received \ref SCSI_CMD_WRITE_10 command from host
- * \param[in]		rhport	    USB Controller ID
+ * \param[in]		rhport	    Root hub port
  * \param[in]		lun         Targeted Logical Unit
  * \param[out]	pp_buffer   Pointer to buffer which application need to update with the address to hold data from host
  *                          Must be accessible by USB controller (see \ref CFG_TUSB_ATTR_USBRAM)
@@ -99,19 +102,19 @@ uint16_t tud_msc_write10_cb(uint8_t rhport, uint8_t lun, void** pp_buffer, uint3
 // p_length [in,out] allocated/maximum length, application update with actual length
 /** \brief 			Callback that is invoked when tinyusb stack received an SCSI command other than \ref SCSI_CMD_WRITE_10 and
  *              \ref SCSI_CMD_READ_10 command from host
- * \param[in]		rhport	    USB Controller ID
+ * \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]	pp_buffer   Pointer to buffer which application need to update with the address to transfer data with host.
+ * \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]		p_length    length
+ * \param[in]		p_length    Expected length from host, Application could update to actual data, but could not larger than original value.
  * \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
  *                          endpoint and return failed status in command status wrapper phase.
  * \note        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 reponse on other classes.
  */
-msc_csw_status_t tud_msc_scsi_cb (uint8_t rhport, uint8_t lun, uint8_t scsi_cmd[16], void const ** pp_buffer, uint16_t* p_length);
+msc_csw_status_t tud_msc_scsi_cb (uint8_t rhport, uint8_t lun, uint8_t scsi_cmd[16], void* buffer, uint16_t* p_len);
 
 /** @} */
 /** @} */