USB: add reset_resume method

This patch (as918) introduces a new USB driver method: reset_resume.
It is called when a device needs to be reset as part of a resume
procedure (whether because of a device quirk or because of the
USB-Persist facility), thereby taking over a role formerly assigned to
the post_reset method.  As a consequence, post_reset no longer needs
an argument indicating whether it is being called as part of a
reset-resume.  This separation of functions makes the code clearer.

In addition, the pre_reset and post_reset method return types are
changed; they now must return an error code.  The return value is
unused at present, but at some later time we may unbind drivers and
re-probe if they encounter an error during reset handling.

The existing pre_reset and post_reset methods in the usbhid,
usb-storage, and hub drivers are updated to match the new
requirements.  For usbhid the post_reset routine is also used for
reset_resume (duplicate method pointers); for the other drivers a new
reset_resume routine is added.  The change to hub.c looks bigger than
it really is, because mark_children_for_reset_resume() gets moved down
next to the new hub_reset_resume() routine.

A minor change to usb-storage makes the usb_stor_report_bus_reset()
routine acquire the host lock instead of requiring the caller to hold
it already.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
CC: Matthew Dharm <mdharm-usb@one-eyed-alien.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

drivers/hid/usbhid/hid-core.c

@@ -1009,20 +1009,22 @@ static int hid_resume(struct usb_interface *intf)
 }
 
 /* Treat USB reset pretty much the same as suspend/resume */
-static void hid_pre_reset(struct usb_interface *intf)
+static int hid_pre_reset(struct usb_interface *intf)
 {
 	/* FIXME: What if the interface is already suspended? */
 	hid_suspend(intf, PMSG_ON);
+	return 0;
 }
 
-static void hid_post_reset(struct usb_interface *intf, int reset_resume)
+/* Same routine used for post_reset and reset_resume */
+static int hid_post_reset(struct usb_interface *intf)
 {
 	struct usb_device *dev = interface_to_usbdev (intf);
 
 	hid_set_idle(dev, intf->cur_altsetting->desc.bInterfaceNumber, 0, 0);
 	/* FIXME: Any more reinitialization needed? */
 
-	hid_resume(intf);
+	return hid_resume(intf);
 }
 
 static struct usb_device_id hid_usb_ids [] = {
@@ -1039,6 +1041,7 @@ static struct usb_driver hid_driver = {
 	.disconnect =	hid_disconnect,
 	.suspend =	hid_suspend,
 	.resume =	hid_resume,
+	.reset_resume =	hid_post_reset,
 	.pre_reset =	hid_pre_reset,
 	.post_reset =	hid_post_reset,
 	.id_table =	hid_usb_ids,

drivers/usb/core/driver.c

@@ -915,21 +915,37 @@ static int usb_resume_interface(struct usb_interface *intf, int reset_resume)
 	}
 	driver = to_usb_driver(intf->dev.driver);
 
-	if (reset_resume && driver->post_reset)
-		driver->post_reset(intf, reset_resume);
-	else if (driver->resume) {
+	if (reset_resume) {
+		if (driver->reset_resume) {
+			status = driver->reset_resume(intf);
+			if (status)
+				dev_err(&intf->dev, "%s error %d\n",
+						"reset_resume", status);
+		} else {
+			// status = -EOPNOTSUPP;
+			dev_warn(&intf->dev, "no %s for driver %s?\n",
+					"reset_resume", driver->name);
+		}
+	} else {
+		if (driver->resume) {
 			status = driver->resume(intf);
 			if (status)
 				dev_err(&intf->dev, "%s error %d\n",
 						"resume", status);
-	} else
-		dev_warn(&intf->dev, "no resume for driver %s?\n",
-				driver->name);
+		} else {
+			// status = -EOPNOTSUPP;
+			dev_warn(&intf->dev, "no %s for driver %s?\n",
+					"resume", driver->name);
+		}
+	}
 
 done:
 	dev_vdbg(&intf->dev, "%s: status %d\n", __FUNCTION__, status);
 	if (status == 0)
 		mark_active(intf);
+
+	/* FIXME: Unbind the driver and reprobe if the resume failed
+	 * (not possible if auto_pm is set) */
 	return status;
 }
 
@@ -966,6 +982,18 @@ static int autosuspend_check(struct usb_device *udev)
 						"for autosuspend\n");
 				return -EOPNOTSUPP;
 			}
+
+			/* Don't allow autosuspend if the device will need
+			 * a reset-resume and any of its interface drivers
+			 * doesn't include support.
+			 */
+			if (udev->quirks & USB_QUIRK_RESET_RESUME) {
+				struct usb_driver *driver;
+
+				driver = to_usb_driver(intf->dev.driver);
+				if (!driver->reset_resume)
+					return -EOPNOTSUPP;
+			}
 		}
 	}
 
