Re: [PATCH v5 2/8] media: v4l2-fwnode: Add common helper library for 1-to-1 subdev registration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Redundant cleanup and misguided teardown of `sd->subdev_notifier`.
--

commit ebb79cbbc440da4b8d1a35f25ce9168deff7467d
Author: Frank Li <Frank.Li@nxp.com>

media: v4l2-fwnode: Add common helper library for 1-to-1 subdev registration

This commit introduces a common helper library to encapsulate registration,
media entity initialization, and cleanup for 1-to-1 V4L2 subdevices,
reducing boilerplate in simple bridge and converter drivers.

> diff --git a/drivers/media/v4l2-core/v4l2-1to1.c b/drivers/media/v4l2-core/v4l2-1to1.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-1to1.c

[ ... ]

> +void media_async_subdev_1to1_cleanup(struct v4l2_subdev_1to1 *sd_1to1)
> +{
> +	struct v4l2_subdev *sd = &sd_1to1->sd;
> +
> +	v4l2_async_unregister_subdev(sd);
> +	v4l2_subdev_cleanup(sd);
> +	media_entity_cleanup(&sd->entity);
> +	v4l2_async_nf_unregister(sd->subdev_notifier);
> +	v4l2_async_nf_cleanup(sd->subdev_notifier);
> +
> +	kfree(sd->subdev_notifier);

[Severity: Low]
Does this code perform redundant cleanups?

Looking at v4l2_async_unregister_subdev(), it already handles unregistering,
cleaning up, freeing, and setting sd->subdev_notifier to NULL:

v4l2_async_unregister_subdev(struct v4l2_subdev *sd) {
    ...
    __v4l2_async_nf_unregister(sd->subdev_notifier);
    __v4l2_async_nf_cleanup(sd->subdev_notifier);
    kfree(sd->subdev_notifier);
    sd->subdev_notifier = NULL;
    ...
}

Since sd->subdev_notifier is NULL after v4l2_async_unregister_subdev()
returns, are the subsequent calls to v4l2_async_nf_unregister(),
v4l2_async_nf_cleanup(), and kfree() necessary here in
media_async_subdev_1to1_cleanup()?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-imx8qxp_pcam-v5-0-7fa6c8e7fba7@nxp.com?part=2