summaryrefslogtreecommitdiff
path: root/drivers/thunderbolt/icm.c
diff options
context:
space:
mode:
authorMika Westerberg <mika.westerberg@linux.intel.com>2019-05-28 18:56:20 +0300
committerMika Westerberg <mika.westerberg@linux.intel.com>2019-06-12 13:30:46 +0300
commit4f7c2e0d8765a0266b920c66ffc495fde44c1ec8 (patch)
treebba2b7c2b916400273b6e1f9584207b9785c831a /drivers/thunderbolt/icm.c
parentd1fdb6d8f6a4109a4263176c84b899076a5f8008 (diff)
thunderbolt: Make sure device runtime resume completes before taking domain lock
When a device is authorized from userspace by writing to authorized attribute we first take the domain lock and then runtime resume the device in question. There are two issues with this. First is that the device connected notifications are blocked during this time which means we get them only after the authorization operation is complete. Because of this the authorization needed flag from the firmware notification is not reflecting the real authorization status anymore. So what happens is that the "authorized" keeps returning 0 even if the device was already authorized properly. Second issue is that each time the controller is runtime resumed the connection_id field of device connected notification may be different than in the previous resume. We need to use the latest connection_id otherwise the firmware rejects the authorization command. Fix these by moving runtime resume operations to happen before the domain lock is taken, and waiting for the updated device connected notification from the firmware before we allow runtime resume of a device to complete. While there add missing locking to tb_switch_nvm_read(). Fixes: 09f11b6c99fe ("thunderbolt: Take domain lock in switch sysfs attribute callbacks") Reported-by: Pengfei Xu <pengfei.xu@intel.com> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Diffstat (limited to 'drivers/thunderbolt/icm.c')
-rw-r--r--drivers/thunderbolt/icm.c54
1 files changed, 53 insertions, 1 deletions
diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index f1c10378fa3e..40a8960b9a7e 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -484,6 +484,7 @@ static void add_switch(struct tb_switch *parent_sw, u64 route,
sw->authorized = authorized;
sw->security_level = security_level;
sw->boot = boot;
+ init_completion(&sw->rpm_complete);
vss = parse_intel_vss(ep_name, ep_name_size);
if (vss)
@@ -523,6 +524,9 @@ static void update_switch(struct tb_switch *parent_sw, struct tb_switch *sw,
/* This switch still exists */
sw->is_unplugged = false;
+
+ /* Runtime resume is now complete */
+ complete(&sw->rpm_complete);
}
static void remove_switch(struct tb_switch *sw)
@@ -1770,6 +1774,32 @@ static void icm_unplug_children(struct tb_switch *sw)
}
}
+static int complete_rpm(struct device *dev, void *data)
+{
+ struct tb_switch *sw = tb_to_switch(dev);
+
+ if (sw)
+ complete(&sw->rpm_complete);
+ return 0;
+}
+
+static void remove_unplugged_switch(struct tb_switch *sw)
+{
+ pm_runtime_get_sync(sw->dev.parent);
+
+ /*
+ * Signal this and switches below for rpm_complete because
+ * tb_switch_remove() calls pm_runtime_get_sync() that then waits
+ * for it.
+ */
+ complete_rpm(&sw->dev, NULL);
+ bus_for_each_dev(&tb_bus_type, &sw->dev, NULL, complete_rpm);
+ tb_switch_remove(sw);
+
+ pm_runtime_mark_last_busy(sw->dev.parent);
+ pm_runtime_put_autosuspend(sw->dev.parent);
+}
+
static void icm_free_unplugged_children(struct tb_switch *sw)
{
unsigned int i;
@@ -1782,7 +1812,7 @@ static void icm_free_unplugged_children(struct tb_switch *sw)
port->xdomain = NULL;
} else if (tb_port_has_remote(port)) {
if (port->remote->sw->is_unplugged) {
- tb_switch_remove(port->remote->sw);
+ remove_unplugged_switch(port->remote->sw);
port->remote = NULL;
} else {
icm_free_unplugged_children(port->remote->sw);
@@ -1831,6 +1861,24 @@ static int icm_runtime_suspend(struct tb *tb)
return 0;
}
+static int icm_runtime_suspend_switch(struct tb_switch *sw)
+{
+ if (tb_route(sw))
+ reinit_completion(&sw->rpm_complete);
+ return 0;
+}
+
+static int icm_runtime_resume_switch(struct tb_switch *sw)
+{
+ if (tb_route(sw)) {
+ if (!wait_for_completion_timeout(&sw->rpm_complete,
+ msecs_to_jiffies(500))) {
+ dev_dbg(&sw->dev, "runtime resuming timed out\n");
+ }
+ }
+ return 0;
+}
+
static int icm_runtime_resume(struct tb *tb)
{
/*
@@ -1910,6 +1958,8 @@ static const struct tb_cm_ops icm_ar_ops = {
.complete = icm_complete,
.runtime_suspend = icm_runtime_suspend,
.runtime_resume = icm_runtime_resume,
+ .runtime_suspend_switch = icm_runtime_suspend_switch,
+ .runtime_resume_switch = icm_runtime_resume_switch,
.handle_event = icm_handle_event,
.get_boot_acl = icm_ar_get_boot_acl,
.set_boot_acl = icm_ar_set_boot_acl,
@@ -1930,6 +1980,8 @@ static const struct tb_cm_ops icm_tr_ops = {
.complete = icm_complete,
.runtime_suspend = icm_runtime_suspend,
.runtime_resume = icm_runtime_resume,
+ .runtime_suspend_switch = icm_runtime_suspend_switch,
+ .runtime_resume_switch = icm_runtime_resume_switch,
.handle_event = icm_handle_event,
.get_boot_acl = icm_ar_get_boot_acl,
.set_boot_acl = icm_ar_set_boot_acl,