From f07600cf9eb3ee92777b2001e564faa413144a99 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 30 May 2007 15:38:16 -0400 Subject: [PATCH] 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 Signed-off-by: Jiri Kosina CC: Matthew Dharm Signed-off-by: Greg Kroah-Hartman --- drivers/hid/usbhid/hid-core.c | 9 ++- drivers/usb/core/driver.c | 51 ++++++++++---- drivers/usb/core/hub.c | 117 ++++++++++++++++++--------------- drivers/usb/storage/scsiglue.c | 8 ++- drivers/usb/storage/usb.c | 28 +++++--- include/linux/usb.h | 7 +- 6 files changed, 140 insertions(+), 80 deletions(-) diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index e221b0d1f6..b2baeaeba9 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/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, diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 6c62a6d914..3cd9af2638 100644 --- a/drivers/usb/core/driver.c +++ b/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) { - 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); + 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 { + // 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() diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index ca3dbf84e8..0b8ed414d5 100644 --- a/drivers/usb/core/hub.c +++ b/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) @@ -1938,8 +1943,9 @@ static inline int remote_wakeup(struct usb_device *udev) return 0; } -#define hub_suspend NULL -#define hub_resume NULL +#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); diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c index e227f64d56..1ba19eaa19 100644 --- a/drivers/usb/storage/scsiglue.c +++ b/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); } /*********************************************************************** diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c index be4cd8fe4c..00521f1d6a 100644 --- a/drivers/usb/storage/usb.c +++ b/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); + 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, diff --git a/include/linux/usb.h b/include/linux/usb.h index 0873c6219e..bde8c65e2b 100644 --- a/include/linux/usb.h +++ b/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; -- 2.39.5