In case of an error during the initialization of the sensor,
the video device is still available since created at the
probe of the dcmi driver. Moreover the device wouldn't
be released even when removing the module since the release
is performed as part of the notifier unbind callback
(not called if no sensor is properly initialized).
This patch move the video device creation with the v4l2 notifier
bound handler in order to avoid having a video device created when
an error happen during the pipe (dcmi - sensor) initialization.
This also makes the video device creation symmetric with the
release which is already done within the notifier unbind handler.
Fixes: 37404f91ef8b ("[media] stm32-dcmi: STM32 DCMI camera interface driver")
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/media/platform/stm32/stm32-dcmi.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index b8931490b83b..5e60d4c6eeeb 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -1747,6 +1747,15 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier,
dev_dbg(dcmi->dev, "Subdev \"%s\" bound\n", subdev->name);
+ ret = video_register_device(dcmi->vdev, VFL_TYPE_VIDEO, -1);
+ if (ret) {
+ dev_err(dcmi->dev, "Failed to register video device\n");
+ return ret;
+ }
+
+ dev_dbg(dcmi->dev, "Device registered as %s\n",
+ video_device_node_name(dcmi->vdev));
+
/*
* Link this sub-device to DCMI, it could be
* a parallel camera sensor or a bridge
@@ -1759,10 +1768,11 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier,
&dcmi->vdev->entity, 0,
MEDIA_LNK_FL_IMMUTABLE |
MEDIA_LNK_FL_ENABLED);
- if (ret)
+ if (ret) {
dev_err(dcmi->dev, "Failed to create media pad link with subdev \"%s\"\n",
subdev->name);
- else
+ video_unregister_device(dcmi->vdev);
+ } else
dev_dbg(dcmi->dev, "DCMI is now linked to \"%s\"\n",
subdev->name);
@@ -1974,15 +1984,6 @@ static int dcmi_probe(struct platform_device *pdev)
}
dcmi->vdev->entity.flags |= MEDIA_ENT_FL_DEFAULT;
- ret = video_register_device(dcmi->vdev, VFL_TYPE_VIDEO, -1);
- if (ret) {
- dev_err(dcmi->dev, "Failed to register video device\n");
- goto err_media_entity_cleanup;
- }
-
- dev_dbg(dcmi->dev, "Device registered as %s\n",
- video_device_node_name(dcmi->vdev));
-
/* Buffer queue */
q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
q->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
--
2.7.4
Reviewed-by: Hugues Fruchet <[email protected]>
On 7/28/20 8:37 AM, Alain Volmat wrote:
> In case of an error during the initialization of the sensor,
> the video device is still available since created at the
> probe of the dcmi driver. Moreover the device wouldn't
> be released even when removing the module since the release
> is performed as part of the notifier unbind callback
> (not called if no sensor is properly initialized).
>
> This patch move the video device creation with the v4l2 notifier
> bound handler in order to avoid having a video device created when
> an error happen during the pipe (dcmi - sensor) initialization.
>
> This also makes the video device creation symmetric with the
> release which is already done within the notifier unbind handler.
>
> Fixes: 37404f91ef8b ("[media] stm32-dcmi: STM32 DCMI camera interface driver")
> Signed-off-by: Alain Volmat <[email protected]>
> ---
> drivers/media/platform/stm32/stm32-dcmi.c | 23 ++++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
> index b8931490b83b..5e60d4c6eeeb 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -1747,6 +1747,15 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier,
>
> dev_dbg(dcmi->dev, "Subdev \"%s\" bound\n", subdev->name);
>
> + ret = video_register_device(dcmi->vdev, VFL_TYPE_VIDEO, -1);
> + if (ret) {
> + dev_err(dcmi->dev, "Failed to register video device\n");
> + return ret;
> + }
> +
> + dev_dbg(dcmi->dev, "Device registered as %s\n",
> + video_device_node_name(dcmi->vdev));
> +
> /*
> * Link this sub-device to DCMI, it could be
> * a parallel camera sensor or a bridge
> @@ -1759,10 +1768,11 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier,
> &dcmi->vdev->entity, 0,
> MEDIA_LNK_FL_IMMUTABLE |
> MEDIA_LNK_FL_ENABLED);
> - if (ret)
> + if (ret) {
> dev_err(dcmi->dev, "Failed to create media pad link with subdev \"%s\"\n",
> subdev->name);
> - else
> + video_unregister_device(dcmi->vdev);
> + } else
> dev_dbg(dcmi->dev, "DCMI is now linked to \"%s\"\n",
> subdev->name);
>
> @@ -1974,15 +1984,6 @@ static int dcmi_probe(struct platform_device *pdev)
> }
> dcmi->vdev->entity.flags |= MEDIA_ENT_FL_DEFAULT;
>
> - ret = video_register_device(dcmi->vdev, VFL_TYPE_VIDEO, -1);
> - if (ret) {
> - dev_err(dcmi->dev, "Failed to register video device\n");
> - goto err_media_entity_cleanup;
> - }
> -
> - dev_dbg(dcmi->dev, "Device registered as %s\n",
> - video_device_node_name(dcmi->vdev));
> -
> /* Buffer queue */
> q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> q->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
>
On 28/07/2020 08:37, Alain Volmat wrote:
> In case of an error during the initialization of the sensor,
> the video device is still available since created at the
> probe of the dcmi driver. Moreover the device wouldn't
> be released even when removing the module since the release
> is performed as part of the notifier unbind callback
> (not called if no sensor is properly initialized).
>
> This patch move the video device creation with the v4l2 notifier
> bound handler in order to avoid having a video device created when
> an error happen during the pipe (dcmi - sensor) initialization.
>
> This also makes the video device creation symmetric with the
> release which is already done within the notifier unbind handler.
>
> Fixes: 37404f91ef8b ("[media] stm32-dcmi: STM32 DCMI camera interface driver")
> Signed-off-by: Alain Volmat <[email protected]>
> ---
> drivers/media/platform/stm32/stm32-dcmi.c | 23 ++++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
> index b8931490b83b..5e60d4c6eeeb 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -1747,6 +1747,15 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier,
>
> dev_dbg(dcmi->dev, "Subdev \"%s\" bound\n", subdev->name);
>
> + ret = video_register_device(dcmi->vdev, VFL_TYPE_VIDEO, -1);
> + if (ret) {
> + dev_err(dcmi->dev, "Failed to register video device\n");
> + return ret;
> + }
Why in the bound callback? The video device is typically created in the complete
callback, since that's the point where everything is ready.
You should not create a video device unless it is ready for use, and that's only
valid at the end of the complete callback.
Regards,
Hans
> +
> + dev_dbg(dcmi->dev, "Device registered as %s\n",
> + video_device_node_name(dcmi->vdev));
> +
> /*
> * Link this sub-device to DCMI, it could be
> * a parallel camera sensor or a bridge
> @@ -1759,10 +1768,11 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier,
> &dcmi->vdev->entity, 0,
> MEDIA_LNK_FL_IMMUTABLE |
> MEDIA_LNK_FL_ENABLED);
> - if (ret)
> + if (ret) {
> dev_err(dcmi->dev, "Failed to create media pad link with subdev \"%s\"\n",
> subdev->name);
> - else
> + video_unregister_device(dcmi->vdev);
> + } else
> dev_dbg(dcmi->dev, "DCMI is now linked to \"%s\"\n",
> subdev->name);
>
> @@ -1974,15 +1984,6 @@ static int dcmi_probe(struct platform_device *pdev)
> }
> dcmi->vdev->entity.flags |= MEDIA_ENT_FL_DEFAULT;
>
> - ret = video_register_device(dcmi->vdev, VFL_TYPE_VIDEO, -1);
> - if (ret) {
> - dev_err(dcmi->dev, "Failed to register video device\n");
> - goto err_media_entity_cleanup;
> - }
> -
> - dev_dbg(dcmi->dev, "Device registered as %s\n",
> - video_device_node_name(dcmi->vdev));
> -
> /* Buffer queue */
> q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> q->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
>