[RFC] usb: storage: Auto-suspend when media is removed.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



(I recently found this patch languishing in my git tree.  I've updated it
to 2.6.35-rc4, but I haven't retested it.)

A while back, Oliver Neukum made a patch to suspend mass storage devices
between SCSI commands.  This patch uses some of the work from Oliver's
patch.

Unfortunately, Oliver's patch made SD readers lose their minds.  After a
suspend, all previous internal state is lost and they can't tell
whether a card had been recently inserted or removed.  After a suspend,
the first TEST_UNIT_READY command sent to an SD card reader will report
"Media may have changed", and the next TEST_UNIT_READY command will report
actual media presence or removal.  If the SD readers are suspended between
TEST_UNIT_READY commands, they will continually report "Media may have
changed", and no media change will ever be detected.

Disable autosuspend for a mass storage device until we know medium is not
present, by peeking into the results of a TEST_UNIT_READY command.  I
matched against the key, asc, and ascq fields, but I believe I only really
need to match the asc and asq fields.

This patch is only useful if the autosuspend timeout is set to 0 seconds
(an immediate suspend).  You can generate the udev rule to enable
autosuspend and set the timeout with my usb-pm tool script
(vid-pid-to-udev-rule.sh) at http://git.moblin.org/cgit.cgi/usb-pm-tool/

This patch would be even more useful if we could get userspace to stop
polling for new media so often.

This patch works fine with empty SD card readers.  Some thumb drives
will also autosuspend when they are unmounted or ejected from the Gnome
filesystem GUI.  Other thumb drives always report "Medium may have
changed" if they need to flash stupid LEDs, like the original Linux
Plumbers Conference thumb drive.

Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
---
 drivers/usb/storage/scsiglue.c  |   11 +++++++++-
 drivers/usb/storage/transport.c |   42 +++++++++++++++++++++++++++++++++++++++
 drivers/usb/storage/usb.c       |   38 ++++++++++++++++++++++++++++++++--
 drivers/usb/storage/usb.h       |    7 ++++++
 4 files changed, 94 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index d8d98cf..85f43e9 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -351,14 +351,23 @@ static int command_abort(struct scsi_cmnd *srb)
 static int device_reset(struct scsi_cmnd *srb)
 {
 	struct us_data *us = host_to_us(srb->device->host);
-	int result;
+	int result, resumption;
 
 	US_DEBUGP("%s called\n", __func__);
 
+	resumption = usb_autopm_get_interface(us->pusb_intf);
+	/*
+	 * in the error case we go on anyway
+	 * as this is a measure of last resort
+	 */
+
 	/* lock the device pointers and do the reset */
 	mutex_lock(&(us->dev_mutex));
 	result = us->transport_reset(us);
 	mutex_unlock(&us->dev_mutex);
+	if (resumption == 0)
+		usb_autopm_put_interface(us->pusb_intf);
+	
 
 	return result < 0 ? FAILED : SUCCESS;
 }
diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 4471642..3bc73d3 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -819,6 +819,48 @@ Retry_Sense:
 				srb->sense_buffer[2] = HARDWARE_ERROR;
 			}
 		}
+
+		/* See if new media is present or tracked media is removed,
+		 * so that we can disable auto-suspend or enable auto-suspend.
+		 *
+		 * We conservatively say new media is present when an auto-sense
+		 * after a TEST_UNIT_READY command fails with the "Media may
+		 * have changed" status, because SD card readers are confused
+		 * about the state of media when they first come out of suspend.
+		 * The first TEST_UNIT_READY command will say "Media may have
+		 * changed", and the next TEST_UNIT_READY command will succeed
+		 * to say medium is present or fail and the auto-sense will say
+		 * "No medium present".  If we auto-suspend the device between
+		 * the two TEST_UNIT_READY commands, the device will always
+		 * report "Medium may have changed."
+		 *
+		 * Disable auto-suspend until we really know the state of the
+		 * media.
+		 */
+		/* Media may have changed, key = 0x6, asc = 0x28, ascq = 0x0 */
+		if (!us->media_present &&
+				srb->cmnd[0] == TEST_UNIT_READY &&
+				(srb->sense_buffer[2] & 0xf) == 0x6 &&
+				srb->sense_buffer[12] == 0x28 &&
+				srb->sense_buffer[13] == 0x0) {
+			us->media_present = 1;
+			usb_autopm_get_interface_async(us->pusb_intf);
+		}
+		/* Media not present, key = 0x2, asc = 0x3a, ascq = 0x0 */
+		if (us->media_present &&
+				srb->cmnd[0] == TEST_UNIT_READY &&
+				(srb->sense_buffer[2] & 0xf) == 0x2 &&
+				srb->sense_buffer[12] == 0x3a &&
+				srb->sense_buffer[13] == 0x0) {
+			us->media_present = 0;
+			usb_autopm_put_interface_async(us->pusb_intf);
+		}
+	} else {
+		/* TEST_UNIT_READY succeeded, media is now present. */
+		if (!us->media_present && srb->cmnd[0] == TEST_UNIT_READY) {
+			us->media_present = 1;
+			usb_autopm_get_interface_async(us->pusb_intf);
+		}
 	}
 
 	/* Did we transfer less than the minimum amount required? */
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index a7d0bf9..f71f3ac 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -143,9 +143,6 @@ int usb_stor_suspend(struct usb_interface *iface, pm_message_t message)
 	if (us->suspend_resume_hook)
 		(us->suspend_resume_hook)(us, US_SUSPEND);
 
