Some minor cleanups. First, some locking in the bus-reset. Next, we move current_sg into struct us_data (why make more memory allocation issues for ourselves?). Next, we change sm_state into a normal variable, since it shouldn't require atomic_t anytmore. Finally, we remove some references to a couple of flags that don't do anything anymore.
Greg, please apply.
Matt
# This is a BitKeeper generated patch for the following project:
# Project Name: greg k-h's linux 2.5 USB kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.1961 -> 1.1962
# drivers/usb/storage/usb.h 1.37 -> 1.38
# drivers/usb/storage/scsiglue.c 1.51 -> 1.52
# drivers/usb/storage/transport.c 1.100 -> 1.101
# drivers/usb/storage/isd200.c 1.37 -> 1.38
# drivers/usb/storage/usb.c 1.88 -> 1.89
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/06/15 [EMAIL PROTECTED] 1.1962
# Update the isd200 invoke_transport() routine to match the
# normal one.
#
# Fix device locking during the bus-reset routine.
#
# Embed current_sg in struct us_data.
#
# Make us->sm_state a regular int instead of an atomic_t.
#
# Remove a couple of references to the START_STOP and IGNORE_SER
# flag bits.
# --------------------------------------------
#
diff -Nru a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
--- a/drivers/usb/storage/isd200.c Sun Jun 15 16:23:56 2003
+++ b/drivers/usb/storage/isd200.c Sun Jun 15 16:23:56 2003
@@ -548,10 +548,9 @@
/* if the command gets aborted by the higher layers, we need to
* short-circuit all other processing
*/
- if (atomic_read(&us->sm_state) == US_STATE_ABORTING) {
- US_DEBUGP("-- transport indicates command was aborted\n");
- srb->result = DID_ABORT << 16;
- return;
+ if (us->sm_state == US_STATE_ABORTING) {
+ US_DEBUGP("-- command was aborted\n");
+ goto Handle_Abort;
}
switch (transferStatus) {
@@ -561,6 +560,11 @@
srb->result = SAM_STAT_GOOD;
break;
+ case USB_STOR_TRANSPORT_NO_SENSE:
+ US_DEBUGP("-- transport indicates protocol failure\n");
+ srb->result = SAM_STAT_CHECK_CONDITION;
+ return;
+
case USB_STOR_TRANSPORT_FAILED:
US_DEBUGP("-- transport indicates command failure\n");
need_auto_sense = 1;
@@ -591,10 +595,9 @@
if (need_auto_sense) {
result = isd200_read_regs(us);
- if (atomic_read(&us->sm_state) == US_STATE_ABORTING) {
+ if (us->sm_state == US_STATE_ABORTING) {
US_DEBUGP("-- auto-sense aborted\n");
- srb->result = DID_ABORT << 16;
- return;
+ goto Handle_Abort;
}
if (result == ISD200_GOOD) {
isd200_build_sense(us, srb);
@@ -603,8 +606,10 @@
/* If things are really okay, then let's show that */
if ((srb->sense_buffer[2] & 0xf) == 0x0)
srb->result = SAM_STAT_GOOD;
- } else
+ } else {
srb->result = DID_ERROR << 16;
+ /* Need reset here */
+ }
}
/* Regardless of auto-sense, if we _know_ we have an error
@@ -612,6 +617,16 @@
*/
if (transferStatus == USB_STOR_TRANSPORT_FAILED)
srb->result = SAM_STAT_CHECK_CONDITION;
+ return;
+
+ /* abort processing: the bulk-only transport requires a reset
+ * following an abort */
+ Handle_Abort:
+ srb->result = DID_ABORT << 16;
+
+ /* permit the reset transfer to take place */
+ clear_bit(US_FLIDX_ABORTING, &us->flags);
+ /* Need reset here */
}
#ifdef CONFIG_USB_STORAGE_DEBUG
diff -Nru a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
--- a/drivers/usb/storage/scsiglue.c Sun Jun 15 16:23:56 2003
+++ b/drivers/usb/storage/scsiglue.c Sun Jun 15 16:23:56 2003
@@ -141,16 +141,15 @@
static int usb_storage_queuecommand( Scsi_Cmnd *srb , void (*done)(Scsi_Cmnd *))
{
struct us_data *us = (struct us_data *)srb->device->host->hostdata[0];
- int state = atomic_read(&us->sm_state);
US_DEBUGP("queuecommand() called\n");
srb->host_scribble = (unsigned char *)us;
/* enqueue the command */
- if (state != US_STATE_IDLE || us->srb != NULL) {
+ if (us->sm_state != US_STATE_IDLE || us->srb != NULL) {
printk(KERN_ERR USB_STORAGE "Error in %s: "
"state = %d, us->srb = %p\n",
- __FUNCTION__, state, us->srb);
+ __FUNCTION__, us->sm_state, us->srb);
return SCSI_MLQUEUE_HOST_BUSY;
}
@@ -173,7 +172,6 @@
{
struct Scsi_Host *host = srb->device->host;
struct us_data *us = (struct us_data *) host->hostdata[0];
- int state = atomic_read(&us->sm_state);
US_DEBUGP("%s called\n", __FUNCTION__);
@@ -186,20 +184,20 @@
/* Normally the current state is RUNNING. If the control thread
* hasn't even started processing this command, the state will be
* IDLE. Anything else is a bug. */
- if (state != US_STATE_RUNNING && state != US_STATE_IDLE) {
+ if (us->sm_state != US_STATE_RUNNING
+ && us->sm_state != US_STATE_IDLE) {
printk(KERN_ERR USB_STORAGE "Error in %s: "
- "invalid state %d\n", __FUNCTION__, state);
+ "invalid state %d\n", __FUNCTION__, us->sm_state);
return FAILED;
}
/* Set state to ABORTING, set the ABORTING bit, and release the lock */
- atomic_set(&us->sm_state, US_STATE_ABORTING);
+ us->sm_state = US_STATE_ABORTING;
set_bit(US_FLIDX_ABORTING, &us->flags);
scsi_unlock(host);
- /* If the state was RUNNING, stop an ongoing USB transfer */
- if (state == US_STATE_RUNNING)
- usb_stor_stop_transport(us);
+ /* Stop an ongoing USB transfer */
+ usb_stor_stop_transport(us);
/* Wait for the aborted command to finish */
wait_for_completion(&us->notify);
@@ -216,32 +214,27 @@
static int usb_storage_device_reset( Scsi_Cmnd *srb )
{
struct us_data *us = (struct us_data *)srb->device->host->hostdata[0];
- int state = atomic_read(&us->sm_state);
int result;
US_DEBUGP("%s called\n", __FUNCTION__);
- if (state != US_STATE_IDLE) {
+ if (us->sm_state != US_STATE_IDLE) {
printk(KERN_ERR USB_STORAGE "Error in %s: "
- "invalid state %d\n", __FUNCTION__, state);
+ "invalid state %d\n", __FUNCTION__, us->sm_state);
return FAILED;
}
/* set the state and release the lock */
- atomic_set(&us->sm_state, US_STATE_RESETTING);
+ us->sm_state = US_STATE_RESETTING;
scsi_unlock(srb->device->host);
- /* lock the device pointers */
+ /* lock the device pointers and do the reset */
down(&(us->dev_semaphore));
-
- /* do the reset */
result = us->transport_reset(us);
-
- /* unlock */
up(&(us->dev_semaphore));
/* lock access to the state and clear it */
scsi_lock(srb->device->host);
- atomic_set(&us->sm_state, US_STATE_IDLE);
+ us->sm_state = US_STATE_IDLE;
return result;
}
@@ -252,42 +245,39 @@
static int usb_storage_bus_reset( Scsi_Cmnd *srb )
{
struct us_data *us = (struct us_data *)srb->device->host->hostdata[0];
- int state = atomic_read(&us->sm_state);
int result;
US_DEBUGP("%s called\n", __FUNCTION__);
- if (state != US_STATE_IDLE) {
+ if (us->sm_state != US_STATE_IDLE) {
printk(KERN_ERR USB_STORAGE "Error in %s: "
- "invalid state %d\n", __FUNCTION__, state);
+ "invalid state %d\n", __FUNCTION__, us->sm_state);
return FAILED;
}
/* set the state and release the lock */
- atomic_set(&us->sm_state, US_STATE_RESETTING);
+ us->sm_state = US_STATE_RESETTING;
scsi_unlock(srb->device->host);
/* The USB subsystem doesn't handle synchronisation between
- a device's several drivers. Therefore we reset only devices
- with just one interface, which we of course own.
- */
-
- //FIXME: needs locking against config changes
+ * a device's several drivers. Therefore we reset only devices
+ * with just one interface, which we of course own. */
- if (us->pusb_dev->actconfig->desc.bNumInterfaces == 1) {
-
- /* lock the device and attempt to reset the port */
- down(&(us->dev_semaphore));
- result = usb_reset_device(us->pusb_dev);
- up(&(us->dev_semaphore));
- US_DEBUGP("usb_reset_device returns %d\n", result);
- } else {
+ down(&(us->dev_semaphore));
+ if (test_bit(US_FLIDX_DISCONNECTING, &us->flags)) {
+ result = -EIO;
+ US_DEBUGP("Attempt to reset during disconnect\n");
+ } else if (us->pusb_dev->actconfig->desc.bNumInterfaces != 1) {
result = -EBUSY;
US_DEBUGP("Refusing to reset a multi-interface device\n");
+ } else {
+ result = usb_reset_device(us->pusb_dev);
+ US_DEBUGP("usb_reset_device returns %d\n", result);
}
+ up(&(us->dev_semaphore));
/* lock access to the state and clear it */
scsi_lock(srb->device->host);
- atomic_set(&us->sm_state, US_STATE_IDLE);
+ us->sm_state = US_STATE_IDLE;
return result < 0 ? FAILED : SUCCESS;
}
@@ -333,8 +323,6 @@
#define DO_FLAG(a) if (f & US_FL_##a) pos += sprintf(pos, " " #a)
DO_FLAG(SINGLE_LUN);
DO_FLAG(MODE_XLATE);
- DO_FLAG(START_STOP);
- DO_FLAG(IGNORE_SER);
DO_FLAG(SCM_MULT_TARG);
DO_FLAG(FIX_INQUIRY);
DO_FLAG(FIX_CAPACITY);
diff -Nru a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
--- a/drivers/usb/storage/transport.c Sun Jun 15 16:23:56 2003
+++ b/drivers/usb/storage/transport.c Sun Jun 15 16:23:56 2003
@@ -136,9 +136,9 @@
struct timer_list to_timer;
int status;
- /* don't submit URBS during abort/disconnect processing */
+ /* don't submit URBs during abort/disconnect processing */
if (us->flags & DONT_SUBMIT)
- return -ECONNRESET;
+ return -EIO;
/* set up data structures for the wakeup system */
init_completion(&urb_done);
@@ -299,17 +299,17 @@
return USB_STOR_XFER_ERROR;
return USB_STOR_XFER_STALLED;
- /* NAK - that means we've retried this a few times already */
+ /* timeout or excessively long NAK */
case -ETIMEDOUT:
- US_DEBUGP("-- device NAKed\n");
+ US_DEBUGP("-- timeout or NAK\n");
return USB_STOR_XFER_ERROR;
/* babble - the device tried to send more than we wanted to read */
case -EOVERFLOW:
- US_DEBUGP("-- Babble\n");
+ US_DEBUGP("-- babble\n");
return USB_STOR_XFER_LONG;
- /* the transfer was cancelled, presumably by an abort */
+ /* the transfer was cancelled by abort, disconnect, or timeout */
case -ECONNRESET:
US_DEBUGP("-- transfer cancelled\n");
return USB_STOR_XFER_ERROR;
@@ -319,6 +319,11 @@
US_DEBUGP("-- short read transfer\n");
return USB_STOR_XFER_SHORT;
+ /* abort or disconnect in progress */
+ case -EIO:
+ US_DEBUGP("-- abort or disconnect in progress\n");
+ return USB_STOR_XFER_ERROR;
+
/* the catch-all error case */
default:
US_DEBUGP("-- unknown error\n");
@@ -430,7 +435,7 @@
/* initialize the scatter-gather request block */
US_DEBUGP("%s: xfer %u bytes, %d entries\n", __FUNCTION__,
length, num_sg);
- result = usb_sg_init(us->current_sg, us->pusb_dev, pipe, 0,
+ result = usb_sg_init(&us->current_sg, us->pusb_dev, pipe, 0,
sg, num_sg, length, SLAB_NOIO);
if (result) {
US_DEBUGP("usb_sg_init returned %d\n", result);
@@ -447,19 +452,19 @@
/* cancel the request, if it hasn't been cancelled already */
if (test_and_clear_bit(US_FLIDX_SG_ACTIVE, &us->flags)) {
US_DEBUGP("-- cancelling sg request\n");
- usb_sg_cancel(us->current_sg);
+ usb_sg_cancel(&us->current_sg);
}
}
/* wait for the completion of the transfer */
- usb_sg_wait(us->current_sg);
+ usb_sg_wait(&us->current_sg);
clear_bit(US_FLIDX_SG_ACTIVE, &us->flags);
- result = us->current_sg->status;
+ result = us->current_sg.status;
if (act_len)
- *act_len = us->current_sg->bytes;
+ *act_len = us->current_sg.bytes;
return interpret_urb_result(us, pipe, length, result,
- us->current_sg->bytes);
+ us->current_sg.bytes);
}
/*
@@ -518,7 +523,7 @@
/* if the command gets aborted by the higher layers, we need to
* short-circuit all other processing
*/
- if (atomic_read(&us->sm_state) == US_STATE_ABORTING) {
+ if (us->sm_state == US_STATE_ABORTING) {
US_DEBUGP("-- command was aborted\n");
goto Handle_Abort;
}
@@ -650,7 +655,7 @@
srb->cmd_len = old_cmd_len;
memcpy(srb->cmnd, old_cmnd, MAX_COMMAND_SIZE);
- if (atomic_read(&us->sm_state) == US_STATE_ABORTING) {
+ if (us->sm_state == US_STATE_ABORTING) {
US_DEBUGP("-- auto-sense aborted\n");
goto Handle_Abort;
}
@@ -734,7 +739,7 @@
/* If we are waiting for a scatter-gather operation, cancel it. */
if (test_and_clear_bit(US_FLIDX_SG_ACTIVE, &us->flags)) {
US_DEBUGP("-- cancelling sg request\n");
- usb_sg_cancel(us->current_sg);
+ usb_sg_cancel(&us->current_sg);
}
}
diff -Nru a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
--- a/drivers/usb/storage/usb.c Sun Jun 15 16:23:56 2003
+++ b/drivers/usb/storage/usb.c Sun Jun 15 16:23:56 2003
@@ -328,13 +328,13 @@
scsi_lock(host);
/* has the command been aborted *already* ? */
- if (atomic_read(&us->sm_state) == US_STATE_ABORTING) {
+ if (us->sm_state == US_STATE_ABORTING) {
us->srb->result = DID_ABORT << 16;
goto SkipForAbort;
}
/* set the state and release the lock */
- atomic_set(&us->sm_state, US_STATE_RUNNING);
+ us->sm_state = US_STATE_RUNNING;
scsi_unlock(host);
/* lock the device pointers */
@@ -404,12 +404,12 @@
* sm_state == US_STATE_ABORTING, not srb->result == DID_ABORT,
* because an abort request might be received after all the
* USB processing was complete. */
- if (atomic_read(&us->sm_state) == US_STATE_ABORTING)
+ if (us->sm_state == US_STATE_ABORTING)
complete(&(us->notify));
/* empty the queue, reset the state, and release the lock */
us->srb = NULL;
- atomic_set(&us->sm_state, US_STATE_IDLE);
+ us->sm_state = US_STATE_IDLE;
scsi_unlock(host);
} /* for (;;) */
@@ -447,7 +447,7 @@
if (dev->descriptor.iProduct)
usb_string(dev, dev->descriptor.iProduct,
us->product, sizeof(us->product));
- if (dev->descriptor.iSerialNumber && !(us->flags & US_FL_IGNORE_SER))
+ if (dev->descriptor.iSerialNumber)
usb_string(dev, dev->descriptor.iSerialNumber,
us->serial, sizeof(us->serial));
@@ -698,13 +698,6 @@
return -ENOMEM;
}
- US_DEBUGP("Allocating scatter-gather request block\n");
- us->current_sg = kmalloc(sizeof(*us->current_sg), GFP_KERNEL);
- if (!us->current_sg) {
- US_DEBUGP("allocation failed\n");
- return -ENOMEM;
- }
-
/* Lock the device while we carry out the next two operations */
down(&us->dev_semaphore);
@@ -720,7 +713,7 @@
up(&us->dev_semaphore);
/* Start up our control thread */
- atomic_set(&us->sm_state, US_STATE_IDLE);
+ us->sm_state = US_STATE_IDLE;
p = kernel_thread(usb_stor_control_thread, us, CLONE_VM);
if (p < 0) {
printk(KERN_WARNING USB_STORAGE
@@ -782,7 +775,7 @@
*/
if (us->pid) {
US_DEBUGP("-- sending exit command to thread\n");
- BUG_ON(atomic_read(&us->sm_state) != US_STATE_IDLE);
+ BUG_ON(us->sm_state != US_STATE_IDLE);
us->srb = NULL;
up(&(us->sema));
wait_for_completion(&(us->notify));
@@ -800,8 +793,6 @@
}
/* Free the USB control blocks */
- if (us->current_sg)
- kfree(us->current_sg);
if (us->current_urb)
usb_free_urb(us->current_urb);
if (us->dr)
diff -Nru a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
--- a/drivers/usb/storage/usb.h Sun Jun 15 16:23:56 2003
+++ b/drivers/usb/storage/usb.h Sun Jun 15 16:23:56 2003
@@ -139,7 +139,7 @@
/* thread information */
int pid; /* control thread */
- atomic_t sm_state; /* what we are doing */
+ int sm_state; /* what we are doing */
/* interrupt communications data */
unsigned char irqdata[2]; /* data from USB IRQ */
@@ -147,7 +147,7 @@
/* control and bulk communications data */
struct urb *current_urb; /* non-int USB requests */
struct usb_ctrlrequest *dr; /* control requests */
- struct usb_sg_request *current_sg; /* scatter-gather USB */
+ struct usb_sg_request current_sg; /* scatter-gather USB */
/* the semaphore for sleeping the control thread */
struct semaphore sema; /* to sleep thread on */
--
Matthew Dharm Home: [EMAIL PROTECTED]
Maintainer, Linux USB Mass Storage Driver
I need a computer?
-- Customer
User Friendly, 2/19/1998
pgp00000.pgp
Description: PGP signature
