Hi,
I'm slowly working my way though this series today (both review, as well
as some tweaks and testing).
More comments inline...
On 04/28/2012 05:09 PM, Hans Verkuil wrote:
From: Hans Verkuil<hans.verkuil@xxxxxxxxx>
Make the necessary changes to allow subdrivers to use the control framework.
This does not add control event support, that needs more work.
Signed-off-by: Hans Verkuil<hans.verkuil@xxxxxxxxx>
---
drivers/media/video/gspca/gspca.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/media/video/gspca/gspca.c b/drivers/media/video/gspca/gspca.c
index ca5a2b1..56dff10 100644
--- a/drivers/media/video/gspca/gspca.c
+++ b/drivers/media/video/gspca/gspca.c
@@ -38,6 +38,7 @@
#include<linux/uaccess.h>
#include<linux/ktime.h>
#include<media/v4l2-ioctl.h>
+#include<media/v4l2-ctrls.h>
#include "gspca.h"
@@ -1006,6 +1007,8 @@ static void gspca_set_default_mode(struct gspca_dev *gspca_dev)
/* set the current control values to their default values
* which may have changed in sd_init() */
+ /* does nothing if ctrl_handler == NULL */
+ v4l2_ctrl_handler_setup(gspca_dev->vdev.ctrl_handler);
ctrl = gspca_dev->cam.ctrls;
if (ctrl != NULL) {
for (i = 0;
@@ -1323,6 +1326,7 @@ static void gspca_release(struct video_device *vfd)
PDEBUG(D_PROBE, "%s released",
video_device_node_name(&gspca_dev->vdev));
+ v4l2_ctrl_handler_free(gspca_dev->vdev.ctrl_handler);
kfree(gspca_dev->usb_buf);
kfree(gspca_dev);
}
@@ -2347,6 +2351,10 @@ int gspca_dev_probe2(struct usb_interface *intf,
gspca_dev->sd_desc = sd_desc;
gspca_dev->nbufread = 2;
gspca_dev->empty_packet = -1; /* don't check the empty packets */
+ gspca_dev->vdev = gspca_template;
+ gspca_dev->vdev.parent =&intf->dev;
+ gspca_dev->module = module;
+ gspca_dev->present = 1;
/* configure the subdriver and initialize the USB device */
ret = sd_desc->config(gspca_dev, id);
You also need to move the initialization of the mutexes here, as the
v4l2_ctrl_handler_setup will call s_ctrl on all the controls, and s_ctrl
should take the usb_lock (see my review of the next patch in this series),
I'll make this change myself and merge it into your patch.
@@ -2368,10 +2376,6 @@ int gspca_dev_probe2(struct usb_interface *intf,
init_waitqueue_head(&gspca_dev->wq);
/* init video stuff */
- memcpy(&gspca_dev->vdev,&gspca_template, sizeof gspca_template);
- gspca_dev->vdev.parent =&intf->dev;
- gspca_dev->module = module;
- gspca_dev->present = 1;
ret = video_register_device(&gspca_dev->vdev,
VFL_TYPE_GRABBER,
-1);
@@ -2391,6 +2395,7 @@ out:
if (gspca_dev->input_dev)
input_unregister_device(gspca_dev->input_dev);
#endif
+ v4l2_ctrl_handler_free(gspca_dev->vdev.ctrl_handler);
kfree(gspca_dev->usb_buf);
kfree(gspca_dev);
return ret;
Otherwise looks good, I've added it to my local tree (with the
described change), and will include it in my next pullreq.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
