Re: [PATCH v5 4/4] media: renesas: rzg2l-core: Add missing media_entity_cleanup()

Jacopo Mondi <[email protected]>
Newsgroups org.kernel.vger.linux-renesas-soc,org.kernel.vger.linux-kernel,org.kernel.vger.linux-media
Message-ID <ak9G5g8sw5W_c8nf@zed>
Hi Biren

On Wed, Jul 08, 2026 at 10:38:48PM +0530, Biren Pandya wrote:
> The probe error paths and the remove function fail to call
> media_entity_cleanup() upon teardown.
>
> While currently a no-op, calling media_entity_cleanup()
> is an API requirement for entities initialized with
> media_entity_pads_init()
> to prevent memory leaks.

Here and in the other patches, why break the line ?

>
> Add the missing media_entity_cleanup() calls.
>
> Additionally, patch the error path in rzg2l_cru_media_init() to ensure
> that media_device_cleanup(), media_entity_cleanup(), and mutex_destroy()
> are invoked and the error code is correctly returned if
> rzg2l_cru_mc_parse_of_graph() fails, rather than silently returning 0
> and leaving the initialized structures lingering.
>
> Signed-off-by: Biren Pandya <[email protected]>
> ---
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> index 3c5fbd857371c..77b17bcf7aeb7 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> @@ -234,6 +234,10 @@ static int rzg2l_cru_media_init(struct rzg2l_cru_dev *cru)
>  		mutex_lock(&cru->mdev_lock);
>  		cru->v4l2_dev.mdev = NULL;
>  		mutex_unlock(&cru->mdev_lock);
> +		media_device_cleanup(mdev);
> +		media_entity_cleanup(&cru->vdev.entity);
> +		mutex_destroy(&cru->mdev_lock);
> +		return ret;

Mmmm, this is growing enough  to qualify for a label I guess...

Unrelated, but I wonder what

        mutex_lock(&cru->mdev_lock);
        cru->v4l2_dev.mdev = NULL;
	mutex_unlock(&cru->mdev_lock);

Protects against...

What do you think of reworking the error path to make it like

	ret = rzg2l_cru_mc_parse_of_graph(cru);
	if (ret)
                goto err_mc_parse;

        return 0;

error_mc_parse:
        media_device_cleanup(mdev);
        cru->v4l2_dev.mdev = NULL;
        media_entity_cleanup(&cru->vdev.entity);
        mutex_destroy(&cru->mdev_lock);
        return ret;

?

In this case, please reword the commit message accordingly, as the
patch won't just be about adding media_entity_cleanup() but rather
about reworking the rzg2l_cru_media_init() error path.

Thanks
  j

>  	}
>
>  	return 0;
> @@ -312,6 +316,7 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
>  	v4l2_async_nf_cleanup(&cru->notifier);
>
>  	rzg2l_cru_video_unregister(cru);
> +	media_entity_cleanup(&cru->vdev.entity);
>  	media_device_cleanup(&cru->mdev);
>  	mutex_destroy(&cru->mdev_lock);
>
> --
> 2.50.1 (Apple Git-155)
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.