Re: [PATCH] media: rcar-isp: release the core interrupt before destroying RPPX1

["Niklas Söderlund" <niklas.soderlund@ragnatech.se>]

Hello Myeonghun,

Thanks for your work.

On 2026-10-11 17:44:48 -0400, Myeonghun Pak wrote:
> risp_core_irq() accesses the RPPX1 object before taking core->lock. The
> interrupt is device managed, whereas risp_core_remove() explicitly
> destroys RPPX1 before device-managed cleanup runs. Stopping the video
> queues does not join an interrupt handler that has already started.

core->lock is also managed by devm. Could you specify which resources 
are accessed incorrectly?

> 
> Release the core interrupt after stopping the I/O nodes and before
> unregistering the V4L2 device or destroying RPPX1. Request it only after
> RPPX1, the subdevice and the handler's locks have been initialized. This
> also keeps the interrupt out of probe error paths that free RPPX1.
> 
> Only treat -ENODEV as an absent optional core when core->base is NULL.
> Resource probe failures clear that pointer, whereas failures after resource
> setup leave it set. An IRQ request returning -ENODEV must therefore fail
> the parent probe after the core has unwound its resources, instead of
> allowing removal to destroy RPPX1 and put the VSP-X device again.

This is a different (potential) issue then the first and should be a 
separate patch.

> 
> This issue was found by static analysis.
> 
> Fixes: 2151350f60d1 ("media: rcar-isp: Add support for ISPCORE")
> 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>
> ---
> Compile-tested with Clang 22.1.2, W=1 (arm64). No runtime testing.
> 
>  drivers/media/platform/renesas/rcar-isp/core.c      | 13 +++++++++----
>  drivers/media/platform/renesas/rcar-isp/csisp.c     |  2 ++
>  drivers/media/platform/renesas/rcar-isp/risp-core.h |  2 ++
>  3 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/core.c b/drivers/media/platform/renesas/rcar-isp/core.c
> index 586a5adbedfa..6bf1dd650486 100644
> --- a/drivers/media/platform/renesas/rcar-isp/core.c
> +++ b/drivers/media/platform/renesas/rcar-isp/core.c
> @@ -802,10 +802,7 @@ static int risp_core_probe_resources(struct rcar_isp_core *core,
>  	if (ret < 0)
>  		return -ENODEV;
>  
> -	ret = devm_request_irq(&pdev->dev, ret, risp_core_irq, IRQF_SHARED,
> -			       KBUILD_MODNAME, core);
> -	if (ret)
> -		return ret;
> +	core->irq = ret;
>  
>  	core->clk = devm_clk_get(&pdev->dev, "core");
>  	if (IS_ERR(core->clk))
> @@ -887,8 +884,15 @@ int risp_core_probe(struct rcar_isp_core *core, struct platform_device *pdev,
>  	spin_lock_init(&core->lock);
>  	INIT_LIST_HEAD(&core->risp_jobs);
>  
> +	ret = devm_request_irq(core->dev, core->irq, risp_core_irq, IRQF_SHARED,
> +			       KBUILD_MODNAME, core);
> +	if (ret)
> +		goto err_destroy_mutex;
> +
>  	return 0;
>  
> +err_destroy_mutex:
> +	mutex_destroy(&core->io_lock);
>  err_unregister_v4l2:
>  	v4l2_device_unregister(&core->v4l2_dev);
>  err_destroy_rpp:
> @@ -908,6 +912,7 @@ void risp_core_remove(struct rcar_isp_core *core)
>  	for (unsigned int i = 0; i < RISP_CORE_NUM_PADS; i++)
>  		risp_core_io_destroy(&core->io[i]);
>  
> +	devm_free_irq(core->dev, core->irq, core);
>  	v4l2_device_unregister(&core->v4l2_dev);
>  
>  	mutex_destroy(&core->io_lock);
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index 53ce47020d17..73bf7e42bad2 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -563,6 +563,8 @@ static int risp_probe(struct platform_device *pdev)
>  		isp->subdev.internal_ops = &risp_cs_internal_ops;
>  		break;
>  	case -ENODEV:
> +		if (isp->core.base)
> +			goto error_subdev;
>  		/* The device don't have an ISP core, that is OK. */
>  		ret = 0;
>  		break;
> diff --git a/drivers/media/platform/renesas/rcar-isp/risp-core.h b/drivers/media/platform/renesas/rcar-isp/risp-core.h
> index 627a762d6b19..70432cf4a88c 100644
> --- a/drivers/media/platform/renesas/rcar-isp/risp-core.h
> +++ b/drivers/media/platform/renesas/rcar-isp/risp-core.h
> @@ -112,6 +112,7 @@ struct rcar_isp_vspx {
>   * @dev:	(OF) device
>   * @rppaddr:	Hardware address of the RPP ISP (from OF)
>   * @clk:	The clock for the ISP CORE
> + * @irq:	Interrupt for the ISP CORE
>   * @rstc:	The reset for the ISP Core
>   * @csrstc:	The reset for the ISP Channel Selector
>   *
> @@ -138,6 +139,7 @@ struct rcar_isp_core {
>  
>  	u32 rppaddr;
>  	struct clk *clk;
> +	int irq;
>  
>  	struct reset_control *rstc;
>  	struct reset_control *csrstc;

-- 
Kind Regards,
Niklas Söderlund