-
Notifications
You must be signed in to change notification settings - Fork 7.4k
usb: device_next: Add USB MTP class support #86832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Dear Reviewers: Could you please help me validate the current architecture first, then we can move to final polishing when I get the PR out of draft state. Thanks a lot. |
7874f45
to
748eb7e
Compare
f6bfa5e
to
02a2fb9
Compare
The class fixup/polishing commits should be squashed together into the main MTP class implementation commit. Sample changes should go into separate commits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements basic USB MTP class support to enable file and directory transfers between a device and a host. Key changes include:
- Adding a new MTP class header with necessary function declarations and context structure.
- Providing a sample application to test and demonstrate the MTP functionality.
- Updating USB descriptors, DTS bindings, and sample configuration to support MTP.
Reviewed Changes
Copilot reviewed 7 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
subsys/usb/device_next/class/usbd_mtp_class.h | Introduces declarations for the USB MTP class support |
samples/subsys/usb/mtp/src/main.c | Implements a sample application to test the MTP class driver |
samples/subsys/usb/mtp/sample.yaml | Adds a sample configuration for the MTP device class |
include/zephyr/usb/usb_ch9.h | Updates USB descriptors to include the MTP image class type |
dts/bindings/fs/zephyr,fstab-common.yaml | Adds a filesystem binding property for enabling MTP file access |
Files not reviewed (11)
- samples/subsys/usb/mtp/CMakeLists.txt: Language not supported
- samples/subsys/usb/mtp/Kconfig: Language not supported
- samples/subsys/usb/mtp/README.rst: Language not supported
- samples/subsys/usb/mtp/boards/rpi_pico.overlay: Language not supported
- samples/subsys/usb/mtp/boards/stm32f769i_disco.overlay: Language not supported
- samples/subsys/usb/mtp/prj.conf: Language not supported
- subsys/usb/device_next/CMakeLists.txt: Language not supported
- subsys/usb/device_next/class/Kconfig: Language not supported
- subsys/usb/device_next/class/Kconfig.mtp: Language not supported
- tests/subsys/usb/device_next/build_all.conf: Language not supported
- tests/subsys/usb/device_next/build_all.overlay: Language not supported
6694e76
to
ce9c991
Compare
3bb7378
to
9910433
Compare
9910433
to
11c91a9
Compare
11c91a9
to
21be132
Compare
It does not seem to show up on Win11, at least for me. Can someone else see if it works on Win11 for them? I get the following: And USB Device Tree Viewer v4.5.0 tells me: But nothing shows up on the "This PC" tab. My phone does show up there and also uses MTP to my knowledge. I get the following Windows Events in respect to the MTP device:
Then, the device status is The device log says nothing spectacular: (private data removed)
Any idea? Did somebody get this to work in the past on Win11? EDIT: I cannot see it on my Ubuntu VM either; this is my dmesg log:
(Note: My device also has two 2x CDC ACM) |
@Finomnis Could you please share the |
@ExtremeGTX can't right now, as it is a commercial product. Will try to reproduce on a dev board and then share. |
Here are two logfiles, one for the working and one for the broken version: |
Another bug report: copying text files to and from the device seems to truncate them to 52 bytes. |
Implement a basic version of USB MTP (Media transfer protocol) class which support the necessary MTP commands to handle Dir/file transfer between a device and host. Signed-off-by: Mohamed ElShahawi <[email protected]>
Add a sample application that demonstrates how to use the new USB MTP device class. This shows how to set up the device tree to expose storage partitions to a host over USB using MTP. Files/Dirs can be accessed from host without host supportfor the underlying filesystem like littlefs. Signed-off-by: Mohamed ElShahawi <[email protected]>
21be132
to
12534fc
Compare
Could you please share the logs with Above tests done while one CDC_ACM instance is enabled in addition to MTP on USB. Thanks for testing and reporting back :) |
|
Sure :) I compiled commit Those are the files I had to modify:
Then I compiled and ran with
Like previously mentioned, Here is the log file: (I annotated where I performed an action) annotated_log_file_round_trip.txt
Note also that the modification time of the file broke in the process: |
Another small insight: When enabling the shell by doing:
And then running
Then the file is there and complete. So the problem must occur during transfer from device to PC. |
Another bugreport: when connecting the device to the PC, one needs to enter the device directory in the explorer twice before the file shows up. When entering the first time, the directory shows as empty. |
I gave your working branch a go on a NXP FRDM RW612 board today, few quirks...
I had to bump the main stack size up to get it to boot, lfs format would fault otherwise. On Ubuntu 24.04 I had it crashing immediately on plugging in, after a quick debugging session I found that in Once enumerated it appeared in my file explorer and somewhat worked, I could browse but writing would cause a crash and leave empty files on the FS. Testing the same setup on windows 10 had a little more success the files written had some data at the start but they were still incomplete (no crashes though). This is a very good start, but certainly still need a lot of testing and polishing 😄 I will continue to watch this space and test occasionally, very keen to see this feature make it into main. p.s. I noted the partial file we copied was also exactly 52 bytes long like the @Finomnis 's test file p.p.s I think your sample needs |
I tried to reproduce on both boards STM32F769i-DISCO and RPI Pico on Windows 10 and 11 using the file attached by @Finomnis and both are working fine, I even tested on another PC which both boards never attached to it before. Although Teensy 4.0 and FRDM RW612 are different boards, both still use the same NXP UDC Driver, so for me it looks like it is either a problem with NXP UDC Driver or there is an edge case (triggered by NXP UDC) not covered in MTP Implementation, because from the logs I see that communication just stops after first packet which includes the necessary MTP headers + only (52 Bytes of data). I might have access to one FRDM board and a Nordic board in the next week or the week after, will double check and report back here. Thanks a lot for your efforts in verifying the implementation, appreciate your support :) |
@Finomnis regarding file date issue, it is because it will require dependency on Hardware RTC which is a dependency didn't want to have (initially at least, later after stabilizing the implementation we can have a Kconfig to enable the integration) or File system supports storing file properties which is not available in a FS like Littlefs. Edit: Typo fix |
I get that, no problem. Regarding the other issue... I might try to improve the current implementation, but not within the next weeks, I'm busy with life. But later I might join your efforts. |
This morning I tried on a |
Hi @dleach02 and @mmahadevan108, Thanks in advance :) |
Implement a basic version of USB MTP (Media transfer protocol) class which support
the necessary MTP commands to handle Dir/file transfer between a device and host.
Fixes: #54468
TODO:
MAX_PACKET_SIZE
value dependent on whether USB is HS/FSThanks:
device_next
works