USBHost_t36 USB Mass Storage Driver Experiments

I have sorted out a few issues that I was seeing with CSW tag errors. I wouldn't be surprised if you are running into the same issues I was seeing where the code wouldn't reload Command wrapper.tag, instead just using the value stored in a register.
 
I should also mention that some of these kinds of issues will disappear when printing debugging information as that often forces the code to reload the value, so make sure you also test without debugging enabled.
 
wwatson said:
Just press enter key to repeat. It does a hex dump of the buffer and then clears the buffer and zeros the sector. I have not gone any further with testing. I think the problems might be initialization.
Please test if you can.
Click to expand...

Gave your sketch a try using the UAM46 drive and as advertised it worked but I had to reformat the disk first.

Also went back to the Wireshark Windows dump and looked at the startup for the drive on windows did notice a couple of scsi cmds setting the drive as direct access etc. 0xa2 and 0x23 but not sure that matters
scsiCMDa2.jpg

scsiCMD23.jpg
 
wwatson said:
@KurtE @mjs513 - A little progress:) I have stooped to single sector reads and writes. Here is a sketch that appears to be working:

Just press enter key to repeat. It does a hex dump of the buffer and then clears the buffer and zeros the sector. I have not gone any further with testing. I think the problems might be initialization.
Please test if you can.
Click to expand...

Code: 
Writing: 'this is a test.' to sector: 1000000
msWriteBlocks()
msDoCommand()
USBDrive CallbackOut (static)
transfer->qtd.token = 0
USBDrive dataOut (static)31
55 53 42 43 09 00 00 00 00 02 00 00 00 00 0A 2A 00 00 0F 42 40 00 00 01 00 00 00 00 00 00 00 
USBDrive CallbackOut (static)
transfer->qtd.token = 0
USBDrive dataOut (static)512
74 68 69 73 20 69 73 20 61 20 74 65 73 74 2E 00 72 73 6C 74 20 3D 20 25 64 0A 00 00 0A 52 65 61 
msGetCSW()
USBDrive CallbackIn (static)
transfer->qtd.token = 0
USBDrive dataIn (static): 13
55 53 42 53 09 00 00 00 00 00 00 00 00 
msProcessError()
rslt = 0

Reading sector:10000000 to cbuff
msReadBlocks()
msDoCommand()
USBDrive CallbackOut (static)
transfer->qtd.token = 0
USBDrive dataOut (static)31
55 53 42 43 0A 00 00 00 00 02 00 00 80 00 0A 28 00 00 0F 42 40 00 00 01 00 00 00 00 00 00 00 
USBDrive CallbackIn (static)
transfer->qtd.token = 0
USBDrive dataIn (static): 512
74 68 69 73 20 69 73 20 61 20 74 65 73 74 2E 00 72 73 6C 74 20 3D 20 25 64 0A 00 00 0A 52 65 61 
msGetCSW()
USBDrive CallbackIn (static)
transfer->qtd.token = 0
USBDrive dataIn (static): 13
55 53 42 53 0A 00 00 00 00 00 00 00 00 
msProcessError()
rslt = 0
BYTE      00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
---------------------------------------------------------
0000      74 68 69 73 20 69 73 20 61 20 74 65 73 74 2e 00   this is a test..

Clearing buffer
msWriteBlocks()
msDoCommand()
USBDrive CallbackOut (static)
transfer->qtd.token = 0
USBDrive dataOut (static)31
55 53 42 43 0B 00 00 00 00 02 00 00 00 00 0A 2A 00 00 0F 42 40 00 00 01 00 00 00 00 00 00 00 
USBDrive CallbackOut (static)
transfer->qtd.token = 0
USBDrive dataOut (static)512
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
msGetCSW()
USBDrive CallbackIn (static)
transfer->qtd.token = 0
USBDrive dataIn (static): 13
55 53 42 53 0B 00 00 00 00 00 00 00 00 
msProcessError()
msReadBlocks()
msDoCommand()
USBDrive CallbackOut (static)
transfer->qtd.token = 0
USBDrive dataOut (static)31
55 53 42 43 0C 00 00 00 00 02 00 00 80 00 0A 28 00 00 0F 42 40 00 00 01 00 00 00 00 00 00 00 
USBDrive CallbackIn (static)
transfer->qtd.token = 0
USBDrive dataIn (static): 512
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
msGetCSW()
USBDrive CallbackIn (static)
transfer->qtd.token = 0
USBDrive dataIn (static): 13
55 53 42 53 0C 00 00 00 00 00 00 00 00 
msProcessError()
BYTE      00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
---------------------------------------------------------
0000      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   ................
press enter to test...
Note: I have a lot of debug turned on. But it does appear to run

Edit: Sometimes when reading product information, it is hard to decipher what the mean.
Screenshot1.png
That is, does it require a USB 3 port to get those speeds or period.
 
@mjs513 - I also noticed a couple of other things in those snapshots. Down a couple of lines you see "URB_FUNCTION_SYNC_RESET_PIPE_AND_CLEAR_STALL" twice. I think this is what we are seeing on the T4. For some reason the flash drive is stalling the pipe and that is why we are seeing the halted pipe and "ERROR followup". I know there is a way to rest and clear the stall. Just not sure how to do this with the T4.

Edit: There is also two "URB_BULK_IN" in a row after a "URB_BULK_OUT".
 

Attachments

  • scsiCMDa2.jpg
    scsiCMDa2.jpg
    91 KB · Views: 73
Last edited:
KurtE said:
Code: 
Writing: 'this is a test.' to sector: 1000000
msWriteBlocks()
msDoCommand()
USBDrive CallbackOut (static)
transfer->qtd.token = 0
USBDrive dataOut (static)31
55 53 42 43 09 00 00 00 00 02 00 00 00 00 0A 2A 00 00 0F 42 40 00 00 01 00 00 00 00 00 00 00 
USBDrive CallbackOut (static)
transfer->qtd.token = 0
USBDrive dataOut (static)512
74 68 69 73 20 69 73 20 61 20 74 65 73 74 2E 00 72 73 6C 74 20 3D 20 25 64 0A 00 00 0A 52 65 61 
msGetCSW()
USBDrive CallbackIn (static)
transfer->qtd.token = 0
USBDrive dataIn (static): 13
55 53 42 53 09 00 00 00 00 00 00 00 00 
msProcessError()
rslt = 0

Reading sector:10000000 to cbuff
msReadBlocks()
msDoCommand()
USBDrive CallbackOut (static)
transfer->qtd.token = 0
USBDrive dataOut (static)31
55 53 42 43 0A 00 00 00 00 02 00 00 80 00 0A 28 00 00 0F 42 40 00 00 01 00 00 00 00 00 00 00 
USBDrive CallbackIn (static)
transfer->qtd.token = 0
USBDrive dataIn (static): 512
74 68 69 73 20 69 73 20 61 20 74 65 73 74 2E 00 72 73 6C 74 20 3D 20 25 64 0A 00 00 0A 52 65 61 
msGetCSW()
USBDrive CallbackIn (static)
transfer->qtd.token = 0
USBDrive dataIn (static): 13
55 53 42 53 0A 00 00 00 00 00 00 00 00 
msProcessError()
rslt = 0
BYTE      00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
---------------------------------------------------------
0000      74 68 69 73 20 69 73 20 61 20 74 65 73 74 2e 00   this is a test..

