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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html