[PATCH v4 2/2] media: amd: isp4: fix self-deadlock in isp4sd_pwron_and_init() error path
From: Yifei Gao
Date: Tue Jul 28 2026 - 15:12:33 EST
isp4sd_pwron_and_init() holds ops_mutex via guard(mutex) and, on any
init failure, jumps to err_deinit and calls isp4sd_pwroff_and_deinit().
That helper takes the same ops_mutex, re-acquiring a non-recursive mutex
already held by the current thread, so any init failure deadlocks.
Unwind the error path in stages instead, releasing only what each
failure point acquired. This also avoids the issues that an
unconditional teardown would hit at the earlier failures, such as a
runtime-PM underflow from pm_runtime_resume_and_get() and MMIO access
while the device is unpowered.
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