[REVIEWv2 PATCH 00/11] Use v4l2_dev instead of parent

[REVIEWv2 PATCH 00/11] Use v4l2_dev instead of parent

[Hans Verkuil]

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 renamed to dev_parent and used again in cx88.

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.

Regarding the cx88 change: I discovered after posting the first version of
this patch series that there is one real use case of the parent pointer: that
is if there is one v4l2_device, but there are multiple parent PCI busses.
In the case of cx88 the video1 node has a different PCI bus parent than the
other nodes. This is the only driver that behaves like that to my knowledge.

The parent pointer used to be set correctly, but it was accidentally removed
in kernel 3.6. This patch series corrects that, and now the sysfs hierarchy of
cx88 is OK again (tested with my cx88 blackbird card).

So this patch series no longer removed the parent field, it just renames it
so I can be sure the compiler catches any remaining usages of the parent field,
both for in and out-of-kernel drivers.

Regards,

        Hans

Changes since v1:

- Add check so v4l2_device_unregister will just return if it was already
  unregistered or was never registered in the first place.
- Call unregister as well in the error path of sn9c102, saa7164, f_uvc and
  omap24xxcam.
- Rename parent to dev_parent instead of removing it altogether.
- Set correct dev_parent in cx88.
- Update v4l2-framework.txt.

[REVIEWv2 PATCH 01/12] v4l2-device: check if already unregistered.

[Hans Verkuil]

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

It was possible to unregister an already unregistered v4l2_device struct.
Add a check whether that already happened and just return if that was
the case.

Also refure to register a v4l2_device if both the dev and name fields are
empty. A warning was already produced in that case, but since the name field
is now used to detect whether or not the v4l2_device was already unregistered
this particular combination should be rejected.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/v4l2-device.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
index 8ed5da2..7f3f822 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -44,7 +44,8 @@ int v4l2_device_register(struct device *dev, struct v4l2_device *v4l2_dev)
 	v4l2_dev->dev = dev;
 	if (dev == NULL) {
 		/* If dev == NULL, then name must be filled in by the caller */
-		WARN_ON(!v4l2_dev->name[0]);
+		if (WARN_ON(!v4l2_dev->name[0]))
+			return -EINVAL;
 		return 0;
 	}
 
@@ -105,7 +106,9 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev)
 {
 	struct v4l2_subdev *sd, *next;
 
-	if (v4l2_dev == NULL)
+	/* Just return if v4l2_dev is NULL or if it was already
+	 * unregistered before. */
+	if (v4l2_dev == NULL || !v4l2_dev->name[0])
 		return;
 	v4l2_device_disconnect(v4l2_dev);
 
@@ -135,6 +138,8 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev)
 		}
 #endif
 	}
+	/* Mark as unregistered, thus preventing duplicate unregistrations */
+	v4l2_dev->name[0] = '\0';
 }
 EXPORT_SYMBOL_GPL(v4l2_device_unregister);
 
-- 
1.7.10.4

[REVIEWv2 PATCH 02/12] soc_camera: replace vdev->parent by vdev->v4l2_dev.

[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

[REVIEWv2 PATCH 03/12] cx23885-417: use v4l2_dev instead of the deprecated parent field.

[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

[REVIEWv2 PATCH 04/12] zoran: use v4l2_dev instead of the deprecated parent field.

[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

[REVIEWv2 PATCH 05/12] sn9c102_core: add v4l2_device and replace parent with v4l2_dev

[Hans Verkuil]

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

This driver did not yet support struct v4l2_device, so add it. This
make it possible to replace the deprecated parent field with the
v4l2_dev field, allowing the eventual removal of the parent field.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/sn9c102/sn9c102.h      |    3 +++
 drivers/media/usb/sn9c102/sn9c102_core.c |   13 ++++++++++++-
 2 files changed, 15 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..2cb44de 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);
 
@@ -3377,6 +3385,7 @@ fail:
 		kfree(cam->control_buffer);
 		if (cam->v4ldev)
 			video_device_release(cam->v4ldev);
+		v4l2_device_unregister(&cam->v4l2_dev);
 		kfree(cam);
 	}
 	return err;
@@ -3407,6 +3416,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

[REVIEWv2 PATCH 06/12] saa7164: add v4l2_device and replace parent with v4l2_dev

[Hans Verkuil]

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

This driver did not yet support struct v4l2_device, so add it. This
make it possible to replace the deprecated parent field with the
v4l2_dev field, allowing the eventual removal of the parent field.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/saa7164/saa7164-core.c    |    7 +++++++
 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, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/saa7164/saa7164-core.c b/drivers/media/pci/saa7164/saa7164-core.c
index 7618fda..5d27865 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)) {
@@ -1367,6 +1372,7 @@ fail_fw:
 fail_irq:
 	saa7164_dev_unregister(dev);
 fail_free:
+	v4l2_device_unregister(&dev->v4l2_dev);
 	kfree(dev);
 	return err;
 }
