soundwire: Use get_device()/put_device() to protect callbacks

Use get_device()/put_device() around calling driver callbacks instead
of holding a mutex. This avoids mutex nesting and possible mutex
inversion. Rename sdw_dev_lock to probe_remove_lock to make its
purpose explicit.

When the core SoundWire code calls a driver callback it must be sure
that the target driver is not unloaded while the callback is running.
The code was using the sdw_dev_lock mutex to block driver remove()
while the callback is executing.

This meant the callbacks were always called while holding that mutex,
which could lead to mutex inversion. For example during enumeration:

1) Take sdw_dev_lock
2) Call update_status()
3) Take regmap lock

But during BRA activity we could have:

1) Take regmap lock
2) Start BRA transaction
3) Take sdw_dev_lock
4) Call port_prep() and/or bus_config() callbacks

A better way to prevent the target driver being unloaded is to
increment its reference count by calling get_device(). This pattern
is seen in other drivers that need to call callbacks in another device.
The mutex is only needed around the call to get_device() to prevent
the driver being removed while its reference count is being incremented.
After that, the mutex can be released becase the driver cannot be
removed until its reference count is decremented.

Change-Id: I1f5cfd81d6204cf3aa92ddc2896af17307edd9c1
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>

Author: rfvirgil

drivers/soundwire/bus.c

@@ -962,17 +962,15 @@ static int sdw_slave_clk_stop_callback(struct sdw_slave *slave,
 {
 	int ret = 0;
 
-	mutex_lock(&slave->sdw_dev_lock);
-
-	if (slave->probed)  {
+	if (sdw_slave_device_get(slave)) {
 		struct device *dev = &slave->dev;
 		struct sdw_driver *drv = drv_to_sdw_driver(dev->driver);
 
 		if (drv->ops && drv->ops->clk_stop)
 			ret = drv->ops->clk_stop(slave, mode, type);
-	}
 
-	mutex_unlock(&slave->sdw_dev_lock);
+		sdw_slave_device_put(slave);
+	}
 
 	return ret;
 }
@@ -1799,9 +1797,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 			if (slave->prop.use_domain_irq && slave->irq)
 				handle_nested_irq(slave->irq);
 
-			mutex_lock(&slave->sdw_dev_lock);
-
-			if (slave->probed) {
+			if (sdw_slave_device_get(slave)) {
 				struct device *dev = &slave->dev;
 				struct sdw_driver *drv = drv_to_sdw_driver(dev->driver);
 
@@ -1813,9 +1809,9 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 
 					drv->ops->interrupt_callback(slave, &slave_intr);
 				}
-			}
 
-			mutex_unlock(&slave->sdw_dev_lock);
+				sdw_slave_device_put(slave);
+			}
 		}
 
 		/* Ack interrupt */
@@ -1887,17 +1883,15 @@ static int sdw_update_slave_status(struct sdw_slave *slave,
 {
 	int ret = 0;
 
-	mutex_lock(&slave->sdw_dev_lock);
-
-	if (slave->probed) {
+	if (sdw_slave_device_get(slave)) {
 		struct device *dev = &slave->dev;
 		struct sdw_driver *drv = drv_to_sdw_driver(dev->driver);
 
 		if (drv->ops && drv->ops->update_status)
 			ret = drv->ops->update_status(slave, status);
-	}
 
-	mutex_unlock(&slave->sdw_dev_lock);
+		sdw_slave_device_put(slave);
+	}
 
 	return ret;
 }

drivers/soundwire/bus.h

@@ -18,6 +18,9 @@ static inline int sdw_acpi_find_slaves(struct sdw_bus *bus)
 }
 #endif
 
+struct device *sdw_slave_device_get(struct sdw_slave *slave);
+void sdw_slave_device_put(struct sdw_slave *slave);
+
 int sdw_of_find_slaves(struct sdw_bus *bus);
 void sdw_extract_slave_id(struct sdw_bus *bus,
 			  u64 addr, struct sdw_slave_id *id);

drivers/soundwire/bus_type.c

@@ -112,7 +112,7 @@ static int sdw_bus_probe(struct device *dev)
 		return ret;
 	}
 
-	mutex_lock(&slave->sdw_dev_lock);
+	mutex_lock(&slave->probe_remove_lock);
 
 	/* device is probed so let's read the properties now */
 	if (drv->ops && drv->ops->read_prop)
@@ -151,7 +151,7 @@ static int sdw_bus_probe(struct device *dev)
 			dev_warn(dev, "failed to update status at probe: %d\n", ret);
 	}
 
-	mutex_unlock(&slave->sdw_dev_lock);
+	mutex_unlock(&slave->probe_remove_lock);
 
 	dev_dbg(dev, "probe complete\n");
 
