add tud_msc_set_sense(), update msc device to reponse with default sense (illegal request) when there is issue with scsi command

src/class/msc/msc.h

@@ -83,8 +83,8 @@ typedef enum
 /// MassStorage Class-Specific Control Request
 typedef enum
 {
-  MSC_REQUEST_GET_MAX_LUN = 254, ///< The Get Max LUN device request is used to determine the number of logical units supported by the device. Logical Unit Numbers on the device shall be numbered contiguously starting from LUN 0 to a maximum LUN of 15
-  MSC_REQUEST_RESET       = 255  ///< This request is used to reset the mass storage device and its associated interface. This class-specific request shall ready the device for the next CBW from the host.
+  MSC_REQ_GET_MAX_LUN = 254, ///< The Get Max LUN device request is used to determine the number of logical units supported by the device. Logical Unit Numbers on the device shall be numbered contiguously starting from LUN 0 to a maximum LUN of 15
+  MSC_REQ_RESET       = 255  ///< This request is used to reset the mass storage device and its associated interface. This class-specific request shall ready the device for the next CBW from the host.
 }msc_request_type_t;
 
 /// \brief Command Block Status Values
@@ -145,19 +145,19 @@ typedef enum
 /// SCSI Sense Key
 typedef enum
 {
-  SCSI_SENSEKEY_NONE            = 0x00, ///< no specific Sense Key. This would be the case for a successful command
-  SCSI_SENSEKEY_RECOVERED_ERROR = 0x01, ///< ndicates the last command completed successfully with some recovery action performed by the disc drive.
-  SCSI_SENSEKEY_NOT_READY       = 0x02, ///< Indicates the logical unit addressed cannot be accessed.
-  SCSI_SENSEKEY_MEDIUM_ERROR    = 0x03, ///< Indicates the command terminated with a non-recovered error condition.
-  SCSI_SENSEKEY_HARDWARE_ERROR  = 0x04, ///< Indicates the disc drive detected a nonrecoverable hardware failure while performing the command or during a self test.
-  SCSI_SENSEKEY_ILLEGLA_REQUEST = 0x05, ///< Indicates an illegal parameter in the command descriptor block or in the additional parameters
-  SCSI_SENSEKEY_UNIT_ATTENTION  = 0x06, ///< Indicates the disc drive may have been reset.
-  SCSI_SENSEKEY_DATA_PROTECT    = 0x07, ///< Indicates that a command that reads or writes the medium was attempted on a block that is protected from this operation. The read or write operation is not performed.
-  SCSI_SENSEKEY_FIRMWARE_ERROR  = 0x08, ///< Vendor specific sense key.
-  SCSI_SENSEKEY_ABORTED_COMMAND = 0x0b, ///< Indicates the disc drive aborted the command.
-  SCSI_SENSEKEY_EQUAL           = 0x0c, ///< Indicates a SEARCH DATA command has satisfied an equal comparison.
-  SCSI_SENSEKEY_VOLUME_OVERFLOW = 0x0d, ///< Indicates a buffered peripheral device has reached the end of medium partition and data remains in the buffer that has not been written to the medium.
-  SCSI_SENSEKEY_MISCOMPARE      = 0x0e  ///< ndicates that the source data did not match the data read from the medium.
+  SCSI_SENSE_NONE            = 0x00, ///< no specific Sense Key. This would be the case for a successful command
+  SCSI_SENSE_RECOVERED_ERROR = 0x01, ///< ndicates the last command completed successfully with some recovery action performed by the disc drive.
+  SCSI_SENSE_NOT_READY       = 0x02, ///< Indicates the logical unit addressed cannot be accessed.
+  SCSI_SENSE_MEDIUM_ERROR    = 0x03, ///< Indicates the command terminated with a non-recovered error condition.
+  SCSI_SENSE_HARDWARE_ERROR  = 0x04, ///< Indicates the disc drive detected a nonrecoverable hardware failure while performing the command or during a self test.
+  SCSI_SENSE_ILLEGAL_REQUEST = 0x05, ///< Indicates an illegal parameter in the command descriptor block or in the additional parameters
+  SCSI_SENSE_UNIT_ATTENTION  = 0x06, ///< Indicates the disc drive may have been reset.
+  SCSI_SENSE_DATA_PROTECT    = 0x07, ///< Indicates that a command that reads or writes the medium was attempted on a block that is protected from this operation. The read or write operation is not performed.
+  SCSI_SENSE_FIRMWARE_ERROR  = 0x08, ///< Vendor specific sense key.
+  SCSI_SENSE_ABORTED_COMMAND = 0x0b, ///< Indicates the disc drive aborted the command.
+  SCSI_SENSE_EQUAL           = 0x0c, ///< Indicates a SEARCH DATA command has satisfied an equal comparison.
+  SCSI_SENSE_VOLUME_OVERFLOW = 0x0d, ///< Indicates a buffered peripheral device has reached the end of medium partition and data remains in the buffer that has not been written to the medium.
+  SCSI_SENSE_MISCOMPARE      = 0x0e  ///< ndicates that the source data did not match the data read from the medium.
 }scsi_sense_key_type_t;
 
 //--------------------------------------------------------------------+

