[PATCH v2 0/2] media: rcar-isp: fix core registration and IRQ lifetime

[PATCH v2 0/2] media: rcar-isp: fix core registration and IRQ lifetime

[Myeonghun Pak]

The ISP core IRQ is registered before the RPPX1 object and interrupt
handler state are initialized. On removal, it remains registered until
after the RPPX1 object has been freed. An interrupt handler can therefore
access RPPX1 outside its lifetime.

This series requests the IRQ after its dependencies are initialized and
releases it before destroying RPPX1. It also distinguishes an absent
optional core from a registration failure after core resources have been
prepared.

Patch 1 handles the -ENODEV distinction. Patch 2 moves IRQ registration
and release within the RPPX1 lifetime. The error-handling change comes
first so a failure from the later IRQ request cannot be mistaken for an
absent core after registration has unwound.

Compile-tested with Clang 22.1.2, W=1 (arm64). No runtime testing.

Changes in v2:
- Split the -ENODEV handling into patch 1.
- Explain the RPPX1 allocation, free and pre-lock IRQ accesses in patch 2.
- Combined code changes are identical to v1.

v1: https://lore.kernel.org/r/20261011214448.2043581-1-mhun512@gmail.com

Myeonghun Pak (2):
  media: rcar-isp: distinguish absent cores from registration failures
  media: rcar-isp: release the core interrupt before destroying RPPX1

 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(-)

base-commit: 238650ef6c7c7cca08e032527329424c9fbd70e5

[PATCH v2 1/2] media: rcar-isp: distinguish absent cores from registration failures

[Myeonghun Pak]

risp_core_probe() clears core->base when resource setup fails. Once
resource setup has succeeded, it leaves core->base set while unwinding
errors from the remaining registration steps, including destruction of
core->rpp and release of the VSPX device reference.

risp_probe() treats every -ENODEV from risp_core_probe() as an absent
optional core. If a later registration step returns that error, the
parent probe can continue and subsequently call risp_core_remove() on
resources that have already been released.

Only accept -ENODEV as an absent core when core->base is NULL. Otherwise,
propagate the error through the parent probe's subdevice cleanup. This
also keeps failures from the later IRQ request in the following patch
from being mistaken for an absent core.

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

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>
---
Changes in v2:
- Split the -ENODEV handling out of the IRQ lifetime fix, as requested
  by Niklas Soderlund.

v1: https://lore.kernel.org/r/20261011214448.2043581-1-mhun512@gmail.com

Compile-tested with Clang 22.1.2, W=1 (arm64). No runtime testing.

 drivers/media/platform/renesas/rcar-isp/csisp.c | 2 ++
 1 file changed, 2 insertions(+)
diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
index 53ce47020d170454b3d208bce7f4d6aad76764d3..73bf7e42bad2ac1ec2493cd8ec81003cce05a3dd 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;

Re: [PATCH v2 1/2] media: rcar-isp: distinguish absent cores from registration failures

[Niklas Söderlund]

Hi Myeonghun,

Thanks for your patch.

On 2026-10-14 20:41:58 -0400, Myeonghun Pak wrote:
> risp_core_probe() clears core->base when resource setup fails. Once
> resource setup has succeeded, it leaves core->base set while unwinding
> errors from the remaining registration steps, including destruction of
> core->rpp and release of the VSPX device reference.
> 
> risp_probe() treats every -ENODEV from risp_core_probe() as an absent
> optional core. If a later registration step returns that error, the
> parent probe can continue and subsequently call risp_core_remove() on
> resources that have already been released.
> 
> Only accept -ENODEV as an absent core when core->base is NULL. Otherwise,
> propagate the error through the parent probe's subdevice cleanup. This
> also keeps failures from the later IRQ request in the following patch
> from being mistaken for an absent core.

While it would indeed be nice to propagate the error I don't like this 
solution. It creates multiple places to check, first the return code of 
risp_core_probe() and then implicit variable contents ontop of that.

Would it not be nicer to add an additional return code to 
risp_core_probe()? If it returns 0 all good, if it returns -ENODEV the 
optional device was not described or a new code (maybe -ENXIO or -EIO ?) 
if the device is described but we fail to initialize it.

For this first check return code, then check some other variable 
solution,

Nacked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> 
> This issue was identified during our ongoing research while reviewing
> kernel code.
> 
> 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>
> ---
> Changes in v2:
> - Split the -ENODEV handling out of the IRQ lifetime fix, as requested
>   by Niklas Soderlund.
> 
> v1: https://lore.kernel.org/r/20261011214448.2043581-1-mhun512@gmail.com
> 
> Compile-tested with Clang 22.1.2, W=1 (arm64). No runtime testing.
> 
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 2 ++
>  1 file changed, 2 insertions(+)
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index 53ce47020d170454b3d208bce7f4d6aad76764d3..73bf7e42bad2ac1ec2493cd8ec81003cce05a3dd 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;

-- 
Kind Regards,
Niklas Söderlund

[PATCH v2 2/2] media: rcar-isp: release the core interrupt before destroying RPPX1

[Myeonghun Pak]

core->rpp is a separately allocated struct rppx1: rppx1_create()
allocates it with kzalloc_obj(), and rppx1_destroy() frees it with
kfree(). It is not part of the device-managed core allocation.

risp_core_irq() calls rppx1_interrupt(core->rpp, ...) before taking
core->lock. rppx1_interrupt() accesses rpp->base through rppx1_read()
and rppx1_write(), and can also use rpp->dev. An interrupt handler that
has passed the ISPCORE status check can still be using this object when
risp_core_remove() frees it. Stopping the video queues and taking
core->lock do not join that earlier part of the handler. The managed IRQ
is otherwise released only after remove returns.

Request the IRQ after the RPPX1 object, subdevice, locks and job list have
been initialized, so probe error paths cannot leave a handler using an
uninitialized or freed RPPX1 object. Release and synchronize the IRQ after
stopping the I/O nodes but before unregistering the V4L2 device and
destroying RPPX1.

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

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>
---
Changes in v2:
- Identify the separately allocated RPPX1 object and the rpp->base/rpp->dev
  accesses in the interrupt handler, rather than the core lock lifetime.
- Move -ENODEV handling into a separate prerequisite patch.
- No code changes relative to the IRQ portion of v1.

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/risp-core.h |  2 ++
 2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/renesas/rcar-isp/core.c b/drivers/media/platform/renesas/rcar-isp/core.c
index 586a5adbedfa87025d4258fd75a312a9a7b05ae2..6bf1dd6504862a50299893270e68f06b0d6ed25d 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/risp-core.h b/drivers/media/platform/renesas/rcar-isp/risp-core.h
index 627a762d6b192c6cac454ba30cde708406f550ce..70432cf4a88ca97fa1c93cf4a42bf7454b7739e8 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;