From ef46d80a7015269a98c9505b5912a83798799199 Mon Sep 17 00:00:00 2001 From: Yifei Gao Date: Thu, 30 Jul 2026 14:14:04 +0000 Subject: [PATCH] media: amd: isp4: fix self-deadlock in isp4sd_pwron_and_init() error path 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 Reviewed-by: Bin Du Signed-off-by: Sakari Ailus --- 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 2a8bc1207843..6716ab9c128a 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; }