->is_power_on can now be reliably determined by looking at the power
policy and the runtime status. ->did_runtime_put is handled
automatically by the pm_runtime parent-child relationship. A few other
cleanups fall out:
1/ No longer any need for usb_hub_set_port_power
2/ hub_power_on() only ever re-enables ports that should otherwise be
powered
Yes, this is still racy, but no worse than the current solution,
synchronization to come.
Cc: Sarah Sharp <sarah.a.sh...@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
---
drivers/usb/core/hub.c | 95 ++++++++---------------------------------------
drivers/usb/core/hub.h | 14 ++-----
drivers/usb/core/port.c | 11 ++++-
3 files changed, 29 insertions(+), 91 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index e06a457094fd..2e0812eff0ae 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -409,7 +409,7 @@ int usb_clear_port_feature(struct usb_device *hdev, int
port1, int feature)
/*
* USB 2.0 spec Section 11.24.2.13
*/
-static int set_port_feature(struct usb_device *hdev, int port1, int feature)
+int usb_set_port_feature(struct usb_device *hdev, int port1, int feature)
{
return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
USB_REQ_SET_FEATURE, USB_RT_PORT, feature, port1,
@@ -426,7 +426,7 @@ static void set_port_led(
int selector
)
{
- int status = set_port_feature(hub->hdev, (selector << 8) | port1,
+ int status = usb_set_port_feature(hub->hdev, (selector << 8) | port1,
USB_PORT_FEAT_INDICATOR);
if (status < 0)
dev_dbg (hub->intfdev,
@@ -731,34 +731,6 @@ static void hub_tt_work(struct work_struct *work)
}
/**
- * usb_hub_set_port_power - control hub port's power state
- * @hdev: USB device belonging to the usb hub
- * @hub: target hub
- * @port1: port index
- * @set: expected status
- *
- * call this function to control port's power via setting or
- * clearing the port's PORT_POWER feature.
- *
- * Return: 0 if successful. A negative error code otherwise.
- */
-int usb_hub_set_port_power(struct usb_device *hdev, struct usb_hub *hub,
- int port1, bool set)
-{
- int ret;
- struct usb_port *port_dev = hub->ports[port1 - 1];
-
- if (set)
- ret = set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
- else
- ret = usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
-
- if (!ret)
- port_dev->power_is_on = set;
- return ret;
-}
-
-/**
* usb_hub_clear_tt_buffer - clear control/bulk TT state in high speed hub
* @urb: an URB associated with the failed or incomplete split transaction
*
@@ -836,11 +808,9 @@ 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->hdev->maxchild; port1++)
- if (hub->ports[port1 - 1]->power_is_on)
- set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
- else
- usb_clear_port_feature(hub->hdev, port1,
- USB_PORT_FEAT_POWER);
+ if (usb_port_power_enabled(hub->ports[port1 - 1]))
+ usb_set_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);
@@ -872,7 +842,7 @@ static int hub_hub_status(struct usb_hub *hub,
static int hub_set_port_link_state(struct usb_hub *hub, int port1,
unsigned int link_status)
{
- return set_port_feature(hub->hdev,
+ return usb_set_port_feature(hub->hdev,
port1 | (link_status << 3),
USB_PORT_FEAT_LINK_STATE);
}
@@ -1092,6 +1062,9 @@ static void hub_activate(struct usb_hub *hub, enum
hub_activation_type type)
struct usb_device *udev = hub->ports[port1 - 1]->child;
u16 portstatus, portchange;
+ if (!usb_port_power_enabled(hub->ports[port1 - 1]))
+ continue;
+
portstatus = portchange = 0;
status = hub_port_status(hub, port1, &portstatus, &portchange);
if (udev || (portstatus & USB_PORT_STAT_CONNECTION))
@@ -1174,17 +1147,10 @@ static void hub_activate(struct usb_hub *hub, enum
hub_activation_type type)
set_bit(port1, hub->change_bits);
} else if (udev->persist_enabled) {
- struct usb_port *port_dev = hub->ports[port1 - 1];
-
#ifdef CONFIG_PM
udev->reset_resume = 1;
#endif
- /* Don't set the change_bits when the device
- * was powered off.
- */
- if (port_dev->power_is_on)
- set_bit(port1, hub->change_bits);
-
+ set_bit(port1, hub->change_bits);
} else {
/* The power session is gone; tell khubd */
usb_set_device_state(udev, USB_STATE_NOTATTACHED);
@@ -2062,16 +2028,9 @@ void usb_disconnect(struct usb_device **pdev)
usb_hcd_synchronize_unlinks(udev);
if (udev->parent) {
- struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
- struct usb_port *port_dev = hub->ports[udev->portnum - 1];
struct usb_device *hdev = udev->parent;
sysfs_remove_link(&hdev->dev.kobj, dev_name(&udev->dev));
-
- if (!port_dev->did_runtime_put)
- pm_runtime_put(&port_dev->dev);
- else
- port_dev->did_runtime_put = false;
}
usb_remove_ep_devs(&udev->ep0);
@@ -2373,16 +2332,12 @@ int usb_new_device(struct usb_device *udev)
/* Create link files between child device and usb port device. */
if (udev->parent) {
- struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
- struct usb_port *port_dev = hub->ports[udev->portnum - 1];
struct usb_device *hdev = udev->parent;
err = sysfs_create_link(&hdev->dev.kobj, &udev->dev.kobj,
dev_name(&udev->dev));
if (err)
goto fail;
-
- pm_runtime_get_sync(&port_dev->dev);
}
(void) usb_create_ep_devs(&udev->dev, &udev->ep0, udev);
@@ -2669,7 +2624,7 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
/* Reset the port */
for (i = 0; i < PORT_RESET_TRIES; i++) {
- status = set_port_feature(hub->hdev, port1, (warm ?
+ status = usb_set_port_feature(hub->hdev, port1, (warm ?
USB_PORT_FEAT_BH_PORT_RESET :
USB_PORT_FEAT_RESET));
if (status == -ENODEV) {
@@ -2965,7 +2920,6 @@ static unsigned wakeup_enabled_descendants(struct
usb_device *udev)
int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
{
struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
- struct usb_port *port_dev = hub->ports[udev->portnum - 1];
int port1 = udev->portnum;
int status;
bool really_suspend = true;
@@ -3020,7 +2974,7 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
* descendants is enabled for remote wakeup.
*/
else if (PMSG_IS_AUTO(msg) || wakeup_enabled_descendants(udev) > 0)
- status = set_port_feature(hub->hdev, port1,
+ status = usb_set_port_feature(hub->hdev, port1,
USB_PORT_FEAT_SUSPEND);
else {
really_suspend = false;
@@ -3059,11 +3013,6 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
usb_set_device_state(udev, USB_STATE_SUSPENDED);
}
- if (status == 0 && !udev->do_remote_wakeup && udev->persist_enabled) {
- pm_runtime_put_sync(&port_dev->dev);
- port_dev->did_runtime_put = true;
- }
-
usb_mark_last_busy(hub->hdev);
return status;
}
@@ -3189,21 +3138,10 @@ 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 = usb_hub_to_struct_hub(udev->parent);
- struct usb_port *port_dev = hub->ports[udev->portnum - 1];
int port1 = udev->portnum;
int status;
u16 portchange, portstatus;
- if (port_dev->did_runtime_put) {
- status = pm_runtime_get_sync(&port_dev->dev);
- port_dev->did_runtime_put = false;
- if (status < 0) {
- dev_dbg(&udev->dev, "can't resume usb port, status
%d\n",
- status);
- 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))
@@ -3347,7 +3285,7 @@ static int hub_suspend(struct usb_interface *intf,
pm_message_t msg)
if (hub_is_superspeed(hdev) && hdev->do_remote_wakeup) {
/* Enable hub to send remote wakeup for all ports. */
for (port1 = 1; port1 <= hdev->maxchild; port1++) {
- status = set_port_feature(hdev,
+ status = usb_set_port_feature(hdev,
port1 |
USB_PORT_FEAT_REMOTE_WAKE_CONNECT |
USB_PORT_FEAT_REMOTE_WAKE_DISCONNECT |
@@ -3578,7 +3516,7 @@ static int usb_set_lpm_timeout(struct usb_device *udev,
return -EINVAL;
}
- ret = set_port_feature(udev->parent,
+ ret = usb_set_port_feature(udev->parent,
USB_PORT_LPM_TIMEOUT(timeout) | udev->portnum,
feature);
if (ret < 0) {
@@ -4449,7 +4387,7 @@ static void hub_port_connect_change(struct usb_hub *hub,
int port1,
/* maybe switch power back on (e.g. root hub was reset) */
if ((wHubCharacteristics & HUB_CHAR_LPSM) < 2
&& !port_is_power_on(hub, portstatus))
- set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
+ usb_set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
if (portstatus & USB_PORT_STAT_ENABLE)
goto done;
@@ -4722,7 +4660,8 @@ static void hub_events(void)
/* deal with port status changes */
for (i = 1; i <= hdev->maxchild; i++) {
- if (test_bit(i, hub->busy_bits))
+ if (test_bit(i, hub->busy_bits)
+ || !usb_port_power_enabled(hub->ports[i-1]))
continue;
connect_change = test_bit(i, hub->change_bits);
wakeup_change = test_and_clear_bit(i, hub->wakeup_bits);
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 9ea075d1b7a3..3eb51068c39f 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -88,8 +88,6 @@ 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
- * @did_runtime_put: port has done pm_runtime_put().
*/
struct usb_port {
struct usb_device *child;
@@ -99,24 +97,20 @@ struct usb_port {
struct dev_state *port_owner;
enum usb_port_connect_type connect_type;
u8 portnum;
- unsigned power_is_on:1;
- unsigned did_runtime_put:1;
};
#define to_usb_port(_dev) \
container_of(_dev, struct usb_port, dev)
-extern int usb_hub_create_port_device(struct usb_hub *hub,
- int port1);
-extern void usb_hub_remove_port_device(struct usb_hub *hub,
- int port1);
-extern int usb_hub_set_port_power(struct usb_device *hdev, struct usb_hub *hub,
- int port1, bool set);
+extern int usb_hub_create_port_device(struct usb_hub *hub, int port1);
+bool usb_port_power_enabled(struct usb_port *port_dev);
+extern void usb_hub_remove_port_device(struct usb_hub *hub, int port1);
extern struct usb_hub *usb_hub_to_struct_hub(struct usb_device *hdev);
extern int hub_port_debounce(struct usb_hub *hub, int port1,
bool must_be_connected);
extern int usb_clear_port_feature(struct usb_device *hdev,
int port1, int feature);
+extern int usb_set_port_feature(struct usb_device *hdev, int port1, int
feature);
static inline int hub_port_debounce_be_connected(struct usb_hub *hub,
int port1)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 3c20cea08d50..89f451fa0ab8 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -84,6 +84,12 @@ static bool is_power_policy_on(struct usb_port *port_dev)
return false;
}
+bool usb_port_power_enabled(struct usb_port *port_dev)
+{
+ return is_power_policy_on(port_dev)
+ || pm_runtime_active(&port_dev->dev);
+}
+
static int usb_port_runtime_poweron(struct device *dev)
{
struct usb_port *port_dev = to_usb_port(dev);
@@ -105,7 +111,7 @@ static int usb_port_runtime_poweron(struct device *dev)
set_bit(port1, hub->busy_bits);
- retval = usb_hub_set_port_power(hdev, hub, port1, true);
+ retval = usb_set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
if (port_dev->child && !retval) {
/*
* Attempt to wait for usb hub port to be reconnected in order
@@ -140,7 +146,7 @@ static int usb_port_runtime_poweroff(struct device *dev)
return 0;
set_bit(port1, hub->busy_bits);
- retval = usb_hub_set_port_power(hdev, hub, port1, false);
+ retval = usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION);
usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
clear_bit(port1, hub->busy_bits);
@@ -178,7 +184,6 @@ 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->dev.parent = hub->intfdev;
port_dev->dev.groups = port_dev_group;
port_dev->dev.type = &usb_port_device_type;
--
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
