Skip to content
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

disco f769: USB mass storage device is broken #15129

Closed
Jookia opened this issue Oct 6, 2021 · 12 comments
Closed

disco f769: USB mass storage device is broken #15129

Jookia opened this issue Oct 6, 2021 · 12 comments

Comments

@Jookia
Copy link
Contributor

Jookia commented Oct 6, 2021

Description of defect

Using the USB mass storage device example does not work.
Reading data fails and causes Linux to reset loop the device hoping it will work.

Target(s) affected by this defect ?

DISCO_F769NI

Toolchain(s) (name and version) displaying this defect ?

GCC_ARM

What version of Mbed-os are you using (tag or sha) ?

9dd6fb4 with #15128 applied.

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

mbed-cli 1.10.5
gcc 11.2.0
Linux 5.14.6

How is this defect reproduced ?

Build and run the USB mass storage example.
Wireshark supports looking at Linux USB data, which roughly shows this:

No.	Time	Source	Destination	Protocol	Length	Info
31925	492.662852	host	1.5.1	USBMS	95	SCSI: Read(10) LUN: 0x00 (LBA: 0x00000000, Len: 8)
Frame 31925: 95 bytes on wire (760 bits), 95 bytes captured (760 bits) on interface usbmon1, id 0
    Interface id: 0 (usbmon1)
    Encapsulation type: USB packets with Linux header and padding (115)
    Arrival Time: Oct  6, 2021 22:11:10.230344000 AEDT
    [Time shift for this packet: 0.000000000 seconds]
    Epoch Time: 1633518670.230344000 seconds
    [Time delta from previous captured frame: 0.006633000 seconds]
    [Time delta from previous displayed frame: 0.007306000 seconds]
    [Time since reference or first frame: 492.662852000 seconds]
    Frame Number: 31925
    Frame Length: 95 bytes (760 bits)
    Capture Length: 95 bytes (760 bits)
    [Frame is marked: False]
    [Frame is ignored: False]
    [Protocols in frame: usb:usbms]
USB URB
    [Source: host]
    [Destination: 1.5.1]
    URB id: 0xffff98dc1327ef00
    URB type: URB_SUBMIT ('S')
    URB transfer type: URB_BULK (0x03)
    Endpoint: 0x01, Direction: OUT
    Device: 5
    URB bus id: 1
    Device setup request: not relevant ('-')
    Data: present (0)
    URB sec: 1633518670
    URB usec: 230344
    URB status: Operation now in progress (-EINPROGRESS) (-115)
    URB length [bytes]: 31
    Data length [bytes]: 31
    [Response in: 31926]
    [bInterfaceClass: Mass Storage (0x08)]
    Unused Setup Header
    Interval: 0
    Start frame: 0
    Copy of Transfer Flags: 0x00000004, No transfer DMA map
    Number of ISO descriptors: 0
USB Mass Storage
    Signature: 0x43425355
    Tag: 0x00000011
    DataTransferLength: 4096
    Flags: 0x80
    .000 .... = Target: 0x0 (0)
    .... 0000 = LUN: 0x0
    ...0 1010 = CDB Length: 0x0a
SCSI CDB Read(10)
    [LUN: 0x0000]
    [Command Set:Direct Access Device (0x00) ]
    Opcode: Read(10) (0x28)
    Flags: 0x00
        000. .... = RDPROTECT: 0x0
        ...0 .... = DPO: Disable page out is DISABLED (cache this data)
        .... 0... = FUA: Read from cache if possible
        .... ..0. = FUA_NV: Read from volatile or non-volatile cache permitted
    Logical Block Address (LBA): 0
    ...0 0000 = Group: 0x00
    Transfer Length: 8
    Control: 0x00
        00.. .... = Vendor specific: 0x0
        ..00 0... = Reserved: 0x0
        .... .0.. = NACA: Normal ACA is not set
        .... ..0. = Obsolete: 0x0
        .... ...0 = Obsolete: 0x0

31926	492.662872	1.5.1	host	USB	64	URB_BULK out
Frame 31926: 64 bytes on wire (512 bits), 64 bytes captured (512 bits) on interface usbmon1, id 0
    Interface id: 0 (usbmon1)
    Encapsulation type: USB packets with Linux header and padding (115)
    Arrival Time: Oct  6, 2021 22:11:10.230364000 AEDT
    [Time shift for this packet: 0.000000000 seconds]
    Epoch Time: 1633518670.230364000 seconds
    [Time delta from previous captured frame: 0.000020000 seconds]
    [Time delta from previous displayed frame: 0.000020000 seconds]
    [Time since reference or first frame: 492.662872000 seconds]
    Frame Number: 31926
    Frame Length: 64 bytes (512 bits)
    Capture Length: 64 bytes (512 bits)
    [Frame is marked: False]
    [Frame is ignored: False]
    [Protocols in frame: usb]
