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 <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb
index f9937ad..9734577 100644 (file)
--- 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.
                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 <stern@rowland.harvard.edu>
+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 6dcd5f8..df54d64 100644 (file)
--- a/Documentation/usb/persist.txt
+++ b/Documentation/usb/persist.txt
@@ -2,7 +2,7 @@
 
                   Alan Stern <stern@rowland.harvard.edu>
 
 
                   Alan Stern <stern@rowland.harvard.edu>
 

-                September 2, 2006 (Updated March 27, 2007)
+                September 2, 2006 (Updated May 29, 2007)

 
 
        What is the problem?
 
 
        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
 
 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,
 
 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
 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
 
 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.
 
 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?
 
 
        Is this the best solution?
 

diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
index 5113ef4..97b09f2 100644 (file)
--- 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
        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
          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!
 
 
                WARNING: This option can be dangerous!
 

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index c4cdb69..50e7901 100644 (file)
--- 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);
 }
 
        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);
 /* 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;
 }
        hub_quiesce(hub);
        return 0;
 }


@@ -1872,50 +1863,39 @@ static int hub_resume(struct usb_interface *intf)
        return 0;
 }
 
        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;
 
        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) {
        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;
 }
        hub_activate(hub);
        return 0;
 }

diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index be37c86..5dfe31b 100644 (file)
--- 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);
 
 }
 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
 #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 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;
 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 */
 
 
 #endif /* CONFIG_USB_SUSPEND */
 

+

 /* Descriptor fields */
 #define usb_descriptor_attr_le16(field, format_string)                 \
 static ssize_t                                                         \
 /* 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;
 
        if (retval)
                return retval;
 

+       retval = add_persist_attributes(dev);
+       if (retval)
+               goto error;
+

        retval = add_power_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);
        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);
 }
 
        sysfs_remove_group(&dev->kobj, &dev_attr_grp);
 }
 

diff --git a/include/linux/usb.h b/include/linux/usb.h
index bde8c65..efce9a4 100644 (file)
--- 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 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
        unsigned autosuspend_disabled:1; /* autosuspend and autoresume */
        unsigned autoresume_disabled:1;  /*  disabled by the user */
 #endif