[PATCH 1/2] media: sti: hva: Drop redundant clock preparation in probe

[PATCH 1/2] media: sti: hva: Drop redundant clock preparation in probe

[Myeonghun Pak]

hva_hw_probe() prepares the clock separately from the runtime PM
callbacks. The callbacks balance their own clk_prepare_enable() and
clk_disable_unprepare() calls, leaving the probe's extra prepare
reference behind on removal.

Let runtime PM own clock preparation. Runtime resume already prepares
the clock before the IP version check and encoding operations, so the
extra reference is unnecessary.

This issue was identified during our ongoing static-analysis research
while reviewing kernel code.

Fixes: 57b2c0628b60 ("[media] st-hva: multi-format video encoder V4L2 driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/media/platform/st/sti/hva/hva-hw.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/st/sti/hva/hva-hw.c b/drivers/media/platform/st/sti/hva/hva-hw.c
--- a/drivers/media/platform/st/sti/hva/hva-hw.c
+++ b/drivers/media/platform/st/sti/hva/hva-hw.c
@@ -329,17 +329,10 @@ int hva_hw_probe(struct platform_device *pdev, struct hva_dev *hva)
 		return PTR_ERR(hva->clk);
 	}
 
-	ret = clk_prepare(hva->clk);
-	if (ret < 0) {
-		dev_err(dev, "%s     failed to prepare clock\n", HVA_PREFIX);
-		hva->clk = ERR_PTR(-EINVAL);
-		return ret;
-	}
-
 	/* get status interruption resource */
 	ret  = platform_get_irq(pdev, 0);
 	if (ret < 0)
-		goto err_clk;
+		return ret;
 	hva->irq_its = ret;
 
 	ret = devm_request_threaded_irq(dev, hva->irq_its, hva_hw_its_interrupt,
@@ -349,14 +342,14 @@ int hva_hw_probe(struct platform_device *pdev, struct hva_dev *hva)
 	if (ret) {
 		dev_err(dev, "%s     failed to install status IRQ 0x%x\n",
 			HVA_PREFIX, hva->irq_its);
-		goto err_clk;
+		return ret;
 	}
 	disable_irq(hva->irq_its);
 
 	/* get error interruption resource */
 	ret = platform_get_irq(pdev, 1);
 	if (ret < 0)
-		goto err_clk;
+		return ret;
 	hva->irq_err = ret;
 
 	ret = devm_request_threaded_irq(dev, hva->irq_err, hva_hw_err_interrupt,
@@ -366,7 +359,7 @@ int hva_hw_probe(struct platform_device *pdev, struct hva_dev *hva)
 	if (ret) {
 		dev_err(dev, "%s     failed to install error IRQ 0x%x\n",
 			HVA_PREFIX, hva->irq_err);
-		goto err_clk;
+		return ret;
 	}
 	disable_irq(hva->irq_err);
 
@@ -405,8 +398,6 @@ err_pm:
 	pm_runtime_put(dev);
 err_disable:
 	pm_runtime_disable(dev);
-err_clk:
-	clk_unprepare(hva->clk);
 
 	return ret;
 }
-- 
2.51.0

[PATCH 2/2] media: sti: hva: Release the runtime clock on teardown

[Myeonghun Pak]

hva_hw_remove() drops the probe's runtime PM reference asynchronously
before disabling runtime PM. Disabling PM can cancel the queued suspend
and leave its clock prepared and enabled. The IP-version error path has
the same problem.

Disable runtime PM before checking its final status, then suspend the
hardware directly if still active and record the suspended state. Drop
the probe reference without queuing more PM work. Skip direct clock
cleanup when CONFIG_PM is disabled, since no runtime callback acquired it.

This issue was identified during our ongoing static-analysis research
while reviewing kernel code.

Fixes: 57b2c0628b60 ("[media] st-hva: multi-format video encoder V4L2 driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/media/platform/st/sti/hva/hva-hw.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/st/sti/hva/hva-hw.c b/drivers/media/platform/st/sti/hva/hva-hw.c
index a608e35..cb55dde 100644
--- a/drivers/media/platform/st/sti/hva/hva-hw.c
+++ b/drivers/media/platform/st/sti/hva/hva-hw.c
@@ -395,9 +395,12 @@ int hva_hw_probe(struct platform_device *pdev, struct hva_dev *hva)
 	return 0;
 
 err_pm:
-	pm_runtime_put(dev);
+	pm_runtime_put_noidle(dev);
 err_disable:
 	pm_runtime_disable(dev);
+	if (IS_ENABLED(CONFIG_PM) && !pm_runtime_status_suspended(dev))
+		hva_hw_runtime_suspend(dev);
+	pm_runtime_set_suspended(dev);
 
 	return ret;
 }
@@ -409,8 +412,11 @@ void hva_hw_remove(struct hva_dev *hva)
 	disable_irq(hva->irq_its);
 	disable_irq(hva->irq_err);
 
-	pm_runtime_put_autosuspend(dev);
 	pm_runtime_disable(dev);
+	if (IS_ENABLED(CONFIG_PM) && !pm_runtime_status_suspended(dev))
+		hva_hw_runtime_suspend(dev);
+	pm_runtime_set_suspended(dev);
+	pm_runtime_put_noidle(dev);
 }
 
 int hva_hw_runtime_suspend(struct device *dev)