From b41a60eca833d76593d4dac8a59f5c38714194ee Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 30 May 2007 15:39:33 -0400 Subject: [PATCH] USB: add power/persist device attribute This patch (as920) adds an extra level of protection to the USB-Persist facility. Now it will apply by default only to hubs; for all other devices the user must enable it explicitly by setting the power/persist device attribute. The disconnect_all_children() routine in hub.c has been removed and its code placed inline. This is the way it was originally as part of hub_pre_reset(); the revised usage in hub_reset_resume() is sufficiently different that the code can no longer be shared. Likewise, mark_children_for_reset() is now inline as part of hub_reset_resume(). The end result looks much cleaner than before. The sysfs interface is updated to add the new attribute file, and there are corresponding documentation updates. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- Documentation/ABI/testing/sysfs-bus-usb | 13 +++++ Documentation/usb/persist.txt | 38 +++++++----- drivers/usb/core/Kconfig | 13 +++-- drivers/usb/core/hub.c | 78 +++++++++---------------- drivers/usb/core/sysfs.c | 75 +++++++++++++++++++++++- include/linux/usb.h | 1 + 6 files changed, 149 insertions(+), 69 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb index f9937add033..9734577d171 100644 --- a/Documentation/ABI/testing/sysfs-bus-usb +++ b/Documentation/ABI/testing/sysfs-bus-usb @@ -39,3 +39,16 @@ Description: If you want to suspend a device immediately but leave it free to wake up in response to I/O requests, you should write "0" to power/autosuspend. + +What: /sys/bus/usb/devices/.../power/persist +Date: May 2007 +KernelVersion: 2.6.23 +Contact: Alan Stern +Description: + If CONFIG_USB_PERSIST is set, then each USB device directory + will contain a file named power/persist. The file holds a + boolean value (0 or 1) indicating whether or not the + "USB-Persist" facility is enabled for the device. Since the + facility is inherently dangerous, it is disabled by default + for all devices except hubs. For more information, see + Documentation/usb/persist.txt. diff --git a/Documentation/usb/persist.txt b/Documentation/usb/persist.txt index 6dcd5f88479..df54d645cbb 100644 --- a/Documentation/usb/persist.txt +++ b/Documentation/usb/persist.txt @@ -2,7 +2,7 @@ Alan Stern - September 2, 2006 (Updated March 27, 2007) + September 2, 2006 (Updated May 29, 2007) What is the problem? @@ -52,9 +52,9 @@ you can convince the BIOS supplier to fix the problem (lots of luck!). On many systems the USB host controllers will get reset after a suspend-to-RAM. On almost all systems, no suspend current is -available during suspend-to-disk (also known as swsusp). You can -check the kernel log after resuming to see if either of these has -happened; look for lines saying "root hub lost power or was reset". +available during hibernation (also known as swsusp or suspend-to-disk). +You can check the kernel log after resuming to see if either of these +has happened; look for lines saying "root hub lost power or was reset". In practice, people are forced to unmount any filesystems on a USB device before suspending. If the root filesystem is on a USB device, @@ -71,15 +71,16 @@ structures are allowed to persist across a power-session disruption. It works like this. If the kernel sees that a USB host controller is not in the expected state during resume (i.e., if the controller was reset or otherwise had lost power) then it applies a persistence check -to each of the USB devices below that controller. It doesn't try to -resume the device; that can't work once the power session is gone. -Instead it issues a USB port reset and then re-enumerates the device. -(This is exactly the same thing that happens whenever a USB device is -reset.) If the re-enumeration shows that the device now attached to -that port has the same descriptors as before, including the Vendor and -Product IDs, then the kernel continues to use the same device -structure. In effect, the kernel treats the device as though it had -merely been reset instead of unplugged. +to each of the USB devices below that controller for which the +"persist" attribute is set. It doesn't try to resume the device; that +can't work once the power session is gone. Instead it issues a USB +port reset and then re-enumerates the device. (This is exactly the +same thing that happens whenever a USB device is reset.) If the +re-enumeration shows that the device now attached to that port has the +same descriptors as before, including the Vendor and Product IDs, then +the kernel continues to use the same device structure. In effect, the +kernel treats the device as though it had merely been reset instead of +unplugged. If no device is now attached to the port, or if the descriptors are different from what the kernel remembers, then the treatment is what @@ -91,6 +92,17 @@ The end result is that the USB device remains available and usable. Filesystem mounts and memory mappings are unaffected, and the world is now a good and happy place. +Note that even when CONFIG_USB_PERSIST is set, the "persist" feature +will be applied only to those devices for which it is enabled. You +can enable the feature by doing (as root): + + echo 1 >/sys/bus/usb/devices/.../power/persist + +where the "..." should be filled in the with the device's ID. Disable +the feature by writing 0 instead of 1. For hubs the feature is +automatically and permanently enabled, so you only have to worry about +setting it for devices where it really matters. + Is this the best solution? diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig index 5113ef4cb7f..97b09f28270 100644 --- a/drivers/usb/core/Kconfig +++ b/drivers/usb/core/Kconfig @@ -91,12 +91,15 @@ config USB_PERSIST depends on USB && PM && EXPERIMENTAL default n help - If you say Y here, USB device data structures will remain + + If you say Y here and enable the "power/persist" attribute + for a USB device, the device's data structures will remain persistent across system suspend, even if the USB bus loses - power. (This includes software-suspend, also known as swsusp, - or suspend-to-disk.) The devices will reappear as if by magic - when the system wakes up, with no need to unmount USB filesystems, - rmmod host-controller drivers, or do anything else. + power. (This includes hibernation, also known as swsusp or + suspend-to-disk.) The devices will reappear as if by magic + when the system wakes up, with no need to unmount USB + filesystems, rmmod host-controller drivers, or do anything + else. WARNING: This option can be dangerous! diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index c4cdb69a6e9..50e79010401 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -596,27 +596,18 @@ static void hub_port_logical_disconnect(struct usb_hub *hub, int port1) kick_khubd(hub); } -static void disconnect_all_children(struct usb_hub *hub, int logical) -{ - struct usb_device *hdev = hub->hdev; - int port1; - - for (port1 = 1; port1 <= hdev->maxchild; ++port1) { - if (hdev->children[port1-1]) { - if (logical) - hub_port_logical_disconnect(hub, port1); - else - usb_disconnect(&hdev->children[port1-1]); - } - } -} - /* caller has locked the hub device */ static int hub_pre_reset(struct usb_interface *intf) { struct usb_hub *hub = usb_get_intfdata(intf); + struct usb_device *hdev = hub->hdev; + int i; - disconnect_all_children(hub, 0); + /* Disconnect all the children */ + for (i = 0; i < hdev->maxchild; ++i) { + if (hdev->children[i]) + usb_disconnect(&hdev->children[i]); + } hub_quiesce(hub); return 0; } @@ -1872,50 +1863,39 @@ static int hub_resume(struct usb_interface *intf) return 0; } -#ifdef CONFIG_USB_PERSIST - -/* 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) +static int hub_reset_resume(struct usb_interface *intf) { + struct usb_hub *hub = usb_get_intfdata(intf); struct usb_device *hdev = hub->hdev; int port1; + hub_power_on(hub); + 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); + + /* For "USB_PERSIST"-enabled children we must + * mark the child device for reset-resume and + * turn off the connect-change status to prevent + * khubd from disconnecting it later. + */ + if (USB_PERSIST && child->persist_enabled) { + child->reset_resume = 1; + clear_port_feature(hdev, port1, + USB_PORT_FEAT_C_CONNECTION); + + /* Otherwise we must disconnect the child, + * but as we may not lock the child device here + * we have to do a "logical" disconnect. + */ + } else { + hub_port_logical_disconnect(hub, port1); + } } } -} -#else - -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; } diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index be37c863fdf..5dfe31bc32b 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -169,6 +169,73 @@ show_quirks(struct device *dev, struct device_attribute *attr, char *buf) } static DEVICE_ATTR(quirks, S_IRUGO, show_quirks, NULL); + +#if defined(CONFIG_USB_PERSIST) || defined(CONFIG_USB_SUSPEND) +static const char power_group[] = "power"; +#endif + +#ifdef CONFIG_USB_PERSIST + +static ssize_t +show_persist(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct usb_device *udev = to_usb_device(dev); + + return sprintf(buf, "%d\n", udev->persist_enabled); +} + +static ssize_t +set_persist(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct usb_device *udev = to_usb_device(dev); + int value; + + /* Hubs are always enabled for USB_PERSIST */ + if (udev->descriptor.bDeviceClass == USB_CLASS_HUB) + return -EPERM; + + if (sscanf(buf, "%d", &value) != 1) + return -EINVAL; + usb_pm_lock(udev); + udev->persist_enabled = !!value; + usb_pm_unlock(udev); + return count; +} + +static DEVICE_ATTR(persist, S_IRUGO | S_IWUSR, show_persist, set_persist); + +static int add_persist_attributes(struct device *dev) +{ + int rc = 0; + + if (is_usb_device(dev)) { + struct usb_device *udev = to_usb_device(dev); + + /* Hubs are automatically enabled for USB_PERSIST */ + if (udev->descriptor.bDeviceClass == USB_CLASS_HUB) + udev->persist_enabled = 1; + rc = sysfs_add_file_to_group(&dev->kobj, + &dev_attr_persist.attr, + power_group); + } + return rc; +} + +static void remove_persist_attributes(struct device *dev) +{ + sysfs_remove_file_from_group(&dev->kobj, + &dev_attr_persist.attr, + power_group); +} + +#else + +#define add_persist_attributes(dev) 0 +#define remove_persist_attributes(dev) do {} while (0) + +#endif /* CONFIG_USB_PERSIST */ + #ifdef CONFIG_USB_SUSPEND static ssize_t @@ -276,8 +343,6 @@ set_level(struct device *dev, struct device_attribute *attr, static DEVICE_ATTR(level, S_IRUGO | S_IWUSR, show_level, set_level); -static char power_group[] = "power"; - static int add_power_attributes(struct device *dev) { int rc = 0; @@ -311,6 +376,7 @@ static void remove_power_attributes(struct device *dev) #endif /* CONFIG_USB_SUSPEND */ + /* Descriptor fields */ #define usb_descriptor_attr_le16(field, format_string) \ static ssize_t \ @@ -384,6 +450,10 @@ int usb_create_sysfs_dev_files(struct usb_device *udev) if (retval) return retval; + retval = add_persist_attributes(dev); + if (retval) + goto error; + retval = add_power_attributes(dev); if (retval) goto error; @@ -421,6 +491,7 @@ void usb_remove_sysfs_dev_files(struct usb_device *udev) device_remove_file(dev, &dev_attr_product); device_remove_file(dev, &dev_attr_serial); remove_power_attributes(dev); + remove_persist_attributes(dev); sysfs_remove_group(&dev->kobj, &dev_attr_grp); } diff --git a/include/linux/usb.h b/include/linux/usb.h index bde8c65e2bf..efce9a4c511 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -404,6 +404,7 @@ struct usb_device { unsigned auto_pm:1; /* autosuspend/resume in progress */ unsigned do_remote_wakeup:1; /* remote wakeup should be enabled */ unsigned reset_resume:1; /* needs reset instead of resume */ + unsigned persist_enabled:1; /* USB_PERSIST enabled for this dev */ unsigned autosuspend_disabled:1; /* autosuspend and autoresume */ unsigned autoresume_disabled:1; /* disabled by the user */ #endif