Fixes for ub in 2.4.9-rc1 from Oliver and Pat

Fixes for ub in 2.4.9-rc1 from Oliver and Pat

[Pete Zaitcev]

- Set the allocation size in REQUEST SENSE (Pat LaVarre)
- Move add_timer invocations to safer places (Oliver Neukum)

-- Pete

--- linux-2.6.9-rc1-mm1-ub/drivers/block/ub.c	2004-08-30 08:37:59.880881584 -0700
+++ linux-2.6.9-rc1-ub/drivers/block/ub.c	2004-08-27 12:24:25.000000000 -0700
@@ -786,17 +786,16 @@ static int ub_scsi_cmd_start(struct ub_d
 	sc->work_urb.error_count = 0;
 	sc->work_urb.status = 0;
 
-	sc->work_timer.expires = jiffies + UB_URB_TIMEOUT;
-	add_timer(&sc->work_timer);
-
 	if ((rc = usb_submit_urb(&sc->work_urb, GFP_ATOMIC)) != 0) {
 		/* XXX Clear stalls */
 		printk("ub: cmd #%d start failed (%d)\n", cmd->tag, rc); /* P3 */
-		del_timer(&sc->work_timer);
 		ub_complete(&sc->work_done);
 		return rc;
 	}
 
+	sc->work_timer.expires = jiffies + UB_URB_TIMEOUT;
+	add_timer(&sc->work_timer);
+
 	cmd->state = UB_CMDST_CMD;
 	ub_cmdtr_state(sc, cmd);
 	return 0;
@@ -968,18 +967,17 @@ static void ub_scsi_urb_compl(struct ub_
 		sc->work_urb.error_count = 0;
 		sc->work_urb.status = 0;
 
-		sc->work_timer.expires = jiffies + UB_URB_TIMEOUT;
-		add_timer(&sc->work_timer);
-
 		if ((rc = usb_submit_urb(&sc->work_urb, GFP_ATOMIC)) != 0) {
 			/* XXX Clear stalls */
 			printk("ub: data #%d submit failed (%d)\n", cmd->tag, rc); /* P3 */
-			del_timer(&sc->work_timer);
 			ub_complete(&sc->work_done);
 			ub_state_done(sc, cmd, rc);
 			return;
 		}
 
+		sc->work_timer.expires = jiffies + UB_URB_TIMEOUT;
+		add_timer(&sc->work_timer);
+
 		cmd->state = UB_CMDST_DATA;
 		ub_cmdtr_state(sc, cmd);
 
@@ -1063,19 +1061,18 @@ static void ub_scsi_urb_compl(struct ub_
 			sc->work_urb.error_count = 0;
 			sc->work_urb.status = 0;
 
-			sc->work_timer.expires = jiffies + UB_URB_TIMEOUT;
-			add_timer(&sc->work_timer);
-
 			rc = usb_submit_urb(&sc->work_urb, GFP_ATOMIC);
 			if (rc != 0) {
 				/* XXX Clear stalls */
 				printk("%s: CSW #%d submit failed (%d)\n",
 				   sc->name, cmd->tag, rc); /* P3 */
-				del_timer(&sc->work_timer);
 				ub_complete(&sc->work_done);
 				ub_state_done(sc, cmd, rc);
 				return;
 			}
+
+			sc->work_timer.expires = jiffies + UB_URB_TIMEOUT;
+			add_timer(&sc->work_timer);
 			return;
 		}
 
@@ -1186,18 +1183,17 @@ static void ub_state_stat(struct ub_dev 
 	sc->work_urb.error_count = 0;
 	sc->work_urb.status = 0;
 
-	sc->work_timer.expires = jiffies + UB_URB_TIMEOUT;
-	add_timer(&sc->work_timer);
-
 	if ((rc = usb_submit_urb(&sc->work_urb, GFP_ATOMIC)) != 0) {
 		/* XXX Clear stalls */
 		printk("ub: CSW #%d submit failed (%d)\n", cmd->tag, rc); /* P3 */
-		del_timer(&sc->work_timer);
 		ub_complete(&sc->work_done);
 		ub_state_done(sc, cmd, rc);
 		return;
 	}
 
+	sc->work_timer.expires = jiffies + UB_URB_TIMEOUT;
+	add_timer(&sc->work_timer);
+
 	cmd->stat_count = 0;
 	cmd->state = UB_CMDST_STAT;
 	ub_cmdtr_state(sc, cmd);
@@ -1217,9 +1213,17 @@ static void ub_state_sense(struct ub_dev
 		goto error;
 	}
 
+	/*
+	 * ``If the allocation length is eighteen or greater, and a device
+	 * server returns less than eithteen bytes of data, the application
+	 * client should assume that the bytes not transferred would have been
+	 * zeroes had the device server returned those bytes.''
+	 */
 	memset(&sc->top_sense, 0, UB_SENSE_SIZE);
+
 	scmd = &sc->top_rqs_cmd;
 	scmd->cdb[0] = REQUEST_SENSE;
+	scmd->cdb[4] = UB_SENSE_SIZE;
 	scmd->cdb_len = 6;
 	scmd->dir = UB_DIR_READ;
 	scmd->state = UB_CMDST_INIT;
@@ -1271,14 +1275,13 @@ static int ub_submit_clear_stall(struct 
 	sc->work_urb.error_count = 0;
 	sc->work_urb.status = 0;
 
-	sc->work_timer.expires = jiffies + UB_CTRL_TIMEOUT;
-	add_timer(&sc->work_timer);
-
 	if ((rc = usb_submit_urb(&sc->work_urb, GFP_ATOMIC)) != 0) {
-		del_timer(&sc->work_timer);
 		ub_complete(&sc->work_done);
 		return rc;
 	}
+
+	sc->work_timer.expires = jiffies + UB_CTRL_TIMEOUT;
+	add_timer(&sc->work_timer);
 	return 0;
 }
 
@@ -1289,6 +1292,9 @@ static void ub_top_sense_done(struct ub_
 	unsigned char *sense = scmd->data;
 	struct ub_scsi_cmd *cmd;
 
+	/*
+	 * Ignoring scmd->act_len, because the buffer was pre-zeroed.
+	 */
 	ub_cmdtr_sense(sc, scmd, sense);
 
 	if ((cmd = ub_cmdq_peek(sc)) == NULL) {
@@ -1725,19 +1731,18 @@ static int ub_probe_clear_stall(struct u
 	sc->work_urb.error_count = 0;
 	sc->work_urb.status = 0;
 
-	init_timer(&timer);
-	timer.function = ub_probe_timeout;
-	timer.data = (unsigned long) &compl;
-	timer.expires = jiffies + UB_CTRL_TIMEOUT;
-	add_timer(&timer);
-
 	if ((rc = usb_submit_urb(&sc->work_urb, GFP_KERNEL)) != 0) {
 		printk(KERN_WARNING
 		     "%s: Unable to submit a probe clear (%d)\n", sc->name, rc);
-		del_timer_sync(&timer);
 		return rc;
 	}
 
+	init_timer(&timer);
+	timer.function = ub_probe_timeout;
+	timer.data = (unsigned long) &compl;
+	timer.expires = jiffies + UB_CTRL_TIMEOUT;
+	add_timer(&timer);
+
 	wait_for_completion(&compl);
 
 	del_timer_sync(&timer);

Re: Fixes for ub in 2.4.9-rc1 from Oliver and Pat

[Greg KH]

On Mon, Aug 30, 2004 at 08:44:55AM -0700, Pete Zaitcev wrote:
> - Set the allocation size in REQUEST SENSE (Pat LaVarre)
> - Move add_timer invocations to safer places (Oliver Neukum)

Applied, thanks.

greg k-h

p.s. next time please add a "Signed-off-by:" line...