[PS2] USB Mass Storage Status Handling

Create a single thread for each patch to be added to the repository. Please try to stay on topic.
Post Reply
Mega Man
Posts: 260
Joined: Sat Jun 18, 2005 3:14 am
Contact:

[PS2] USB Mass Storage Status Handling

Post by 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;
 
 
User avatar
jbit
Site Admin
Posts: 293
Joined: Sat May 28, 2005 3:11 am
Location: København, Danmark
Contact:

Post by jbit »

Code: Select all

if &#40;&#40;ret == USB_RC_STALL&#41; || &#40;ret != USB_RC_OK&#41;&#41; &#123; /* 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
Posts: 260
Joined: Sat Jun 18, 2005 3:14 am
Contact:

Post by 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	&#40;Revision 1596&#41;
+++ iop/mass_stor.c	&#40;Arbeitskopie&#41;
@@ -49,6 +49,8 @@
 #define MASS_CONNECT_CALLBACK    0x0012
 #define MASS_DISCONNECT_CALLBACK 0x0013
 
+#define CSW_PHASE_ERROR 0x02
+
 // Added by Hermes
 int getBI32&#40;unsigned char* buf&#41; &#123;
 	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
Posts: 1145
Joined: Fri Jun 10, 2005 4:21 am
Location: Boston

Post by 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.
Post Reply