[PATCH v4 04/13] ASoC: SDCA: Correct FDL locking in sdca_fdl_process()

[Charles Keepax <ckeepax@opensource.cirrus.com>]

The current locking in sdca_fdl_process() locks over
sdca_ump_cancel_timeout() and the timeout work function takes the same
lock, this can lead to a deadlock if the work runs as part of the
cancel. To fix this use scoped_guard and move the cancel timeout to be
outside the lock.

Fixes: e92e25f77748 ("ASoC: SDCA: Add UMP timeout handling for FDL")
Tested-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Maciej Strozek <mstrozek@opensource.cirrus.com>
Reviewed-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Tested-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---

Changes since v3:
 - Move back to using scoped_guard, we can't just move the guard because
   there is an exit from the function before the guard is called, and
   the mutex would get released before it has been claimed on that path.

 sound/soc/sdca/sdca_fdl.c | 89 ++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 44 deletions(-)

diff --git a/sound/soc/sdca/sdca_fdl.c b/sound/soc/sdca/sdca_fdl.c
index cb79dc3131b82..6568afacdf1a1 100644
--- a/sound/soc/sdca/sdca_fdl.c
+++ b/sound/soc/sdca/sdca_fdl.c
@@ -402,8 +402,6 @@ int sdca_fdl_process(struct sdca_interrupt *interrupt)
 	unsigned int reg, status;
 	int response, ret;
 
-	guard(mutex)(&fdl_state->lock);
-
 	ret = sdca_ump_get_owner_host(dev, interrupt->function_regmap,
 				      interrupt->function, interrupt->entity,
 				      interrupt->control);
@@ -412,56 +410,59 @@ int sdca_fdl_process(struct sdca_interrupt *interrupt)
 
 	sdca_ump_cancel_timeout(&fdl_state->timeout);
 
-	reg = SDW_SDCA_CTL(interrupt->function->desc->adr, interrupt->entity->id,
-			   SDCA_CTL_XU_FDL_STATUS, 0);
-	ret = regmap_read(interrupt->function_regmap, reg, &status);
-	if (ret < 0) {
-		dev_err(dev, "failed to read FDL status: %d\n", ret);
-		return ret;
-	}
+	scoped_guard(mutex, &fdl_state->lock) {
+		reg = SDW_SDCA_CTL(interrupt->function->desc->adr,
+				   interrupt->entity->id, SDCA_CTL_XU_FDL_STATUS, 0);
+		ret = regmap_read(interrupt->function_regmap, reg, &status);
+		if (ret < 0) {
+			dev_err(dev, "failed to read FDL status: %d\n", ret);
+			return ret;
+		}
 
-	dev_dbg(dev, "FDL status: %#x\n", status);
+		dev_dbg(dev, "FDL status: %#x\n", status);
 
-	ret = fdl_status_process(interrupt, status);
-	if (ret < 0)
-		goto reset_function;
-
-	response = ret;
+		ret = fdl_status_process(interrupt, status);
+		if (ret < 0)
+			goto reset_function;
 
-	dev_dbg(dev, "FDL response: %#x\n", response);
+		response = ret;
 
-	ret = regmap_write(interrupt->function_regmap, reg,
-			   response | (status & ~SDCA_CTL_XU_FDLH_MASK));
-	if (ret < 0) {
-		dev_err(dev, "failed to set FDL status signal: %d\n", ret);
-		return ret;
-	}
+		dev_dbg(dev, "FDL response: %#x\n", response);
 
-	ret = sdca_ump_set_owner_device(dev, interrupt->function_regmap,
-					interrupt->function, interrupt->entity,
-					interrupt->control);
-	if (ret)
-		return ret;
-
-	switch (response) {
-	case SDCA_CTL_XU_FDLH_RESET_ACK:
-		dev_dbg(dev, "FDL request reset\n");
+		ret = regmap_write(interrupt->function_regmap, reg,
+				   response | (status & ~SDCA_CTL_XU_FDLH_MASK));
+		if (ret < 0) {
+			dev_err(dev, "failed to set FDL status signal: %d\n", ret);
+			return ret;
+		}
 
-		switch (xu->reset_mechanism) {
-		default:
-			dev_warn(dev, "Requested reset mechanism not implemented\n");
+		ret = sdca_ump_set_owner_device(dev, interrupt->function_regmap,
+						interrupt->function,
+						interrupt->entity,
+						interrupt->control);
+		if (ret)
+			return ret;
+
+		switch (response) {
+		case SDCA_CTL_XU_FDLH_RESET_ACK:
+			dev_dbg(dev, "FDL request reset\n");
+
+			switch (xu->reset_mechanism) {
+			default:
+				dev_warn(dev, "Requested reset mechanism not implemented\n");
+				fallthrough;
+			case SDCA_XU_RESET_FUNCTION:
+				goto reset_function;
+			}
+		case SDCA_CTL_XU_FDLH_COMPLETE:
+			if (status & SDCA_CTL_XU_FDLD_REQ_ABORT ||
+			    status == SDCA_CTL_XU_FDLD_COMPLETE)
+				return 0;
 			fallthrough;
-		case SDCA_XU_RESET_FUNCTION:
-			goto reset_function;
-		}
-	case SDCA_CTL_XU_FDLH_COMPLETE:
-		if (status & SDCA_CTL_XU_FDLD_REQ_ABORT ||
-		    status == SDCA_CTL_XU_FDLD_COMPLETE)
+		default:
+			sdca_ump_schedule_timeout(&fdl_state->timeout, xu->max_delay);
 			return 0;
-		fallthrough;
-	default:
-		sdca_ump_schedule_timeout(&fdl_state->timeout, xu->max_delay);
-		return 0;
+		}
 	}
 
 reset_function:
-- 
2.47.3