Re: [PATCH] media: rcar-vin: Clean up notifier on probe failure

Niklas Söderlund
2026-05-01T09:56:45Z

Hello Myeonghun and Ijae,

Thanks for your patch.

On 2026-04-28 21:57:04 +0900, Myeonghun Pak wrote:
> rvin_group_notifier_init() initializes the group notifier before
> parsing the graph and adding async connections. If parsing fails
> after that point, the function returns without cleaning up the
> notifier, leaving the async connections and their fwnode references
> behind.
>
> The Gen2/default probe path also fails to drop the group reference
> when rvin_group_notifier_init() returns an error after
> rvin_group_get() succeeded. Unlike a successful probe, .remove() is
> not called for this path, so the probe error path must unwind it
> directly.
>
> Route notifier-init failures through a common cleanup path, clear
> the cached async connection pointers after cleanup, and make the
> default probe path mirror the CSI-2/ISP group unwind.
>
> Fixes: 856b49c71ae5 ("media: rcar-vin: Merge all notifiers")
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>

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

> ---
> .../media/platform/renesas/rcar-vin/rcar-core.c | 32 ++++++++++++++++++----
> 1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> index c8d564aa1e..8ba8d49c3e 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> @@ -318,6 +318,20 @@ static const struct v4l2_async_notifier_operations rvin_group_notify_ops = {
> .complete = rvin_group_notify_complete,
> };
>
> +static void rvin_group_notifier_cleanup(struct rvin_group *group)
> +{
> + v4l2_async_nf_cleanup(&group->notifier);
> +
> + guard(mutex)(&group->lock);
> +
> + for (unsigned int i = 0; i < RCAR_VIN_NUM; i++)
> + if (group->vin[i])
> + group->vin[i]->parallel.asc = NULL;
> +
> + for (unsigned int i = 0; i < ARRAY_SIZE(group->remotes); i++)
> + group->remotes[i].asc = NULL;
> +}
> +
> static int rvin_group_parse_of(struct rvin_dev *vin, unsigned int port,
> unsigned int id)
> {
> @@ -440,7 +454,7 @@ static int rvin_group_notifier_init(struct rvin_dev *vin, unsigned int port,
> /* Parse local subdevice. */
> ret = rvin_parallel_parse_of(vin->group->vin[i]);
> if (ret)
> - return ret;
> + goto err_cleanup;
>
> /* Parse shared subdevices. */
> for (id = 0; id < max_id; id++) {
> @@ -449,7 +463,7 @@ static int rvin_group_notifier_init(struct rvin_dev *vin, unsigned int port,
>
> ret = rvin_group_parse_of(vin->group->vin[i], port, id);
> if (ret)
> - return ret;
> + goto err_cleanup;
> }
> }
>
> @@ -460,11 +474,14 @@ static int rvin_group_notifier_init(struct rvin_dev *vin, unsigned int port,
> ret = v4l2_async_nf_register(&vin->group->notifier);
> if (ret < 0) {
> vin_err(vin, "Notifier registration failed\n");
> - v4l2_async_nf_cleanup(&vin->group->notifier);
> - return ret;
> + goto err_cleanup;
> }
>
> return 0;
> +
> +err_cleanup:
> + rvin_group_notifier_cleanup(vin->group);
> + return ret;
> }
>
> /* -----------------------------------------------------------------------------
> @@ -1228,8 +1245,11 @@ static int rcar_vin_probe(struct platform_device *pdev)
> break;
> default:
> ret = rvin_group_get(vin, rvin_parallel_setup_links, NULL);
> - if (!ret)
> + if (!ret) {
> ret = rvin_group_notifier_init(vin, 0, 0);
> + if (ret)
> + rvin_group_put(vin);
> + }
>
> if (vin->info->scaler)
> vin->scaler = vin->info->scaler;
> @@ -1264,7 +1284,7 @@ static void rcar_vin_remove(struct platform_device *pdev)
>
> if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
> v4l2_async_nf_unregister(&vin->group->notifier);
> - v4l2_async_nf_cleanup(&vin->group->notifier);
> + rvin_group_notifier_cleanup(vin->group);
> }
>
> rvin_group_put(vin);
> --
> 2.49.0

--
Kind Regards,
Niklas Söderlund