<html><head><meta name="color-scheme" content="light dark"></head><body><pre style="word-wrap: break-word; white-space: pre-wrap;">ChangeSet 1.831.15.14, 2002/12/09 10:59:40-08:00, mdharm-usb@one-eyed-alien.net

[PATCH] usb-storage: make CBI interrupt URBs one-shot

Change interrupt used for CBI from periodic to one-shot.  This (a) reduces
consumed bandwidth, (b) makes the logic clearer, and (c) makes the abort
mechanism more uniform.


diff -Nru a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
--- a/drivers/usb/storage/transport.c	Mon Dec  9 11:41:02 2002
+++ b/drivers/usb/storage/transport.c	Mon Dec  9 11:41:02 2002
@@ -503,6 +503,35 @@
 	return status;
 }
 
+/* This is our function to submit interrupt URBs with enough control
+ * to make aborts/resets/timeouts work
+ *
+ * This routine always uses us-&gt;recv_intr_pipe as the pipe and
+ * us-&gt;ep_int-&gt;bInterval as the interrupt interval.
+ */
+int usb_stor_interrupt_msg(struct us_data *us, void *data,
+			unsigned int len, unsigned int *act_len)
+{
+	unsigned int pipe = us-&gt;recv_intr_pipe;
+	unsigned int maxp;
+	int status;
+
+	/* calculate the max packet size */
+	maxp = usb_maxpacket(us-&gt;pusb_dev, pipe, usb_pipeout(pipe));
+	if (maxp &gt; len)
+		maxp = len;
+
+	/* fill and submit the URB */
+	usb_fill_int_urb(us-&gt;current_urb, us-&gt;pusb_dev, pipe, data,
+			maxp, usb_stor_blocking_completion, NULL,
+			us-&gt;ep_int-&gt;bInterval);
+	status = usb_stor_msg_common(us);
+
+	/* store the actual length of the data transferred */
+	*act_len = us-&gt;current_urb-&gt;actual_length;
+	return status;
+}
+
 /* This is a version of usb_clear_halt() that doesn't read the status from
  * the device -- this is because some devices crash their internal firmware
  * when the status is requested after a halt.
@@ -627,6 +656,29 @@
 }
 
 /*
+ * Receive one buffer via interrupt transfer
+ *
+ * This function does basically the same thing as usb_stor_interrupt_msg()
+ * above, except that return codes are USB_STOR_XFER_xxx rather than the
+ * urb status.
+ */
+int usb_stor_intr_transfer(struct us_data *us, void *buf,
+		unsigned int length, unsigned int *act_len)
+{
+	int result;
+	unsigned int partial;
+
+	/* transfer the data */
+	US_DEBUGP("usb_stor_intr_transfer(): xfer %u bytes\n", length);
+	result = usb_stor_interrupt_msg(us, buf, length, &amp;partial);
+	if (act_len)
+		*act_len = partial;
+
+	return interpret_urb_result(us, us-&gt;recv_intr_pipe,
+			length, result, partial);
+}
+
+/*
  * Transfer one buffer via bulk transfer
  *
  * This function does basically the same thing as usb_stor_bulk_msg()
@@ -947,7 +999,7 @@
 	host = us-&gt;srb-&gt;host;
 	scsi_unlock(host);
 
-	/* If the state machine is blocked waiting for an URB or an IRQ,
+	/* If the state machine is blocked waiting for an URB,
 	 * let's wake it up */
 
 	/* If we have an URB pending, cancel it.  The test_and_clear_bit()
@@ -964,12 +1016,6 @@
 		usb_sg_cancel(us-&gt;current_sg);
 	}
 
-	/* If we are waiting for an IRQ, simulate it */
-	if (test_bit(US_FLIDX_IP_WANTED, &amp;us-&gt;flags)) {
-		US_DEBUGP("-- simulating missing IRQ\n");
-		usb_stor_CBI_irq(us-&gt;irq_urb, NULL);
-	}
-
 	/* Wait for the aborted command to finish */
 	wait_for_completion(&amp;us-&gt;notify);
 
@@ -981,94 +1027,11 @@
  * Control/Bulk/Interrupt transport
  */
 
