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

[Myeonghun Pak]

Hi Niklas,

You're right. I missed the resource-probe error handling.
Sorry for the incomplete revision.

I'll review the code and error paths carefully, then fix and
validate the series before resending.

Best regards,
Myeonghun

2026년 10월 18일 (일) 오전 2:54, Niklas Söderlund
<niklas.soderlund@xxxxxxxxxxxx>님이 작성:
>
> 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