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@xxxxxxxxx>
> Signed-off-by: Ijae Kim <ae878000@xxxxxxxxx>
> Signed-off-by: Myeonghun Pak <mhun512@xxxxxxxxx>
>
> ---
> 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