-	/* When runtime PM is working, we'll set a flag to indicate
-	 * whether we should autoresume when a SCSI request arrives. */
-
 	mutex_unlock(&us->dev_mutex);
 	return 0;
 }
@@ -265,6 +262,7 @@ static int usb_stor_control_thread(void * __us)
 {
 	struct us_data *us = (struct us_data *)__us;
 	struct Scsi_Host *host = us_to_host(us);
+	int autopm_rc;
 
 	for(;;) {
 		US_DEBUGP("*** thread sleeping.\n");
@@ -273,6 +271,9 @@ static int usb_stor_control_thread(void * __us)
 
 		US_DEBUGP("*** thread awakened.\n");
 
+		/* Autoresume the device */
+		autopm_rc = usb_autopm_get_interface(us->pusb_intf);
+
 		/* lock the device pointers */
 		mutex_lock(&(us->dev_mutex));
 
@@ -332,6 +333,12 @@ static int usb_stor_control_thread(void * __us)
 			us->srb->result = SAM_STAT_GOOD;
 		}
 
+		/* Did the autoresume fail? */
+		else if (autopm_rc < 0) {
+			US_DEBUGP("Could not wake device\n");
+			us->srb->result = DID_ERROR << 16;
+		}
+
 		/* we've got a command, let's do it! */
 		else {
 			US_DEBUG(usb_stor_show_command(us->srb));
@@ -370,6 +377,10 @@ SkipForAbort:
 
 		/* unlock the device pointers */
 		mutex_unlock(&us->dev_mutex);
+
+		/* Start an autosuspend if media was removed. */
+		if (autopm_rc == 0)
+			usb_autopm_put_interface(us->pusb_intf);
 	} /* for (;;) */
 
 	/* Wait until we are told to stop */
@@ -753,6 +764,20 @@ static void usb_stor_release_resources(struct us_data *us)
 	/* Free the extra data and the URB */
 	kfree(us->extra);
 	usb_free_urb(us->current_urb);
+
+	/* Balance calls to usb_autopm_get_interface() and
+	 * usb_autopm_put_interface().
+	 *
+	 * If we've disabled auto-suspend because medium is present, then
+	 * re-enable it when the driver is being disconnected.
+	 *
+	 * us->media_present is zeroed on allocation.  If we never grabbed the
+	 * interface (meaning probe failed early), this variable will be zeroed,
+	 * the device's autopm counter will remain unchanged, and
+	 * auto-suspend will be enabled.
+	 */
+	if (us->media_present)
+		usb_autopm_put_interface(us->pusb_intf);
 }
 
 /* Dissociate from the USB device */
@@ -968,6 +993,12 @@ int usb_stor_probe2(struct us_data *us)
 		goto BadDevice;
 	}
 
+	/*
+	 * Prevent the device from suspending until we know
+	 * if it has removable media that is not present.
+	 */
+	us->media_present = 1;
+	usb_autopm_get_interface(us->pusb_intf);
 	wake_up_process(th);
 
 	return 0;
@@ -1041,6 +1072,7 @@ static struct usb_driver usb_storage_driver = {
 	.post_reset =	usb_stor_post_reset,
 	.id_table =	usb_storage_usb_ids,
 	.soft_unbind =	1,
+	.supports_autosuspend = 1,
 };
 
 static int __init usb_stor_init(void)
diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
index 89d3bff..30c77ee 100644
--- a/drivers/usb/storage/usb.h
+++ b/drivers/usb/storage/usb.h
@@ -153,6 +153,13 @@ struct us_data {
 	extra_data_destructor	extra_destructor;/* extra data destructor   */
 #ifdef CONFIG_PM
 	pm_hook			suspend_resume_hook;
+
+	/* Cleared when a TEST_UNIT_READY command fails and the auto-sense
+	 * indicates there is no media present.  This means a device with
+	 * removable media is safe to auto-suspend (until the next
+	 * TEST_UNIT_READY command).
+	 */
+	int			media_present:1;
 #endif
 
 	/* hacks for READ CAPACITY bug handling */
-- 
1.6.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux