On 2012年12月07日 23:22, Alan Stern wrote:
> On Fri, 7 Dec 2012, Lan Tianyu wrote:
> 
>>> Maybe you really need two flags.  Do whatever is best; I'm sure you can
>>> figure out a good scheme.
>> Yeah. Two flags maybe good. In this situation, it should be call
>> "power_is_on", right? power_is_on can be used to avoid to sending
>> redundant PORT_POWER request. The other flag is dedicated to prevent
>> device from being disconnected. Something like hub->busy_bits. We can
>> can "child_busy", I am not very good at giving a name. So I'd like to
>> see your opinion. :)
> 
> How about "needs_debounce"?
> 
> Alan Stern
> 
Hi Alan:
        I write a patch based on the needs_debounce flag. The flag will be set
when port has child device and power on successfully. Otherwise, I
separate resume port and wait for connect in the usb_port_resume().
Device will not be disconnected when power_is_on is false or
needs_debounce  is set. Welcome comments.

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 86e595c..aa41d3a 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -26,6 +26,7 @@
 #include <linux/mutex.h>
 #include <linux/freezer.h>
 #include <linux/random.h>
+#include <linux/pm_qos.h>

 #include <asm/uaccess.h>
 #include <asm/byteorder.h>
@@ -108,11 +109,14 @@ MODULE_PARM_DESC(use_both_schemes,
 DECLARE_RWSEM(ehci_cf_port_reset_rwsem);
 EXPORT_SYMBOL_GPL(ehci_cf_port_reset_rwsem);

+#define HUB_PORT_RECONNECT_TRIES       20
+
 #define HUB_DEBOUNCE_TIMEOUT   1500
 #define HUB_DEBOUNCE_STEP        25
 #define HUB_DEBOUNCE_STABLE     100

 static int usb_reset_and_verify_device(struct usb_device *udev);
+static int hub_port_debounce(struct usb_hub *hub, int port1);

 static inline char *portspeed(struct usb_hub *hub, int portstatus)
 {
@@ -718,13 +722,26 @@ int usb_hub_set_port_power(struct usb_device
*hdev, int port1,
                bool set)
 {
        int ret;
+       struct usb_hub *hub = hdev_to_hub(hdev);
+       struct usb_port *port_dev
+               = hub->ports[port1 - 1];
+
+       if (set) {
+               if (port_dev->power_is_on)
+                       return 0;

-       if (set)
                ret = set_port_feature(hdev, port1,
                                USB_PORT_FEAT_POWER);
-       else
+       } else {
+               if (!port_dev->power_is_on)
+                       return 0;
+
                ret = clear_port_feature(hdev, port1,
                                USB_PORT_FEAT_POWER);
+       }
+
+       if (!ret)
+               port_dev->power_is_on = set;
        return ret;
 }

@@ -804,7 +821,11 @@ static unsigned hub_power_on(struct usb_hub *hub,
bool do_delay)
                dev_dbg(hub->intfdev, "trying to enable port power on "
                                "non-switchable hub\n");
        for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++)
-               set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
+               if (hub->ports[port1 - 1]->power_is_on)
+                       set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
+               else
+                       clear_port_feature(hub->hdev, port1,
+                                               USB_PORT_FEAT_POWER);

        /* Wait at least 100 msec for power to become stable */
        delay = max(pgood_delay, (unsigned) 100);
@@ -2768,6 +2789,7 @@ EXPORT_SYMBOL_GPL(usb_enable_ltm);
 int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 {
        struct usb_hub  *hub = hdev_to_hub(udev->parent);
+       struct usb_port *port_dev = hub->ports[udev->portnum - 1];
        int             port1 = udev->portnum;
        int             status;

@@ -2862,6 +2884,18 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
                udev->port_is_suspended = 1;
                msleep(10);
        }
+
+       /*
+        * Check whether current status is meeting requirement of
+        * usb port power off mechanism
+        */
+       if (!udev->do_remote_wakeup
+                       && (dev_pm_qos_flags(&port_dev->dev,
+                       PM_QOS_FLAG_NO_POWER_OFF) != PM_QOS_FLAGS_ALL)
+                       && udev->persist_enabled
+                       && !status)
+               pm_runtime_put_sync(&port_dev->dev);
+
        usb_mark_last_busy(hub->hdev);
        return status;
 }
@@ -2945,6 +2979,25 @@ static int finish_port_resume(struct usb_device
*udev)
        return status;
 }

+static int usb_port_wait_for_connected(struct usb_hub *hub, int port1)
+{
+       int status, i;
+
+       for (i = 0; i < HUB_PORT_RECONNECT_TRIES; i++) {
+               status = hub_port_debounce(hub, port1);
+               if (status & USB_PORT_STAT_CONNECTION) {
+                       /*
+                        * just clear enable-change feature since debounce
+                        *  has cleared connect-change feature.
+                        */
+                       clear_port_feature(hub->hdev, port1,
+                                       USB_PORT_FEAT_C_ENABLE);
+                       return 0;
+               }
+       }
+       return -ETIMEDOUT;
+}
+
 /*
  * usb_port_resume - re-activate a suspended usb device's upstream port
  * @udev: device to re-activate, not a root hub
@@ -2982,10 +3035,39 @@ static int finish_port_resume(struct usb_device
*udev)
 int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 {
        struct usb_hub  *hub = hdev_to_hub(udev->parent);
+       struct usb_port *port_dev = hub->ports[udev->portnum  - 1];
        int             port1 = udev->portnum;
        int             status;
        u16             portchange, portstatus;

+
+       set_bit(port1, hub->busy_bits);
+
+       if (!port_dev->power_is_on) {
+               status = pm_runtime_get_sync(&port_dev->dev);
+               if (status < 0) {
+                       dev_dbg(&udev->dev, "can't resume usb port, status 
%d\n",
+                                       status);
+                       clear_bit(port1, hub->busy_bits);
+                       return status;
+               }
+       }
+
+
+       /*
+        * Wait for usb hub port to be reconnected in order to make
+        * the resume procedure successful.
+        */
+       if (port_dev->needs_debounce) {
+               status = usb_port_wait_for_connected(hub, port1);
+               if (status < 0) {
+                       dev_dbg(&udev->dev, "can't get reconnection after 
setting port
power on, status %d\n",
+                                       status);
+                       clear_bit(port1, hub->busy_bits);
+                       return status;
+               }
+       }
+
        /* Skip the initial Clear-Suspend step for a remote wakeup */
        status = hub_port_status(hub, port1, &portstatus, &portchange);
        if (status == 0 && !port_is_suspended(hub, portstatus))
@@ -2993,8 +3075,6 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)

        // dev_dbg(hub->intfdev, "resume port %d\n", port1);

-       set_bit(port1, hub->busy_bits);
-
        /* see 7.1.7.7; affects power usage, but not budgeting */
        if (hub_is_superspeed(hub->hdev))
                status = set_port_feature(hub->hdev,
@@ -4126,6 +4206,7 @@ static void hub_port_connect_change(struct usb_hub
*hub, int port1,
        struct usb_device *hdev = hub->hdev;
        struct device *hub_dev = hub->intfdev;
        struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
+       struct usb_port *port_dev = hub->ports[port1 - 1];
        unsigned wHubCharacteristics =
                        le16_to_cpu(hub->descriptor->wHubCharacteristics);
        struct usb_device *udev;
@@ -4175,6 +4256,11 @@ static void hub_port_connect_change(struct
usb_hub *hub, int port1,
                }
        }

+       if (!port_dev->power_is_on || port_dev->needs_debounce) {
+               clear_bit(port1, hub->change_bits);
+               return;
+       }
+
        /* Disconnect any existing devices under this port */
        if (udev) {
                if (hcd->phy && !hdev->parent &&
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 8ea6bc8..e561ec2 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -80,6 +80,8 @@ struct usb_hub {
  * @port_owner: port's owner
  * @connect_type: port's connect type
  * @portnum: port index num based one
+ * @power_is_on: port's power state
+ * @needs_debounce: flag to ignore some port events after power on.
  */
 struct usb_port {
        struct usb_device *child;
@@ -87,6 +89,8 @@ struct usb_port {
        struct dev_state *port_owner;
        enum usb_port_connect_type connect_type;
        u8 portnum;
+       bool power_is_on;
+       bool needs_debounce;
 };

 #define to_usb_port(_dev) \
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index f5f8890..46a31fe 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -74,9 +74,14 @@ static int usb_port_runtime_resume(struct device *dev)
        struct usb_port *port_dev = to_usb_port(dev);
        struct usb_device *hdev =
                to_usb_device(dev->parent->parent);
+       int retval;

-       return usb_hub_set_port_power(hdev, port_dev->portnum,
+       retval = usb_hub_set_port_power(hdev, port_dev->portnum,
                        true);
+
+       if (port_dev->child && !retval)
+               port_dev->needs_debounce = true;
+       return retval;
 }

 static int usb_port_runtime_suspend(struct device *dev)
@@ -120,6 +125,8 @@ int usb_hub_create_port_device(struct usb_hub *hub,
int port1)

        hub->ports[port1 - 1] = port_dev;
        port_dev->portnum = port1;
+       port_dev->power_is_on = true;
+       port_dev->needs_debounce = false;
        port_dev->dev.parent = hub->intfdev;
        port_dev->dev.groups = port_dev_group;
        port_dev->dev.type = &usb_port_device_type;


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

Reply via email to