[PATCH] ub: fix for blank CDs

This patch fixes a microcode lockup in my CD-ROM adapters when a blank CD
is inserted.  However, do not try to burn CDs yet!  I'm pretty sure that
trying it will end in coasters.

 - Fix a few cases where we were unable to resynchronize with replies
   for previous commands. The main thing is to keep reading replies
   in case of a stall. This is done with the new state CLRRS.
 - Since I am forgetting the basic state machine already, document it.
 - Move counter increments in the looping path in its own function.
 - Fix a harmless buglet in case CSW read fails to submit: do not
   override state.
 - Implement the Alan Stern's idea for adaptive signature checking.

Signed-off-by: Pete Zaitcev <zaitcev@yahoo.com>
Cc: Greg KH <greg@kroah.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

1 files changed, 170 insertions, 41 deletions

diff --git a/drivers/block/ub.c b/drivers/block/ub.c
index 685f061..a026567 100644
--- a/drivers/block/ub.c
+++ b/drivers/block/ub.c
@@ -23,6 +23,7 @@
  *  -- Exterminate P3 printks
  *  -- Resove XXX's
  *  -- Redo "benh's retries", perhaps have spin-up code to handle them. V:D=?
+ *  -- CLEAR, CLR2STS, CLRRS seem to be ripe for refactoring.
  */
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -38,6 +39,73 @@
 #define UB_MAJOR 180
 
 /*
+ * The command state machine is the key model for understanding of this driver.
+ *
+ * The general rule is that all transitions are done towards the bottom
+ * of the diagram, thus preventing any loops.
+ *
+ * An exception to that is how the STAT state is handled. A counter allows it
+ * to be re-entered along the path marked with [C].
+ *
+ *       +--------+
+ *       ! INIT   !
+ *       +--------+
+ *           !
+ *        ub_scsi_cmd_start fails ->--------------------------------------\
+ *           !                                                            !
+ *           V                                                            !
+ *       +--------+                                                       !
+ *       ! CMD    !                                                       !
+ *       +--------+                                                       !
+ *           !                                            +--------+      !
+ *         was -EPIPE -->-------------------------------->! CLEAR  !      !
+ *           !                                            +--------+      !
+ *           !                                                !           !
+ *         was error -->------------------------------------- ! --------->\
+ *           !                                                !           !
+ *  /--<-- cmd->dir == NONE ?                                 !           !
+ *  !        !                                                !           !
+ *  !        V                                                !           !
+ *  !    +--------+                                           !           !
+ *  !    ! DATA   !                                           !           !
+ *  !    +--------+                                           !           !
+ *  !        !                           +---------+          !           !
+ *  !      was -EPIPE -->--------------->! CLR2STS !          !           !
+ *  !        !                           +---------+          !           !
+ *  !        !                                !               !           !
+ *  !        !                              was error -->---- ! --------->\
+ *  !      was error -->--------------------- ! ------------- ! --------->\
+ *  !        !                                !               !           !
+ *  !        V                                !               !           !
+ *  \--->+--------+                           !               !           !
+ *       ! STAT   !<--------------------------/               !           !
+ *  /--->+--------+                                           !           !
+ *  !        !                                                !           !
+ * [C]     was -EPIPE -->-----------\                         !           !
+ *  !        !                      !                         !           !
+ *  +<---- len == 0                 !                         !           !
+ *  !        !                      !                         !           !
+ *  !      was error -->--------------------------------------!---------->\
+ *  !        !                      !                         !           !
+ *  +<---- bad CSW                  !                         !           !
+ *  +<---- bad tag                  !                         !           !
+ *  !        !                      V                         !           !
+ *  !        !                 +--------+                     !           !
+ *  !        !                 ! CLRRS  !                     !           !
+ *  !        !                 +--------+                     !           !
+ *  !        !                      !                         !           !
+ *  \------- ! --------------------[C]--------\               !           !
+ *           !                                !               !           !
+ *         cmd->error---\                +--------+           !           !
+ *           !          +--------------->! SENSE  !<----------/           !
+ *         STAT_FAIL----/                +--------+                       !
+ *           !                                !                           V
+ *           !                                V                      +--------+
+ *           \--------------------------------\--------------------->! DONE   !
+ *                                                                   +--------+
+ */
+
+/*
  * Definitions which have to be scattered once we understand the layout better.
  */
 