-/* The interrupt handler for CBI devices */
-void usb_stor_CBI_irq(struct urb *urb, struct pt_regs *regs)
-{
-	struct us_data *us = (struct us_data *)urb-&gt;context;
-	int status;
-
-	US_DEBUGP("USB IRQ received for device on host %d\n", us-&gt;host_no);
-	US_DEBUGP("-- IRQ data length is %d\n", urb-&gt;actual_length);
-	US_DEBUGP("-- IRQ state is %d\n", urb-&gt;status);
-	US_DEBUGP("-- Interrupt Status (0x%x, 0x%x)\n",
-			us-&gt;irqbuf[0], us-&gt;irqbuf[1]);
-
-	/* has the current command been aborted? */
-	if (atomic_read(&amp;us-&gt;sm_state) == US_STATE_ABORTING) {
-
-		/* was this a wanted interrupt? */
-		if (!test_and_clear_bit(US_FLIDX_IP_WANTED, &amp;us-&gt;flags)) {
-			US_DEBUGP("ERROR: Unwanted interrupt received!\n");
-			goto exit;
-		}
-		US_DEBUGP("-- command aborted\n");
-
-		/* wake up the command thread */
-		up(&amp;us-&gt;ip_waitq);
-		goto exit;
-	}
-
-	/* is the device removed? */
-	if (urb-&gt;status == -ENOENT) {
-		US_DEBUGP("-- device has been removed\n");
-
-		/* was this a wanted interrupt? */
-		if (!test_and_clear_bit(US_FLIDX_IP_WANTED, &amp;us-&gt;flags))
-			return;
-
-		/* indicate a transport error -- this is the best we can do */
-		us-&gt;irqdata[0] = us-&gt;irqdata[1] = 0xFF;
-
-		/* wake up the command thread */
-		up(&amp;us-&gt;ip_waitq);
-		return;
-	}
-
-	/* reject improper IRQs */
-	if (urb-&gt;actual_length != 2) {
-		US_DEBUGP("-- IRQ too short\n");
-		goto exit;
-	}
-
-	/* was this a command-completion interrupt? */
-	if (us-&gt;irqbuf[0] &amp;&amp; (us-&gt;subclass != US_SC_UFI)) {
-		US_DEBUGP("-- not a command-completion IRQ\n");
-		goto exit;
-	}
-
-	/* was this a wanted interrupt? */
-	if (!test_and_clear_bit(US_FLIDX_IP_WANTED, &amp;us-&gt;flags)) {
-		US_DEBUGP("ERROR: Unwanted interrupt received!\n");
-		goto exit;
-	}
-		
-	/* copy the valid data */
-	us-&gt;irqdata[0] = us-&gt;irqbuf[0];
-	us-&gt;irqdata[1] = us-&gt;irqbuf[1];
-
-	/* wake up the command thread */
-	up(&amp;(us-&gt;ip_waitq));
-
-exit:
-	/* resubmit the urb */
-	status = usb_submit_urb (urb, GFP_ATOMIC);
-	if (status)
-		err ("%s - usb_submit_urb failed with result %d",
-		     __FUNCTION__, status);
-}
-
 int usb_stor_CBI_transport(Scsi_Cmnd *srb, struct us_data *us)
 {
 	unsigned int transfer_length = usb_stor_transfer_length(srb);
 	int result;
 
-	/* re-initialize the mutex so that we avoid any races with
-	 * early/late IRQs from previous commands */
-	init_MUTEX_LOCKED(&amp;(us-&gt;ip_waitq));
-
-	/* Set up for status notification */
-	set_bit(US_FLIDX_IP_WANTED, &amp;us-&gt;flags);
-
 	/* COMMAND STAGE */
 	/* let's send the command via the control pipe */
 	result = usb_stor_ctrl_transfer(us, us-&gt;send_ctrl_pipe,
@@ -1079,8 +1042,6 @@
 	/* check the return code for the command */
 	US_DEBUGP("Call to usb_stor_ctrl_transfer() returned %d\n", result);
 	if (result != USB_STOR_XFER_GOOD) {
-		/* Reset flag for status notification */
-		clear_bit(US_FLIDX_IP_WANTED, &amp;us-&gt;flags);
 		/* Uh oh... serious problem here */
 		return USB_STOR_TRANSPORT_ERROR;
 	}
@@ -1093,25 +1054,17 @@
 		result = usb_stor_bulk_transfer_srb(us, pipe, srb,
 					transfer_length);
 		US_DEBUGP("CBI data stage result is 0x%x\n", result);
-		if (result == USB_STOR_XFER_ERROR) {
-			clear_bit(US_FLIDX_IP_WANTED, &amp;us-&gt;flags);
+		if (result == USB_STOR_XFER_ERROR)
 			return USB_STOR_TRANSPORT_ERROR;
-		}
 	}
 
 	/* STATUS STAGE */
-
-	/* go to sleep until we get this interrupt */
-	down(&amp;(us-&gt;ip_waitq));
-
-	/* has the current command been aborted? */
-	if (atomic_read(&amp;us-&gt;sm_state) == US_STATE_ABORTING) {
-		US_DEBUGP("CBI interrupt aborted\n");
-		return USB_STOR_TRANSPORT_ERROR;
-	}
-
+	result = usb_stor_intr_transfer(us, us-&gt;irqdata,
+					sizeof(us-&gt;irqdata), NULL);
 	US_DEBUGP("Got interrupt data (0x%x, 0x%x)\n", 
 			us-&gt;irqdata[0], us-&gt;irqdata[1]);
+	if (result != USB_STOR_XFER_GOOD)
+		return USB_STOR_TRANSPORT_ERROR;
 
 	/* UFI gives us ASC and ASCQ, like a request sense
 	 *
diff -Nru a/drivers/usb/storage/transport.h b/drivers/usb/storage/transport.h
--- a/drivers/usb/storage/transport.h	Mon Dec  9 11:41:02 2002
+++ b/drivers/usb/storage/transport.h	Mon Dec  9 11:41:02 2002
@@ -145,7 +145,6 @@
 
 #define US_CBI_ADSC		0
 
-extern void usb_stor_CBI_irq(struct urb*, struct pt_regs *);
 extern int usb_stor_CBI_transport(Scsi_Cmnd*, struct us_data*);
 
 extern int usb_stor_CB_transport(Scsi_Cmnd*, struct us_data*);
@@ -164,11 +163,15 @@
 extern int usb_stor_control_msg(struct us_data *us, unsigned int pipe,
 		u8 request, u8 requesttype, u16 value, u16 index,
 		void *data, u16 size);
+extern int usb_stor_interrupt_msg(struct us_data *us, void *data,
+		unsigned int len, unsigned int *act_len);
 
 extern int usb_stor_clear_halt(struct us_data*, unsigned int pipe);
 extern int usb_stor_ctrl_transfer(struct us_data *us, unsigned int pipe,
 		u8 request, u8 requesttype, u16 value, u16 index,
 		void *data, u16 size);
+extern int usb_stor_intr_transfer(struct us_data *us, void *buf,
+		unsigned int length, unsigned int *act_len);
 extern int usb_stor_bulk_transfer_buf(struct us_data *us, unsigned int pipe,
 		void *buf, unsigned int length, unsigned int *act_len);
 extern int usb_stor_bulk_transfer_sglist(struct us_data *us, unsigned int pipe,
diff -Nru a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
--- a/drivers/usb/storage/usb.c	Mon Dec  9 11:41:02 2002
+++ b/drivers/usb/storage/usb.c	Mon Dec  9 11:41:02 2002
@@ -477,24 +477,21 @@
 	return 0;
 }	
 
-/* Set up the URB, the usb_ctrlrequest, and the IRQ pipe and handler.
+/* Set up the URB and the usb_ctrlrequest.
  * ss-&gt;dev_semaphore must already be locked.
  * Note that this function assumes that all the data in the us_data
- * strucuture is current.  This includes the ep_int field, which gives us
- * the endpoint for the interrupt.
+ * structure is current.
  * Returns non-zero on failure, zero on success
  */ 
 static int usb_stor_allocate_urbs(struct us_data *ss)
 {
-	unsigned int pipe;
-	int maxp;
-	int result;
-
 	/* calculate and store the pipe values */
 	ss-&gt;send_bulk_pipe = usb_sndbulkpipe(ss-&gt;pusb_dev, ss-&gt;ep_out);
 	ss-&gt;recv_bulk_pipe = usb_rcvbulkpipe(ss-&gt;pusb_dev, ss-&gt;ep_in);
 	ss-&gt;send_ctrl_pipe = usb_sndctrlpipe(ss-&gt;pusb_dev, 0);
 	ss-&gt;recv_ctrl_pipe = usb_rcvctrlpipe(ss-&gt;pusb_dev, 0);
+	ss-&gt;recv_intr_pipe = usb_rcvintpipe(ss-&gt;pusb_dev,
+		    ss-&gt;ep_int-&gt;bEndpointAddress &amp; USB_ENDPOINT_NUMBER_MASK);
 
 	/* allocate the usb_ctrlrequest for control packets */
 	US_DEBUGP("Allocating usb_ctrlrequest\n");
@@ -519,45 +516,6 @@
 		return 5;
 	}
 
-	/* allocate the IRQ URB, if it is needed */
-	if (ss-&gt;protocol == US_PR_CBI) {
-		US_DEBUGP("Allocating IRQ for CBI transport\n");
-
-		/* lock access to the data structure */
-		down(&amp;(ss-&gt;irq_urb_sem));
-
-		/* allocate the URB */
-		ss-&gt;irq_urb = usb_alloc_urb(0, GFP_KERNEL);
-		if (!ss-&gt;irq_urb) {
-			up(&amp;(ss-&gt;irq_urb_sem));
-			US_DEBUGP("couldn't allocate interrupt URB");
-			return 3;
-		}
-
-		/* calculate the pipe and max packet size */
-		pipe = usb_rcvintpipe(ss-&gt;pusb_dev,
-		    ss-&gt;ep_int-&gt;bEndpointAddress &amp; USB_ENDPOINT_NUMBER_MASK);
-		maxp = usb_maxpacket(ss-&gt;pusb_dev, pipe, usb_pipeout(pipe));
-		if (maxp &gt; sizeof(ss-&gt;irqbuf))
-			maxp = sizeof(ss-&gt;irqbuf);
-
-		/* fill in the URB with our data */
-		usb_fill_int_urb(ss-&gt;irq_urb, ss-&gt;pusb_dev, pipe, ss-&gt;irqbuf,
-			maxp, usb_stor_CBI_irq, ss, ss-&gt;ep_int-&gt;bInterval); 
-
-		/* submit the URB for processing */
-		result = usb_submit_urb(ss-&gt;irq_urb, GFP_KERNEL);
-		US_DEBUGP("usb_submit_urb() returns %d\n", result);
-		if (result) {
-			up(&amp;(ss-&gt;irq_urb_sem));
-			return 4;
-		}
-
-		/* unlock the data structure */
-		up(&amp;(ss-&gt;irq_urb_sem));
-
-	} /* ss-&gt;protocol == US_PR_CBI */
-
 	return 0;	/* success */
 }
 
@@ -568,17 +526,6 @@
 {
 	int result;
 
-	/* release the IRQ, if we have one */
-	down(&amp;(ss-&gt;irq_urb_sem));
-	if (ss-&gt;irq_urb) {
-		US_DEBUGP("-- releasing irq URB\n");
-		result = usb_unlink_urb(ss-&gt;irq_urb);
-		US_DEBUGP("-- usb_unlink_urb() returned %d\n", result);
-		usb_free_urb(ss-&gt;irq_urb);
-		ss-&gt;irq_urb = NULL;
-	}
-	up(&amp;(ss-&gt;irq_urb_sem));
-
 	/* free the scatter-gather request block */
 	if (ss-&gt;current_sg) {
 		kfree(ss-&gt;current_sg);
@@ -810,8 +757,6 @@
 
 		/* Initialize the mutexes only when the struct is new */
 		init_completion(&amp;(ss-&gt;notify));
-		init_MUTEX_LOCKED(&amp;(ss-&gt;ip_waitq));
-		init_MUTEX(&amp;(ss-&gt;irq_urb_sem));
 		init_MUTEX_LOCKED(&amp;(ss-&gt;dev_semaphore));
 
 		/* copy over the subclass and protocol data */
diff -Nru a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
--- a/drivers/usb/storage/usb.h	Mon Dec  9 11:41:02 2002
+++ b/drivers/usb/storage/usb.h	Mon Dec  9 11:41:02 2002
@@ -105,7 +105,6 @@
 #define US_FL_FIX_CAPACITY    0x00000080 /* READ CAPACITY response too big  */
 
 #define US_FL_DEV_ATTACHED    0x00010000 /* is the device attached?	    */
-#define US_FLIDX_IP_WANTED   17  /* 0x00020000	is an IRQ expected?	    */
 #define US_FLIDX_CAN_CANCEL  18  /* 0x00040000  okay to cancel current_urb? */
 #define US_FLIDX_CANCEL_SG   19  /* 0x00080000	okay to cancel current_sg?  */
 
@@ -139,6 +138,7 @@
 	unsigned int		recv_bulk_pipe;
 	unsigned int		send_ctrl_pipe;
 	unsigned int		recv_ctrl_pipe;
+	unsigned int		recv_intr_pipe;
 
 	/* information about the device -- always good */
 	char			vendor[USB_STOR_STRING_LEN];
@@ -173,13 +173,7 @@
 	int			pid;		 /* control thread	 */
 	atomic_t		sm_state;	 /* what we are doing	 */
 
-	/* interrupt info for CBI devices -- only good if attached */
-	struct semaphore	ip_waitq;	 /* for CBI interrupts	 */
-
 	/* interrupt communications data */
-	struct semaphore	irq_urb_sem;	 /* to protect irq_urb	 */
-	struct urb		*irq_urb;	 /* for USB int requests */
-	unsigned char		irqbuf[2];	 /* buffer for USB IRQ	 */
 	unsigned char		irqdata[2];	 /* data from USB IRQ	 */
 
 	/* control and bulk communications data */
</pre></body></html>