[PATCH] media: nxp: imx8-isi: Cleanup video device in error path

[Dan Scally]

Hi Laurent

On 22/05/2025 16:26, Laurent Pinchart wrote:
> Hi Dan,
>
> Thank you for the patch.
>
> On Thu, May 22, 2025 at 03:30:08PM +0100, Daniel Scally wrote:
>> mxc_isi_v4l2_init() registers video devices and v4l2 subdevices but
>> the video devices are not cleaned up in that function's error path
>> which means they're left hanging if it fails. Update the function
>> to clean them up properly.
>>
>> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
>> ---
>>   drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
>> index 1e79b1211b603a71f0427e82a997787110f7e4ac..4a74b988217f504a03dfe330a7bac76cdecc933c 100644
>> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
>> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
>> @@ -154,7 +154,7 @@ static int mxc_isi_v4l2_init(struct mxc_isi_dev *isi)
>>   		if (ret < 0) {
>>   			dev_err(isi->dev, "Failed to register pipe%u: %d\n", i,
>>   				ret);
>> -			goto err_v4l2;
>> +			goto err_cleanup_pipe;
>>   		}
>>   
>>   		ret = media_create_pad_link(&isi->crossbar.sd.entity,
>> @@ -164,14 +164,14 @@ static int mxc_isi_v4l2_init(struct mxc_isi_dev *isi)
>>   					    MEDIA_LNK_FL_IMMUTABLE |
>>   					    MEDIA_LNK_FL_ENABLED);
>>   		if (ret < 0)
>> -			goto err_v4l2;
>> +			goto err_cleanup_pipe;
>>   	}
>>   
>>   	/* Register the M2M device. */
>>   	ret = mxc_isi_m2m_register(isi, v4l2_dev);
>>   	if (ret < 0) {
>>   		dev_err(isi->dev, "Failed to register M2M device: %d\n", ret);
>> -		goto err_v4l2;
>> +		goto err_cleanup_pipe;
>>   	}
>>   
>>   	/* Initialize, fill and register the async notifier. */
>> @@ -212,6 +212,9 @@ static int mxc_isi_v4l2_init(struct mxc_isi_dev *isi)
>>   err_m2m:
>>   	mxc_isi_m2m_unregister(isi);
>>   	v4l2_async_nf_cleanup(&isi->notifier);
>> +err_cleanup_pipe:
>> +	for (i = 0; i < isi->pdata->num_channels; ++i)
>> +		mxc_isi_pipe_unregister(&isi->pipes[i]);
> mxc_isi_pipe_unregister() calls mxc_isi_video_unregister(), which locks
> the video->lock mutex. The mutex is initialized in
> mxc_isi_video_register(), so you will have an issue here when reaching
> this label from within the pipe registration loop for any non-registered
> pipe.
>
> One option to address this would be to return from
> mxc_isi_video_unregister() if video->pipe is NULL.


Ah - good spot, thanks

>>   err_v4l2:
>>   	v4l2_device_unregister(v4l2_dev);
>>   err_media:
>>
>> ---
>> base-commit: b64b134942c8cf4801ea288b3fd38b509aedec21
>> change-id: 20250522-djrscally-imx8-isi-fixes-fed2d7cffb1c