ChangeSet 1.1254.1.77, 2003/06/01 23:00:01-07:00, mdharm-usb@one-eyed-alien.net [PATCH] USB: usb-storage: usb_stor_control_msg() and stuff This patch replaces usb_control_msg() with usb_stor_control_msg() everywhere, which allows better abort/disconnect processing. Some comments are fixed-up. The GetMaxLUN function is moved later so URBs are initialized (now that it uses the new control_msg() ). There is also some locking cleanup during reset. drivers/usb/storage/freecom.c | 6 ++--- drivers/usb/storage/initializers.c | 2 - drivers/usb/storage/initializers.h | 1 drivers/usb/storage/scsiglue.c | 40 +++++++++++++++++++++++-------------- drivers/usb/storage/transport.c | 11 ++++------ drivers/usb/storage/usb.c | 13 ++++++++---- 6 files changed, 44 insertions(+), 29 deletions(-) diff -Nru a/drivers/usb/storage/freecom.c b/drivers/usb/storage/freecom.c --- a/drivers/usb/storage/freecom.c Mon Jun 2 11:20:17 2003 +++ b/drivers/usb/storage/freecom.c Mon Jun 2 11:20:17 2003 @@ -399,7 +399,7 @@ } } - result = usb_control_msg(us->pusb_dev, us->recv_ctrl_pipe, + result = usb_stor_control_msg(us, us->recv_ctrl_pipe, 0x4c, 0xc0, 0x4346, 0x0, buffer, 0x20, 3*HZ); buffer[32] = '\0'; US_DEBUGP("String returned from FC init is: %s\n", buffer); @@ -411,7 +411,7 @@ */ /* send reset */ - result = usb_control_msg(us->pusb_dev, us->send_ctrl_pipe, + result = usb_stor_control_msg(us, us->send_ctrl_pipe, 0x4d, 0x40, 0x24d8, 0x0, NULL, 0x0, 3*HZ); US_DEBUGP("result from activate reset is %d\n", result); @@ -419,7 +419,7 @@ mdelay(250); /* clear reset */ - result = usb_control_msg(us->pusb_dev, us->send_ctrl_pipe, + result = usb_stor_control_msg(us, us->send_ctrl_pipe, 0x4d, 0x40, 0x24f8, 0x0, NULL, 0x0, 3*HZ); US_DEBUGP("result from clear reset is %d\n", result); diff -Nru a/drivers/usb/storage/initializers.c b/drivers/usb/storage/initializers.c --- a/drivers/usb/storage/initializers.c Mon Jun 2 11:20:17 2003 +++ b/drivers/usb/storage/initializers.c Mon Jun 2 11:20:17 2003 @@ -50,7 +50,7 @@ int result; US_DEBUGP("Attempting to init eUSCSI bridge...\n"); - result = usb_control_msg(us->pusb_dev, us->send_ctrl_pipe, + result = usb_stor_control_msg(us, us->send_ctrl_pipe, 0x0C, USB_RECIP_INTERFACE | USB_TYPE_VENDOR, 0x01, 0x0, &data, 0x1, 5*HZ); US_DEBUGP("-- result is %d\n", result); diff -Nru a/drivers/usb/storage/initializers.h b/drivers/usb/storage/initializers.h --- a/drivers/usb/storage/initializers.h Mon Jun 2 11:20:17 2003 +++ b/drivers/usb/storage/initializers.h Mon Jun 2 11:20:17 2003 @@ -39,6 +39,7 @@ #include #include "usb.h" +#include "transport.h" /* This places the Shuttle/SCM USB<->SCSI bridge devices in multi-target * mode */ diff -Nru a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c --- a/drivers/usb/storage/scsiglue.c Mon Jun 2 11:20:17 2003 +++ b/drivers/usb/storage/scsiglue.c Mon Jun 2 11:20:17 2003 @@ -219,7 +219,7 @@ int state = atomic_read(&us->sm_state); int result; - US_DEBUGP("device_reset() called\n" ); + US_DEBUGP("%s called\n", __FUNCTION__); if (state != US_STATE_IDLE) { printk(KERN_ERR USB_STORAGE "Error in %s: " "invalid state %d\n", __FUNCTION__, state); @@ -245,39 +245,49 @@ return result; } -/* This resets the device port */ +/* This resets the device's USB port. */ /* It refuses to work if there's more than one interface in - this device, so that other users are not affected. */ + * the device, so that other users are not affected. */ /* This is always called with scsi_lock(srb->host) held */ - static int usb_storage_bus_reset( Scsi_Cmnd *srb ) { - struct us_data *us; + struct us_data *us = (struct us_data *)srb->device->host->hostdata[0]; + int state = atomic_read(&us->sm_state); int result; - /* we use the usb_reset_device() function to handle this for us */ - US_DEBUGP("bus_reset() called\n"); + US_DEBUGP("%s called\n", __FUNCTION__); + if (state != US_STATE_IDLE) { + printk(KERN_ERR USB_STORAGE "Error in %s: " + "invalid state %d\n", __FUNCTION__, state); + return FAILED; + } + + /* set the state and release the lock */ + atomic_set(&us->sm_state, US_STATE_RESETTING); scsi_unlock(srb->device->host); - us = (struct us_data *)srb->device->host->hostdata[0]; /* The USB subsystem doesn't handle synchronisation between a device's several drivers. Therefore we reset only devices - with one interface which we of course own. + with just one interface, which we of course own. */ - + //FIXME: needs locking against config changes - - if ( us->pusb_dev->actconfig->desc.bNumInterfaces == 1) { - /* attempt to reset the port */ + + 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 { result = -EBUSY; - US_DEBUGP("cannot reset a multiinterface device. failing to reset.\n"); + US_DEBUGP("Refusing to reset a multi-interface device\n"); } - US_DEBUGP("bus_reset() complete\n"); + /* lock access to the state and clear it */ scsi_lock(srb->device->host); + atomic_set(&us->sm_state, US_STATE_IDLE); return result < 0 ? FAILED : SUCCESS; } diff -Nru a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c --- a/drivers/usb/storage/transport.c Mon Jun 2 11:20:17 2003 +++ b/drivers/usb/storage/transport.c Mon Jun 2 11:20:17 2003 @@ -875,11 +875,8 @@ unsigned char data; int result; - /* Issue the command -- use usb_control_msg() because this is - * not a scsi queued-command. Also note that at this point the - * cached pipe values have not yet been stored. */ - result = usb_control_msg(us->pusb_dev, - usb_rcvctrlpipe(us->pusb_dev, 0), + /* issue the command */ + result = usb_stor_control_msg(us, us->recv_ctrl_pipe, US_BULK_GET_MAX_LUN, USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE, @@ -1027,10 +1024,12 @@ return FAILED; } - /* long wait for reset */ + /* long wait for reset, so unlock to allow disconnects */ + up(&us->dev_semaphore); set_current_state(TASK_UNINTERRUPTIBLE); schedule_timeout(HZ*6); set_current_state(TASK_RUNNING); + down(&us->dev_semaphore); US_DEBUGP("Soft reset: clearing bulk-in endpoint halt\n"); result = usb_stor_clear_halt(us, us->recv_bulk_pipe); diff -Nru a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c --- a/drivers/usb/storage/usb.c Mon Jun 2 11:20:17 2003 +++ b/drivers/usb/storage/usb.c Mon Jun 2 11:20:17 2003 @@ -412,9 +412,11 @@ US_DEBUGP("scsi command aborted\n"); } - /* in case an abort request was received after the command - * completed, we must use a separate test to see whether - * we need to signal that the abort has finished */ + /* If an abort request was received we need to signal that + * the abort has finished. The proper test for this is + * 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) complete(&(us->notify)); @@ -715,7 +717,6 @@ us->transport_name = "Bulk"; us->transport = usb_stor_Bulk_transport; us->transport_reset = usb_stor_Bulk_reset; - us->max_lun = usb_stor_Bulk_max_lun(us); break; #ifdef CONFIG_USB_STORAGE_HP8200e @@ -841,6 +842,10 @@ /* allocate the URB, the usb_ctrlrequest, and the IRQ URB */ if (usb_stor_allocate_urbs(us)) goto BadDevice; + + /* For bulk-only devices, determine the max LUN value */ + if (us->protocol == US_PR_BULK) + us->max_lun = usb_stor_Bulk_max_lun(us); /* * Since this is a new device, we need to generate a scsi