@@ -1146,7 +1174,8 @@ static int usb_resume_both(struct usb_device *udev)
 			status = usb_autoresume_device(parent);
 			if (status == 0) {
 				status = usb_resume_device(udev);
-				if (status) {
+				if (status || udev->state ==
+						USB_STATE_NOTATTACHED) {
 					usb_autosuspend_device(parent);
 
 					/* It's possible usb_resume_device()

drivers/usb/core/hub.c

@@ -605,73 +605,26 @@ static void disconnect_all_children(struct usb_hub *hub, int logical)
 	}
 }
 
-#ifdef	CONFIG_USB_PERSIST
-
-#define USB_PERSIST	1
-
-/* For "persistent-device" resets we must mark the child devices for reset
- * and turn off a possible connect-change status (so khubd won't disconnect
- * them later).
- */
-static void mark_children_for_reset_resume(struct usb_hub *hub)
-{
-	struct usb_device *hdev = hub->hdev;
-	int port1;
-
-	for (port1 = 1; port1 <= hdev->maxchild; ++port1) {
-		struct usb_device *child = hdev->children[port1-1];
-
-		if (child) {
-			child->reset_resume = 1;
-			clear_port_feature(hdev, port1,
-					USB_PORT_FEAT_C_CONNECTION);
-		}
-	}
-}
-
-#else
-
-#define USB_PERSIST	0
-
-static inline void mark_children_for_reset_resume(struct usb_hub *hub)
-{ }
-
-#endif	/* CONFIG_USB_PERSIST */
-
 /* caller has locked the hub device */
-static void hub_pre_reset(struct usb_interface *intf)
+static int hub_pre_reset(struct usb_interface *intf)
 {
 	struct usb_hub *hub = usb_get_intfdata(intf);
 
-	/* This routine doesn't run as part of a reset-resume, so it's safe
-	 * to disconnect all the drivers below the hub.
-	 */
 	disconnect_all_children(hub, 0);
 	hub_quiesce(hub);
+	return 0;
 }
 
 /* caller has locked the hub device */
-static void hub_post_reset(struct usb_interface *intf, int reset_resume)
+static int hub_post_reset(struct usb_interface *intf)
 {
 	struct usb_hub *hub = usb_get_intfdata(intf);
 
 	hub_power_on(hub);
-	if (reset_resume) {
-		if (USB_PERSIST)
-			mark_children_for_reset_resume(hub);
-		else {
-			/* Reset-resume doesn't call pre_reset, so we have to
-			 * disconnect the children here.  But we may not lock
-			 * the child devices, so we have to do a "logical"
-			 * disconnect.
-			 */
-			disconnect_all_children(hub, 1);
-		}
-	}
 	hub_activate(hub);
+	return 0;
 }
 
-
 static int hub_configure(struct usb_hub *hub,
 	struct usb_endpoint_descriptor *endpoint)
 {
@@ -1931,6 +1884,58 @@ static int hub_resume(struct usb_interface *intf)
 	return 0;
 }
 
+#ifdef	CONFIG_USB_PERSIST
+
+#define USB_PERSIST	1
+
+/* For "persistent-device" resets we must mark the child devices for reset
+ * and turn off a possible connect-change status (so khubd won't disconnect
+ * them later).
+ */
+static void mark_children_for_reset_resume(struct usb_hub *hub)
+{
+	struct usb_device *hdev = hub->hdev;
+	int port1;
+
+	for (port1 = 1; port1 <= hdev->maxchild; ++port1) {
+		struct usb_device *child = hdev->children[port1-1];
+
+		if (child) {
+			child->reset_resume = 1;
+			clear_port_feature(hdev, port1,
+					USB_PORT_FEAT_C_CONNECTION);
+		}
+	}
+}
+
+#else
+
+#define USB_PERSIST	0
+
+static inline void mark_children_for_reset_resume(struct usb_hub *hub)
+{ }
+
+#endif	/* CONFIG_USB_PERSIST */
+
+static int hub_reset_resume(struct usb_interface *intf)
+{
+	struct usb_hub *hub = usb_get_intfdata(intf);
+
+	hub_power_on(hub);
+	if (USB_PERSIST)
+		mark_children_for_reset_resume(hub);
+	else {
+		/* Reset-resume doesn't call pre_reset, so we have to
+		 * disconnect the children here.  But we may not lock
+		 * the child devices, so we have to do a "logical"
+		 * disconnect.
+		 */
+		disconnect_all_children(hub, 1);
+	}
+	hub_activate(hub);
+	return 0;
+}
+
 #else	/* CONFIG_PM */
 
 static inline int remote_wakeup(struct usb_device *udev)
@@ -1940,6 +1945,7 @@ static inline int remote_wakeup(struct usb_device *udev)
 
 #define hub_suspend		NULL
 #define hub_resume		NULL
+#define hub_reset_resume	NULL
 #endif
 
 
@@ -2768,6 +2774,7 @@ static struct usb_driver hub_driver = {
 	.disconnect =	hub_disconnect,
 	.suspend =	hub_suspend,
 	.resume =	hub_resume,
+	.reset_resume =	hub_reset_resume,
 	.pre_reset =	hub_pre_reset,
 	.post_reset =	hub_post_reset,
 	.ioctl =	hub_ioctl,
@@ -3021,6 +3028,7 @@ int usb_reset_composite_device(struct usb_device *udev,
 				drv = to_usb_driver(cintf->dev.driver);
 				if (drv->pre_reset)
 					(drv->pre_reset)(cintf);
+	/* FIXME: Unbind if pre_reset returns an error or isn't defined */
 			}
 		}
 	}
@@ -3038,7 +3046,8 @@ int usb_reset_composite_device(struct usb_device *udev,
 					cintf->dev.driver) {
 				drv = to_usb_driver(cintf->dev.driver);
 				if (drv->post_reset)
-					(drv->post_reset)(cintf, 0);
+					(drv->post_reset)(cintf);
+	/* FIXME: Unbind if post_reset returns an error or isn't defined */
 			}
 			if (cintf != iface)
 				up(&cintf->dev.sem);

drivers/usb/storage/scsiglue.c

@@ -321,10 +321,14 @@ void usb_stor_report_device_reset(struct us_data *us)
 
 /* Report a driver-initiated bus reset to the SCSI layer.
  * Calling this for a SCSI-initiated reset is unnecessary but harmless.
- * The caller must own the SCSI host lock. */
+ * The caller must not own the SCSI host lock. */
 void usb_stor_report_bus_reset(struct us_data *us)
 {
-	scsi_report_bus_reset(us_to_host(us), 0);
+	struct Scsi_Host *host = us_to_host(us);
+
+	scsi_lock(host);
+	scsi_report_bus_reset(host, 0);
+	scsi_unlock(host);
 }
 
 /***********************************************************************

drivers/usb/storage/usb.c

@@ -219,6 +219,20 @@ static int storage_resume(struct usb_interface *iface)
 	return 0;
 }
 
+static int storage_reset_resume(struct usb_interface *iface)
+{
+	struct us_data *us = usb_get_intfdata(iface);
+
+	US_DEBUGP("%s\n", __FUNCTION__);
+
+	/* Report the reset to the SCSI core */
+	usb_stor_report_bus_reset(us);
+
+	/* FIXME: Notify the subdrivers that they need to reinitialize
+	 * the device */
+	return 0;
+}
+
 #endif /* CONFIG_PM */
 
 /*
@@ -226,7 +240,7 @@ static int storage_resume(struct usb_interface *iface)
  * a USB port reset, whether from this driver or a different one.
  */
 
-static void storage_pre_reset(struct usb_interface *iface)
+static int storage_pre_reset(struct usb_interface *iface)
 {
 	struct us_data *us = usb_get_intfdata(iface);
 
@@ -234,26 +248,23 @@ static void storage_pre_reset(struct usb_interface *iface)
 
 	/* Make sure no command runs during the reset */
 	mutex_lock(&us->dev_mutex);
+	return 0;
 }
 
-static void storage_post_reset(struct usb_interface *iface, int reset_resume)
+static int storage_post_reset(struct usb_interface *iface)
 {
 	struct us_data *us = usb_get_intfdata(iface);
 
 	US_DEBUGP("%s\n", __FUNCTION__);
 
 	/* Report the reset to the SCSI core */
-	scsi_lock(us_to_host(us));
 	usb_stor_report_bus_reset(us);
-	scsi_unlock(us_to_host(us));
 
 	/* FIXME: Notify the subdrivers that they need to reinitialize
 	 * the device */
 
-	/* If this is a reset-resume then the pre_reset routine wasn't
-	 * called, so we don't need to unlock the mutex. */
-	if (!reset_resume)
 	mutex_unlock(&us->dev_mutex);
+	return 0;
 }
 
 /*
@@ -1061,6 +1072,7 @@ static struct usb_driver usb_storage_driver = {
 #ifdef CONFIG_PM
 	.suspend =	storage_suspend,
 	.resume =	storage_resume,
+	.reset_resume =	storage_reset_resume,
 #endif
 	.pre_reset =	storage_pre_reset,
 	.post_reset =	storage_post_reset,

include/linux/usb.h

@@ -839,6 +839,8 @@ struct usbdrv_wrap {
  *	do (or don't) show up otherwise in the filesystem.
  * @suspend: Called when the device is going to be suspended by the system.
  * @resume: Called when the device is being resumed by the system.
+ * @reset_resume: Called when the suspended device has been reset instead
+ *	of being resumed.
  * @pre_reset: Called by usb_reset_composite_device() when the device
  *	is about to be reset.
  * @post_reset: Called by usb_reset_composite_device() after the device
@@ -885,9 +887,10 @@ struct usb_driver {
 
 	int (*suspend) (struct usb_interface *intf, pm_message_t message);
 	int (*resume) (struct usb_interface *intf);
+	int (*reset_resume)(struct usb_interface *intf);
 
-	void (*pre_reset) (struct usb_interface *intf);
-	void (*post_reset) (struct usb_interface *intf, int reset_resume);
+	int (*pre_reset)(struct usb_interface *intf);
+	int (*post_reset)(struct usb_interface *intf);
 
 	const struct usb_device_id *id_table;