Re: [PATCH] media: imx-jpeg: convert kzalloc() to devm_kzalloc()

[Sanjay Chitroda <sanjayembeddedse@gmail.com>]

On 9 March 2026 9:24:20 pm IST, Frank Li <Frank.li@nxp.com> wrote:
>On Sun, Mar 08, 2026 at 02:34:04AM +0530, Sanjay Chitroda wrote:
>> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>>
>> The driver allcoates memory using kzalloc() and frees it in the relase
>> path. since the allocated memory is tied to the lifetime of the device,
>> devm_kzalloc() can be used instead.
>
>static const struct v4l2_file_operations mxc_jpeg_fops = {
>        .owner          = THIS_MODULE,
>        .open           = mxc_jpeg_open,
>        .release        = mxc_jpeg_release,
>        .poll           = v4l2_m2m_fop_poll,
>        .unlocked_ioctl = video_ioctl2,
>        .mmap           = v4l2_m2m_fop_mmap,
>};
>
>Look like it is not true. .open() should be called only when device open,
>not at probe()?

You are correct.

Since the context structure is allocated in .open() and released in
.release(), its lifetime is tied to the file handle rather than the
device.

Using devm_kzalloc() would defer freeing the memory until device
removal, which could cause memory accumulation across multiple
open()/close() cycles.

I'll drop this change.

>
>Frank
>
>>
>> Using device-managed allocation simplifies the error handling paths and
>> remove the need for manual cleanup.
>>
>> No functional change intended.
>>
>> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>> ---
>>  drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 11 +++--------
>>  1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> index b558700d1d96..bd4b5f08a85c 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> @@ -2200,14 +2200,12 @@ static int mxc_jpeg_open(struct file *file)
>>  	struct mxc_jpeg_ctx *ctx;
>>  	int ret = 0;
>>
>> -	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>>  	if (!ctx)
>>  		return -ENOMEM;
>>
>> -	if (mutex_lock_interruptible(&mxc_jpeg->lock)) {
>> -		ret = -ERESTARTSYS;
>> -		goto free;
>> -	}
>> +	if (mutex_lock_interruptible(&mxc_jpeg->lock))
>> +		return -ERESTARTSYS;
>>
>>  	v4l2_fh_init(&ctx->fh, mxc_vfd);
>>  	v4l2_fh_add(&ctx->fh, file);
>> @@ -2246,8 +2244,6 @@ static int mxc_jpeg_open(struct file *file)
>>  	v4l2_fh_del(&ctx->fh, file);
>>  	v4l2_fh_exit(&ctx->fh);
>>  	mutex_unlock(&mxc_jpeg->lock);
>> -free:
>> -	kfree(ctx);
>>  	return ret;
>>  }
>>
>> @@ -2754,7 +2750,6 @@ static int mxc_jpeg_release(struct file *file)
>>  	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
>>  	v4l2_fh_del(&ctx->fh, file);
>>  	v4l2_fh_exit(&ctx->fh);
>> -	kfree(ctx);
>>  	mutex_unlock(&mxc_jpeg->lock);
>>
>>  	return 0;
>> --
>> 2.34.1
>>