@@ -1439,6 +1445,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

[REVIEWv2 PATCH 07/12] pvrusb2: use v4l2_dev instead of the deprecated parent field.

[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

[REVIEWv2 PATCH 08/12] f_uvc: add v4l2_device and replace parent with v4l2_dev

[Hans Verkuil]

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

This driver did not yet support struct v4l2_device, so add it. This
make it possible to replace the deprecated parent field with the
v4l2_dev field, allowing the eventual removal of the parent field.

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

diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
index 38dcedd..1d06567 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)
@@ -712,6 +718,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	return 0;
 
 error:
+	v4l2_device_unregister(&uvc->v4l2_dev);
 	if (uvc->vdev)
 		video_device_release(uvc->vdev);
 
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

[REVIEWv2 PATCH 09/12] omap24xxcam: add v4l2_device and replace parent with v4l2_dev

[Hans Verkuil]

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

This driver did not yet support struct v4l2_device, so add it. This
make it possible to replace the deprecated parent field with the
v4l2_dev field, allowing the eventual removal of the parent field.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 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

[REVIEWv2 PATCH 10/12] v4l2: always require v4l2_dev, rename parent to dev_parent

[Hans Verkuil]

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

The last set of drivers still using the parent field of video_device instead
of the v4l2_dev field have been converted, so v4l2_dev is now always set.

A proper pointer to v4l2_dev is necessary these days otherwise the advanced
debugging ioctls will not work when addressing sub-devices. It also ensures
that the core can always go from a video_device struct to the top-level
v4l2_device struct.

There is still one single use case for the parent pointer: if there are
multiple busses, each being the parent of one or more video nodes, and if
they all share the same v4l2_device struct. In that case one still needs a
parent pointer since the v4l2_device struct can only refer to a single
parent device. The cx88 driver is one such case. Unfortunately, the cx88
failed to set the parent pointer since 3.6. The next patch will correct this.

In order to support this use-case the parent pointer is only renamed to
dev_parent, not removed altogether. It has been renamed to ensure that the
compiler will catch any (possibly out-of-tree) drivers that were missed during
the conversion.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@iki.fi>
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             |    4 ++--
 3 files changed, 18 insertions(+), 27 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 2f3fac5..8856ef1 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,14 @@ 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->dev_parent == NULL)
+		vdev->dev_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;
 
 	/* Part 2: find a free minor, device node number and device index. */
 #ifdef CONFIG_VIDEO_FIXED_MINOR_RANGES
@@ -897,8 +894,7 @@ 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;
+	vdev->dev.parent = vdev->dev_parent;
 	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..c768c9f 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -96,9 +96,9 @@ 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 */
+	/* Only set parent if that can't be deduced from v4l2_dev */
+	struct device *dev_parent;	/* device parent */
 
 	/* Control handler associated with this device node. May be NULL. */
 	struct v4l2_ctrl_handler *ctrl_handler;
-- 
1.7.10.4

[REVIEWv2 PATCH 11/12] cx88: set dev_parent to the correct parent PCI bus.

[Hans Verkuil]

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

The cx88 driver has one v4l2_device, but the video nodes are owned by two
different PCI busses. So the dev_parent pointer should be set to the correct
parent bus, otherwise sysfs won't show the correct device hierarchy.

