* [PATCHv4 1/9] cec: fill in cec chardev kobject to ease debugging
2019-03-08 13:56 [PATCHv4 0/9] Various core and virtual driver fixes Hans Verkuil
@ 2019-03-08 13:56 ` Hans Verkuil
2019-03-08 13:56 ` [PATCHv4 2/9] media-devnode: fill in media " Hans Verkuil
` (7 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-03-08 13:56 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Helen Koike, Hans Verkuil
The cec chardev kobject has no name, which made it hard to
debug when kobject debugging is turned on.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/cec/cec-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index cc875dabd765..f5d1578e256a 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -126,6 +126,7 @@ static int __must_check cec_devnode_register(struct cec_devnode *devnode,
/* Part 2: Initialize and register the character device */
cdev_init(&devnode->cdev, &cec_devnode_fops);
devnode->cdev.owner = owner;
+ kobject_set_name(&devnode->cdev.kobj, "cec%d", devnode->minor);
ret = cdev_device_add(&devnode->cdev, &devnode->dev);
if (ret) {
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCHv4 2/9] media-devnode: fill in media chardev kobject to ease debugging
2019-03-08 13:56 [PATCHv4 0/9] Various core and virtual driver fixes Hans Verkuil
2019-03-08 13:56 ` [PATCHv4 1/9] cec: fill in cec chardev kobject to ease debugging Hans Verkuil
@ 2019-03-08 13:56 ` Hans Verkuil
2019-03-08 13:56 ` [PATCHv4 3/9] vivid: use vzalloc for dev->bitmap_out Hans Verkuil
` (6 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-03-08 13:56 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Helen Koike, Hans Verkuil
The media chardev kobject has no name, which made it hard to
debug when kobject debugging is turned on.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/media-devnode.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index 6b87a721dc49..61dc05fcc55c 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -251,6 +251,7 @@ int __must_check media_devnode_register(struct media_device *mdev,
/* Part 2: Initialize the character device */
cdev_init(&devnode->cdev, &media_devnode_fops);
devnode->cdev.owner = owner;
+ kobject_set_name(&devnode->cdev.kobj, "media%d", devnode->minor);
/* Part 3: Add the media and char device */
ret = cdev_device_add(&devnode->cdev, &devnode->dev);
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCHv4 3/9] vivid: use vzalloc for dev->bitmap_out
2019-03-08 13:56 [PATCHv4 0/9] Various core and virtual driver fixes Hans Verkuil
2019-03-08 13:56 ` [PATCHv4 1/9] cec: fill in cec chardev kobject to ease debugging Hans Verkuil
2019-03-08 13:56 ` [PATCHv4 2/9] media-devnode: fill in media " Hans Verkuil
@ 2019-03-08 13:56 ` Hans Verkuil
2019-03-08 13:56 ` [PATCHv4 4/9] vim2m: replace devm_kzalloc by kzalloc Hans Verkuil
` (5 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-03-08 13:56 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Helen Koike, Hans Verkuil
When vivid is unloaded it used vfree to free dev->bitmap_out,
but it was actually allocated using kmalloc. Use vzalloc
instead, conform what vivid-vid-cap.c does.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/vivid/vivid-vid-out.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/vivid/vivid-vid-out.c b/drivers/media/platform/vivid/vivid-vid-out.c
index e61b91b414f9..9350ca65dd91 100644
--- a/drivers/media/platform/vivid/vivid-vid-out.c
+++ b/drivers/media/platform/vivid/vivid-vid-out.c
@@ -798,7 +798,7 @@ int vivid_vid_out_s_selection(struct file *file, void *fh, struct v4l2_selection
s->r.height *= factor;
if (dev->bitmap_out && (compose->width != s->r.width ||
compose->height != s->r.height)) {
- kfree(dev->bitmap_out);
+ vfree(dev->bitmap_out);
dev->bitmap_out = NULL;
}
*compose = s->r;
@@ -941,15 +941,19 @@ int vidioc_s_fmt_vid_out_overlay(struct file *file, void *priv,
return ret;
if (win->bitmap) {
- new_bitmap = memdup_user(win->bitmap, bitmap_size);
+ new_bitmap = vzalloc(bitmap_size);
- if (IS_ERR(new_bitmap))
- return PTR_ERR(new_bitmap);
+ if (!new_bitmap)
+ return -ENOMEM;
+ if (copy_from_user(new_bitmap, win->bitmap, bitmap_size)) {
+ vfree(new_bitmap);
+ return -EFAULT;
+ }
}
dev->overlay_out_top = win->w.top;
dev->overlay_out_left = win->w.left;
- kfree(dev->bitmap_out);
+ vfree(dev->bitmap_out);
dev->bitmap_out = new_bitmap;
dev->clipcount_out = win->clipcount;
if (dev->clipcount_out)
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCHv4 4/9] vim2m: replace devm_kzalloc by kzalloc
2019-03-08 13:56 [PATCHv4 0/9] Various core and virtual driver fixes Hans Verkuil
` (2 preceding siblings ...)
2019-03-08 13:56 ` [PATCHv4 3/9] vivid: use vzalloc for dev->bitmap_out Hans Verkuil
@ 2019-03-08 13:56 ` Hans Verkuil
2019-03-08 13:56 ` [PATCHv4 5/9] v4l2-subdev: add release() internal op Hans Verkuil
` (4 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-03-08 13:56 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Helen Koike, Hans Verkuil
It is not possible to use devm_kzalloc since that memory is
freed immediately when the device instance is unbound.
Various objects like the video device may still be in use
since someone has the device node open, and when that is closed
it expects the memory to be around.
So use kzalloc and release it at the appropriate time.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/vim2m.c | 35 +++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 34dcaca45d8b..dd47821fc661 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -1262,6 +1262,15 @@ static int vim2m_release(struct file *file)
return 0;
}
+static void vim2m_device_release(struct video_device *vdev)
+{
+ struct vim2m_dev *dev = container_of(vdev, struct vim2m_dev, vfd);
+
+ v4l2_device_unregister(&dev->v4l2_dev);
+ v4l2_m2m_release(dev->m2m_dev);
+ kfree(dev);
+}
+
static const struct v4l2_file_operations vim2m_fops = {
.owner = THIS_MODULE,
.open = vim2m_open,
@@ -1277,7 +1286,7 @@ static const struct video_device vim2m_videodev = {
.fops = &vim2m_fops,
.ioctl_ops = &vim2m_ioctl_ops,
.minor = -1,
- .release = video_device_release_empty,
+ .release = vim2m_device_release,
.device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING,
};
@@ -1298,13 +1307,13 @@ static int vim2m_probe(struct platform_device *pdev)
struct video_device *vfd;
int ret;
- dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
+ dev = kzalloc(sizeof(*dev), GFP_KERNEL);
if (!dev)
return -ENOMEM;
ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
if (ret)
- return ret;
+ goto error_free;
atomic_set(&dev->num_inst, 0);
mutex_init(&dev->dev_mutex);
@@ -1317,7 +1326,7 @@ static int vim2m_probe(struct platform_device *pdev)
ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
if (ret) {
v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
- goto unreg_v4l2;
+ goto error_v4l2;
}
video_set_drvdata(vfd, dev);
@@ -1330,7 +1339,7 @@ static int vim2m_probe(struct platform_device *pdev)
if (IS_ERR(dev->m2m_dev)) {
v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem device\n");
ret = PTR_ERR(dev->m2m_dev);
- goto unreg_dev;
+ goto error_dev;
}
#ifdef CONFIG_MEDIA_CONTROLLER
@@ -1346,27 +1355,29 @@ static int vim2m_probe(struct platform_device *pdev)
MEDIA_ENT_F_PROC_VIDEO_SCALER);
if (ret) {
v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller\n");
- goto unreg_m2m;
+ goto error_m2m;
}
ret = media_device_register(&dev->mdev);
if (ret) {
v4l2_err(&dev->v4l2_dev, "Failed to register mem2mem media device\n");
- goto unreg_m2m_mc;
+ goto error_m2m_mc;
}
#endif
return 0;
#ifdef CONFIG_MEDIA_CONTROLLER
-unreg_m2m_mc:
+error_m2m_mc:
v4l2_m2m_unregister_media_controller(dev->m2m_dev);
-unreg_m2m:
+error_m2m:
v4l2_m2m_release(dev->m2m_dev);
#endif
-unreg_dev:
+error_dev:
video_unregister_device(&dev->vfd);
-unreg_v4l2:
+error_v4l2:
v4l2_device_unregister(&dev->v4l2_dev);
+error_free:
+ kfree(dev);
return ret;
}
@@ -1382,9 +1393,7 @@ static int vim2m_remove(struct platform_device *pdev)
v4l2_m2m_unregister_media_controller(dev->m2m_dev);
media_device_cleanup(&dev->mdev);
#endif
- v4l2_m2m_release(dev->m2m_dev);
video_unregister_device(&dev->vfd);
- v4l2_device_unregister(&dev->v4l2_dev);
return 0;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCHv4 5/9] v4l2-subdev: add release() internal op
2019-03-08 13:56 [PATCHv4 0/9] Various core and virtual driver fixes Hans Verkuil
` (3 preceding siblings ...)
2019-03-08 13:56 ` [PATCHv4 4/9] vim2m: replace devm_kzalloc by kzalloc Hans Verkuil
@ 2019-03-08 13:56 ` Hans Verkuil
2019-03-08 13:56 ` [PATCHv4 6/9] v4l2-subdev: handle module refcounting here Hans Verkuil
` (3 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-03-08 13:56 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Helen Koike, Hans Verkuil
If the subdevice created a device node, then the v4l2_subdev cannot
be freed until the last user of the device node closes it.
This means that we need a release() callback in v4l2_subdev_internal_ops
that is called from the video_device release function so the subdevice
driver can postpone freeing memory until the that callback is called.
If no video device node was created then the release callback can
be called immediately when the subdev is unregistered.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/v4l2-core/v4l2-device.c | 19 ++++++++++++++-----
include/media/v4l2-subdev.h | 13 ++++++++++++-
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
index e0ddb9a52bd1..7cca0de1b730 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -216,10 +216,18 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
}
EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
+static void v4l2_subdev_release(struct v4l2_subdev *sd)
+{
+ struct module *owner = !sd->owner_v4l2_dev ? sd->owner : NULL;
+
+ if (sd->internal_ops && sd->internal_ops->release)
+ sd->internal_ops->release(sd);
+ module_put(owner);
+}
+
static void v4l2_device_release_subdev_node(struct video_device *vdev)
{
- struct v4l2_subdev *sd = video_get_drvdata(vdev);
- sd->devnode = NULL;
+ v4l2_subdev_release(video_get_drvdata(vdev));
kfree(vdev);
}
@@ -318,8 +326,9 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)
media_device_unregister_entity(&sd->entity);
}
#endif
- video_unregister_device(sd->devnode);
- if (!sd->owner_v4l2_dev)
- module_put(sd->owner);
+ if (sd->devnode)
+ video_unregister_device(sd->devnode);
+ else
+ v4l2_subdev_release(sd);
}
EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev);
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 349e1c18cf48..0ee7ecd5ce77 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -755,7 +755,17 @@ struct v4l2_subdev_ops {
*
* @open: called when the subdev device node is opened by an application.
*
- * @close: called when the subdev device node is closed.
+ * @close: called when the subdev device node is closed. Please note that
+ * it is possible for @close to be called after @unregistered!
+ *
+ * @release: called when the last user of the subdev device is gone. This
+ * happens after the @unregistered callback and when the last open
+ * filehandle to the v4l-subdevX device node was closed. If no device
+ * node was created for this sub-device, then the @release callback
+ * is called right after the @unregistered callback.
+ * The @release callback is typically used to free the memory containing
+ * the v4l2_subdev structure. It is almost certainly required for any
+ * sub-device that sets the V4L2_SUBDEV_FL_HAS_DEVNODE flag.
*
* .. note::
* Never call this from drivers, only the v4l2 framework can call
@@ -766,6 +776,7 @@ struct v4l2_subdev_internal_ops {
void (*unregistered)(struct v4l2_subdev *sd);
int (*open)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh);
int (*close)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh);
+ void (*release)(struct v4l2_subdev *sd);
};
#define V4L2_SUBDEV_NAME_SIZE 32
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCHv4 6/9] v4l2-subdev: handle module refcounting here
2019-03-08 13:56 [PATCHv4 0/9] Various core and virtual driver fixes Hans Verkuil
` (4 preceding siblings ...)
2019-03-08 13:56 ` [PATCHv4 5/9] v4l2-subdev: add release() internal op Hans Verkuil
@ 2019-03-08 13:56 ` Hans Verkuil
2019-03-08 13:56 ` [PATCHv4 7/9] vimc: zero the media_device on probe Hans Verkuil
` (2 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-03-08 13:56 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Helen Koike, Hans Verkuil
The module ownership refcounting was done in media_entity_get/put,
but that was very confusing and it did not work either in case an
application had a v4l-subdevX device open and the module was
unbound. When the v4l-subdevX device was closed the media_entity_put
was never called and the module refcount was left one too high, making
it impossible to unload it.
Since v4l2-subdev.c was the only place where media_entity_get/put was
called, just move the functionality to v4l2-subdev.c and drop those
confusing entity functions.
Store the module in subdev_fh so module_put no longer depends on
the media_entity struct.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/media-entity.c | 28 ---------------------------
drivers/media/v4l2-core/v4l2-subdev.c | 22 +++++++++------------
include/media/media-entity.h | 24 -----------------------
include/media/v4l2-subdev.h | 2 ++
4 files changed, 11 insertions(+), 65 deletions(-)
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 0b1cb3559140..dd859d7b235a 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -17,7 +17,6 @@
*/
#include <linux/bitmap.h>
-#include <linux/module.h>
#include <linux/property.h>
#include <linux/slab.h>
#include <media/media-entity.h>
@@ -587,33 +586,6 @@ void media_pipeline_stop(struct media_entity *entity)
}
EXPORT_SYMBOL_GPL(media_pipeline_stop);
-/* -----------------------------------------------------------------------------
- * Module use count
- */
-
-struct media_entity *media_entity_get(struct media_entity *entity)
-{
- if (entity == NULL)
- return NULL;
-
- if (entity->graph_obj.mdev->dev &&
- !try_module_get(entity->graph_obj.mdev->dev->driver->owner))
- return NULL;
-
- return entity;
-}
-EXPORT_SYMBOL_GPL(media_entity_get);
-
-void media_entity_put(struct media_entity *entity)
-{
- if (entity == NULL)
- return;
-
- if (entity->graph_obj.mdev->dev)
- module_put(entity->graph_obj.mdev->dev->driver->owner);
-}
-EXPORT_SYMBOL_GPL(media_entity_put);
-
/* -----------------------------------------------------------------------------
* Links management
*/
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index f5f0d71ec745..d75815ab0d7b 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -18,6 +18,7 @@
#include <linux/ioctl.h>
#include <linux/mm.h>
+#include <linux/module.h>
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/videodev2.h>
@@ -54,9 +55,6 @@ static int subdev_open(struct file *file)
struct video_device *vdev = video_devdata(file);
struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
struct v4l2_subdev_fh *subdev_fh;
-#if defined(CONFIG_MEDIA_CONTROLLER)
- struct media_entity *entity = NULL;
-#endif
int ret;
subdev_fh = kzalloc(sizeof(*subdev_fh), GFP_KERNEL);
@@ -73,12 +71,15 @@ static int subdev_open(struct file *file)
v4l2_fh_add(&subdev_fh->vfh);
file->private_data = &subdev_fh->vfh;
#if defined(CONFIG_MEDIA_CONTROLLER)
- if (sd->v4l2_dev->mdev) {
- entity = media_entity_get(&sd->entity);
- if (!entity) {
+ if (sd->v4l2_dev->mdev && sd->entity.graph_obj.mdev->dev) {
+ struct module *owner;
+
+ owner = sd->entity.graph_obj.mdev->dev->driver->owner;
+ if (!try_module_get(owner)) {
ret = -EBUSY;
goto err;
}
+ subdev_fh->owner = owner;
}
#endif
@@ -91,9 +92,7 @@ static int subdev_open(struct file *file)
return 0;
err:
-#if defined(CONFIG_MEDIA_CONTROLLER)
- media_entity_put(entity);
-#endif
+ module_put(subdev_fh->owner);
v4l2_fh_del(&subdev_fh->vfh);
v4l2_fh_exit(&subdev_fh->vfh);
subdev_fh_free(subdev_fh);
@@ -111,10 +110,7 @@ static int subdev_close(struct file *file)
if (sd->internal_ops && sd->internal_ops->close)
sd->internal_ops->close(sd, subdev_fh);
-#if defined(CONFIG_MEDIA_CONTROLLER)
- if (sd->v4l2_dev->mdev)
- media_entity_put(&sd->entity);
-#endif
+ module_put(subdev_fh->owner);
v4l2_fh_del(vfh);
v4l2_fh_exit(vfh);
subdev_fh_free(subdev_fh);
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index e5f6960d92f6..3cbad42e3693 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -864,19 +864,6 @@ struct media_link *media_entity_find_link(struct media_pad *source,
*/
struct media_pad *media_entity_remote_pad(const struct media_pad *pad);
-/**
- * media_entity_get - Get a reference to the parent module
- *
- * @entity: The entity
- *
- * Get a reference to the parent media device module.
- *
- * The function will return immediately if @entity is %NULL.
- *
- * Return: returns a pointer to the entity on success or %NULL on failure.
- */
-struct media_entity *media_entity_get(struct media_entity *entity);
-
/**
* media_entity_get_fwnode_pad - Get pad number from fwnode
*
@@ -916,17 +903,6 @@ __must_check int media_graph_walk_init(
*/
void media_graph_walk_cleanup(struct media_graph *graph);
-/**
- * media_entity_put - Release the reference to the parent module
- *
- * @entity: The entity
- *
- * Release the reference count acquired by media_entity_get().
- *
- * The function will return immediately if @entity is %NULL.
- */
-void media_entity_put(struct media_entity *entity);
-
/**
* media_graph_walk_start - Start walking the media graph at a
* given entity
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 0ee7ecd5ce77..0c2e3a8f8200 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -910,9 +910,11 @@ struct v4l2_subdev {
*
* @vfh: pointer to &struct v4l2_fh
* @pad: pointer to &struct v4l2_subdev_pad_config
+ * @owner: module pointer to the owner of this file handle
*/
struct v4l2_subdev_fh {
struct v4l2_fh vfh;
+ struct module *owner;
#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
struct v4l2_subdev_pad_config *pad;
#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCHv4 7/9] vimc: zero the media_device on probe
2019-03-08 13:56 [PATCHv4 0/9] Various core and virtual driver fixes Hans Verkuil
` (5 preceding siblings ...)
2019-03-08 13:56 ` [PATCHv4 6/9] v4l2-subdev: handle module refcounting here Hans Verkuil
@ 2019-03-08 13:56 ` Hans Verkuil
2019-03-08 14:09 ` Laurent Pinchart
2019-03-08 13:56 ` [PATCHv4 8/9] vimc: free vimc_cap_device when the last user disappears Hans Verkuil
2019-03-08 13:56 ` [PATCHv4 9/9] vimc: use new release op Hans Verkuil
8 siblings, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2019-03-08 13:56 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Helen Koike, Hans Verkuil
The media_device is part of a static global vimc_device struct.
The media framework expects this to be zeroed before it is
used, however, since this is a global this is not the case if
vimc is unbound and then bound again.
So call memset to ensure any left-over values are cleared.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/platform/vimc/vimc-core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/platform/vimc/vimc-core.c b/drivers/media/platform/vimc/vimc-core.c
index 0fbb7914098f..3aa62d7e3d0e 100644
--- a/drivers/media/platform/vimc/vimc-core.c
+++ b/drivers/media/platform/vimc/vimc-core.c
@@ -304,6 +304,8 @@ static int vimc_probe(struct platform_device *pdev)
dev_dbg(&pdev->dev, "probe");
+ memset(&vimc->mdev, 0, sizeof(vimc->mdev));
+
/* Create platform_device for each entity in the topology*/
vimc->subdevs = devm_kcalloc(&vimc->pdev.dev, vimc->pipe_cfg->num_ents,
sizeof(*vimc->subdevs), GFP_KERNEL);
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCHv4 7/9] vimc: zero the media_device on probe
2019-03-08 13:56 ` [PATCHv4 7/9] vimc: zero the media_device on probe Hans Verkuil
@ 2019-03-08 14:09 ` Laurent Pinchart
2019-03-08 14:13 ` Hans Verkuil
0 siblings, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2019-03-08 14:09 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Helen Koike
Hi Hans,
Thank you for the patch.
On Fri, Mar 08, 2019 at 02:56:23PM +0100, Hans Verkuil wrote:
> The media_device is part of a static global vimc_device struct.
> The media framework expects this to be zeroed before it is
> used, however, since this is a global this is not the case if
> vimc is unbound and then bound again.
>
> So call memset to ensure any left-over values are cleared.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Do I recall correctly that you mentioned there's work in progress that
will allocate this dynamically ? If so feel free to mention it in the
commit message if you want.
> ---
> drivers/media/platform/vimc/vimc-core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/platform/vimc/vimc-core.c b/drivers/media/platform/vimc/vimc-core.c
> index 0fbb7914098f..3aa62d7e3d0e 100644
> --- a/drivers/media/platform/vimc/vimc-core.c
> +++ b/drivers/media/platform/vimc/vimc-core.c
> @@ -304,6 +304,8 @@ static int vimc_probe(struct platform_device *pdev)
>
> dev_dbg(&pdev->dev, "probe");
>
> + memset(&vimc->mdev, 0, sizeof(vimc->mdev));
> +
> /* Create platform_device for each entity in the topology*/
> vimc->subdevs = devm_kcalloc(&vimc->pdev.dev, vimc->pipe_cfg->num_ents,
> sizeof(*vimc->subdevs), GFP_KERNEL);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv4 7/9] vimc: zero the media_device on probe
2019-03-08 14:09 ` Laurent Pinchart
@ 2019-03-08 14:13 ` Hans Verkuil
0 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-03-08 14:13 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, Helen Koike
On 3/8/19 3:09 PM, Laurent Pinchart wrote:
> Hi Hans,
>
> Thank you for the patch.
>
> On Fri, Mar 08, 2019 at 02:56:23PM +0100, Hans Verkuil wrote:
>> The media_device is part of a static global vimc_device struct.
>> The media framework expects this to be zeroed before it is
>> used, however, since this is a global this is not the case if
>> vimc is unbound and then bound again.
>>
>> So call memset to ensure any left-over values are cleared.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Do I recall correctly that you mentioned there's work in progress that
> will allocate this dynamically ? If so feel free to mention it in the
> commit message if you want.
There is, but it might take some time before that will land (it's Helen's
patch series that converts vimc to use configfs to configure all the
subdevs and topology).
Regards,
Hans
>
>> ---
>> drivers/media/platform/vimc/vimc-core.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/media/platform/vimc/vimc-core.c b/drivers/media/platform/vimc/vimc-core.c
>> index 0fbb7914098f..3aa62d7e3d0e 100644
>> --- a/drivers/media/platform/vimc/vimc-core.c
>> +++ b/drivers/media/platform/vimc/vimc-core.c
>> @@ -304,6 +304,8 @@ static int vimc_probe(struct platform_device *pdev)
>>
>> dev_dbg(&pdev->dev, "probe");
>>
>> + memset(&vimc->mdev, 0, sizeof(vimc->mdev));
>> +
>> /* Create platform_device for each entity in the topology*/
>> vimc->subdevs = devm_kcalloc(&vimc->pdev.dev, vimc->pipe_cfg->num_ents,
>> sizeof(*vimc->subdevs), GFP_KERNEL);
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCHv4 8/9] vimc: free vimc_cap_device when the last user disappears
2019-03-08 13:56 [PATCHv4 0/9] Various core and virtual driver fixes Hans Verkuil
` (6 preceding siblings ...)
2019-03-08 13:56 ` [PATCHv4 7/9] vimc: zero the media_device on probe Hans Verkuil
@ 2019-03-08 13:56 ` Hans Verkuil
2019-03-08 13:56 ` [PATCHv4 9/9] vimc: use new release op Hans Verkuil
8 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-03-08 13:56 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Helen Koike, Hans Verkuil
Don't free vimc_cap_device immediately, instead do this
in the video_device release function which is called when the
last user closes the video device. Only then is it safe to
free the memory.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/vimc/vimc-capture.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
index ea869631a3f6..3d433361d297 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -338,6 +338,15 @@ static const struct media_entity_operations vimc_cap_mops = {
.link_validate = vimc_link_validate,
};
+static void vimc_cap_release(struct video_device *vdev)
+{
+ struct vimc_cap_device *vcap = container_of(vdev, struct vimc_cap_device,
+ vdev);
+
+ vimc_pads_cleanup(vcap->ved.pads);
+ kfree(vcap);
+}
+
static void vimc_cap_comp_unbind(struct device *comp, struct device *master,
void *master_data)
{
@@ -348,8 +357,6 @@ static void vimc_cap_comp_unbind(struct device *comp, struct device *master,
vb2_queue_release(&vcap->queue);
media_entity_cleanup(ved->ent);
video_unregister_device(&vcap->vdev);
- vimc_pads_cleanup(vcap->ved.pads);
- kfree(vcap);
}
static void *vimc_cap_process_frame(struct vimc_ent_device *ved,
@@ -467,7 +474,7 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master,
vdev = &vcap->vdev;
vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
vdev->entity.ops = &vimc_cap_mops;
- vdev->release = video_device_release_empty;
+ vdev->release = vimc_cap_release;
vdev->fops = &vimc_cap_fops;
vdev->ioctl_ops = &vimc_cap_ioctl_ops;
vdev->lock = &vcap->lock;
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCHv4 9/9] vimc: use new release op
2019-03-08 13:56 [PATCHv4 0/9] Various core and virtual driver fixes Hans Verkuil
` (7 preceding siblings ...)
2019-03-08 13:56 ` [PATCHv4 8/9] vimc: free vimc_cap_device when the last user disappears Hans Verkuil
@ 2019-03-08 13:56 ` Hans Verkuil
2019-03-11 12:34 ` [PATCHv4.1 " Hans Verkuil
8 siblings, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2019-03-08 13:56 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Helen Koike, Hans Verkuil
Use the new v4l2_subdev_internal_ops release op to free the
subdev memory only once the last user closed the file handle.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/vimc/vimc-common.c | 2 ++
drivers/media/platform/vimc/vimc-common.h | 2 ++
drivers/media/platform/vimc/vimc-debayer.c | 15 +++++++++++++--
drivers/media/platform/vimc/vimc-scaler.c | 15 +++++++++++++--
drivers/media/platform/vimc/vimc-sensor.c | 19 +++++++++++++++----
5 files changed, 45 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c
index c1a74bb2df58..0adbfd8fd26d 100644
--- a/drivers/media/platform/vimc/vimc-common.c
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -380,6 +380,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
u32 function,
u16 num_pads,
const unsigned long *pads_flag,
+ const struct v4l2_subdev_internal_ops *sd_int_ops,
const struct v4l2_subdev_ops *sd_ops)
{
int ret;
@@ -394,6 +395,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
/* Initialize the subdev */
v4l2_subdev_init(sd, sd_ops);
+ sd->internal_ops = sd_int_ops;
sd->entity.function = function;
sd->entity.ops = &vimc_ent_sd_mops;
sd->owner = THIS_MODULE;
diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
index 84539430b5e7..c439cbf2f030 100644
--- a/drivers/media/platform/vimc/vimc-common.h
+++ b/drivers/media/platform/vimc/vimc-common.h
@@ -187,6 +187,7 @@ const struct vimc_pix_map *vimc_pix_map_by_pixelformat(u32 pixelformat);
* @function: media entity function defined by MEDIA_ENT_F_* macros
* @num_pads: number of pads to initialize
* @pads_flag: flags to use in each pad
+ * @sd_int_ops: pointer to &struct v4l2_subdev_internal_ops
* @sd_ops: pointer to &struct v4l2_subdev_ops.
*
* Helper function initialize and register the struct vimc_ent_device and struct
@@ -199,6 +200,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
u32 function,
u16 num_pads,
const unsigned long *pads_flag,
+ const struct v4l2_subdev_internal_ops *sd_int_ops,
const struct v4l2_subdev_ops *sd_ops);
/**
diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
index 7d77c63b99d2..eaed4233ad1b 100644
--- a/drivers/media/platform/vimc/vimc-debayer.c
+++ b/drivers/media/platform/vimc/vimc-debayer.c
@@ -489,6 +489,18 @@ static void *vimc_deb_process_frame(struct vimc_ent_device *ved,
}
+static void vimc_deb_release(struct v4l2_subdev *sd)
+{
+ struct vimc_deb_device *vdeb =
+ container_of(sd, struct vimc_deb_device, sd);
+
+ kfree(vdeb);
+}
+
+static const struct v4l2_subdev_internal_ops vimc_deb_int_ops = {
+ .release = vimc_deb_release,
+};
+
static void vimc_deb_comp_unbind(struct device *comp, struct device *master,
void *master_data)
{
@@ -497,7 +509,6 @@ static void vimc_deb_comp_unbind(struct device *comp, struct device *master,
ved);
vimc_ent_sd_unregister(ved, &vdeb->sd);
- kfree(vdeb);
}
static int vimc_deb_comp_bind(struct device *comp, struct device *master,
@@ -519,7 +530,7 @@ static int vimc_deb_comp_bind(struct device *comp, struct device *master,
MEDIA_ENT_F_PROC_VIDEO_PIXEL_ENC_CONV, 2,
(const unsigned long[2]) {MEDIA_PAD_FL_SINK,
MEDIA_PAD_FL_SOURCE},
- &vimc_deb_ops);
+ &vimc_deb_int_ops, &vimc_deb_ops);
if (ret) {
kfree(vdeb);
return ret;
diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
index 39b2a73dfcc1..2028afa4ef7a 100644
--- a/drivers/media/platform/vimc/vimc-scaler.c
+++ b/drivers/media/platform/vimc/vimc-scaler.c
@@ -348,6 +348,18 @@ static void *vimc_sca_process_frame(struct vimc_ent_device *ved,
return vsca->src_frame;
};
+static void vimc_sca_release(struct v4l2_subdev *sd)
+{
+ struct vimc_sca_device *vsca =
+ container_of(sd, struct vimc_sca_device, sd);
+
+ kfree(vsca);
+}
+
+static const struct v4l2_subdev_internal_ops vimc_sca_int_ops = {
+ .release = vimc_sca_release,
+};
+
static void vimc_sca_comp_unbind(struct device *comp, struct device *master,
void *master_data)
{
@@ -356,7 +368,6 @@ static void vimc_sca_comp_unbind(struct device *comp, struct device *master,
ved);
vimc_ent_sd_unregister(ved, &vsca->sd);
- kfree(vsca);
}
@@ -379,7 +390,7 @@ static int vimc_sca_comp_bind(struct device *comp, struct device *master,
MEDIA_ENT_F_PROC_VIDEO_SCALER, 2,
(const unsigned long[2]) {MEDIA_PAD_FL_SINK,
MEDIA_PAD_FL_SOURCE},
- &vimc_sca_ops);
+ &vimc_sca_int_ops, &vimc_sca_ops);
if (ret) {
kfree(vsca);
return ret;
diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
index 59195f262623..d7891d3bbeaa 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -301,6 +301,20 @@ static const struct v4l2_ctrl_ops vimc_sen_ctrl_ops = {
.s_ctrl = vimc_sen_s_ctrl,
};
+static void vimc_sen_release(struct v4l2_subdev *sd)
+{
+ struct vimc_sen_device *vsen =
+ container_of(sd, struct vimc_sen_device, sd);
+
+ v4l2_ctrl_handler_free(&vsen->hdl);
+ tpg_free(&vsen->tpg);
+ kfree(vsen);
+}
+
+static const struct v4l2_subdev_internal_ops vimc_sen_int_ops = {
+ .release = vimc_sen_release,
+};
+
static void vimc_sen_comp_unbind(struct device *comp, struct device *master,
void *master_data)
{
@@ -309,9 +323,6 @@ static void vimc_sen_comp_unbind(struct device *comp, struct device *master,
container_of(ved, struct vimc_sen_device, ved);
vimc_ent_sd_unregister(ved, &vsen->sd);
- v4l2_ctrl_handler_free(&vsen->hdl);
- tpg_free(&vsen->tpg);
- kfree(vsen);
}
/* Image Processing Controls */
@@ -371,7 +382,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct device *master,
pdata->entity_name,
MEDIA_ENT_F_CAM_SENSOR, 1,
(const unsigned long[1]) {MEDIA_PAD_FL_SOURCE},
- &vimc_sen_ops);
+ &vimc_sen_int_ops, &vimc_sen_ops);
if (ret)
goto err_free_hdl;
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCHv4.1 9/9] vimc: use new release op
2019-03-08 13:56 ` [PATCHv4 9/9] vimc: use new release op Hans Verkuil
@ 2019-03-11 12:34 ` Hans Verkuil
0 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-03-11 12:34 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Helen Koike
Use the new v4l2_subdev_internal_ops release op to free the
subdev memory only when the last user closed the file handle.
Move v4l2_device_unregister_subdev() to the end of the
vimc_ent_sd_unregister() function since now the unregister_subdev()
call may free the vimc_ent_device struct which is used after the
unregister_subdev() call. So this now has to be done last.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Change since v4:
Move v4l2_device_unregister_subdev() to the end in function
vimc_ent_sd_unregister() to avoid a use-after-free.
---
drivers/media/platform/vimc/vimc-common.c | 4 +++-
drivers/media/platform/vimc/vimc-common.h | 2 ++
drivers/media/platform/vimc/vimc-debayer.c | 15 +++++++++++++--
drivers/media/platform/vimc/vimc-scaler.c | 15 +++++++++++++--
drivers/media/platform/vimc/vimc-sensor.c | 19 +++++++++++++++----
5 files changed, 46 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c
index c1a74bb2df58..e8e6c6b95db9 100644
--- a/drivers/media/platform/vimc/vimc-common.c
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -380,6 +380,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
u32 function,
u16 num_pads,
const unsigned long *pads_flag,
+ const struct v4l2_subdev_internal_ops *sd_int_ops,
const struct v4l2_subdev_ops *sd_ops)
{
int ret;
@@ -394,6 +395,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
/* Initialize the subdev */
v4l2_subdev_init(sd, sd_ops);
+ sd->internal_ops = sd_int_ops;
sd->entity.function = function;
sd->entity.ops = &vimc_ent_sd_mops;
sd->owner = THIS_MODULE;
@@ -431,9 +433,9 @@ EXPORT_SYMBOL_GPL(vimc_ent_sd_register);
void vimc_ent_sd_unregister(struct vimc_ent_device *ved, struct v4l2_subdev *sd)
{
- v4l2_device_unregister_subdev(sd);
media_entity_cleanup(ved->ent);
vimc_pads_cleanup(ved->pads);
+ v4l2_device_unregister_subdev(sd);
}
EXPORT_SYMBOL_GPL(vimc_ent_sd_unregister);
diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
index 84539430b5e7..c439cbf2f030 100644
--- a/drivers/media/platform/vimc/vimc-common.h
+++ b/drivers/media/platform/vimc/vimc-common.h
@@ -187,6 +187,7 @@ const struct vimc_pix_map *vimc_pix_map_by_pixelformat(u32 pixelformat);
* @function: media entity function defined by MEDIA_ENT_F_* macros
* @num_pads: number of pads to initialize
* @pads_flag: flags to use in each pad
+ * @sd_int_ops: pointer to &struct v4l2_subdev_internal_ops
* @sd_ops: pointer to &struct v4l2_subdev_ops.
*
* Helper function initialize and register the struct vimc_ent_device and struct
@@ -199,6 +200,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
u32 function,
u16 num_pads,
const unsigned long *pads_flag,
+ const struct v4l2_subdev_internal_ops *sd_int_ops,
const struct v4l2_subdev_ops *sd_ops);
/**
diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
index 7d77c63b99d2..eaed4233ad1b 100644
--- a/drivers/media/platform/vimc/vimc-debayer.c
+++ b/drivers/media/platform/vimc/vimc-debayer.c
@@ -489,6 +489,18 @@ static void *vimc_deb_process_frame(struct vimc_ent_device *ved,
}
+static void vimc_deb_release(struct v4l2_subdev *sd)
+{
+ struct vimc_deb_device *vdeb =
+ container_of(sd, struct vimc_deb_device, sd);
+
+ kfree(vdeb);
+}
+
+static const struct v4l2_subdev_internal_ops vimc_deb_int_ops = {
+ .release = vimc_deb_release,
+};
+
static void vimc_deb_comp_unbind(struct device *comp, struct device *master,
void *master_data)
{
@@ -497,7 +509,6 @@ static void vimc_deb_comp_unbind(struct device *comp, struct device *master,
ved);
vimc_ent_sd_unregister(ved, &vdeb->sd);
- kfree(vdeb);
}
static int vimc_deb_comp_bind(struct device *comp, struct device *master,
@@ -519,7 +530,7 @@ static int vimc_deb_comp_bind(struct device *comp, struct device *master,
MEDIA_ENT_F_PROC_VIDEO_PIXEL_ENC_CONV, 2,
(const unsigned long[2]) {MEDIA_PAD_FL_SINK,
MEDIA_PAD_FL_SOURCE},
- &vimc_deb_ops);
+ &vimc_deb_int_ops, &vimc_deb_ops);
if (ret) {
kfree(vdeb);
return ret;
diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
index 39b2a73dfcc1..2028afa4ef7a 100644
--- a/drivers/media/platform/vimc/vimc-scaler.c
+++ b/drivers/media/platform/vimc/vimc-scaler.c
@@ -348,6 +348,18 @@ static void *vimc_sca_process_frame(struct vimc_ent_device *ved,
return vsca->src_frame;
};
+static void vimc_sca_release(struct v4l2_subdev *sd)
+{
+ struct vimc_sca_device *vsca =
+ container_of(sd, struct vimc_sca_device, sd);
+
+ kfree(vsca);
+}
+
+static const struct v4l2_subdev_internal_ops vimc_sca_int_ops = {
+ .release = vimc_sca_release,
+};
+
static void vimc_sca_comp_unbind(struct device *comp, struct device *master,
void *master_data)
{
@@ -356,7 +368,6 @@ static void vimc_sca_comp_unbind(struct device *comp, struct device *master,
ved);
vimc_ent_sd_unregister(ved, &vsca->sd);
- kfree(vsca);
}
@@ -379,7 +390,7 @@ static int vimc_sca_comp_bind(struct device *comp, struct device *master,
MEDIA_ENT_F_PROC_VIDEO_SCALER, 2,
(const unsigned long[2]) {MEDIA_PAD_FL_SINK,
MEDIA_PAD_FL_SOURCE},
- &vimc_sca_ops);
+ &vimc_sca_int_ops, &vimc_sca_ops);
if (ret) {
kfree(vsca);
return ret;
diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
index 59195f262623..d7891d3bbeaa 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -301,6 +301,20 @@ static const struct v4l2_ctrl_ops vimc_sen_ctrl_ops = {
.s_ctrl = vimc_sen_s_ctrl,
};
+static void vimc_sen_release(struct v4l2_subdev *sd)
+{
+ struct vimc_sen_device *vsen =
+ container_of(sd, struct vimc_sen_device, sd);
+
+ v4l2_ctrl_handler_free(&vsen->hdl);
+ tpg_free(&vsen->tpg);
+ kfree(vsen);
+}
+
+static const struct v4l2_subdev_internal_ops vimc_sen_int_ops = {
+ .release = vimc_sen_release,
+};
+
static void vimc_sen_comp_unbind(struct device *comp, struct device *master,
void *master_data)
{
@@ -309,9 +323,6 @@ static void vimc_sen_comp_unbind(struct device *comp, struct device *master,
container_of(ved, struct vimc_sen_device, ved);
vimc_ent_sd_unregister(ved, &vsen->sd);
- v4l2_ctrl_handler_free(&vsen->hdl);
- tpg_free(&vsen->tpg);
- kfree(vsen);
}
/* Image Processing Controls */
@@ -371,7 +382,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct device *master,
pdata->entity_name,
MEDIA_ENT_F_CAM_SENSOR, 1,
(const unsigned long[1]) {MEDIA_PAD_FL_SOURCE},
- &vimc_sen_ops);
+ &vimc_sen_int_ops, &vimc_sen_ops);
if (ret)
goto err_free_hdl;
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread