[PATCH] media: s5p_cec: release IRQ before deleting the adapter

[PATCH] media: s5p_cec: release IRQ before deleting the adapter

[Myeonghun Pak]

The threaded IRQ handler reports transmit and receive events through
cec->adap. Adapter unregistration masks the hardware interrupts but does
not wait for this handler. With no open device-node references, it can
free the adapter while an already queued IRQ thread still uses it.
Devres also unmaps the registers before releasing the IRQ, since the
mapping was acquired after the IRQ.

Free the managed IRQ before unregistering the adapter and disabling
runtime PM. Request it only after the register mapping and adapter have
been initialized, and free it on adapter registration failure before
deleting the adapter. This also makes IRQ release precede MMIO release
on probe unwind.

This issue was identified as part of our ongoing research while
reviewing kernel code.

Fixes: 1bcbf6f4b6b0 ("[media] cec: s5p-cec: Add s5p-cec driver")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>

---
Compile-tested with Clang, W=1. No hardware runtime testing.

Based on mainline 0fcb05b72344f40806cbe810de8872f1698bf053.

diff --git a/drivers/media/cec/platform/s5p/s5p_cec.c b/drivers/media/cec/platform/s5p/s5p_cec.c
index 4a92d32..e83705f 100644
--- a/drivers/media/cec/platform/s5p/s5p_cec.c
+++ b/drivers/media/cec/platform/s5p/s5p_cec.c
@@ -199,11 +199,6 @@ static int s5p_cec_probe(struct platform_device *pdev)
 	if (cec->irq < 0)
 		return cec->irq;
 
-	ret = devm_request_threaded_irq(dev, cec->irq, s5p_cec_irq_handler,
-		s5p_cec_irq_handler_thread, 0, pdev->name, cec);
-	if (ret)
-		return ret;
-
 	cec->clk = devm_clk_get(dev, "hdmicec");
 	if (IS_ERR(cec->clk))
 		return PTR_ERR(cec->clk);
@@ -231,16 +226,24 @@ static int s5p_cec_probe(struct platform_device *pdev)
 		goto err_delete_adapter;
 	}
 
-	ret = cec_register_adapter(cec->adap, &pdev->dev);
+	ret = devm_request_threaded_irq(dev, cec->irq, s5p_cec_irq_handler,
+					s5p_cec_irq_handler_thread, 0, pdev->name, cec);
 	if (ret)
 		goto err_notifier;
 
+	ret = cec_register_adapter(cec->adap, &pdev->dev);
+	if (ret)
+		goto err_free_irq;
+
 	platform_set_drvdata(pdev, cec);
 	pm_runtime_enable(dev);
 
 	dev_dbg(dev, "successfully probed\n");
 	return 0;
 
+err_free_irq:
+	devm_free_irq(dev, cec->irq, cec);
+
 err_notifier:
 	cec_notifier_cec_adap_unregister(cec->notifier, cec->adap);
 
@@ -253,6 +256,7 @@ static void s5p_cec_remove(struct platform_device *pdev)
 {
 	struct s5p_cec_dev *cec = platform_get_drvdata(pdev);
 
+	devm_free_irq(&pdev->dev, cec->irq, cec);
 	cec_notifier_cec_adap_unregister(cec->notifier, cec->adap);
 	cec_unregister_adapter(cec->adap);
 	pm_runtime_disable(&pdev->dev);

Re: [PATCH] media: s5p_cec: release IRQ before deleting the adapter

[Marek Szyprowski]

On 18.10.2026 05:51, Myeonghun Pak wrote:
> The threaded IRQ handler reports transmit and receive events through
> cec->adap. Adapter unregistration masks the hardware interrupts but does
> not wait for this handler. With no open device-node references, it can
> free the adapter while an already queued IRQ thread still uses it.
> Devres also unmaps the registers before releasing the IRQ, since the
> mapping was acquired after the IRQ.
>
> Free the managed IRQ before unregistering the adapter and disabling
> runtime PM. Request it only after the register mapping and adapter have
> been initialized, and free it on adapter registration failure before
> deleting the adapter. This also makes IRQ release precede MMIO release
> on probe unwind.
>
> This issue was identified as part of our ongoing research while
> reviewing kernel code.
>
> Fixes: 1bcbf6f4b6b0 ("[media] cec: s5p-cec: Add s5p-cec driver")
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
>
> ---
> Compile-tested with Clang, W=1. No hardware runtime testing.
>
> Based on mainline 0fcb05b72344f40806cbe810de8872f1698bf053.
>
> diff --git a/drivers/media/cec/platform/s5p/s5p_cec.c b/drivers/media/cec/platform/s5p/s5p_cec.c
> index 4a92d32..e83705f 100644
> --- a/drivers/media/cec/platform/s5p/s5p_cec.c
> +++ b/drivers/media/cec/platform/s5p/s5p_cec.c
> @@ -199,11 +199,6 @@ static int s5p_cec_probe(struct platform_device *pdev)
>  	if (cec->irq < 0)
>  		return cec->irq;
>  
> -	ret = devm_request_threaded_irq(dev, cec->irq, s5p_cec_irq_handler,
> -		s5p_cec_irq_handler_thread, 0, pdev->name, cec);
> -	if (ret)
> -		return ret;
> -
>  	cec->clk = devm_clk_get(dev, "hdmicec");
>  	if (IS_ERR(cec->clk))
>  		return PTR_ERR(cec->clk);
> @@ -231,16 +226,24 @@ static int s5p_cec_probe(struct platform_device *pdev)
>  		goto err_delete_adapter;
>  	}
>  
> -	ret = cec_register_adapter(cec->adap, &pdev->dev);
> +	ret = devm_request_threaded_irq(dev, cec->irq, s5p_cec_irq_handler,
> +					s5p_cec_irq_handler_thread, 0, pdev->name, cec);
>  	if (ret)
>  		goto err_notifier;
>  
> +	ret = cec_register_adapter(cec->adap, &pdev->dev);
> +	if (ret)
> +		goto err_free_irq;
> +
>  	platform_set_drvdata(pdev, cec);
>  	pm_runtime_enable(dev);
>  
>  	dev_dbg(dev, "successfully probed\n");
>  	return 0;
>  
> +err_free_irq:
> +	devm_free_irq(dev, cec->irq, cec);
There is no point in using devm_* API if one needs to manually free the
resource for whatever reason.


