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->recv_intr_pipe as the pipe and + * us->ep_int->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->recv_intr_pipe; + unsigned int maxp; + int status; + + /* calculate the max packet size */ + maxp = usb_maxpacket(us->pusb_dev, pipe, usb_pipeout(pipe)); + if (maxp > len) + maxp = len; + + /* fill and submit the URB */ + usb_fill_int_urb(us->current_urb, us->pusb_dev, pipe, data, + maxp, usb_stor_blocking_completion, NULL, + us->ep_int->bInterval); + status = usb_stor_msg_common(us); + + /* store the actual length of the data transferred */ + *act_len = us->current_urb->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, &partial); + if (act_len) + *act_len = partial; + + return interpret_urb_result(us, us->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->srb->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->current_sg); } - /* If we are waiting for an IRQ, simulate it */ - if (test_bit(US_FLIDX_IP_WANTED, &us->flags)) { - US_DEBUGP("-- simulating missing IRQ\n"); - usb_stor_CBI_irq(us->irq_urb, NULL); - } - /* Wait for the aborted command to finish */ wait_for_completion(&us->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->context; - int status; - - US_DEBUGP("USB IRQ received for device on host %d\n", us->host_no); - US_DEBUGP("-- IRQ data length is %d\n", urb->actual_length); - US_DEBUGP("-- IRQ state is %d\n", urb->status); - US_DEBUGP("-- Interrupt Status (0x%x, 0x%x)\n", - us->irqbuf[0], us->irqbuf[1]); - - /* has the current command been aborted? */ - if (atomic_read(&us->sm_state) == US_STATE_ABORTING) { - - /* was this a wanted interrupt? */ - if (!test_and_clear_bit(US_FLIDX_IP_WANTED, &us->flags)) { - US_DEBUGP("ERROR: Unwanted interrupt received!\n"); - goto exit; - } - US_DEBUGP("-- command aborted\n"); - - /* wake up the command thread */ - up(&us->ip_waitq); - goto exit; - } - - /* is the device removed? */ - if (urb->status == -ENOENT) { - US_DEBUGP("-- device has been removed\n"); - - /* was this a wanted interrupt? */ - if (!test_and_clear_bit(US_FLIDX_IP_WANTED, &us->flags)) - return; - - /* indicate a transport error -- this is the best we can do */ - us->irqdata[0] = us->irqdata[1] = 0xFF; - - /* wake up the command thread */ - up(&us->ip_waitq); - return; - } - - /* reject improper IRQs */ - if (urb->actual_length != 2) { - US_DEBUGP("-- IRQ too short\n"); - goto exit; - } - - /* was this a command-completion interrupt? */ - if (us->irqbuf[0] && (us->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, &us->flags)) { - US_DEBUGP("ERROR: Unwanted interrupt received!\n"); - goto exit; - } - - /* copy the valid data */ - us->irqdata[0] = us->irqbuf[0]; - us->irqdata[1] = us->irqbuf[1]; - - /* wake up the command thread */ - up(&(us->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(&(us->ip_waitq)); - - /* Set up for status notification */ - set_bit(US_FLIDX_IP_WANTED, &us->flags); - /* COMMAND STAGE */ /* let's send the command via the control pipe */ result = usb_stor_ctrl_transfer(us, us->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, &us->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, &us->flags); + if (result == USB_STOR_XFER_ERROR) return USB_STOR_TRANSPORT_ERROR; - } } /* STATUS STAGE */ - - /* go to sleep until we get this interrupt */ - down(&(us->ip_waitq)); - - /* has the current command been aborted? */ - if (atomic_read(&us->sm_state) == US_STATE_ABORTING) { - US_DEBUGP("CBI interrupt aborted\n"); - return USB_STOR_TRANSPORT_ERROR; - } - + result = usb_stor_intr_transfer(us, us->irqdata, + sizeof(us->irqdata), NULL); US_DEBUGP("Got interrupt data (0x%x, 0x%x)\n", us->irqdata[0], us->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->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->send_bulk_pipe = usb_sndbulkpipe(ss->pusb_dev, ss->ep_out); ss->recv_bulk_pipe = usb_rcvbulkpipe(ss->pusb_dev, ss->ep_in); ss->send_ctrl_pipe = usb_sndctrlpipe(ss->pusb_dev, 0); ss->recv_ctrl_pipe = usb_rcvctrlpipe(ss->pusb_dev, 0); + ss->recv_intr_pipe = usb_rcvintpipe(ss->pusb_dev, + ss->ep_int->bEndpointAddress & 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->protocol == US_PR_CBI) { - US_DEBUGP("Allocating IRQ for CBI transport\n"); - - /* lock access to the data structure */ - down(&(ss->irq_urb_sem)); - - /* allocate the URB */ - ss->irq_urb = usb_alloc_urb(0, GFP_KERNEL); - if (!ss->irq_urb) { - up(&(ss->irq_urb_sem)); - US_DEBUGP("couldn't allocate interrupt URB"); - return 3; - } - - /* calculate the pipe and max packet size */ - pipe = usb_rcvintpipe(ss->pusb_dev, - ss->ep_int->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK); - maxp = usb_maxpacket(ss->pusb_dev, pipe, usb_pipeout(pipe)); - if (maxp > sizeof(ss->irqbuf)) - maxp = sizeof(ss->irqbuf); - - /* fill in the URB with our data */ - usb_fill_int_urb(ss->irq_urb, ss->pusb_dev, pipe, ss->irqbuf, - maxp, usb_stor_CBI_irq, ss, ss->ep_int->bInterval); - - /* submit the URB for processing */ - result = usb_submit_urb(ss->irq_urb, GFP_KERNEL); - US_DEBUGP("usb_submit_urb() returns %d\n", result); - if (result) { - up(&(ss->irq_urb_sem)); - return 4; - } - - /* unlock the data structure */ - up(&(ss->irq_urb_sem)); - - } /* ss->protocol == US_PR_CBI */ - return 0; /* success */ } @@ -568,17 +526,6 @@ { int result; - /* release the IRQ, if we have one */ - down(&(ss->irq_urb_sem)); - if (ss->irq_urb) { - US_DEBUGP("-- releasing irq URB\n"); - result = usb_unlink_urb(ss->irq_urb); - US_DEBUGP("-- usb_unlink_urb() returned %d\n", result); - usb_free_urb(ss->irq_urb); - ss->irq_urb = NULL; - } - up(&(ss->irq_urb_sem)); - /* free the scatter-gather request block */ if (ss->current_sg) { kfree(ss->current_sg); @@ -810,8 +757,6 @@ /* Initialize the mutexes only when the struct is new */ init_completion(&(ss->notify)); - init_MUTEX_LOCKED(&(ss->ip_waitq)); - init_MUTEX(&(ss->irq_urb_sem)); init_MUTEX_LOCKED(&(ss->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 */