@@ -163,11 +163,11 @@ static void sdw_bus_remove(struct device *dev)
 	struct sdw_slave *slave = dev_to_sdw_dev(dev);
 	struct sdw_driver *drv = drv_to_sdw_driver(dev->driver);
 
-	mutex_lock(&slave->sdw_dev_lock);
+	mutex_lock(&slave->probe_remove_lock);
 
 	slave->probed = false;
 
-	mutex_unlock(&slave->sdw_dev_lock);
+	mutex_unlock(&slave->probe_remove_lock);
 
 	if (drv->remove)
 		drv->remove(slave);

drivers/soundwire/slave.c

@@ -2,19 +2,36 @@
 // Copyright(c) 2015-17 Intel Corporation.
 
 #include <linux/acpi.h>
+#include <linux/cleanup.h>
+#include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/soundwire/sdw.h>
 #include <linux/soundwire/sdw_type.h>
 #include <sound/sdca.h>
 #include "bus.h"
 #include "sysfs_local.h"
 
+struct device *sdw_slave_device_get(struct sdw_slave *slave)
+{
+	guard(mutex)(&slave->probe_remove_lock);
+
+	if (!slave->probed)
+		return NULL;
+
+	return get_device(&slave->dev);
+}
+
+void sdw_slave_device_put(struct sdw_slave *slave)
+{
+	put_device(&slave->dev);
+}
+
 static void sdw_slave_release(struct device *dev)
 {
 	struct sdw_slave *slave = dev_to_sdw_dev(dev);
 
 	of_node_put(slave->dev.of_node);
-	mutex_destroy(&slave->sdw_dev_lock);
+	mutex_destroy(&slave->probe_remove_lock);
 	kfree(slave);
 }
 
@@ -64,7 +81,7 @@ int sdw_slave_add(struct sdw_bus *bus,
 	slave->dev_num = 0;
 	slave->probed = false;
 	slave->first_interrupt_done = false;
-	mutex_init(&slave->sdw_dev_lock);
+	mutex_init(&slave->probe_remove_lock);
 
 	for (i = 0; i < SDW_MAX_PORTS; i++)
 		init_completion(&slave->port_ready[i]);

drivers/soundwire/stream.c

@@ -430,9 +430,7 @@ static int sdw_do_port_prep(struct sdw_slave_runtime *s_rt,
 	int ret = 0;
 	struct sdw_slave *slave = s_rt->slave;
 
-	mutex_lock(&slave->sdw_dev_lock);
-
-	if (slave->probed) {
+	if (sdw_slave_device_get(slave)) {
 		struct device *dev = &slave->dev;
 		struct sdw_driver *drv = drv_to_sdw_driver(dev->driver);
 
@@ -442,9 +440,9 @@ static int sdw_do_port_prep(struct sdw_slave_runtime *s_rt,
 				dev_err(dev, "Slave Port Prep cmd %d failed: %d\n",
 					cmd, ret);
 		}
-	}
 
-	mutex_unlock(&slave->sdw_dev_lock);
+		sdw_slave_device_put(slave);
+	}
 
 	return ret;
 }
@@ -642,9 +640,7 @@ static int sdw_notify_config(struct sdw_master_runtime *m_rt)
 	list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
 		slave = s_rt->slave;
 
-		mutex_lock(&slave->sdw_dev_lock);
-
-		if (slave->probed) {
+		if (sdw_slave_device_get(slave)) {
 			struct device *dev = &slave->dev;
 			struct sdw_driver *drv = drv_to_sdw_driver(dev->driver);
 
@@ -653,13 +649,13 @@ static int sdw_notify_config(struct sdw_master_runtime *m_rt)
 				if (ret < 0) {
 					dev_err(dev, "Notify Slave: %d failed\n",
 						slave->dev_num);
-					mutex_unlock(&slave->sdw_dev_lock);
+					sdw_slave_device_put(slave);
 					return ret;
 				}
 			}
-		}
 
-		mutex_unlock(&slave->sdw_dev_lock);
+			sdw_slave_device_put(slave);
+		}
 	}
 
 	return 0;

include/linux/soundwire/sdw.h

@@ -659,7 +659,7 @@ struct sdw_slave_ops {
  * for a Slave happens for the first time after enumeration
  * @is_mockup_device: status flag used to squelch errors in the command/control
  * protocol for SoundWire mockup devices
- * @sdw_dev_lock: mutex used to protect callbacks/remove races
+ * @probe_remove_lock: mutex used to protect callbacks/remove races
  * @sdca_data: structure containing all device data for SDCA helpers
  */
 struct sdw_slave {
@@ -684,7 +684,7 @@ struct sdw_slave {
 	u32 unattach_request;
 	bool first_interrupt_done;
 	bool is_mockup_device;
-	struct mutex sdw_dev_lock; /* protect callbacks/remove races */
+	struct mutex probe_remove_lock;
 	struct sdca_device_data sdca_data;
 };