Clearing buffer
msWriteBlocks()
msDoCommand()
USBDrive CallbackOut (static)
transfer->qtd.token = 0
USBDrive dataOut (static)31
55 53 42 43 0B 00 00 00 00 02 00 00 00 00 0A 2A 00 00 0F 42 40 00 00 01 00 00 00 00 00 00 00 
USBDrive CallbackOut (static)
transfer->qtd.token = 0
USBDrive dataOut (static)512
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
msGetCSW()
USBDrive CallbackIn (static)
transfer->qtd.token = 0
USBDrive dataIn (static): 13
55 53 42 53 0B 00 00 00 00 00 00 00 00 
msProcessError()
msReadBlocks()
msDoCommand()
USBDrive CallbackOut (static)
transfer->qtd.token = 0
USBDrive dataOut (static)31
55 53 42 43 0C 00 00 00 00 02 00 00 80 00 0A 28 00 00 0F 42 40 00 00 01 00 00 00 00 00 00 00 
USBDrive CallbackIn (static)
transfer->qtd.token = 0
USBDrive dataIn (static): 512
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
msGetCSW()
USBDrive CallbackIn (static)
transfer->qtd.token = 0
USBDrive dataIn (static): 13
55 53 42 53 0C 00 00 00 00 00 00 00 00 
msProcessError()
BYTE      00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
---------------------------------------------------------
0000      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   ................
press enter to test...
Note: I have a lot of debug turned on. But it does appear to run

Edit: Sometimes when reading product information, it is hard to decipher what the mean.
View attachment 30721
That is, does it require a USB 3 port to get those speeds or period.
Click to expand...

I have a magnifying lite and a magnifier for just such an occasion:) It states that it is backward compatible with USB 2.0. I think it's time to do some online research...

Edit: Here is a link to the support page for the ultra dualdrive I am testing:
https://support-en.wd.com/app/products/product-detailweb/p/8779
 
Last edited:
wwatson said:
@mjs513 - I also noticed a couple of other things in those snapshots. Down a couple of lines you see "URB_FUNCTION_SYNC_RESET_PIPE_AND_CLEAR_STALL" twice. I think this is what we are seeing on the T4. For some reason the flash drive is stalling the pipe and that is why we are seeing the halted pipe and "ERROR followup". I know there is a way to rest and clear the stall. Just not sure how to do this with the T4.

Edit: There is also two "URB_BULK_IN" in a row after a "URB_BULK_OUT".
Click to expand...

Think we had an issue with USB Pipe Stall before: https://forum.pjrc.com/threads/6813...ng-MTP-and-MSC?p=292596&viewfull=1#post292596 but I remember similar issues - just can't find them now.
 
@mjs513 @wwatson

@PaullStoffregen (if you are watching) - thread shows last look about a week ago.

Some of this lower level USB stuff is above my pay grade (of $0) ;)

Stalls - I have looked a little bit through the T3.6 and IMXRT manuals and trying to decipher some of the Jargons.

But my impression is that maybe to clear a stall there appears to be some Endpoint control registers
USB_nENDPOINTCTRLn on IMXRT and USBHS_EPCRn on T3.6 but I don't see anywhere in our code base that references these registers.
They are defined in imxrt.h...

Not sure if they would work or not. Might experiment later if we see the stall bit to try clearing it there/here and see if it does anything?
 
@KurtE @mjs513 - Here is a web article of interest with information on stalled USB 3.0 pipes and how to reset and clear them:
http://billauer.co.il/blog/2019/12/usb-bulk-overflow-halt-reset/

Would like to adapt this:
Code: 
if (rc == LIBUSB_ERROR_OVERFLOW) {
	rc = libusb_control_transfer(dev_handle,
				     0x02, // bmRequestType, endpoint
				     0x03, // bRequest = SET_FEATURE
				     0x00, // wValue = ENDPOINT_HALT
				     (1 | LIBUSB_ENDPOINT_IN), // wIndex = ep
				     NULL, // Data (no data)
				     0, // wLength = 0
				     100); // Timeout, ms

	if (rc) {
	  print_usberr(rc, "Failed to halt endpoint");
	  break;
    }

	rc = libusb_control_transfer(dev_handle,
				     0x02, // bmRequestType, endpoint
				     0x01, // bRequest = CLEAR_FEATURE
				     0x00, // wValue = ENDPOINT_HALT
				     (1 | LIBUSB_ENDPOINT_IN), // wIndex = ep
				     NULL, // Data (no data)
				     0, // wLength = 0
				     100); // Timeout, ms

	if (rc) {
	  print_usberr(rc, "Failed to unhalt endpoint");
	  break;
	}

	continue;
}
to our USBHost library and see if I can blowup something:)

It seems to be referencing Linux and xHCI. EHCI? Also here is the Linux lsusb output for the Sandisk Ultra 3.0 dualdrive:
Code: 
Bus 002 Device 010: ID 0781:5590 SanDisk Corp. Ultra Dual
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               3.00
  bDeviceClass            0 
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0         9
  idVendor           0x0781 SanDisk Corp.
  idProduct          0x5590 Ultra Dual
  bcdDevice            1.00
  iManufacturer           1 SanDisk
  iProduct                2 Ultra
  iSerial                 3 4C530001120612122373
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength       0x002c
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              896mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass         8 Mass Storage
      bInterfaceSubClass      6 SCSI
      bInterfaceProtocol     80 Bulk-Only
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0400  1x 1024 bytes
        bInterval               0
        bMaxBurst               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x02  EP 2 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0400  1x 1024 bytes
        bInterval               0
        bMaxBurst              15
[COLOR="#FF0000"]Binary Object Store Descriptor:
  bLength                 5
  bDescriptorType        15
  wTotalLength       0x0016
  bNumDeviceCaps          2
  USB 2.0 Extension Device Capability:
    bLength                 7
    bDescriptorType        16
    bDevCapabilityType      2
    bmAttributes   0x00000002
      HIRD Link Power Management (LPM) Supported
  SuperSpeed USB Device Capability:
    bLength                10
    bDescriptorType        16
    bDevCapabilityType      3
    bmAttributes         0x00
    wSpeedsSupported   0x000e
      Device can operate at Full Speed (12Mbps)
      Device can operate at High Speed (480Mbps)
      Device can operate at SuperSpeed (5Gbps)
    bFunctionalitySupport   1
      Lowest fully-functional device speed is Full Speed (12Mbps)
    bU1DevExitLat          10 micro seconds
    bU2DevExitLat         256 micro seconds
Device Status:     0x0000
  (Bus Powered)[/COLOR]

The portion marked in red makes me wonder if there is configuration needed. This was list on a computer with USB 3.0 ports. The same listing on a computer with USB 2.0 ports shows max packets sizes of 512 instead of 1024...
 
KurtE said:
Sounds like something to try. I have seen the clear feature like this somewhere else.
Click to expand...

Didn't we run into a similar problem with MTP where USB was hanging with SD Cards- could have sworn it involved stalls as well
 
yeahtuna said:
I opened an issue on github in regards to possible erroneous CommandBlockStatus.Tag errors here:
https://github.com/PaulStoffregen/USBHost_t36/issues/113

Is my logic incorrect? Can we be sure that the compiler won't optimize away the reading of StatusBlockWrapper.Tag?
Click to expand...

The function calls queue_Data_Transfer and passes the address of StatusBlockWrapper as an argument. So it must exist in memory, and the compiler can't assume it is unmodified (unless that argument is a const pointer).
 
@ All - In mscInit() function, comment out call to msReset() function:
Code: 
// Initialize Mass Storage Device
uint8_t USBDrive::mscInit(void) {
#ifdef DBGprint
	println("mscIint()");
#endif
	uint8_t msResult = MS_CBW_PASS;

	CBWTag = 0;
	uint32_t start = millis();
	// Check if device is connected.
	do {
		if((millis() - start) >= MSC_CONNECT_TIMEOUT) {
			return MS_NO_MEDIA_ERR;  // Not connected Error.
		}
		yield();
	} while(!available());
  
[COLOR="#FF0000"]//	msReset();[/COLOR]
	// delay(500); // Not needed any more.
	maxLUN = msGetMaxLun();

//	msResult = msReportLUNs(&maxLUN);
//println("maxLUN = ");
//println(maxLUN);
//	delay(150);
	//-------------------------------------------------------
	msResult = msStartStopUnit(1);
	msResult = WaitMediaReady();
	if(msResult)
		return msResult;
		
	// Retrieve drive information.
	msDriveInfo.initialized = true;
	msDriveInfo.hubNumber = getHubNumber();			// Which HUB.
	msDriveInfo.hubPort = getHubPort();				// Which HUB port.
	msDriveInfo.deviceAddress = getDeviceAddress();	// Device addreess.
	msDriveInfo.idVendor = getIDVendor();  			// USB Vendor ID.
	msDriveInfo.idProduct = getIDProduct();  		// USB Product ID.
	msResult = msDeviceInquiry(&msInquiry);			// Config Info.
	if(msResult)
		return msResult;
	msResult = msReadDeviceCapacity(&msCapacity);	// Size Info.
	if(msResult)
		return msResult;
	memcpy(&msDriveInfo.inquiry, &msInquiry, sizeof(msInquiryResponse_t));
	memcpy(&msDriveInfo.capacity, &msCapacity, sizeof(msSCSICapacity_t));
	return msResult;
}

That took care of all issues with these SanDisk devices we were seeing. Other devices seem to still work but probably should be selective about which ones need to be reset (using vid and pid). I remembered seeing that some flash drives would fail if reset. The possibility exists the default power on configuration works and if msReset() is applied the device needs to be configured back to that same working configuration.

What a lot of horsing around for that fix:)
 
wwatson said:
That took care of all issues with these SanDisk devices we were seeing. Other devices seem to still work but probably should be selective about which ones need to be reset (using vid and pid). I remembered seeing that some flash drives would fail if reset. The possibility exists the default power on configuration works and if msReset() is applied the device needs to be configured back to that same working configuration.

What a lot of horsing around for that fix
Click to expand...

I hate issues like that.... Anyway just tried my read/write USB sketch on the GAM46 (the sansdisk ultra and the UAM46 old gen) and looks like no more hangs:
Code: 
Initializing USB MSC drive...Waiting for USB Filesystem

 === Task() Drive 0x20004120 connected ===
>> USBDrive::startFilesystems called 0x20004120
	>>Partition 1 VT:1 T:12 32 60062468
	>>USBFilesystem::claimPartition 0x1 called >>USBFilesystem::check_voltype_guid(1, 0x20067f28)
+ Claimed

Try Partition list>>USBDrive::printPartionTable

Partition Table
	part,boot,bgnCHS[3],type,endCHS[3],start,length
FAT32:	1,0,0x0,0x21,0x0,0xC,0xFE,0xFF,0xFF,32,60062468
pt_#0:	2,0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0,0
pt_#0:	3,0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0,0
pt_#0:	4,0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0,0
Filesystem started
SanDiskSecureAccess/
  DownloadSanDiskSecureAccess_Mac.pdf             355233
  SanDisk_SecureAccess_QSG.pdf                    2676267
Back Up Your Files to the Cloud.pdf               403851
RunSanDiskSecureAccess_Win.exe                    16024600
System Volume Information/
  WPSettings.dat                                  12
  IndexerVolumeGuid                               76
LOST.DIR/
Writing to test.txt...done.
SanDiskSecureAccess/
  DownloadSanDiskSecureAccess_Mac.pdf             355233
  SanDisk_SecureAccess_QSG.pdf                    2676267
Back Up Your Files to the Cloud.pdf               403851
RunSanDiskSecureAccess_Win.exe                    16024600
System Volume Information/
  WPSettings.dat                                  12
  IndexerVolumeGuid                               76
LOST.DIR/
test.txt                                          18
test.txt:
testing 1, 2, 3.
SanDiskSecureAccess/
  DownloadSanDiskSecureAccess_Mac.pdf             355233
  SanDisk_SecureAccess_QSG.pdf                    2676267
Back Up Your Files to the Cloud.pdf               403851
RunSanDiskSecureAccess_Win.exe                    16024600
System Volume Information/
  WPSettings.dat                                  12
  IndexerVolumeGuid                               76
LOST.DIR/
test.txt                                          18

on the UAM46 same thing
Code: 
Initializing USB MSC drive...Waiting for USB Filesystem

 === Task() Drive 0x20004120 connected ===
>> USBDrive::startFilesystems called 0x20004120
	>>Partition 1 VT:1 T:12 2112 60121088
	>>USBFilesystem::claimPartition 0x1 called >>USBFilesystem::check_voltype_guid(1, 0x20067f28)
+ Claimed

Try Partition list>>USBDrive::printPartionTable

Partition Table
	part,boot,bgnCHS[3],type,endCHS[3],start,length
FAT32:	1,0,0x21,0x22,0x0,0xC,0xFE,0xFF,0xFF,2112,60121088
pt_#0:	2,0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0,0
pt_#0:	3,0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0,0
pt_#0:	4,0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0,0
	 < unused area starting at: 60123200 length 1983 >
Filesystem started
System Volume Information/
  WPSettings.dat                                  12
  IndexerVolumeGuid                               76
Writing to test.txt...done.
System Volume Information/
  WPSettings.dat                                  12
  IndexerVolumeGuid                               76
test.txt                                          18
test.txt:
testing 1, 2, 3.
System Volume Information/
  WPSettings.dat                                  12
  IndexerVolumeGuid                               76
test.txt                                          18
 
Looks like it even fixed the issue with my ancient sansdisk cruzr (2gb drive)
Code: 
Partition Table
	part,boot,bgnCHS[3],type,endCHS[3],start,length
FAT32:	1,0,0x20,0x21,0x0,0xB,0xAA,0x28,0x82,2048,2097152
FAT16:	2,0,0xAA,0x29,0x82,0x6,0xFE,0x3F,0xF8,2099200,1912832
pt_#0:	3,0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0,0
pt_#0:	4,0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0,0
	 < unused area starting at: 4012032 length 1680 >
Filesystem started
System Volume Information/
  IndexerVolumeGuid                               76
  WPSettings.dat                                  12
Writing to test.txt...done.
System Volume Information/
  IndexerVolumeGuid                               76
  WPSettings.dat                                  12
test.txt                                          18
test.txt:
testing 1, 2, 3.
System Volume Information/
  IndexerVolumeGuid                               76
  WPSettings.dat                                  12
test.txt                                          18
 
jmarsh said:
The function calls queue_Data_Transfer and passes the address of StatusBlockWrapper as an argument. So it must exist in memory, and the compiler can't assume it is unmodified (unless that argument is a const pointer).
Click to expand...

But after the call to queue_Data_Transfer() completes, the value of StatusBlockWrapper.Tag is still 0, and its value is only updated outside of the scope of msGetCSW().
 
Last edited:
yeahtuna said:
But after the call to queue_Data_Transfer() completes, the value of StatusBlockWrapper.Tag is still 0, and its value is only updated outside of the scope of msGetCSW().
Click to expand...

Look at the name of the function: queue_Data_transfer. It doesn't update the buffer directly, it just queues it - and the return value isn't being checked. Queueing can fail and the buffer will keep its original state. This isn't an optimization/compiler problem.
 
jmarsh said:
Look at the name of the function: queue_Data_transfer. It doesn't update the buffer directly, it just queues it.
Click to expand...

That's my point. From the perspective of msGetCSW(), the value of .Tag doesn't change and when it gets to this point:

Code: 
if(StatusBlockWrapper.Tag != CBWTag)

It seems to me that there's a chance that the real value of StatusBlockWrapper.Tag won't be used. It's possible that the code will just assume StatusBlockWrapper.Tag is still 0.
 
Last edited:
Think about it for a moment: if it were optimized out it would happen every time, not just on rare occasions.

The compiler simply isn't allowed to perform the optimization you're proposing because the address of the struct is being used in an intermediate function call.
 
@wwatson @mjs513 - I tried the change and as mentioned it appears to fix the issue with at least the problem child I picked up. I ran the MTP USB example and was able to both have it create the log file on the drive. It also allowed me to copy files to the disk.

This was after I fixed that sketch to build with the current stuff (I pushed up those changes).

Side question to myself and others who do stuff on the MTP_Teensy library. Wondering if I/We should create some github rules type stuff, that does not allow code to go in until github verifies that all of the examples compile. Was added recently to my ILI9341_t3n library...

@wwatson - wondering are you going to submit a PR to @PaulStoffregen on this? Default to just not call it? Or have list of drives to call it on or not call it on?
 
KurtE said:
Will have to take a look at those.

But first I am curious about the failure:
Code: 
File "annie3 - Copy.bmp" size:230454 Created:641d6f21 Modified:61e3a02e
 read consumed all data (TODO: how to check ZLP)
checkConnectedInitialized()msWriteBlocks()
msDoCommand()
USBDrive CallbackOut (static)
transfer->qtd.token = 0
[COLOR="#FFA500"]USBDrive dataOut (static)31[/COLOR]
55 53 42 43 0E 00 00 00 00 02 00 00 00 00 0A 2A 00 00 00 80 20 00 00 01 00 00 00 00 00 00 00 
[COLOR="#FFA500"]USBDrive CallbackOut (static)
transfer->qtd.token = 0[/COLOR]
[COLOR="#FF0000"]USBDrive dataOut (static)512[/COLOR]
55 53 42 43 0F 00 00 00 FC 00 00 00 80 00 06 03 00 00 00 FC 00 00 00 00 00 00 00 00 00 00 00 00 
msGetCSW()
[COLOR="#FF0000"]USBDrive CallbackIn (static)
transfer->qtd.token = 64
	800D8140[/COLOR]
USBDrive dataIn (static): 0
ERROR Followup
msGetCSW TAG_ERROR: sig:53425355 Tag:0 DR:0 ST:0
msProcessError()
CSW Tag Error: 253
msRequestSense()
msDoCommand()
USBDrive CallbackOut (static)
transfer->qtd.token = 0
USBDrive dataOut (static)31
The first data out of 31 bytes, came back on callback OUT with the QTD.token = 0

Where the RED one was 512 bytes and callback was on the IN, with the last byte of QTD of 64.
I also printed out the whole QTD in hex. So last byte is 0x40 or bit 6 set. Which means:

So I am assuming everything after this is garbage.

EDIT: forgot to mention, that in the with the full QTX token: 800D8140
bits 16-30 are how many bytes left to transfer
So that is: D (13 bytes left)

Bits: 8-9 are the PID Code which is: 01 (IN Token generates token (69H) )
Bits 10-11 is the count down error count: 00
12-14 is current page: 0
15 is interrupt on completion: 1
Click to expand...

The logged info here looks really suspicious. First a CBW goes out for a WRITE_10 command of length one sector, then the 512 bytes of sector data go out, then trying to bring in the 13 byte CSW fails with a STALL. But have a look at the dumped sector data:
Code: 
55 53 42 43 0F 00 00 00 FC 00 00 00 80 00 06 03 00 00 00 FC 00 00 00 00 00 00 00 00 00 00 00 00
That doesn't look like random file data... it's a CBW for a REQUEST_SENSE command, with the next successive tag after the WRITE_10 command! How did it end up in the transfer buffer?
Something very strange is going on here. The CBW/CSWs are all created as local variables (on the stack) so either there's major stack corruption happening or somehow the Transfer_t lists in ehci.cpp are getting corrupted...

With regards to clearing halted endpoints, this code that wwatson posted is spot on:
Code: 
rc = libusb_control_transfer(dev_handle,
				     0x02, // bmRequestType, endpoint
				     0x01, // bRequest = CLEAR_FEATURE
				     0x00, // wValue = ENDPOINT_HALT
				     (1 | LIBUSB_ENDPOINT_IN), // wIndex = ep
				     NULL, // Data (no data)
				     0, // wLength = 0
				     100); // Timeout, ms
I.E. it's just a special control transfer, you don't need to do any magic hardware pokes of the host registers. Years ago I helped implement a homebrew usbstorage driver for the Nintendo Wii, we definitely had to add error handling (with resetting and clearing halts on the endpoints) to ensure most devices worked correctly. Another thing to watch out for is sector size - it's not safe to assume 1 dataIn/dataOut transfer = 1 sector because optical drives use 2048 bytes per sector (that would be why KurtE was getting errors earlier in the thread when trying to use one) and some HDDs use 4096 byte sectors. Other large (>2TB) HDDs still use 512b sectors but because they have over 2^32 LBAs, they require different READ/WRITE commands to be able to access the entire disk. I think there's a special inquiry page or code to check if that's needed but I don't remember exactly off the top of my head.
 
Ok did some testing with MTP (writing/reading files and transferring files) and all worked fine with that change.

Drives tested so far:
1. Crucial MX500 500GB 3D NAND SATA 2.5 Inch Internal SSD, up to 560MB/s - CT500MX500SSD1
2. INDMEM mSATA SSD 256GB Internal Mini SATA III SSD Micro-SATA TLC 3D NAND Flash 256 GB
3. HP SSD S 600 120GB


4. Silicon Power 2TB Rugged Portable External Hard Drive Armor A30:
This was a strange one:
Code: 
       connected: 1
     initialized: 1
   USB Vendor ID: 174c
  USB Product ID: 55aa
      HUB Number: 0
        HUB Port: 0
  Device Address: 1
Removable Device: NO
        VendorID: PHD 3.0 
       ProductID: Silicon-Power   
      RevisionID: 0   
         Version: 6
    Sector Count:[COLOR="#FF0000"] -387938129[/COLOR]
     Sector size: 512
   Disk Capacity: 2000398933504 Bytes
notice the sector count?

PS did find adding a delay(500) allowed the drive to come on line
Code: 
  while (!myDrive) {
    myusb.Task();
  }
[COLOR="#FF0000"]delay(500)[/COLOR];
 
A little bit of an update
The Silicon Power 2TB HD does need a powered hub to be able to write/read using MTP.

Also just tried hooking up a very old Samsung HDD which definitely needs a powered hub
Code: 
         VendorID:   Initio
       ProductID: MP0603H         
      RevisionID: 1.06
         Version: 0
    Sector Count: 117304991
     Sector size: 512
   Disk Capacity: 60060155392 Bytes
and still worked with the change
 
KurtE said:
@wwatson @mjs513 - I tried the change and as mentioned it appears to fix the issue with at least the problem child I picked up. I ran the MTP USB example and was able to both have it create the log file on the drive. It also allowed me to copy files to the disk.

This was after I fixed that sketch to build with the current stuff (I pushed up those changes).

Side question to myself and others who do stuff on the MTP_Teensy library. Wondering if I/We should create some github rules type stuff, that does not allow code to go in until github verifies that all of the examples compile. Was added recently to my ILI9341_t3n library...

@wwatson - wondering are you going to submit a PR to @PaulStoffregen on this? Default to just not call it? Or have list of drives to call it on or not call it on?
Click to expand...

I have a few things I have to do this morning so after that I want to test more devices SD card readers, Hard Drives, etc. Just to make sure msReset() is not needed. For the most part I am convinced it's not needed. I will issue a PR once I done some more testing. I still have one PR in for the timeouts in "msc.h":
Code: 
// These two defines are timeouts for detecting a connected drive
// and waiting for it to be operational.
#define MEDIA_READY_TIMEOUT 5000   // Was 1000
#define MSC_CONNECT_TIMEOUT 5000 // Was 4000
These timeouts were not long enough for detection of my SD card reader or SSD drive.
 
You must log in or register to reply here.
Back
Top