linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [REVIEW PATCH 0/9] Use v4l2_dev instead of parent.
@ 2013-06-10 12:48 Hans Verkuil
  2013-06-10 12:48 ` [REVIEW PATCH 1/9] soc_camera: replace vdev->parent by vdev->v4l2_dev Hans Verkuil
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Hans Verkuil @ 2013-06-10 12:48 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, Guennadi Liakhovetski, Mike Isely

This patch series converts the last set of drivers still using the parent
field of video_device to using v4l2_dev instead and at the end the parent
field is removed altogether.

A proper pointer to v4l2_dev is necessary otherwise the advanced debugging
ioctls will not work when addressing sub-devices.

I have been steadily converting drivers to set the v4l2_dev pointer correctly,
and this patch series finishes that work.

Guennadi, the first patch replaces the broken version I posted earlier as part
of the 'current_norm' removal patch series. I've tested it with my renesas
board.

Note that this patch series sits on top of my for_v3.11 branch.

Regards,

	Hans


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [REVIEW PATCH 1/9] soc_camera: replace vdev->parent by vdev->v4l2_dev.
  2013-06-10 12:48 [REVIEW PATCH 0/9] Use v4l2_dev instead of parent Hans Verkuil
@ 2013-06-10 12:48 ` Hans Verkuil
  2013-06-10 12:48 ` [REVIEW PATCH 2/9] cx23885-417: use v4l2_dev instead of the deprecated parent field Hans Verkuil
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2013-06-10 12:48 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Mike Isely, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

The parent field will eventually disappear to be replaced by v4l2_dev.
soc_camera does provide a v4l2_device struct but did not point to it in
struct video_device. This is now fixed.

Now the video nodes can be found under the correct platform bus, and
the advanced debug ioctls work correctly as well (the core implementation
of those ioctls requires that v4l2_dev is set correctly).

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/platform/soc_camera/soc_camera.c |    5 +++--
 include/media/soc_camera.h                     |    4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index 0252fbb..9a43560 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -524,7 +524,7 @@ static int soc_camera_open(struct file *file)
 		return -ENODEV;
 	}
 
-	icd = dev_get_drvdata(vdev->parent);
+	icd = video_get_drvdata(vdev);
 	ici = to_soc_camera_host(icd->parent);
 
 	ret = try_module_get(ici->ops->owner) ? 0 : -ENODEV;
@@ -1477,7 +1477,7 @@ static int video_dev_create(struct soc_camera_device *icd)
 
 	strlcpy(vdev->name, ici->drv_name, sizeof(vdev->name));
 
-	vdev->parent		= icd->pdev;
+	vdev->v4l2_dev		= &ici->v4l2_dev;
 	vdev->fops		= &soc_camera_fops;
 	vdev->ioctl_ops		= &soc_camera_ioctl_ops;
 	vdev->release		= video_device_release;
@@ -1500,6 +1500,7 @@ static int soc_camera_video_start(struct soc_camera_device *icd)
 	if (!icd->parent)
 		return -ENODEV;
 
+	video_set_drvdata(icd->vdev, icd);
 	ret = video_register_device(icd->vdev, VFL_TYPE_GRABBER, -1);
 	if (ret < 0) {
 		dev_err(icd->pdev, "video_register_device failed: %d\n", ret);
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index ff77d08..31a4bfe 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -346,9 +346,9 @@ static inline struct soc_camera_subdev_desc *soc_camera_i2c_to_desc(const struct
 	return client->dev.platform_data;
 }
 
-static inline struct v4l2_subdev *soc_camera_vdev_to_subdev(const struct video_device *vdev)
+static inline struct v4l2_subdev *soc_camera_vdev_to_subdev(struct video_device *vdev)
 {
-	struct soc_camera_device *icd = dev_get_drvdata(vdev->parent);
+	struct soc_camera_device *icd = video_get_drvdata(vdev);
 	return soc_camera_to_subdev(icd);
 }
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [REVIEW PATCH 2/9] cx23885-417: use v4l2_dev instead of the deprecated parent field.
  2013-06-10 12:48 [REVIEW PATCH 0/9] Use v4l2_dev instead of parent Hans Verkuil
  2013-06-10 12:48 ` [REVIEW PATCH 1/9] soc_camera: replace vdev->parent by vdev->v4l2_dev Hans Verkuil
@ 2013-06-10 12:48 ` Hans Verkuil
  2013-06-10 12:48 ` [REVIEW PATCH 3/9] zoran: " Hans Verkuil
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2013-06-10 12:48 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Mike Isely, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/cx23885/cx23885-417.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/cx23885/cx23885-417.c b/drivers/media/pci/cx23885/cx23885-417.c
index f4f9ef0..e3fc2c7 100644
--- a/drivers/media/pci/cx23885/cx23885-417.c
+++ b/drivers/media/pci/cx23885/cx23885-417.c
@@ -1732,7 +1732,7 @@ static struct video_device *cx23885_video_dev_alloc(
 	*vfd = *template;
 	snprintf(vfd->name, sizeof(vfd->name), "%s (%s)",
 		cx23885_boards[tsport->dev->board].name, type);
-	vfd->parent  = &pci->dev;
+	vfd->v4l2_dev = &dev->v4l2_dev;
 	vfd->release = video_device_release;
 	return vfd;
 }
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [REVIEW PATCH 3/9] zoran: use v4l2_dev instead of the deprecated parent field.
  2013-06-10 12:48 [REVIEW PATCH 0/9] Use v4l2_dev instead of parent Hans Verkuil
  2013-06-10 12:48 ` [REVIEW PATCH 1/9] soc_camera: replace vdev->parent by vdev->v4l2_dev Hans Verkuil
  2013-06-10 12:48 ` [REVIEW PATCH 2/9] cx23885-417: use v4l2_dev instead of the deprecated parent field Hans Verkuil
@ 2013-06-10 12:48 ` Hans Verkuil
  2013-06-10 12:48 ` [REVIEW PATCH 4/9] sn9c102: " Hans Verkuil
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2013-06-10 12:48 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Mike Isely, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/zoran/zoran_card.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/zoran/zoran_card.c b/drivers/media/pci/zoran/zoran_card.c
index bb53d24..923d59a 100644
--- a/drivers/media/pci/zoran/zoran_card.c
+++ b/drivers/media/pci/zoran/zoran_card.c
@@ -1050,7 +1050,7 @@ static int zr36057_init (struct zoran *zr)
 	 *   Now add the template and register the device unit.
 	 */
 	memcpy(zr->video_dev, &zoran_template, sizeof(zoran_template));
-	zr->video_dev->parent = &zr->pci_dev->dev;
+	zr->video_dev->v4l2_dev = &zr->v4l2_dev;
 	strcpy(zr->video_dev->name, ZR_DEVNAME(zr));
 	/* It's not a mem2mem device, but you can both capture and output from
 	   one and the same device. This should really be split up into two
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [REVIEW PATCH 4/9] sn9c102: use v4l2_dev instead of the deprecated parent field.
  2013-06-10 12:48 [REVIEW PATCH 0/9] Use v4l2_dev instead of parent Hans Verkuil
                   ` (2 preceding siblings ...)
  2013-06-10 12:48 ` [REVIEW PATCH 3/9] zoran: " Hans Verkuil
@ 2013-06-10 12:48 ` Hans Verkuil
  2013-06-10 12:48 ` [REVIEW PATCH 5/9] saa7164: " Hans Verkuil
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2013-06-10 12:48 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Mike Isely, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/sn9c102/sn9c102.h      |    3 +++
 drivers/media/usb/sn9c102/sn9c102_core.c |   12 +++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/sn9c102/sn9c102.h b/drivers/media/usb/sn9c102/sn9c102.h
index 2bc153e..8a917f06 100644
--- a/drivers/media/usb/sn9c102/sn9c102.h
+++ b/drivers/media/usb/sn9c102/sn9c102.h
@@ -25,6 +25,7 @@
 #include <linux/videodev2.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-device.h>
 #include <linux/device.h>
 #include <linux/list.h>
 #include <linux/spinlock.h>
@@ -100,6 +101,8 @@ static DECLARE_RWSEM(sn9c102_dev_lock);
 struct sn9c102_device {
 	struct video_device* v4ldev;
 
+	struct v4l2_device v4l2_dev;
+
 	enum sn9c102_bridge bridge;
 	struct sn9c102_sensor sensor;
 
diff --git a/drivers/media/usb/sn9c102/sn9c102_core.c b/drivers/media/usb/sn9c102/sn9c102_core.c
index c957e9a..b1f8082 100644
--- a/drivers/media/usb/sn9c102/sn9c102_core.c
+++ b/drivers/media/usb/sn9c102/sn9c102_core.c
@@ -1737,6 +1737,7 @@ static void sn9c102_release_resources(struct kref *kref)
 	    video_device_node_name(cam->v4ldev));
 	video_set_drvdata(cam->v4ldev, NULL);
 	video_unregister_device(cam->v4ldev);
+	v4l2_device_unregister(&cam->v4l2_dev);
 	usb_put_dev(cam->usbdev);
 	kfree(cam->control_buffer);
 	kfree(cam);
@@ -3254,6 +3255,13 @@ sn9c102_usb_probe(struct usb_interface* intf, const struct usb_device_id* id)
 
 	cam->usbdev = udev;
 
+	/* register v4l2_device early so it can be used for printks */
+	if (v4l2_device_register(&intf->dev, &cam->v4l2_dev)) {
+		dev_err(&intf->dev, "v4l2_device_register failed\n");
+		err = -ENOMEM;
+		goto fail;
+	}
+
 	if (!(cam->control_buffer = kzalloc(8, GFP_KERNEL))) {
 		DBG(1, "kzalloc() failed");
 		err = -ENOMEM;
@@ -3325,7 +3333,7 @@ sn9c102_usb_probe(struct usb_interface* intf, const struct usb_device_id* id)
 	strcpy(cam->v4ldev->name, "SN9C1xx PC Camera");
 	cam->v4ldev->fops = &sn9c102_fops;
 	cam->v4ldev->release = video_device_release;
-	cam->v4ldev->parent = &udev->dev;
+	cam->v4ldev->v4l2_dev = &cam->v4l2_dev;
 
 	init_completion(&cam->probe);
 
@@ -3407,6 +3415,8 @@ static void sn9c102_usb_disconnect(struct usb_interface* intf)
 
 	wake_up_interruptible_all(&cam->wait_open);
 
+	v4l2_device_disconnect(&cam->v4l2_dev);
+
 	kref_put(&cam->kref, sn9c102_release_resources);
 
 	up_write(&sn9c102_dev_lock);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [REVIEW PATCH 5/9] saa7164: use v4l2_dev instead of the deprecated parent field.
  2013-06-10 12:48 [REVIEW PATCH 0/9] Use v4l2_dev instead of parent Hans Verkuil
                   ` (3 preceding siblings ...)
  2013-06-10 12:48 ` [REVIEW PATCH 4/9] sn9c102: " Hans Verkuil
@ 2013-06-10 12:48 ` Hans Verkuil
  2013-06-10 12:48 ` [REVIEW PATCH 6/9] omap24xxcam: " Hans Verkuil
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2013-06-10 12:48 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Mike Isely, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/saa7164/saa7164-core.c    |    6 ++++++
 drivers/media/pci/saa7164/saa7164-encoder.c |    2 +-
 drivers/media/pci/saa7164/saa7164-vbi.c     |    2 +-
 drivers/media/pci/saa7164/saa7164.h         |    3 +++
 4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/saa7164/saa7164-core.c b/drivers/media/pci/saa7164/saa7164-core.c
index 7618fda..8fa5ba7 100644
--- a/drivers/media/pci/saa7164/saa7164-core.c
+++ b/drivers/media/pci/saa7164/saa7164-core.c
@@ -1196,6 +1196,11 @@ static int saa7164_initdev(struct pci_dev *pci_dev,
 	if (NULL == dev)
 		return -ENOMEM;
 
+	if (v4l2_device_register(&pci_dev->dev, &dev->v4l2_dev)) {
+		dev_err(&pci_dev->dev, "v4l2_device_register failed\n");
+		goto fail_free;
+	}
+
 	/* pci init */
 	dev->pci = pci_dev;
 	if (pci_enable_device(pci_dev)) {
@@ -1439,6 +1444,7 @@ static void saa7164_finidev(struct pci_dev *pci_dev)
 	mutex_unlock(&devlist);
 
 	saa7164_dev_unregister(dev);
+	v4l2_device_unregister(&dev->v4l2_dev);
 	kfree(dev);
 }
 
diff --git a/drivers/media/pci/saa7164/saa7164-encoder.c b/drivers/media/pci/saa7164/saa7164-encoder.c
index 7b7ed97..9266965 100644
--- a/drivers/media/pci/saa7164/saa7164-encoder.c
+++ b/drivers/media/pci/saa7164/saa7164-encoder.c
@@ -1348,7 +1348,7 @@ static struct video_device *saa7164_encoder_alloc(
 	snprintf(vfd->name, sizeof(vfd->name), "%s %s (%s)", dev->name,
 		type, saa7164_boards[dev->board].name);
 
-	vfd->parent  = &pci->dev;
+	vfd->v4l2_dev  = &dev->v4l2_dev;
 	vfd->release = video_device_release;
 	return vfd;
 }
diff --git a/drivers/media/pci/saa7164/saa7164-vbi.c b/drivers/media/pci/saa7164/saa7164-vbi.c
index 552c01a..6e025fe 100644
--- a/drivers/media/pci/saa7164/saa7164-vbi.c
+++ b/drivers/media/pci/saa7164/saa7164-vbi.c
@@ -1297,7 +1297,7 @@ static struct video_device *saa7164_vbi_alloc(
 	snprintf(vfd->name, sizeof(vfd->name), "%s %s (%s)", dev->name,
 		type, saa7164_boards[dev->board].name);
 
-	vfd->parent  = &pci->dev;
+	vfd->v4l2_dev  = &dev->v4l2_dev;
 	vfd->release = video_device_release;
 	return vfd;
 }
diff --git a/drivers/media/pci/saa7164/saa7164.h b/drivers/media/pci/saa7164/saa7164.h
index 2df47ea..8b29e89 100644
--- a/drivers/media/pci/saa7164/saa7164.h
+++ b/drivers/media/pci/saa7164/saa7164.h
@@ -63,6 +63,7 @@
 #include <dmxdev.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-device.h>
 
 #include "saa7164-reg.h"
 #include "saa7164-types.h"
@@ -427,6 +428,8 @@ struct saa7164_dev {
 	struct list_head	devlist;
 	atomic_t		refcount;
 
+	struct v4l2_device v4l2_dev;
+
 	/* pci stuff */
 	struct pci_dev	*pci;
 	unsigned char	pci_rev, pci_lat;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [REVIEW PATCH 6/9] omap24xxcam: use v4l2_dev instead of the deprecated parent field.
  2013-06-10 12:48 [REVIEW PATCH 0/9] Use v4l2_dev instead of parent Hans Verkuil
                   ` (4 preceding siblings ...)
  2013-06-10 12:48 ` [REVIEW PATCH 5/9] saa7164: " Hans Verkuil
@ 2013-06-10 12:48 ` Hans Verkuil
  2013-06-11 11:42   ` Sakari Ailus
  2013-06-10 12:48 ` [REVIEW PATCH 7/9] pvrusb2: " Hans Verkuil
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Hans Verkuil @ 2013-06-10 12:48 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Mike Isely, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/platform/omap24xxcam.c |    9 ++++++++-
 drivers/media/platform/omap24xxcam.h |    3 +++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/omap24xxcam.c b/drivers/media/platform/omap24xxcam.c
index debb44c..d2b440c 100644
--- a/drivers/media/platform/omap24xxcam.c
+++ b/drivers/media/platform/omap24xxcam.c
@@ -1656,7 +1656,7 @@ static int omap24xxcam_device_register(struct v4l2_int_device *s)
 	}
 	vfd->release = video_device_release;
 
-	vfd->parent = cam->dev;
+	vfd->v4l2_dev = &cam->v4l2_dev;
 
 	strlcpy(vfd->name, CAM_NAME, sizeof(vfd->name));
 	vfd->fops		 = &omap24xxcam_fops;
@@ -1752,6 +1752,11 @@ static int omap24xxcam_probe(struct platform_device *pdev)
 
 	cam->dev = &pdev->dev;
 
+	if (v4l2_device_register(&pdev->dev, &cam->v4l2_dev)) {
+		dev_err(&pdev->dev, "v4l2_device_register failed\n");
+		goto err;
+	}
+
 	/*
 	 * Impose a lower limit on the amount of memory allocated for
 	 * capture. We require at least enough memory to double-buffer
@@ -1849,6 +1854,8 @@ static int omap24xxcam_remove(struct platform_device *pdev)
 		cam->mmio_base_phys = 0;
 	}
 
+	v4l2_device_unregister(&cam->v4l2_dev);
+
 	kfree(cam);
 
 	return 0;
diff --git a/drivers/media/platform/omap24xxcam.h b/drivers/media/platform/omap24xxcam.h
index c439595..7f6f791 100644
--- a/drivers/media/platform/omap24xxcam.h
+++ b/drivers/media/platform/omap24xxcam.h
@@ -29,6 +29,7 @@
 
 #include <media/videobuf-dma-sg.h>
 #include <media/v4l2-int-device.h>
+#include <media/v4l2-device.h>
 
 /*
  *
@@ -462,6 +463,8 @@ struct omap24xxcam_device {
 	 */
 	struct mutex mutex;
 
+	struct v4l2_device v4l2_dev;
+
 	/*** general driver state information ***/
 	atomic_t users;
 	/*
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [REVIEW PATCH 7/9] pvrusb2: use v4l2_dev instead of the deprecated parent field.
  2013-06-10 12:48 [REVIEW PATCH 0/9] Use v4l2_dev instead of parent Hans Verkuil
                   ` (5 preceding siblings ...)
  2013-06-10 12:48 ` [REVIEW PATCH 6/9] omap24xxcam: " Hans Verkuil
@ 2013-06-10 12:48 ` Hans Verkuil
  2013-06-10 12:48 ` [REVIEW PATCH 8/9] f_uvc: " Hans Verkuil
  2013-06-10 12:48 ` [REVIEW PATCH 9/9] v4l2: remove parent from v4l2 core Hans Verkuil
  8 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2013-06-10 12:48 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Mike Isely, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/pvrusb2/pvrusb2-hdw.c  |    4 ++++
 drivers/media/usb/pvrusb2/pvrusb2-hdw.h  |    4 ++++
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c |    7 ++++---
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
index d329209..c4d51d7 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
@@ -2704,6 +2704,10 @@ static void pvr2_hdw_remove_usb_stuff(struct pvr2_hdw *hdw)
 	pvr2_hdw_render_useless(hdw);
 }
 
+void pvr2_hdw_set_v4l2_dev(struct pvr2_hdw *hdw, struct video_device *vdev)
+{
+	vdev->v4l2_dev = &hdw->v4l2_dev;
+}
 
 /* Destroy hardware interaction structure */
 void pvr2_hdw_destroy(struct pvr2_hdw *hdw)
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.h b/drivers/media/usb/pvrusb2/pvrusb2-hdw.h
index 1a135cf..4184707 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.h
+++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.h
@@ -22,6 +22,7 @@
 
 #include <linux/usb.h>
 #include <linux/videodev2.h>
+#include <media/v4l2-dev.h>
 #include "pvrusb2-io.h"
 #include "pvrusb2-ctrl.h"
 
@@ -138,6 +139,9 @@ const char *pvr2_hdw_get_device_identifier(struct pvr2_hdw *);
 /* Called when hardware has been unplugged */
 void pvr2_hdw_disconnect(struct pvr2_hdw *);
 
+/* Sets v4l2_dev of a video_device struct */
+void pvr2_hdw_set_v4l2_dev(struct pvr2_hdw *, struct video_device *);
+
 /* Get the number of defined controls */
 unsigned int pvr2_hdw_get_ctrl_count(struct pvr2_hdw *);
 
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
index 82f619b..d77069e 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
@@ -31,6 +31,7 @@
 #include <linux/videodev2.h>
 #include <linux/module.h>
 #include <media/v4l2-dev.h>
+#include <media/v4l2-device.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
 
@@ -870,8 +871,8 @@ static void pvr2_v4l2_dev_destroy(struct pvr2_v4l2_dev *dip)
 static void pvr2_v4l2_dev_disassociate_parent(struct pvr2_v4l2_dev *dip)
 {
 	if (!dip) return;
-	if (!dip->devbase.parent) return;
-	dip->devbase.parent = NULL;
+	if (!dip->devbase.v4l2_dev->dev) return;
+	dip->devbase.v4l2_dev->dev = NULL;
 	device_move(&dip->devbase.dev, NULL, DPM_ORDER_NONE);
 }
 
@@ -1321,7 +1322,7 @@ static void pvr2_v4l2_dev_init(struct pvr2_v4l2_dev *dip,
 	if (nr_ptr && (unit_number >= 0) && (unit_number < PVR_NUM)) {
 		mindevnum = nr_ptr[unit_number];
 	}
-	dip->devbase.parent = &usbdev->dev;
+	pvr2_hdw_set_v4l2_dev(hdw, &dip->devbase);
 	if ((video_register_device(&dip->devbase,
 				   dip->v4l_type, mindevnum) < 0) &&
 	    (video_register_device(&dip->devbase,
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [REVIEW PATCH 8/9] f_uvc: use v4l2_dev instead of the deprecated parent field.
  2013-06-10 12:48 [REVIEW PATCH 0/9] Use v4l2_dev instead of parent Hans Verkuil
                   ` (6 preceding siblings ...)
  2013-06-10 12:48 ` [REVIEW PATCH 7/9] pvrusb2: " Hans Verkuil
@ 2013-06-10 12:48 ` Hans Verkuil
  2013-06-10 18:50   ` Laurent Pinchart
  2013-06-10 12:48 ` [REVIEW PATCH 9/9] v4l2: remove parent from v4l2 core Hans Verkuil
  8 siblings, 1 reply; 17+ messages in thread
From: Hans Verkuil @ 2013-06-10 12:48 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Mike Isely, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/usb/gadget/f_uvc.c |    8 +++++++-
 drivers/usb/gadget/uvc.h   |    2 ++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
index 38dcedd..762e82f 100644
--- a/drivers/usb/gadget/f_uvc.c
+++ b/drivers/usb/gadget/f_uvc.c
@@ -413,7 +413,7 @@ uvc_register_video(struct uvc_device *uvc)
 	if (video == NULL)
 		return -ENOMEM;
 
-	video->parent = &cdev->gadget->dev;
+	video->v4l2_dev = &uvc->v4l2_dev;
 	video->fops = &uvc_v4l2_fops;
 	video->release = video_device_release;
 	strlcpy(video->name, cdev->gadget->name, sizeof(video->name));
@@ -570,6 +570,7 @@ uvc_function_unbind(struct usb_configuration *c, struct usb_function *f)
 	INFO(cdev, "uvc_function_unbind\n");
 
 	video_unregister_device(uvc->vdev);
+	v4l2_device_unregister(&uvc->v4l2_dev);
 	uvc->control_ep->driver_data = NULL;
 	uvc->video.ep->driver_data = NULL;
 
@@ -697,6 +698,11 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	if ((ret = usb_function_deactivate(f)) < 0)
 		goto error;
 
+	if (v4l2_device_register(&cdev->gadget->dev, &uvc->v4l2_dev)) {
+		printk(KERN_INFO "v4l2_device_register failed\n");
+		goto error;
+	}
+
 	/* Initialise video. */
 	ret = uvc_video_init(&uvc->video);
 	if (ret < 0)
diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h
index 817e9e1..7a9111d 100644
--- a/drivers/usb/gadget/uvc.h
+++ b/drivers/usb/gadget/uvc.h
@@ -57,6 +57,7 @@ struct uvc_event
 #include <linux/videodev2.h>
 #include <linux/version.h>
 #include <media/v4l2-fh.h>
+#include <media/v4l2-device.h>
 
 #include "uvc_queue.h"
 
@@ -145,6 +146,7 @@ enum uvc_state
 struct uvc_device
 {
 	struct video_device *vdev;
+	struct v4l2_device v4l2_dev;
 	enum uvc_state state;
 	struct usb_function func;
 	struct uvc_video video;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [REVIEW PATCH 9/9] v4l2: remove parent from v4l2 core.
  2013-06-10 12:48 [REVIEW PATCH 0/9] Use v4l2_dev instead of parent Hans Verkuil
                   ` (7 preceding siblings ...)
  2013-06-10 12:48 ` [REVIEW PATCH 8/9] f_uvc: " Hans Verkuil
@ 2013-06-10 12:48 ` Hans Verkuil
  2013-06-10 18:51   ` Laurent Pinchart
                     ` (2 more replies)
  8 siblings, 3 replies; 17+ messages in thread
From: Hans Verkuil @ 2013-06-10 12:48 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Mike Isely, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/v4l2-dev.c   |   34 +++++++++++++++-------------------
 drivers/media/v4l2-core/v4l2-ioctl.c |    7 +------
 include/media/v4l2-dev.h             |    2 --
 3 files changed, 16 insertions(+), 27 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 2f3fac5..61e82f8 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -495,8 +495,8 @@ static const struct file_operations v4l2_fops = {
 };
 
 /**
- * get_index - assign stream index number based on parent device
- * @vdev: video_device to assign index number to, vdev->parent should be assigned
+ * get_index - assign stream index number based on v4l2_dev
+ * @vdev: video_device to assign index number to, vdev->v4l2_dev should be assigned
  *
  * Note that when this is called the new device has not yet been registered
  * in the video_device array, but it was able to obtain a minor number.
@@ -514,15 +514,11 @@ static int get_index(struct video_device *vdev)
 	static DECLARE_BITMAP(used, VIDEO_NUM_DEVICES);
 	int i;
 
-	/* Some drivers do not set the parent. In that case always return 0. */
-	if (vdev->parent == NULL)
-		return 0;
-
 	bitmap_zero(used, VIDEO_NUM_DEVICES);
 
 	for (i = 0; i < VIDEO_NUM_DEVICES; i++) {
 		if (video_device[i] != NULL &&
-		    video_device[i]->parent == vdev->parent) {
+		    video_device[i]->v4l2_dev == vdev->v4l2_dev) {
 			set_bit(video_device[i]->index, used);
 		}
 	}
@@ -776,6 +772,9 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
 	/* the release callback MUST be present */
 	if (WARN_ON(!vdev->release))
 		return -EINVAL;
+	/* the v4l2_dev pointer MUST be present */
+	if (WARN_ON(!vdev->v4l2_dev))
+		return -EINVAL;
 
 	/* v4l2_fh support */
 	spin_lock_init(&vdev->fh_lock);
@@ -803,16 +802,13 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
 
 	vdev->vfl_type = type;
 	vdev->cdev = NULL;
-	if (vdev->v4l2_dev) {
-		if (vdev->v4l2_dev->dev)
-			vdev->parent = vdev->v4l2_dev->dev;
-		if (vdev->ctrl_handler == NULL)
-			vdev->ctrl_handler = vdev->v4l2_dev->ctrl_handler;
-		/* If the prio state pointer is NULL, then use the v4l2_device
-		   prio state. */
-		if (vdev->prio == NULL)
-			vdev->prio = &vdev->v4l2_dev->prio;
-	}
+
+	if (vdev->ctrl_handler == NULL)
+		vdev->ctrl_handler = vdev->v4l2_dev->ctrl_handler;
+	/* If the prio state pointer is NULL, then use the v4l2_device
+	   prio state. */
+	if (vdev->prio == NULL)
+		vdev->prio = &vdev->v4l2_dev->prio;
 
 	/* Part 2: find a free minor, device node number and device index. */
 #ifdef CONFIG_VIDEO_FIXED_MINOR_RANGES
@@ -897,8 +893,8 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
 	/* Part 4: register the device with sysfs */
 	vdev->dev.class = &video_class;
 	vdev->dev.devt = MKDEV(VIDEO_MAJOR, vdev->minor);
-	if (vdev->parent)
-		vdev->dev.parent = vdev->parent;
+	if (vdev->v4l2_dev->dev)
+		vdev->dev.parent = vdev->v4l2_dev->dev;
 	dev_set_name(&vdev->dev, "%s%d", name_base, vdev->num);
 	ret = device_register(&vdev->dev);
 	if (ret < 0) {
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 19e2988..3dcdaa3 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1845,12 +1845,7 @@ static int v4l_dbg_g_chip_info(const struct v4l2_ioctl_ops *ops,
 			p->flags |= V4L2_CHIP_FL_WRITABLE;
 		if (ops->vidioc_g_register)
 			p->flags |= V4L2_CHIP_FL_READABLE;
-		if (vfd->v4l2_dev)
-			strlcpy(p->name, vfd->v4l2_dev->name, sizeof(p->name));
-		else if (vfd->parent)
-			strlcpy(p->name, vfd->parent->driver->name, sizeof(p->name));
-		else
-			strlcpy(p->name, "bridge", sizeof(p->name));
+		strlcpy(p->name, vfd->v4l2_dev->name, sizeof(p->name));
 		if (ops->vidioc_g_chip_info)
 			return ops->vidioc_g_chip_info(file, fh, arg);
 		if (p->match.addr)
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index b2c3776..4d10e66 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -96,8 +96,6 @@ struct video_device
 	struct device dev;		/* v4l device */
 	struct cdev *cdev;		/* character device */
 
-	/* Set either parent or v4l2_dev if your driver uses v4l2_device */
-	struct device *parent;		/* device parent */
 	struct v4l2_device *v4l2_dev;	/* v4l2_device parent */
 
 	/* Control handler associated with this device node. May be NULL. */
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [REVIEW PATCH 8/9] f_uvc: use v4l2_dev instead of the deprecated parent field.
  2013-06-10 12:48 ` [REVIEW PATCH 8/9] f_uvc: " Hans Verkuil
@ 2013-06-10 18:50   ` Laurent Pinchart
  2013-06-11 12:13     ` Hans Verkuil
  0 siblings, 1 reply; 17+ messages in thread
From: Laurent Pinchart @ 2013-06-10 18:50 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Guennadi Liakhovetski, Mike Isely, Hans Verkuil

Hi Hans,

Thanks for the patch.

On Monday 10 June 2013 14:48:37 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/usb/gadget/f_uvc.c |    8 +++++++-
>  drivers/usb/gadget/uvc.h   |    2 ++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
> index 38dcedd..762e82f 100644
> --- a/drivers/usb/gadget/f_uvc.c
> +++ b/drivers/usb/gadget/f_uvc.c
> @@ -413,7 +413,7 @@ uvc_register_video(struct uvc_device *uvc)
>  	if (video == NULL)
>  		return -ENOMEM;
> 
> -	video->parent = &cdev->gadget->dev;
> +	video->v4l2_dev = &uvc->v4l2_dev;
>  	video->fops = &uvc_v4l2_fops;
>  	video->release = video_device_release;
>  	strlcpy(video->name, cdev->gadget->name, sizeof(video->name));
> @@ -570,6 +570,7 @@ uvc_function_unbind(struct usb_configuration *c, struct
> usb_function *f) INFO(cdev, "uvc_function_unbind\n");
> 
>  	video_unregister_device(uvc->vdev);
> +	v4l2_device_unregister(&uvc->v4l2_dev);
>  	uvc->control_ep->driver_data = NULL;
>  	uvc->video.ep->driver_data = NULL;
> 
> @@ -697,6 +698,11 @@ uvc_function_bind(struct usb_configuration *c, struct
> usb_function *f) if ((ret = usb_function_deactivate(f)) < 0)
>  		goto error;
> 
> +	if (v4l2_device_register(&cdev->gadget->dev, &uvc->v4l2_dev)) {
> +		printk(KERN_INFO "v4l2_device_register failed\n");
> +		goto error;
> +	}
> +
>  	/* Initialise video. */
>  	ret = uvc_video_init(&uvc->video);
>  	if (ret < 0)

Shouldn't you add the corresponding cleanup code in the error path at the end 
of the function ?

> diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h
> index 817e9e1..7a9111d 100644
> --- a/drivers/usb/gadget/uvc.h
> +++ b/drivers/usb/gadget/uvc.h
> @@ -57,6 +57,7 @@ struct uvc_event
>  #include <linux/videodev2.h>
>  #include <linux/version.h>
>  #include <media/v4l2-fh.h>
> +#include <media/v4l2-device.h>
> 
>  #include "uvc_queue.h"
> 
> @@ -145,6 +146,7 @@ enum uvc_state
>  struct uvc_device
>  {
>  	struct video_device *vdev;
> +	struct v4l2_device v4l2_dev;
>  	enum uvc_state state;
>  	struct usb_function func;
>  	struct uvc_video video;
-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [REVIEW PATCH 9/9] v4l2: remove parent from v4l2 core.
  2013-06-10 12:48 ` [REVIEW PATCH 9/9] v4l2: remove parent from v4l2 core Hans Verkuil
@ 2013-06-10 18:51   ` Laurent Pinchart
  2013-06-11 11:44   ` Sakari Ailus
  2013-06-11 16:53   ` Ezequiel Garcia
  2 siblings, 0 replies; 17+ messages in thread
From: Laurent Pinchart @ 2013-06-10 18:51 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Guennadi Liakhovetski, Mike Isely, Hans Verkuil

Hi Hans,

Thanks for the patch.

On Monday 10 June 2013 14:48:38 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-dev.c   |   34  ++++++++++++++----------------
>  drivers/media/v4l2-core/v4l2-ioctl.c |    7 +------
>  include/media/v4l2-dev.h             |    2 --
>  3 files changed, 16 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c
> b/drivers/media/v4l2-core/v4l2-dev.c index 2f3fac5..61e82f8 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -495,8 +495,8 @@ static const struct file_operations v4l2_fops = {
>  };
> 
>  /**
> - * get_index - assign stream index number based on parent device
> - * @vdev: video_device to assign index number to, vdev->parent should be
> assigned + * get_index - assign stream index number based on v4l2_dev
> + * @vdev: video_device to assign index number to, vdev->v4l2_dev should be
> assigned *
>   * Note that when this is called the new device has not yet been registered
> * in the video_device array, but it was able to obtain a minor number. @@
> -514,15 +514,11 @@ static int get_index(struct video_device *vdev) static
> DECLARE_BITMAP(used, VIDEO_NUM_DEVICES);
>  	int i;
> 
> -	/* Some drivers do not set the parent. In that case always return 0. */
> -	if (vdev->parent == NULL)
> -		return 0;
> -
>  	bitmap_zero(used, VIDEO_NUM_DEVICES);
> 
>  	for (i = 0; i < VIDEO_NUM_DEVICES; i++) {
>  		if (video_device[i] != NULL &&
> -		    video_device[i]->parent == vdev->parent) {
> +		    video_device[i]->v4l2_dev == vdev->v4l2_dev) {
>  			set_bit(video_device[i]->index, used);
>  		}
>  	}
> @@ -776,6 +772,9 @@ int __video_register_device(struct video_device *vdev,
> int type, int nr, /* the release callback MUST be present */
>  	if (WARN_ON(!vdev->release))
>  		return -EINVAL;
> +	/* the v4l2_dev pointer MUST be present */
> +	if (WARN_ON(!vdev->v4l2_dev))
> +		return -EINVAL;
> 
>  	/* v4l2_fh support */
>  	spin_lock_init(&vdev->fh_lock);
> @@ -803,16 +802,13 @@ int __video_register_device(struct video_device *vdev,
> int type, int nr,
> 
>  	vdev->vfl_type = type;
>  	vdev->cdev = NULL;
> -	if (vdev->v4l2_dev) {
> -		if (vdev->v4l2_dev->dev)
> -			vdev->parent = vdev->v4l2_dev->dev;
> -		if (vdev->ctrl_handler == NULL)
> -			vdev->ctrl_handler = vdev->v4l2_dev->ctrl_handler;
> -		/* If the prio state pointer is NULL, then use the v4l2_device
> -		   prio state. */
> -		if (vdev->prio == NULL)
> -			vdev->prio = &vdev->v4l2_dev->prio;
> -	}
> +
> +	if (vdev->ctrl_handler == NULL)
> +		vdev->ctrl_handler = vdev->v4l2_dev->ctrl_handler;
> +	/* If the prio state pointer is NULL, then use the v4l2_device
> +	   prio state. */
> +	if (vdev->prio == NULL)
> +		vdev->prio = &vdev->v4l2_dev->prio;
> 
>  	/* Part 2: find a free minor, device node number and device index. */
>  #ifdef CONFIG_VIDEO_FIXED_MINOR_RANGES
> @@ -897,8 +893,8 @@ int __video_register_device(struct video_device *vdev,
> int type, int nr, /* Part 4: register the device with sysfs */
>  	vdev->dev.class = &video_class;
>  	vdev->dev.devt = MKDEV(VIDEO_MAJOR, vdev->minor);
> -	if (vdev->parent)
> -		vdev->dev.parent = vdev->parent;
> +	if (vdev->v4l2_dev->dev)
> +		vdev->dev.parent = vdev->v4l2_dev->dev;
>  	dev_set_name(&vdev->dev, "%s%d", name_base, vdev->num);
>  	ret = device_register(&vdev->dev);
>  	if (ret < 0) {
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c index 19e2988..3dcdaa3 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1845,12 +1845,7 @@ static int v4l_dbg_g_chip_info(const struct
> v4l2_ioctl_ops *ops, p->flags |= V4L2_CHIP_FL_WRITABLE;
>  		if (ops->vidioc_g_register)
>  			p->flags |= V4L2_CHIP_FL_READABLE;
> -		if (vfd->v4l2_dev)
> -			strlcpy(p->name, vfd->v4l2_dev->name, sizeof(p->name));
> -		else if (vfd->parent)
> -			strlcpy(p->name, vfd->parent->driver->name, sizeof(p->name));
> -		else
> -			strlcpy(p->name, "bridge", sizeof(p->name));
> +		strlcpy(p->name, vfd->v4l2_dev->name, sizeof(p->name));
>  		if (ops->vidioc_g_chip_info)
>  			return ops->vidioc_g_chip_info(file, fh, arg);
>  		if (p->match.addr)
> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
> index b2c3776..4d10e66 100644
> --- a/include/media/v4l2-dev.h
> +++ b/include/media/v4l2-dev.h
> @@ -96,8 +96,6 @@ struct video_device
>  	struct device dev;		/* v4l device */
>  	struct cdev *cdev;		/* character device */
> 
> -	/* Set either parent or v4l2_dev if your driver uses v4l2_device */
> -	struct device *parent;		/* device parent */
>  	struct v4l2_device *v4l2_dev;	/* v4l2_device parent */
> 
>  	/* Control handler associated with this device node. May be NULL. */
-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [REVIEW PATCH 6/9] omap24xxcam: use v4l2_dev instead of the deprecated parent field.
  2013-06-10 12:48 ` [REVIEW PATCH 6/9] omap24xxcam: " Hans Verkuil
@ 2013-06-11 11:42   ` Sakari Ailus
  0 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2013-06-11 11:42 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, Laurent Pinchart, Guennadi Liakhovetski, Mike Isely,
	Hans Verkuil

On Mon, Jun 10, 2013 at 02:48:35PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Thanks!

Acked-by: Sakari Ailus <sakari.ailus@iki.fi>

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [REVIEW PATCH 9/9] v4l2: remove parent from v4l2 core.
  2013-06-10 12:48 ` [REVIEW PATCH 9/9] v4l2: remove parent from v4l2 core Hans Verkuil
  2013-06-10 18:51   ` Laurent Pinchart
@ 2013-06-11 11:44   ` Sakari Ailus
  2013-06-11 16:53   ` Ezequiel Garcia
  2 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2013-06-11 11:44 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, Laurent Pinchart, Guennadi Liakhovetski, Mike Isely,
	Hans Verkuil

On Mon, Jun 10, 2013 at 02:48:38PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Sakari Ailus <sakari.ailus@iki.fi>

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [REVIEW PATCH 8/9] f_uvc: use v4l2_dev instead of the deprecated parent field.
  2013-06-10 18:50   ` Laurent Pinchart
@ 2013-06-11 12:13     ` Hans Verkuil
  2013-06-11 12:25       ` Laurent Pinchart
  0 siblings, 1 reply; 17+ messages in thread
From: Hans Verkuil @ 2013-06-11 12:13 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, Guennadi Liakhovetski, Mike Isely, Hans Verkuil

On Mon 10 June 2013 20:50:42 Laurent Pinchart wrote:
> Hi Hans,
> 
> Thanks for the patch.
> 
> On Monday 10 June 2013 14:48:37 Hans Verkuil wrote:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > ---
> >  drivers/usb/gadget/f_uvc.c |    8 +++++++-
> >  drivers/usb/gadget/uvc.h   |    2 ++
> >  2 files changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
> > index 38dcedd..762e82f 100644
> > --- a/drivers/usb/gadget/f_uvc.c
> > +++ b/drivers/usb/gadget/f_uvc.c
> > @@ -413,7 +413,7 @@ uvc_register_video(struct uvc_device *uvc)
> >  	if (video == NULL)
> >  		return -ENOMEM;
> > 
> > -	video->parent = &cdev->gadget->dev;
> > +	video->v4l2_dev = &uvc->v4l2_dev;
> >  	video->fops = &uvc_v4l2_fops;
> >  	video->release = video_device_release;
> >  	strlcpy(video->name, cdev->gadget->name, sizeof(video->name));
> > @@ -570,6 +570,7 @@ uvc_function_unbind(struct usb_configuration *c, struct
> > usb_function *f) INFO(cdev, "uvc_function_unbind\n");
> > 
> >  	video_unregister_device(uvc->vdev);
> > +	v4l2_device_unregister(&uvc->v4l2_dev);
> >  	uvc->control_ep->driver_data = NULL;
> >  	uvc->video.ep->driver_data = NULL;
> > 
> > @@ -697,6 +698,11 @@ uvc_function_bind(struct usb_configuration *c, struct
> > usb_function *f) if ((ret = usb_function_deactivate(f)) < 0)
> >  		goto error;
> > 
> > +	if (v4l2_device_register(&cdev->gadget->dev, &uvc->v4l2_dev)) {
> > +		printk(KERN_INFO "v4l2_device_register failed\n");
> > +		goto error;
> > +	}
> > +
> >  	/* Initialise video. */
> >  	ret = uvc_video_init(&uvc->video);
> >  	if (ret < 0)
> 
> Shouldn't you add the corresponding cleanup code in the error path at the end 
> of the function ?

Not really necessary as long as there are no subdevices registered. Still, it
is probably safer to do it anyway.

Regards,

	Hans

> 
> > diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h
> > index 817e9e1..7a9111d 100644
> > --- a/drivers/usb/gadget/uvc.h
> > +++ b/drivers/usb/gadget/uvc.h
> > @@ -57,6 +57,7 @@ struct uvc_event
> >  #include <linux/videodev2.h>
> >  #include <linux/version.h>
> >  #include <media/v4l2-fh.h>
> > +#include <media/v4l2-device.h>
> > 
> >  #include "uvc_queue.h"
> > 
> > @@ -145,6 +146,7 @@ enum uvc_state
> >  struct uvc_device
> >  {
> >  	struct video_device *vdev;
> > +	struct v4l2_device v4l2_dev;
> >  	enum uvc_state state;
> >  	struct usb_function func;
> >  	struct uvc_video video;
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [REVIEW PATCH 8/9] f_uvc: use v4l2_dev instead of the deprecated parent field.
  2013-06-11 12:13     ` Hans Verkuil
@ 2013-06-11 12:25       ` Laurent Pinchart
  0 siblings, 0 replies; 17+ messages in thread
From: Laurent Pinchart @ 2013-06-11 12:25 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Guennadi Liakhovetski, Mike Isely, Hans Verkuil

Hi Hans,

On Tuesday 11 June 2013 14:13:33 Hans Verkuil wrote:
> On Mon 10 June 2013 20:50:42 Laurent Pinchart wrote:
> > On Monday 10 June 2013 14:48:37 Hans Verkuil wrote:
> > > From: Hans Verkuil <hans.verkuil@cisco.com>
> > > 
> > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > > ---
> > > 
> > >  drivers/usb/gadget/f_uvc.c |    8 +++++++-
> > >  drivers/usb/gadget/uvc.h   |    2 ++
> > >  2 files changed, 9 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
> > > index 38dcedd..762e82f 100644
> > > --- a/drivers/usb/gadget/f_uvc.c
> > > +++ b/drivers/usb/gadget/f_uvc.c
> > > @@ -413,7 +413,7 @@ uvc_register_video(struct uvc_device *uvc)
> > > 
> > >  	if (video == NULL)
> > >  	
> > >  		return -ENOMEM;
> > > 
> > > -	video->parent = &cdev->gadget->dev;
> > > +	video->v4l2_dev = &uvc->v4l2_dev;
> > > 
> > >  	video->fops = &uvc_v4l2_fops;
> > >  	video->release = video_device_release;
> > >  	strlcpy(video->name, cdev->gadget->name, sizeof(video->name));
> > > 
> > > @@ -570,6 +570,7 @@ uvc_function_unbind(struct usb_configuration *c,
> > > struct
> > > usb_function *f) INFO(cdev, "uvc_function_unbind\n");
> > > 
> > >  	video_unregister_device(uvc->vdev);
> > > 
> > > +	v4l2_device_unregister(&uvc->v4l2_dev);
> > > 
> > >  	uvc->control_ep->driver_data = NULL;
> > >  	uvc->video.ep->driver_data = NULL;
> > > 
> > > @@ -697,6 +698,11 @@ uvc_function_bind(struct usb_configuration *c,
> > > struct
> > > usb_function *f) if ((ret = usb_function_deactivate(f)) < 0)
> > > 
> > >  		goto error;
> > > 
> > > +	if (v4l2_device_register(&cdev->gadget->dev, &uvc->v4l2_dev)) {
> > > +		printk(KERN_INFO "v4l2_device_register failed\n");
> > > +		goto error;
> > > +	}
> > > +
> > > 
> > >  	/* Initialise video. */
> > >  	ret = uvc_video_init(&uvc->video);
> > >  	if (ret < 0)
> > 
> > Shouldn't you add the corresponding cleanup code in the error path at the
> > end of the function ?
> 
> Not really necessary as long as there are no subdevices registered. Still,
> it is probably safer to do it anyway.

v4l2_device_unregister() calls v4l2_device_disconnect(), which in turn calls 
put_device() on the underlying device. Even if no subdev is registered I think 
that's required.

> > > diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h
> > > index 817e9e1..7a9111d 100644
> > > --- a/drivers/usb/gadget/uvc.h
> > > +++ b/drivers/usb/gadget/uvc.h
> > > @@ -57,6 +57,7 @@ struct uvc_event
> > > 
> > >  #include <linux/videodev2.h>
> > >  #include <linux/version.h>
> > >  #include <media/v4l2-fh.h>
> > > 
> > > +#include <media/v4l2-device.h>
> > > 
> > >  #include "uvc_queue.h"
> > > 
> > > @@ -145,6 +146,7 @@ enum uvc_state
> > > 
> > >  struct uvc_device
> > >  {
> > >  
> > >  	struct video_device *vdev;
> > > 
> > > +	struct v4l2_device v4l2_dev;
> > > 
> > >  	enum uvc_state state;
> > >  	struct usb_function func;
> > >  	struct uvc_video video;
-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [REVIEW PATCH 9/9] v4l2: remove parent from v4l2 core.
  2013-06-10 12:48 ` [REVIEW PATCH 9/9] v4l2: remove parent from v4l2 core Hans Verkuil
  2013-06-10 18:51   ` Laurent Pinchart
  2013-06-11 11:44   ` Sakari Ailus
@ 2013-06-11 16:53   ` Ezequiel Garcia
  2 siblings, 0 replies; 17+ messages in thread
From: Ezequiel Garcia @ 2013-06-11 16:53 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, Laurent Pinchart, Guennadi Liakhovetski, Mike Isely,
	Hans Verkuil

Hi Hans,

On Mon, Jun 10, 2013 at 9:48 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/v4l2-core/v4l2-dev.c   |   34 +++++++++++++++-------------------
>  drivers/media/v4l2-core/v4l2-ioctl.c |    7 +------
>  include/media/v4l2-dev.h             |    2 --
>  3 files changed, 16 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 2f3fac5..61e82f8 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -495,8 +495,8 @@ static const struct file_operations v4l2_fops = {
>  };
>
>  /**
> - * get_index - assign stream index number based on parent device
> - * @vdev: video_device to assign index number to, vdev->parent should be assigned
> + * get_index - assign stream index number based on v4l2_dev
> + * @vdev: video_device to assign index number to, vdev->v4l2_dev should be assigned
>   *
>   * Note that when this is called the new device has not yet been registered
>   * in the video_device array, but it was able to obtain a minor number.
> @@ -514,15 +514,11 @@ static int get_index(struct video_device *vdev)
>         static DECLARE_BITMAP(used, VIDEO_NUM_DEVICES);
>         int i;
>
> -       /* Some drivers do not set the parent. In that case always return 0. */
> -       if (vdev->parent == NULL)
> -               return 0;
> -
>         bitmap_zero(used, VIDEO_NUM_DEVICES);
>
>         for (i = 0; i < VIDEO_NUM_DEVICES; i++) {
>                 if (video_device[i] != NULL &&
> -                   video_device[i]->parent == vdev->parent) {
> +                   video_device[i]->v4l2_dev == vdev->v4l2_dev) {
>                         set_bit(video_device[i]->index, used);
>                 }
>         }
> @@ -776,6 +772,9 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
>         /* the release callback MUST be present */
>         if (WARN_ON(!vdev->release))
>                 return -EINVAL;
> +       /* the v4l2_dev pointer MUST be present */
> +       if (WARN_ON(!vdev->v4l2_dev))
> +               return -EINVAL;
>
>         /* v4l2_fh support */
>         spin_lock_init(&vdev->fh_lock);
> @@ -803,16 +802,13 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
>
>         vdev->vfl_type = type;
>         vdev->cdev = NULL;
> -       if (vdev->v4l2_dev) {
> -               if (vdev->v4l2_dev->dev)
> -                       vdev->parent = vdev->v4l2_dev->dev;
> -               if (vdev->ctrl_handler == NULL)
> -                       vdev->ctrl_handler = vdev->v4l2_dev->ctrl_handler;
> -               /* If the prio state pointer is NULL, then use the v4l2_device
> -                  prio state. */
> -               if (vdev->prio == NULL)
> -                       vdev->prio = &vdev->v4l2_dev->prio;
> -       }
> +
> +       if (vdev->ctrl_handler == NULL)
> +               vdev->ctrl_handler = vdev->v4l2_dev->ctrl_handler;
> +       /* If the prio state pointer is NULL, then use the v4l2_device
> +          prio state. */
> +       if (vdev->prio == NULL)
> +               vdev->prio = &vdev->v4l2_dev->prio;
>
>         /* Part 2: find a free minor, device node number and device index. */
>  #ifdef CONFIG_VIDEO_FIXED_MINOR_RANGES
> @@ -897,8 +893,8 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
>         /* Part 4: register the device with sysfs */
>         vdev->dev.class = &video_class;
>         vdev->dev.devt = MKDEV(VIDEO_MAJOR, vdev->minor);
> -       if (vdev->parent)
> -               vdev->dev.parent = vdev->parent;
> +       if (vdev->v4l2_dev->dev)
> +               vdev->dev.parent = vdev->v4l2_dev->dev;
>         dev_set_name(&vdev->dev, "%s%d", name_base, vdev->num);
>         ret = device_register(&vdev->dev);
>         if (ret < 0) {
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 19e2988..3dcdaa3 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1845,12 +1845,7 @@ static int v4l_dbg_g_chip_info(const struct v4l2_ioctl_ops *ops,
>                         p->flags |= V4L2_CHIP_FL_WRITABLE;
>                 if (ops->vidioc_g_register)
>                         p->flags |= V4L2_CHIP_FL_READABLE;
> -               if (vfd->v4l2_dev)
> -                       strlcpy(p->name, vfd->v4l2_dev->name, sizeof(p->name));
> -               else if (vfd->parent)
> -                       strlcpy(p->name, vfd->parent->driver->name, sizeof(p->name));
> -               else
> -                       strlcpy(p->name, "bridge", sizeof(p->name));
> +               strlcpy(p->name, vfd->v4l2_dev->name, sizeof(p->name));
>                 if (ops->vidioc_g_chip_info)
>                         return ops->vidioc_g_chip_info(file, fh, arg);
>                 if (p->match.addr)
> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
> index b2c3776..4d10e66 100644
> --- a/include/media/v4l2-dev.h
> +++ b/include/media/v4l2-dev.h
> @@ -96,8 +96,6 @@ struct video_device
>         struct device dev;              /* v4l device */
>         struct cdev *cdev;              /* character device */
>
> -       /* Set either parent or v4l2_dev if your driver uses v4l2_device */
> -       struct device *parent;          /* device parent */
>         struct v4l2_device *v4l2_dev;   /* v4l2_device parent */
>
>         /* Control handler associated with this device node. May be NULL. */
> --
> 1.7.10.4
>

I know there's a cover letter for this changeset. However, for such an
intrusive change
as the one contained in this patch I think that not having any
description/changelog
for the commit might not be a good idea.

Thanks,
-- 
    Ezequiel

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-06-11 16:53 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-10 12:48 [REVIEW PATCH 0/9] Use v4l2_dev instead of parent Hans Verkuil
2013-06-10 12:48 ` [REVIEW PATCH 1/9] soc_camera: replace vdev->parent by vdev->v4l2_dev Hans Verkuil
2013-06-10 12:48 ` [REVIEW PATCH 2/9] cx23885-417: use v4l2_dev instead of the deprecated parent field Hans Verkuil
2013-06-10 12:48 ` [REVIEW PATCH 3/9] zoran: " Hans Verkuil
2013-06-10 12:48 ` [REVIEW PATCH 4/9] sn9c102: " Hans Verkuil
2013-06-10 12:48 ` [REVIEW PATCH 5/9] saa7164: " Hans Verkuil
2013-06-10 12:48 ` [REVIEW PATCH 6/9] omap24xxcam: " Hans Verkuil
2013-06-11 11:42   ` Sakari Ailus
2013-06-10 12:48 ` [REVIEW PATCH 7/9] pvrusb2: " Hans Verkuil
2013-06-10 12:48 ` [REVIEW PATCH 8/9] f_uvc: " Hans Verkuil
2013-06-10 18:50   ` Laurent Pinchart
2013-06-11 12:13     ` Hans Verkuil
2013-06-11 12:25       ` Laurent Pinchart
2013-06-10 12:48 ` [REVIEW PATCH 9/9] v4l2: remove parent from v4l2 core Hans Verkuil
2013-06-10 18:51   ` Laurent Pinchart
2013-06-11 11:44   ` Sakari Ailus
2013-06-11 16:53   ` Ezequiel Garcia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).