> +
>  err_notifier:
>  	cec_notifier_cec_adap_unregister(cec->notifier, cec->adap);
>  
> @@ -253,6 +256,7 @@ static void s5p_cec_remove(struct platform_device *pdev)
>  {
>  	struct s5p_cec_dev *cec = platform_get_drvdata(pdev);
>  
> +	devm_free_irq(&pdev->dev, cec->irq, cec);
>  	cec_notifier_cec_adap_unregister(cec->notifier, cec->adap);
>  	cec_unregister_adapter(cec->adap);
>  	pm_runtime_disable(&pdev->dev);
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH] media: s5p_cec: release IRQ before deleting the adapter

[Myeonghun Pak]

Hi Marek,

Agreed. I'll switch to request_threaded_irq() and free_irq() in v2.

Thanks for the review.

Best regards,
Myeonghun

2026년 10월 19일 (월) 오전 2:54, Marek Szyprowski <m.szyprowski@samsung.com>님이 작성:
>
> On 18.10.2026 05:51, Myeonghun Pak wrote:
> > The threaded IRQ handler reports transmit and receive events through
> > cec->adap. Adapter unregistration masks the hardware interrupts but does
> > not wait for this handler. With no open device-node references, it can
> > free the adapter while an already queued IRQ thread still uses it.
> > Devres also unmaps the registers before releasing the IRQ, since the
> > mapping was acquired after the IRQ.
> >
> > Free the managed IRQ before unregistering the adapter and disabling
> > runtime PM. Request it only after the register mapping and adapter have
> > been initialized, and free it on adapter registration failure before
> > deleting the adapter. This also makes IRQ release precede MMIO release
> > on probe unwind.
> >
> > This issue was identified as part of our ongoing research while
> > reviewing kernel code.
> >
> > Fixes: 1bcbf6f4b6b0 ("[media] cec: s5p-cec: Add s5p-cec driver")
> > Co-developed-by: Ijae Kim <ae878000@gmail.com>
> > Signed-off-by: Ijae Kim <ae878000@gmail.com>
> > Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> >
> > ---
> > Compile-tested with Clang, W=1. No hardware runtime testing.
> >
> > Based on mainline 0fcb05b72344f40806cbe810de8872f1698bf053.
> >
> > diff --git a/drivers/media/cec/platform/s5p/s5p_cec.c b/drivers/media/cec/platform/s5p/s5p_cec.c
> > index 4a92d32..e83705f 100644
> > --- a/drivers/media/cec/platform/s5p/s5p_cec.c
> > +++ b/drivers/media/cec/platform/s5p/s5p_cec.c
> > @@ -199,11 +199,6 @@ static int s5p_cec_probe(struct platform_device *pdev)
> >       if (cec->irq < 0)
> >               return cec->irq;
> >
> > -     ret = devm_request_threaded_irq(dev, cec->irq, s5p_cec_irq_handler,
> > -             s5p_cec_irq_handler_thread, 0, pdev->name, cec);
> > -     if (ret)
> > -             return ret;
> > -
> >       cec->clk = devm_clk_get(dev, "hdmicec");
> >       if (IS_ERR(cec->clk))
> >               return PTR_ERR(cec->clk);
> > @@ -231,16 +226,24 @@ static int s5p_cec_probe(struct platform_device *pdev)
> >               goto err_delete_adapter;
> >       }
> >
> > -     ret = cec_register_adapter(cec->adap, &pdev->dev);
> > +     ret = devm_request_threaded_irq(dev, cec->irq, s5p_cec_irq_handler,
> > +                                     s5p_cec_irq_handler_thread, 0, pdev->name, cec);
> >       if (ret)
> >               goto err_notifier;
> >
> > +     ret = cec_register_adapter(cec->adap, &pdev->dev);
> > +     if (ret)
> > +             goto err_free_irq;
> > +
> >       platform_set_drvdata(pdev, cec);
> >       pm_runtime_enable(dev);
> >
> >       dev_dbg(dev, "successfully probed\n");
> >       return 0;
> >
> > +err_free_irq:
> > +     devm_free_irq(dev, cec->irq, cec);
> There is no point in using devm_* API if one needs to manually free the
> resource for whatever reason.
>
>
> > +
> >  err_notifier:
> >       cec_notifier_cec_adap_unregister(cec->notifier, cec->adap);
> >
> > @@ -253,6 +256,7 @@ static void s5p_cec_remove(struct platform_device *pdev)
> >  {
> >       struct s5p_cec_dev *cec = platform_get_drvdata(pdev);
> >
> > +     devm_free_irq(&pdev->dev, cec->irq, cec);
> >       cec_notifier_cec_adap_unregister(cec->notifier, cec->adap);
> >       cec_unregister_adapter(cec->adap);
> >       pm_runtime_disable(&pdev->dev);
> >
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>