[PATCH] media: atomisp: drain assert recovery before tearing down resources

[PATCH] media: atomisp: drain assert recovery before tearing down resources

[Myeonghun Pak]

The threaded IRQ queues assert_recovery_work, which can stop, reset and
restart CSS. Removal currently tears down CSS and the CPU latency QoS
request while recovery can still be running.

Disable and drain recovery before atomisp_pm_uninit(), keeping the IRQ
available until recovery finishes. Then release the IRQ before CSS and
firmware teardown. Apply the same ordering on probe failure, and skip
the uninitialized recovery work in PM-only mode.

This issue was found by static analysis.

Fixes: 93d3fb35aa10 ("media: atomisp: Remove watchdog timer")
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/staging/media/atomisp/pci/atomisp_v4l2.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 812230397409..768b35468380 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -1443,9 +1443,13 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	return 0;
 
 error_unload_firmware:
+	disable_work_sync(&isp->assert_recovery_work);
 	atomisp_pm_uninit(isp);
+	devm_free_irq(&pdev->dev, pdev->irq, isp);
 	ia_css_unload_firmware();
+	goto error_unregister_entities;
 error_free_irq:
+	disable_work_sync(&isp->assert_recovery_work);
 	devm_free_irq(&pdev->dev, pdev->irq, isp);
 error_unregister_entities:
 	hmm_cleanup();
@@ -1464,15 +1468,19 @@ static void atomisp_pci_remove(struct pci_dev *pdev)
 {
 	struct atomisp_device *isp = pci_get_drvdata(pdev);
 
+	if (!isp->pm_only)
+		disable_work_sync(&isp->assert_recovery_work);
+
 	atomisp_pm_uninit(isp);
 
 	if (isp->pm_only)
 		return;
 
+	devm_free_irq(&pdev->dev, pdev->irq, isp);
+
 	/* Undo ia_css_init() from atomisp_power_on() */
 	atomisp_css_uninit(isp);
 	ia_css_unload_firmware();
-	devm_free_irq(&pdev->dev, pdev->irq, isp);
 	hmm_cleanup();
 
 	atomisp_unregister_entities(isp);

Re: [PATCH] media: atomisp: drain assert recovery before tearing down resources

[Andy Shevchenko]

On Mon, Oct 12, 2026 at 7:47 AM Myeonghun Pak <mhun512@gmail.com> wrote:
>
> The threaded IRQ queues assert_recovery_work, which can stop, reset and
> restart CSS. Removal currently tears down CSS and the CPU latency QoS
> request while recovery can still be running.
>
> Disable and drain recovery before atomisp_pm_uninit(), keeping the IRQ
> available until recovery finishes. Then release the IRQ before CSS and
> firmware teardown. Apply the same ordering on probe failure, and skip
> the uninitialized recovery work in PM-only mode.
>
> This issue was found by static analysis.

Nice, have you read the code that you sent?

...

>  error_unload_firmware:
> +       disable_work_sync(&isp->assert_recovery_work);
>         atomisp_pm_uninit(isp);
> +       devm_free_irq(&pdev->dev, pdev->irq, isp);

^^^

>         ia_css_unload_firmware();
> +       goto error_unregister_entities;
>  error_free_irq:
> +       disable_work_sync(&isp->assert_recovery_work);
>         devm_free_irq(&pdev->dev, pdev->irq, isp);

^^^

>  error_unregister_entities:
>         hmm_cleanup();

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] media: atomisp: drain assert recovery before tearing down resources

[Andy Shevchenko]

On Mon, Oct 12, 2026 at 12:21 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Oct 12, 2026 at 7:47 AM Myeonghun Pak <mhun512@gmail.com> wrote:
> >
> > The threaded IRQ queues assert_recovery_work, which can stop, reset and
> > restart CSS. Removal currently tears down CSS and the CPU latency QoS
> > request while recovery can still be running.
> >
> > Disable and drain recovery before atomisp_pm_uninit(), keeping the IRQ
> > available until recovery finishes. Then release the IRQ before CSS and
> > firmware teardown. Apply the same ordering on probe failure, and skip
> > the uninitialized recovery work in PM-only mode.
> >
> > This issue was found by static analysis.
>
> Nice, have you read the code that you sent?

Next time, please also test this on real HW.

...

> >  error_unload_firmware:
> > +       disable_work_sync(&isp->assert_recovery_work);
> >         atomisp_pm_uninit(isp);
> > +       devm_free_irq(&pdev->dev, pdev->irq, isp);
>
> ^^^
>
> >         ia_css_unload_firmware();
> > +       goto error_unregister_entities;
> >  error_free_irq:
> > +       disable_work_sync(&isp->assert_recovery_work);
> >         devm_free_irq(&pdev->dev, pdev->irq, isp);
>
> ^^^
>
> >  error_unregister_entities:
> >         hmm_cleanup();

-- 
With Best Regards,
Andy Shevchenko