@@ -91,8 +159,6 @@ struct bulk_cs_wrap {
 
 #define US_BULK_CS_WRAP_LEN	13
 #define US_BULK_CS_SIGN		0x53425355	/* spells out 'USBS' */
-/* This is for Olympus Camedia digital cameras */
-#define US_BULK_CS_OLYMPUS_SIGN	0x55425355	/* spells out 'USBU' */
 #define US_BULK_STAT_OK		0
 #define US_BULK_STAT_FAIL	1
 #define US_BULK_STAT_PHASE	2
@@ -135,6 +201,7 @@ enum ub_scsi_cmd_state {
 	UB_CMDST_CLR2STS,		/* Clearing before requesting status */
 	UB_CMDST_STAT,			/* Status phase */
 	UB_CMDST_CLEAR,			/* Clearing a stall (halt, actually) */
+	UB_CMDST_CLRRS,			/* Clearing before retrying status */
 	UB_CMDST_SENSE,			/* Sending Request Sense */
 	UB_CMDST_DONE			/* Final state */
 };
@@ -146,6 +213,7 @@ static char *ub_scsi_cmd_stname[] = {
 	"c2s",
 	"sts",
 	"clr",
+	"crs",
 	"Sen",
 	"fin"
 };
@@ -316,6 +384,7 @@ struct ub_dev {
 	struct urb work_urb;
 	struct timer_list work_timer;
 	int last_pipe;			/* What might need clearing */
+	__le32 signature;		/* Learned signature */
 	struct bulk_cb_wrap work_bcb;
 	struct bulk_cs_wrap work_bcs;
 	struct usb_ctrlrequest work_cr;
@@ -339,8 +408,9 @@ static void ub_scsi_action(unsigned long _dev);
 static void ub_scsi_dispatch(struct ub_dev *sc);
 static void ub_scsi_urb_compl(struct ub_dev *sc, struct ub_scsi_cmd *cmd);
 static void ub_state_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd, int rc);
-static void __ub_state_stat(struct ub_dev *sc, struct ub_scsi_cmd *cmd);
+static int __ub_state_stat(struct ub_dev *sc, struct ub_scsi_cmd *cmd);
 static void ub_state_stat(struct ub_dev *sc, struct ub_scsi_cmd *cmd);
+static void ub_state_stat_counted(struct ub_dev *sc, struct ub_scsi_cmd *cmd);
 static void ub_state_sense(struct ub_dev *sc, struct ub_scsi_cmd *cmd);
 static int ub_submit_clear_stall(struct ub_dev *sc, struct ub_scsi_cmd *cmd,
     int stalled_pipe);
@@ -1085,6 +1155,28 @@ static void ub_scsi_urb_compl(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
 
 		ub_state_stat(sc, cmd);
 
+	} else if (cmd->state == UB_CMDST_CLRRS) {
+		if (urb->status == -EPIPE) {
+			/*
+			 * STALL while clearning STALL.
+			 * The control pipe clears itself - nothing to do.
+			 * XXX Might try to reset the device here and retry.
+			 */
+			printk(KERN_NOTICE "%s: stall on control pipe\n",
+			    sc->name);
+			goto Bad_End;
+		}
+
+		/*
+		 * We ignore the result for the halt clear.
+		 */
+
+		/* reset the endpoint toggle */
+		usb_settoggle(sc->dev, usb_pipeendpoint(sc->last_pipe),
+			usb_pipeout(sc->last_pipe), 0);
+
+		ub_state_stat_counted(sc, cmd);
+
 	} else if (cmd->state == UB_CMDST_CMD) {
 		if (urb->status == -EPIPE) {
 			rc = ub_submit_clear_stall(sc, cmd, sc->last_pipe);
@@ -1190,52 +1282,57 @@ static void ub_scsi_urb_compl(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
 				 */
 				goto Bad_End;
 			}
-			cmd->state = UB_CMDST_CLEAR;
+
+			/*
+			 * Having a stall when getting CSW is an error, so
+			 * make sure uppper levels are not oblivious to it.
+			 */
+			cmd->error = -EIO;		/* A cheap trick... */
+
+			cmd->state = UB_CMDST_CLRRS;
 			ub_cmdtr_state(sc, cmd);
 			return;
 		}
+		if (urb->status == -EOVERFLOW) {
+			/*
+			 * XXX We are screwed here. Retrying is pointless,
+			 * because the pipelined data will not get in until
+			 * we read with a big enough buffer. We must reset XXX.
+			 */
+			goto Bad_End;
+		}
 		if (urb->status != 0)
 			goto Bad_End;
 
 		if (urb->actual_length == 0) {
-			/*
-			 * Some broken devices add unnecessary zero-length
-			 * packets to the end of their data transfers.
-			 * Such packets show up as 0-length CSWs. If we
-			 * encounter such a thing, try to read the CSW again.
-			 */
-			if (++cmd->stat_count >= 4) {
-				printk(KERN_NOTICE "%s: unable to get CSW\n",
-				    sc->name);
-				goto Bad_End;
-			}
-			__ub_state_stat(sc, cmd);
+			ub_state_stat_counted(sc, cmd);
 			return;
 		}
 
 		/*
 		 * Check the returned Bulk protocol status.
+		 * The status block has to be validated first.
 		 */
 
 		bcs = &sc->work_bcs;
-		rc = le32_to_cpu(bcs->Residue);
-		if (rc != cmd->len - cmd->act_len) {
+
+		if (sc->signature == cpu_to_le32(0)) {
 			/*
-			 * It is all right to transfer less, the caller has
-			 * to check. But it's not all right if the device
-			 * counts disagree with our counts.
+			 * This is the first reply, so do not perform the check.
+			 * Instead, remember the signature the device uses
+			 * for future checks. But do not allow a nul.
 			 */
-			/* P3 */ printk("%s: resid %d len %d act %d\n",
-			    sc->name, rc, cmd->len, cmd->act_len);
-			goto Bad_End;
-		}
-
-#if 0
-		if (bcs->Signature != cpu_to_le32(US_BULK_CS_SIGN) &&
-		    bcs->Signature != cpu_to_le32(US_BULK_CS_OLYMPUS_SIGN)) {
-			/* Windows ignores signatures, so do we. */
+			sc->signature = bcs->Signature;
+			if (sc->signature == cpu_to_le32(0)) {
+				ub_state_stat_counted(sc, cmd);
+				return;
+			}
+		} else {
+			if (bcs->Signature != sc->signature) {
+				ub_state_stat_counted(sc, cmd);
+				return;
+			}
 		}
-#endif
 
 		if (bcs->Tag != cmd->tag) {
 			/*
@@ -1245,16 +1342,22 @@ static void ub_scsi_urb_compl(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
 			 * commands and reply at commands we timed out before.
 			 * Without flushing these replies we loop forever.
 			 */
-			if (++cmd->stat_count >= 4) {
-				printk(KERN_NOTICE "%s: "
-				    "tag mismatch orig 0x%x reply 0x%x\n",
-				    sc->name, cmd->tag, bcs->Tag);
-				goto Bad_End;
-			}
-			__ub_state_stat(sc, cmd);
+			ub_state_stat_counted(sc, cmd);
 			return;
 		}
 
+		rc = le32_to_cpu(bcs->Residue);
+		if (rc != cmd->len - cmd->act_len) {
+			/*
+			 * It is all right to transfer less, the caller has
+			 * to check. But it's not all right if the device
+			 * counts disagree with our counts.
+			 */
+			/* P3 */ printk("%s: resid %d len %d act %d\n",
+			    sc->name, rc, cmd->len, cmd->act_len);
+			goto Bad_End;
+		}
+
 		switch (bcs->Status) {
 		case US_BULK_STAT_OK:
 			break;
@@ -1272,6 +1375,10 @@ static void ub_scsi_urb_compl(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
 		}
 
 		/* Not zeroing error to preserve a babble indicator */
+		if (cmd->error != 0) {
+			ub_state_sense(sc, cmd);
+			return;
+		}
 		cmd->state = UB_CMDST_DONE;
 		ub_cmdtr_state(sc, cmd);
 		ub_cmdq_pop(sc);
@@ -1310,7 +1417,7 @@ static void ub_state_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd, int rc)
  * Factorization helper for the command state machine:
  * Submit a CSW read.
  */
-static void __ub_state_stat(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
+static int __ub_state_stat(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
 {
 	int rc;
 
@@ -1328,11 +1435,12 @@ static void __ub_state_stat(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
 		/* XXX Clear stalls */
 		ub_complete(&sc->work_done);
 		ub_state_done(sc, cmd, rc);
-		return;
+		return -1;
 	}
 
 	sc->work_timer.expires = jiffies + UB_STAT_TIMEOUT;
 	add_timer(&sc->work_timer);
+	return 0;
 }
 
 /*
@@ -1341,7 +1449,9 @@ static void __ub_state_stat(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
  */
 static void ub_state_stat(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
 {
-	__ub_state_stat(sc, cmd);
+
+	if (__ub_state_stat(sc, cmd) != 0)
+		return;
 
 	cmd->stat_count = 0;
 	cmd->state = UB_CMDST_STAT;
@@ -1350,6 +1460,25 @@ static void ub_state_stat(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
 
 /*
  * Factorization helper for the command state machine:
+ * Submit a CSW read and go to STAT state with counter (along [C] path).
+ */
+static void ub_state_stat_counted(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
+{
+
+	if (++cmd->stat_count >= 4) {
+		ub_state_sense(sc, cmd);
+		return;
+	}
+
+	if (__ub_state_stat(sc, cmd) != 0)
+		return;
+
+	cmd->state = UB_CMDST_STAT;
+	ub_cmdtr_state(sc, cmd);
+}
+
+/*
+ * Factorization helper for the command state machine:
  * Submit a REQUEST SENSE and go to SENSE state.
  */
 static void ub_state_sense(struct ub_dev *sc, struct ub_scsi_cmd *cmd)