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

[Niklas Söderlund]

On 2026-10-17 22:46:53 -0400, Myeonghun Pak wrote:
> risp_probe() interprets -ENODEV from risp_core_probe() as an absent
> optional core and continues with the channel selector. Keep that result
> for the existing resource-setup fallback, but distinguish it from a
> failure after core resources have been initialized.
> 
> The following patch moves the core IRQ request after RPPX1 and the
> registration state have been initialized.

No it doesn't.

> If that request returns
> -ENODEV, its error path releases RPPX1 and the VSPX device reference.
> Passing -ENODEV back unchanged would make the parent continue probing,
> although core->base still indicates a core whose resources have already
> been released.
> 
> Translate -ENODEV to -ENXIO at the registration error exit, after the
> existing cleanup. The parent can then handle the failure using only the
> return value, without inspecting core->base. Leave all resource-setup
> results unchanged, including the existing fallback when VSPX information
> is absent.
> 
> This prepares the error handling for the IRQ lifetime fix in the next
> patch; it does not depend on an existing -ENODEV return from the V4L2
> registration helpers.
> 
> This issue was identified during our ongoing research while reviewing
> kernel code.
> 
> Suggested-by: Niklas Söderlund <niklas.soderlund@xxxxxxxxxxxx>
> Assisted-by: LLM
> Co-developed-by: Ijae Kim <ae878000@xxxxxxxxx>
> Signed-off-by: Ijae Kim <ae878000@xxxxxxxxx>
> Signed-off-by: Myeonghun Pak <mhun512@xxxxxxxxx>
> ---
> Changes in v3:
> - Express the distinction in the return value, as suggested by Niklas,
>   instead of checking core->base in the caller.
> - Translate -ENODEV only after resource setup has succeeded, keeping
>   existing resource-setup and channel-selector fallback behavior.
> - Describe this patch as a prerequisite for the IRQ registration move.
> 
> Changes in v2:
> - Split the error-handling change out of the IRQ lifetime fix.
> 
> v2: https://lore.kernel.org/r/20261015004159.2202899-1-mhun512@xxxxxxxxx
> v1: https://lore.kernel.org/r/20261011214448.2043581-1-mhun512@xxxxxxxxx
> 
> Compile-tested with Clang 22.1.2, W=1 (arm64). No runtime testing.
> 
>  drivers/media/platform/renesas/rcar-isp/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/core.c b/drivers/media/platform/renesas/rcar-isp/core.c
> index 586a5adbedfa8..373d9ca9374fe 100644
> --- a/drivers/media/platform/renesas/rcar-isp/core.c
> +++ b/drivers/media/platform/renesas/rcar-isp/core.c
> @@ -894,7 +894,8 @@ int risp_core_probe(struct rcar_isp_core *core, struct platform_device *pdev,
>  err_destroy_rpp:
>  	rppx1_destroy(core->rpp);
>  	put_device(core->vspx.dev);
> -	return ret;
> +	/* Do not report a registration failure as an absent core. */
> +	return ret == -ENODEV ? -ENXIO : ret;

This completely misses the error code from risp_core_probe_resources() 
which, as I understand it, is the one you initially wanted to 
differentiate on? This change still propagates the return code from it 
and just chnages -ENODEV to -ENXIO if it happens to be returned from 
v4l2_device_register() or risp_core_create_subdev().

Please stop using an LLM to throw spaghetti at the wall and if you want 
to have this change read, understand and change the code by hand.

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

>  }
>  
>  void risp_core_remove(struct rcar_isp_core *core)

-- 
Kind Regards,
Niklas Söderlund