This broke starting in 3.6 after a driver change, so this patch resurrects
the correct behavior.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/cx88/cx88-core.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/pci/cx88/cx88-core.c b/drivers/media/pci/cx88/cx88-core.c
index c8f3dcc..ad59dc9 100644
--- a/drivers/media/pci/cx88/cx88-core.c
+++ b/drivers/media/pci/cx88/cx88-core.c
@@ -1034,7 +1034,14 @@ struct video_device *cx88_vdev_init(struct cx88_core *core,
 	if (NULL == vfd)
 		return NULL;
 	*vfd = *template_;
+	/*
+	 * The dev pointer of v4l2_device is NULL, instead we set the
+	 * video_device dev_parent pointer to the correct PCI bus device.
+	 * This driver is a rare example where there is one v4l2_device,
+	 * but the video nodes have different parent (PCI) devices.
+	 */
 	vfd->v4l2_dev = &core->v4l2_dev;
+	vfd->dev_parent = &pci->dev;
 	vfd->release = video_device_release;
 	snprintf(vfd->name, sizeof(vfd->name), "%s %s (%s)",
 		 core->name, type, core->board.name);
-- 
1.7.10.4

[REVIEWv2 PATCH 12/12] v4l2-framework: update documentation

[Hans Verkuil]

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

'parent' was renamed to 'dev_parent'. Clarify how/when this should be used.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/video4linux/v4l2-framework.txt |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index 24353ec..3944d5c 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -574,9 +574,13 @@ of the video device exits.
 The default video_device_release() callback just calls kfree to free the
 allocated memory.
 
+There is also a video_device_release_empty() function that does nothing
+(is empty) and can be used if the struct is embedded and there is nothing
+to do when it is released.
+
 You should also set these fields:
 
-- v4l2_dev: set to the v4l2_device parent device.
+- v4l2_dev: must be set to the v4l2_device parent device.
 
 - name: set to something descriptive and unique.
 
@@ -613,15 +617,16 @@ You should also set these fields:
   If you want to have a separate priority state per (group of) device node(s),
   then you can point it to your own struct v4l2_prio_state.
 
-- parent: you only set this if v4l2_device was registered with NULL as
+- dev_parent: you only set this if v4l2_device was registered with NULL as
   the parent device struct. This only happens in cases where one hardware
   device has multiple PCI devices that all share the same v4l2_device core.
 
   The cx88 driver is an example of this: one core v4l2_device struct, but
-  it is used by both an raw video PCI device (cx8800) and a MPEG PCI device
-  (cx8802). Since the v4l2_device cannot be associated with a particular
-  PCI device it is setup without a parent device. But when the struct
-  video_device is setup you do know which parent PCI device to use.
+  it is used by both a raw video PCI device (cx8800) and a MPEG PCI device
+  (cx8802). Since the v4l2_device cannot be associated with a two PCI devices
+  at the same time it is setup without a parent device. But when the struct
+  video_device is initialized you *do* know which parent PCI device to use and
+  so you set dev_device to the correct PCI device.
 
 - flags: optional. Set to V4L2_FL_USE_FH_PRIO if you want to let the framework
   handle the VIDIOC_G/S_PRIORITY ioctls. This requires that you use struct
-- 
1.7.10.4

Re: [REVIEWv2 PATCH 07/12] pvrusb2: use v4l2_dev instead of the deprecated parent field.

[Mike Isely]

Acked-By: Mike Isely <isely@pobox.com>

  -Mike

On Wed, 12 Jun 2013, Hans Verkuil wrote:

> 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,
> 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

Re: [REVIEWv2 PATCH 01/12] v4l2-device: check if already unregistered.

[Prabhakar Lad]

Hi Hans,

Thanks for the patch.  The patch looks OK expect for the small typo below in
the commit message.

On Wed, Jun 12, 2013 at 8:30 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> It was possible to unregister an already unregistered v4l2_device struct.
> Add a check whether that already happened and just return if that was
> the case.
>
> Also refure to register a v4l2_device if both the dev and name fields are

s/refure/refuse

Regards,
--Prabhakar Lad

Re: [REVIEWv2 PATCH 08/12] f_uvc: add v4l2_device and replace parent with v4l2_dev

[Laurent Pinchart]

Hi Hans,

Thanks for the patch.

On Wednesday 12 June 2013 17:00:58 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This driver did not yet support struct v4l2_device, so add it. This
> make it possible to replace the deprecated parent field with the
> v4l2_dev field, allowing the eventual removal of the parent field.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

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

> ---
>  drivers/usb/gadget/f_uvc.c |    9 ++++++++-
>  drivers/usb/gadget/uvc.h   |    2 ++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
> index 38dcedd..1d06567 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)
> @@ -712,6 +718,7 @@ uvc_function_bind(struct usb_configuration *c, struct
> usb_function *f) return 0;
> 
>  error:
> +	v4l2_device_unregister(&uvc->v4l2_dev);
>  	if (uvc->vdev)
>  		video_device_release(uvc->vdev);
> 
> 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