USB URB
    [Source: 1.5.1]
    [Destination: host]
    URB id: 0xffff98dc1327ef00
    URB type: URB_COMPLETE ('C')
    URB transfer type: URB_BULK (0x03)
    Endpoint: 0x01, Direction: OUT
    Device: 5
    URB bus id: 1
    Device setup request: not relevant ('-')
    Data: not present ('>')
    URB sec: 1633518670
    URB usec: 230364
    URB status: Success (0)
    URB length [bytes]: 31
    Data length [bytes]: 0
    [Request in: 31925]
    [Time from request: 0.000020000 seconds]
    [bInterfaceClass: Mass Storage (0x08)]
    Unused Setup Header
    Interval: 0
    Start frame: 0
    Copy of Transfer Flags: 0x00000004, No transfer DMA map
    Number of ISO descriptors: 0

31927	492.662881	host	1.5.1	USB	64	URB_BULK in
Frame 31927: 64 bytes on wire (512 bits), 64 bytes captured (512 bits) on interface usbmon1, id 0
    Interface id: 0 (usbmon1)
    Encapsulation type: USB packets with Linux header and padding (115)
    Arrival Time: Oct  6, 2021 22:11:10.230373000 AEDT
    [Time shift for this packet: 0.000000000 seconds]
    Epoch Time: 1633518670.230373000 seconds
    [Time delta from previous captured frame: 0.000009000 seconds]
    [Time delta from previous displayed frame: 0.000009000 seconds]
    [Time since reference or first frame: 492.662881000 seconds]
    Frame Number: 31927
    Frame Length: 64 bytes (512 bits)
    Capture Length: 64 bytes (512 bits)
    [Frame is marked: False]
    [Frame is ignored: False]
    [Protocols in frame: usb]
USB URB
    [Source: host]
    [Destination: 1.5.1]
    URB id: 0xffff98dcd5435e00
    URB type: URB_SUBMIT ('S')
    URB transfer type: URB_BULK (0x03)
    Endpoint: 0x81, Direction: IN
    Device: 5
    URB bus id: 1
    Device setup request: not relevant ('-')
    Data: not present ('<')
    URB sec: 1633518670
    URB usec: 230373
    URB status: Operation now in progress (-EINPROGRESS) (-115)
    URB length [bytes]: 4096
    Data length [bytes]: 0
    [Response in: 32250]
    [bInterfaceClass: Mass Storage (0x08)]
    Unused Setup Header
    Interval: 0
    Start frame: 0
    Copy of Transfer Flags: 0x00000201, Short not OK, Dir IN
    Number of ISO descriptors: 0

32250	492.831667	1.5.1	host	USBMS	128	SCSI: Data In LUN: 0x00 (Read(10) Response Data) 
Frame 32250: 128 bytes on wire (1024 bits), 128 bytes captured (1024 bits) on interface usbmon1, id 0
    Interface id: 0 (usbmon1)
    Encapsulation type: USB packets with Linux header and padding (115)
    Arrival Time: Oct  6, 2021 22:11:10.399159000 AEDT
    [Time shift for this packet: 0.000000000 seconds]
    Epoch Time: 1633518670.399159000 seconds
    [Time delta from previous captured frame: 0.000430000 seconds]
    [Time delta from previous displayed frame: 0.168786000 seconds]
    [Time since reference or first frame: 492.831667000 seconds]
    Frame Number: 32250
    Frame Length: 128 bytes (1024 bits)
    Capture Length: 128 bytes (1024 bits)
    [Frame is marked: False]
    [Frame is ignored: False]
    [Protocols in frame: usb:usbms]
USB URB
    [Source: 1.5.1]
    [Destination: host]
    URB id: 0xffff98dcd5435e00
    URB type: URB_COMPLETE ('C')
    URB transfer type: URB_BULK (0x03)
    Endpoint: 0x81, Direction: IN
    Device: 5
    URB bus id: 1
    Device setup request: not relevant ('-')
    Data: present (0)
    URB sec: 1633518670
    URB usec: 399159
    URB status: Remote I/O error (-EREMOTEIO) (-121)
    URB length [bytes]: 64
    Data length [bytes]: 64
    [Request in: 31927]
    [Time from request: 0.168786000 seconds]
    [bInterfaceClass: Mass Storage (0x08)]
    Unused Setup Header
    Interval: 0
    Start frame: 0
    Copy of Transfer Flags: 0x00000201, Short not OK, Dir IN
    Number of ISO descriptors: 0
USB Mass Storage
SCSI Payload (Read(10) Response Data)
    [LUN: 0x0000]
    [Command Set:Direct Access Device (0x00) ]
    [SBC Opcode: Read(10) (0x28)]
    [Request in: 31925]

32251	492.831676	host	1.5.1	USB	64	URB_BULK in
Frame 32251: 64 bytes on wire (512 bits), 64 bytes captured (512 bits) on interface usbmon1, id 0
    Interface id: 0 (usbmon1)
    Encapsulation type: USB packets with Linux header and padding (115)
    Arrival Time: Oct  6, 2021 22:11:10.399168000 AEDT
    [Time shift for this packet: 0.000000000 seconds]
    Epoch Time: 1633518670.399168000 seconds
    [Time delta from previous captured frame: 0.000009000 seconds]
    [Time delta from previous displayed frame: 0.000009000 seconds]
    [Time since reference or first frame: 492.831676000 seconds]
    Frame Number: 32251
    Frame Length: 64 bytes (512 bits)
    Capture Length: 64 bytes (512 bits)
    [Frame is marked: False]
    [Frame is ignored: False]
    [Protocols in frame: usb]
USB URB
    [Source: host]
    [Destination: 1.5.1]
    URB id: 0xffff98dc1327ef00
    URB type: URB_SUBMIT ('S')
    URB transfer type: URB_BULK (0x03)
    Endpoint: 0x81, Direction: IN
    Device: 5
    URB bus id: 1
    Device setup request: not relevant ('-')
    Data: not present ('<')
    URB sec: 1633518670
    URB usec: 399168
    URB status: Operation now in progress (-EINPROGRESS) (-115)
    URB length [bytes]: 13
    Data length [bytes]: 0
    [Response in: 32728]
    [bInterfaceClass: Mass Storage (0x08)]
    Unused Setup Header
    Interval: 0
    Start frame: 0
    Copy of Transfer Flags: 0x00000204, No transfer DMA map, Dir IN
    Number of ISO descriptors: 0

32728	493.079826	1.5.1	host	USB	64	URB_BULK in
Frame 32728: 64 bytes on wire (512 bits), 64 bytes captured (512 bits) on interface usbmon1, id 0
    Interface id: 0 (usbmon1)
    Encapsulation type: USB packets with Linux header and padding (115)
    Arrival Time: Oct  6, 2021 22:11:10.647318000 AEDT
    [Time shift for this packet: 0.000000000 seconds]
    Epoch Time: 1633518670.647318000 seconds
    [Time delta from previous captured frame: 0.000561000 seconds]
    [Time delta from previous displayed frame: 0.248150000 seconds]
    [Time since reference or first frame: 493.079826000 seconds]
    Frame Number: 32728
    Frame Length: 64 bytes (512 bits)
    Capture Length: 64 bytes (512 bits)
    [Frame is marked: False]
    [Frame is ignored: False]
    [Protocols in frame: usb]
USB URB
    [Source: 1.5.1]
    [Destination: host]
    URB id: 0xffff98dc1327ef00
    URB type: URB_COMPLETE ('C')
    URB transfer type: URB_BULK (0x03)
    Endpoint: 0x81, Direction: IN
    Device: 5
    URB bus id: 1
    Device setup request: not relevant ('-')
    Data: present (0)
    URB sec: 1633518670
    URB usec: 647318
    URB status: Value too large for defined data type (-EOVERFLOW) (-75)
    URB length [bytes]: 0
    Data length [bytes]: 0
    [Request in: 32251]
    [Time from request: 0.248150000 seconds]
    [bInterfaceClass: Mass Storage (0x08)]
    Unused Setup Header
    Interval: 0
    Start frame: 0
    Copy of Transfer Flags: 0x00000204, No transfer DMA map, Dir IN
    Number of ISO descriptors: 0
@0xc0170 0xc0170 changed the title USB mass storage device is broken disco f769: USB mass storage device is broken Oct 6, 2021
@Jookia
Copy link
Contributor Author

Jookia commented Oct 6, 2021

I found something that seems to work:

diff --git a/drivers/usb/include/usb/USBMSD.h b/drivers/usb/include/usb/USBMSD.h
index a570df7d82..b516f918c1 100644
--- a/drivers/usb/include/usb/USBMSD.h
+++ b/drivers/usb/include/usb/USBMSD.h
@@ -249,8 +249,8 @@ private:
     // endpoints
     usb_ep_t _bulk_in;
     usb_ep_t _bulk_out;
-    uint8_t _bulk_in_buf[64];
-    uint8_t _bulk_out_buf[64];
+    uint8_t _bulk_in_buf[512];
+    uint8_t _bulk_out_buf[512];
     bool _out_ready;
     bool _in_ready;
     uint32_t _bulk_out_size;
diff --git a/drivers/usb/source/USBDevice.cpp b/drivers/usb/source/USBDevice.cpp
index 6d3cfc87d4..0181561d8c 100644
--- a/drivers/usb/source/USBDevice.cpp
+++ b/drivers/usb/source/USBDevice.cpp
@@ -53,7 +53,7 @@
 #if defined(MAX_PACKET_SIZE_EP0)
 #undef MAX_PACKET_SIZE_EP0
 #endif
-#define MAX_PACKET_SIZE_EP0 64
+#define MAX_PACKET_SIZE_EP0 512
 
 #define USB_MIN(a, b) ((a) > (b) ? (b) : (a))
 
diff --git a/drivers/usb/source/USBMSD.cpp b/drivers/usb/source/USBMSD.cpp
index fcf416c823..c5cde22724 100644
--- a/drivers/usb/source/USBMSD.cpp
+++ b/drivers/usb/source/USBMSD.cpp
@@ -56,7 +56,7 @@
 #define DEFAULT_CONFIGURATION (1)
 
 // max packet size
-#define MAX_PACKET  64
+#define MAX_PACKET  512
 
 // CSW Status
 enum Status {

It looks like the HS OTG code is setting up a max packet size of 512 while this code expects 64.

Edit: The max packet size for EP0 might be unneccesary, not sure.

@Jookia
Copy link
Contributor Author

Jookia commented Oct 6, 2021

I'm not sure about the right way to actually update this since changing the packet size to 512 like this will break it on platforms that have a max size of 64.

@jeromecoutant
Copy link
Collaborator

jeromecoutant commented Oct 6, 2021

We should create something like:
#define USB_HIGH_SPEED_SUPPORT
?
@0xc0170

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 6, 2021

we don't support high speed, it would require more work as I was told.

All Host Controllers are required to have support for 8-, 16-, 32-, and 64-byte maximum packet sizes for
full-speed bulk endpoints and 512 bytes for high-speed bulk endpoints. No Host Controller is required to
support larger or smaller maximum packet sizes.

@Jookia
Copy link
Contributor Author

Jookia commented Oct 7, 2021 via email

@jeromecoutant
Copy link
Collaborator

we don't support high speed, it would require more work as I was told.

@facchinm do you have any tip ?
Thx

@facchinm
Copy link
Contributor

facchinm commented Oct 7, 2021

Our lite-fork of USBMSD library supports both, probably a similar patch could work for mainline too
https://github.com/arduino/ArduinoCore-mbed/blob/1f87e87dc5775ca177375d71a491cb3c801b3036/libraries/USBMSD/PluggableUSBMSD.h#L38-L43

@jeromecoutant
Copy link
Collaborator

and you don't need patches like 7 comments above ?

#15129 (comment)

@facchinm
Copy link
Contributor

facchinm commented Oct 7, 2021

Yes, the link was just the tip of the iceberg 🙂 The define is then applied throughout all the other files

@Jookia
Copy link
Contributor Author

Jookia commented Oct 7, 2021

So @facchinm has already come to the proper solution by conditionally setting MSD_MAX_PACKET_SIZE ?

If that seems like a decent enough solution (and it does actually fix things), I'll be happy to do a PR for it.

@jeromecoutant
Copy link
Collaborator

If that seems like a decent enough solution (and it does actually fix things), I'll be happy to do a PR for it.

Maybe you can start with this kind of patch ?
jeromecoutant@33f9c9c

@0xc0170
Copy link
Contributor

0xc0170 commented Jul 22, 2022

As it's been almost a year without any update, I'll close this as won't fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants