[PS2] USB Mass Storage Status Handling

[Mega Man]

I had problems with one of my USB memory sticks, which was not detected. I saw the comment "see flow chart in the usbmassbulk_10.pdf doc (page 15)" in the code and realized that the code is not implemented as the flow chart implies. I fixed this and my USB stick was working. I can't generate a patch with "svn diff", because the files are marked as binary. Could someone fix the problems in SVN?

Code: Select all

--- usb_mass/iop/mass_stor.c	2009-08-02 13:16:57.000000000 +0200
+++ usb_mass/iop/mass_stor.c	2009-08-02 23:48:18.000000000 +0200
@@ -49,6 +49,8 @@
 #define MASS_CONNECT_CALLBACK    0x0012
 #define MASS_DISCONNECT_CALLBACK 0x0013
 
+#define CSW_PHASE_ERROR 0x02
+
 // Added by Hermes
 int getBI32(unsigned char* buf) {
 	return &#40;buf&#91;3&#93;  + &#40;buf&#91;2&#93; <<8&#41; + &#40;buf&#91;1&#93; << 16&#41; + &#40;buf&#91;0&#93; << 24&#41;&#41;;
@@ -438,13 +440,13 @@ int usb_bulk_status&#40;mass_dev* dev, csw_p
 	if&#40;ret != USB_RC_OK&#41; &#123;
 		XPRINTF&#40;"Usb&#58; Error sending csw bulk command\n"&#41;;
 		DeleteSema&#40;semh&#41;;
-		return -1;
+		return ret;
 	&#125;else &#123;
 		WaitSema&#40;semh&#41;;
 	&#125;
 	DeleteSema&#40;semh&#41;;
 	XPRINTF&#40;"Usb&#58; bulk csw.status&#58; %i\n", csw->status&#41;;
-	return csw->status;
+	return ret;
 &#125;
 
 /* see flow chart in the usbmassbulk_10.pdf doc &#40;page 15&#41; */
@@ -454,26 +456,21 @@ int usb_bulk_manage_status&#40;mass_dev* dev
 
 	XPRINTF&#40;"...waiting for status 1 ...\n"&#41;;
 	ret = usb_bulk_status&#40;dev, &csw, tag&#41;; /* Attempt to read CSW from bulk in endpoint */
-    if &#40;ret == USB_RC_STALL || ret < 0&#41; &#123; /* STALL bulk in  -OR- Bulk error */
+    if &#40;&#40;ret == USB_RC_STALL&#41; || &#40;ret != USB_RC_OK&#41;&#41; &#123; /* STALL bulk in  -OR- Bulk error */
 		usb_bulk_clear_halt&#40;dev, 0&#41;; /* clear the stall condition for bulk in */
 
 		XPRINTF&#40;"...waiting for status 2 ...\n"&#41;;
 		ret = usb_bulk_status&#40;dev, &csw, tag&#41;; /* Attempt to read CSW from bulk in endpoint */
-
 	&#125;
 
 	/* CSW not valid  or stalled or phase error */
-	if &#40;csw.signature !=  0x53425355 || csw.tag != tag&#41; &#123;
+    if &#40;&#40;ret == USB_RC_STALL&#41; || &#40;ret != USB_RC_OK&#41; /* STALL bulk in  -OR- Bulk error */
+	    || &#40;csw.status == CSW_PHASE_ERROR&#41; || &#40;csw.signature !=  0x53425355&#41; || &#40;csw.tag != tag&#41;&#41; &#123;
 		XPRINTF&#40;"...call reset recovery ...\n"&#41;;
 		usb_bulk_reset&#40;dev, 3&#41;;	/* Perform reset recovery */
-	&#125;
 
-   if &#40;ret != 0&#41; &#123;
-      XPRINTF&#40;"Mass storage device not ready!\n"&#41;;
-      return 1; //device is not ready, need to retry!
-   &#125; else &#123;
-      return 0; //device is ready to process next CBW
-   &#125;
+	&#125;
+    return 0; //device is ready to process next CBW
 &#125;
 
 

[jbit]

Code: Select all

if ((ret == USB_RC_STALL) || (ret != USB_RC_OK)) { /* STALL bulk in  -OR- Bulk error */ 

Lines like this concern me... clean them up and I'll commit it (along with your other patches)

hint: ret!=USB_RC_OK implies ret can equal USB_RC_STALL. I get that you were probably just editing code and that's how it ended up this way, but it's best to avoid code which makes you think "what is the actual intended behaviour"...

Thanks for the patches :)

edit: Fixed mime-type issue in usb_mass, SVN should think they're text files now

[Mega Man]

I used this style, because it was written this way in the flow chart (STALL Bulk-In -OR- Bulk error).
The SVN files are text files now, but the files still have DOS line endings when checking out under Linux.

Code: Select all

--- iop/mass_stor.c	(Revision 1596)
+++ iop/mass_stor.c	(Arbeitskopie)
@@ -49,6 +49,8 @@
 #define MASS_CONNECT_CALLBACK    0x0012
 #define MASS_DISCONNECT_CALLBACK 0x0013
 
+#define CSW_PHASE_ERROR 0x02
+
 // Added by Hermes
 int getBI32(unsigned char* buf) {
 	return &#40;buf&#91;3&#93;  + &#40;buf&#91;2&#93; <<8&#41; + &#40;buf&#91;1&#93; << 16&#41; + &#40;buf&#91;0&#93; << 24&#41;&#41;;
@@ -438,13 +440,13 @@
 	if&#40;ret != USB_RC_OK&#41; &#123;
 		XPRINTF&#40;"Usb&#58; Error sending csw bulk command\n"&#41;;
 		DeleteSema&#40;semh&#41;;
-		return -1;
+		return ret;
 	&#125;else &#123;
 		WaitSema&#40;semh&#41;;
 	&#125;
 	DeleteSema&#40;semh&#41;;
 	XPRINTF&#40;"Usb&#58; bulk csw.status&#58; %i\n", csw->status&#41;;
-	return csw->status;
+	return ret;
 &#125;
 
 /* see flow chart in the usbmassbulk_10.pdf doc &#40;page 15&#41; */
@@ -454,26 +456,22 @@
 
 	XPRINTF&#40;"...waiting for status 1 ...\n"&#41;;
 	ret = usb_bulk_status&#40;dev, &csw, tag&#41;; /* Attempt to read CSW from bulk in endpoint */
-    if &#40;ret == USB_RC_STALL || ret < 0&#41; &#123; /* STALL bulk in  -OR- Bulk error */
+    if &#40;ret != USB_RC_OK&#41; &#123; /* STALL bulk in  -OR- Bulk error */
 		usb_bulk_clear_halt&#40;dev, 0&#41;; /* clear the stall condition for bulk in */
 
 		XPRINTF&#40;"...waiting for status 2 ...\n"&#41;;
 		ret = usb_bulk_status&#40;dev, &csw, tag&#41;; /* Attempt to read CSW from bulk in endpoint */
-
 	&#125;
 
-	/* CSW not valid  or stalled or phase error */
-	if &#40;csw.signature !=  0x53425355 || csw.tag != tag&#41; &#123;
+	/* stalled or bulk error or phase error or CSW not valid */
+    if &#40;&#40;ret != USB_RC_OK&#41;
+	    || &#40;csw.status == CSW_PHASE_ERROR&#41; || &#40;csw.signature !=  0x53425355&#41; || &#40;csw.tag != tag&#41;&#41; &#123;
 		XPRINTF&#40;"...call reset recovery ...\n"&#41;;
 		usb_bulk_reset&#40;dev, 3&#41;;	/* Perform reset recovery */
-	&#125;
 
-   if &#40;ret != 0&#41; &#123;
-      XPRINTF&#40;"Mass storage device not ready!\n"&#41;;
-      return 1; //device is not ready, need to retry!
-   &#125; else &#123;
-      return 0; //device is ready to process next CBW
-   &#125;
+	&#125;
+	XPRINTF&#40;"... proces next CBW ...\n"&#41;;
+    return 0; //device is ready to process next CBW
 &#125;
 
 

[jimparis]

I added the patch in rev 1598.
The forum damaged whitespace, so I had to apply it by hand, and may have made mistakes -- please check.