Review comments

- mtp: use `usbd_bus_speed` to get the correct descriptor which holds the Manufacturer/Product/SerialNumber strings
- mtp_class: return OPERATION_NOT_SUPPORTED when not supported command is received.
- tests: Add mtp to build_all
- Updated Sample README file
- Fix build error
- Cleanup multiple files

Author: ExtremeGTX

samples/subsys/usb/mtp/README.rst

@@ -1,36 +1,36 @@
-.. zephyr:code-sample:: usb-cdc-acm
-   :name: USB CDC-ACM
-   :relevant-api: usbd_api _usb_device_core_api uart_interface
+.. zephyr:code-sample:: usb-mtp
+   :name: USB MTP
+   :relevant-api: usbd_api _usb_device_core_api file_system_api
 
-   Use USB CDC-ACM driver to implement a serial port echo.
+   Use USB MTP driver which implement a Media Transfer Protocol device.
 
 Overview
 ********
 
-This sample app demonstrates use of a USB Communication Device Class (CDC)
-Abstract Control Model (ACM) driver provided by the Zephyr project.
-Received data from the serial port is echoed back to the same port
-provided by this driver.
-This sample can be found under :zephyr_file:`samples/subsys/usb/cdc_acm` in the
+This sample app demonstrates the use of USB Media Transfer Protocol (MTP)
+driver provided by the Zephyr project. It allows the device to be mounted
+as a media device on the host system, enabling file transfers between the
+host and device.
+This sample can be found under :zephyr_file:`samples/subsys/usb/mtp` in the
 Zephyr project tree.
 
 Requirements
 ************
 
-This project requires an USB device driver, which is available for multiple
-boards supported in Zephyr.
+This project requires:
+- USB device driver support
+- Storage media with Littlefs/FAT filesystem support
 
 Building and Running
 ********************
 
 Reel Board
 ===========
 
-To see the console output of the app, open a serial port emulator and
-attach it to the USB to TTL Serial cable. Build and flash the project:
+Build and flash the project:
 
 .. zephyr-app-commands::
-   :zephyr-app: samples/subsys/usb/cdc_acm
+   :zephyr-app: samples/subsys/usb/mtp
    :board: reel_board
    :goals: flash
    :compact:
@@ -46,53 +46,34 @@ The board will be detected as shown by the Linux dmesg command:
    usb 9-1: new full-speed USB device number 112 using uhci_hcd
    usb 9-1: New USB device found, idVendor=8086, idProduct=f8a1
    usb 9-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
-   usb 9-1: Product: CDC-ACM
-   usb 9-1: Manufacturer: Intel
-   usb 9-1: SerialNumber: 00.01
-   cdc_acm 9-1:1.0: ttyACM1: USB ACM device
+   usb 9-1: Product: Zephyr MTP
+   usb 9-1: Manufacturer: ZEPHYR
+   usb 9-1: SerialNumber: 0123456789AB
 
-The app prints on serial output (UART1), used for the console:
+Once connected, the device should appear as a media device on your host system.
+You can then:
+- Browse the device storage
+- Transfer files to/from the device
+- Create/delete directories
+- Manage files on the device
 
-.. code-block:: console
-
-   Wait for DTR
-
-Open a serial port emulator, for example minicom
-and attach it to detected CDC ACM device:
-
-.. code-block:: console
-
-   minicom --device /dev/ttyACM1
-
-The app should respond on serial output with:
-
-.. code-block:: console
-
-   DTR set, start test
-   Baudrate detected: 115200
-
-And on ttyACM device, provided by zephyr USB device stack:
-
-.. code-block:: console
+The storage contents will persist across device reboots as long as the
+filesystem is properly unmounted before disconnecting or resetting the device.
 
-   Send characters to the UART device
-   Characters read:
+Troubleshooting
+==============
 
-The characters entered in serial port emulator will be echoed back.
+If the device is not recognized properly:
 
-Troubleshooting
-===============
+1. Ensure the storage medium is properly initialized
+2. Check that the filesystem is mounted correctly
+3. Verify USB configuration is correct for your board
+4. Some systems may require additional MTP device support to be installed
 
-If the ModemManager runs on your operating system, it will try
-to access the CDC ACM device and maybe you can see several characters
-including "AT" on the terminal attached to the CDC ACM device.
-You can add or extend the udev rule for your board to inform
-ModemManager to skip the CDC ACM device.
-For this example, it would look like this:
+For debugging purposes, you can enable USB debug logs by setting the following
+in your project's configuration:
 
 .. code-block:: none
 
-   ATTRS{idVendor}=="8086" ATTRS{idProduct}=="f8a1", ENV{ID_MM_DEVICE_IGNORE}="1"
-
-You can use
-``/lib/udev/rules.d/77-mm-usb-device-blacklist.rules`` as reference.
+   CONFIG_USB_DEVICE_LOG_LEVEL_DBG=y
+   CONFIG_USB_MTP_LOG_LEVEL_DBG=y

samples/subsys/usb/mtp/boards/stm32f769i_disco.overlay