Re: [REVIEWv2 PATCH 12/12] v4l2-framework: update documentation

[Laurent Pinchart]

Hi Hans,

Thanks for the patch.

On Wednesday 12 June 2013 17:01:02 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> 'parent' was renamed to 'dev_parent'. Clarify how/when this should be used.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

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

with a small comment below.

> ---
>  Documentation/video4linux/v4l2-framework.txt |   17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/video4linux/v4l2-framework.txt
> b/Documentation/video4linux/v4l2-framework.txt index 24353ec..3944d5c
> 100644
> --- a/Documentation/video4linux/v4l2-framework.txt
> +++ b/Documentation/video4linux/v4l2-framework.txt
> @@ -574,9 +574,13 @@ of the video device exits.
>  The default video_device_release() callback just calls kfree to free the
>  allocated memory.
> 
> +There is also a video_device_release_empty() function that does nothing
> +(is empty) and can be used if the struct is embedded and there is nothing
> +to do when it is released.
> +
>  You should also set these fields:
> 
> -- v4l2_dev: set to the v4l2_device parent device.
> +- v4l2_dev: must be set to the v4l2_device parent device.
> 
>  - name: set to something descriptive and unique.
> 
> @@ -613,15 +617,16 @@ You should also set these fields:
>    If you want to have a separate priority state per (group of) device
> node(s), then you can point it to your own struct v4l2_prio_state.
> 
> -- parent: you only set this if v4l2_device was registered with NULL as
> +- dev_parent: you only set this if v4l2_device was registered with NULL as
>    the parent device struct. This only happens in cases where one hardware
>    device has multiple PCI devices that all share the same v4l2_device core.
> 
>    The cx88 driver is an example of this: one core v4l2_device struct, but
> -  it is used by both an raw video PCI device (cx8800) and a MPEG PCI device
> -  (cx8802). Since the v4l2_device cannot be associated with a particular
> -  PCI device it is setup without a parent device. But when the struct
> -  video_device is setup you do know which parent PCI device to use.
> +  it is used by both a raw video PCI device (cx8800) and a MPEG PCI device
> +  (cx8802). Since the v4l2_device cannot be associated with a two PCI

s/a two/two/

> +  devices at the same time it is setup without a parent device. But when
> +  the struct video_device is initialized you *do* know which parent PCI
> +  device to use and  so you set dev_device to the correct PCI device.
> 
>  - flags: optional. Set to V4L2_FL_USE_FH_PRIO if you want to let the
> framework handle the VIDIOC_G/S_PRIORITY ioctls. This requires that you use
> struct
-- 
Regards,

Laurent Pinchart

Re: [REVIEWv2 PATCH 02/12] soc_camera: replace vdev->parent by vdev->v4l2_dev.

[Hans Verkuil]

Guennadi,

Can you review/ack this? This patch series really should be merged for 3.11
to prevent problems with the debug ioctls being unable to discover and access
subdevices in the v4l2-core code.

I've tested this on my renesas board.

If you prefer, you can also take care of this patch yourself.

Regards,

	Hans

On Wed June 12 2013 17:00:52 Hans Verkuil wrote:
> 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);
>  }
>  
> 

Re: [REVIEWv2 PATCH 02/12] soc_camera: replace vdev->parent by vdev->v4l2_dev.

[Guennadi Liakhovetski]

Hi Hans

Thanks for the patch.

On Wed, 12 Jun 2013, Hans Verkuil wrote:

> 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>

I'm not quite sure about this patch, because where before only one 
dev_drvdata pointer was used to store a pointed to the icd context, now 
two pointers are used and your vdev_set_drvdata() is now called much later 
than platform_set_drvdata() in soc_camera_pdrv_probe(). So, I had to 
verify no calls to vdev_get_drvdata() are made between those two moments. 
I think it should be ok, specifically, an uncertain place is 
soc_camera_vdev_to_subdev(), used by the mt9t031 driver from its runtime 
PM. But I don't have access to that sensor, so, it hasn't been tested in 
ages. In fact, I think, I should remove its runtime PM and just call the 
resume from s_power(1). If anything breaks and onyone notices it - they 
will complain. Anyway, for now here's my

Acked-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Thanks
Guennadi

> ---
>  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
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/