Reply
Highlighted
Posts: 132
Registered: ‎06-05-2004

USB Mass Storage Reference Design bugs

I played around with the mass storage reference design last weekend and encountered some bugs that should be fixed in future releases:

The response to the SCSI_INQUIRY command is wrong. The length parameter (byte at offset 4) does not match the actual length, the product identification string is too short and the revision level is missing. To fix this, the Scsi_Standard_Inquiry_Data byte-array needs to be modified (F34x_MSD_Scsi.c) and can look like:
Code:
code const BYTE Scsi_Standard_Inquiry_Data[36]= {
0x00, // Peripheral qualifier & peripheral device type
0x80, // Removable medium
0x05, // Version of the standard (2=obsolete, 5=SPC-3)
0x02, // No NormACA, No HiSup, response data format=2
0x20, // No extra parameters
0x00, // No flags
0x80, // 0x80 => BQue => Basic Task Management supported
0x00, // No flags
'S','i','L','a','b','s',' ',' ', // Requested by Dekimo via www.t10.org
'M','a','s','s',' ','S','t','o','r','a','g','e',' ',' ',' ',' ',
'0','0','0','1'

};


There is a typo in the Wait_ns-function (F34x_MSD_MMC.c). This is the correct function:
Code:
void Wait_ns(unsigned int count)
{

#ifdef F340_24M
count/=40;
#else
count/=20;
#endif

while(count--) ;
}


The OUT_EP2 definition (F34x_MSD_USB_Main.h) for the F34x version is wrong. This can cause wrong behavior during GET_STATUS-, CLEAR_FEATURE- and SET_FEATURE-requests. To fix this, open F34x_MSD_USB_Main.h and change the definition to two:
Code:
#ifdef __F326_VER__
#define OUT_EP2 0x01
#else
#define OUT_EP2 0x02
#endif


There is an error with the preprocessor directives within Timer2_Isr in F34x_USB_Main.c. This is the correct ISR:
Code:
void Timer2_ISR(void) interrupt 5 {
tickcount++;
#ifdef __F340_VER__
#ifdef F340_24M
TMR2RL = 0xF82e;
#else
TMR2RL = 0xF05e;
#endif
#else
TMR2RL = 0xFC16; // Re-initialize reload value (1kHz, 1ms)
#endif
TF2H=0; // Clear interrupt
}

BTW: Isn't it enough to load the reload register once during initialization? What is the purpose to do so within the ISR?


Another problem is the missing handler for the SCSI_REQUEST_SENSE command, which is required. The missing handler causes Windows to reset the device multiple times. As you can imagine, this is bad in particular when reworking the code for supporting multiple interfaces. When the device resets regularly during enumeration, some drivers can't load correctly.

First, add a global byte-array with the information necessary:
Code:
// The current SiLabs MSD implementation returns SCSI_FAILED in its status
// wrapper only for unhandled commands. Because of this and because of
// simplicity, sense data is held in FLASH.
code const BYTE Scsi_Sense_Data[] = {
0x70, // Response Code (Valid)
0x00, // Obsolete
0x05, // Filemark, EOM, ILI, Sense Key (Illegal Request)
0x00, 0x00, 0x00, 0x00, // Information
0x0A, // Additional Sense Length
0x00, 0x00, 0x00, 0x00, // Command-Specific Information
0x20, // Additional Sense Code (ASC) (Invalid Cmd Opcode)
0x00, // Additional Sense Code Qualifier (ASCQ)
0x00, // Field Replaceable Unit Code
0x00, 0x00, 0x00 // Sense Key Specific
};

Second, add the handler function:
Code:
void Scsi_Request_Sense()
{
Scsi_Status=SCSI_PASSED;
Scsi_Send(Scsi_Sense_Data,sizeof(Scsi_Sense_Data));
}

Finally, call the handler function from within the Scsi_Rx-function found in F34x_MSD_Scsi.c:
Code:
.
.
.
case SCSI_PREVENT_ALLOW_MEDIUM_REMOVAL:
Scsi_Status=SCSI_PASSED;
break;
case SCSI_REQUEST_SENSE:
Scsi_Request_Sense();
break;

default:
//printf('Unknown SCSI Cmd (0x%02X).
',(int)cbw.CBWCB[0]);
break;
.
.
.

The device will enumerate correctly now, but the system will not ask for the product string found in F34x_MSD_USB_Descriptor.c. This is because of the missing identifier within the device descriptor (idProduct is not the string identifier). This is the corrected descriptor:
Code:
const device_descriptor Device_Desc =
{
0x12, // bLength
0x01, // bDescriptorType
0x1001, // bcdUSB
0x00, // bDeviceClass
0x00, // bDeviceSubClass
0x00, // bDeviceProtocol
EP0_PACKET_SIZE, // bMaxPacketSize0
0xC410, // idVendor
0x0002, // idProduct
0x0000, // bcdDevice
0x01, // iManufacturer
0x02, // iProduct
0x03, // iSerialNumber
0x01 // bNumConfigurations
}; //end of Device_Desc


The low-speed EP0 packet size is set to an incorrect value (F34x_MSD_USB_Main.h), it must not be greater than 8. The corrected version:
Code:
// Define Endpoint Packet Sizes
#ifdef _USB_LOW_SPEED_
#define EP0_PACKET_SIZE 0x08
#else
#define EP0_PACKET_SIZE 0x40
#endif /* _USB_LOW_SPEED_ */

However, I think it's better to remove the low-speed code entirely. The MSD relies on bulk transfers and low-speed devices must not have bulk endpoints.

[This message has been edited by ReneK (edited April 02, 2007).]
Posts: 298
Registered: ‎04-19-2006

Re: USB Mass Storage Reference Design bugs

Good work, thanks! Needless for me now, but who knows later.

[This message has been edited by Patryk (edited April 05, 2007).]