s2255: v4l2 compliance fixes

s2255: v4l2 compliance fixes

[Hans Verkuil]

This patch series fixes v4l2-compliance issues in the Sensoray s2255drv
driver and makes it run on a big endian system. It has been tested with
a s2255 unit generously supplied by Sensoray (thanks!).

The only changes since the previous RFC patches are the addition of
patches 10 and 11.

If there are no comments, then I'll post a pull request for 3.10 on
Friday.

As usual with these patch series, I did not update the driver to the
vb2 framework. That's something for the future.

Regards,

        Hans

[REVIEW PATCH 01/11] s2255: convert to the control framework.

[Hans Verkuil]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/s2255/s2255drv.c |  172 ++++++++++++------------------------
 include/uapi/linux/v4l2-controls.h |    4 +
 2 files changed, 59 insertions(+), 117 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 498c57e..2dcb29b 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -47,6 +47,7 @@
 #include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-ctrls.h>
 #include <linux/vmalloc.h>
 #include <linux/usb.h>
 
@@ -217,6 +218,7 @@ struct s2255_dev;
 
 struct s2255_channel {
 	struct video_device	vdev;
+	struct v4l2_ctrl_handler hdl;
 	int			resources;
 	struct s2255_dmaqueue	vidq;
 	struct s2255_bufferi	buffer;
@@ -336,7 +338,7 @@ struct s2255_fh {
  */
 #define S2255_V4L2_YC_ON  1
 #define S2255_V4L2_YC_OFF 0
-#define V4L2_CID_PRIVATE_COLORFILTER (V4L2_CID_PRIVATE_BASE + 0)
+#define V4L2_CID_S2255_COLORFILTER (V4L2_CID_USER_S2255_BASE + 0)
 
 /* frame prefix size (sent once every frame) */
 #define PREFIX_SIZE		512
@@ -810,28 +812,6 @@ static void res_free(struct s2255_fh *fh)
 	dprintk(1, "res: put\n");
 }
 
-static int vidioc_querymenu(struct file *file, void *priv,
-			    struct v4l2_querymenu *qmenu)
-{
-	static const char *colorfilter[] = {
-		"Off",
-		"On",
-		NULL
-	};
-	if (qmenu->id == V4L2_CID_PRIVATE_COLORFILTER) {
-		int i;
-		const char **menu_items = colorfilter;
-		for (i = 0; i < qmenu->index && menu_items[i]; i++)
-			; /* do nothing (from v4l2-common.c) */
-		if (menu_items[i] == NULL || menu_items[i][0] == '\0')
-			return -EINVAL;
-		strlcpy(qmenu->name, menu_items[qmenu->index],
-			sizeof(qmenu->name));
-		return 0;
-	}
-	return v4l2_ctrl_query_menu(qmenu, NULL, NULL);
-}
-
 static int vidioc_querycap(struct file *file, void *priv,
 			   struct v4l2_capability *cap)
 {
@@ -1427,109 +1407,32 @@ static int vidioc_s_input(struct file *file, void *priv, unsigned int i)
 	return 0;
 }
 
-/* --- controls ---------------------------------------------- */
-static int vidioc_queryctrl(struct file *file, void *priv,
-			    struct v4l2_queryctrl *qc)
+static int s2255_s_ctrl(struct v4l2_ctrl *ctrl)
 {
-	struct s2255_fh *fh = priv;
-	struct s2255_channel *channel = fh->channel;
-	struct s2255_dev *dev = fh->dev;
-	switch (qc->id) {
-	case V4L2_CID_BRIGHTNESS:
-		v4l2_ctrl_query_fill(qc, -127, 127, 1, DEF_BRIGHT);
-		break;
-	case V4L2_CID_CONTRAST:
-		v4l2_ctrl_query_fill(qc, 0, 255, 1, DEF_CONTRAST);
-		break;
-	case V4L2_CID_SATURATION:
-		v4l2_ctrl_query_fill(qc, 0, 255, 1, DEF_SATURATION);
-		break;
-	case V4L2_CID_HUE:
-		v4l2_ctrl_query_fill(qc, 0, 255, 1, DEF_HUE);
-		break;
-	case V4L2_CID_PRIVATE_COLORFILTER:
-		if (dev->dsp_fw_ver < S2255_MIN_DSP_COLORFILTER)
-			return -EINVAL;
-		if ((dev->pid == 0x2257) && (channel->idx > 1))
-			return -EINVAL;
-		strlcpy(qc->name, "Color Filter", sizeof(qc->name));
-		qc->type = V4L2_CTRL_TYPE_MENU;
-		qc->minimum = 0;
-		qc->maximum = 1;
-		qc->step = 1;
-		qc->default_value = 1;
-		qc->flags = 0;
-		break;
-	default:
-		return -EINVAL;
-	}
-	dprintk(4, "%s, id %d\n", __func__, qc->id);
-	return 0;
-}
-
-static int vidioc_g_ctrl(struct file *file, void *priv,
-			 struct v4l2_control *ctrl)
-{
-	struct s2255_fh *fh = priv;
-	struct s2255_dev *dev = fh->dev;
-	struct s2255_channel *channel = fh->channel;
-	switch (ctrl->id) {
-	case V4L2_CID_BRIGHTNESS:
-		ctrl->value = channel->mode.bright;
-		break;
-	case V4L2_CID_CONTRAST:
-		ctrl->value = channel->mode.contrast;
-		break;
-	case V4L2_CID_SATURATION:
-		ctrl->value = channel->mode.saturation;
-		break;
-	case V4L2_CID_HUE:
-		ctrl->value = channel->mode.hue;
-		break;
-	case V4L2_CID_PRIVATE_COLORFILTER:
-		if (dev->dsp_fw_ver < S2255_MIN_DSP_COLORFILTER)
-			return -EINVAL;
-		if ((dev->pid == 0x2257) && (channel->idx > 1))
-			return -EINVAL;
-		ctrl->value = !((channel->mode.color & MASK_INPUT_TYPE) >> 16);
-		break;
-	default:
-		return -EINVAL;
-	}
-	dprintk(4, "%s, id %d val %d\n", __func__, ctrl->id, ctrl->value);
-	return 0;
-}
-
-static int vidioc_s_ctrl(struct file *file, void *priv,
-			 struct v4l2_control *ctrl)
-{
-	struct s2255_fh *fh = priv;
-	struct s2255_channel *channel = fh->channel;
-	struct s2255_dev *dev = to_s2255_dev(channel->vdev.v4l2_dev);
+	struct s2255_channel *channel =
+		container_of(ctrl->handler, struct s2255_channel, hdl);
 	struct s2255_mode mode;
+
 	mode = channel->mode;
 	dprintk(4, "%s\n", __func__);
+
 	/* update the mode to the corresponding value */
 	switch (ctrl->id) {
 	case V4L2_CID_BRIGHTNESS:
-		mode.bright = ctrl->value;
+		mode.bright = ctrl->val;
 		break;
 	case V4L2_CID_CONTRAST:
-		mode.contrast = ctrl->value;
+		mode.contrast = ctrl->val;
 		break;
 	case V4L2_CID_HUE:
-		mode.hue = ctrl->value;
+		mode.hue = ctrl->val;
 		break;
 	case V4L2_CID_SATURATION:
-		mode.saturation = ctrl->value;
+		mode.saturation = ctrl->val;
 		break;
-	case V4L2_CID_PRIVATE_COLORFILTER:
-		if (dev->dsp_fw_ver < S2255_MIN_DSP_COLORFILTER)
-			return -EINVAL;
-		if ((dev->pid == 0x2257) && (channel->idx > 1))
-			return -EINVAL;
+	case V4L2_CID_S2255_COLORFILTER:
 		mode.color &= ~MASK_INPUT_TYPE;
-		mode.color |= ((ctrl->value ? 0 : 1) << 16);
+		mode.color |= !ctrl->val << 16;
 		break;
 	default:
 		return -EINVAL;
@@ -1539,7 +1442,7 @@ static int vidioc_s_ctrl(struct file *file, void *priv,
 	   some V4L programs restart stream unnecessarily
 	   after a s_crtl.
 	*/
-	s2255_set_mode(fh->channel, &mode);
+	s2255_set_mode(channel, &mode);
 	return 0;
 }
 
@@ -1886,7 +1789,6 @@ static const struct v4l2_file_operations s2255_fops_v4l = {
 };
 
 static const struct v4l2_ioctl_ops s2255_ioctl_ops = {
-	.vidioc_querymenu = vidioc_querymenu,
 	.vidioc_querycap = vidioc_querycap,
 	.vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
 	.vidioc_g_fmt_vid_cap = vidioc_g_fmt_vid_cap,
@@ -1900,9 +1802,6 @@ static const struct v4l2_ioctl_ops s2255_ioctl_ops = {
 	.vidioc_enum_input = vidioc_enum_input,
 	.vidioc_g_input = vidioc_g_input,
 	.vidioc_s_input = vidioc_s_input,
-	.vidioc_queryctrl = vidioc_queryctrl,
-	.vidioc_g_ctrl = vidioc_g_ctrl,
-	.vidioc_s_ctrl = vidioc_s_ctrl,
 	.vidioc_streamon = vidioc_streamon,
 	.vidioc_streamoff = vidioc_streamoff,
 	.vidioc_s_jpegcomp = vidioc_s_jpegcomp,
@@ -1915,8 +1814,13 @@ static const struct v4l2_ioctl_ops s2255_ioctl_ops = {
 static void s2255_video_device_release(struct video_device *vdev)
 {
 	struct s2255_dev *dev = to_s2255_dev(vdev->v4l2_dev);
-	dprintk(4, "%s, chnls: %d \n", __func__,
+	struct s2255_channel *channel =
+		container_of(vdev, struct s2255_channel, vdev);
+
+	v4l2_ctrl_handler_free(&channel->hdl);
+	dprintk(4, "%s, chnls: %d\n", __func__,
 		atomic_read(&dev->num_channels));
+
 	if (atomic_dec_and_test(&dev->num_channels))
 		s2255_destroy(dev);
 	return;
@@ -1931,6 +1835,20 @@ static struct video_device template = {
 	.current_norm = V4L2_STD_NTSC_M,
 };
 
+static const struct v4l2_ctrl_ops s2255_ctrl_ops = {
+	.s_ctrl = s2255_s_ctrl,
+};
+
+static const struct v4l2_ctrl_config color_filter_ctrl = {
+	.ops = &s2255_ctrl_ops,
+	.name = "Color Filter",
+	.id = V4L2_CID_S2255_COLORFILTER,
+	.type = V4L2_CTRL_TYPE_BOOLEAN,
+	.max = 1,
+	.step = 1,
+	.def = 1,
+};
+
 static int s2255_probe_v4l(struct s2255_dev *dev)
 {
 	int ret;
@@ -1945,9 +1863,29 @@ static int s2255_probe_v4l(struct s2255_dev *dev)
 	for (i = 0; i < MAX_CHANNELS; i++) {
 		channel = &dev->channel[i];
 		INIT_LIST_HEAD(&channel->vidq.active);
+
+		v4l2_ctrl_handler_init(&channel->hdl, 5);
+		v4l2_ctrl_new_std(&channel->hdl, &s2255_ctrl_ops,
+				V4L2_CID_BRIGHTNESS, -127, 127, 1, DEF_BRIGHT);
+		v4l2_ctrl_new_std(&channel->hdl, &s2255_ctrl_ops,
+				V4L2_CID_CONTRAST, 0, 255, 1, DEF_CONTRAST);
+		v4l2_ctrl_new_std(&channel->hdl, &s2255_ctrl_ops,
+				V4L2_CID_SATURATION, 0, 255, 1, DEF_SATURATION);
+		v4l2_ctrl_new_std(&channel->hdl, &s2255_ctrl_ops,
+				V4L2_CID_HUE, 0, 255, 1, DEF_HUE);
+		if (dev->dsp_fw_ver >= S2255_MIN_DSP_COLORFILTER &&
+		    (dev->pid != 0x2257 || channel->idx <= 1))
+			v4l2_ctrl_new_custom(&channel->hdl, &color_filter_ctrl, NULL);
+		if (channel->hdl.error) {
+			ret = channel->hdl.error;
+			v4l2_ctrl_handler_free(&channel->hdl);
+			dev_err(&dev->udev->dev, "couldn't register control\n");
+			break;
+		}
 		channel->vidq.dev = dev;
 		/* register 4 video devices */
 		channel->vdev = template;
+		channel->vdev.ctrl_handler = &channel->hdl;
 		channel->vdev.lock = &dev->lock;
 		channel->vdev.v4l2_dev = &dev->v4l2_dev;
 		video_set_drvdata(&channel->vdev, channel);
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index dcd6374..f6ba2fc 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -146,6 +146,10 @@ enum v4l2_colorfx {
  * of controls. We reserve 16 controls for this driver. */
 #define V4L2_CID_USER_MEYE_BASE			(V4L2_CID_USER_BASE + 0x1000)
 
+/* The base for the s2255 driver controls.
+ * We reserve 8 controls for this driver. */
+#define V4L2_CID_USER_S2255_BASE		(V4L2_CID_USER_BASE + 0x1010)
+
 /* MPEG-class control IDs */
 
 #define V4L2_CID_MPEG_BASE 			(V4L2_CTRL_CLASS_MPEG | 0x900)
-- 
1.7.10.4

[REVIEW PATCH 02/11] s2255: add V4L2_CID_JPEG_COMPRESSION_QUALITY

[Hans Verkuil]

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

The use of the V4L2_CID_JPEG_COMPRESSION_QUALITY control is recommended over
the G/S_JPEGCOMP ioctls.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/s2255/s2255drv.c |   24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 2dcb29b..42c3afe 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -219,12 +219,13 @@ struct s2255_dev;
 struct s2255_channel {
 	struct video_device	vdev;
 	struct v4l2_ctrl_handler hdl;
+	struct v4l2_ctrl	*jpegqual_ctrl;
 	int			resources;
 	struct s2255_dmaqueue	vidq;
 	struct s2255_bufferi	buffer;
 	struct s2255_mode	mode;
 	/* jpeg compression */
-	struct v4l2_jpegcompression jc;
+	unsigned		jpegqual;
 	/* capture parameters (for high quality mode full size) */
 	struct v4l2_captureparm cap_parm;
 	int			cur_frame;
@@ -1015,7 +1016,7 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 	case V4L2_PIX_FMT_MJPEG:
 		mode.color &= ~MASK_COLOR;
 		mode.color |= COLOR_JPG;
-		mode.color |= (channel->jc.quality << 8);
+		mode.color |= (channel->jpegqual << 8);
 		break;
 	case V4L2_PIX_FMT_YUV422P:
 		mode.color &= ~MASK_COLOR;
@@ -1185,7 +1186,7 @@ static int s2255_set_mode(struct s2255_channel *channel,
 		mode->color &= ~MASK_COLOR;
 		mode->color |= COLOR_JPG;
 		mode->color &= ~MASK_JPG_QUALITY;
-		mode->color |= (channel->jc.quality << 8);
+		mode->color |= (channel->jpegqual << 8);
 	}
 	/* save the mode */
 	channel->mode = *mode;
@@ -1434,6 +1435,9 @@ static int s2255_s_ctrl(struct v4l2_ctrl *ctrl)
 		mode.color &= ~MASK_INPUT_TYPE;
 		mode.color |= !ctrl->val << 16;
 		break;
+	case V4L2_CID_JPEG_COMPRESSION_QUALITY:
+		channel->jpegqual = ctrl->val;
+		return 0;
 	default:
 		return -EINVAL;
 	}
@@ -1451,7 +1455,9 @@ static int vidioc_g_jpegcomp(struct file *file, void *priv,
 {
 	struct s2255_fh *fh = priv;
 	struct s2255_channel *channel = fh->channel;
-	*jc = channel->jc;
+
+	memset(jc, 0, sizeof(*jc));
+	jc->quality = channel->jpegqual;
 	dprintk(2, "%s: quality %d\n", __func__, jc->quality);
 	return 0;
 }
@@ -1463,7 +1469,7 @@ static int vidioc_s_jpegcomp(struct file *file, void *priv,
 	struct s2255_channel *channel = fh->channel;
 	if (jc->quality < 0 || jc->quality > 100)
 		return -EINVAL;
-	channel->jc.quality = jc->quality;
+	v4l2_ctrl_s_ctrl(channel->jpegqual_ctrl, jc->quality);
 	dprintk(2, "%s: quality %d\n", __func__, jc->quality);
 	return 0;
 }
@@ -1864,7 +1870,7 @@ static int s2255_probe_v4l(struct s2255_dev *dev)
 		channel = &dev->channel[i];
 		INIT_LIST_HEAD(&channel->vidq.active);
 
-		v4l2_ctrl_handler_init(&channel->hdl, 5);
+		v4l2_ctrl_handler_init(&channel->hdl, 6);
 		v4l2_ctrl_new_std(&channel->hdl, &s2255_ctrl_ops,
 				V4L2_CID_BRIGHTNESS, -127, 127, 1, DEF_BRIGHT);
 		v4l2_ctrl_new_std(&channel->hdl, &s2255_ctrl_ops,
@@ -1873,6 +1879,10 @@ static int s2255_probe_v4l(struct s2255_dev *dev)
 				V4L2_CID_SATURATION, 0, 255, 1, DEF_SATURATION);
 		v4l2_ctrl_new_std(&channel->hdl, &s2255_ctrl_ops,
 				V4L2_CID_HUE, 0, 255, 1, DEF_HUE);
+		channel->jpegqual_ctrl = v4l2_ctrl_new_std(&channel->hdl,
+				&s2255_ctrl_ops,
+				V4L2_CID_JPEG_COMPRESSION_QUALITY,
+				0, 100, 1, S2255_DEF_JPEG_QUAL);
 		if (dev->dsp_fw_ver >= S2255_MIN_DSP_COLORFILTER &&
 		    (dev->pid != 0x2257 || channel->idx <= 1))
 			v4l2_ctrl_new_custom(&channel->hdl, &color_filter_ctrl, NULL);
@@ -2238,7 +2248,7 @@ static int s2255_board_init(struct s2255_dev *dev)
 		channel->mode = mode_def;
 		if (dev->pid == 0x2257 && j > 1)
 			channel->mode.color |= (1 << 16);
-		channel->jc.quality = S2255_DEF_JPEG_QUAL;
+		channel->jpegqual = S2255_DEF_JPEG_QUAL;
 		channel->width = LINE_SZ_4CIFS_NTSC;
 		channel->height = NUM_LINES_4CIFS_NTSC * 2;
 		channel->fmt = &formats[0];
-- 
1.7.10.4

[REVIEW PATCH 03/11] s2255: add support for control events and prio handling.

[Hans Verkuil]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/s2255/s2255drv.c |   22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 42c3afe..55c972a 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -43,13 +43,14 @@
 #include <linux/slab.h>
 #include <linux/videodev2.h>
 #include <linux/mm.h>
+#include <linux/vmalloc.h>
+#include <linux/usb.h>
 #include <media/videobuf-vmalloc.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-ctrls.h>
-#include <linux/vmalloc.h>
-#include <linux/usb.h>
+#include <media/v4l2-event.h>
 
 #define S2255_VERSION		"1.22.1"
 #define FIRMWARE_FILE_NAME "f2255usb.bin"
@@ -295,6 +296,8 @@ struct s2255_buffer {
 };
 
 struct s2255_fh {
+	/* this must be the first field in this struct */
+	struct v4l2_fh		fh;
 	struct s2255_dev	*dev;
 	struct videobuf_queue	vb_vidq;
 	enum v4l2_buf_type	type;
@@ -1666,7 +1669,9 @@ static int __s2255_open(struct file *file)
 	fh = kzalloc(sizeof(*fh), GFP_KERNEL);
 	if (NULL == fh)
 		return -ENOMEM;
-	file->private_data = fh;
+	v4l2_fh_init(&fh->fh, vdev);
+	v4l2_fh_add(&fh->fh);
+	file->private_data = &fh->fh;
 	fh->dev = dev;
 	fh->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	fh->channel = channel;
@@ -1709,12 +1714,13 @@ static unsigned int s2255_poll(struct file *file,
 {
 	struct s2255_fh *fh = file->private_data;
 	struct s2255_dev *dev = fh->dev;
-	int rc;
+	int rc = v4l2_ctrl_poll(file, wait);
+
 	dprintk(100, "%s\n", __func__);
 	if (V4L2_BUF_TYPE_VIDEO_CAPTURE != fh->type)
 		return POLLERR;
 	mutex_lock(&dev->lock);
-	rc = videobuf_poll_stream(file, &fh->vb_vidq, wait);
+	rc |= videobuf_poll_stream(file, &fh->vb_vidq, wait);
 	mutex_unlock(&dev->lock);
 	return rc;
 }
@@ -1761,6 +1767,8 @@ static int s2255_release(struct file *file)
 	videobuf_mmap_free(&fh->vb_vidq);
 	mutex_unlock(&dev->lock);
 	dprintk(1, "%s (dev=%s)\n", __func__, video_device_node_name(vdev));
+	v4l2_fh_del(&fh->fh);
+	v4l2_fh_exit(&fh->fh);
 	kfree(fh);
 	return 0;
 }
@@ -1815,6 +1823,9 @@ static const struct v4l2_ioctl_ops s2255_ioctl_ops = {
 	.vidioc_s_parm = vidioc_s_parm,
 	.vidioc_g_parm = vidioc_g_parm,
 	.vidioc_enum_frameintervals = vidioc_enum_frameintervals,
+	.vidioc_log_status  = v4l2_ctrl_log_status,
+	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
+	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
 };
 
 static void s2255_video_device_release(struct video_device *vdev)
@@ -1898,6 +1909,7 @@ static int s2255_probe_v4l(struct s2255_dev *dev)
 		channel->vdev.ctrl_handler = &channel->hdl;
 		channel->vdev.lock = &dev->lock;
 		channel->vdev.v4l2_dev = &dev->v4l2_dev;
+		set_bit(V4L2_FL_USE_FH_PRIO, &channel->vdev.flags);
 		video_set_drvdata(&channel->vdev, channel);
 		if (video_nr == -1)
 			ret = video_register_device(&channel->vdev,
-- 
1.7.10.4

[REVIEW PATCH 04/11] s2255: add device_caps support to querycap.

[Hans Verkuil]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/s2255/s2255drv.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 55c972a..9cb8325 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -821,10 +821,12 @@ static int vidioc_querycap(struct file *file, void *priv,
 {
 	struct s2255_fh *fh = file->private_data;
 	struct s2255_dev *dev = fh->dev;
+
 	strlcpy(cap->driver, "s2255", sizeof(cap->driver));
 	strlcpy(cap->card, "s2255", sizeof(cap->card));
 	usb_make_path(dev->udev, cap->bus_info, sizeof(cap->bus_info));
-	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
+	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
+	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
 
-- 
1.7.10.4

[REVIEW PATCH 05/11] s2255: fixes in the way standards are handled.

[Hans Verkuil]

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

Instead of comparing against STD_NTSC and STD_PAL compare against 60 and
50 Hz formats. That's what you really want.

When the standard is changed, make sure the width and height of the format
are also updated to reflect the current standard.

Also replace the deprecated current_norm by the g_std ioctl.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/s2255/s2255drv.c |   61 +++++++++++++++++++++++-------------
 1 file changed, 39 insertions(+), 22 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 9cb8325..88f728d 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -225,6 +225,7 @@ struct s2255_channel {
 	struct s2255_dmaqueue	vidq;
 	struct s2255_bufferi	buffer;
 	struct s2255_mode	mode;
+	v4l2_std_id		std;
 	/* jpeg compression */
 	unsigned		jpegqual;
 	/* capture parameters (for high quality mode full size) */
@@ -312,7 +313,7 @@ struct s2255_fh {
 /* Need DSP version 5+ for video status feature */
 #define S2255_MIN_DSP_STATUS      5
 #define S2255_MIN_DSP_COLORFILTER 8
-#define S2255_NORMS		(V4L2_STD_PAL | V4L2_STD_NTSC)
+#define S2255_NORMS		(V4L2_STD_ALL)
 
 /* private V4L2 controls */
 
@@ -443,27 +444,27 @@ static const struct s2255_fmt formats[] = {
 	}
 };
 
-static int norm_maxw(struct video_device *vdev)
+static int norm_maxw(struct s2255_channel *channel)
 {
-	return (vdev->current_norm & V4L2_STD_NTSC) ?
+	return (channel->std & V4L2_STD_525_60) ?
 	    LINE_SZ_4CIFS_NTSC : LINE_SZ_4CIFS_PAL;
 }
 
-static int norm_maxh(struct video_device *vdev)
+static int norm_maxh(struct s2255_channel *channel)
 {
-	return (vdev->current_norm & V4L2_STD_NTSC) ?
+	return (channel->std & V4L2_STD_525_60) ?
 	    (NUM_LINES_1CIFS_NTSC * 2) : (NUM_LINES_1CIFS_PAL * 2);
 }
 
-static int norm_minw(struct video_device *vdev)
+static int norm_minw(struct s2255_channel *channel)
 {
-	return (vdev->current_norm & V4L2_STD_NTSC) ?
+	return (channel->std & V4L2_STD_525_60) ?
 	    LINE_SZ_1CIFS_NTSC : LINE_SZ_1CIFS_PAL;
 }
 
-static int norm_minh(struct video_device *vdev)
+static int norm_minh(struct s2255_channel *channel)
 {
-	return (vdev->current_norm & V4L2_STD_NTSC) ?
+	return (channel->std & V4L2_STD_525_60) ?
 	    (NUM_LINES_1CIFS_NTSC) : (NUM_LINES_1CIFS_PAL);
 }
 
@@ -725,10 +726,10 @@ static int buffer_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
 	if (channel->fmt == NULL)
 		return -EINVAL;
 
-	if ((w < norm_minw(&channel->vdev)) ||
-	    (w > norm_maxw(&channel->vdev)) ||
-	    (h < norm_minh(&channel->vdev)) ||
-	    (h > norm_maxh(&channel->vdev))) {
+	if ((w < norm_minw(channel)) ||
+	    (w > norm_maxw(channel)) ||
+	    (h < norm_minh(channel)) ||
+	    (h > norm_maxh(channel))) {
 		dprintk(4, "invalid buffer prepare\n");
 		return -EINVAL;
 	}
@@ -870,8 +871,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 	struct s2255_fh *fh = priv;
 	struct s2255_channel *channel = fh->channel;
 	int is_ntsc;
-	is_ntsc =
-		(channel->vdev.current_norm & V4L2_STD_NTSC) ? 1 : 0;
+	is_ntsc = (channel->std & V4L2_STD_525_60) ? 1 : 0;
 
 	fmt = format_by_fourcc(f->fmt.pix.pixelformat);
 
@@ -998,8 +998,8 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 	channel->height = f->fmt.pix.height;
 	fh->vb_vidq.field = f->fmt.pix.field;
 	fh->type = f->type;
-	if (channel->width > norm_minw(&channel->vdev)) {
-		if (channel->height > norm_minh(&channel->vdev)) {
+	if (channel->width > norm_minw(channel)) {
+		if (channel->height > norm_minh(channel)) {
 			if (channel->cap_parm.capturemode &
 			    V4L2_MODE_HIGHQUALITY)
 				mode.scale = SCALE_4CIFSI;
@@ -1323,7 +1323,9 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id *i)
 	struct s2255_fh *fh = priv;
 	struct s2255_mode mode;
 	struct videobuf_queue *q = &fh->vb_vidq;
+	struct s2255_channel *channel = fh->channel;
 	int ret = 0;
+
 	mutex_lock(&q->vb_lock);
 	if (videobuf_queue_is_busy(q)) {
 		dprintk(1, "queue busy\n");
@@ -1336,24 +1338,30 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id *i)
 		goto out_s_std;
 	}
 	mode = fh->channel->mode;
-	if (*i & V4L2_STD_NTSC) {
-		dprintk(4, "%s NTSC\n", __func__);
+	if (*i & V4L2_STD_525_60) {
+		dprintk(4, "%s 60 Hz\n", __func__);
 		/* if changing format, reset frame decimation/intervals */
 		if (mode.format != FORMAT_NTSC) {
 			mode.restart = 1;
 			mode.format = FORMAT_NTSC;
 			mode.fdec = FDEC_1;
+			channel->width = LINE_SZ_4CIFS_NTSC;
+			channel->height = NUM_LINES_4CIFS_NTSC * 2;
 		}
-	} else if (*i & V4L2_STD_PAL) {
-		dprintk(4, "%s PAL\n", __func__);
+	} else if (*i & V4L2_STD_625_50) {
+		dprintk(4, "%s 50 Hz\n", __func__);
 		if (mode.format != FORMAT_PAL) {
 			mode.restart = 1;
 			mode.format = FORMAT_PAL;
 			mode.fdec = FDEC_1;
+			channel->width = LINE_SZ_4CIFS_PAL;
+			channel->height = NUM_LINES_4CIFS_PAL * 2;
 		}
 	} else {
 		ret = -EINVAL;
+		goto out_s_std;
 	}
+	fh->channel->std = *i;
 	if (mode.restart)
 		s2255_set_mode(fh->channel, &mode);
 out_s_std:
@@ -1361,6 +1369,14 @@ out_s_std:
 	return ret;
 }
 
+static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *i)
+{
+	struct s2255_fh *fh = priv;
+
+	*i = fh->channel->std;
+	return 0;
+}
+
 /* Sensoray 2255 is a multiple channel capture device.
    It does not have a "crossbar" of inputs.
    We use one V4L device per channel. The user must
@@ -1815,6 +1831,7 @@ static const struct v4l2_ioctl_ops s2255_ioctl_ops = {
 	.vidioc_qbuf = vidioc_qbuf,
 	.vidioc_dqbuf = vidioc_dqbuf,
 	.vidioc_s_std = vidioc_s_std,
+	.vidioc_g_std = vidioc_g_std,
 	.vidioc_enum_input = vidioc_enum_input,
 	.vidioc_g_input = vidioc_g_input,
 	.vidioc_s_input = vidioc_s_input,
@@ -1851,7 +1868,6 @@ static struct video_device template = {
 	.ioctl_ops = &s2255_ioctl_ops,
 	.release = s2255_video_device_release,
 	.tvnorms = S2255_NORMS,
-	.current_norm = V4L2_STD_NTSC_M,
 };
 
 static const struct v4l2_ctrl_ops s2255_ctrl_ops = {
@@ -2265,6 +2281,7 @@ static int s2255_board_init(struct s2255_dev *dev)
 		channel->jpegqual = S2255_DEF_JPEG_QUAL;
 		channel->width = LINE_SZ_4CIFS_NTSC;
 		channel->height = NUM_LINES_4CIFS_NTSC * 2;
+		channel->std = V4L2_STD_NTSC_M;
 		channel->fmt = &formats[0];
 		channel->mode.restart = 1;
 		channel->req_image_size = get_transfer_size(&mode_def);
-- 
1.7.10.4

[REVIEW PATCH 06/11] s2255: zero priv and set colorspace.

[Hans Verkuil]

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

Set priv field of struct v4l2_pix_format to 0 and fill in colorspace.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/s2255/s2255drv.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 88f728d..a16bc6c 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -859,6 +859,8 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
 	f->fmt.pix.pixelformat = channel->fmt->fourcc;
 	f->fmt.pix.bytesperline = f->fmt.pix.width * (channel->fmt->depth >> 3);
 	f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
+	f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
+	f->fmt.pix.priv = 0;
 	return 0;
 }
 
@@ -954,6 +956,8 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 	f->fmt.pix.field = field;
 	f->fmt.pix.bytesperline = (f->fmt.pix.width * fmt->depth) >> 3;
 	f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
+	f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
+	f->fmt.pix.priv = 0;
 	dprintk(50, "%s: set width %d height %d field %d\n", __func__,
 		f->fmt.pix.width, f->fmt.pix.height, f->fmt.pix.field);
 	return 0;
-- 
1.7.10.4

[REVIEW PATCH 07/11] s2255: fix field handling

[Hans Verkuil]

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

Just set the field value based on the chosen format. It's either INTERLACED
or TOP.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/s2255/s2255drv.c |   61 +++++++++---------------------------
 1 file changed, 15 insertions(+), 46 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index a16bc6c..9693eb9 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -852,10 +852,15 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
 {
 	struct s2255_fh *fh = priv;
 	struct s2255_channel *channel = fh->channel;
+	int is_ntsc = channel->std & V4L2_STD_525_60;
 
 	f->fmt.pix.width = channel->width;
 	f->fmt.pix.height = channel->height;
-	f->fmt.pix.field = fh->vb_vidq.field;
+	if (f->fmt.pix.height >=
+	    (is_ntsc ? NUM_LINES_1CIFS_NTSC : NUM_LINES_1CIFS_PAL) * 2)
+		f->fmt.pix.field = V4L2_FIELD_INTERLACED;
+	else
+		f->fmt.pix.field = V4L2_FIELD_TOP;
 	f->fmt.pix.pixelformat = channel->fmt->fourcc;
 	f->fmt.pix.bytesperline = f->fmt.pix.width * (channel->fmt->depth >> 3);
 	f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
@@ -869,11 +874,9 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 {
 	const struct s2255_fmt *fmt;
 	enum v4l2_field field;
-	int  b_any_field = 0;
 	struct s2255_fh *fh = priv;
 	struct s2255_channel *channel = fh->channel;
-	int is_ntsc;
-	is_ntsc = (channel->std & V4L2_STD_525_60) ? 1 : 0;
+	int is_ntsc = channel->std & V4L2_STD_525_60;
 
 	fmt = format_by_fourcc(f->fmt.pix.pixelformat);
 
@@ -881,8 +884,6 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 		return -EINVAL;
 
 	field = f->fmt.pix.field;
-	if (field == V4L2_FIELD_ANY)
-		b_any_field = 1;
 
 	dprintk(50, "%s NTSC: %d suggested width: %d, height: %d\n",
 		__func__, is_ntsc, f->fmt.pix.width, f->fmt.pix.height);
@@ -890,24 +891,10 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 		/* NTSC */
 		if (f->fmt.pix.height >= NUM_LINES_1CIFS_NTSC * 2) {
 			f->fmt.pix.height = NUM_LINES_1CIFS_NTSC * 2;
-			if (b_any_field) {
-				field = V4L2_FIELD_SEQ_TB;
-			} else if (!((field == V4L2_FIELD_INTERLACED) ||
-				      (field == V4L2_FIELD_SEQ_TB) ||
-				      (field == V4L2_FIELD_INTERLACED_TB))) {
-				dprintk(1, "unsupported field setting\n");
-				return -EINVAL;
-			}
+			field = V4L2_FIELD_INTERLACED;
 		} else {
 			f->fmt.pix.height = NUM_LINES_1CIFS_NTSC;
-			if (b_any_field) {
-				field = V4L2_FIELD_TOP;
-			} else if (!((field == V4L2_FIELD_TOP) ||
-				      (field == V4L2_FIELD_BOTTOM))) {
-				dprintk(1, "unsupported field setting\n");
-				return -EINVAL;
-			}
-
+			field = V4L2_FIELD_TOP;
 		}
 		if (f->fmt.pix.width >= LINE_SZ_4CIFS_NTSC)
 			f->fmt.pix.width = LINE_SZ_4CIFS_NTSC;
@@ -921,37 +908,19 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 		/* PAL */
 		if (f->fmt.pix.height >= NUM_LINES_1CIFS_PAL * 2) {
 			f->fmt.pix.height = NUM_LINES_1CIFS_PAL * 2;
-			if (b_any_field) {
-				field = V4L2_FIELD_SEQ_TB;
-			} else if (!((field == V4L2_FIELD_INTERLACED) ||
-				      (field == V4L2_FIELD_SEQ_TB) ||
-				      (field == V4L2_FIELD_INTERLACED_TB))) {
-				dprintk(1, "unsupported field setting\n");
-				return -EINVAL;
-			}
+			field = V4L2_FIELD_INTERLACED;
 		} else {
 			f->fmt.pix.height = NUM_LINES_1CIFS_PAL;
-			if (b_any_field) {
-				field = V4L2_FIELD_TOP;
-			} else if (!((field == V4L2_FIELD_TOP) ||
-				     (field == V4L2_FIELD_BOTTOM))) {
-				dprintk(1, "unsupported field setting\n");
-				return -EINVAL;
-			}
+			field = V4L2_FIELD_TOP;
 		}
-		if (f->fmt.pix.width >= LINE_SZ_4CIFS_PAL) {
+		if (f->fmt.pix.width >= LINE_SZ_4CIFS_PAL)
 			f->fmt.pix.width = LINE_SZ_4CIFS_PAL;
-			field = V4L2_FIELD_SEQ_TB;
-		} else if (f->fmt.pix.width >= LINE_SZ_2CIFS_PAL) {
+		else if (f->fmt.pix.width >= LINE_SZ_2CIFS_PAL)
 			f->fmt.pix.width = LINE_SZ_2CIFS_PAL;
-			field = V4L2_FIELD_TOP;
-		} else if (f->fmt.pix.width >= LINE_SZ_1CIFS_PAL) {
+		else if (f->fmt.pix.width >= LINE_SZ_1CIFS_PAL)
 			f->fmt.pix.width = LINE_SZ_1CIFS_PAL;
-			field = V4L2_FIELD_TOP;
-		} else {
+		else
 			f->fmt.pix.width = LINE_SZ_1CIFS_PAL;
-			field = V4L2_FIELD_TOP;
-		}
 	}
 	f->fmt.pix.field = field;
 	f->fmt.pix.bytesperline = (f->fmt.pix.width * fmt->depth) >> 3;
-- 
1.7.10.4

[REVIEW PATCH 08/11] s2255: don't zero struct v4l2_streamparm

[Hans Verkuil]

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

All fields after 'type' are already zeroed by the core framework.
Clearing the full struct also clears 'type', which causes a wrong
type value to be returned.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/s2255/s2255drv.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 9693eb9..eaae9d1 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -1476,7 +1476,6 @@ static int vidioc_g_parm(struct file *file, void *priv,
 	struct s2255_channel *channel = fh->channel;
 	if (sp->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
-	memset(sp, 0, sizeof(struct v4l2_streamparm));
 	sp->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
 	sp->parm.capture.capturemode = channel->cap_parm.capturemode;
 	def_num = (channel->mode.format == FORMAT_NTSC) ? 1001 : 1000;
-- 
1.7.10.4

[REVIEW PATCH 09/11] s2255: Add ENUM_FRAMESIZES support.

[Hans Verkuil]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/s2255/s2255drv.c |   73 +++++++++++++++++++++++++-----------
 1 file changed, 51 insertions(+), 22 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index eaae9d1..59d40e6 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -1545,36 +1545,64 @@ static int vidioc_s_parm(struct file *file, void *priv,
 	return 0;
 }
 
+#define NUM_SIZE_ENUMS 3
+static const struct v4l2_frmsize_discrete ntsc_sizes[] = {
+	{ 640, 480 },
+	{ 640, 240 },
+	{ 320, 240 },
+};
+static const struct v4l2_frmsize_discrete pal_sizes[] = {
+	{ 704, 576 },
+	{ 704, 288 },
+	{ 352, 288 },
+};
+
+static int vidioc_enum_framesizes(struct file *file, void *priv,
+			    struct v4l2_frmsizeenum *fe)
+{
+	struct s2255_fh *fh = priv;
+	struct s2255_channel *channel = fh->channel;
+	int is_ntsc = channel->std & V4L2_STD_525_60;
+	const struct s2255_fmt *fmt;
+
+	if (fe->index >= NUM_SIZE_ENUMS)
+		return -EINVAL;
+
+	fmt = format_by_fourcc(fe->pixel_format);
+	if (fmt == NULL)
+		return -EINVAL;
+	fe->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+	fe->discrete = is_ntsc ?  ntsc_sizes[fe->index] : pal_sizes[fe->index];
+	return 0;
+}
+
 static int vidioc_enum_frameintervals(struct file *file, void *priv,
 			    struct v4l2_frmivalenum *fe)
 {
-	int is_ntsc = 0;
+	struct s2255_fh *fh = priv;
+	struct s2255_channel *channel = fh->channel;
+	const struct s2255_fmt *fmt;
+	const struct v4l2_frmsize_discrete *sizes;
+	int is_ntsc = channel->std & V4L2_STD_525_60;
 #define NUM_FRAME_ENUMS 4
 	int frm_dec[NUM_FRAME_ENUMS] = {1, 2, 3, 5};
+	int i;
+
 	if (fe->index >= NUM_FRAME_ENUMS)
 		return -EINVAL;
-	switch (fe->width) {
-	case 640:
-		if (fe->height != 240 && fe->height != 480)
-			return -EINVAL;
-		is_ntsc = 1;
-		break;
-	case 320:
-		if (fe->height != 240)
-			return -EINVAL;
-		is_ntsc = 1;
-		break;
-	case 704:
-		if (fe->height != 288 && fe->height != 576)
-			return -EINVAL;
-		break;
-	case 352:
-		if (fe->height != 288)
-			return -EINVAL;
-		break;
-	default:
+
+	fmt = format_by_fourcc(fe->pixel_format);
+	if (fmt == NULL)
 		return -EINVAL;
-	}
+
+	sizes = is_ntsc ? ntsc_sizes : pal_sizes;
+	for (i = 0; i < NUM_SIZE_ENUMS; i++, sizes++)
+		if (fe->width == sizes->width &&
+		    fe->height == sizes->height)
+			break;
+	if (i == NUM_SIZE_ENUMS)
+		return -EINVAL;
+
 	fe->type = V4L2_FRMIVAL_TYPE_DISCRETE;
 	fe->discrete.denominator = is_ntsc ? 30000 : 25000;
 	fe->discrete.numerator = (is_ntsc ? 1001 : 1000) * frm_dec[fe->index];
@@ -1813,6 +1841,7 @@ static const struct v4l2_ioctl_ops s2255_ioctl_ops = {
 	.vidioc_g_jpegcomp = vidioc_g_jpegcomp,
 	.vidioc_s_parm = vidioc_s_parm,
 	.vidioc_g_parm = vidioc_g_parm,
+	.vidioc_enum_framesizes = vidioc_enum_framesizes,
 	.vidioc_enum_frameintervals = vidioc_enum_frameintervals,
 	.vidioc_log_status  = v4l2_ctrl_log_status,
 	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
-- 
1.7.10.4

[REVIEW PATCH 10/11] s2255: choose YUYV as the default format, not YUV422P

[Hans Verkuil]

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

The planar YUV422P is quite unusual and few if any applications support it.
Instead choose the common YUYV format as the default.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/s2255/s2255drv.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 59d40e6..cd06f3c 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -416,11 +416,6 @@ MODULE_DEVICE_TABLE(usb, s2255_table);
 /* JPEG formats must be defined last to support jpeg_enable parameter */
 static const struct s2255_fmt formats[] = {
 	{
-		.name = "4:2:2, planar, YUV422P",
-		.fourcc = V4L2_PIX_FMT_YUV422P,
-		.depth = 16
-
-	}, {
 		.name = "4:2:2, packed, YUYV",
 		.fourcc = V4L2_PIX_FMT_YUYV,
 		.depth = 16
@@ -430,6 +425,11 @@ static const struct s2255_fmt formats[] = {
 		.fourcc = V4L2_PIX_FMT_UYVY,
 		.depth = 16
 	}, {
+		.name = "4:2:2, planar, YUV422P",
+		.fourcc = V4L2_PIX_FMT_YUV422P,
+		.depth = 16
+
+	}, {
 		.name = "8bpp GREY",
 		.fourcc = V4L2_PIX_FMT_GREY,
 		.depth = 8
-- 
1.7.10.4

[REVIEW PATCH 11/11] s2255: fix big-endian support.

[Hans Verkuil]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/s2255/s2255drv.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index cd06f3c..2bd8613 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -522,7 +522,7 @@ static void s2255_timer(unsigned long user_data)
 
 
 /* this loads the firmware asynchronously.
-   Originally this was done synchroously in probe.
+   Originally this was done synchronously in probe.
    But it is better to load it asynchronously here than block
    inside the probe function. Blocking inside probe affects boot time.
    FW loading is triggered by the timer in the probe function
@@ -1157,6 +1157,8 @@ static int s2255_set_mode(struct s2255_channel *channel,
 	__le32 *buffer;
 	unsigned long chn_rev;
 	struct s2255_dev *dev = to_s2255_dev(channel->vdev.v4l2_dev);
+	int i;
+
 	chn_rev = G_chnmap[channel->idx];
 	dprintk(3, "%s channel: %d\n", __func__, channel->idx);
 	/* if JPEG, set the quality */
@@ -1179,7 +1181,8 @@ static int s2255_set_mode(struct s2255_channel *channel,
 	buffer[0] = IN_DATA_TOKEN;
 	buffer[1] = (__le32) cpu_to_le32(chn_rev);
 	buffer[2] = CMD_SET_MODE;
-	memcpy(&buffer[3], &channel->mode, sizeof(struct s2255_mode));
+	for (i = 0; i < sizeof(struct s2255_mode) / sizeof(u32); i++)
+		buffer[3 + i] = cpu_to_le32(((u32 *)&channel->mode)[i]);
 	channel->setmode_ready = 0;
 	res = s2255_write_config(dev->udev, (unsigned char *)buffer, 512);
 	if (debug)
@@ -2511,7 +2514,7 @@ static int s2255_probe(struct usb_interface *interface,
 		return -ENOMEM;
 	}
 	atomic_set(&dev->num_channels, 0);
-	dev->pid = id->idProduct;
+	dev->pid = le16_to_cpu(id->idProduct);
 	dev->fw_data = kzalloc(sizeof(struct s2255_fw), GFP_KERNEL);
 	if (!dev->fw_data)
 		goto errorFWDATA1;
@@ -2581,7 +2584,7 @@ static int s2255_probe(struct usb_interface *interface,
 		/* make sure firmware is the latest */
 		__le32 *pRel;
 		pRel = (__le32 *) &dev->fw_data->fw->data[fw_size - 4];
-		printk(KERN_INFO "s2255 dsp fw version %x\n", *pRel);
+		printk(KERN_INFO "s2255 dsp fw version %x\n", le32_to_cpu(*pRel));
 		dev->dsp_fw_ver = le32_to_cpu(*pRel);
 		if (dev->dsp_fw_ver < S2255_CUR_DSP_FWVER)
 			printk(KERN_INFO "s2255: f2255usb.bin out of date.\n");
-- 
1.7.10.4

Re: [REVIEW PATCH 01/11] s2255: convert to the control framework.

[Sylwester Nawrocki]

Hi Hans,

On 02/26/2013 06:35 PM, Hans Verkuil wrote:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index dcd6374..f6ba2fc 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -146,6 +146,10 @@ enum v4l2_colorfx {
>    * of controls. We reserve 16 controls for this driver. */
>   #define V4L2_CID_USER_MEYE_BASE			(V4L2_CID_USER_BASE + 0x1000)

I couldn't find a patch adding this hunk in my e-mail archive so I'm
commenting here. Shouldn't V4L2_CID_USER_MEYE_BASE start at a higher value,
e.g. (V4L2_CID_USER_BASE + 0x1010) to account for drivers that already
use private controls ? There is couple of them with a few control IDs
starting at V4L2_CID_USER_BASE.

$ git grep V4L2_CID_USER_BASE

drivers/media/i2c/mt9p031.c:#define V4L2_CID_BLC_AUTO 
(V4L2_CID_USER_BASE | 0x1002)
drivers/media/i2c/mt9p031.c:#define V4L2_CID_BLC_TARGET_LEVEL 
(V4L2_CID_USER_BASE | 0x1003)
drivers/media/i2c/mt9p031.c:#define V4L2_CID_BLC_ANALOG_OFFSET 
(V4L2_CID_USER_BASE | 0x1004)
drivers/media/i2c/mt9p031.c:#define V4L2_CID_BLC_DIGITAL_OFFSET 
(V4L2_CID_USER_BASE | 0x1005)
drivers/media/i2c/mt9t001.c:#define V4L2_CID_TEST_PATTERN_COLOR 
(V4L2_CID_USER_BASE | 0x1001)
drivers/media/i2c/mt9t001.c:#define V4L2_CID_BLACK_LEVEL_AUTO 
(V4L2_CID_USER_BASE | 0x1002)
drivers/media/i2c/mt9t001.c:#define V4L2_CID_BLACK_LEVEL_OFFSET 
(V4L2_CID_USER_BASE | 0x1003)
drivers/media/i2c/mt9t001.c:#define V4L2_CID_BLACK_LEVEL_CALIBRATE 
(V4L2_CID_USER_BASE | 0x1004)
drivers/media/i2c/mt9v032.c:#define V4L2_CID_TEST_PATTERN_COLOR 
(V4L2_CID_USER_BASE | 0x1001)
drivers/media/platform/mem2mem_testdev.c:#define 
V4L2_CID_TRANS_TIME_MSEC       (V4L2_CID_USER_BASE + 0x1000)
drivers/media/platform/mem2mem_testdev.c:#define V4L2_CID_TRANS_NUM_BUFS 
                (V4L2_CID_USER_BASE + 0x1001)
drivers/media/platform/vivi.c:#define VIVI_CID_CUSTOM_BASE 
(V4L2_CID_USER_BASE | 0xf000)
drivers/media/usb/cpia2/cpia2_v4l.c:#define CPIA2_CID_USB_ALT 
(V4L2_CID_USER_BASE | 0xf000)
drivers/media/usb/pwc/pwc-v4l.c:#define PWC_CID_CUSTOM(ctrl) 
((V4L2_CID_USER_BASE | 0xf000) + custom_ ## ctrl)
drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h:#define 
VPFE_ISIF_CID_CRGAIN             (V4L2_CID_USER_BASE | 0xa001)
drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h:#define 
VPFE_ISIF_CID_CGRGAIN            (V4L2_CID_USER_BASE | 0xa002)
drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h:#define 
VPFE_ISIF_CID_CGBGAIN            (V4L2_CID_USER_BASE | 0xa003)
drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h:#define 
VPFE_ISIF_CID_CBGAIN             (V4L2_CID_USER_BASE | 0xa004)
drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h:#define 
VPFE_ISIF_CID_GAIN_OFFSET        (V4L2_CID_USER_BASE | 0xa005)
drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h:#define 
VPFE_CID_DPCM_PREDICTOR          (V4L2_CID_USER_BASE | 0xa006)
include/uapi/linux/v4l2-controls.h:#define V4L2_CID_USER_BASE 
V4L2_CID_BASE
include/uapi/linux/v4l2-controls.h:#define V4L2_CID_USER_MEYE_BASE 
                 (V4L2_CID_USER_BASE + 0x1000)

And also

$ git grep V4L2_CTRL_CLASS_CAMERA

drivers/media/i2c/mt9t001.c:#define V4L2_CID_GAIN_RED 
(V4L2_CTRL_CLASS_CAMERA | 0x1001)
drivers/media/i2c/mt9t001.c:#define V4L2_CID_GAIN_GREEN_RED 
(V4L2_CTRL_CLASS_CAMERA | 0x1002)
drivers/media/i2c/mt9t001.c:#define V4L2_CID_GAIN_GREEN_BLUE 
(V4L2_CTRL_CLASS_CAMERA | 0x1003)
drivers/media/i2c/mt9t001.c:#define V4L2_CID_GAIN_BLUE 
(V4L2_CTRL_CLASS_CAMERA | 0x1004)
drivers/media/i2c/s5k6aa.c:#define V4L2_CID_RED_GAIN 
(V4L2_CTRL_CLASS_CAMERA | 0x1001)
drivers/media/i2c/s5k6aa.c:#define V4L2_CID_GREEN_GAIN 
(V4L2_CTRL_CLASS_CAMERA | 0x1002)
drivers/media/i2c/s5k6aa.c:#define V4L2_CID_BLUE_GAIN 
(V4L2_CTRL_CLASS_CAMERA | 0x1003)

> +/* The base for the s2255 driver controls.
> + * We reserve 8 controls for this driver. */
> +#define V4L2_CID_USER_S2255_BASE		(V4L2_CID_USER_BASE + 0x1010)
> +

--

Regards,
Sylwester

Re: [REVIEW PATCH 01/11] s2255: convert to the control framework.

[Hans Verkuil]

On Tue 26 February 2013 23:21:19 Sylwester Nawrocki wrote:
> Hi Hans,
> 
> On 02/26/2013 06:35 PM, Hans Verkuil wrote:
> > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > index dcd6374..f6ba2fc 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -146,6 +146,10 @@ enum v4l2_colorfx {
> >    * of controls. We reserve 16 controls for this driver. */
> >   #define V4L2_CID_USER_MEYE_BASE			(V4L2_CID_USER_BASE + 0x1000)
> 
> I couldn't find a patch adding this hunk in my e-mail archive so I'm
> commenting here. Shouldn't V4L2_CID_USER_MEYE_BASE start at a higher value,
> e.g. (V4L2_CID_USER_BASE + 0x1010) to account for drivers that already
> use private controls ? There is couple of them with a few control IDs
> starting at V4L2_CID_USER_BASE.

Private controls always had overlapping IDs. During one of the mini-summits
last year we decided to change that so they all had their own ID. The meye
driver is one of the first to have a proper range defined, eventually all
other drivers that have private controls will be added there. That includes
those you found with grep.

So give me time and it will all be fixed :-)

Regards,

	Hans

> 
> $ git grep V4L2_CID_USER_BASE
> 
> drivers/media/i2c/mt9p031.c:#define V4L2_CID_BLC_AUTO 
> (V4L2_CID_USER_BASE | 0x1002)
> drivers/media/i2c/mt9p031.c:#define V4L2_CID_BLC_TARGET_LEVEL 
> (V4L2_CID_USER_BASE | 0x1003)
> drivers/media/i2c/mt9p031.c:#define V4L2_CID_BLC_ANALOG_OFFSET 
> (V4L2_CID_USER_BASE | 0x1004)
> drivers/media/i2c/mt9p031.c:#define V4L2_CID_BLC_DIGITAL_OFFSET 
> (V4L2_CID_USER_BASE | 0x1005)
> drivers/media/i2c/mt9t001.c:#define V4L2_CID_TEST_PATTERN_COLOR 
> (V4L2_CID_USER_BASE | 0x1001)
> drivers/media/i2c/mt9t001.c:#define V4L2_CID_BLACK_LEVEL_AUTO 
> (V4L2_CID_USER_BASE | 0x1002)
> drivers/media/i2c/mt9t001.c:#define V4L2_CID_BLACK_LEVEL_OFFSET 
> (V4L2_CID_USER_BASE | 0x1003)
> drivers/media/i2c/mt9t001.c:#define V4L2_CID_BLACK_LEVEL_CALIBRATE 
> (V4L2_CID_USER_BASE | 0x1004)
> drivers/media/i2c/mt9v032.c:#define V4L2_CID_TEST_PATTERN_COLOR 
> (V4L2_CID_USER_BASE | 0x1001)
> drivers/media/platform/mem2mem_testdev.c:#define 
> V4L2_CID_TRANS_TIME_MSEC       (V4L2_CID_USER_BASE + 0x1000)
> drivers/media/platform/mem2mem_testdev.c:#define V4L2_CID_TRANS_NUM_BUFS 
>                 (V4L2_CID_USER_BASE + 0x1001)
> drivers/media/platform/vivi.c:#define VIVI_CID_CUSTOM_BASE 
> (V4L2_CID_USER_BASE | 0xf000)
> drivers/media/usb/cpia2/cpia2_v4l.c:#define CPIA2_CID_USB_ALT 
> (V4L2_CID_USER_BASE | 0xf000)
> drivers/media/usb/pwc/pwc-v4l.c:#define PWC_CID_CUSTOM(ctrl) 
> ((V4L2_CID_USER_BASE | 0xf000) + custom_ ## ctrl)
> drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h:#define 
> VPFE_ISIF_CID_CRGAIN             (V4L2_CID_USER_BASE | 0xa001)
> drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h:#define 
> VPFE_ISIF_CID_CGRGAIN            (V4L2_CID_USER_BASE | 0xa002)
> drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h:#define 
> VPFE_ISIF_CID_CGBGAIN            (V4L2_CID_USER_BASE | 0xa003)
> drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h:#define 
> VPFE_ISIF_CID_CBGAIN             (V4L2_CID_USER_BASE | 0xa004)
> drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h:#define 
> VPFE_ISIF_CID_GAIN_OFFSET        (V4L2_CID_USER_BASE | 0xa005)
> drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h:#define 
> VPFE_CID_DPCM_PREDICTOR          (V4L2_CID_USER_BASE | 0xa006)
> include/uapi/linux/v4l2-controls.h:#define V4L2_CID_USER_BASE 
> V4L2_CID_BASE
> include/uapi/linux/v4l2-controls.h:#define V4L2_CID_USER_MEYE_BASE 
>                  (V4L2_CID_USER_BASE + 0x1000)
> 
> And also
> 
> $ git grep V4L2_CTRL_CLASS_CAMERA
> 
> drivers/media/i2c/mt9t001.c:#define V4L2_CID_GAIN_RED 
> (V4L2_CTRL_CLASS_CAMERA | 0x1001)
> drivers/media/i2c/mt9t001.c:#define V4L2_CID_GAIN_GREEN_RED 
> (V4L2_CTRL_CLASS_CAMERA | 0x1002)
> drivers/media/i2c/mt9t001.c:#define V4L2_CID_GAIN_GREEN_BLUE 
> (V4L2_CTRL_CLASS_CAMERA | 0x1003)
> drivers/media/i2c/mt9t001.c:#define V4L2_CID_GAIN_BLUE 
> (V4L2_CTRL_CLASS_CAMERA | 0x1004)
> drivers/media/i2c/s5k6aa.c:#define V4L2_CID_RED_GAIN 
> (V4L2_CTRL_CLASS_CAMERA | 0x1001)
> drivers/media/i2c/s5k6aa.c:#define V4L2_CID_GREEN_GAIN 
> (V4L2_CTRL_CLASS_CAMERA | 0x1002)
> drivers/media/i2c/s5k6aa.c:#define V4L2_CID_BLUE_GAIN 
> (V4L2_CTRL_CLASS_CAMERA | 0x1003)
> 
> > +/* The base for the s2255 driver controls.
> > + * We reserve 8 controls for this driver. */
> > +#define V4L2_CID_USER_S2255_BASE		(V4L2_CID_USER_BASE + 0x1010)
> > +
> 
> --
> 
> Regards,
> Sylwester
> 

Re: [REVIEW PATCH 01/11] s2255: convert to the control framework.

[Sylwester Nawrocki]

On 02/27/2013 10:05 AM, Hans Verkuil wrote:
> On Tue 26 February 2013 23:21:19 Sylwester Nawrocki wrote:
>> On 02/26/2013 06:35 PM, Hans Verkuil wrote:
[...]
> Private controls always had overlapping IDs. During one of the mini-summits
> last year we decided to change that so they all had their own ID. The meye
> driver is one of the first to have a proper range defined, eventually all
> other drivers that have private controls will be added there. That includes
> those you found with grep.
>
> So give me time and it will all be fixed :-)

Ah, that's the whole plan then! It indeed makes sense. I just thought 
only new
drivers would use fixed not overlapping control IDs. But having the existing
ones fixed as well sounds a way better.

Regards,
Sylwester