* [PATCH] media: nxp: imx8-isi: Cleanup video device in error path
@ 2025-05-22 14:30 Daniel Scally
2025-05-22 14:32 ` Fabio Estevam
2025-05-22 15:26 ` Laurent Pinchart
0 siblings, 2 replies; 5+ messages in thread
From: Daniel Scally @ 2025-05-22 14:30 UTC (permalink / raw)
To: Laurent Pinchart, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
Cc: linux-media, imx, linux-arm-kernel, Daniel Scally
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@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]);
err_v4l2:
v4l2_device_unregister(v4l2_dev);
err_media:
---
base-commit: b64b134942c8cf4801ea288b3fd38b509aedec21
change-id: 20250522-djrscally-imx8-isi-fixes-fed2d7cffb1c
Best regards,
--
Daniel Scally <dan.scally@ideasonboard.com>
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] media: nxp: imx8-isi: Cleanup video device in error path
2025-05-22 14:30 [PATCH] media: nxp: imx8-isi: Cleanup video device in error path Daniel Scally
@ 2025-05-22 14:32 ` Fabio Estevam
2025-05-22 14:37 ` Dan Scally
2025-05-22 15:26 ` Laurent Pinchart
1 sibling, 1 reply; 5+ messages in thread
From: Fabio Estevam @ 2025-05-22 14:32 UTC (permalink / raw)
To: Daniel Scally
Cc: Laurent Pinchart, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, linux-media, imx, linux-arm-kernel
Hi Daniel,
On Thu, May 22, 2025 at 11:30 AM Daniel Scally
<dan.scally@ideasonboard.com> 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@ideasonboard.com>
It looks like this deserves a Fixes: tag.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] media: nxp: imx8-isi: Cleanup video device in error path
2025-05-22 14:32 ` Fabio Estevam
@ 2025-05-22 14:37 ` Dan Scally
0 siblings, 0 replies; 5+ messages in thread
From: Dan Scally @ 2025-05-22 14:37 UTC (permalink / raw)
To: Fabio Estevam
Cc: Laurent Pinchart, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, linux-media, imx, linux-arm-kernel
Hi Fabio
On 22/05/2025 15:32, Fabio Estevam wrote:
> Hi Daniel,
>
> On Thu, May 22, 2025 at 11:30 AM Daniel Scally
> <dan.scally@ideasonboard.com> 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@ideasonboard.com>
> It looks like this deserves a Fixes: tag.
Ah, yes indeed, that should be:
Fixes: cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver")
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] media: nxp: imx8-isi: Cleanup video device in error path
2025-05-22 14:30 [PATCH] media: nxp: imx8-isi: Cleanup video device in error path Daniel Scally
2025-05-22 14:32 ` Fabio Estevam
@ 2025-05-22 15:26 ` Laurent Pinchart
2025-05-23 14:34 ` Dan Scally
1 sibling, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2025-05-22 15:26 UTC (permalink / raw)
To: Daniel Scally
Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
linux-media, imx, linux-arm-kernel
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@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.
> err_v4l2:
> v4l2_device_unregister(v4l2_dev);
> err_media:
>
> ---
> base-commit: b64b134942c8cf4801ea288b3fd38b509aedec21
> change-id: 20250522-djrscally-imx8-isi-fixes-fed2d7cffb1c
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] media: nxp: imx8-isi: Cleanup video device in error path
2025-05-22 15:26 ` Laurent Pinchart
@ 2025-05-23 14:34 ` Dan Scally
0 siblings, 0 replies; 5+ messages in thread
From: Dan Scally @ 2025-05-23 14:34 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
linux-media, imx, linux-arm-kernel
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@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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-05-23 14:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-22 14:30 [PATCH] media: nxp: imx8-isi: Cleanup video device in error path Daniel Scally
2025-05-22 14:32 ` Fabio Estevam
2025-05-22 14:37 ` Dan Scally
2025-05-22 15:26 ` Laurent Pinchart
2025-05-23 14:34 ` Dan Scally
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox