[PATCH v3 2/2] media: amd: isp4: fix self-deadlock in isp4sd_pwron_and_init() error path

[Yifei Gao]

isp4sd_pwron_and_init() holds ops_mutex through guard(mutex) for the
whole function. On any initialization failure it jumps to the err_deinit
label and calls isp4sd_pwroff_and_deinit(), which takes the same
ops_mutex through its own guard(mutex). Because the guard in
isp4sd_pwron_and_init() still holds the lock at err_deinit, the call
re-acquires a non-recursive mutex held by the same thread and deadlocks
on any initialization failure.

The error path cannot simply drop the lock and keep calling the full
teardown, because that teardown is not valid at the earlier failure
points. In particular, pm_runtime_resume_and_get() drops the usage
counter again on failure (via pm_runtime_put_noidle()), so no PM
reference is held when it returns an error; calling pm_runtime_put_sync()
unconditionally would underflow the usage count, and the teardown would
also touch ISP MMIO while the device is not powered. This was previously
masked by the deadlock, since the thread never reached the teardown body.

Replace the single teardown call with staged labels that unwind only the
resources actually acquired at each failure point. The error path no
longer calls the locked isp4sd_pwroff_and_deinit(), which removes the
deadlock, and each failure now skips the steps it never reached, keeping
the runtime-PM count balanced and not accessing MMIO while unpowered.
isp4if_start() and isp4sd_start_resp_proc_threads() already clean up
after themselves on failure, so their resources are not unwound again.

Fixes: 4e5e7a7ddb4a ("media: platform: amd: isp4 subdev and firmware loading handling added")
Assisted-by: Claude:claude-opus-4-8 smatch
Signed-off-by: Yifei Gao <gyf161023@xxxxxxxxx>
---
 drivers/media/platform/amd/isp4/isp4_subdev.c | 28 +++++++++++++++----
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/amd/isp4/isp4_subdev.c b/drivers/media/platform/amd/isp4/isp4_subdev.c
index 48deea79ce6c..868d1c74d35e 100644
--- a/drivers/media/platform/amd/isp4/isp4_subdev.c
+++ b/drivers/media/platform/amd/isp4/isp4_subdev.c
@@ -687,7 +687,7 @@ int isp4sd_pwron_and_init(struct v4l2_subdev *sd)
 		if (ret) {
 			dev_err(dev, "fail to power on isp_subdev ret %d\n",
 				ret);
-			goto err_deinit;
+			goto err_module_disable;
 		}
 
 		/* ISPPG ISP Power Status */
@@ -697,7 +697,7 @@ int isp4sd_pwron_and_init(struct v4l2_subdev *sd)
 			dev_err(dev,
 				"fail to set performance state %u, ret %d\n",
 				perf_state, ret);
-			goto err_deinit;
+			goto err_power_off;
 		}
 
 		ispif->status = ISP4IF_STATUS_PWR_ON;
@@ -709,12 +709,12 @@ int isp4sd_pwron_and_init(struct v4l2_subdev *sd)
 	ret = isp4if_start(ispif);
 	if (ret) {
 		dev_err(dev, "fail to start isp_subdev interface\n");
-		goto err_deinit;
+		goto err_perf_restore;
 	}
 
 	if (isp4sd_start_resp_proc_threads(isp_subdev)) {
 		dev_err(dev, "isp_start_resp_proc_threads fail\n");
-		goto err_deinit;
+		goto err_stop_interface;
 	}
 
 	dev_dbg(dev, "create resp threads ok\n");
@@ -724,8 +724,24 @@ int isp4sd_pwron_and_init(struct v4l2_subdev *sd)
 	isp_subdev->irq_enabled = true;
 
 	return 0;
-err_deinit:
-	isp4sd_pwroff_and_deinit(sd);
+
+err_stop_interface:
+	isp4if_stop(ispif);
+err_perf_restore:
+	ret = dev_pm_genpd_set_performance_state(dev, ISP4SD_PERFORMANCE_STATE_LOW);
+	if (ret)
+		dev_err(dev, "fail to set performance state %u, ret %d\n",
+			ISP4SD_PERFORMANCE_STATE_LOW, ret);
+err_power_off:
+	isp4hw_wreg(isp_subdev->mmio, ISP_SOFT_RESET, 0);
+	isp4hw_wreg(isp_subdev->mmio, ISP_POWER_STATUS, 0);
+	ret = pm_runtime_put_sync(dev);
+	if (ret)
+		dev_err(dev, "power off isp_subdev fail %d\n", ret);
+	ispif->status = ISP4IF_STATUS_PWR_OFF;
+err_module_disable:
+	isp4sd_module_enable(isp_subdev, false);
+	msleep(20);
 	return -EINVAL;
 }
 
-- 
2.43.0