src/class/msc/msc_device.c

@@ -116,11 +116,25 @@ static inline uint16_t rdwr10_get_blockcount(uint8_t const command[])
   return __be2n_16(block_count);
 }
 
+//--------------------------------------------------------------------+
+// APPLICATION API
+//--------------------------------------------------------------------+
 bool tud_msc_ready(void)
 {
   return ( _mscd_itf.ep_in != 0 ) && ( _mscd_itf.ep_out != 0 ) ;
 }
 
+bool tud_msc_set_sense(uint8_t lun, uint8_t sense_key, uint8_t add_sense_code, uint8_t add_sense_qualifier)
+{
+  (void) lun;
+
+  _mscd_itf.sense_key           = sense_key;
+  _mscd_itf.add_sense_code      = add_sense_code;
+  _mscd_itf.add_sense_qualifier = add_sense_qualifier;
+
+  return true;
+}
+
 
 //--------------------------------------------------------------------+
 // USBD-CLASS API
@@ -162,11 +176,11 @@ tusb_error_t mscd_control_request_st(uint8_t rhport, tusb_control_request_t cons
 
   TU_ASSERT(p_request->bmRequestType_bit.type == TUSB_REQ_TYPE_CLASS, TUSB_ERROR_DCD_CONTROL_REQUEST_NOT_SUPPORT);
 
-  if(MSC_REQUEST_RESET == p_request->bRequest)
+  if(MSC_REQ_RESET == p_request->bRequest)
   {
     dcd_control_status(rhport, p_request->bmRequestType_bit.direction);
   }
-  else if (MSC_REQUEST_GET_MAX_LUN == p_request->bRequest)
+  else if (MSC_REQ_GET_MAX_LUN == p_request->bRequest)
   {
     // returned MAX LUN is minus 1 by specs
     _usbd_ctrl_buf[0] = CFG_TUD_MSC_MAXLUN-1;
@@ -245,22 +259,21 @@ int32_t proc_builtin_scsi(msc_cbw_t const * p_cbw, uint8_t* buffer, uint32_t buf
 
       sense_rsp.add_sense_len = sizeof(scsi_sense_fixed_data_t) - 8;
 
+      sense_rsp.sense_key           = _mscd_itf.sense_key;
+      sense_rsp.add_sense_code      = _mscd_itf.add_sense_code;
+      sense_rsp.add_sense_qualifier = _mscd_itf.add_sense_qualifier;
+
       ret = sizeof(sense_rsp);
       memcpy(buffer, &sense_rsp, ret);
+
+      // Clear sense data after copy
+      tud_msc_set_sense(p_cbw->lun, 0, 0, 0);
     }
     break;
 
     default: ret = -1; break;
   }
 
-  //------------- clear sense data if it is not request sense command -------------//
-//  if ( SCSI_CMD_REQUEST_SENSE != p_cbw->command[0])
-//  {
-//    sense_rsp.sense_key           = SCSI_SENSEKEY_NONE;
-//    sense_rsp.add_sense_code      = 0;
-//    sense_rsp.add_sense_qualifier = 0;
-//  }
-
   return ret;
 }
 
@@ -309,9 +322,18 @@ tusb_error_t mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, tusb_event_t event, u
         {
           int32_t const cb_result = tud_msc_scsi_cb(p_cbw->lun, p_cbw->command, NULL, 0);
 
-          p_csw->status   = (cb_result == 0) ? MSC_CSW_STATUS_PASSED : MSC_CSW_STATUS_FAILED;
           p_msc->data_len = 0;
           p_msc->stage    = MSC_STAGE_STATUS;
+
+          if ( cb_result < 0 )
+          {
+            p_csw->status = MSC_CSW_STATUS_FAILED;
+            tud_msc_set_sense(p_cbw->lun, SCSI_SENSE_ILLEGAL_REQUEST, 0x20, 0x00); // Sense = Invalid Command Operation
+          }
+          else
+          {
+            p_csw->status = MSC_CSW_STATUS_PASSED;
+          }
         }
         else if ( !BIT_TEST_(p_cbw->dir, 7) )
         {
@@ -334,25 +356,18 @@ tusb_error_t mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, tusb_event_t event, u
 
           if ( cb_result > 0 )
           {
-            p_csw->status   = MSC_CSW_STATUS_PASSED;
             p_msc->data_len = (uint32_t) cb_result;
-          }else
-          {
-            p_csw->status = MSC_CSW_STATUS_FAILED;
-            p_msc->data_len = 0;
-          }
-
-          TU_ASSERT( p_cbw->xfer_bytes >= p_msc->data_len, TUSB_ERROR_INVALID_PARA ); // cannot return more than host expect
+            p_csw->status   = MSC_CSW_STATUS_PASSED;
 
-          if ( p_msc->data_len )
-          {
+            TU_ASSERT( p_cbw->xfer_bytes >= p_msc->data_len, TUSB_ERROR_INVALID_PARA ); // cannot return more than host expect
             TU_ASSERT( dcd_edpt_xfer(rhport, p_msc->ep_in, _mscd_buf, p_msc->data_len), TUSB_ERROR_DCD_EDPT_XFER );
           }else
           {
-            // callback does not provide response's data --> possibly unsupported SCSI command
-            p_csw->status = MSC_CSW_STATUS_FAILED;
-            p_msc->stage  = MSC_STAGE_STATUS;
+            p_msc->data_len = 0;
+            p_csw->status   = MSC_CSW_STATUS_FAILED;
+            p_msc->stage    = MSC_STAGE_STATUS;
 
+            tud_msc_set_sense(p_cbw->lun, SCSI_SENSE_ILLEGAL_REQUEST, 0x20, 0x00); // Sense = Invalid Command Operation
             dcd_edpt_stall(rhport, p_msc->ep_in);
           }
         }
@@ -365,7 +380,16 @@ tusb_error_t mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, tusb_event_t event, u
       {
         if ( SCSI_CMD_WRITE_10 != p_cbw->command[0] )
         {
-          p_csw->status = (tud_msc_scsi_cb(p_cbw->lun, p_cbw->command, _mscd_buf, p_msc->data_len) >= 0 ) ? MSC_CSW_STATUS_PASSED : MSC_CSW_STATUS_FAILED;
+          int32_t cb_result = tud_msc_scsi_cb(p_cbw->lun, p_cbw->command, _mscd_buf, p_msc->data_len);
+
+          if ( cb_result < 0 )
+          {
+            p_csw->status = MSC_CSW_STATUS_FAILED;
+            tud_msc_set_sense(p_cbw->lun, SCSI_SENSE_ILLEGAL_REQUEST, 0x20, 0x00); // Sense = Invalid Command Operation
+          }else
+          {
+            p_csw->status = MSC_CSW_STATUS_PASSED;
+          }
         }
         else
         {
@@ -382,8 +406,9 @@ tusb_error_t mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, tusb_event_t event, u
             // negative means error -> skip to status phase, 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;
+            p_msc->stage        = MSC_STAGE_STATUS;
 
-            p_msc->stage = MSC_STAGE_STATUS;
+            tud_msc_set_sense(p_cbw->lun, SCSI_SENSE_ILLEGAL_REQUEST, 0x20, 0x00); // Sense = Invalid Command Operation
             break;
           }else
           {
@@ -403,8 +428,7 @@ tusb_error_t mscd_xfer_cb(uint8_t rhport, uint8_t ep_addr, tusb_event_t event, u
             }
             else
             {
-              // Application consume all bytes in our buffer
-              // Nothing to do, process with normal flow
+              // Application consume all bytes in our buffer. Nothing to do, process with normal flow
             }
           }
         }
@@ -503,6 +527,7 @@ static void proc_read10_cmd(uint8_t rhport, mscd_interface_t* p_msc)
     p_csw->data_residue = p_cbw->xfer_bytes - p_msc->xferred_len;
     p_csw->status       = MSC_CSW_STATUS_FAILED;
 
+    tud_msc_set_sense(p_cbw->lun, SCSI_SENSE_ILLEGAL_REQUEST, 0x20, 0x00); // Sense = Invalid Command Operation
     dcd_edpt_stall(rhport, p_msc->ep_in);
   }
   else if ( nbytes == 0 )

src/class/msc/msc_device.h

@@ -97,6 +97,7 @@ VERIFY_STATIC(CFG_TUD_MSC_BUFSIZE < UINT16_MAX, "Size is not correct");
 
 // Check if MSC interface is ready to use
 bool tud_msc_ready(void);
+bool tud_msc_set_sense(uint8_t lun, uint8_t sense_key, uint8_t add_sense_code, uint8_t add_sense_qualifier);
 
 //--------------------------------------------------------------------+
 // APPLICATION CALLBACK API (WEAK is optional)

src/class/msc/msc_host.c

@@ -329,7 +329,7 @@ tusb_error_t msch_open_subtask(uint8_t dev_addr, tusb_desc_interface_t const *p_
   //------------- Get Max Lun -------------//
   STASK_INVOKE(
     usbh_control_xfer_subtask( dev_addr, bm_request_type(TUSB_DIR_IN, TUSB_REQ_TYPE_CLASS, TUSB_REQ_RCPT_INTERFACE),
-                               MSC_REQUEST_GET_MAX_LUN, 0, msch_data[dev_addr-1].interface_number,
+                               MSC_REQ_GET_MAX_LUN, 0, msch_data[dev_addr-1].interface_number,
                                1, msch_buffer ),
     error
   );
@@ -341,7 +341,7 @@ tusb_error_t msch_open_subtask(uint8_t dev_addr, tusb_desc_interface_t const *p_
   //------------- Reset -------------//
   STASK_INVOKE(
     usbh_control_xfer_subtask( dev_addr, bm_request_type(TUSB_DIR_OUT, TUSB_REQ_TYPE_CLASS, TUSB_REQ_RCPT_INTERFACE),
-                               MSC_REQUEST_RESET, 0, msch_data[dev_addr-1].interface_number,
+                               MSC_REQ_RESET, 0, msch_data[dev_addr-1].interface_number,
                                0, NULL ),
     error
   );

tests/lpc18xx_43xx/test/host/msc/test_msc_host.c

@@ -105,7 +105,7 @@ tusb_error_t stub_control_xfer(uint8_t dev_addr, uint8_t bmRequestType, uint8_t
     case 0: // get max lun
       TEST_ASSERT_EQUAL(bm_request_type(TUSB_DIR_DEV_TO_HOST, TUSB_REQ_TYPE_CLASS, TUSB_REQ_RECIPIENT_INTERFACE),
                         bmRequestType);
-      TEST_ASSERT_EQUAL(MSC_REQUEST_GET_MAX_LUN, bRequest);
+      TEST_ASSERT_EQUAL(MSC_REQ_GET_MAX_LUN, bRequest);
       TEST_ASSERT_EQUAL(p_msc_interface_desc->bInterfaceNumber, wIndex);
       TEST_ASSERT_EQUAL(1, wLength);
       *data = 1; // TODO multiple LUN support