All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
To: Biren Pandya <birenpandya@gmail.com>
Cc: niklas.soderlund@ragnatech.se, mchehab@kernel.org,
	 geert+renesas@glider.be, magnus.damm@gmail.com,
	 Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>,
	 linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 4/4] media: renesas: rzg2l-core: Add missing media_entity_cleanup()
Date: Thu, 9 Jul 2026 09:05:46 +0200	[thread overview]
Message-ID: <ak9G5g8sw5W_c8nf@zed> (raw)
In-Reply-To: <20260708170843.55076-10-birenpandya@gmail.com>

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 <birenpandya@gmail.com>
> ---
>  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)
>

      reply	other threads:[~2026-07-09  7:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-08 17:08 [PATCH v5 0/4] media: renesas: Add missing media_entity_cleanup() Biren Pandya
2026-07-08 17:08 ` [PATCH v5 1/4] media: renesas: rcar-csi2: " Biren Pandya
2026-07-08 17:08 ` [PATCH v5 2/4] media: renesas: csisp: " Biren Pandya
2026-07-08 17:08 ` [PATCH v5 3/4] media: renesas: rcar-core: " Biren Pandya
2026-07-08 17:08 ` [PATCH v5 4/4] media: renesas: rzg2l-core: " Biren Pandya
2026-07-09  7:05   ` Jacopo Mondi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ak9G5g8sw5W_c8nf@zed \
    --to=jacopo.mondi@ideasonboard.com \
    --cc=birenpandya@gmail.com \
    --cc=geert+renesas@glider.be \
    --cc=jacopo.mondi+renesas@ideasonboard.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=magnus.damm@gmail.com \
    --cc=mchehab@kernel.org \
    --cc=niklas.soderlund@ragnatech.se \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.