3 files changed, 132 insertions, 72 deletions

diff --git a/src/class/mtp/mtp.h b/src/class/mtp/mtp.h
index 40b6dd8b0..7b22837cd 100644
--- a/src/class/mtp/mtp.h
+++ b/src/class/mtp/mtp.h
@@ -798,45 +798,65 @@ TU_ATTR_ALWAYS_INLINE static inline uint32_t mtp_container_add_array(mtp_contain
 }
 
 TU_ATTR_ALWAYS_INLINE static inline uint32_t mtp_container_add_string(mtp_container_info_t* p_container, uint16_t* utf16) {
-  uint8_t count = 0;
-  while (utf16[count]) {
+  uint32_t count = 0;
+  while (utf16[count] != 0u) {
     count++;
   }
-  const uint32_t added_len = 1u + 2u * count;
-  TU_ASSERT(p_container->header->len + added_len < CFG_TUD_MTP_EP_BUFSIZE, 0);
+  // MTP strings store length in a single uint8_t, including trailing null.
+  TU_ASSERT(count < UINT8_MAX, 0);
+  count++;
+
   uint8_t* buf = p_container->payload + p_container->header->len - sizeof(mtp_container_header_t);
 
-  *buf++ = count;
+  if (count == 1) {
+    // empty string (size only): single zero byte
+    TU_ASSERT(p_container->header->len + 1 < CFG_TUD_MTP_EP_BUFSIZE, 0);
+    *buf = 0;
+    p_container->header->len++;
+    return 1u;
+  }
+
+  const uint32_t added_len = 1u + count * 2u;
+  TU_ASSERT(p_container->header->len + added_len < CFG_TUD_MTP_EP_BUFSIZE, 0);
+
+  *buf++ = (uint8_t) count;
   p_container->header->len++;
 
-  memcpy(buf, utf16, 2 * count);
-  p_container->header->len += 2 * count;
+  memcpy(buf, utf16, 2u * count);
+  p_container->header->len += 2u * count;
 
   return added_len;
 }
 
 TU_ATTR_ALWAYS_INLINE static inline uint32_t mtp_container_add_cstring(mtp_container_info_t* p_container, const char* str) {
-  const uint8_t len = (uint8_t) (strlen(str) + 1); // include null
-  TU_ASSERT(p_container->header->len + 1 + 2 * len < CFG_TUD_MTP_EP_BUFSIZE, 0);
+  const size_t cstr_len = strlen(str);
+  // MTP strings store length in a single uint8_t, including trailing null.
+  TU_ASSERT(cstr_len < UINT8_MAX, 0);
+
+  const uint32_t count = (uint32_t) cstr_len + 1u; // include null
   uint8_t* buf = p_container->payload + p_container->header->len - sizeof(mtp_container_header_t);
 
-  if (len == 1) {
-    // empty string (null only): single zero byte
+  if (count == 1u) {
+    // empty string (size only): single zero byte
+    TU_ASSERT(p_container->header->len + 1 < CFG_TUD_MTP_EP_BUFSIZE, 0);
     *buf = 0;
     p_container->header->len++;
-    return 1;
-  } else {
-    *buf++ = len;
-    p_container->header->len++;
+    return 1u;
+  }
+
+  const uint32_t added_len = 1u + 2u * count;
+  TU_ASSERT(p_container->header->len + added_len < CFG_TUD_MTP_EP_BUFSIZE, 0);
 
-    for (uint8_t i = 0; i < len; i++) {
-      buf[0] = str[i];
-      buf[1] = 0;
-      buf += 2;
-      p_container->header->len += 2;
-    }
-    return 1u + 2u * len;
+  *buf++ = (uint8_t) count;
+  p_container->header->len++;
+
+  for (uint32_t i = 0; i < count; i++) {
+    *buf++ = (uint8_t) str[i];
+    *buf++ = 0;
   }
+  p_container->header->len += 2u * count;
+
+  return added_len;
 }
 
 TU_ATTR_ALWAYS_INLINE static inline uint32_t mtp_container_add_uint8(mtp_container_info_t* p_container, uint8_t data) {
@@ -875,8 +895,8 @@ TU_ATTR_ALWAYS_INLINE static inline uint32_t mtp_container_add_auint32(mtp_conta
 //
 //--------------------------------------------------------------------+
 TU_ATTR_ALWAYS_INLINE static inline uint32_t mtp_container_get_string(uint8_t* buf, uint16_t utf16[]) {
-  uint8_t nchars = *buf++;
-  memcpy(utf16, buf, 2 * nchars);
+  size_t nchars = *buf++;
+  memcpy(utf16, buf, 2u * nchars);
   return 1u + 2u * nchars;
 }
 
diff --git a/src/class/mtp/mtp_device.c b/src/class/mtp/mtp_device.c
index 798a965eb..1f76dfcc7 100644
--- a/src/class/mtp/mtp_device.c
+++ b/src/class/mtp/mtp_device.c
@@ -92,8 +92,9 @@ typedef struct {
   uint8_t itf_num;
   uint8_t ep_in;
   uint8_t ep_out;
-  uint8_t ep_event;
 
+  uint8_t ep_event;
+  uint8_t ep_sz_fs;
   // Bulk Only Transfer (BOT) Protocol
   uint8_t  phase;
 
@@ -191,45 +192,50 @@ TU_ATTR_UNUSED static const char* _mtp_phase_str[] = {
 //--------------------------------------------------------------------+
 static bool prepare_new_command(mtpd_interface_t* p_mtp) {
   p_mtp->phase = MTP_PHASE_COMMAND;
-  return usbd_edpt_xfer(p_mtp->rhport, p_mtp->ep_out, _mtpd_epbuf.buf, CFG_TUD_MTP_EP_BUFSIZE);
+  return usbd_edpt_xfer(p_mtp->rhport, p_mtp->ep_out, _mtpd_epbuf.buf, CFG_TUD_MTP_EP_BUFSIZE, false);
 }
 
-static bool mtpd_data_xfer(mtp_container_info_t* p_container, uint8_t ep_addr) {
-  mtpd_interface_t* p_mtp = &_mtpd_itf;
+bool tud_mtp_data_send(mtp_container_info_t *p_container) {
+  mtpd_interface_t *p_mtp = &_mtpd_itf;
   if (p_mtp->phase == MTP_PHASE_COMMAND) {
     // 1st data block: header + payload
     p_mtp->phase = MTP_PHASE_DATA;
     p_mtp->xferred_len = 0;
+    p_mtp->total_len   = p_container->header->len;
 
-    if (tu_edpt_dir(ep_addr) == TUSB_DIR_IN) {
-      p_mtp->total_len = p_container->header->len;
-      p_container->header->type = MTP_CONTAINER_TYPE_DATA_BLOCK;
-      p_container->header->transaction_id = p_mtp->command.header.transaction_id;
-      p_mtp->io_header = *p_container->header; // save header for subsequent data
-    } else {
-      // OUT transfer: total length is at least max packet size
-      p_mtp->total_len = tu_max32(p_container->header->len, CFG_TUD_MTP_EP_BUFSIZE);
-    }
-  } else {
-    // subsequent data block: payload only
-    TU_ASSERT(p_mtp->phase == MTP_PHASE_DATA);
+    p_container->header->type           = MTP_CONTAINER_TYPE_DATA_BLOCK;
+    p_container->header->transaction_id = p_mtp->command.header.transaction_id;
+    p_mtp->io_header                    = *p_container->header; // save header for subsequent data
   }
 
-  const uint16_t xact_len = tu_min16((uint16_t) (p_mtp->total_len - p_mtp->xferred_len), CFG_TUD_MTP_EP_BUFSIZE);
+  const uint16_t xact_len = (uint16_t)tu_min32(p_mtp->total_len - p_mtp->xferred_len, CFG_TUD_MTP_EP_BUFSIZE);
+
+  TU_LOG_DRV("  MTP Data IN: xferred_len/total_len=%lu/%lu, xact_len=%u\r\n", p_mtp->xferred_len, p_mtp->total_len,
+             xact_len);
   if (xact_len) {
-    // already transferred all bytes in header's length. Application make an unnecessary extra call
-    TU_VERIFY(usbd_edpt_claim(p_mtp->rhport, ep_addr));
-    TU_ASSERT(usbd_edpt_xfer(p_mtp->rhport, ep_addr, _mtpd_epbuf.buf, xact_len));
+    TU_VERIFY(usbd_edpt_claim(p_mtp->rhport, p_mtp->ep_in));
+    TU_ASSERT(usbd_edpt_xfer(p_mtp->rhport, p_mtp->ep_in, _mtpd_epbuf.buf, xact_len, false));
   }
   return true;
 }
 
-bool tud_mtp_data_send(mtp_container_info_t* p_container) {
-  return mtpd_data_xfer(p_container, _mtpd_itf.ep_in);
-}
+bool tud_mtp_data_receive(mtp_container_info_t *p_container) {
+  mtpd_interface_t *p_mtp = &_mtpd_itf;
+  if (p_mtp->phase == MTP_PHASE_COMMAND) {
+    // 1st data block: header + payload
+    p_mtp->phase       = MTP_PHASE_DATA;
+    p_mtp->xferred_len = 0;
+    p_mtp->total_len   = p_container->header->len;
+  }
+
+  // up to buffer size since 1st packet (with header) may also contain payload
+  const uint16_t xact_len = CFG_TUD_MTP_EP_BUFSIZE;
 
-bool tud_mtp_data_receive(mtp_container_info_t* p_container) {
-  return mtpd_data_xfer(p_container, _mtpd_itf.ep_out);
+  TU_LOG_DRV("  MTP Data OUT: xferred_len/total_len=%lu/%lu, xact_len=%u\r\n", p_mtp->xferred_len, p_mtp->total_len,
+             xact_len);
+  TU_VERIFY(usbd_edpt_claim(p_mtp->rhport, p_mtp->ep_out));
+  TU_ASSERT(usbd_edpt_xfer(p_mtp->rhport, p_mtp->ep_out, _mtpd_epbuf.buf, xact_len, false));
+  return true;
 }
 
 bool tud_mtp_response_send(mtp_container_info_t* p_container) {
@@ -238,7 +244,7 @@ bool tud_mtp_response_send(mtp_container_info_t* p_container) {
   p_container->header->type = MTP_CONTAINER_TYPE_RESPONSE_BLOCK;
   p_container->header->transaction_id = p_mtp->command.header.transaction_id;
   TU_VERIFY(usbd_edpt_claim(p_mtp->rhport, p_mtp->ep_in));
-  return usbd_edpt_xfer(p_mtp->rhport, p_mtp->ep_in, _mtpd_epbuf.buf, (uint16_t)p_container->header->len);
+  return usbd_edpt_xfer(p_mtp->rhport, p_mtp->ep_in, _mtpd_epbuf.buf, (uint16_t) p_container->header->len, false);
 }
 
 bool tud_mtp_mounted(void) {
@@ -251,7 +257,7 @@ bool tud_mtp_event_send(mtp_event_t* event) {
   TU_VERIFY(p_mtp->ep_event != 0);
   _mtpd_epbuf.buf_event = *event;
   TU_VERIFY(usbd_edpt_claim(p_mtp->rhport, p_mtp->ep_event)); // Claim the endpoint
-  return usbd_edpt_xfer(p_mtp->rhport, p_mtp->ep_event, (uint8_t*) &_mtpd_epbuf.buf_event, sizeof(mtp_event_t));
+  return usbd_edpt_xfer(p_mtp->rhport, p_mtp->ep_event, (uint8_t*) &_mtpd_epbuf.buf_event, sizeof(mtp_event_t), false);
 }
 
 //--------------------------------------------------------------------+
@@ -287,15 +293,20 @@ uint16_t mtpd_open(uint8_t rhport, tusb_desc_interface_t const* itf_desc, uint16
   p_mtp->itf_num = itf_desc->bInterfaceNumber;
 
   // Open interrupt IN endpoint
-  const tusb_desc_endpoint_t* ep_desc = (const tusb_desc_endpoint_t*) tu_desc_next(itf_desc);
-  TU_ASSERT(ep_desc->bDescriptorType == TUSB_DESC_ENDPOINT && ep_desc->bmAttributes.xfer == TUSB_XFER_INTERRUPT, 0);
-  TU_ASSERT(usbd_edpt_open(rhport, ep_desc), 0);
-  p_mtp->ep_event = ep_desc->bEndpointAddress;
+  const tusb_desc_endpoint_t* ep_desc_int = (const tusb_desc_endpoint_t*) tu_desc_next(itf_desc);
+  TU_ASSERT(ep_desc_int->bDescriptorType == TUSB_DESC_ENDPOINT && ep_desc_int->bmAttributes.xfer == TUSB_XFER_INTERRUPT, 0);
+  TU_ASSERT(usbd_edpt_open(rhport, ep_desc_int), 0);
+  p_mtp->ep_event = ep_desc_int->bEndpointAddress;
 
   // Open endpoint pair
-  TU_ASSERT(usbd_open_edpt_pair(rhport, tu_desc_next(ep_desc), 2, TUSB_XFER_BULK, &p_mtp->ep_out, &p_mtp->ep_in), 0);
+  const tusb_desc_endpoint_t* ep_desc_bulk = (const tusb_desc_endpoint_t*) tu_desc_next(ep_desc_int);
+  TU_ASSERT(usbd_open_edpt_pair(rhport, (const uint8_t*)ep_desc_bulk, 2, TUSB_XFER_BULK, &p_mtp->ep_out, &p_mtp->ep_in), 0);
   TU_ASSERT(prepare_new_command(p_mtp), 0);
 
+  if (tud_speed_get() == TUSB_SPEED_FULL) {
+    p_mtp->ep_sz_fs = (uint8_t)tu_edpt_packet_size(ep_desc_bulk);
+  }
+
   return mtpd_itf_size;
 }
 
@@ -310,7 +321,7 @@ bool mtpd_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t
     .session_id = p_mtp->session_id,
     .request = request,
     .buf = p_mtp->control_buf,
-    .bufsize = tu_le16toh(request->wLength),
+    .bufsize = request->wLength,
   };
 
   switch (request->bRequest) {
@@ -377,8 +388,8 @@ bool mtpd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t event, uint32_t
   mtp_generic_container_t* p_container = (mtp_generic_container_t*) _mtpd_epbuf.buf;
 
 #if CFG_TUSB_DEBUG >= CFG_TUD_MTP_LOG_LEVEL
-  tu_lookup_find(&_mtp_op_table, p_mtp->command.header.code);
-  TU_LOG_DRV("  MTP %s: %s phase\r\n", (const char *) tu_lookup_find(&_mtp_op_table, p_mtp->command.header.code),
+  const uint16_t code = (p_mtp->phase == MTP_PHASE_COMMAND) ? p_container->header.code : p_mtp->command.header.code;
+  TU_LOG_DRV("  MTP %s: %s phase\r\n", (const char *) tu_lookup_find(&_mtp_op_table, code),
     _mtp_phase_str[p_mtp->phase]);
 #endif
 
@@ -417,19 +428,42 @@ bool mtpd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t event, uint32_t
     }
 
     case MTP_PHASE_DATA: {
-      const uint16_t bulk_mps = (tud_speed_get() == TUSB_SPEED_HIGH) ? 512 : 64;
       p_mtp->xferred_len += xferred_bytes;
       cb_data.total_xferred_bytes = p_mtp->xferred_len;
 
-      bool is_complete = false;
-      // complete if ZLP or short packet or overflow
-      if (xferred_bytes == 0 || // ZLP
-          (xferred_bytes & (bulk_mps - 1)) || // short packet
-          p_mtp->xferred_len > p_mtp->total_len) {
-        is_complete = true;
+      const bool is_data_in = (ep_addr == p_mtp->ep_in);
+      // For IN endpoint, threshold is bulk max packet size
+      // For OUT endpoint, threshold is endpoint buffer size, since we always queue fixed size
+      uint16_t threshold;
+      if (is_data_in) {
+        threshold = (p_mtp->ep_sz_fs > 0) ? p_mtp->ep_sz_fs : 512; // full speed bulk if set
+      } else {
+        threshold = CFG_TUD_MTP_EP_BUFSIZE;
+      }
+
+      // Check completion for IN and OUT separately
+      bool is_complete;
+      if (is_data_in) {
+        // IN completion: short packet, ZLP, or reaching total_len
+        is_complete = (xferred_bytes == 0 || xferred_bytes < threshold || p_mtp->xferred_len >= p_mtp->total_len);
+      } else {
+        // OUT completion: reaching total_len or ZLP only. A short packet does NOT end the phase
+        // (an early short packet before total_len is the cancel case, not normal completion).
+        is_complete = (p_mtp->xferred_len >= p_mtp->total_len) || ((xferred_bytes == 0 && p_mtp->xferred_len > 0));
+      }
+
+      TU_LOG_DRV("  MTP Data %s CB: xferred_bytes=%lu, xferred_len/total_len=%lu/%lu, is_complete=%d\r\n",
+                 is_data_in ? "IN" : "OUT", xferred_bytes, p_mtp->xferred_len, p_mtp->total_len, is_complete ? 1 : 0);
+
+      // Send/queue ZLP if packet is full-sized but transfer is complete
+      if (is_complete && xferred_bytes > 0 && !(xferred_bytes & (threshold - 1))) {
+        TU_LOG_DRV("  queue ZLP\r\n");
+        TU_VERIFY(usbd_edpt_claim(p_mtp->rhport, ep_addr));
+        TU_ASSERT(usbd_edpt_xfer(p_mtp->rhport, ep_addr, NULL, 0, false));
+        return true;
       }
 
-      if (ep_addr == p_mtp->ep_in) {
+      if (is_data_in) {
         // Data In
         if (is_complete) {
           cb_data.io_container.header->len = sizeof(mtp_container_header_t);
diff --git a/src/class/mtp/mtp_device.h b/src/class/mtp/mtp_device.h
index 397fbbbce..f2c5cef7f 100644
--- a/src/class/mtp/mtp_device.h
+++ b/src/class/mtp/mtp_device.h
@@ -18,7 +18,7 @@
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
  * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
  * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN0
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  *
  * This file is part of the TinyUSB stack.
@@ -53,12 +53,14 @@ typedef struct {
 typedef struct {
   uint8_t idx;
   uint8_t stage; // control stage
-  uint32_t session_id;
 
-  const tusb_control_request_t* request;
   // buffer for data stage
-  uint8_t* buf;
   uint16_t bufsize;
+  uint8_t* buf;
+
+  const tusb_control_request_t* request;
+
+  uint32_t session_id;
 } tud_mtp_request_cb_data_t;
 
 // Number of supported operations, events, device properties, capture formats, playback formats
@@ -76,9 +78,13 @@ typedef struct {
     mtp_auint16_t(_capture_count) capture_formats; \
     mtp_auint16_t(_playback_count) playback_formats; \
     /* string fields will be added using append function */ \
+    /* mtp_string_t() Manufacturer */ \
+    /* mtp_string_t() Model */ \
+    /* mtp_string_t() Device Version */ \
+    /* mtp_string_t() Serial Number */ \
   }
 
-typedef MTP_DEVICE_INFO_STRUCT(
+typedef MTP_DEVICE_INFO_STRUCT( //-V2586 [MISRA-C-18.7] Flexible array members should not be declared
   sizeof(CFG_TUD_MTP_DEVICEINFO_EXTENSIONS), TU_ARGS_NUM(CFG_TUD_MTP_DEVICEINFO_SUPPORTED_OPERATIONS),
   TU_ARGS_NUM(CFG_TUD_MTP_DEVICEINFO_SUPPORTED_EVENTS), TU_ARGS_NUM(CFG_TUD_MTP_DEVICEINFO_SUPPORTED_DEVICE_PROPERTIES),
   TU_ARGS_NUM(CFG_TUD_MTP_DEVICEINFO_CAPTURE_FORMATS), TU_ARGS_NUM(CFG_TUD_MTP_DEVICEINFO_PLAYBACK_FORMATS)