Em Fri, 16 Dec 2016 11:11:25 +0100
Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
> > Would it make sense to enforce that dependency. Can we tie /dev/media usecount
> > to /dev/video etc. usecount? In other words:
> >
> > /dev/video is opened, then open /dev/media.
>
> When a device node is registered it should increase the refcount on the media_device
> (as I proposed, that would be mdev->dev). When a device node is unregistered and the
> last user disappeared, then it can decrease the media_device refcount.
>
> So as long as anyone is using a device node, the media_device will stick around as
> well.
>
> No need to take refcounts on open/close.
That makes sense. You're meaning something like the enclosed (untested)
patch?
> One note: as I mentioned, the video_device does not set the cdev parent correctly,
> so that bug needs to be fixed first for this to work.
Actually, __video_register_device() seems to be setting the parent
properly:
if (vdev->dev_parent == NULL)
vdev->dev_parent = vdev->v4l2_dev->dev;
Thanks,
Mauro
[PATCH] Be sure that the media_device won't be freed too early
This code snippet is untested.
Signed-off-by: Mauro Carvalho chehab <mchehab@xxxxxxxxxxxxxxxx>
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 8756275e9fc4..5fdeab382069 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -706,7 +706,7 @@ int __must_check __media_device_register(struct media_device *mdev,
struct media_devnode *devnode;
int ret;
- devnode = kzalloc(sizeof(*devnode), GFP_KERNEL);
+ devnode = devm_kzalloc(mdev->dev, sizeof(*devnode), GFP_KERNEL);
if (!devnode)
return -ENOMEM;
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 8be561ab2615..14a3c56dbcac 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -196,6 +196,7 @@ static void v4l2_device_release(struct device *cd)
#if defined(CONFIG_MEDIA_CONTROLLER)
if (v4l2_dev->mdev) {
/* Remove interfaces and interface links */
+ put_device(v4l2_dev->mdev->dev);
media_devnode_remove(vdev->intf_devnode);
if (vdev->entity.function != MEDIA_ENT_F_UNKNOWN)
media_device_unregister_entity(&vdev->entity);
@@ -810,6 +811,7 @@ static int video_register_media_controller(struct video_device *vdev, int type)
return -ENOMEM;
}
}
+ get_device(vdev->v4l2_dev->dev);
/* FIXME: how to create the other interface links? */
@@ -1015,6 +1017,11 @@ void video_unregister_device(struct video_device *vdev)
if (!vdev || !video_is_registered(vdev))
return;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ if (vdev->v4l2_dev->dev)
+ put_device(vdev->v4l2_dev->dev);
+#endif
+
mutex_lock(&videodev_lock);
/* This must be in a critical section to prevent a race with v4l2_open.
* Once this bit has been cleared video_get may never be called again.
diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
index 62bbed76dbbc..53f42090c762 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -188,6 +188,7 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
err = media_device_register_entity(v4l2_dev->mdev, entity);
if (err < 0)
goto error_module;
+ get_device(v4l2_dev->mdev->dev);
}
#endif
@@ -205,6 +206,8 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
error_unregister:
#if defined(CONFIG_MEDIA_CONTROLLER)
+ if (v4l2_dev->mdev)
+ put_device(v4l2_dev->mdev->dev);
media_device_unregister_entity(entity);
#endif
error_module:
@@ -310,6 +313,7 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)
* links are removed by the function below, in the right order
*/
media_device_unregister_entity(&sd->entity);
+ put_device(v4l2_dev->mdev->dev);
}
#endif
video_unregister_device(sd->devnode);
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