@@ -1,25 +1,27 @@
+/*
+ * Copyright (c) 2025 Mohamed ElShahawi (extremegtx@hotmail.com)
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
 
 /delete-node/ &storage_partition;
 
+&mx25l51245g {
+	partitions {
+		compatible = "fixed-partitions";
+		#address-cells = <1>;
+		#size-cells = <1>;
 
-	&mx25l51245g {
-		partitions {
-			compatible = "fixed-partitions";
-			#address-cells = <1>;
-			#size-cells = <1>;
-
-			storage_partition: partition@1a0000 {
-				label = "storage";
-				reg = <0x001a0000 DT_SIZE_M(40)>;
-			};
-			storage_partition2: partition@2ba0000 {
-				label = "storage2";
-				reg = <0x2ba0000 DT_SIZE_M(20)>;
-			};
+		storage_partition: partition@1a0000 {
+			label = "storage";
+			reg = <0x001a0000 DT_SIZE_M(40)>;
+		};
+		storage_partition2: partition@2ba0000 {
+			label = "storage2";
+			reg = <0x2ba0000 DT_SIZE_M(20)>;
 		};
 	};
-
-
+};
 
 / {
 	fstab {
@@ -48,12 +50,5 @@
 			automount;
 			mtp-enabled;
 		};
-// Not Working
-//		sd0: sd0 {
-//			compatible = "zephyr,fstab-common";
-//			partition = <&sdmmc2>;
-//			mount-point = "/SD:";
-//			automount;
-//		};
 	};
 };

samples/subsys/usb/mtp/prj.conf

@@ -8,11 +8,6 @@ CONFIG_LOG=y
 CONFIG_SAMPLE_USBD_PRODUCT="USBD MTP sample"
 CONFIG_SAMPLE_USBD_PID=0x0009
 
-
-
-
-
-
 CONFIG_USB_DEVICE_INITIALIZE_AT_BOOT=n
 
 CONFIG_FLASH=y
@@ -22,40 +17,22 @@ CONFIG_MAIN_STACK_SIZE=2048
 CONFIG_FILE_SYSTEM=y
 CONFIG_FILE_SYSTEM_MKFS=y
 CONFIG_FILE_SYSTEM_LITTLEFS=y
-#CONFIG_FS_LITTLEFS_FMP_DEV=y
 
 CONFIG_SHELL=y
 CONFIG_FILE_SYSTEM_SHELL=y
 CONFIG_HEAP_MEM_POOL_SIZE=16384
 
-#CONFIG_DISK_DRIVERS=y
-#CONFIG_DISK_ACCESS=y
-#CONFIG_DISK_DRIVER_FLASH=y
-
 #CONFIG_USB_DEVICE_LOG_LEVEL_DBG=y
 CONFIG_LOG_PROCESS_TRIGGER_THRESHOLD=5
 CONFIG_LOG_BUFFER_SIZE=65536
 
 CONFIG_USBD_LOG_LEVEL_WRN=y
 #CONFIG_USBD_LOG_LEVEL_DBG=y
 
-#CONFIG_SAMPLE_USBD_PID=0x6860
-#CONFIG_SAMPLE_USBD_PRODUCT="SAMSUNG_Android"
-#CONFIG_SAMPLE_USBD_MANUFACTURER="SAMSUNG"
-
-
-#SDCARD
-CONFIG_DISK_ACCESS=y
-CONFIG_DISK_DRIVER_SDMMC=y
-CONFIG_SDMMC_STM32=y
-CONFIG_SDMMC_SUBSYS=y
-
-
 CONFIG_FAT_FILESYSTEM_ELM=y
 
 CONFIG_USBD_THREAD_STACK_SIZE=2048
 
-
 CONFIG_OUTPUT_DISASSEMBLY=y
 CONFIG_OUTPUT_DISASSEMBLY_WITH_SOURCE=y
 CONFIG_FLASH_SHELL=y

subsys/usb/device_next/CMakeLists.txt

@@ -102,7 +102,7 @@ zephyr_linker_sources_ifdef(
 zephyr_library_sources_ifdef(
 	CONFIG_USBD_MTP_CLASS
 	class/usbd_mtp.c
-	class/usbd_mtp_impl.c
+	class/usbd_mtp_class.c
 )
 
 zephyr_linker_sources(DATA_SECTIONS usbd_data.ld)

subsys/usb/device_next/class/usbd_mtp.c

@@ -12,10 +12,6 @@
 #include <zephyr/logging/log.h>
 LOG_MODULE_REGISTER(usb_mtp, CONFIG_USBD_MTP_LOG_LEVEL);
 
-#define MTP_RESPONSE_OK          0x2001
-#define MTP_RESPONSE_DEVICE_BUSY 0x2019
-
-
 /* Endpoint addresses */
 #define MTP_IN_EP_ADDR   0x81 /* Bulk IN */
 #define MTP_OUT_EP_ADDR  0x01 /* Bulk OUT */
