[PATCH] ub: fix for blank CDs
authorPete Zaitcev <zaitcev@redhat.com>
Wed, 27 Jul 2005 18:43:51 +0000 (11:43 -0700)
committerLinus Torvalds <torvalds@g5.osdl.org>
Wed, 27 Jul 2005 23:25:52 +0000 (16:25 -0700)
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>
drivers/block/ub.c

index 685f061e69b2971484a9546de5f3e32cc7b32801..a026567f5d1878aaf53268ce8dceebb00194bd01 100644 (file)
@@ -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>
 
 #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,13 +1449,34 @@ 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;
        ub_cmdtr_state(sc, 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.