@@ -290,26 +286,31 @@ static int usbd_mtp_init(struct usbd_class_data *c_data)
 	LOG_INF("Init class instance %p", c_data);
 
 	char *manufacturer, *model, *device_version, *serial_number;
-	struct usb_device_descriptor *hs_desc =
-		(struct usb_device_descriptor *)c_data->uds_ctx->hs_desc;
+	struct usb_device_descriptor *usbd_desc;
 	struct usbd_desc_node *d_nd;
 
-	d_nd = usbd_get_descriptor(c_data->uds_ctx, USB_DESC_STRING, hs_desc->iManufacturer);
+	if (usbd_bus_speed(c_data->uds_ctx) == USBD_SPEED_HS) {
+		usbd_desc = (struct usb_device_descriptor *)c_data->uds_ctx->hs_desc;
+	} else {
+		usbd_desc = (struct usb_device_descriptor *)c_data->uds_ctx->fs_desc;
+	}
+
+	d_nd = usbd_get_descriptor(c_data->uds_ctx, USB_DESC_STRING, usbd_desc->iManufacturer);
 	manufacturer = (char *)d_nd->ptr;
 
-	d_nd = usbd_get_descriptor(c_data->uds_ctx, USB_DESC_STRING, hs_desc->iProduct);
+	d_nd = usbd_get_descriptor(c_data->uds_ctx, USB_DESC_STRING, usbd_desc->iProduct);
 	model = (char *)d_nd->ptr;
 
-	d_nd = usbd_get_descriptor(c_data->uds_ctx, USB_DESC_STRING, hs_desc->iSerialNumber);
+	d_nd = usbd_get_descriptor(c_data->uds_ctx, USB_DESC_STRING, usbd_desc->iSerialNumber);
 	serial_number = (char *)d_nd->ptr;
 
 	device_version = "1.0";
 
-	LOG_DBG("Manufacturer: %s", manufacturer);
-	LOG_DBG("Product: %s", model);
-	if (d_nd->bLength > 0) {
-		LOG_DBG("Serial Number: %s", serial_number);
-	}
+	LOG_DBG("Desc data: Manufacturer: %s, Product: %s, SN: %s",
+			manufacturer,
+			model,
+			d_nd->bLength > 0 ? serial_number : "NULL");
+
 	mtp_init(manufacturer, model, device_version, serial_number);
 
 	return 0;

subsys/usb/device_next/class/usbd_mtp_class.c

@@ -910,7 +910,7 @@ static int continue_get_object(struct net_buf *buf)
 			set_pending_packet(continue_get_object);
 		}
 	} else {
-		LOG_ERR("shouldn't happen !!!!!!!!!");
+		LOG_ERR("shouldn't happen !");
 	}
 	return 0;
 }
@@ -1296,7 +1296,15 @@ int mtp_commands_handler(struct net_buf *buf_in, struct net_buf *buf)
 		MTP_CMD(MTP_OP_GET_OBJECT_REFERENCES);
 		break;
 	default:
-		LOG_ERR("Unknown cmd 0x%x!", mtp_command->hdr.code);
+		LOG_ERR("Not supported cmd 0x%x!", mtp_command->hdr.code);
+
+		struct mtp_header mtp_response = {
+			.length = sizeof(struct mtp_header),
+			.type = MTP_CONTAINER_RESPONSE,
+			.code = MTP_RESP_OPERATION_NOT_SUPPORTED,
+			.transaction_id = mtp_command->hdr.transaction_id };
+
+		net_buf_add_mem(buf, &mtp_response, sizeof(struct mtp_header));
 		break;
 	}
 
@@ -1389,15 +1397,12 @@ int mtp_init(const char *manufacturer, const char *model, const char *device_ver
 	/* Zephyr set Serial Number descriptor after MTP init, so for now use this one */
 	dev_info.serial_number = "0123456789ABCDEF";
 
-	for (int i = 1; i < ARRAY_SIZE(storage); i++) {
-		memset(storage[i].filelist, 0x00, sizeof(storage[i].filelist));
-		storage[i].files_count = 0;
-	}
-	mtp_ctx.session_opened = false;
+	mtp_reset();
 
 	return 0;
 }
 
+#if CONFIG_SHELL
 static int cmd_mtp_list(const struct shell *sh, size_t argc, char **argv)
 {
 	for (int storageIdx = 1; storageIdx < ARRAY_SIZE(storage); storageIdx++) {
@@ -1422,3 +1427,4 @@ SHELL_STATIC_SUBCMD_SET_CREATE(sub_mtp,
 			       SHELL_SUBCMD_SET_END);
 
 SHELL_CMD_REGISTER(mtp, &sub_mtp, "USB MTP commands", NULL);
+#endif

tests/subsys/usb/device_next/build_all.conf

@@ -22,6 +22,7 @@ CONFIG_UART_LINE_CTRL=y
 CONFIG_USBD_CDC_ACM_CLASS=y
 
 CONFIG_USBD_MSC_CLASS=y
+CONFIG_USBD_MTP_CLASS=y
 
 CONFIG_USBD_AUDIO2_CLASS=y