Skip to contentSkip to conversation

V4L: stk11xx, add a new webcam driver

PATCH In review Jiri Slaby 8 participants 33 messages May 24, 2007 – Jun 19, 2007 2 messages not in the archive Media (V4L/DVB)

lore
Share & tools
lore

Jiri Slaby

author
#0 lore raw

PATCH 1/1 V4L: stk11xx, add a new webcam driver

Well, no objections on v4l list, try to merge it. Any further comments will be appreciated.

--

stk11xx, add a new webcam driver

Cc Mauro Carvalho Chehab <[email protected]>

---
commit 1f023432798b6e6fd52ad81af0eab250e1a71449
tree 4045ace3523a78ae2b5bff194ed9c10945b27171
parent 119ce83eb190c70d50ed7558c7b67b400ea00ec7
author Jiri Slaby <[email protected]> Thu, 24 May 2007 15:56:41 +0200 committer Jiri Slaby <[email protected]> Thu, 24 May 2007 15:56:41 +0200

 drivers/media/video/Kconfig   |    8 
 drivers/media/video/Makefile  |    2 
 drivers/media/video/stk11xx.c | 3608 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 3618 insertions(+), 0 deletions(-)
drivers/media/video/Kconfig +8 -0
index 070f4b1..cbab1ed 100644
@@ -792,6 +792,14 @@ config USB_ZR364XX
 	  To compile this driver as a module, choose M here: the
 	  module will be called zr364xx.
 
+config USB_STK11XX
+	tristate "STK11XX based webcams"
+	depends on VIDEO_V4L2
+	---help---
+	  This will add support for Syntek webcams such as dc1125 and stk1135.
+
+	  If you choose to build it as module, it will be called stk11xx.
+
 endif # V4L_USB_DRIVERS
 
 endif # VIDEO_CAPTURE_DRIVERS
drivers/media/video/Makefile +2 -0
index 3202e87..409125d 100644
@@ -106,6 +106,8 @@ obj-$(CONFIG_USB_STV680)        += stv680.o
 obj-$(CONFIG_USB_W9968CF)       += w9968cf.o
 obj-$(CONFIG_USB_ZR364XX)       += zr364xx.o
 
+obj-$(CONFIG_USB_STK11XX)	+= stk11xx.o
+
 obj-$(CONFIG_USB_SN9C102)       += sn9c102/
 obj-$(CONFIG_USB_ET61X251)      += et61x251/
 obj-$(CONFIG_USB_PWC)           += pwc/
drivers/media/video/stk11xx.c new file +3608 -0

2,978 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Part of the diff is not shown here. View the raw message.

#1 lore raw

Hi Jiri,

Quoted 310 lines · Jiri Slaby

Well, no objections on v4l list, try to merge it. Any further comments will be
appreciated.

--

stk11xx, add a new webcam driver

Cc Mauro Carvalho Chehab <[email protected]>

---
commit 1f023432798b6e6fd52ad81af0eab250e1a71449
tree 4045ace3523a78ae2b5bff194ed9c10945b27171
parent 119ce83eb190c70d50ed7558c7b67b400ea00ec7
author Jiri Slaby <[email protected]> Thu, 24 May 2007 15:56:41 +0200 committer Jiri Slaby <[email protected]> Thu, 24 May 2007 15:56:41 +0200

 drivers/media/video/Kconfig   |    8
 drivers/media/video/Makefile  |    2
 drivers/media/video/stk11xx.c | 3608
+++++++++++++++++++++++++++++++++++++++++
 3 files changed, 3618 insertions(+), 0 deletions(-)
drivers/media/video/Kconfig +8 -0
index 070f4b1..cbab1ed 100644
@@ -792,6 +792,14 @@ config USB_ZR364XX
 	  To compile this driver as a module, choose M here: the
 	  module will be called zr364xx.
 
+config USB_STK11XX
+	tristate "STK11XX based webcams"
+	depends on VIDEO_V4L2
+	---help---
+	  This will add support for Syntek webcams such as dc1125 and stk1135.
+
+	  If you choose to build it as module, it will be called stk11xx.
+
 endif # V4L_USB_DRIVERS
 
 endif # VIDEO_CAPTURE_DRIVERS
drivers/media/video/Makefile +2 -0
index 3202e87..409125d 100644
@@ -106,6 +106,8 @@ obj-$(CONFIG_USB_STV680)        += stv680.o
 obj-$(CONFIG_USB_W9968CF)       += w9968cf.o
 obj-$(CONFIG_USB_ZR364XX)       += zr364xx.o
 
+obj-$(CONFIG_USB_STK11XX)	+= stk11xx.o
+
 obj-$(CONFIG_USB_SN9C102)       += sn9c102/
 obj-$(CONFIG_USB_ET61X251)      += et61x251/
 obj-$(CONFIG_USB_PWC)           += pwc/
drivers/media/video/stk11xx.c new file +136 -0
new file mode 100644
index 0000000..53c36cb
@@ -0,0 +1,3608 @@
+/*
+ * Driver for Syntek USB video camera
+ *
+ * Copyright (C) Nicolas VIVIEN
+ * Copyright (c) 2007 Jiri Slaby <[email protected]>
+ *
+ * Licences
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+#include <linux/usb.h>
+#include <media/v4l2-common.h>
+
+#define DRIVER_DESC			"Syntek USB Video Camera"
+#define DRIVER_VERSION			"v0.0.1"
+#define DRIVER_VERSION_NUM		0x000001
+
+#define USB_SYNTEK1_VENDOR_ID		0x174F
+#define USB_SYNTEK2_VENDOR_ID		0x05E1
+
+#define USB_STK1125_PRODUCT_ID		0xa311
+#define USB_STK1135_PRODUCT_ID		0xa821
+#define USB_DC1125_PRODUCT_ID		0x0501
+
+typedef enum {
+	SYNTEK_DC1125 = 1,
+	SYNTEK_STK1135 = 2
+} T_SYNTEK_DEVICE;
+
+/**
+ * @def MAX_ISO_BUFS
+ *   Number maximal of ISOC buffers
+ *
+ * @def ISO_FRAMES_PER_DESC
+ *   Number frames per ISOC descriptor
+ *
+ * @def ISO_MAX_FRAME_SIZE
+ *   Maximale size of frame
+ *
+ * @def ISO_BUFFER_SIZE
+ *   Maximal size of buffer
+ */
+#define MAX_ISO_BUFS			16 /* 2 */
+#define ISO_FRAMES_PER_DESC		10
+#define ISO_MAX_FRAME_SIZE		3 * 1024
+#define ISO_BUFFER_SIZE			(ISO_FRAMES_PER_DESC * ISO_MAX_FRAME_SIZE)
+
+/**
+ * @def STK11XX_MAX_IMAGES
+ *   Absolute maximum number of buffers available for mmap()
+ *
+ * @def STK11XX_FRAME_SIZE
+ *   Maximum size after decompression
+ */
+#define STK11XX_MAX_IMAGES		10
+#define STK11XX_FRAME_SIZE		(1280 * 1024 * 3)
+
+#ifndef CONFIG_STK11XX_DEBUG_STREAM
+#define CONFIG_STK11XX_DEBUG_STREAM	0
+#endif
+
+#if CONFIG_STK11XX_DEBUG_STREAM
+#define STK_STREAM(str, args...)	printk(KERN_DEBUG "stk11xx: " str, \
+									##args)
+#else
+#define STK_STREAM(str, args...)	do { } while(0)
+#endif
+
+enum {
+	STK11XX_80x60,
+	STK11XX_160x120,
+	STK11XX_320x240,
+	STK11XX_640x480,
+	STK11XX_800x600,
+	STK11XX_1024x758,
+	STK11XX_1280x1024,
+	STK11XX_NBR_SIZES
+};
+
+struct stk11xx_iso_buf {
+	void *data;
+	int length;
+	int read;
+	struct urb *urb;
+};
+
+struct stk11xx_frame_buf {
+	void *data;
+	volatile int filled;
+	struct stk11xx_frame_buf *next;
+};
+
+struct stk11xx_image_buf {
+	unsigned long offset;
+	int vma_use_count;
+};
+
+struct stk11xx_coord {
+	int x;
+	int y;
+};
+
+struct stk11xx_video {
+	int fps;
+	int brightness;
+	int contrast;
+	int whiteness;
+	u32 pixformat;
+};
+
+struct stk11xx {
+	struct video_device *vdev;
+	struct usb_device *udev;
+
+	int webcam_model;
+
+	unsigned char *int_in_buffer;	/**< Interrupt IN buffer */
+	size_t int_in_size;		/**< Interrupt IN buffer size */
+	u8 int_in_endpointAddr;		/**< Interrupt IN endpoint address */
+
+	size_t isoc_in_size;		/**< Isochrone IN size */
+	u8 isoc_in_endpointAddr;	/**< Isochrone IN endpoint address */
+
+	int watchdog;		/**< Counter for the software watchdog */
+
+	struct stk11xx_video vsettings;		/**< Video settings (brightness,

whiteness...) */

drivers/media/video/stk11xx.c new file +43 -0
+
+	int error_status;
+
+	struct mutex open_lock;
+	unsigned int vopen;
+	int visoc_errors;
+	int vframes_error;
+	int vlast_packet_size;
+	char vsync;
+
+	spinlock_t spinlock;
+	wait_queue_head_t wait_frame;
+
+	/* 1: isoc */
+	char isoc_init_ok;
+	struct stk11xx_iso_buf isobuf[MAX_ISO_BUFS];
+
+	/* 2: frame */
+	int frame_size;
+	struct stk11xx_frame_buf *framebuf;
+	struct stk11xx_frame_buf *empty_frames, *empty_frames_tail;
+	struct stk11xx_frame_buf *full_frames, *full_frames_tail;
+	struct stk11xx_frame_buf *fill_frame;
+	struct stk11xx_frame_buf *read_frame;
+
+	/* 3: decompression */
+	void *decompress_data;
+	
+	/* 4: image */
+	int image_size;
+	void *image_data;
+	struct stk11xx_image_buf images[STK11XX_MAX_IMAGES];
+	u32 image_f[STK11XX_MAX_IMAGES];
+	unsigned int nbuffers;
+	unsigned int len_per_image;
+	int image_read_pos;
+	int fill_image;
+	int resolution;
+	struct stk11xx_coord view;
+	struct stk11xx_coord image;
+};
+
+static const struct stk11xx_coord stk11xx_image_sizes[STK11XX_NBR_SIZES] =

{

drivers/media/video/stk11xx.c new file +17 -0
+	{   80,   60 },
+	{  160,  120 },
+	{  320,  240 },
+	{  640,  480 },
+	{  800,  600 },
+	{ 1024,  758 },
+	{ 1280, 1024 }
+};
+
+static int default_nbrframebuf = 3;	/* Number of frame buffer by default */
+static int default_fps = 10;
+
+static int fps;
+module_param(fps, int, 0444);
+MODULE_PARM_DESC(fps, "Frames per second [5-30]");
+
+static int stk11xx_read_registry(struct stk11xx *dev, u16 index, int

*value)

drivers/media/video/stk11xx.c new file +49 -0
+{
+	struct usb_device *udev = dev->udev;
+	int result;
+
+	*value = 0;
+
+	result = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), 0x00,
+			USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, 0x00,
+			index, (u8 *)value, sizeof(u8), 500);
+
+	if (result < 0)
+		dev_err(&udev->dev, "Read registry fails %02X\n", index);
+
+	return result;
+}
+
+static int stk11xx_write_registry(struct stk11xx *dev, u16 index,
+			       u16 value)
+{
+	struct usb_device *udev = dev->udev;
+	int result;
+
+	result = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x01,
+			USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE, value,
+			index, NULL, 0, 500);
+
+	if (result < 0)
+		dev_err(&udev->dev, "Write registry fails %02X = %02X\n", index,
+				value);
+
+	return result;
+}
+
+static int stk11xx_set_feature(struct stk11xx *dev, int index)
+{
+	struct usb_device *udev = dev->udev;
+	int result;
+
+	result = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+				 USB_REQ_SET_FEATURE,
+				 USB_TYPE_STANDARD | USB_DIR_OUT |
+				 USB_RECIP_DEVICE, USB_DEVICE_REMOTE_WAKEUP,
+				 index, NULL, 0, 500);
+
+	if (result < 0)
+		dev_err(&udev->dev, "SET FEATURE fail !\n");
+
+	return result;
+}

drivers/media/video/stk11xx.c line 255

Quoted 10 lines
drivers/media/video/stk11xx.c new file +10 -0
+
+/*
+ * Bayer conversion
+ */
+
+#define STK11XX_AVG2(x,y) (u8)(((int)x + (int)y) / 2)
+#define STK11XX_AVG4(a,b,c,d) (u8)(((int)a + (int)b + (int)c + (int)d) / 4)
+
+static void stk11xx_bayer_to_rgb(u8 *rgb, const u8 *bayer,
+			  const int width, const int height)

hmm.. this is probably to support xawtv/tvtime?
It would be better to do that in userspace, but the argument which seems to be against it is that userspace applications often don't support bayer.

Quoted 199 lines
drivers/media/video/stk11xx.c new file +199 -0
+{
+	unsigned int i, j, x, y;
+	int rpos, wpos, ltpos, lbpos;
+	int runlength = width > height ? width : height;
+	int lastrow = ((height - 1) * width) * 3;
+	int lastcolumn = (width - 1) * 3;
+
+	/* blit out initial data */
+	for (j = 0, y = 0; y < height; y++) {
+		for (i = 0, x = 0; x < width; x++) {
+			if (y & 1) {
+				/* GBGBGB line */
+				if (x & 1) {	/* blue */
+					rgb[j * width * 3 + i * 3 + 2] =
+					    bayer[(height - y) * width + x];
+				} else {	/* green */
+					rgb[j * width * 3 + i * 3 + 1] =
+					    bayer[(height - y) * width + x];
+				}
+			} else {
+				/* RGRGRG line */
+				if (x & 1) {	/* green */
+					rgb[j * width * 3 + i * 3 + 1] =
+					    bayer[(height - y) * width + x];
+				} else {	/* red */
+					rgb[j * width * 3 + i * 3 + 0] =
+					    bayer[(height - y) * width + x];
+				}
+			}
+
+			i++;
+		}
+
+		j++;
+	}
+
+	/* blit in everything but the borders */
+	for (y = 1; y < (height - 1); y++) {
+		if (y & 1) {
+			/* GBGBGB line */
+			for (x = 1; x < (width - 1); x++) {
+				rpos = (height - y) * width + x;
+				wpos = y * width + x;
+				ltpos = rpos - width;
+				lbpos = rpos + width;
+
+				if (x & 1) {	/* blue is known */
+					/* calculate red */
+					rgb[(wpos) * 3 + 0] = STK11XX_AVG4(
+						/*Rtl */ bayer[ltpos - 1],
+						/*Rtr */ bayer[ltpos + 1],
+						/*Rbl */ bayer[lbpos - 1],
+						/*Rbr */ bayer[lbpos + 1]);
+					/* calculate green */
+					rgb[(wpos) * 3 + 1] = STK11XX_AVG4(
+						/*Gt */ bayer[ltpos],
+						/*Gl */ bayer[rpos - 1],
+						/*Gb */ bayer[lbpos],
+						/*Gr */ bayer[rpos + 1]);
+				} else {	/* green is known */
+					/* calculate red */
+					rgb[(wpos) * 3 + 0] = STK11XX_AVG2(
+						/*Rt */ bayer[ltpos],
+						/*Rb */ bayer[lbpos]);
+					/* calculate blue */
+					rgb[(wpos) * 3 + 2] = STK11XX_AVG2(
+						/*Bl */ bayer[rpos - 1],
+						/*Br */ bayer[rpos + 1]);
+				}
+			}
+		} else {
+			/* RGRG line */
+			for (x = 1; x < (width - 1); x++) {
+				rpos = (height - y) * width + x;
+				wpos = y * width + x;
+				ltpos = rpos - width;
+				lbpos = rpos + width;
+
+				if (x & 1) {	/* green is known */
+					/* calculate red */
+					rgb[(wpos) * 3 + 0] = STK11XX_AVG2(
+						/*Rl */ bayer[rpos - 1],
+						/*Rr */ bayer[rpos + 1]);
+					/* calculate blue */
+					rgb[(wpos) * 3 + 2] = STK11XX_AVG2(
+						/*Bt */ bayer[ltpos],
+						/*Bb */ bayer[lbpos]);
+
+				} else {	/* red is known */
+					/* calculate blue */
+					rgb[(wpos) * 3 + 2] = STK11XX_AVG4(
+						/*Btl */ bayer[ltpos - 1],
+						/*Btr */ bayer[ltpos + 1],
+						/*Bbl */ bayer[lbpos - 1],
+						/*Bbr */ bayer[lbpos + 1]);
+					/* calculate green */
+					rgb[(wpos) * 3 + 1] = STK11XX_AVG4(
+						/*Gt */ bayer[ltpos],
+						/*Gl */ bayer[rpos - 1],
+						/*Gb */ bayer[lbpos],
+						/*Gr */ bayer[rpos + 1]);
+				}
+			}
+		}
+	}
+
+	for (i = 0; i < runlength; i++) {
+		if (i < width) {
+			/* top border: we have certain red and green pixels */
+			if ((i & 1) == 1) {
+				/* red, we use avg4(bot, bot, l-bot, l) */
+				rgb[i * 3] = STK11XX_AVG4(
+					rgb[(i + width) * 3],
+					rgb[(i + width) * 3],
+					rgb[(i + width - 1) * 3],
+					rgb[(i - 1) * 3]);
+			} else {
+				/* green, we use avg4(bot, bot, r-bot, r) */
+				rgb[i * 3 + 1] = STK11XX_AVG4(
+					rgb[(i + width) * 3 + 1],
+					rgb[(i + width) * 3 + 1],
+					rgb[(i + width + 1) * 3 + 1],
+					rgb[(i + 1) * 3 + 1]);
+			}
+			/* blue has to be calculated for every pixel, we'll
+			   use bottom and one more bottom */
+			rgb[i * 3 + 2] = STK11XX_AVG2(rgb[(i + width) * 3 + 2],
+					rgb[(i + width * 2) * 3 + 2]);
+
+			/* bottom border: we have green and blue pixels for
+			   sure */
+			if ((i & 1) == 1) {
+				/* green, we use avg4(top, top, l-top, l) */
+				rgb[lastrow + i * 3 + 1] = STK11XX_AVG4(
+					rgb[lastrow + (i - width) * 3 + 1],
+					rgb[lastrow + (i - width) * 3 + 1],
+					rgb[lastrow + (i - width - 1) * 3 + 1],
+					rgb[lastrow + (i - 1) * 3 + 1]);
+			} else {
+				/* blue, we use avg4(top, top, r-top, r) */
+				rgb[lastrow + i * 3 + 2] = STK11XX_AVG4(
+					rgb[lastrow + (i - width) * 3 + 2],
+					rgb[lastrow + (i - width) * 3 + 2],
+					rgb[lastrow + (i - width + 1) * 3 + 2],
+					rgb[lastrow + (i + 1) * 3 + 2]);
+			}
+			/* red has to be calculated for every pixel, we'll use
+			   top and another top */
+			rgb[lastrow + i * 3] = STK11XX_AVG2(
+					rgb[lastrow + (i - width) * 3],
+					rgb[lastrow + (i - width * 2) * 3]);
+
+		}
+		if (i < height) {	/* the left and right borders */
+			/* left border */
+			if ((i & 1) == 0) {
+				/* calc green, just avg right and bottom */
+				rgb[(i * width) * 3 + 1] = STK11XX_AVG2(
+					rgb[(i * width + 1) * 3 + 1],
+					rgb[((i + 1) * width) * 3 + 1]);
+				/* calc blue, avg right and right bottom */
+				rgb[(i * width) * 3 + 2] = STK11XX_AVG2(
+					rgb[(i * width + 1) * 3 + 2],
+					rgb[((i + 1) * width + 1) * 3 + 2]);
+			} else {
+				/* calc red, 2x top , 1xtopright, 1xright */
+				rgb[(i * width) * 3] = STK11XX_AVG4(
+					rgb[((i - 1) * width) * 3],
+					rgb[((i - 1) * width) * 3],
+					rgb[((i - 1) * width + 1) * 3],
+					rgb[(i * width + 1) * 3]);
+				/* calc blue, avg top and right */
+				rgb[(i * width) * 3 + 2] = STK11XX_AVG2(
+					rgb[((i - 1) * width) * 3 + 2],
+					rgb[(i * width + 1) * 3 + 2]);
+			}
+			/* right border */
+			if ((i & 1) == 0) {
+				/* calc red, take left and bottom left */
+				rgb[lastcolumn + (i * width) * 3] =STK11XX_AVG2(
+					rgb[lastcolumn + (i * width - 1) * 3],
+					rgb[lastcolumn + ((i+1)*width-1) * 3]);
+				/* calc blue, left and bottom */
+				rgb[lastcolumn + (i * width) * 3 + 2] =
+				    STK11XX_AVG2(
+					rgb[lastcolumn + (i*width - 1) * 3 + 2],
+					rgb[lastcolumn + ((i+1)*width)*3 + 2]);
+			} else {	/* i&1 == 1 */
+				/* calc red, top and left */
+				rgb[lastcolumn + (i * width) * 3] =STK11XX_AVG2(
+					rgb[lastcolumn +((i-1)*width - 1) * 3],
+					rgb[lastcolumn + (i * width - 1) * 3]);
+				/* calc green, 2xtop, topleft, left */
+				rgb[lastcolumn + (i * width) * 3 + 1] =
+				    STK11XX_AVG4(
+					rgb[lastcolumn + ((i-1)*width) * 3 + 1],
+					rgb[lastcolumn + ((i-1)*width) * 3 + 1],
+					rgb[lastcolumn + ((i-1)*width-1)*3 + 1],
+					rgb[lastcolumn + (i*width-1) * 3 + 1]);

drivers/media/video/stk11xx.c line 464

Quoted 10 lines
drivers/media/video/stk11xx.c new file +10 -0
+			}
+
+		}
+
+	}
+
+}
+
+static void stk11xx_resize_image(u8 *out, const u8 *in,
+		unsigned int width, unsigned int height, unsigned int factor)

scaling in kernelspace? is there anything which is against doing that in userspace (where it should be done)?
When using Xvideo this might be done in hardware too without burning any CPU cycles for scaling.

Quoted 14 lines
drivers/media/video/stk11xx.c new file +14 -0
+{
+	unsigned int x, y, nwidth, nheight;
+
+	nheight = height / factor;
+	nwidth = width / factor;
+
+	for (y = 0; y < nheight; y++) {
+		for (x = 0; x < nwidth; x++) {
+			/* R */
+			out[y * nwidth * 3 + x * 3 + 0] =
+			    in[y * factor * width * 3 + x * factor * 3 + 0];
+			/* G */
+			out[y * nwidth * 3 + x * 3 + 1] =
+			    in[y * factor * width * 3 + x * factor * 3 + 1];

drivers/media/video/stk11xx.c line 488

Quoted 10 lines
drivers/media/video/stk11xx.c new file +10 -0
+			/* B */
+			out[y * nwidth * 3 + x * 3 + 2] =
+			    in[y * factor * width * 3 + x * factor * 3 + 2];
+		}
+	}
+}
+
+static void stk11xx_correct_brightness(u8 *rgb, unsigned int width,
+				unsigned int height, int brightness)
+{

same here, why do you want to have that in kernelspace?

Quoted 139 lines
drivers/media/video/stk11xx.c new file +139 -0
+	unsigned int i;
+
+	for (i = 0; i < width * height; i++) {
+		brightness = min_t(int, brightness, min(255 - rgb[i],
+			min(255 - rgb[i + 1], 255 - rgb[i + 2])));
+		brightness = max(brightness, -min_t(int, rgb[i],
+			min(rgb[i + 1], rgb[i + 2])));
+
+		rgb[i] = rgb[i] + brightness;
+		rgb[i + 1] = rgb[i + 1] + brightness;
+		rgb[i + 2] = rgb[i + 2] + brightness;
+	}
+}
+
+static int stk11xx_decompress(struct stk11xx *dev)
+{
+	struct stk11xx_frame_buf *framebuf = dev->read_frame;
+	void *data, *image = dev->image_data;
+	void *decompress = dev->decompress_data;
+	unsigned int width, height, factor;
+
+	if (framebuf == NULL)
+		return -EFAULT;
+
+	image += dev->images[dev->fill_image].offset;
+
+	data = framebuf->data;
+
+	switch (dev->resolution) {
+	case STK11XX_80x60:
+		factor = 8;
+		width = stk11xx_image_sizes[STK11XX_640x480].x;
+		height = stk11xx_image_sizes[STK11XX_640x480].y;
+		break;
+
+	case STK11XX_160x120:
+		factor = 4;
+		width = stk11xx_image_sizes[STK11XX_640x480].x;
+		height = stk11xx_image_sizes[STK11XX_640x480].y;
+		break;
+
+	case STK11XX_320x240:
+		factor = 2;
+		width = stk11xx_image_sizes[STK11XX_640x480].x;
+		height = stk11xx_image_sizes[STK11XX_640x480].y;
+		break;
+
+	case STK11XX_640x480:
+		factor = 1;
+		width = stk11xx_image_sizes[STK11XX_640x480].x;
+		height = stk11xx_image_sizes[STK11XX_640x480].y;
+		break;
+
+	case STK11XX_800x600:
+		factor = 1;
+		width = stk11xx_image_sizes[STK11XX_1280x1024].x;
+		height = stk11xx_image_sizes[STK11XX_1280x1024].y;
+		break;
+
+	case STK11XX_1024x758:
+		factor = 1;
+		width = stk11xx_image_sizes[STK11XX_1280x1024].x;
+		height = stk11xx_image_sizes[STK11XX_1280x1024].y;
+		break;
+
+	case STK11XX_1280x1024:
+		factor = 1;
+		width = stk11xx_image_sizes[STK11XX_1280x1024].x;
+		height = stk11xx_image_sizes[STK11XX_1280x1024].y;
+		break;
+
+	default:
+		return -EFAULT;
+	}
+
+	stk11xx_bayer_to_rgb(decompress, data, width, height);
+
+	stk11xx_resize_image(image, decompress, width, height, factor);
+
+	stk11xx_correct_brightness(image, width / factor, height / factor,
+				   dev->vsettings.brightness - 200);
+
+	return 0;
+}
+
+/*
+ * Device
+ */
+
+static int stk11xx_setting_camera(struct stk11xx *dev);
+
+/*
+ * called when a frame is ready to prepare the next frame
+ */
+static int stk11xx_next_frame(struct stk11xx *dev)
+{
+	unsigned long flags;
+	int ret = 0;
+
+	STK_STREAM("Select next frame\n");
+
+	spin_lock_irqsave(&dev->spinlock, flags);
+
+	if (dev->fill_frame != NULL) {
+		if (dev->full_frames == NULL) {
+			dev->full_frames = dev->fill_frame;
+			dev->full_frames_tail = dev->full_frames;
+		} else {
+			dev->full_frames_tail->next = dev->fill_frame;
+			dev->full_frames_tail = dev->fill_frame;
+		}
+	}
+
+	if (dev->empty_frames != NULL) {
+		dev->fill_frame = dev->empty_frames;
+		dev->empty_frames = dev->empty_frames->next;
+	} else {
+		if (dev->full_frames == NULL) {
+			dev_err(&dev->udev->dev, "neither empty or full frames "
+					"available!\n");
+			spin_unlock_irqrestore(&dev->spinlock, flags);
+			return -EINVAL;
+		}
+
+		dev->fill_frame = dev->full_frames;
+		dev->full_frames = dev->full_frames->next;
+
+		ret = 1;
+	}
+
+	dev->fill_frame->next = NULL;
+
+	spin_unlock_irqrestore(&dev->spinlock, flags);
+
+	return ret;
+}
+
+/*
+ * This function is called as an URB transfert is complete (Isochronous
Quoted 6 lines

pipe).
+ * So, the traitement is done in interrupt time, so it has be fast, not crash,
+ * ans not stall. Neat.
+ */
+static void stk11xx_isoc_handler(struct urb *urb)

there was an API change, to be compatible with <kernel 2.6.19 you need to add another argument here (the v4l-dvb tree is supposed to be downward compatible)

#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,19)
static void stk11xx_isoc_handler(struct urb *urb, struct pt_regs *regs) #else
static void stk11xx_isoc_handler(struct urb *urb)
#endif

(it's just for compatibility if someone wants to upgrade a 2.6.16 kernel with the latest v4l-dvb tree, it will get parsed out for upstream)

Quoted 608 lines
drivers/media/video/stk11xx.c new file +608 -0
+{
+	struct stk11xx *dev = urb->context;
+	struct stk11xx_frame_buf *framebuf;
+	unsigned char *fill = NULL, *iso_buf = NULL;
+	unsigned int i;
+	int ret, skip, awake = 0, framestatus, framelen;
+
+	STK_STREAM("Isoc handler\n");
+
+	if (dev == NULL) {
+		dev_err(&dev->udev->dev, "isoc_handler called with NULL "
+				"device\n");
+		return;
+	}
+
+	if (urb->status == -ENOENT || urb->status == -ECONNRESET) {
+		dev_dbg(&dev->udev->dev, "URB unlinked synchronuously\n");
+		return;
+	}
+
+	if (urb->status != -EINPROGRESS && urb->status != 0) {
+		dev_err(&dev->udev->dev, "isoc_handler called with status %d\n",
+				urb->status);
+
+		wake_up_interruptible(&dev->wait_frame);
+
+		urb->dev = dev->udev;
+		ret = usb_submit_urb(urb, GFP_ATOMIC);
+		if (ret != 0)
+			dev_err(&dev->udev->dev, "error (%d) re-submitting urb "
+				"in isoc_handler\n", ret);
+
+		return;
+	}
+
+	framebuf = dev->fill_frame;
+	if (framebuf == NULL) {
+		dev_err(&dev->udev->dev, "isoc_handler without valid fill "
+				"frame\n");
+
+		wake_up_interruptible(&dev->wait_frame);
+
+		urb->dev = dev->udev;
+		ret = usb_submit_urb(urb, GFP_ATOMIC);
+
+		if (ret != 0)
+			dev_err(&dev->udev->dev, "error (%d) re-submitting urb "
+					"in isoc_handler\n", ret);
+
+		return;
+	} else
+		fill = framebuf->data + framebuf->filled;
+
+	/* Reset ISOC error counter */
+	dev->visoc_errors = 0;
+
+	/* Compact data */
+	for (i = 0; i < urb->number_of_packets; i++) {
+		framestatus = urb->iso_frame_desc[i].status;
+		framelen = urb->iso_frame_desc[i].actual_length;
+		iso_buf = urb->transfer_buffer + urb->iso_frame_desc[i].offset;
+
+		if (framestatus == 0) {
+			skip = 4;
+
+			if (framelen > 4) {
+	/* we found something informational from there */
+	/* the isoc frames have two type of headers */
+	/* type1: 00 xx 00 00 or 20 xx 00 00 */
+	/* type2: 80 xx 00 00 00 00 00 00 or a0 xx 00 00 00 00 00 00 */
+	/* xx is a sequencer which has never been seen over 0x3f */
+
+	/* imho data written down looks like bayer, i see similarities after */
+				/* every 640 bytes */
+				if (*iso_buf & 0x80) {
+					skip = 8;
+				}
+
+				if (framelen - skip + framebuf->filled >
+						dev->frame_size) {
+					dev_err(&dev->udev->dev, "frame buffer "
+							"overflow\n");
+				} else {
+					memcpy(fill, iso_buf + skip,
+					       framelen - skip);
+					fill += framelen - skip;
+				}
+
+				framebuf->filled += framelen - skip;
+			}
+
+			STK_STREAM("URB : Length = %d - Skip = %d - Buffer "
+					"size = %d\n", framelen, skip,
+					framebuf->filled);
+
+			if (framelen == 4) {
+				if (framebuf->filled > 0) {
+					stk11xx_next_frame(dev);
+
+					awake = 1;
+					framebuf = dev->fill_frame;
+					framebuf->filled = 0;
+					fill = framebuf->data;
+				}
+			}
+		} else
+			dev_err(&dev->udev->dev, "iso frame %d has error %d\n",
+					i, framestatus);
+	}
+
+	if (awake == 1)
+		wake_up_interruptible(&dev->wait_frame);
+
+	urb->dev = dev->udev;
+
+	ret = usb_submit_urb(urb, GFP_ATOMIC);
+	if (ret != 0)
+		dev_err(&dev->udev->dev, "error (%d) re-submitting urb in "
+				"isoc_handler.\n", ret);
+}
+
+static void stk11xx_isoc_cleanup(struct stk11xx *dev)
+{
+	unsigned int i;
+
+	dev_dbg(&dev->udev->dev, "isoc cleanup\n");
+
+	if (dev->isoc_init_ok == 0)
+		return;
+
+	/* Unlinking ISOC buffers */
+	for (i = 0; i < MAX_ISO_BUFS; i++) {
+		struct urb *urb;
+
+		urb = dev->isobuf[i].urb;
+
+		if (urb != 0) {
+			if (dev->isoc_init_ok)
+				usb_kill_urb(urb);
+
+			usb_free_urb(urb);
+			dev->isobuf[i].urb = NULL;
+		}
+	}
+
+	/* All is done */
+	dev->isoc_init_ok = 0;
+}
+
+static int stk11xx_isoc_init(struct stk11xx *dev)
+{
+	struct usb_device *udev;
+	struct urb *urb;
+	unsigned int i, j;
+	int ret = 0;
+
+	if (dev->isoc_init_ok)
+		return 0;
+
+	udev = dev->udev;
+
+	dev_dbg(&udev->dev, "%s\n", __FUNCTION__);
+
+	/* Allocate URB structure */
+	for (i = 0; i < MAX_ISO_BUFS; i++) {
+		urb = usb_alloc_urb(ISO_FRAMES_PER_DESC, GFP_KERNEL);
+
+		if (urb == NULL) {
+			dev_err(&udev->dev, "failed to allocate URB %d\n", i);
+			ret = -ENOMEM;
+			break;
+		}
+
+		dev->isobuf[i].urb = urb;
+	}
+
+	if (ret) {
+		while (i >= 0) {
+			if (dev->isobuf[i].urb != NULL)
+				usb_free_urb(dev->isobuf[i].urb);
+
+			dev->isobuf[i].urb = NULL;
+			i--;
+		}
+
+		return ret;
+	}
+	/* Init URB structure */
+	for (i = 0; i < MAX_ISO_BUFS; i++) {
+		urb = dev->isobuf[i].urb;
+
+		urb->interval = 1;
+		urb->dev = udev;
+		urb->pipe = usb_rcvisocpipe(udev, dev->isoc_in_endpointAddr);
+		urb->transfer_flags = URB_ISO_ASAP;
+		urb->transfer_buffer = dev->isobuf[i].data;
+		urb->transfer_buffer_length = ISO_BUFFER_SIZE;
+		urb->complete = stk11xx_isoc_handler;
+		urb->context = dev;
+		urb->start_frame = 0;
+		urb->number_of_packets = ISO_FRAMES_PER_DESC;
+
+		for (j = 0; j < ISO_FRAMES_PER_DESC; j++) {
+			urb->iso_frame_desc[j].offset = j * ISO_MAX_FRAME_SIZE;
+			urb->iso_frame_desc[j].length = ISO_MAX_FRAME_SIZE;
+		}
+	}
+
+	dev_dbg(&dev->udev->dev, "isoc_in_size = %zx, "
+			"isoc_in_endpointAddr = %x\n",
+			dev->isoc_in_size, dev->isoc_in_endpointAddr);
+
+	/* Link */
+	for (i = 0; i < MAX_ISO_BUFS; i++) {
+		ret = usb_submit_urb(dev->isobuf[i].urb, GFP_KERNEL);
+
+		if (ret)
+			dev_err(&dev->udev->dev, "isoc_init submit_urb %d "
+					"failed with error %d\n", i, ret);
+		else
+			dev_dbg(&dev->udev->dev, "URB 0x%p submitted\n",
+					dev->isobuf[i].urb);
+	}
+
+	/* All is done */
+	dev->isoc_init_ok = 1;
+
+	return 0;
+}
+
+/*
+ * When we configure the stk11xx, this function is used to check the device
+ * status.
+ *   - If the read value is 0x00, then the device isn't ready.
+ *   - If the read value is 0x04, then the device is ready.
+ *   - If the read value is other, then the device is misconfigured.
+ */
+static int stk11xx_check_device(struct stk11xx *dev, int nbr)
+{
+	unsigned int i;
+	int value;
+
+	for (i = 0; i < nbr; i++) {
+		stk11xx_read_registry(dev, 0x201, &value);
+
+		switch (value) {
+		case 0x00:
+			break;
+		case 0x01:
+		case 0x04:
+			return 1;
+		default:
+			dev_err(&dev->udev->dev, "check device return error "
+					"(0x201 = %02X)\n", value);
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
+static int stk11xx_camera_off(struct stk11xx *dev)
+{
+	struct usb_device *udev = dev->udev;
+	int ret;
+
+	ret = usb_set_interface(udev, 0, 0);
+
+	if (ret < 0)
+		dev_err(&udev->dev, "usb_set_interface failed\n");
+
+	return 0;
+}
+
+/*
+ * STK-1125 API
+ */
+
+static int stk1125_load_microcode(struct stk11xx *dev)
+{
+	unsigned int i;
+	const u8 *values_204, *values_205;
+	int retok, value;
+
+	/* From 80x60 to 640x480 */
+	const u8 values_1_204[] = {
+		0x12, 0x11, 0x3b, 0x6a, 0x13, 0x10, 0x00, 0x01, 0x02, 0x13,
+		0x39, 0x38, 0x37, 0x35, 0x0e, 0x12, 0x04, 0x0c, 0x0d, 0x17,
+		0x18, 0x32, 0x19, 0x1a, 0x03, 0x1b, 0x16, 0x33, 0x34, 0x41,
+		0x96, 0x3d, 0x69, 0x3a, 0x8e, 0x3c, 0x8f, 0x8b, 0x8c, 0x94,
+		0x95, 0x40, 0x29, 0x0f, 0xa5, 0x1e, 0xa9, 0xaa, 0xab, 0x90,
+		0x91, 0x9f, 0xa0, 0x24, 0x25, 0x26, 0x14, 0x2a, 0x2b
+	};
+	const u8 values_1_205[] = {
+		0x45, 0x80, 0x01, 0x7d, 0x80, 0x00, 0x00, 0x80, 0x80, 0x80,
+		0x50, 0x93, 0x00, 0x81, 0x20, 0x45, 0x00, 0x00, 0x00, 0x24,
+		0xc4, 0xb6, 0x00, 0x3c, 0x36, 0x00, 0x07, 0xe2, 0xbf, 0x00,
+		0x04, 0x19, 0x40, 0x0d, 0x00, 0x73, 0xdf, 0x06, 0x20, 0x88,
+		0x88, 0xc1, 0x3f, 0x42, 0x80, 0x04, 0xb8, 0x92, 0x0a, 0x00,
+		0x00, 0x00, 0x00, 0x68, 0x5c, 0xc3, 0x2e, 0x00, 0x00
+	};
+
+	/* From 800x600 to 1280x1024 */
+	const u8 values_2_204[] = {
+		0x12, 0x11, 0x3b, 0x6a, 0x13, 0x10, 0x00, 0x01, 0x02, 0x13,
+		0x39, 0x38, 0x37, 0x35, 0x0e, 0x12, 0x04, 0x0c, 0x0d, 0x17,
+		0x18, 0x32, 0x19, 0x1a, 0x03, 0x1b, 0x16, 0x33, 0x34, 0x41,
+		0x96, 0x3d, 0x69, 0x3a, 0x8e, 0x3c, 0x8f, 0x8b, 0x8c, 0x94,
+		0x95, 0x40, 0x29, 0x0f, 0xa5, 0x1e, 0xa9, 0xaa, 0xab, 0x90,
+		0x91, 0x9f, 0xa0, 0x24, 0x25, 0x26, 0x14, 0x2a, 0x2b
+	};
+	const u8 values_2_205[] = {
+		0x05, 0x80, 0x01, 0x7d, 0x80, 0x00, 0x00, 0x80, 0x80, 0x80,
+		0x50, 0x93, 0x00, 0x81, 0x20, 0x05, 0x00, 0x00, 0x00, 0x1b,
+		0xbb, 0xa4, 0x01, 0x81, 0x12, 0x00, 0x07, 0xe2, 0xbf, 0x00,
+		0x04, 0x19, 0x40, 0x0d, 0x00, 0x73, 0xdf, 0x06, 0x20, 0x88,
+		0x88, 0xc1, 0x3f, 0x42, 0x80, 0x04, 0xb8, 0x92, 0x0a, 0x00,
+		0x00, 0x00, 0x00, 0x68, 0x5c, 0xc3, 0x2e, 0x00, 0x00
+	};
+
+	/* From the resolution */
+	switch (dev->resolution) {
+	case STK11XX_1280x1024:
+	case STK11XX_1024x758:
+	case STK11XX_800x600:
+		values_204 = values_2_204;
+		values_205 = values_2_205;
+		break;
+
+	case STK11XX_640x480:
+	case STK11XX_320x240:
+	case STK11XX_160x120:
+	case STK11XX_80x60:
+	default:
+		values_204 = values_1_204;
+		values_205 = values_1_205;
+		break;
+	}
+
+	for (i = 0; i < 59; i++) {
+		stk11xx_read_registry(dev, 0x02FF, &value);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+
+		stk11xx_write_registry(dev, 0x0204, values_204[i]);
+		stk11xx_write_registry(dev, 0x0205, values_205[i]);
+		stk11xx_write_registry(dev, 0x0200, 0x0001);
+
+		retok = stk11xx_check_device(dev, 500);
+		if (retok != 1) {
+			dev_err(&dev->udev->dev, "load microcode fail\n");
+			return -1;
+		}
+
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+	}
+
+	stk11xx_check_device(dev, 500);
+
+	return 0;
+}
+
+/*
+ * The configuration of device is composed of 11 steps.
+ * This function is called by the initialization process.
+ *
+ * We don't know the meaning of these steps! We only replay the USB log.
+ *
+ * The steps 0 to 9 are called during the initialization.
+ * Then, the driver choose the last step :
+ *   10 : for a resolution from 80x60 to 640x480
+ *   11 : for a resolution from 800x600 to 1280x1024
+ */
+static int stk1125_configure_device(struct stk11xx *dev, int step)
+{
+	int value;
+
+	/*      0,    1,    2,    3,    4,    5,    6,    7,    8,    9,
+		    10,   11 */
+
+	const u8 values_001B[] = {
+		0x0E, 0x03, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E,
+		    0x0E, 0x0E
+	};
+	const u8 values_001C[] = {
+		0x06, 0x02, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46,
+		    0x46, 0x0E
+	};
+	const u8 values_0202[] = {
+		0x1E, 0x0A, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E,
+		    0x1E, 0x1E
+	};
+	const u8 values_0110[] = {
+		0x07, 0x00, 0x00, 0x00, 0x00, 0x3E, 0x3E, 0x00, 0x00, 0x00,
+		    0x00, 0x00
+	};
+	const u8 values_0112[] = {
+		0x07, 0x00, 0x00, 0x00, 0x00, 0x09, 0x09, 0x00, 0x00, 0x00,
+		    0x00, 0x00
+	};
+	const u8 values_0114[] = {
+		0x87, 0x80, 0x80, 0x80, 0x80, 0xBE, 0xBE, 0x80, 0x80, 0x80,
+		    0x80, 0x00
+	};
+	const u8 values_0115[] = {
+		0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02,
+		    0x02, 0x05
+	};
+	const u8 values_0116[] = {
+		0xE7, 0xE0, 0xE0, 0xE0, 0xE0, 0xE9, 0xE9, 0xE0, 0xE0, 0xE0,
+		    0xE0, 0x00
+	};
+	const u8 values_0117[] = {
+		0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
+		    0x01, 0x04
+	};
+	const u8 values_0100[] = {
+		0x20, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21,
+		    0x21, 0x21
+	};
+
+	dev_dbg(&dev->udev->dev, "stk1125_configure_device: %d\n", step);
+
+	stk11xx_write_registry(dev, 0x0000, 0x0024);
+	stk11xx_write_registry(dev, 0x0002, 0x0068);
+	stk11xx_write_registry(dev, 0x0003, 0x0080);
+	stk11xx_write_registry(dev, 0x0005, 0x0000);
+
+	stk11xx_write_registry(dev, 0x0007, 0x0003);
+	stk11xx_write_registry(dev, 0x000d, 0x0000);
+	stk11xx_write_registry(dev, 0x000f, 0x0002);
+	stk11xx_write_registry(dev, 0x0300, 0x0012);
+	stk11xx_write_registry(dev, 0x0350, 0x0041);
+
+	stk11xx_write_registry(dev, 0x0351, 0x0000);
+	stk11xx_write_registry(dev, 0x0352, 0x0000);
+	stk11xx_write_registry(dev, 0x0353, 0x0000);
+	stk11xx_write_registry(dev, 0x0018, 0x0010);
+	stk11xx_write_registry(dev, 0x0019, 0x0000);
+
+	stk11xx_write_registry(dev, 0x001b, values_001B[step]);
+	stk11xx_write_registry(dev, 0x001c, values_001C[step]);
+	stk11xx_write_registry(dev, 0x0300, 0x0080);
+	stk11xx_write_registry(dev, 0x001a, 0x0004);
+	stk11xx_write_registry(dev, 0x0202, values_0202[step]);
+
+	stk11xx_write_registry(dev, 0x0110, values_0110[step]);
+	stk11xx_write_registry(dev, 0x0111, 0x0000);
+	stk11xx_write_registry(dev, 0x0112, values_0112[step]);
+	stk11xx_write_registry(dev, 0x0113, 0x0000);
+	stk11xx_write_registry(dev, 0x0114, values_0114[step]);
+
+	stk11xx_write_registry(dev, 0x0115, values_0115[step]);
+	stk11xx_write_registry(dev, 0x0116, values_0116[step]);
+	stk11xx_write_registry(dev, 0x0117, values_0117[step]);
+
+	stk11xx_read_registry(dev, 0x0100, &value);
+	stk11xx_write_registry(dev, 0x0100, values_0100[step]);
+
+	stk11xx_write_registry(dev, 0x0200, 0x0080);
+	stk11xx_write_registry(dev, 0x0200, 0x0000);
+	stk11xx_write_registry(dev, 0x02FF, 0x0000);
+
+	switch (step) {
+	case 0:
+		stk11xx_write_registry(dev, 0x0203, 0x0040);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0041);
+		stk11xx_write_registry(dev, 0x0205, 0x0001);
+		stk11xx_write_registry(dev, 0x0204, 0x001C);
+		stk11xx_write_registry(dev, 0x0205, 0x0002);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 1:
+		stk11xx_write_registry(dev, 0x0203, 0x0022);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0027);
+		stk11xx_write_registry(dev, 0x0205, 0x00A5);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 2:
+		stk11xx_write_registry(dev, 0x0203, 0x0060);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0012);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+		stk11xx_write_registry(dev, 0x0204, 0x0013);
+		stk11xx_write_registry(dev, 0x0205, 0x00BF);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 3:
+		stk11xx_write_registry(dev, 0x0203, 0x0042);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0012);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+		stk11xx_write_registry(dev, 0x0204, 0x0024);
+		stk11xx_write_registry(dev, 0x0205, 0x00A5);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 4:
+		stk11xx_write_registry(dev, 0x0203, 0x0042);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0012);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+		stk11xx_write_registry(dev, 0x0204, 0x0013);
+		stk11xx_write_registry(dev, 0x0205, 0x00E0);
+		stk11xx_write_registry(dev, 0x0204, 0x0024);
+		stk11xx_write_registry(dev, 0x0205, 0x00A5);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 5:
+		stk11xx_write_registry(dev, 0x0203, 0x0060);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0012);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+		stk11xx_write_registry(dev, 0x0204, 0x0013);
+		stk11xx_write_registry(dev, 0x0205, 0x00FF);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 6:
+		stk11xx_write_registry(dev, 0x0203, 0x0060);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0012);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+		stk11xx_write_registry(dev, 0x0204, 0x0013);
+		stk11xx_write_registry(dev, 0x0205, 0x00FF);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 7:
+		stk11xx_write_registry(dev, 0x0203, 0x0060);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0012);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+		stk11xx_write_registry(dev, 0x0204, 0x0013);
+		stk11xx_write_registry(dev, 0x0205, 0x00B7);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 8:
+		stk11xx_write_registry(dev, 0x0203, 0x0060);
+
+		stk1125_load_microcode(dev);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0080);
+		stk11xx_write_registry(dev, 0x0200, 0x0000);
+		stk11xx_write_registry(dev, 0x02FF, 0x0001);
+		stk11xx_write_registry(dev, 0x0203, 0x00A0);
+
+		break;
+
+	case 9:
+		stk11xx_write_registry(dev, 0x0203, 0x0060);
+
+		stk1125_load_microcode(dev);
+
+		stk11xx_write_registry(dev, 0x0104, 0x0000);
+		stk11xx_write_registry(dev, 0x0105, 0x0000);
+		stk11xx_write_registry(dev, 0x0106, 0x0000);
+
+		break;
+
+	case 10:
+	case 11:
+		stk11xx_write_registry(dev, 0x0203, 0x0060);
+
+		stk1125_load_microcode(dev);
+
+		stk11xx_write_registry(dev, 0x0106, 0x0000);
+		stk11xx_read_registry(dev, 0x02FF, &value);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+		stk11xx_write_registry(dev, 0x0204, 0x002A);
+		stk11xx_write_registry(dev, 0x0205, 0x0000);
+		stk11xx_write_registry(dev, 0x0200, 0x0001);
+		stk11xx_check_device(dev, 500);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+		stk11xx_read_registry(dev, 0x02FF, &value);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+		stk11xx_write_registry(dev, 0x0204, 0x002B);
+		stk11xx_write_registry(dev, 0x0205, 0x0000);
+		stk11xx_write_registry(dev, 0x0200, 0x0001);
+		stk11xx_check_device(dev, 500);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+
+		break;
+	}
+
+	return 0;

drivers/media/video/stk11xx.c line 1249

Quoted 10 lines
drivers/media/video/stk11xx.c new file +10 -0
+}
+
+static int stk1125_camera_asleep(struct stk11xx *dev)
+{
+	int value;
+
+	stk11xx_read_registry(dev, 0x0104, &value);
+	stk11xx_read_registry(dev, 0x0105, &value);
+	stk11xx_read_registry(dev, 0x0106, &value);
+

why do you read these values (this is also something in the ongoing code I see, the read value just gets overwritten all the time)?

Quoted 326 lines
drivers/media/video/stk11xx.c new file +21 -0
+	stk11xx_write_registry(dev, 0x0100, 0x0021);
+	stk11xx_write_registry(dev, 0x0116, 0x0000);
+	stk11xx_write_registry(dev, 0x0117, 0x0000);
+	stk11xx_write_registry(dev, 0x0018, 0x0000);
+
+	stk11xx_read_registry(dev, 0x0000, &value);
+	stk11xx_write_registry(dev, 0x0000, 0x004C);
+
+	return 0;
+}
+
+/*
+ * This functions permits to modify the settings :
+ *   - brightness
+ *   - contrast
+ *   - white balance
+ *   - ...
+ *
+ * 0x204 = 0xA1 : unknown (by default 0x00)
+ * 0x204 = 0x10 : contrast (by default 0x7c)
+ * 0x204 = 0x04 : Mode (unknown) (by default 0x00) (=> already looked 0x01

and 0x02)
+ * 0x204 = 0x00 : brightness / white balance (by default 0x00)
+ * 0x204 = 0x2E : Fps MSB (by default 0x01)
+ * 0x204 = 0x2D : Fps LSB (by default 0x00)
+ *
+ * 0x2E | 0x2D | Nbr fps
+ * -----+------+--------

+ * 0x00 | 0x00 |  30
+ * 0x01 | 0x00 |  20
+ * 0x02 | 0x00 |  15
+ * 0x03 | 0x00 |  12
+ * 0x04 | 0x00 |  10

+ */

drivers/media/video/stk11xx.c new file +292 -0
+static int stk1125_setting_camera(struct stk11xx *dev)
+{
+	int ret;
+
+	stk11xx_write_registry(dev, 0x0200, 0x0000);
+
+	/* Unknown register */
+	stk11xx_write_registry(dev, 0x0204, 0x00A1);
+	stk11xx_write_registry(dev, 0x0205, 0x0000);
+
+	/* Contrast register */
+	stk11xx_write_registry(dev, 0x0204, 0x0010);
+	stk11xx_write_registry(dev, 0x0205, dev->vsettings.contrast);
+
+	/* Unknown register */
+	stk11xx_write_registry(dev, 0x0204, 0x0004);
+	stk11xx_write_registry(dev, 0x0205, 0x0000);
+
+	/* Whiteness register */
+	stk11xx_write_registry(dev, 0x0204, 0x0000);
+	stk11xx_write_registry(dev, 0x0205, dev->vsettings.whiteness);
+
+	/* FPS register */
+	switch (dev->vsettings.fps) {
+	case 10:
+		stk11xx_write_registry(dev, 0x0204, 0x002E);
+		stk11xx_write_registry(dev, 0x0205, 0x0004);
+		stk11xx_write_registry(dev, 0x0204, 0x002D);
+		stk11xx_write_registry(dev, 0x0205, 0x0000);
+		break;
+
+	case 15:
+		stk11xx_write_registry(dev, 0x0204, 0x002E);
+		stk11xx_write_registry(dev, 0x0205, 0x0002);
+		stk11xx_write_registry(dev, 0x0204, 0x002D);
+		stk11xx_write_registry(dev, 0x0205, 0x0000);
+		break;
+
+	case 20:
+		stk11xx_write_registry(dev, 0x0204, 0x002E);
+		stk11xx_write_registry(dev, 0x0205, 0x0001);
+		stk11xx_write_registry(dev, 0x0204, 0x002D);
+		stk11xx_write_registry(dev, 0x0205, 0x0000);
+		break;
+
+	case 30:
+		stk11xx_write_registry(dev, 0x0204, 0x002E);
+		stk11xx_write_registry(dev, 0x0205, 0x0000);
+		stk11xx_write_registry(dev, 0x0204, 0x002D);
+		stk11xx_write_registry(dev, 0x0205, 0x0000);
+		break;
+
+	default:
+		stk11xx_write_registry(dev, 0x0204, 0x002E);
+		stk11xx_write_registry(dev, 0x0205, 0x0001);
+		stk11xx_write_registry(dev, 0x0204, 0x002D);
+		stk11xx_write_registry(dev, 0x0205, 0x0000);
+	}
+
+	stk11xx_write_registry(dev, 0x0200, 0x0006);
+
+	dev_dbg(&dev->udev->dev, "set contrast: %d, whiteness: %d, "
+			"brightness: %d\n", dev->vsettings.contrast,
+			dev->vsettings.whiteness, dev->vsettings.brightness);
+
+	ret = stk11xx_check_device(dev, 500);
+	if (!ret)
+		dev_dbg(&dev->udev->dev, "find not 0x4 ... seems OK\n");
+
+	return 0;
+}
+
+static int stk1125_init_camera(struct stk11xx *dev)
+{
+	int value;
+
+	stk1125_camera_asleep(dev);
+
+	stk11xx_set_feature(dev, 0);
+
+	stk11xx_write_registry(dev, 0x0000, 0x00E0);
+	stk11xx_write_registry(dev, 0x0002, 0x00E8);
+	stk11xx_write_registry(dev, 0x0002, 0x0068);
+	stk11xx_write_registry(dev, 0x0000, 0x0020);
+
+	stk1125_configure_device(dev, 9);
+
+	stk11xx_camera_off(dev);
+
+	stk11xx_read_registry(dev, 0x02FF, &value);
+	stk11xx_write_registry(dev, 0x02FF, 0x0000);
+	stk11xx_write_registry(dev, 0x0204, 0x002a);
+	stk11xx_write_registry(dev, 0x0205, 0x0000);
+	stk11xx_write_registry(dev, 0x0200, 0x0001);
+	stk11xx_check_device(dev, 500);
+	stk11xx_write_registry(dev, 0x02FF, 0x0000);
+	stk11xx_read_registry(dev, 0x02FF, &value);
+	stk11xx_write_registry(dev, 0x02FF, 0x0000);
+	stk11xx_write_registry(dev, 0x0204, 0x002B);
+	stk11xx_write_registry(dev, 0x0205, 0x0000);
+	stk11xx_write_registry(dev, 0x0200, 0x0001);
+	stk11xx_check_device(dev, 500);
+	stk11xx_write_registry(dev, 0x02FF, 0x0000);
+
+	stk1125_setting_camera(dev);
+
+	return 0;
+}
+
+static int stk1125_start_stream(struct stk11xx *dev)
+{
+	int value, value_116, value_117;
+
+	stk11xx_read_registry(dev, 0x0116, &value_116);
+	stk11xx_read_registry(dev, 0x0117, &value_117);
+
+	stk11xx_write_registry(dev, 0x0116, 0x0000);
+	stk11xx_write_registry(dev, 0x0117, 0x0000);
+
+	stk11xx_read_registry(dev, 0x0100, &value);	/* read 0x21 */
+	stk11xx_write_registry(dev, 0x0100, 0x00A1);
+
+	stk11xx_write_registry(dev, 0x0116, value_116);
+	stk11xx_write_registry(dev, 0x0117, value_117);
+
+	return 0;
+}
+
+static int stk1125_reconf_camera(struct stk11xx *dev)
+{
+	int step;
+
+	switch (dev->resolution) {
+	case STK11XX_1280x1024:
+	case STK11XX_1024x758:
+	case STK11XX_800x600:
+		step = 11;
+		break;
+
+	case STK11XX_640x480:
+	case STK11XX_320x240:
+	case STK11XX_160x120:
+	case STK11XX_80x60:
+	default:
+		step = 10;
+		break;
+	}
+
+	stk1125_configure_device(dev, step);
+
+	stk11xx_setting_camera(dev);
+
+	return 0;
+}
+
+static int stk1125_stop_stream(struct stk11xx *dev)
+{
+	int value;
+
+	stk11xx_read_registry(dev, 0x0100, &value);
+	stk11xx_write_registry(dev, 0x0100, 0x0021);
+
+	return 0;
+}
+
+/*
+ * STK-1135 API
+ */
+
+static int stk1135_load_microcode(struct stk11xx *dev)
+{
+	unsigned int i;
+	int retok, value;
+
+	const u8 values_204[] = {
+		0x17, 0x19, 0xb4, 0xa6, 0x12, 0x13, 0x1e, 0x21, 0x24, 0x32,
+		0x36, 0x39, 0x4d, 0x53, 0x5d, 0x5f, 0x60, 0x61, 0x62, 0x63,
+		0x64, 0x65, 0x66, 0x82, 0x83, 0x85, 0x86, 0x89, 0x97, 0x98,
+		0xad, 0xae, 0xb8, 0xb9, 0xba, 0xbb, 0xbc, 0xbf, 0x48, 0xd8,
+		0x76, 0x77, 0x78, 0x79, 0x7a, 0x7b, 0x7c, 0x7d, 0x7e, 0x7f,
+		0x80, 0x81, 0xd8, 0x76, 0x77, 0x78, 0x79, 0x7a, 0x7b, 0x7c,
+		0x7d, 0x7e, 0x7f, 0x80, 0x81, 0xd8, 0x76, 0x77, 0x78, 0x79,
+		0x7a, 0x7b, 0x7c, 0x7d, 0x7e, 0x7f, 0x80, 0x81, 0x5c, 0xc0,
+		0x59, 0x5a, 0x5b, 0xd4, 0x8e, 0x8f, 0x90, 0x91, 0x92, 0x93,
+		0x94, 0x95, 0x96, 0xb3, 0x73, 0x06, 0x07, 0x0b, 0x15, 0x20,
+		0x4e, 0x4f, 0x49, 0x4a, 0x4b, 0x4c, 0x46, 0x06, 0x07, 0xb9,
+		0xba, 0xbb, 0xbc, 0x61, 0x62, 0x65, 0x66
+	};
+	const u8 values_205[] = {
+		0x41, 0x41, 0x03, 0x06, 0x06, 0x08, 0x06, 0x00, 0x02, 0x69,
+		0x35, 0x60, 0xfe, 0x1c, 0x04, 0x08, 0x08, 0x08, 0x08, 0x00,
+		0x00, 0x10, 0x14, 0x01, 0x80, 0x0c, 0xb6, 0x00, 0x25, 0x25,
+		0x3f, 0x24, 0x10, 0x07, 0xcc, 0x1f, 0x30, 0x02, 0x9c, 0x80,
+		0x00, 0x0d, 0x18, 0x22, 0x2c, 0x3e, 0x4f, 0x6f, 0x8e, 0xac,
+		0xc8, 0xe5, 0xa0, 0x00, 0x0d, 0x18, 0x22, 0x2c, 0x3e, 0x4f,
+		0x6f, 0x8e, 0xac, 0xc8, 0xe5, 0xc0, 0x00, 0x0d, 0x18, 0x22,
+		0x2c, 0x3e, 0x4f, 0x6f, 0x8e, 0xac, 0xc8, 0xe5, 0x70, 0x18,
+		0x09, 0x07, 0x07, 0x3c, 0x3d, 0x95, 0x88, 0x89, 0x47, 0x9c,
+		0x81, 0x9c, 0x3d, 0x76, 0x76, 0x01, 0xf3, 0x05, 0x00, 0x44,
+		0x06, 0x0a, 0x96, 0x00, 0x7d, 0x00, 0x20, 0x01, 0xf3, 0x04,
+		0xe4, 0x09, 0xc8, 0x08, 0x08, 0x10, 0x14
+	};
+
+	for (i = 0; i < 59; i++) {
+		stk11xx_read_registry(dev, 0x02FF, &value);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+
+		stk11xx_write_registry(dev, 0x0204, values_204[i]);
+		stk11xx_write_registry(dev, 0x0205, values_205[i]);
+		stk11xx_write_registry(dev, 0x0200, 0x0001);
+
+		retok = stk11xx_check_device(dev, 500);
+		if (retok != 1) {
+			dev_err(&dev->udev->dev, "load microcode failed\n");
+			return -1;
+		}
+
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+	}
+
+	stk11xx_check_device(dev, 500);
+
+	return 0;
+}
+
+/*
+ * The configuration of device is composed of 12 steps.
+ * This function is called by the initialization process.
+ *
+ * We don't know the meaning of these steps! We only replay the USB log.
+ */
+static int stk1135_configure_device(struct stk11xx *dev, int step)
+{
+	int value;
+
+	/*      0,    1,    2,    3,    4,    5,    6,    7,    8,    9,
+		    10,   11,   12,   13 */
+
+	const u8 values_001B[] = {
+		0x0E, 0x03, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x07,
+		    0x07, 0x07, 0x07, 0x07
+	};
+	const u8 values_001C[] = {
+		0x06, 0x02, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46, 0x06, 0x06,
+		    0x06, 0x06, 0x06, 0x07
+	};
+	const u8 values_0202[] = {
+		0x1E, 0x0A, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E,
+		    0x1E, 0x1E, 0x1E, 0x1E
+	};
+	const u8 values_0110[] = {
+		0x07, 0x00, 0x00, 0x00, 0x00, 0x3E, 0x3E, 0x00, 0x04, 0x00,
+		    0x00, 0x00, 0x00, 0x00
+	};
+	const u8 values_0112[] = {
+		0x07, 0x00, 0x00, 0x00, 0x00, 0x09, 0x09, 0x00, 0x04, 0x00,
+		    0x00, 0x00, 0x00, 0x00
+	};
+	const u8 values_0114[] = {
+		0x87, 0x80, 0x80, 0x80, 0x80, 0xBE, 0xBE, 0x80, 0x84, 0x80,
+		    0x80, 0x80, 0x80, 0x80
+	};
+	const u8 values_0116[] = {
+		0xE7, 0xE0, 0xE0, 0xE0, 0xE0, 0xE9, 0xE9, 0xE0, 0xE4, 0xE0,
+		    0xE0, 0xE0, 0xE0, 0xE0
+	};
+	const u8 values_0100[] = {
+		0x20, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x23, 0x20,
+		    0x20, 0x20, 0x20, 0x20
+	};
+
+	dev_dbg(&dev->udev->dev, "stk1135_configure_device: %d\n", step);
+
+	stk11xx_write_registry(dev, 0x0000, 0x0024);
+	stk11xx_write_registry(dev, 0x0002, 0x0068);
+	stk11xx_write_registry(dev, 0x0003, 0x0080);
+	stk11xx_write_registry(dev, 0x0005, 0x0000);
+
+	stk11xx_write_registry(dev, 0x0007, 0x0003);
+	stk11xx_write_registry(dev, 0x000d, 0x0000);
+	stk11xx_write_registry(dev, 0x000f, 0x0002);
+	stk11xx_write_registry(dev, 0x0300, 0x0012);
+	stk11xx_write_registry(dev, 0x0350, 0x0041);
+
+	stk11xx_write_registry(dev, 0x0351, 0x0000);
+	stk11xx_write_registry(dev, 0x0352, 0x0000);
+	stk11xx_write_registry(dev, 0x0353, 0x0000);
+	stk11xx_write_registry(dev, 0x0018, 0x0010);
+	stk11xx_write_registry(dev, 0x0019, 0x0000);
+
+	stk11xx_write_registry(dev, 0x001b, values_001B[step]);
+	stk11xx_write_registry(dev, 0x001c, values_001C[step]);

drivers/media/video/stk11xx.c line 1584

Quoted 10 lines
drivers/media/video/stk11xx.c new file +10 -0
+	stk11xx_write_registry(dev, 0x0300, 0x0080);
+	stk11xx_write_registry(dev, 0x001a, 0x0004);
+	stk11xx_write_registry(dev, 0x0202, values_0202[step]);
+
+	stk11xx_write_registry(dev, 0x0110, values_0110[step]);
+	stk11xx_write_registry(dev, 0x0111, 0x0000);
+	stk11xx_write_registry(dev, 0x0112, values_0112[step]);
+	stk11xx_write_registry(dev, 0x0113, 0x0000);
+	stk11xx_write_registry(dev, 0x0114, values_0114[step]);
+

if possible some error checking would be nice here, if you unplug the device you'll rely on the underlying subsystem to handle your errors I don't think that's a good way to go.
Did you test unplugging your device while it's in use with mplayer/xawtv/..?

cheers,
Markus

Quoted 2,048 lines
drivers/media/video/stk11xx.c new file +377 -0
+	stk11xx_write_registry(dev, 0x0115, 0x0002);
+	stk11xx_write_registry(dev, 0x0116, values_0116[step]);
+	stk11xx_write_registry(dev, 0x0117, 0x0001);
+
+	stk11xx_read_registry(dev, 0x0100, &value);
+	stk11xx_write_registry(dev, 0x0100, values_0100[step]);
+
+	stk11xx_write_registry(dev, 0x0200, 0x0080);
+	stk11xx_write_registry(dev, 0x0200, 0x0000);
+	stk11xx_write_registry(dev, 0x02FF, 0x0000);
+
+	switch (step) {
+	case 0:
+		stk11xx_write_registry(dev, 0x0203, 0x0040);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0041);
+		stk11xx_write_registry(dev, 0x0205, 0x0001);
+		stk11xx_write_registry(dev, 0x0204, 0x001C);
+		stk11xx_write_registry(dev, 0x0205, 0x0002);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 1:
+		stk11xx_write_registry(dev, 0x0203, 0x0022);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0027);
+		stk11xx_write_registry(dev, 0x0205, 0x00A5);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 2:
+		stk11xx_write_registry(dev, 0x0203, 0x0060);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0012);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+		stk11xx_write_registry(dev, 0x0204, 0x0013);
+		stk11xx_write_registry(dev, 0x0205, 0x00BF);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 3:
+		stk11xx_write_registry(dev, 0x0203, 0x0042);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0012);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+		stk11xx_write_registry(dev, 0x0204, 0x0024);
+		stk11xx_write_registry(dev, 0x0205, 0x00A5);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 4:
+		stk11xx_write_registry(dev, 0x0203, 0x0042);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0012);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+		stk11xx_write_registry(dev, 0x0204, 0x0013);
+		stk11xx_write_registry(dev, 0x0205, 0x00E0);
+		stk11xx_write_registry(dev, 0x0204, 0x0024);
+		stk11xx_write_registry(dev, 0x0205, 0x00A5);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 5:
+		stk11xx_write_registry(dev, 0x0203, 0x0060);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0012);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+		stk11xx_write_registry(dev, 0x0204, 0x0013);
+		stk11xx_write_registry(dev, 0x0205, 0x00FF);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 6:
+		stk11xx_write_registry(dev, 0x0203, 0x0060);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0012);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+		stk11xx_write_registry(dev, 0x0204, 0x0013);
+		stk11xx_write_registry(dev, 0x0205, 0x00FF);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 7:
+		stk11xx_write_registry(dev, 0x0203, 0x0060);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0012);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+		stk11xx_write_registry(dev, 0x0204, 0x0013);
+		stk11xx_write_registry(dev, 0x0205, 0x00B7);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 8:
+		stk11xx_write_registry(dev, 0x0203, 0x0080);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0012);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+		stk11xx_write_registry(dev, 0x0204, 0x000A);
+		stk11xx_write_registry(dev, 0x0205, 0x00FF);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		break;
+
+	case 9:
+		stk11xx_write_registry(dev, 0x0203, 0x00DC);
+
+		stk11xx_write_registry(dev, 0x0204, 0x0015);
+		stk11xx_write_registry(dev, 0x0205, 0x0080);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0005);
+
+		stk11xx_check_device(dev, 500);
+		stk11xx_read_registry(dev, 0x02FF, &value);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+		stk11xx_write_registry(dev, 0x0208, 0x0000);
+		stk11xx_write_registry(dev, 0x0200, 0x0020);
+		stk11xx_check_device(dev, 500);
+		stk11xx_read_registry(dev, 0x0209, &value);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+		stk11xx_read_registry(dev, 0x02FF, &value);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+		stk11xx_write_registry(dev, 0x0208, 0x0001);
+		stk11xx_write_registry(dev, 0x0200, 0x0020);
+		stk11xx_check_device(dev, 500);
+		stk11xx_read_registry(dev, 0x0209, &value);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+		stk11xx_read_registry(dev, 0x02FF, &value);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+		stk11xx_write_registry(dev, 0x0208, 0x0002);
+		stk11xx_write_registry(dev, 0x0200, 0x0020);
+		stk11xx_check_device(dev, 500);
+		stk11xx_read_registry(dev, 0x0209, &value);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+		stk11xx_write_registry(dev, 0x0002, 0x006F);
+
+		break;
+
+	case 10:
+		stk11xx_write_registry(dev, 0x0203, 0x00DC);
+
+		stk1135_load_microcode(dev);
+
+		break;
+
+	case 11:
+		stk11xx_write_registry(dev, 0x0203, 0x00DC);
+
+		stk1135_load_microcode(dev);
+
+		stk11xx_write_registry(dev, 0x0104, 0x0000);
+		stk11xx_write_registry(dev, 0x0105, 0x0000);
+		stk11xx_write_registry(dev, 0x0106, 0x0000);
+
+		break;
+
+	case 12:
+		stk11xx_write_registry(dev, 0x0203, 0x00DC);
+
+		stk1135_load_microcode(dev);
+
+		stk11xx_write_registry(dev, 0x0104, 0x0000);
+		stk11xx_write_registry(dev, 0x0105, 0x0000);
+		stk11xx_write_registry(dev, 0x0106, 0x0000);
+
+		break;
+
+	case 13:
+		stk11xx_write_registry(dev, 0x0203, 0x00DC);
+
+		stk1135_load_microcode(dev);
+	}
+
+	return 0;
+}
+
+static int stk1135_camera_asleep(struct stk11xx *dev)
+{
+	int value;
+
+	stk11xx_read_registry(dev, 0x0104, &value);
+	stk11xx_read_registry(dev, 0x0105, &value);
+	stk11xx_read_registry(dev, 0x0106, &value);
+
+	stk11xx_write_registry(dev, 0x0100, 0x0021);
+	stk11xx_write_registry(dev, 0x0116, 0x0000);
+	stk11xx_write_registry(dev, 0x0117, 0x0000);
+	stk11xx_write_registry(dev, 0x0018, 0x0000);
+
+	stk11xx_read_registry(dev, 0x0000, &value);
+	stk11xx_write_registry(dev, 0x0000, 0x0049);
+
+	return 0;
+}
+
+static int stk1135_setting_camera(struct stk11xx *dev)
+{
+	unsigned int i;
+	int ret, value;
+
+	const u8 values_204[] = {
+		0x46, 0x06, 0x07, 0xb9, 0xba, 0xbb, 0xbc, 0x61, 0x62, 0x65, 0x66
+	};
+	const u8 values_205[] = {
+		0x20, 0x01, 0xf3, 0x04, 0xe4, 0x09, 0xc8, 0x08, 0x08, 0x10, 0x14
+	};
+
+	for (i = 0; i < 11; i++) {
+		stk11xx_read_registry(dev, 0x02FF, &value);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+
+		stk11xx_write_registry(dev, 0x0204, values_204[i]);
+		stk11xx_write_registry(dev, 0x0205, values_205[i]);
+
+		stk11xx_write_registry(dev, 0x0200, 0x0001);
+		ret = stk11xx_check_device(dev, 500);
+		stk11xx_write_registry(dev, 0x02FF, 0x0000);
+	}
+
+	return 0;
+}
+
+static int stk1135_init_camera(struct stk11xx *dev)
+{
+	stk1135_camera_asleep(dev);
+
+	stk11xx_set_feature(dev, 0);
+
+	stk11xx_write_registry(dev, 0x0000, 0x00E0);
+	stk11xx_write_registry(dev, 0x0002, 0x00E8);
+	stk11xx_write_registry(dev, 0x0002, 0x0068);
+	stk11xx_write_registry(dev, 0x0000, 0x0020);
+
+	stk1135_configure_device(dev, 11);
+
+	stk11xx_camera_off(dev);
+
+	stk1135_setting_camera(dev);
+
+	stk1135_camera_asleep(dev);
+
+	stk11xx_set_feature(dev, 0);
+
+	stk11xx_write_registry(dev, 0x0000, 0x00E0);
+	stk11xx_write_registry(dev, 0x0002, 0x00E8);
+	stk11xx_write_registry(dev, 0x0002, 0x0068);
+	stk11xx_write_registry(dev, 0x0000, 0x0020);
+
+	stk1135_configure_device(dev, 12);
+
+	stk11xx_camera_off(dev);
+
+	stk1135_setting_camera(dev);
+
+	return 0;
+}
+
+static int stk1135_start_stream(struct stk11xx *dev)
+{
+	int value, value_116, value_117;
+
+	stk11xx_read_registry(dev, 0x0116, &value_116);
+	stk11xx_read_registry(dev, 0x0117, &value_117);
+
+	stk11xx_write_registry(dev, 0x0116, 0x0000);
+	stk11xx_write_registry(dev, 0x0117, 0x0000);
+
+	stk11xx_read_registry(dev, 0x0100, &value);	/* read 0x21 */
+	stk11xx_write_registry(dev, 0x0100, 0x00A0);
+
+	stk11xx_write_registry(dev, 0x0116, value_116);
+	stk11xx_write_registry(dev, 0x0117, value_117);
+
+	return 0;
+}
+
+static int stk1135_reconf_camera(struct stk11xx *dev)
+{
+	stk1135_configure_device(dev, 13);
+
+	return 0;
+}
+
+static int stk1135_stop_stream(struct stk11xx *dev)
+{
+	int value;
+
+	stk11xx_read_registry(dev, 0x0100, &value);
+	stk11xx_write_registry(dev, 0x0100, 0x0020);
+
+	return 0;
+}
+
+static int stk11xx_setting_camera(struct stk11xx *dev)
+{
+	int ret = -1;
+
+	switch (dev->webcam_model) {
+	case SYNTEK_DC1125:
+		ret = stk1125_setting_camera(dev);
+		break;
+	case SYNTEK_STK1135:
+		ret = stk1135_setting_camera(dev);
+		break;
+	}
+
+	return ret;
+}
+
+/*
+ * It's the start. This function has to be called at first, before
+ * enabling the video stream.
+ */
+static int stk11xx_init_camera(struct stk11xx *dev)
+{
+	int ret = -1;
+
+	switch (dev->webcam_model) {
+	case SYNTEK_DC1125:
+		ret = stk1125_init_camera(dev);
+		break;
+	case SYNTEK_STK1135:
+		ret = stk1135_init_camera(dev);
+		break;
+	}
+
+	return ret;
+}
+
+static int stk11xx_camera_on(struct stk11xx *dev)
+{
+	struct usb_device *udev = dev->udev;
+	int ret;
+
+	ret = usb_set_interface(udev, 0, 5);
+
+	if (ret < 0)
+		dev_err(&udev->dev, "usb_set_interface failed\n");
+
+	return ret;
+}
+
+/* Wake-up the camera */
+static int stk11xx_camera_asleep(struct stk11xx *dev)
+{
+	int ret = -1;
+
+	switch (dev->webcam_model) {
+	case SYNTEK_DC1125:
+		ret = stk1125_camera_asleep(dev);
+		break;
+	case SYNTEK_STK1135:
+		ret = stk1135_camera_asleep(dev);
+		break;
+	}
+
+	return ret;
+}
+
+/*
+ * After the initialization of the device and the initialization of the

video
+ * stream, this function enables the stream.
+ */

drivers/media/video/stk11xx.c new file +252 -0
+static int stk11xx_start_stream(struct stk11xx *dev)
+{
+	int ret = -1;
+
+	switch (dev->webcam_model) {
+	case SYNTEK_DC1125:
+		ret = stk1125_start_stream(dev);
+		break;
+	case SYNTEK_STK1135:
+		ret = stk1135_start_stream(dev);
+		break;
+	}
+
+	return ret;
+}
+
+/* before enabling the video stream, you have to reconfigure the device */
+static int stk11xx_reconf_camera(struct stk11xx *dev)
+{
+	int ret = -1;
+
+	switch (dev->webcam_model) {
+	case SYNTEK_DC1125:
+		ret = stk1125_reconf_camera(dev);
+		break;
+	case SYNTEK_STK1135:
+		ret = stk1135_reconf_camera(dev);
+		break;
+	}
+
+	return ret;
+}
+
+static int stk11xx_stop_stream(struct stk11xx *dev)
+{
+	int ret = -1;
+
+	switch (dev->webcam_model) {
+	case SYNTEK_DC1125:
+		ret = stk1125_stop_stream(dev);
+		break;
+	case SYNTEK_STK1135:
+		ret = stk1135_stop_stream(dev);
+		break;
+	}
+
+	return ret;
+}
+
+static void *stk11xx_rvmalloc(unsigned long size)
+{
+	void *mem;
+	unsigned long addr;
+
+	mem = vmalloc_user(size);
+
+	if (!mem)
+		return NULL;
+
+	memset(mem, 0, size);
+
+	addr = (unsigned long)mem;
+
+	while (size > 0) {
+		SetPageReserved(vmalloc_to_page((void *)addr));
+		addr += PAGE_SIZE;
+		size -= PAGE_SIZE;
+	}
+
+	return mem;
+}
+
+static int stk11xx_free_buffers(struct stk11xx *dev);
+
+static int stk11xx_allocate_buffers(struct stk11xx *dev)
+{
+	unsigned int i;
+
+	dev_dbg(&dev->udev->dev, "%s\n", __FUNCTION__);
+
+	/* Allocate isochronous pipe buffers */
+	for (i = 0; i < MAX_ISO_BUFS; i++)
+		if (dev->isobuf[i].data == NULL) {
+			dev->isobuf[i].data = kzalloc(ISO_BUFFER_SIZE,
+					GFP_KERNEL);
+			if (dev->isobuf[i].data == NULL) {
+				dev_err(&dev->udev->dev, "failed to allocate "
+					"iso buffer %d\n", i);
+				goto err;
+			}
+		}
+
+	/* Allocate frame buffer structure */
+	if (dev->framebuf == NULL) {
+		dev->framebuf = kzalloc(default_nbrframebuf *
+			    sizeof(struct stk11xx_frame_buf), GFP_KERNEL);
+		if (dev->framebuf == NULL) {
+			dev_err(&dev->udev->dev, "failed to allocate frame "
+					"buffer structure\n");
+			goto err;
+		}
+	}
+	/* Create frame buffers and make circular ring */
+	for (i = 0; i < default_nbrframebuf; i++)
+		if (dev->framebuf[i].data == NULL) {
+			dev->framebuf[i].data = vmalloc(STK11XX_FRAME_SIZE);
+			if (dev->framebuf[i].data == NULL) {
+				dev_err(&dev->udev->dev, "failed to allocate "
+						"frame buffer %d\n", i);
+				goto err;
+			}
+			memset(dev->framebuf[i].data, 0, STK11XX_FRAME_SIZE);
+		}
+
+	/* Allocate decompressor table space */
+	dev->decompress_data = stk11xx_rvmalloc(dev->len_per_image);
+	if (dev->decompress_data == NULL) {
+		dev_err(&dev->udev->dev, "failed to allocate decompress "
+				"buffer(s). needed (%d)\n", dev->len_per_image);
+		goto err;
+	}
+
+	/* Allocate image buffer; double buffer for mmap() */
+	dev->image_data = stk11xx_rvmalloc(dev->nbuffers * dev->len_per_image);
+	if (dev->image_data == NULL) {
+		dev_err(&dev->udev->dev, "failed to allocate image buffer(s). "
+			"needed (%d)\n", dev->nbuffers * dev->len_per_image);
+		goto err;
+	}
+
+	for (i = 0; i < dev->nbuffers; i++) {
+		dev->images[i].offset = i * dev->len_per_image;
+		dev->images[i].vma_use_count = 0;
+	}
+
+	for (; i < STK11XX_MAX_IMAGES; i++)
+		dev->images[i].offset = 0;
+
+	return 0;
+err:
+	stk11xx_free_buffers(dev);
+	return -ENOMEM;
+}
+
+static void stk11xx_rvfree(void *mem, unsigned long size)
+{
+	unsigned long addr;
+
+	if (!mem)
+		return;
+
+	addr = (unsigned long)mem;
+
+	while ((long)size > 0) {
+		ClearPageReserved(vmalloc_to_page((void *)addr));
+		addr += PAGE_SIZE;
+		size -= PAGE_SIZE;
+	}
+
+	vfree(mem);
+}
+
+static int stk11xx_free_buffers(struct stk11xx *dev)
+{
+	unsigned int i;
+
+	dev_dbg(&dev->udev->dev, "%s\n", __FUNCTION__);
+
+	if (dev == NULL)
+		return -1;
+
+	/* Release iso pipe buffers */
+	for (i = 0; i < MAX_ISO_BUFS; i++)
+		if (dev->isobuf[i].data != NULL) {
+			kfree(dev->isobuf[i].data);
+			dev->isobuf[i].data = NULL;
+		}
+
+	/* Release frame buffers */
+	if (dev->framebuf != NULL) {
+		for (i = 0; i < default_nbrframebuf; i++)
+			if (dev->framebuf[i].data != NULL) {
+				vfree(dev->framebuf[i].data);
+				dev->framebuf[i].data = NULL;
+			}
+
+		kfree(dev->framebuf);
+	}
+	/* Release decompression buffers */
+	if (dev->decompress_data != NULL)
+		stk11xx_rvfree(dev->decompress_data, dev->len_per_image);
+
+	/* Release image buffers */
+	if (dev->image_data != NULL)
+		stk11xx_rvfree(dev->image_data,
+			       dev->nbuffers * dev->len_per_image);
+
+	dev->decompress_data = dev->framebuf = dev->image_data = NULL;
+
+	return 0;
+}
+
+static int stk11xx_reset_buffers(struct stk11xx *dev)
+{
+	unsigned long flags;
+	unsigned int i;
+
+	dev_dbg(&dev->udev->dev, "%s\n", __FUNCTION__);
+
+	spin_lock_irqsave(&dev->spinlock, flags);
+
+	dev->full_frames = NULL;
+	dev->full_frames_tail = NULL;
+
+	for (i = 0; i < dev->nbuffers; i++) {
+		dev->framebuf[i].filled = 0;
+
+		if (i > 0)
+			dev->framebuf[i].next = &dev->framebuf[i - 1];
+		else
+			dev->framebuf->next = NULL;
+	}
+
+	dev->empty_frames = &dev->framebuf[dev->nbuffers - 1];
+	dev->empty_frames_tail = dev->framebuf;
+	dev->read_frame = NULL;
+	dev->fill_frame = dev->empty_frames;
+	dev->empty_frames = dev->empty_frames->next;
+
+	dev->image_read_pos = 0;
+	dev->fill_image = 0;
+
+	spin_unlock_irqrestore(&dev->spinlock, flags);
+
+	for (i = 0; i < dev->nbuffers; i++)
+		dev->image_f[i] = 0;
+
+	return 0;
+}
+
+/* called when an image is ready to prepare the next image */
+static void stk11xx_next_image(struct stk11xx *dev)
+{
+	STK_STREAM("Select next image\n");
+
+	dev->image_f[dev->fill_image] = 0;
+	dev->fill_image = (dev->fill_image + 1) % dev->nbuffers;
+}
+
+/*
+ * This function reads periodically the value of register 0x0001.
+ * We don't know the purpose. I assume that it seems to a software

watchdog.
+ */

drivers/media/video/stk11xx.c new file +15 -0
+static int stk11xx_watchdog_camera(struct stk11xx *dev)
+{
+	int value;
+
+	stk11xx_read_registry(dev, 0x0001, &value);
+
+	if (value != 0x03)
+		dev_err(&dev->udev->dev, "Error: Register 0x0001 = %02X\n",
+				value);
+
+	return value;
+}
+
+/*
+ * This function gets called for the isochronous pipe. This function is

only
+ * called when a frame is ready. So we have to be fast to decompress the data.
+ */

drivers/media/video/stk11xx.c new file +50 -0
+static int stk11xx_handle_frame(struct stk11xx *dev)
+{
+	unsigned long flags;
+	int ret = 0;
+
+	STK_STREAM("Sync Handle Frame\n");
+
+	spin_lock_irqsave(&dev->spinlock, flags);
+
+	if (dev->read_frame != NULL) {
+		spin_unlock_irqrestore(&dev->spinlock, flags);
+		return ret;
+	}
+
+	if (dev->full_frames != NULL) {
+		dev->read_frame = dev->full_frames;
+		dev->full_frames = dev->full_frames->next;
+		dev->read_frame->next = NULL;
+	}
+
+	if (dev->read_frame != NULL) {
+		spin_unlock_irqrestore(&dev->spinlock, flags);
+		ret = stk11xx_decompress(dev);
+		spin_lock_irqsave(&dev->spinlock, flags);
+
+		if (dev->empty_frames == NULL) {
+			dev->empty_frames = dev->read_frame;
+			dev->empty_frames_tail = dev->empty_frames;
+		} else {
+			dev->empty_frames_tail->next = dev->read_frame;
+			dev->empty_frames_tail = dev->read_frame;
+		}
+
+		dev->read_frame = NULL;
+	}
+
+	spin_unlock_irqrestore(&dev->spinlock, flags);
+
+	stk11xx_watchdog_camera(dev);
+
+	return ret;
+}
+
+static int stk11xx_select_video_mode(struct stk11xx *dev, int width,
+				  int height)
+{
+	unsigned int i, find;
+
+	/* Check width and height */
+	/* Driver can't build an image more little than the minimal resolution !

*/

drivers/media/video/stk11xx.c new file +139 -0
+	if ((width < stk11xx_image_sizes[0].x) ||
+			(height < stk11xx_image_sizes[0].y))
+		return -1;
+
+	/* Seek the best resolution */
+	switch (dev->webcam_model) {
+	case SYNTEK_DC1125:
+		for (i = 0, find = 0; i < STK11XX_NBR_SIZES; i++) {
+			if (stk11xx_image_sizes[i].x <= width &&
+					stk11xx_image_sizes[i].y <= height)
+				find = i;
+		}
+		break;
+
+	case SYNTEK_STK1135:
+		for (i = 0, find = 0; i < STK11XX_NBR_SIZES - 3; i++) {
+			if (stk11xx_image_sizes[i].x <= width &&
+					stk11xx_image_sizes[i].y <= height)
+				find = i;
+		}
+		break;
+
+	default:
+		return -1;
+	}
+
+	/* Save the new resolution */
+	dev->resolution = find;
+
+	dev_dbg(&dev->udev->dev, "set mode %d [%dx%d]\n", dev->resolution,
+			stk11xx_image_sizes[dev->resolution].x,
+			stk11xx_image_sizes[dev->resolution].y);
+
+	/* Save the new size */
+	dev->view.x = width;
+	dev->view.y = height;
+
+	/* Calculate the frame size */
+	switch (dev->resolution) {
+	case STK11XX_80x60:
+	case STK11XX_160x120:
+	case STK11XX_320x240:
+	case STK11XX_640x480:
+		dev->frame_size = stk11xx_image_sizes[STK11XX_640x480].x *
+				stk11xx_image_sizes[STK11XX_640x480].y;
+		dev->image_size = 3 * dev->frame_size;
+		break;
+
+	case STK11XX_800x600:
+	case STK11XX_1024x758:
+	case STK11XX_1280x1024:
+		dev->frame_size = stk11xx_image_sizes[STK11XX_1280x1024].x *
+				stk11xx_image_sizes[STK11XX_1280x1024].y;
+		dev->image_size = 3 * dev->frame_size;
+		break;
+	}
+
+	return 0;
+}
+
+static int stk11xx_open(struct inode *inode, struct file *fp)
+{
+	struct video_device *vdev = video_devdata(fp);
+	struct stk11xx *dev = video_get_drvdata(vdev);
+	int retval;
+
+	BUG_ON(dev == NULL);
+
+	nonseekable_open(inode, fp);
+
+	if (mutex_lock_interruptible(&dev->open_lock))
+		return -ERESTARTSYS;
+
+	if (dev->vopen) {
+		retval = -EBUSY;
+		goto end;
+	}
+
+	retval = stk11xx_allocate_buffers(dev);
+	if (retval < 0) {
+		dev_err(&dev->udev->dev, "failed to allocate buffer memory\n");
+		goto end;
+	}
+
+	stk11xx_reset_buffers(dev);
+
+	stk11xx_select_video_mode(dev, 640, 480);
+
+	stk11xx_init_camera(dev);
+	stk11xx_camera_on(dev);
+	stk11xx_reconf_camera(dev);
+
+	dev->error_status = 0;
+	dev->vsettings.brightness = 200;
+	dev->vsettings.contrast = 0x7c;
+	dev->vsettings.whiteness = 0x80;
+	dev->vsettings.pixformat = V4L2_PIX_FMT_BGR24;
+
+	retval = stk11xx_isoc_init(dev);
+	if (retval) {
+		dev_err(&dev->udev->dev, "failed to init ISOC stuff\n");
+		stk11xx_isoc_cleanup(dev);
+		stk11xx_free_buffers(dev);
+		goto end;
+	}
+
+	stk11xx_start_stream(dev);
+
+	dev->vopen++;
+	fp->private_data = dev;
+	retval = 0;
+end:
+	mutex_unlock(&dev->open_lock);
+
+	return retval;
+}
+
+static int stk11xx_release(struct inode *inode, struct file *fp)
+{
+	struct stk11xx *dev = fp->private_data;
+
+	mutex_lock(&dev->open_lock);
+	if (dev->vopen == 0)
+		dev_warn(&dev->udev->dev, "v4l_release called on closed "
+				"device\n");
+
+	stk11xx_stop_stream(dev);
+	stk11xx_isoc_cleanup(dev);
+	stk11xx_free_buffers(dev);
+	stk11xx_camera_off(dev);
+	stk11xx_camera_asleep(dev);
+
+	dev->vopen--;
+	mutex_unlock(&dev->open_lock);
+
+	return 0;
+}
+
+static ssize_t stk11xx_read(struct file *fp, char __user *buf, size_t

count,

drivers/media/video/stk11xx.c new file +140 -0
+		loff_t *f_pos)
+{
+	struct stk11xx *dev = fp->private_data;
+	void *image_buffer_addr;
+	int bytes_to_read, retval;
+
+	STK_STREAM("Read vdev=0x%p, buf=0x%p, count=%zd\n", vdev, buf, count);
+
+	if (dev->image_read_pos == 0) {
+		if ((fp->f_flags & O_NONBLOCK) && dev->full_frames == NULL)
+			return -EWOULDBLOCK;
+
+		retval = wait_event_interruptible(dev->wait_frame,
+			dev->full_frames != NULL || dev->error_status);
+
+		if (retval)
+			return retval;
+		if (dev->error_status)
+			return -dev->error_status;
+
+		if (stk11xx_handle_frame(dev))
+			return -EFAULT;
+	}
+
+	bytes_to_read = dev->image_size;
+
+	if (count + dev->image_read_pos > bytes_to_read)
+		count = bytes_to_read - dev->image_read_pos;
+
+	image_buffer_addr = dev->image_data;
+	image_buffer_addr += dev->images[dev->fill_image].offset;
+	image_buffer_addr += dev->image_read_pos;
+
+	if (copy_to_user(buf, image_buffer_addr, count))
+		return -EFAULT;
+
+	dev->image_read_pos += count;
+
+	if (dev->image_read_pos >= bytes_to_read) {
+		dev->image_read_pos = 0;
+		stk11xx_next_image(dev);
+	}
+
+	return count;
+}
+
+static unsigned int stk11xx_poll(struct file *fp, poll_table * wait)
+{
+	struct stk11xx *dev = fp->private_data;
+
+	STK_STREAM("Poll\n");
+
+	poll_wait(fp, &dev->wait_frame, wait);
+
+	if (dev->error_status)
+		return POLLERR;
+
+	if (dev->full_frames != NULL)
+		return (POLLIN | POLLRDNORM);
+
+	return 0;
+}
+
+static int stk11xx_mmap(struct file *fp, struct vm_area_struct *vma)
+{
+	struct stk11xx *dev = fp->private_data;
+	unsigned long pos, size = vma->vm_end - vma->vm_start;
+	unsigned int i;
+
+	/* Find the buffer for this mapping... */
+	for (i = 0; i < dev->nbuffers; i++) {
+		pos = dev->images[i].offset;
+
+		if ((pos >> PAGE_SHIFT) == vma->vm_pgoff)
+			break;
+	}
+
+	if (i >= STK11XX_MAX_IMAGES) {
+		dev_err(&dev->udev->dev, "mmap no buffer found\n");
+		return -EINVAL;
+	}
+
+	/* map either whole space or only one buffer! */
+	if (size > dev->len_per_image && (i > 0 ||
+			(i == 0 && size != dev->nbuffers * dev->len_per_image)))
+		return -EINVAL;
+
+	vma->vm_flags |= VM_IO;
+
+	return remap_vmalloc_range(vma, dev->image_data, vma->vm_pgoff);
+}
+
+static struct v4l2_queryctrl stk11xx_controls[] = {
+	{
+		.id      = V4L2_CID_BRIGHTNESS,
+		.type    = V4L2_CTRL_TYPE_INTEGER,
+		.name    = "Brightness",
+		.minimum = 0,
+		.maximum = 400,
+		.step    = 1,
+		.default_value = 200,
+	},
+	{
+		.id      = V4L2_CID_CONTRAST,
+		.type    = V4L2_CTRL_TYPE_INTEGER,
+		.name    = "Contrast",
+		.minimum = 0,
+		.maximum = 0xff,
+		.step    = 1,
+		.default_value = 0x7c,
+	},
+	{
+		.id      = V4L2_CID_WHITENESS,
+		.type    = V4L2_CTRL_TYPE_INTEGER,
+		.name    = "Whiteness",
+		.minimum = 0,
+		.maximum = 0xff,
+		.step    = 1,
+		.default_value = 0x80,
+	},
+};
+
+static int stk11xx_querycap(struct file *file, void *fh,
+		struct v4l2_capability *cap)
+{
+	struct stk11xx *dev = fh;
+
+	strlcpy(cap->driver, "stk11xx", sizeof(cap->driver));
+
+	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE |
+			V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
+	cap->version = (u32)DRIVER_VERSION_NUM;
+	strlcpy(cap->card, dev->vdev->name, sizeof(cap->card));
+
+	if (usb_make_path(dev->udev, cap->bus_info, sizeof(cap->bus_info)) < 0)
+		strlcpy(cap->bus_info, dev->vdev->name, sizeof(cap->bus_info));
+
+	return 0;
+}
+static int stk11xx_enum_input(struct file *file, void *fh, struct

v4l2_input *i)

drivers/media/video/stk11xx.c new file +42 -0
+{
+	if (i->index)
+		return -EINVAL;
+
+	strlcpy(i->name, "USB", sizeof(i->name));
+	i->type = V4L2_INPUT_TYPE_CAMERA;
+
+	return 0;
+}
+
+static int stk11xx_g_input(struct file *file, void *fh, unsigned int *i)
+{
+	*i = 0;
+	return 0;
+}
+
+static int stk11xx_s_input(struct file *file, void *fh, unsigned int i)
+{
+	return i ? -EINVAL : 0;
+}
+
+static int stk11xx_queryctrl(struct file *file, void *fh,
+		struct v4l2_queryctrl *c)
+{
+	unsigned int i;
+
+	pr_debug("VIDIOC_QUERYCTRL id = %d\n", c->id);
+
+	for (i = 0; i < ARRAY_SIZE(stk11xx_controls); i++)
+		if (stk11xx_controls[i].id == c->id) {
+			pr_debug("VIDIOC_QUERYCTRL found\n");
+			memcpy(c, &stk11xx_controls[i], sizeof(*c));
+			break;
+		}
+
+	if (i >= ARRAY_SIZE(stk11xx_controls))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int stk11xx_g_ctrl(struct file *file, void *fh, struct v4l2_control

*c)

drivers/media/video/stk11xx.c new file +22 -0
+{
+	struct stk11xx *dev = fh;
+
+	dev_dbg(&dev->udev->dev, "GET CTRL id=%d\n", c->id);
+
+	switch (c->id) {
+	case V4L2_CID_BRIGHTNESS:
+		c->value = dev->vsettings.brightness;
+		break;
+	case V4L2_CID_CONTRAST:
+		c->value = dev->vsettings.contrast;
+		break;
+	case V4L2_CID_WHITENESS:
+		c->value = dev->vsettings.whiteness;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int stk11xx_s_ctrl(struct file *file, void *fh, struct v4l2_control

*c)

drivers/media/video/stk11xx.c new file +178 -0
+{
+	struct stk11xx *dev = fh;
+
+	dev_dbg(&dev->udev->dev, "SET CTRL id=%d\n", c->id);
+
+	switch (c->id) {
+	case V4L2_CID_BRIGHTNESS:
+		dev->vsettings.brightness = c->value;
+		break;
+	case V4L2_CID_CONTRAST:
+		dev->vsettings.contrast = c->value;
+		break;
+	case V4L2_CID_WHITENESS:
+		dev->vsettings.whiteness = c->value;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return stk11xx_setting_camera(dev) ? -EIO : 0;
+}
+
+static int stk11xx_enum_fmt_cap(struct file *file, void *fh,
+		struct v4l2_fmtdesc *fmtd)
+{
+	u32 index = fmtd->index;
+
+	pr_debug("VIDIOC_ENUM_FMT %d\n", index);
+
+	if (index)
+		return -EINVAL;
+
+	fmtd->pixelformat = V4L2_PIX_FMT_BGR24;
+	strcpy(fmtd->description, "rgb24");
+
+	return 0;
+}
+
+static int stk11xx_g_fmt_cap(struct file *file, void *fh,
+	   	struct v4l2_format *fmtd)
+{
+	struct stk11xx *dev = fh;
+	struct v4l2_pix_format *pix = &fmtd->fmt.pix;
+
+	pix->width = dev->view.x;
+	pix->height = dev->view.y;
+	pix->pixelformat = V4L2_PIX_FMT_BGR24;
+	pix->field = V4L2_FIELD_NONE;
+	pix->bytesperline = 3 * pix->width;
+	pix->sizeimage = pix->width * pix->height * 3;
+	pix->colorspace = V4L2_COLORSPACE_SRGB;
+	pix->priv = 0;
+
+	return 0;
+}
+
+static int stk11xx_s_fmt_cap(struct file *file, void *fh,
+	   	struct v4l2_format *fmtd)
+{
+	struct stk11xx *dev = fh;
+	struct v4l2_pix_format *pix = &fmtd->fmt.pix;
+	unsigned int a;
+
+	dev_dbg(&dev->udev->dev, "set width=%d, height=%d\n", pix->width,
+			pix->height);
+
+	if (pix->pixelformat && pix->pixelformat != V4L2_PIX_FMT_BGR24)
+		return -EINVAL;
+
+	for (a = 0; a < ARRAY_SIZE(stk11xx_image_sizes); a++)
+		if (pix->width == stk11xx_image_sizes[a].x &&
+				pix->height == stk11xx_image_sizes[a].y)
+			break;
+	if (a >= ARRAY_SIZE(stk11xx_image_sizes))
+		return -EINVAL;
+
+	stk11xx_stop_stream(dev);
+	stk11xx_isoc_cleanup(dev);
+	stk11xx_camera_off(dev);
+	stk11xx_camera_asleep(dev);
+
+	/* Reset buffers and parameters */
+	/*stk11xx_reset_buffers(dev); */
+
+	if (stk11xx_select_video_mode(dev, pix->width, pix->height)) {
+		dev_err(&dev->udev->dev, "Select video mode failed\n");
+		return -EINVAL;
+	}
+
+	stk11xx_init_camera(dev);
+	stk11xx_camera_on(dev);
+	stk11xx_reconf_camera(dev);
+	stk11xx_isoc_init(dev);
+	stk11xx_start_stream(dev);
+
+	return 0;
+}
+
+static int stk11xx_try_fmt_cap(struct file *file, void *fh,
+	   	struct v4l2_format *fmtd)
+{
+	struct v4l2_pix_format *pix = &fmtd->fmt.pix;
+
+	if (pix->pixelformat != V4L2_PIX_FMT_BGR24)
+		return -EINVAL;
+
+	if (pix->width > stk11xx_image_sizes[STK11XX_NBR_SIZES - 1].x)
+		pix->width = stk11xx_image_sizes[STK11XX_NBR_SIZES - 1].x;
+	else if (pix->width < stk11xx_image_sizes[0].x)
+		pix->width = stk11xx_image_sizes[0].x;
+
+	if (pix->height > stk11xx_image_sizes[STK11XX_NBR_SIZES - 1].y)
+		pix->height = stk11xx_image_sizes[STK11XX_NBR_SIZES - 1].y;
+	else if (pix->height < stk11xx_image_sizes[0].y)
+		pix->height = stk11xx_image_sizes[0].y;
+
+	return 0;
+}
+
+static int stk11xx_querystd(struct file *file, void *fh, v4l2_std_id *a)
+{
+	*a = V4L2_STD_UNKNOWN;
+	return 0;
+}
+
+static int stk11xx_s_std(struct file *file, void *fh, v4l2_std_id *a)
+{
+	return *a == V4L2_STD_UNKNOWN ? 0 : -EINVAL;
+}
+
+static int stk11xx_reqbufs(struct file *file, void *fh,
+		struct v4l2_requestbuffers *rb)
+{
+	struct stk11xx *dev = fh;
+	u32 nbuffers = rb->count;
+
+	if (rb->memory != V4L2_MEMORY_MMAP)
+		return -EINVAL;
+
+	if (nbuffers < 2)
+		nbuffers = 2;
+	else if (nbuffers > dev->nbuffers)
+		nbuffers = dev->nbuffers;
+
+	rb->count = dev->nbuffers;
+
+	return 0;
+}
+
+static int stk11xx_querybuf(struct file *file, void *fh,
+		struct v4l2_buffer *buf)
+{
+	struct stk11xx *dev = fh;
+	u32 index = buf->index;
+
+	dev_dbg(&dev->udev->dev, "QUERY BUFFERS %d %d\n", buf->index,
+		  dev->nbuffers);
+
+	if (index >= dev->nbuffers)
+		return -EINVAL;
+
+	memset(buf, 0, sizeof(struct v4l2_buffer));
+
+	buf->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	buf->index = index;
+	buf->memory = V4L2_MEMORY_MMAP;
+	buf->m.offset = index * dev->len_per_image;
+	buf->bytesused = dev->image_size;
+	buf->field = V4L2_FIELD_NONE;
+	buf->length = dev->len_per_image;
+	buf->flags = V4L2_BUF_FLAG_MAPPED | dev->image_f[index];
+	if (dev->full_frames != NULL)
+		buf->flags |= V4L2_BUF_FLAG_DONE;
+
+	return 0;
+}
+
+static int stk11xx_qbuf(struct file *file, void *fh, struct v4l2_buffer

*buf)

drivers/media/video/stk11xx.c new file +22 -0
+{
+	struct stk11xx *dev = fh;
+
+	dev_dbg(&dev->udev->dev, "VIDIOC_QBUF\n");
+
+	if (buf->memory != V4L2_MEMORY_MMAP)
+		return -EINVAL;
+
+	if (buf->index >= dev->nbuffers)
+		return -EINVAL;
+
+	if (dev->image_f[buf->index] & V4L2_BUF_FLAG_QUEUED)
+		return -EBUSY;
+
+	dev->image_f[buf->index] |= V4L2_BUF_FLAG_QUEUED;
+	buf->flags |= V4L2_BUF_FLAG_QUEUED;
+	buf->flags &= ~V4L2_BUF_FLAG_DONE;
+
+	return 0;
+}
+
+static int stk11xx_dqbuf(struct file *file, void *fh, struct v4l2_buffer

*buf)

drivers/media/video/stk11xx.c new file +40 -0
+{
+	struct stk11xx *dev = fh;
+	int retval;
+
+	dev_dbg(&dev->udev->dev, "VIDIOC_DQBUF\n");
+
+	if (dev->full_frames == NULL && (file->f_flags & O_NONBLOCK))
+		return -EAGAIN;
+
+	retval = wait_event_interruptible(dev->wait_frame,
+			dev->full_frames != NULL || dev->error_status);
+
+	if (retval)
+		return retval;
+	if (dev->error_status)
+		return -dev->error_status;
+
+	dev_dbg(&dev->udev->dev, "VIDIOC_DQBUF: frame ready\n");
+
+	retval = stk11xx_handle_frame(dev);
+
+	if (retval)
+		return -EFAULT;
+
+	buf->index = dev->fill_image;
+	buf->bytesused = dev->image_size;
+	buf->flags = V4L2_BUF_FLAG_MAPPED | V4L2_BUF_FLAG_DONE;
+	buf->field = V4L2_FIELD_NONE;
+	do_gettimeofday(&buf->timestamp);
+	buf->sequence = 0;
+	buf->memory = V4L2_MEMORY_MMAP;
+	buf->m.offset = dev->fill_image * dev->len_per_image;
+	buf->length = buf->bytesused;
+
+	stk11xx_next_image(dev);
+
+	return 0;
+}
+
+static int stk11xx_streamon(struct file *file, void *fh, enum v4l2_buf_type

i)

drivers/media/video/stk11xx.c new file +5 -0
+{
+	return i == V4L2_BUF_TYPE_VIDEO_CAPTURE ? stk11xx_isoc_init(fh):-EINVAL;
+}
+
+static int stk11xx_streamoff(struct file *file, void *fh, enum

v4l2_buf_type i)

drivers/media/video/stk11xx.c new file +24 -0
+{
+	return i == V4L2_BUF_TYPE_VIDEO_CAPTURE ?
+		stk11xx_isoc_cleanup(fh), 0 : -EINVAL;
+}
+
+static int stk11xx_g_parm(struct file *file, void *fh,
+		struct v4l2_streamparm *sp)
+{
+	pr_debug("GET PARM %d\n", sp->type);
+
+	if (sp->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	sp->parm.capture.capability = 0;
+	sp->parm.capture.capturemode = 0;
+	sp->parm.capture.timeperframe.numerator = 1;
+	sp->parm.capture.timeperframe.denominator = 30;
+	sp->parm.capture.readbuffers = 2;
+	sp->parm.capture.extendedmode = 0;
+
+	return 0;
+}
+/*
+static int vidioc_cropcap(struct file *file, void *fh, struct v4l2_cropcap

*cc)

drivers/media/video/stk11xx.c new file +18 -0
+{
+	cc->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	cc->pixelaspect.numerator = 1;
+	cc->pixelaspect.denominator = 1;
+	cc->bounds.top = 0;
+	cc->bounds.left = 0;
+	cc->bounds.width = 640;
+	cc->bounds.height = 480;
+	cc->defrect.top = 0;
+	cc->defrect.left = 0;
+	cc->defrect.width = 640;
+	cc->defrect.height = 480;
+
+	return 0;
+}
+*/
+#ifdef CONFIG_VIDEO_V4L1_COMPAT
+static int stk11xx_gmbuf(struct file *file, void *fh, struct video_mbuf

*vm)

drivers/media/video/stk11xx.c new file +544 -0
+{
+	struct stk11xx *dev = fh;
+	unsigned int i;
+
+	dev_dbg(&dev->udev->dev, "VIDIOCGMBUF\n");
+
+	vm->size = dev->nbuffers * dev->len_per_image;
+	vm->frames = dev->nbuffers;
+
+	for (i = 0; i < dev->nbuffers; i++)
+		vm->offsets[i] = i * dev->len_per_image;
+
+	return 0;
+}
+#else
+#define stk11xx_gmbuf NULL
+#endif
+
+static struct file_operations stk11xx_fops = {
+	.owner = THIS_MODULE,
+	.open = stk11xx_open,
+	.release = stk11xx_release,
+	.read = stk11xx_read,
+	.poll = stk11xx_poll,
+	.mmap = stk11xx_mmap,
+	.ioctl = video_ioctl2
+};
+
+static const struct video_device stk11xx_vdev_template = {
+	.name = DRIVER_DESC,
+
+	.type2 = VID_TYPE_CAPTURE,
+	.tvnorms = V4L2_STD_UNKNOWN,
+	.current_norm = V4L2_STD_UNKNOWN,
+	.fops = &stk11xx_fops,
+	.release = video_device_release,
+	.minor = -1,
+
+	.vidioc_querycap = stk11xx_querycap,
+	.vidioc_enum_input = stk11xx_enum_input,
+	.vidioc_g_input = stk11xx_g_input,
+	.vidioc_s_input = stk11xx_s_input,
+	.vidioc_queryctrl = stk11xx_queryctrl,
+	.vidioc_g_ctrl = stk11xx_g_ctrl,
+	.vidioc_s_ctrl = stk11xx_s_ctrl,
+	.vidioc_enum_fmt_cap = stk11xx_enum_fmt_cap,
+	.vidioc_g_fmt_cap = stk11xx_g_fmt_cap,
+	.vidioc_s_fmt_cap = stk11xx_s_fmt_cap,
+	.vidioc_try_fmt_cap = stk11xx_try_fmt_cap,
+	.vidioc_querystd = stk11xx_querystd,
+	.vidioc_s_std = stk11xx_s_std,
+	.vidioc_reqbufs = stk11xx_reqbufs,
+	.vidioc_querybuf = stk11xx_querybuf,
+	.vidioc_qbuf = stk11xx_qbuf,
+	.vidioc_dqbuf = stk11xx_dqbuf,
+	.vidioc_streamon = stk11xx_streamon,
+	.vidioc_streamoff = stk11xx_streamoff,
+	.vidioc_g_parm = stk11xx_g_parm,
+
+	.vidiocgmbuf = stk11xx_gmbuf,
+};
+
+/*
+ * sysfs stuff
+ */
+
+static ssize_t show_brightness(struct class_device *class, char *buf)
+{
+	struct stk11xx *dev = video_get_drvdata(to_video_device(class));
+
+	return sprintf(buf, "%X\n", dev->vsettings.brightness);
+}
+
+static ssize_t show_contrast(struct class_device *class, char *buf)
+{
+	struct stk11xx *dev = video_get_drvdata(to_video_device(class));
+
+	return sprintf(buf, "%X\n", dev->vsettings.contrast);
+}
+
+static ssize_t show_whitebalance(struct class_device *class, char *buf)
+{
+	struct stk11xx *dev = video_get_drvdata(to_video_device(class));
+
+	return sprintf(buf, "%X\n", dev->vsettings.whiteness);
+}
[… more lines in the raw message]

Lines of this file are not shown. View the raw message.

The rest of this diff is not shown. View the raw message.

*/
+/* the apis say it should be done, none of the video usb drivers are doing it */
+/* NICKLAS: Claiming the interface is usefull when the driver want manage severals interfaces. */

drivers/media/video/stk11xx.c new file +149 -1

Lines of this file are not shown. View the raw message.

The rest of this diff is not shown. View the raw message.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 1 line
Markus Rechberger

Part of the diff is not shown here. View the raw message.

author
#2 lore raw
Quoted 14 lines · Markus Rechberger

Hi Jiri,

Quoted 11 lines · Jiri Slaby

Well, no objections on v4l list, try to merge it. Any further comments will be
appreciated.

--

stk11xx, add a new webcam driver

Cc Mauro Carvalho Chehab <[email protected]>

[...]

Quoted 6 lines
Quoted 6 lines
drivers/media/video/stk11xx.c new file +6 -0
+/*
+ * Bayer conversion
+ */
+
+#define STK11XX_AVG2(x,y) (u8)(((int)x + (int)y) / 2)
+#define STK11XX_AVG4(a,b,c,d) (u8)(((int)a + (int)b + (int)c + (int)d) / 4)
Quoted 8 lines

drivers/media/video/stk11xx.c line 255

Quoted 3 lines
drivers/media/video/stk11xx.c new file +3 -0
+
+static void stk11xx_bayer_to_rgb(u8 *rgb, const u8 *bayer,
+                       const int width, const int height)

hmm.. this is probably to support xawtv/tvtime?
It would be better to do that in userspace, but the argument which seems to be against it is that userspace applications often don't support bayer.

Exactly. This would make the webcam driver unusable. Another one, which awaits RGB is camstream, which also I use. After moving all major used userspace apps to something like pwlib, this may be wiped out.

Quoted 18 lines
Quoted 18 lines
drivers/media/video/stk11xx.c new file +18 -0
+{
+     unsigned int x, y, nwidth, nheight;
+
+     nheight = height / factor;
+     nwidth = width / factor;
+
+     for (y = 0; y < nheight; y++) {
+             for (x = 0; x < nwidth; x++) {
+                     /* R */
+                     out[y * nwidth * 3 + x * 3 + 0] =
+                         in[y * factor * width * 3 + x * factor * 3 + 0];
+                     /* G */
+                     out[y * nwidth * 3 + x * 3 + 1] =
+                         in[y * factor * width * 3 + x * factor * 3 + 1];
+                     /* B */
+                     out[y * nwidth * 3 + x * 3 + 2] =
+                         in[y * factor * width * 3 + x * factor * 3 + 2];
+             }
Quoted 8 lines

drivers/media/video/stk11xx.c line 488

Quoted 6 lines
drivers/media/video/stk11xx.c new file +6 -0
+     }
+}
+
+static void stk11xx_correct_brightness(u8 *rgb, unsigned int width,
+                             unsigned int height, int brightness)
+{

same here, why do you want to have that in kernelspace?

not sure who from userspace can do this nowadays. Will check.

Quoted 74 lines
Quoted 74 lines
drivers/media/video/stk11xx.c new file +74 -0
+/*
+ * The configuration of device is composed of 12 steps.
+ * This function is called by the initialization process.
+ *
+ * We don't know the meaning of these steps! We only replay the USB log.
+ */
+static int stk1135_configure_device(struct stk11xx *dev, int step)
+{
+     int value;
+
+     /*      0,    1,    2,    3,    4,    5,    6,    7,    8,    9,
+                 10,   11,   12,   13 */
+
+     const u8 values_001B[] = {
+             0x0E, 0x03, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x07,
+                 0x07, 0x07, 0x07, 0x07
+     };
+     const u8 values_001C[] = {
+             0x06, 0x02, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46, 0x06, 0x06,
+                 0x06, 0x06, 0x06, 0x07
+     };
+     const u8 values_0202[] = {
+             0x1E, 0x0A, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E,
+                 0x1E, 0x1E, 0x1E, 0x1E
+     };
+     const u8 values_0110[] = {
+             0x07, 0x00, 0x00, 0x00, 0x00, 0x3E, 0x3E, 0x00, 0x04, 0x00,
+                 0x00, 0x00, 0x00, 0x00
+     };
+     const u8 values_0112[] = {
+             0x07, 0x00, 0x00, 0x00, 0x00, 0x09, 0x09, 0x00, 0x04, 0x00,
+                 0x00, 0x00, 0x00, 0x00
+     };
+     const u8 values_0114[] = {
+             0x87, 0x80, 0x80, 0x80, 0x80, 0xBE, 0xBE, 0x80, 0x84, 0x80,
+                 0x80, 0x80, 0x80, 0x80
+     };
+     const u8 values_0116[] = {
+             0xE7, 0xE0, 0xE0, 0xE0, 0xE0, 0xE9, 0xE9, 0xE0, 0xE4, 0xE0,
+                 0xE0, 0xE0, 0xE0, 0xE0
+     };
+     const u8 values_0100[] = {
+             0x20, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x23, 0x20,
+                 0x20, 0x20, 0x20, 0x20
+     };
+
+     dev_dbg(&dev->udev->dev, "stk1135_configure_device: %d\n", step);
+
+     stk11xx_write_registry(dev, 0x0000, 0x0024);
+     stk11xx_write_registry(dev, 0x0002, 0x0068);
+     stk11xx_write_registry(dev, 0x0003, 0x0080);
+     stk11xx_write_registry(dev, 0x0005, 0x0000);
+
+     stk11xx_write_registry(dev, 0x0007, 0x0003);
+     stk11xx_write_registry(dev, 0x000d, 0x0000);
+     stk11xx_write_registry(dev, 0x000f, 0x0002);
+     stk11xx_write_registry(dev, 0x0300, 0x0012);
+     stk11xx_write_registry(dev, 0x0350, 0x0041);
+
+     stk11xx_write_registry(dev, 0x0351, 0x0000);
+     stk11xx_write_registry(dev, 0x0352, 0x0000);
+     stk11xx_write_registry(dev, 0x0353, 0x0000);
+     stk11xx_write_registry(dev, 0x0018, 0x0010);
+     stk11xx_write_registry(dev, 0x0019, 0x0000);
+
+     stk11xx_write_registry(dev, 0x001b, values_001B[step]);
+     stk11xx_write_registry(dev, 0x001c, values_001C[step]);
+     stk11xx_write_registry(dev, 0x0300, 0x0080);
+     stk11xx_write_registry(dev, 0x001a, 0x0004);
+     stk11xx_write_registry(dev, 0x0202, values_0202[step]);
+
+     stk11xx_write_registry(dev, 0x0110, values_0110[step]);
+     stk11xx_write_registry(dev, 0x0111, 0x0000);
+     stk11xx_write_registry(dev, 0x0112, values_0112[step]);
Quoted 8 lines

drivers/media/video/stk11xx.c line 1082

Quoted 3 lines
drivers/media/video/stk11xx.c new file +3 -0
+     stk11xx_write_registry(dev, 0x0113, 0x0000);
+     stk11xx_write_registry(dev, 0x0114, values_0114[step]);
+

if possible some error checking would be nice here, if you unplug the device you'll rely on the underlying subsystem to handle your errors I don't think that's a good way to go.
Did you test unplugging your device while it's in use with mplayer/xawtv/..?

hmm, no. It's physically impossible -- it's built in. Is there any possibility of sw unplugging, e.g. writing something somewhere to /proc or /sys?

thanks,

Signature · 4 lines
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E
#3 lore raw
Quoted 161 lines · Jiri Slaby
Quoted 4 lines · Markus Rechberger

Hi Jiri,

Quoted 1 line · Jiri Slaby

Well, no objections on v4l list, try to merge it. Any further comments

will

Quoted 10 lines
Quoted 10 lines

be
appreciated.

--

stk11xx, add a new webcam driver

Cc Mauro Carvalho Chehab <[email protected]>

[...]

Quoted 6 lines

drivers/media/video/stk11xx.c

Quoted 6 lines
drivers/media/video/stk11xx.c new file +6 -0
+/*
+ * Bayer conversion
+ */
+
+#define STK11XX_AVG2(x,y) (u8)(((int)x + (int)y) / 2)
+#define STK11XX_AVG4(a,b,c,d) (u8)(((int)a + (int)b + (int)c + (int)d)

/ 4)

Quoted 8 lines

drivers/media/video/stk11xx.c line 255

Quoted 3 lines
drivers/media/video/stk11xx.c new file +3 -0
+
+static void stk11xx_bayer_to_rgb(u8 *rgb, const u8 *bayer,
+                       const int width, const int height)

hmm.. this is probably to support xawtv/tvtime?
It would be better to do that in userspace, but the argument which seems to be against it is that userspace applications often don't support bayer.

Exactly. This would make the webcam driver unusable. Another one, which awaits RGB is camstream, which also I use. After moving all major used userspace apps to something like pwlib, this may be wiped out.

Quoted 11 lines

drivers/media/video/stk11xx.c

Quoted 11 lines
drivers/media/video/stk11xx.c new file +11 -0
+{
+     unsigned int x, y, nwidth, nheight;
+
+     nheight = height / factor;
+     nwidth = width / factor;
+
+     for (y = 0; y < nheight; y++) {
+             for (x = 0; x < nwidth; x++) {
+                     /* R */
+                     out[y * nwidth * 3 + x * 3 + 0] =
+                         in[y * factor * width * 3 + x * factor * 3 +

0];

Quoted 3 lines

drivers/media/video/stk11xx.c

Quoted 3 lines
drivers/media/video/stk11xx.c new file +3 -0
+                     /* G */
+                     out[y * nwidth * 3 + x * 3 + 1] =
+                         in[y * factor * width * 3 + x * factor * 3 +

1];

Quoted 3 lines

drivers/media/video/stk11xx.c

Quoted 3 lines
drivers/media/video/stk11xx.c new file +3 -0
+                     /* B */
+                     out[y * nwidth * 3 + x * 3 + 2] =
+                         in[y * factor * width * 3 + x * factor * 3 +

2];

Quoted 9 lines

drivers/media/video/stk11xx.c line 488

Quoted 7 lines
drivers/media/video/stk11xx.c new file +7 -0
+             }
+     }
+}
+
+static void stk11xx_correct_brightness(u8 *rgb, unsigned int width,
+                             unsigned int height, int brightness)
+{

same here, why do you want to have that in kernelspace?

not sure who from userspace can do this nowadays. Will check.

Quoted 5 lines
Quoted 5 lines

+/*
+ * The configuration of device is composed of 12 steps.
+ * This function is called by the initialization process.
+ *
+ * We don't know the meaning of these steps! We only replay the USB

log.

Quoted 10 lines

drivers/media/video/stk11xx.c

Quoted 10 lines

+ */

drivers/media/video/stk11xx.c new file +9 -0
+static int stk1135_configure_device(struct stk11xx *dev, int step)
+{
+     int value;
+
+     /*      0,    1,    2,    3,    4,    5,    6,    7,    8,    9,
+                 10,   11,   12,   13 */
+
+     const u8 values_001B[] = {
+             0x0E, 0x03, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E,

0x07,

Quoted 4 lines

drivers/media/video/stk11xx.c

Quoted 4 lines
drivers/media/video/stk11xx.c new file +4 -0
+                 0x07, 0x07, 0x07, 0x07
+     };
+     const u8 values_001C[] = {
+             0x06, 0x02, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46, 0x06,

0x06,

Quoted 4 lines

drivers/media/video/stk11xx.c

Quoted 4 lines
drivers/media/video/stk11xx.c new file +4 -0
+                 0x06, 0x06, 0x06, 0x07
+     };
+     const u8 values_0202[] = {
+             0x1E, 0x0A, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E,

0x1E,

Quoted 4 lines

drivers/media/video/stk11xx.c

Quoted 4 lines
drivers/media/video/stk11xx.c new file +4 -0
+                 0x1E, 0x1E, 0x1E, 0x1E
+     };
+     const u8 values_0110[] = {
+             0x07, 0x00, 0x00, 0x00, 0x00, 0x3E, 0x3E, 0x00, 0x04,

0x00,

Quoted 4 lines

drivers/media/video/stk11xx.c

Quoted 4 lines
drivers/media/video/stk11xx.c new file +4 -0
+                 0x00, 0x00, 0x00, 0x00
+     };
+     const u8 values_0112[] = {
+             0x07, 0x00, 0x00, 0x00, 0x00, 0x09, 0x09, 0x00, 0x04,

0x00,

Quoted 4 lines

drivers/media/video/stk11xx.c

Quoted 4 lines
drivers/media/video/stk11xx.c new file +4 -0
+                 0x00, 0x00, 0x00, 0x00
+     };
+     const u8 values_0114[] = {
+             0x87, 0x80, 0x80, 0x80, 0x80, 0xBE, 0xBE, 0x80, 0x84,

0x80,

Quoted 4 lines

drivers/media/video/stk11xx.c

Quoted 4 lines
drivers/media/video/stk11xx.c new file +4 -0
+                 0x80, 0x80, 0x80, 0x80
+     };
+     const u8 values_0116[] = {
+             0xE7, 0xE0, 0xE0, 0xE0, 0xE0, 0xE9, 0xE9, 0xE0, 0xE4,

0xE0,

Quoted 4 lines

drivers/media/video/stk11xx.c

Quoted 4 lines
drivers/media/video/stk11xx.c new file +4 -0
+                 0xE0, 0xE0, 0xE0, 0xE0
+     };
+     const u8 values_0100[] = {
+             0x20, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x23,

0x20,

Quoted 31 lines
Quoted 31 lines
drivers/media/video/stk11xx.c new file +31 -0
+                 0x20, 0x20, 0x20, 0x20
+     };
+
+     dev_dbg(&dev->udev->dev, "stk1135_configure_device: %d\n", step);
+
+     stk11xx_write_registry(dev, 0x0000, 0x0024);
+     stk11xx_write_registry(dev, 0x0002, 0x0068);
+     stk11xx_write_registry(dev, 0x0003, 0x0080);
+     stk11xx_write_registry(dev, 0x0005, 0x0000);
+
+     stk11xx_write_registry(dev, 0x0007, 0x0003);
+     stk11xx_write_registry(dev, 0x000d, 0x0000);
+     stk11xx_write_registry(dev, 0x000f, 0x0002);
+     stk11xx_write_registry(dev, 0x0300, 0x0012);
+     stk11xx_write_registry(dev, 0x0350, 0x0041);
+
+     stk11xx_write_registry(dev, 0x0351, 0x0000);
+     stk11xx_write_registry(dev, 0x0352, 0x0000);
+     stk11xx_write_registry(dev, 0x0353, 0x0000);
+     stk11xx_write_registry(dev, 0x0018, 0x0010);
+     stk11xx_write_registry(dev, 0x0019, 0x0000);
+
+     stk11xx_write_registry(dev, 0x001b, values_001B[step]);
+     stk11xx_write_registry(dev, 0x001c, values_001C[step]);
+     stk11xx_write_registry(dev, 0x0300, 0x0080);
+     stk11xx_write_registry(dev, 0x001a, 0x0004);
+     stk11xx_write_registry(dev, 0x0202, values_0202[step]);
+
+     stk11xx_write_registry(dev, 0x0110, values_0110[step]);
+     stk11xx_write_registry(dev, 0x0111, 0x0000);
+     stk11xx_write_registry(dev, 0x0112, values_0112[step]);
Quoted 8 lines

drivers/media/video/stk11xx.c line 1082

Quoted 3 lines
drivers/media/video/stk11xx.c new file +3 -0
+     stk11xx_write_registry(dev, 0x0113, 0x0000);
+     stk11xx_write_registry(dev, 0x0114, values_0114[step]);
+

if possible some error checking would be nice here, if you unplug the device you'll rely on the underlying subsystem to handle your errors I don't think that's a good way to go.
Did you test unplugging your device while it's in use with

Quoted 6 lines

mplayer/xawtv/..?

hmm, no. It's physically impossible -- it's built in. Is there any possibility of sw unplugging, e.g. writing something somewhere to /proc or /sys?

You can use the unbind option of the usb framework to disconnect the driver while it's in use.
Your device driver should be listed in /sys/bus/usb/drivers, change the directory into your driver's directory and list the files which are in it.
It should show up a numeric symlink (like 4-1:1.0) which is linked to the corresponding usb device, you can unbind it by echo'ing that name into unbind eg.

$ echo -n 4-1:1.0 > unbind

Markus

Quoted 7 lines

thanks,
--
http://www.fi.muni.cz/~xslaby/ Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8 22A0 32CC 55C3 39D4 7A7E

Signature · 1 line
Markus Rechberger
author
#4 lore raw
In reply to Markus Rechberger
Quoted 10 lines · Markus Rechberger

Hi Jiri,

Quoted 7 lines · Jiri Slaby

Well, no objections on v4l list, try to merge it. Any further comments will be
appreciated.

--

stk11xx, add a new webcam driver

[...]

Quoted 11 lines

drivers/media/video/stk11xx.c line 1249

Quoted 8 lines
drivers/media/video/stk11xx.c new file +8 -0
+static int stk1125_camera_asleep(struct stk11xx *dev)
+{
+     int value;
+
+     stk11xx_read_registry(dev, 0x0104, &value);
+     stk11xx_read_registry(dev, 0x0105, &value);
+     stk11xx_read_registry(dev, 0x0106, &value);
+

why do you read these values (this is also something in the ongoing code I see, the read value just gets overwritten all the time)?

Well, as I tested, reads are neccesary, otherwise it doesn't work. And when they are needed, you need to read the value to some place in memory -- the &value.

thanks,

Signature · 4 lines
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E
#5 lore raw
Quoted 15 lines · Jiri Slaby
Quoted 1 line · Markus Rechberger

On 5/24/07, Jiri Slaby <[email protected]> wrote:

[...]

Quoted 11 lines

drivers/media/video/stk11xx.c line 1249

Quoted 8 lines
drivers/media/video/stk11xx.c new file +8 -0
+static int stk1125_camera_asleep(struct stk11xx *dev)
+{
+     int value;
+
+     stk11xx_read_registry(dev, 0x0104, &value);
+     stk11xx_read_registry(dev, 0x0105, &value);
+     stk11xx_read_registry(dev, 0x0106, &value);
+

why do you read these values (this is also something in the ongoing code I see, the read value just gets overwritten all the time)?

Quoted 3 lines

Well, as I tested, reads are neccesary, otherwise it doesn't work. And when they are needed, you need to read the value to some place in memory -- the &value.

Perhaps a few comments are warranted here and there about why some of those less obvious things are done.

Or do it as more or less self-documenting code like

static inline void stk11xx_unjam_registry(struct stk11xx *dev, u16 index)

{
	int dummy;

	stk11xx_read_registry(dev, index, &dummy);
}
Signature · 3 lines
Stefan Richter
-=====-=-=== -=-= ==--=
http://arcgraph.de/sr/
author
#6 lore raw
Quoted 25 lines · Stefan Richter
Quoted 15 lines · Jiri Slaby
Quoted 1 line · Markus Rechberger

On 5/24/07, Jiri Slaby <[email protected]> wrote:

[...]

Quoted 11 lines

drivers/media/video/stk11xx.c line 1249

Quoted 8 lines
drivers/media/video/stk11xx.c new file +8 -0
+static int stk1125_camera_asleep(struct stk11xx *dev)
+{
+     int value;
+
+     stk11xx_read_registry(dev, 0x0104, &value);
+     stk11xx_read_registry(dev, 0x0105, &value);
+     stk11xx_read_registry(dev, 0x0106, &value);
+

why do you read these values (this is also something in the ongoing code I see, the read value just gets overwritten all the time)?

Quoted 3 lines

Well, as I tested, reads are neccesary, otherwise it doesn't work. And when they are needed, you need to read the value to some place in memory -- the &value.

Perhaps a few comments are warranted here and there about why some of those less obvious things are done.

Or do it as more or less self-documenting code like

Quoted 7 lines

static inline void stk11xx_unjam_registry(struct stk11xx *dev, u16 index)

{
        int dummy;

        stk11xx_read_registry(dev, index, &dummy);
}

yeah, seems reasonable to me.

thanks,

Signature · 4 lines
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E
#7 lore raw
In reply to Jiri Slaby

drivers/media/video/Kconfig config USB_ZR364XX · line 801

Quoted 7 lines · Jiri Slaby
drivers/media/video/Kconfig +7 -0
+config USB_STK11XX
+	tristate "STK11XX based webcams"
+	depends on VIDEO_V4L2
+	---help---
+	  This will add support for Syntek webcams such as dc1125 and stk1135.
+
+	  If you choose to build it as module, it will be called stk11xx.

Maybe this is a too picky requeriment, but IMO it would be nice if the module would be called "camera_stk11xx", or had any other prefix that gives some meaning about what is doing. It's hard to decipher from the name when you run lsmod.

#8 lore raw
Quoted 1 line · On Thursday 24 May 2007 20:38:05 Diego Calleja wrote:

El Thu, 24 May 2007 16:01:33 +0200 (CEST), Jiri Slaby <[email protected]>

Quoted 4 lines · escribió:
Quoted 4 lines
drivers/media/video/Kconfig +4 -0
+config USB_STK11XX
+	tristate "STK11XX based webcams"
+	depends on VIDEO_V4L2
+	---help---
Quoted 8 lines

drivers/media/video/Kconfig config USB_ZR364XX · line 801

Quoted 3 lines
drivers/media/video/Kconfig +3 -0
+	  This will add support for Syntek webcams such as dc1125 and stk1135.
+
+	  If you choose to build it as module, it will be called stk11xx.

Maybe this is a too picky requeriment, but IMO it would be nice if the module would be called "camera_stk11xx", or had any other prefix that gives some meaning about what is doing. It's hard to decipher from the name when you run lsmod.

That would be too picky indeed since other camera modules have no camera_ prefix like pwc,spca5xx, etc.

Regards,
ismail

Signature · 1 line
Perfect is the enemy of good
#9 lore raw
Quoted 16 lines · Ismail Dönmez
Quoted 1 line · On Thursday 24 May 2007 20:38:05 Diego Calleja wrote:

El Thu, 24 May 2007 16:01:33 +0200 (CEST), Jiri Slaby <[email protected]>

Quoted 4 lines · escribió:
Quoted 4 lines
drivers/media/video/Kconfig +4 -0
+config USB_STK11XX
+	tristate "STK11XX based webcams"
+	depends on VIDEO_V4L2
+	---help---
Quoted 8 lines

drivers/media/video/Kconfig config USB_ZR364XX · line 801

Quoted 3 lines
drivers/media/video/Kconfig +3 -0
+	  This will add support for Syntek webcams such as dc1125 and stk1135.
+
+	  If you choose to build it as module, it will be called stk11xx.

Maybe this is a too picky requeriment, but IMO it would be nice if the module would be called "camera_stk11xx", or had any other prefix that gives some meaning about what is doing. It's hard to decipher from the name when you run lsmod.

Quoted 2 lines

That would be too picky indeed since other camera modules have no camera_ prefix like pwc,spca5xx, etc.

Then how about adding that prefix to all of them and provide the current name as module alias?

Or at least, why not use better names for new modules?

Signature · 3 lines
Stefan Richter
-=====-=-=== -=-= ==--=
http://arcgraph.de/sr/

A message that is not in the archive (sent off-list or not yet received): [email protected]

#11 lore raw
Quoted 11 lines · Jiri Slaby
Quoted 10 lines · Stefan Richter
Quoted 8 lines · Ismail Dönmez
Quoted 4 lines · On Thursday 24 May 2007 20:38:05 Diego Calleja wrote:

Maybe this is a too picky requeriment, but IMO it would be nice if the module would be called "camera_stk11xx", or had any other prefix that gives some meaning about what is doing. It's hard to decipher from the name when you run lsmod.

That would be too picky indeed since other camera modules have no camera_ prefix like pwc,spca5xx, etc.

Quoted 8 lines
Quoted 4 lines

Then how about adding that prefix to all of them and provide the current name as module alias?

Or at least, why not use better names for new modules?

Just because modules can serve more that one interface of the device? (audio/mic, camera), (remote controller, dvb grabber). I didn't get the point of having the prefix in the name, what's the matter?

A prefix which describes the function of a driver is of course only useful if it /correctly/ describes it. What would be /correct/? If a driver for a audio + video capable camera supports either both audio and video capture or only video capture, I don't see anything wrong with a prefix like "camera".

That said, we have "modinfo" at our disposal if we spot something nondescript in lsmod's listing.

Signature · 3 lines
Stefan Richter
-=====-=-=== -=-= ==--=
http://arcgraph.de/sr/
#12 lore raw
In reply to Jiri Slaby

Hi Jiri,

I have some comments for your driver.

Quoted 1 line

+ * Copyright (C) Nicolas VIVIEN

It would be interesting to have Nicolas SOB as well, if possible.

drivers/media/video/stk11xx.c line 72

Quoted 6 lines
drivers/media/video/stk11xx.c new file +6 -0
+
+#ifndef CONFIG_STK11XX_DEBUG_STREAM
+#define CONFIG_STK11XX_DEBUG_STREAM	0
+#endif
+
+#if CONFIG_STK11XX_DEBUG_STREAM

I would instead use:
#ifdef CONFIG_STK11XX_DEBUG_STREAM

Quoted 3 lines

+/*
+ * Bayer conversion
+ */

We don't do format conversions in kernel. Instead, you should return a proper Bayer Fourcc format (like V4L2_PIX_FMT_SBGGR8).

drivers/media/video/stk11xx.c line 464

Quoted 2 lines
drivers/media/video/stk11xx.c new file +2 -0
+static void stk11xx_resize_image(u8 *out, const u8 *in,
+		unsigned int width, unsigned int height, unsigned int factor)

Also, kernel shouldn't be doing image resize.

drivers/media/video/stk11xx.c line 504

Quoted 2 lines
drivers/media/video/stk11xx.c new file +2 -0
+static int stk11xx_decompress(struct stk11xx *dev)
+{

We don't do format conversions in kernel. Instead, you should return a proper Bayer Fourcc format (like V4L2_PIX_FMT_SBGGR8).

Quoted 31 lines
drivers/media/video/stk11xx.c new file +31 -0
+static int stk1125_load_microcode(struct stk11xx *dev)
+{
+	unsigned int i;
+	const u8 *values_204, *values_205;
+	int retok, value;
+
+	/* From 80x60 to 640x480 */
+	const u8 values_1_204[] = {
+		0x12, 0x11, 0x3b, 0x6a, 0x13, 0x10, 0x00, 0x01, 0x02, 0x13,
+		0x39, 0x38, 0x37, 0x35, 0x0e, 0x12, 0x04, 0x0c, 0x0d, 0x17,
+		0x18, 0x32, 0x19, 0x1a, 0x03, 0x1b, 0x16, 0x33, 0x34, 0x41,
+		0x96, 0x3d, 0x69, 0x3a, 0x8e, 0x3c, 0x8f, 0x8b, 0x8c, 0x94,
+		0x95, 0x40, 0x29, 0x0f, 0xa5, 0x1e, 0xa9, 0xaa, 0xab, 0x90,
+		0x91, 0x9f, 0xa0, 0x24, 0x25, 0x26, 0x14, 0x2a, 0x2b
+	};
+	const u8 values_1_205[] = {
+		0x45, 0x80, 0x01, 0x7d, 0x80, 0x00, 0x00, 0x80, 0x80, 0x80,
+		0x50, 0x93, 0x00, 0x81, 0x20, 0x45, 0x00, 0x00, 0x00, 0x24,
+		0xc4, 0xb6, 0x00, 0x3c, 0x36, 0x00, 0x07, 0xe2, 0xbf, 0x00,
+		0x04, 0x19, 0x40, 0x0d, 0x00, 0x73, 0xdf, 0x06, 0x20, 0x88,
+		0x88, 0xc1, 0x3f, 0x42, 0x80, 0x04, 0xb8, 0x92, 0x0a, 0x00,
+		0x00, 0x00, 0x00, 0x68, 0x5c, 0xc3, 0x2e, 0x00, 0x00
+	};
+
+	/* From 800x600 to 1280x1024 */
+	const u8 values_2_204[] = {
+		0x12, 0x11, 0x3b, 0x6a, 0x13, 0x10, 0x00, 0x01, 0x02, 0x13,
+		0x39, 0x38, 0x37, 0x35, 0x0e, 0x12, 0x04, 0x0c, 0x0d, 0x17,
+		0x18, 0x32, 0x19, 0x1a, 0x03, 0x1b, 0x16, 0x33, 0x34, 0x41,
+		0x96, 0x3d, 0x69, 0x3a, 0x8e, 0x3c, 0x8f, 0x8b, 0x8c, 0x94,
+		0x95, 0x40, 0x29, 0x0f, 0xa5, 0x1e, 0xa9, 0xaa, 0xab, 0x90,

drivers/media/video/stk11xx.c line 950

Quoted 10 lines
drivers/media/video/stk11xx.c new file +10 -0
+		0x91, 0x9f, 0xa0, 0x24, 0x25, 0x26, 0x14, 0x2a, 0x2b
+	};
+	const u8 values_2_205[] = {
+		0x05, 0x80, 0x01, 0x7d, 0x80, 0x00, 0x00, 0x80, 0x80, 0x80,
+		0x50, 0x93, 0x00, 0x81, 0x20, 0x05, 0x00, 0x00, 0x00, 0x1b,
+		0xbb, 0xa4, 0x01, 0x81, 0x12, 0x00, 0x07, 0xe2, 0xbf, 0x00,
+		0x04, 0x19, 0x40, 0x0d, 0x00, 0x73, 0xdf, 0x06, 0x20, 0x88,
+		0x88, 0xc1, 0x3f, 0x42, 0x80, 0x04, 0xb8, 0x92, 0x0a, 0x00,
+		0x00, 0x00, 0x00, 0x68, 0x5c, 0xc3, 0x2e, 0x00, 0x00
+	};

Please use instead the load_firmware routines. It is not a good idea to have firmware inside the kernel. Also, this might rise some legal issues due to licensing models.

Quoted 56 lines
drivers/media/video/stk11xx.c new file +56 -0
+
+/* 
+ * The configuration of device is composed of 11 steps.
+ * This function is called by the initialization process.
+ *
+ * We don't know the meaning of these steps! We only replay the USB log.
+ *
+ * The steps 0 to 9 are called during the initialization.
+ * Then, the driver choose the last step :
+ *   10 : for a resolution from 80x60 to 640x480 
+ *   11 : for a resolution from 800x600 to 1280x1024
+ */
+static int stk1125_configure_device(struct stk11xx *dev, int step)
+{
+	int value;
+
+	/*      0,    1,    2,    3,    4,    5,    6,    7,    8,    9, 
+		    10,   11 */
+
+	const u8 values_001B[] = {
+		0x0E, 0x03, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E,
+		    0x0E, 0x0E
+	};
+	const u8 values_001C[] = {
+		0x06, 0x02, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46,
+		    0x46, 0x0E
+	};
+	const u8 values_0202[] = {
+		0x1E, 0x0A, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E,
+		    0x1E, 0x1E
+	};
+	const u8 values_0110[] = {
+		0x07, 0x00, 0x00, 0x00, 0x00, 0x3E, 0x3E, 0x00, 0x00, 0x00,
+		    0x00, 0x00
+	};
+	const u8 values_0112[] = {
+		0x07, 0x00, 0x00, 0x00, 0x00, 0x09, 0x09, 0x00, 0x00, 0x00,
+		    0x00, 0x00
+	};
+	const u8 values_0114[] = {
+		0x87, 0x80, 0x80, 0x80, 0x80, 0xBE, 0xBE, 0x80, 0x80, 0x80,
+		    0x80, 0x00
+	};
+	const u8 values_0115[] = {
+		0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02,
+		    0x02, 0x05
+	};
+	const u8 values_0116[] = {
+		0xE7, 0xE0, 0xE0, 0xE0, 0xE0, 0xE9, 0xE9, 0xE0, 0xE0, 0xE0,
+		    0xE0, 0x00
+	};
+	const u8 values_0117[] = {
+		0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
+		    0x01, 0x04
+	};
+	const u8 values_0100[] = {

drivers/media/video/stk11xx.c line 1057

Quoted 10 lines
drivers/media/video/stk11xx.c new file +10 -0
+		0x20, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21,
+		    0x21, 0x21
+	};
+
+	dev_dbg(&dev->udev->dev, "stk1125_configure_device: %d\n", step);
+
+	stk11xx_write_registry(dev, 0x0000, 0x0024);
+	stk11xx_write_registry(dev, 0x0002, 0x0068);
+	stk11xx_write_registry(dev, 0x0003, 0x0080);
+	stk11xx_write_registry(dev, 0x0005, 0x0000);
	...

Instead of using all those write, you should consider creating a table of values and use something like:

	stk11xx_write_regs(dev, table1);
Quoted 28 lines
drivers/media/video/stk11xx.c new file +27 -0
+/*
+ * STK-1135 API
+ */
+
+static int stk1135_load_microcode(struct stk11xx *dev)
+{
+	unsigned int i;
+	int retok, value;
 
+
+	const u8 values_204[] = {
+		0x17, 0x19, 0xb4, 0xa6, 0x12, 0x13, 0x1e, 0x21, 0x24, 0x32,
+		0x36, 0x39, 0x4d, 0x53, 0x5d, 0x5f, 0x60, 0x61, 0x62, 0x63,
+		0x64, 0x65, 0x66, 0x82, 0x83, 0x85, 0x86, 0x89, 0x97, 0x98,
+		0xad, 0xae, 0xb8, 0xb9, 0xba, 0xbb, 0xbc, 0xbf, 0x48, 0xd8,
+		0x76, 0x77, 0x78, 0x79, 0x7a, 0x7b, 0x7c, 0x7d, 0x7e, 0x7f,
+		0x80, 0x81, 0xd8, 0x76, 0x77, 0x78, 0x79, 0x7a, 0x7b, 0x7c,
+		0x7d, 0x7e, 0x7f, 0x80, 0x81, 0xd8, 0x76, 0x77, 0x78, 0x79,
+		0x7a, 0x7b, 0x7c, 0x7d, 0x7e, 0x7f, 0x80, 0x81, 0x5c, 0xc0,
+		0x59, 0x5a, 0x5b, 0xd4, 0x8e, 0x8f, 0x90, 0x91, 0x92, 0x93,
+		0x94, 0x95, 0x96, 0xb3, 0x73, 0x06, 0x07, 0x0b, 0x15, 0x20,
+		0x4e, 0x4f, 0x49, 0x4a, 0x4b, 0x4c, 0x46, 0x06, 0x07, 0xb9,
+		0xba, 0xbb, 0xbc, 0x61, 0x62, 0x65, 0x66
+	};
+	const u8 values_205[] = {
+		0x41, 0x41, 0x03, 0x06, 0x06, 0x08, 0x06, 0x00, 0x02, 0x69,
+		0x35, 0x60, 0xfe, 0x1c, 0x04, 0x08, 0x08, 0x08, 0x08, 0x00,
+		0x00, 0x10, 0x14, 0x01, 0x80, 0x0c, 0xb6, 0x00, 0x25, 0x25,

drivers/media/video/stk11xx.c line 1484

Quoted 10 lines
drivers/media/video/stk11xx.c new file +10 -0
+		0x3f, 0x24, 0x10, 0x07, 0xcc, 0x1f, 0x30, 0x02, 0x9c, 0x80,
+		0x00, 0x0d, 0x18, 0x22, 0x2c, 0x3e, 0x4f, 0x6f, 0x8e, 0xac,
+		0xc8, 0xe5, 0xa0, 0x00, 0x0d, 0x18, 0x22, 0x2c, 0x3e, 0x4f,
+		0x6f, 0x8e, 0xac, 0xc8, 0xe5, 0xc0, 0x00, 0x0d, 0x18, 0x22,
+		0x2c, 0x3e, 0x4f, 0x6f, 0x8e, 0xac, 0xc8, 0xe5, 0x70, 0x18,
+		0x09, 0x07, 0x07, 0x3c, 0x3d, 0x95, 0x88, 0x89, 0x47, 0x9c,
+		0x81, 0x9c, 0x3d, 0x76, 0x76, 0x01, 0xf3, 0x05, 0x00, 0x44,
+		0x06, 0x0a, 0x96, 0x00, 0x7d, 0x00, 0x20, 0x01, 0xf3, 0x04,
+		0xe4, 0x09, 0xc8, 0x08, 0x08, 0x10, 0x14
+	};

Same as commented before about microcode.

You may also consider writing a separate c file for stk1135. Having a large .c file is not very nice. The better is to split the code into a few parts.

Quoted 1 line

+static void *stk11xx_rvmalloc(unsigned long size)

Another rvmalloc implementation? You should consider using the one already at kernel.

drivers/media/video/stk11xx.c line 2108

Quoted 2 lines
drivers/media/video/stk11xx.c new file +2 -0
+
+static void stk11xx_rvfree(void *mem, unsigned long size)

same as above.

drivers/media/video/stk11xx.c line 2554

Quoted 1 line
drivers/media/video/stk11xx.c new file +1 -0
+	cap->version = (u32)DRIVER_VERSION_NUM;

You should use, instead, KERNEL_VERSION macro.

drivers/media/video/stk11xx.c line 2069

Quoted 4 lines
drivers/media/video/stk11xx.c new file +4 -0
+       /* Create frame buffers and make circular ring */
+       for (i = 0; i < default_nbrframebuf; i++)
+               if (dev->framebuf[i].data == NULL) {
+                       dev->framebuf[i].data = vmalloc(STK11XX_FRAME_SIZE); 

STK11XX_FRAME_SIZE is defined previously as (1280 * 1024 * 3). Why to allocate so much memory, even if the user selects a lower format? You should, instead, allocate memory dynamically as needed by the user.

drivers/media/video/stk11xx.c line 2755

Quoted 11 lines

+static int stk11xx_querystd(struct file *file, void *fh, v4l2_std_id *a)

drivers/media/video/stk11xx.c new file +9 -0
+{
+	*a = V4L2_STD_UNKNOWN;
+	return 0;
+}
+
+static int stk11xx_s_std(struct file *file, void *fh, v4l2_std_id *a)
+{
+	return *a == V4L2_STD_UNKNOWN ? 0 : -EINVAL;
+}

This raises an interesting discussion about video standards. I would, instead, use V4L2_STD_ALL, since webcams are capable of properly handling on 525 raw lines/60Hz based standards (those derived from 640x480 res) as well as 625 raw lines/50Hz based standards (those derived from ITU-T CIF resolutions).

As reference, STD_ALL is defined as:

#define V4L2_STD_ALL            (V4L2_STD_525_60        |\
                                 V4L2_STD_625_50)

---

Also, I noticed that several hexadecimal values are using uppercases. The better is to convert all of them to lowercase, to keep the same codingstyle as the other drivers.

Signature · 2 lines
Cheers,
Mauro
#13 lore raw
Quoted 18 lines · Mauro Carvalho Chehab

Hi Jiri,

I have some comments for your driver.

Quoted 1 line

+ * Copyright (C) Nicolas VIVIEN

It would be interesting to have Nicolas SOB as well, if possible.

drivers/media/video/stk11xx.c line 72

Quoted 6 lines
drivers/media/video/stk11xx.c new file +6 -0
+
+#ifndef CONFIG_STK11XX_DEBUG_STREAM
+#define CONFIG_STK11XX_DEBUG_STREAM	0
+#endif
+
+#if CONFIG_STK11XX_DEBUG_STREAM

I would instead use:
#ifdef CONFIG_STK11XX_DEBUG_STREAM

Quoted 7 lines
Quoted 3 lines

+/*
+ * Bayer conversion
+ */

We don't do format conversions in kernel. Instead, you should return a proper Bayer Fourcc format (like V4L2_PIX_FMT_SBGGR8).

It's ok in his case since most userspace applications do not support this format, it helps noone if there's a driver out there which isn't supported by most userspace applications and I don't expect that all applications will add a conversion for that format soon. This would moreover raise the question about a userspace library, though there are many more important issues which are outstanding and which got smashed down.

Quoted 48 lines

drivers/media/video/stk11xx.c line 464

Quoted 2 lines
drivers/media/video/stk11xx.c new file +2 -0
+static void stk11xx_resize_image(u8 *out, const u8 *in,
+		unsigned int width, unsigned int height, unsigned int factor)

Also, kernel shouldn't be doing image resize.

drivers/media/video/stk11xx.c line 504

Quoted 2 lines
drivers/media/video/stk11xx.c new file +2 -0
+static int stk11xx_decompress(struct stk11xx *dev)
+{

We don't do format conversions in kernel. Instead, you should return a proper Bayer Fourcc format (like V4L2_PIX_FMT_SBGGR8).

Quoted 37 lines
drivers/media/video/stk11xx.c new file +37 -0
+static int stk1125_load_microcode(struct stk11xx *dev)
+{
+	unsigned int i;
+	const u8 *values_204, *values_205;
+	int retok, value;
+
+	/* From 80x60 to 640x480 */
+	const u8 values_1_204[] = {
+		0x12, 0x11, 0x3b, 0x6a, 0x13, 0x10, 0x00, 0x01, 0x02, 0x13,
+		0x39, 0x38, 0x37, 0x35, 0x0e, 0x12, 0x04, 0x0c, 0x0d, 0x17,
+		0x18, 0x32, 0x19, 0x1a, 0x03, 0x1b, 0x16, 0x33, 0x34, 0x41,
+		0x96, 0x3d, 0x69, 0x3a, 0x8e, 0x3c, 0x8f, 0x8b, 0x8c, 0x94,
+		0x95, 0x40, 0x29, 0x0f, 0xa5, 0x1e, 0xa9, 0xaa, 0xab, 0x90,
+		0x91, 0x9f, 0xa0, 0x24, 0x25, 0x26, 0x14, 0x2a, 0x2b
+	};
+	const u8 values_1_205[] = {
+		0x45, 0x80, 0x01, 0x7d, 0x80, 0x00, 0x00, 0x80, 0x80, 0x80,
+		0x50, 0x93, 0x00, 0x81, 0x20, 0x45, 0x00, 0x00, 0x00, 0x24,
+		0xc4, 0xb6, 0x00, 0x3c, 0x36, 0x00, 0x07, 0xe2, 0xbf, 0x00,
+		0x04, 0x19, 0x40, 0x0d, 0x00, 0x73, 0xdf, 0x06, 0x20, 0x88,
+		0x88, 0xc1, 0x3f, 0x42, 0x80, 0x04, 0xb8, 0x92, 0x0a, 0x00,
+		0x00, 0x00, 0x00, 0x68, 0x5c, 0xc3, 0x2e, 0x00, 0x00
+	};
+
+	/* From 800x600 to 1280x1024 */
+	const u8 values_2_204[] = {
+		0x12, 0x11, 0x3b, 0x6a, 0x13, 0x10, 0x00, 0x01, 0x02, 0x13,
+		0x39, 0x38, 0x37, 0x35, 0x0e, 0x12, 0x04, 0x0c, 0x0d, 0x17,
+		0x18, 0x32, 0x19, 0x1a, 0x03, 0x1b, 0x16, 0x33, 0x34, 0x41,
+		0x96, 0x3d, 0x69, 0x3a, 0x8e, 0x3c, 0x8f, 0x8b, 0x8c, 0x94,
+		0x95, 0x40, 0x29, 0x0f, 0xa5, 0x1e, 0xa9, 0xaa, 0xab, 0x90,
+		0x91, 0x9f, 0xa0, 0x24, 0x25, 0x26, 0x14, 0x2a, 0x2b
+	};
+	const u8 values_2_205[] = {
+		0x05, 0x80, 0x01, 0x7d, 0x80, 0x00, 0x00, 0x80, 0x80, 0x80,
+		0x50, 0x93, 0x00, 0x81, 0x20, 0x05, 0x00, 0x00, 0x00, 0x1b,
+		0xbb, 0xa4, 0x01, 0x81, 0x12, 0x00, 0x07, 0xe2, 0xbf, 0x00,
Quoted 8 lines

drivers/media/video/stk11xx.c line 932

Quoted 4 lines
drivers/media/video/stk11xx.c new file +4 -0
+		0x04, 0x19, 0x40, 0x0d, 0x00, 0x73, 0xdf, 0x06, 0x20, 0x88,
+		0x88, 0xc1, 0x3f, 0x42, 0x80, 0x04, 0xb8, 0x92, 0x0a, 0x00,
+		0x00, 0x00, 0x00, 0x68, 0x5c, 0xc3, 0x2e, 0x00, 0x00
+	};

Please use instead the load_firmware routines. It is not a good idea to have firmware inside the kernel. Also, this might rise some legal issues due to licensing models.

Jiri, are you allowed to include that microcode, did you get any information about this from the manufacturer which could allow the inclusion?
The sequences are rather small not putting it into extra firmware files would make life much easier for some users, on the other side if it raises legal issues Mauro's right with loading it from a file

Markus

Quoted 179 lines
Quoted 57 lines
drivers/media/video/stk11xx.c new file +56 -0
+
+/*
+ * The configuration of device is composed of 11 steps.
+ * This function is called by the initialization process.
+ *
+ * We don't know the meaning of these steps! We only replay the USB log.
+ *
+ * The steps 0 to 9 are called during the initialization.
+ * Then, the driver choose the last step :
+ *   10 : for a resolution from 80x60 to 640x480
+ *   11 : for a resolution from 800x600 to 1280x1024
+ */
+static int stk1125_configure_device(struct stk11xx *dev, int step)
+{
+	int value;
+
+	/*      0,    1,    2,    3,    4,    5,    6,    7,    8,    9,
+		    10,   11 */
+
+	const u8 values_001B[] = {
+		0x0E, 0x03, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E,
+		    0x0E, 0x0E
+	};
+	const u8 values_001C[] = {
+		0x06, 0x02, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46,
+		    0x46, 0x0E
+	};
+	const u8 values_0202[] = {
+		0x1E, 0x0A, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E,
+		    0x1E, 0x1E
+	};
+	const u8 values_0110[] = {
+		0x07, 0x00, 0x00, 0x00, 0x00, 0x3E, 0x3E, 0x00, 0x00, 0x00,
+		    0x00, 0x00
+	};
+	const u8 values_0112[] = {
+		0x07, 0x00, 0x00, 0x00, 0x00, 0x09, 0x09, 0x00, 0x00, 0x00,
+		    0x00, 0x00
+	};
+	const u8 values_0114[] = {
+		0x87, 0x80, 0x80, 0x80, 0x80, 0xBE, 0xBE, 0x80, 0x80, 0x80,
+		    0x80, 0x00
+	};
+	const u8 values_0115[] = {
+		0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02,
+		    0x02, 0x05
+	};
+	const u8 values_0116[] = {
+		0xE7, 0xE0, 0xE0, 0xE0, 0xE0, 0xE9, 0xE9, 0xE0, 0xE0, 0xE0,
+		    0xE0, 0x00
+	};
+	const u8 values_0117[] = {
+		0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
+		    0x01, 0x04
+	};
+	const u8 values_0100[] = {

drivers/media/video/stk11xx.c line 1057

Quoted 10 lines
drivers/media/video/stk11xx.c new file +10 -0
+		0x20, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21,
+		    0x21, 0x21
+	};
+
+	dev_dbg(&dev->udev->dev, "stk1125_configure_device: %d\n", step);
+
+	stk11xx_write_registry(dev, 0x0000, 0x0024);
+	stk11xx_write_registry(dev, 0x0002, 0x0068);
+	stk11xx_write_registry(dev, 0x0003, 0x0080);
+	stk11xx_write_registry(dev, 0x0005, 0x0000);
	...

Instead of using all those write, you should consider creating a table of values and use something like:

	stk11xx_write_regs(dev, table1);
Quoted 28 lines
drivers/media/video/stk11xx.c new file +27 -0
+/*
+ * STK-1135 API
+ */
+
+static int stk1135_load_microcode(struct stk11xx *dev)
+{
+	unsigned int i;
+	int retok, value;
 
+
+	const u8 values_204[] = {
+		0x17, 0x19, 0xb4, 0xa6, 0x12, 0x13, 0x1e, 0x21, 0x24, 0x32,
+		0x36, 0x39, 0x4d, 0x53, 0x5d, 0x5f, 0x60, 0x61, 0x62, 0x63,
+		0x64, 0x65, 0x66, 0x82, 0x83, 0x85, 0x86, 0x89, 0x97, 0x98,
+		0xad, 0xae, 0xb8, 0xb9, 0xba, 0xbb, 0xbc, 0xbf, 0x48, 0xd8,
+		0x76, 0x77, 0x78, 0x79, 0x7a, 0x7b, 0x7c, 0x7d, 0x7e, 0x7f,
+		0x80, 0x81, 0xd8, 0x76, 0x77, 0x78, 0x79, 0x7a, 0x7b, 0x7c,
+		0x7d, 0x7e, 0x7f, 0x80, 0x81, 0xd8, 0x76, 0x77, 0x78, 0x79,
+		0x7a, 0x7b, 0x7c, 0x7d, 0x7e, 0x7f, 0x80, 0x81, 0x5c, 0xc0,
+		0x59, 0x5a, 0x5b, 0xd4, 0x8e, 0x8f, 0x90, 0x91, 0x92, 0x93,
+		0x94, 0x95, 0x96, 0xb3, 0x73, 0x06, 0x07, 0x0b, 0x15, 0x20,
+		0x4e, 0x4f, 0x49, 0x4a, 0x4b, 0x4c, 0x46, 0x06, 0x07, 0xb9,
+		0xba, 0xbb, 0xbc, 0x61, 0x62, 0x65, 0x66
+	};
+	const u8 values_205[] = {
+		0x41, 0x41, 0x03, 0x06, 0x06, 0x08, 0x06, 0x00, 0x02, 0x69,
+		0x35, 0x60, 0xfe, 0x1c, 0x04, 0x08, 0x08, 0x08, 0x08, 0x00,
+		0x00, 0x10, 0x14, 0x01, 0x80, 0x0c, 0xb6, 0x00, 0x25, 0x25,

drivers/media/video/stk11xx.c line 1484

Quoted 10 lines
drivers/media/video/stk11xx.c new file +10 -0
+		0x3f, 0x24, 0x10, 0x07, 0xcc, 0x1f, 0x30, 0x02, 0x9c, 0x80,
+		0x00, 0x0d, 0x18, 0x22, 0x2c, 0x3e, 0x4f, 0x6f, 0x8e, 0xac,
+		0xc8, 0xe5, 0xa0, 0x00, 0x0d, 0x18, 0x22, 0x2c, 0x3e, 0x4f,
+		0x6f, 0x8e, 0xac, 0xc8, 0xe5, 0xc0, 0x00, 0x0d, 0x18, 0x22,
+		0x2c, 0x3e, 0x4f, 0x6f, 0x8e, 0xac, 0xc8, 0xe5, 0x70, 0x18,
+		0x09, 0x07, 0x07, 0x3c, 0x3d, 0x95, 0x88, 0x89, 0x47, 0x9c,
+		0x81, 0x9c, 0x3d, 0x76, 0x76, 0x01, 0xf3, 0x05, 0x00, 0x44,
+		0x06, 0x0a, 0x96, 0x00, 0x7d, 0x00, 0x20, 0x01, 0xf3, 0x04,
+		0xe4, 0x09, 0xc8, 0x08, 0x08, 0x10, 0x14
+	};

Same as commented before about microcode.

You may also consider writing a separate c file for stk1135. Having a large .c file is not very nice. The better is to split the code into a few parts.

Quoted 1 line

+static void *stk11xx_rvmalloc(unsigned long size)

Another rvmalloc implementation? You should consider using the one already at kernel.

drivers/media/video/stk11xx.c line 2108

Quoted 2 lines
drivers/media/video/stk11xx.c new file +2 -0
+
+static void stk11xx_rvfree(void *mem, unsigned long size)

same as above.

drivers/media/video/stk11xx.c line 2554

Quoted 1 line
drivers/media/video/stk11xx.c new file +1 -0
+	cap->version = (u32)DRIVER_VERSION_NUM;

You should use, instead, KERNEL_VERSION macro.

drivers/media/video/stk11xx.c

Quoted 4 lines
drivers/media/video/stk11xx.c new file +4 -0
+       /* Create frame buffers and make circular ring */
+       for (i = 0; i < default_nbrframebuf; i++)
+               if (dev->framebuf[i].data == NULL) {
+                       dev->framebuf[i].data =

vmalloc(STK11XX_FRAME_SIZE);

STK11XX_FRAME_SIZE is defined previously as (1280 * 1024 * 3). Why to allocate so much memory, even if the user selects a lower format? You should, instead, allocate memory dynamically as needed by the user.

drivers/media/video/stk11xx.c line 2755

Quoted 11 lines

+static int stk11xx_querystd(struct file *file, void *fh, v4l2_std_id *a)

drivers/media/video/stk11xx.c new file +9 -0
+{
+	*a = V4L2_STD_UNKNOWN;
+	return 0;
+}
+
+static int stk11xx_s_std(struct file *file, void *fh, v4l2_std_id *a)
+{
+	return *a == V4L2_STD_UNKNOWN ? 0 : -EINVAL;
+}

This raises an interesting discussion about video standards. I would, instead, use V4L2_STD_ALL, since webcams are capable of properly handling on 525 raw lines/60Hz based standards (those derived from 640x480 res) as well as 625 raw lines/50Hz based standards (those derived from ITU-T CIF resolutions).

As reference, STD_ALL is defined as:

#define V4L2_STD_ALL            (V4L2_STD_525_60        |\
                                 V4L2_STD_625_50)

---

Also, I noticed that several hexadecimal values are using uppercases. The better is to convert all of them to lowercase, to keep the same codingstyle as the other drivers.

--
Cheers,
Mauro

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Signature · 1 line
Markus Rechberger
#14 lore raw
Quoted 11 lines · On Monday 28 May 2007 17:14:51 Markus Rechberger wrote:
Quoted 2 lines · Mauro Carvalho Chehab

We don't do format conversions in kernel. Instead, you should return a proper Bayer Fourcc format (like V4L2_PIX_FMT_SBGGR8).

It's ok in his case since most userspace applications do not support this format, it helps noone if there's a driver out there which isn't supported by most userspace applications and I don't expect that all applications will add a conversion for that format soon. This would moreover raise the question about a userspace library, though there are many more important issues which are outstanding and which got smashed down.

Markus, I do think the userspace library is THE real problem with the whole V4L subsystem. It's annoying to keep repeating why these convertions should be done outside the kernel, be them in a library or not. I deliberatly never implemented the above convertion in the drivers that I have been maintaing for years, since I had the idea that this approach whould have pushed the developers to write the _best_ algorithm by themselves. I still think it's the correct approach, but now I give up. Mauro, if you are planning to accept this BayerToSomething color convertion, be also prepared to accept the same convertion for other drivers in the kernel. Rules are made for everyone.

Best regards,
Luca

#15 lore raw
Quoted 13 lines · Luca Risolia
Quoted 11 lines · On Monday 28 May 2007 17:14:51 Markus Rechberger wrote:
Quoted 2 lines · Mauro Carvalho Chehab

We don't do format conversions in kernel. Instead, you should return a proper Bayer Fourcc format (like V4L2_PIX_FMT_SBGGR8).

It's ok in his case since most userspace applications do not support this format, it helps noone if there's a driver out there which isn't supported by most userspace applications and I don't expect that all applications will add a conversion for that format soon. This would moreover raise the question about a userspace library, though there are many more important issues which are outstanding and which got smashed down.

Quoted 11 lines

Markus, I do think the userspace library is THE real problem with the whole V4L subsystem. It's annoying to keep repeating why these convertions should be done outside the kernel, be them in a library or not. I deliberatly never implemented the above convertion in the drivers that I have been maintaing for years, since I had the idea that this approach whould have pushed the developers to write the _best_ algorithm by themselves. I still think it's the correct approach, but now I give up. Mauro, if you are planning to accept
this BayerToSomething color convertion, be also prepared to accept the same convertion for other drivers in the kernel. Rules are made for everyone.

Initially asking for help doesn't lead anywhere either because most people aren't skilled enough or have personal issues with helping out at linuxtv.
I'm fine with the conversion, a valid point is scaling, this is something userspace can handle already.
And as I mentioned there are more important issues out there which should be handled first, but they're greatly getting ignored again (just as usual, and this also takes regressions and current implementation bugs in account).

Markus

#16 lore raw
In reply to Markus Rechberger

Em Seg, 2007-05-28 às 17:14 +0200, Markus Rechberger escreveu:

Quoted 8 lines
Quoted 7 lines
Quoted 3 lines

+/*
+ * Bayer conversion
+ */

We don't do format conversions in kernel. Instead, you should return a proper Bayer Fourcc format (like V4L2_PIX_FMT_SBGGR8).

Quoted 7 lines

It's ok in his case since most userspace applications do not support this format, it helps noone if there's a driver out there which isn't supported by most userspace applications and I don't expect that all applications will add a conversion for that format soon. This would moreover raise the question about a userspace library, though there are many more important issues which are outstanding and which got smashed down.

As Luca pointed, if we add conversion for one driver, we should add for the rest.

Instead, it would be better if Jiri sends the decoding and the rescaling stuff as a patch to v4l2-apps/lib, starting the API decoding library. Once we have a library, we can ask the userspace developers to use it for the formats not recognized by their userspace apps.

Cheers,
Mauro

#17 lore raw
Quoted 18 lines · Mauro Carvalho Chehab

Em Seg, 2007-05-28 às 17:14 +0200, Markus Rechberger escreveu:

Quoted 8 lines
Quoted 7 lines
Quoted 3 lines

+/*
+ * Bayer conversion
+ */

We don't do format conversions in kernel. Instead, you should return a proper Bayer Fourcc format (like V4L2_PIX_FMT_SBGGR8).

Quoted 7 lines

It's ok in his case since most userspace applications do not support this format, it helps noone if there's a driver out there which isn't supported by most userspace applications and I don't expect that all applications will add a conversion for that format soon. This would moreover raise the question about a userspace library, though there are many more important issues which are outstanding and which got smashed down.

Quoted 8 lines

As Luca pointed, if we add conversion for one driver, we should add for the rest.

Instead, it would be better if Jiri sends the decoding and the rescaling stuff as a patch to v4l2-apps/lib, starting the API decoding library. Once we have a library, we can ask the userspace developers to use it for the formats not recognized by their userspace apps.

I think it would be better to evaluate existing solutions (eg. libpw). Instead starting another one (which would lack any design since I haven't seen any thread that was picking together requirements), improving that existing one might also be helpful for applications like ekiga.
As for libpw it also supports colourspace conversion and software scaling, the used algorithms seem to need some improvement, but alot work has already been done there.
Of course other libraries should also be taken into account, maybe someone knows something better which is more complete.

Markus

#18 lore raw
Quoted 11 lines
Quoted 9 lines

As Luca pointed, if we add conversion for one driver, we should add for the rest.

Instead, it would be better if Jiri sends the decoding and the rescaling stuff as a patch to v4l2-apps/lib, starting the API decoding library. Once we have a library, we can ask the userspace developers to use it for the formats not recognized by their userspace apps.

I think it would be better to evaluate existing solutions (eg. libpw).

Agreed. Volunteers?

Signature · 2 lines
Cheers,
Mauro
#19 lore raw
Quoted 9 lines · Mauro Carvalho Chehab
Quoted 9 lines
Quoted 9 lines

As Luca pointed, if we add conversion for one driver, we should add for the rest.

Instead, it would be better if Jiri sends the decoding and the rescaling stuff as a patch to v4l2-apps/lib, starting the API decoding library. Once we have a library, we can ask the userspace developers to use it for the formats not recognized by their userspace apps.

Quoted 6 lines
Quoted 2 lines

I think it would be better to evaluate existing solutions (eg. libpw).

Agreed. Volunteers?

Hi Mauro and Markus,
Just to summ up what I understood we need:

What do we need in userspace, only for v4l (dvb is not concerned):
- colorspace translations
- filters that be done in hardware if the selected hardware can, otherwise software plugin
- decompression algorithm like stk11xx or usbvision (the decompression algorithm is in kernelspace since it is of linear complexity but shall be moved to userspace)

Using pwlib will not mean that application developers will use pwlib to decode v4l driver outputs.
C bindings are much more popular than C++ bindings and do not prevent object oriented design.
Application developers implement their own codecs.
As an example, every application do deinterlacing internally or not...
Application developers will probably not use pwlib v4l extensions because they will prefer to write adapted codecs for their framework.

Much more important for me is to see the actual specification of the needed v4l extensions points, with advice/participation of application/codec developers.

Thierry

#20 lore raw
Quoted 26 lines · Thierry Merle
Quoted 9 lines · Mauro Carvalho Chehab
Quoted 9 lines
Quoted 9 lines

As Luca pointed, if we add conversion for one driver, we should add for the rest.

Instead, it would be better if Jiri sends the decoding and the rescaling stuff as a patch to v4l2-apps/lib, starting the API decoding library. Once we have a library, we can ask the userspace developers to use it for the formats not recognized by their userspace apps.

Quoted 6 lines
Quoted 2 lines

I think it would be better to evaluate existing solutions (eg. libpw).

Agreed. Volunteers?

Hi Mauro and Markus,
Just to summ up what I understood we need:

What do we need in userspace, only for v4l (dvb is not concerned):
- colorspace translations
- filters that be done in hardware if the selected hardware can,
otherwise software plugin
- decompression algorithm like stk11xx or usbvision (the decompression algorithm is in kernelspace since it is of linear complexity but shall be moved to userspace)

Quoted 7 lines

Using pwlib will not mean that application developers will use pwlib to decode v4l driver outputs.
C bindings are much more popular than C++ bindings and do not prevent object oriented design.
Application developers implement their own codecs.
As an example, every application do deinterlacing internally or not...
Application developers will probably not use pwlib v4l extensions because they will prefer to write adapted codecs for their framework.

Much more important for me is to see the actual specification of the needed v4l extensions points, with advice/participation of application/codec developers.

As an example, we could empacket frames with a header containing audio/video format as it is done for MPEG streams. Is it possible without breaking the current ABI?
Do application developers would cope with that?

Quoted 3 lines

Thierry

#21 lore raw
Quoted 11 lines
Quoted 11 lines

Hi Mauro and Markus,
Just to summ up what I understood we need:

What do we need in userspace, only for v4l (dvb is not concerned):
- colorspace translations
- filters that be done in hardware if the selected hardware can, otherwise software plugin
- decompression algorithm like stk11xx or usbvision (the decompression algorithm is in kernelspace since it is of linear complexity but shall be moved to userspace)

Yes. The first focus, IMO, should be the last one.

Quoted 4 lines
Quoted 4 lines

Using pwlib will not mean that application developers will use pwlib to decode v4l driver outputs.
C bindings are much more popular than C++ bindings and do not prevent object oriented design.

IMO, we should implement very simple and efficient C subroutines.

Quoted 1 line
Quoted 1 line

Application developers implement their own codecs.

They can do it if they want. However, if we have a very consistent and easy to use subroutines for those weird decompress stuff, it is likely that they will use it.

Quoted 3 lines
Quoted 3 lines

As an example, every application do deinterlacing internally or not... Application developers will probably not use pwlib v4l extensions because they will prefer to write adapted codecs for their framework.

I think we shouldn't deal with deinterlacing. the API should be as simple as possible, focusing on implementing some stuff highly linked with the hardware (like specific decompression stuff, proprietary colospace conversions, etc).

Quoted 7 lines
Quoted 4 lines

Much more important for me is to see the actual specification of the needed v4l extensions points, with advice/participation of application/codec developers.

As an example, we could empacket frames with a header containing audio/video format as it is done for MPEG streams.
Is it possible without breaking the current ABI?

Yes, it is possible. In fact, some devices currently work by generating a common audio and video stream. The driver may just send the packet as-is, leaving to the userspace API the function to de-merge and synchronize audio and video.

Quoted 1 line

Do application developers would cope with that?

Maybe.

Cheers,
Mauro

#22 lore raw
Quoted 13 lines · Mauro Carvalho Chehab
Quoted 5 lines
Quoted 5 lines

Hi Mauro and Markus,
Just to summ up what I understood we need:

Quoted 7 lines
Quoted 7 lines

What do we need in userspace, only for v4l (dvb is not concerned):
- colorspace translations
- filters that be done in hardware if the selected hardware can,
otherwise software plugin
- decompression algorithm like stk11xx or usbvision (the decompression algorithm is in kernelspace since it is of linear complexity but shall be moved to userspace)

Quoted 3 lines

Yes. The first focus, IMO, should be the last one.

OK, so we must open the V4L2 API to add a custom pixel format.
While brainstorming, I thought it would be funny to create userspace drivers like FUSE based projects do. Applications would see nothing but a new device driver to access, all decompression algorithm would be in userspace. The path from the application to the v4l2 kernel driver:
[Application]<->[/mnt/video0 (created by FUSE-like userspace lib)]<->[/dev/video0 (created by kernel v4l2 driver)] There are limitations that I don't know, but this would be transparent for the applications.

Quoted 26 lines
Quoted 3 lines
Quoted 3 lines

Using pwlib will not mean that application developers will use pwlib to decode v4l driver outputs. C bindings are much more popular than C++ bindings and do not prevent object oriented design.

IMO, we should implement very simple and efficient C subroutines.

Quoted 2 lines
Quoted 2 lines

Application developers implement their own codecs.

They can do it if they want. However, if we have a very consistent and easy to use subroutines for those weird decompress stuff, it is likely that they will use it.

Quoted 3 lines
Quoted 3 lines

As an example, every application do deinterlacing internally or not...
Application developers will probably not use pwlib v4l extensions because they will prefer to write adapted codecs for their framework.

I think we shouldn't deal with deinterlacing. the API should be as simple as possible, focusing on implementing some stuff highly linked with the hardware (like specific decompression stuff, proprietary colospace conversions, etc).

Quoted 2 lines
Quoted 2 lines

Much more important for me is to see the actual specification of the needed v4l extensions points, with advice/participation of application/codec developers.

Quoted 9 lines
Quoted 3 lines

As an example, we could empacket frames with a header containing audio/video format as it is done for MPEG streams. Is it possible without breaking the current ABI?

Yes, it is possible. In fact, some devices currently work by generating a common audio and video stream. The driver may just send the packet as-is, leaving to the userspace API the function to de-merge and synchronize audio and video.

So we have a userspace lib that will demux/uncompress device driver output.
This lib will know each device driver particularities to know how to demux/uncompress in a a/v format that will be understood by the application.

Quoted 10 lines
Quoted 3 lines

Do application developers would cope with that?

Maybe.

Cheers,
Mauro

Thanks
Thierry

#23 lore raw

Em Ter, 2007-05-29 às 21:04 +0200, Thierry Merle escreveu:

Quoted 16 lines
Quoted 14 lines · Mauro Carvalho Chehab
Quoted 13 lines
Quoted 13 lines

Hi Mauro and Markus,
Just to summ up what I understood we need:

What do we need in userspace, only for v4l (dvb is not concerned):
- colorspace translations
- filters that be done in hardware if the selected hardware can, otherwise software plugin
- decompression algorithm like stk11xx or usbvision (the decompression algorithm is in kernelspace since it is of linear complexity but shall be moved to userspace)

Quoted 5 lines
Quoted 3 lines

Yes. The first focus, IMO, should be the last one.

OK, so we must open the V4L2 API to add a custom pixel format.

I don't think we need to change the API. The API already exports the format, as a fourcc-like info. We just need to add newer codes as they'll be added into kernel.

Quoted 9 lines

While brainstorming, I thought it would be funny to create userspace drivers like FUSE based projects do.
Applications would see nothing but a new device driver to access, all decompression algorithm would be in userspace.
The path from the application to the v4l2 kernel driver:
[Application]<->[/mnt/video0 (created by FUSE-like userspace lib)]<->[/dev/video0 (created by kernel v4l2 driver)]
There are limitations that I don't know, but this would be transparent for the applications.

This seems to be an interesting approach.

Quoted 8 lines
Quoted 5 lines

Yes, it is possible. In fact, some devices currently work by generating a common audio and video stream. The driver may just send the packet as-is, leaving to the userspace API the function to de-merge and synchronize audio and video.

So we have a userspace lib that will demux/uncompress device driver output. This lib will know each device driver particularities to know how to demux/uncompress in a a/v format that will be understood by the application.

Yes. However, we should try to do loose coupling whenever possible, to keep maintainership of the "library"(*) as simple as possible.

(*) With this approach, it seems that it will turn into a helper daemon, instead of just a library.

Cheers,
Mauro

#24 lore raw
Quoted 29 lines · Mauro Carvalho Chehab

Em Ter, 2007-05-29 às 21:04 +0200, Thierry Merle escreveu:

Quoted 16 lines
Quoted 14 lines · Mauro Carvalho Chehab
Quoted 14 lines
Quoted 14 lines

Hi Mauro and Markus,
Just to summ up what I understood we need:

What do we need in userspace, only for v4l (dvb is not concerned):
- colorspace translations
- filters that be done in hardware if the selected hardware can,
otherwise software plugin
- decompression algorithm like stk11xx or usbvision (the decompression algorithm is in kernelspace since it is of linear complexity but shall be moved to userspace)

Quoted 6 lines
Quoted 4 lines

Yes. The first focus, IMO, should be the last one.

OK, so we must open the V4L2 API to add a custom pixel format.

I don't think we need to change the API. The API already exports the format, as a fourcc-like info. We just need to add newer codes as they'll be added into kernel.

Quoted 10 lines
Quoted 6 lines

While brainstorming, I thought it would be funny to create userspace drivers like FUSE based projects do. Applications would see nothing but a new device driver to access, all decompression algorithm would be in userspace. The path from the application to the v4l2 kernel driver:
[Application]<->[/mnt/video0 (created by FUSE-like userspace lib)]<->[/dev/video0 (created by kernel v4l2 driver)] There are limitations that I don't know, but this would be transparent for the applications.

This seems to be an interesting approach.

Interesting but impossible to do for ioctl calls.
When the application does a ioctl(fd_of_mnt_video0,VIDIOC_G_FMT,&arg) for example, there is no way for the userspace helper to catch this ioctl.
The application could only open/read from the userspace helper's file /mnt/video0.
ioctl would still have to be done on the kernel device driver.
I thought also about a /proc interface for decompression algorithms (a helper would listen on a /proc file and write on another /proc file) but /proc is not designed for that kind of thing. A separate library seems to be the simplest solution.

Quoted 20 lines
Quoted 9 lines
Quoted 6 lines

Yes, it is possible. In fact, some devices currently work by generating a common audio and video stream. The driver may just send the packet as-is, leaving to the userspace API the function to de-merge and synchronize audio and video.

So we have a userspace lib that will demux/uncompress device driver output.
This lib will know each device driver particularities to know how to demux/uncompress in a a/v format that will be understood by the application.

Yes. However, we should try to do loose coupling whenever possible, to keep maintainership of the "library"(*) as simple as possible.

(*) With this approach, it seems that it will turn into a helper daemon, instead of just a library.

Cheers,
Mauro

#25 lore raw
Quoted 4 lines
Quoted 4 lines

This seems to be an interesting approach.

Quoted 10 lines

Interesting but impossible to do for ioctl calls.
When the application does a ioctl(fd_of_mnt_video0,VIDIOC_G_FMT,&arg) for example, there is no way for the userspace helper to catch this ioctl. The application could only open/read from the userspace helper's file /mnt/video0.
ioctl would still have to be done on the kernel device driver. I thought also about a /proc interface for decompression algorithms (a helper would listen on a /proc file and write on another /proc file) but /proc is not designed for that kind of thing.
A separate library seems to be the simplest solution.

There are some ways for this to work. For example, you may create a helper device for the daemon driver to bind, even requiring it to have root permission.

Signature · 2 lines
Cheers,
Mauro
#26 lore raw
Quoted 12 lines · Mauro Carvalho Chehab
Quoted 11 lines
Quoted 4 lines

This seems to be an interesting approach.

Interesting but impossible to do for ioctl calls.
When the application does a ioctl(fd_of_mnt_video0,VIDIOC_G_FMT,&arg) for example, there is no way for the userspace helper to catch this ioctl.
The application could only open/read from the userspace helper's file /mnt/video0.
ioctl would still have to be done on the kernel device driver.
I thought also about a /proc interface for decompression algorithms (a helper would listen on a /proc file and write on another /proc file) but /proc is not designed for that kind of thing. A separate library seems to be the simplest solution.

Quoted 5 lines

There are some ways for this to work. For example, you may create a helper device for the daemon driver to bind, even requiring it to have root permission.

Right, but I am afraid that complex userspace/kernelspace context changes breaks performance. Nevertheless, I will start to specify the framework.
The helper daemon would link to the v4l2-apps/lib.

#27 lore raw

Em Sáb, 2007-06-02 às 11:00 +0200, Thierry Merle escreveu:

Quoted 18 lines
Quoted 16 lines · Mauro Carvalho Chehab
Quoted 15 lines
Quoted 4 lines

This seems to be an interesting approach.

Interesting but impossible to do for ioctl calls.
When the application does a ioctl(fd_of_mnt_video0,VIDIOC_G_FMT,&arg) for example, there is no way for the userspace helper to catch this ioctl. The application could only open/read from the userspace helper's file /mnt/video0.
ioctl would still have to be done on the kernel device driver. I thought also about a /proc interface for decompression algorithms (a helper would listen on a /proc file and write on another /proc file) but /proc is not designed for that kind of thing.
A separate library seems to be the simplest solution.

Quoted 7 lines
Quoted 5 lines

There are some ways for this to work. For example, you may create a helper device for the daemon driver to bind, even requiring it to have root permission.

Right, but I am afraid that complex userspace/kernelspace context changes breaks performance.

Yes, there's a risk.

Quoted 2 lines

Nevertheless, I will start to specify the framework. The helper daemon would link to the v4l2-apps/lib.

Ok, thanks!

Signature · 2 lines
Cheers,
Mauro
author
#28 lore raw
Quoted 24 lines · Mauro Carvalho Chehab

Em Sáb, 2007-06-02 às 11:00 +0200, Thierry Merle escreveu:

Quoted 23 lines
Quoted 21 lines · Mauro Carvalho Chehab
Quoted 4 lines
Quoted 4 lines

This seems to be an interesting approach.

Quoted 11 lines

Interesting but impossible to do for ioctl calls.
When the application does a ioctl(fd_of_mnt_video0,VIDIOC_G_FMT,&arg) for example, there is no way for the userspace helper to catch this ioctl. The application could only open/read from the userspace helper's file /mnt/video0.
ioctl would still have to be done on the kernel device driver. I thought also about a /proc interface for decompression algorithms (a helper would listen on a /proc file and write on another /proc file) but /proc is not designed for that kind of thing.
A separate library seems to be the simplest solution.

There are some ways for this to work. For example, you may create a helper device for the daemon driver to bind, even requiring it to have root permission.

Quoted 8 lines
Quoted 2 lines

Right, but I am afraid that complex userspace/kernelspace context changes breaks performance.

Yes, there's a risk.

Quoted 2 lines

Nevertheless, I will start to specify the framework. The helper daemon would link to the v4l2-apps/lib.

Ok, thanks!

Ok, here comes updated version of stk11xx driver. Note that it's not still separated into more files (I'll do this as the last step) and also resize and bayer->rgb conversion remains. This is what I'm interested in -- is there some embryo of library with resize and conversion? Or what was the decision to create?

--

stk11xx, add a new webcam driver

Cc Mauro Carvalho Chehab <[email protected]>

---
commit e6b9d9178baf000f9faae116163bf4d14749b130
tree 3c8ddda1c78a73bceabf090745af9ef61607a12a
parent 5d7a1b0492bec9989614c48f59257eaba4e4482d
author Jiri Slaby <[email protected]> Fri, 15 Jun 2007 21:44:33 +0200 committer Jiri Slaby <[email protected]> Fri, 15 Jun 2007 21:44:33 +0200

 drivers/media/video/Kconfig   |    8 
 drivers/media/video/Makefile  |    2 
 drivers/media/video/stk11xx.c | 3231 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 3241 insertions(+), 0 deletions(-)
drivers/media/video/Kconfig +8 -0
index e88d5d7..8a71df3 100644
@@ -792,6 +792,14 @@ config USB_ZR364XX
 	  To compile this driver as a module, choose M here: the
 	  module will be called zr364xx.
 
+config USB_STK11XX
+	tristate "STK11XX based webcams"
+	depends on VIDEO_V4L2
+	---help---
+	  This will add support for Syntek webcams such as dc1125 and stk1135.
+
+	  If you choose to build it as module, it will be called stk11xx.
+
 endif # V4L_USB_DRIVERS
 
 endif # VIDEO_CAPTURE_DRIVERS
drivers/media/video/Makefile +2 -0
index 3202e87..409125d 100644
@@ -106,6 +106,8 @@ obj-$(CONFIG_USB_STV680)        += stv680.o
 obj-$(CONFIG_USB_W9968CF)       += w9968cf.o
 obj-$(CONFIG_USB_ZR364XX)       += zr364xx.o
 
+obj-$(CONFIG_USB_STK11XX)	+= stk11xx.o
+
 obj-$(CONFIG_USB_SN9C102)       += sn9c102/
 obj-$(CONFIG_USB_ET61X251)      += et61x251/
 obj-$(CONFIG_USB_PWC)           += pwc/
drivers/media/video/stk11xx.c new file +3231 -0

2,978 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Part of the diff is not shown here. View the raw message.

#29 lore raw
Quoted 41 lines · Jiri Slaby
Quoted 9 lines · Mauro Carvalho Chehab

Em Sáb, 2007-06-02 às 11:00 +0200, Thierry Merle escreveu:

Quoted 8 lines

Mauro Carvalho Chehab a écrit :

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

This seems to be an interesting approach.

Interesting but impossible to do for ioctl calls.
When the application does a

ioctl(fd_of_mnt_video0,VIDIOC_G_FMT,&arg)

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

for example, there is no way for the userspace helper to catch

this ioctl.

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

The application could only open/read from the userspace

helper's file

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

/mnt/video0.
ioctl would still have to be done on the kernel device driver. I thought also about a /proc interface for decompression

algorithms (a

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

helper would listen on a /proc file and write on another /proc

file) but

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines

/proc is not designed for that kind of thing. A separate library seems to be the simplest solution.

There are some ways for this to work. For example, you may

create a

Quoted 1 line
Quoted 1 line
Quoted 1 line

helper device for the daemon driver to bind, even requiring it

to have

Quoted 11 lines
Quoted 5 lines
Quoted 3 lines

root permission.

Right, but I am afraid that complex userspace/kernelspace context changes breaks performance.

Yes, there's a risk.

Quoted 2 lines

Nevertheless, I will start to specify the framework. The helper daemon would link to the v4l2-apps/lib.

Ok, thanks!

Quoted 9 lines

Ok, here comes updated version of stk11xx driver. Note that it's not still
separated into more files (I'll do this as the last step) and also resize
and bayer->rgb conversion remains. This is what I'm interested in -- is
there some embryo of library with resize and conversion? Or what was the
decision to create?

Hi Jiri,
sorry I started the specification but nothing finalized (and consequently no code).
Here is the start of my idea for a v4l2_extension:
http://thierry.merle.free.fr/v4l2_extension/general.html
My new USB tuner took me all my little time and I did not get into details (the spec is incomplete).
Well, as you insist I will explain a bit more :)
Please comment, tell me what you don't understand I will try to update during this week-end...

Thierry

author
#30 lore raw

Thierry Merle napsal(a):

Quoted 16 lines · Jiri Slaby
Quoted 5 lines
Quoted 4 lines · Mauro Carvalho Chehab

Em Sáb, 2007-06-02 às 11:00 +0200, Thierry Merle escreveu:

Quoted 2 lines

Nevertheless, I will start to specify the framework. The helper daemon would link to the v4l2-apps/lib.

Ok, thanks!

Quoted 9 lines

Ok, here comes updated version of stk11xx driver. Note that it's not still
separated into more files (I'll do this as the last step) and also resize
and bayer->rgb conversion remains. This is what I'm interested in -- is
there some embryo of library with resize and conversion? Or what was the
decision to create?

Quoted 10 lines

Hi Jiri,
sorry I started the specification but nothing finalized (and consequently no code).
Here is the start of my idea for a v4l2_extension:
http://thierry.merle.free.fr/v4l2_extension/general.html
My new USB tuner took me all my little time and I did not get into details (the spec is incomplete).
Well, as you insist I will explain a bit more :)
Please comment, tell me what you don't understand I will try to update during this week-end...

At the first look seems good, the SD is self-explaining. If you want me to help you with something, feel free to drop a message ;).

In the meantime until some early increment would be developped, is it feasible/acceptable to merge the driver as is (after changing some things which will be eventually commented plus splitting into several files)?

thanks,

Signature · 4 lines
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#32 lore raw

Oliver Neukum napsal(a):

Quoted 9 lines

Am Freitag, 15. Juni 2007 schrieb Jiri Slaby:

drivers/media/video/stk11xx.c line 105

Quoted 6 lines
drivers/media/video/stk11xx.c new file +6 -0
+struct stk11xx_frame_buf {
+       void *data;
+       volatile int filled;
+       struct stk11xx_frame_buf *next;
+};
+

What's the "volatile" doing here?

removed (s/volatile/unsigned/), thanks,

Signature · 4 lines
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E
author
#33 lore raw
In reply to Mauro Carvalho Chehab

Mauro Carvalho Chehab napsal(a):

Quoted 1 line

Hi Jiri,

Hi.

Quoted 1 line

I have some comments for your driver.

Well, that was exactly what the code needed, yet another eyes.

Quoted 3 lines
Quoted 1 line

+ * Copyright (C) Nicolas VIVIEN

It would be interesting to have Nicolas SOB as well, if possible.

I don't think he ever knows about this version of the driver. I got his GPL driver, cleaned up -- coding style, v4l1 and v4l2 ioctl conversion to v4l2 functions, some bug fixes and so on... If you still want him to sign this of, I'll try my best to catch him but can't guarantee any results.

Quoted 9 lines

drivers/media/video/stk11xx.c line 72

Quoted 6 lines
drivers/media/video/stk11xx.c new file +6 -0
+
+#ifndef CONFIG_STK11XX_DEBUG_STREAM
+#define CONFIG_STK11XX_DEBUG_STREAM	0
+#endif
+
+#if CONFIG_STK11XX_DEBUG_STREAM

I would instead use:
#ifdef CONFIG_STK11XX_DEBUG_STREAM

Hmm, no, I would rather get rid of CONFIG_ thing, it may make things unclear, beacuse there is (will be) no option in Kconfig for this, because this is the most verbose option for the driver mainly used for algorithms debugging. Standard DEBUG (pr_debug, dev_dbg) is intended to be a real debug print here.

It will be always defined due to few lines above it.

Quoted 12 lines
Quoted 3 lines

+/*
+ * Bayer conversion
+ */

We don't do format conversions in kernel. Instead, you should return a proper Bayer Fourcc format (like V4L2_PIX_FMT_SBGGR8).

drivers/media/video/stk11xx.c line 464

Quoted 2 lines
drivers/media/video/stk11xx.c new file +2 -0
+static void stk11xx_resize_image(u8 *out, const u8 *in,
+		unsigned int width, unsigned int height, unsigned int factor)

Also, kernel shouldn't be doing image resize.

Quoted 5 lines

drivers/media/video/stk11xx.c line 504

Quoted 2 lines
drivers/media/video/stk11xx.c new file +2 -0
+static int stk11xx_decompress(struct stk11xx *dev)
+{

We don't do format conversions in kernel. Instead, you should return a proper Bayer Fourcc format (like V4L2_PIX_FMT_SBGGR8).

Ok, there is a debate about this, I will do the changes after some decision will be made.

Quoted 37 lines
Quoted 37 lines
drivers/media/video/stk11xx.c new file +37 -0
+static int stk1125_load_microcode(struct stk11xx *dev)
+{
+	unsigned int i;
+	const u8 *values_204, *values_205;
+	int retok, value;
+
+	/* From 80x60 to 640x480 */
+	const u8 values_1_204[] = {
+		0x12, 0x11, 0x3b, 0x6a, 0x13, 0x10, 0x00, 0x01, 0x02, 0x13,
+		0x39, 0x38, 0x37, 0x35, 0x0e, 0x12, 0x04, 0x0c, 0x0d, 0x17,
+		0x18, 0x32, 0x19, 0x1a, 0x03, 0x1b, 0x16, 0x33, 0x34, 0x41,
+		0x96, 0x3d, 0x69, 0x3a, 0x8e, 0x3c, 0x8f, 0x8b, 0x8c, 0x94,
+		0x95, 0x40, 0x29, 0x0f, 0xa5, 0x1e, 0xa9, 0xaa, 0xab, 0x90,
+		0x91, 0x9f, 0xa0, 0x24, 0x25, 0x26, 0x14, 0x2a, 0x2b
+	};
+	const u8 values_1_205[] = {
+		0x45, 0x80, 0x01, 0x7d, 0x80, 0x00, 0x00, 0x80, 0x80, 0x80,
+		0x50, 0x93, 0x00, 0x81, 0x20, 0x45, 0x00, 0x00, 0x00, 0x24,
+		0xc4, 0xb6, 0x00, 0x3c, 0x36, 0x00, 0x07, 0xe2, 0xbf, 0x00,
+		0x04, 0x19, 0x40, 0x0d, 0x00, 0x73, 0xdf, 0x06, 0x20, 0x88,
+		0x88, 0xc1, 0x3f, 0x42, 0x80, 0x04, 0xb8, 0x92, 0x0a, 0x00,
+		0x00, 0x00, 0x00, 0x68, 0x5c, 0xc3, 0x2e, 0x00, 0x00
+	};
+
+	/* From 800x600 to 1280x1024 */
+	const u8 values_2_204[] = {
+		0x12, 0x11, 0x3b, 0x6a, 0x13, 0x10, 0x00, 0x01, 0x02, 0x13,
+		0x39, 0x38, 0x37, 0x35, 0x0e, 0x12, 0x04, 0x0c, 0x0d, 0x17,
+		0x18, 0x32, 0x19, 0x1a, 0x03, 0x1b, 0x16, 0x33, 0x34, 0x41,
+		0x96, 0x3d, 0x69, 0x3a, 0x8e, 0x3c, 0x8f, 0x8b, 0x8c, 0x94,
+		0x95, 0x40, 0x29, 0x0f, 0xa5, 0x1e, 0xa9, 0xaa, 0xab, 0x90,
+		0x91, 0x9f, 0xa0, 0x24, 0x25, 0x26, 0x14, 0x2a, 0x2b
+	};
+	const u8 values_2_205[] = {
+		0x05, 0x80, 0x01, 0x7d, 0x80, 0x00, 0x00, 0x80, 0x80, 0x80,
+		0x50, 0x93, 0x00, 0x81, 0x20, 0x05, 0x00, 0x00, 0x00, 0x1b,
+		0xbb, 0xa4, 0x01, 0x81, 0x12, 0x00, 0x07, 0xe2, 0xbf, 0x00,
Quoted 8 lines

drivers/media/video/stk11xx.c line 932

Quoted 4 lines
drivers/media/video/stk11xx.c new file +4 -0
+		0x04, 0x19, 0x40, 0x0d, 0x00, 0x73, 0xdf, 0x06, 0x20, 0x88,
+		0x88, 0xc1, 0x3f, 0x42, 0x80, 0x04, 0xb8, 0x92, 0x0a, 0x00,
+		0x00, 0x00, 0x00, 0x68, 0x5c, 0xc3, 0x2e, 0x00, 0x00
+	};

Please use instead the load_firmware routines. It is not a good idea to have firmware inside the kernel. Also, this might rise some legal issues due to licensing models.

Markus wrote:
<cite>
Jiri, are you allowed to include that microcode, did you get any information about this from the manufacturer which could allow the inclusion?
The sequences are rather small not putting it into extra firmware files would make life much easier for some users, on the other side if it raises legal issues Mauro's right with loading it from a file </cite>

This seems to be a reverse engineered driver, I think, all those values are intercepted, so there are no licensing issues.

Quoted 68 lines
Quoted 56 lines
drivers/media/video/stk11xx.c new file +56 -0
+
+/* 
+ * The configuration of device is composed of 11 steps.
+ * This function is called by the initialization process.
+ *
+ * We don't know the meaning of these steps! We only replay the USB log.
+ *
+ * The steps 0 to 9 are called during the initialization.
+ * Then, the driver choose the last step :
+ *   10 : for a resolution from 80x60 to 640x480 
+ *   11 : for a resolution from 800x600 to 1280x1024
+ */
+static int stk1125_configure_device(struct stk11xx *dev, int step)
+{
+	int value;
+
+	/*      0,    1,    2,    3,    4,    5,    6,    7,    8,    9, 
+		    10,   11 */
+
+	const u8 values_001B[] = {
+		0x0E, 0x03, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E, 0x0E,
+		    0x0E, 0x0E
+	};
+	const u8 values_001C[] = {
+		0x06, 0x02, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46, 0x46,
+		    0x46, 0x0E
+	};
+	const u8 values_0202[] = {
+		0x1E, 0x0A, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E, 0x1E,
+		    0x1E, 0x1E
+	};
+	const u8 values_0110[] = {
+		0x07, 0x00, 0x00, 0x00, 0x00, 0x3E, 0x3E, 0x00, 0x00, 0x00,
+		    0x00, 0x00
+	};
+	const u8 values_0112[] = {
+		0x07, 0x00, 0x00, 0x00, 0x00, 0x09, 0x09, 0x00, 0x00, 0x00,
+		    0x00, 0x00
+	};
+	const u8 values_0114[] = {
+		0x87, 0x80, 0x80, 0x80, 0x80, 0xBE, 0xBE, 0x80, 0x80, 0x80,
+		    0x80, 0x00
+	};
+	const u8 values_0115[] = {
+		0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02,
+		    0x02, 0x05
+	};
+	const u8 values_0116[] = {
+		0xE7, 0xE0, 0xE0, 0xE0, 0xE0, 0xE9, 0xE9, 0xE0, 0xE0, 0xE0,
+		    0xE0, 0x00
+	};
+	const u8 values_0117[] = {
+		0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
+		    0x01, 0x04
+	};
+	const u8 values_0100[] = {

drivers/media/video/stk11xx.c line 1057

Quoted 10 lines
drivers/media/video/stk11xx.c new file +10 -0
+		0x20, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21, 0x21,
+		    0x21, 0x21
+	};
+
+	dev_dbg(&dev->udev->dev, "stk1125_configure_device: %d\n", step);
+
+	stk11xx_write_registry(dev, 0x0000, 0x0024);
+	stk11xx_write_registry(dev, 0x0002, 0x0068);
+	stk11xx_write_registry(dev, 0x0003, 0x0080);
+	stk11xx_write_registry(dev, 0x0005, 0x0000);
	...
Quoted 3 lines

Instead of using all those write, you should consider creating a table of values and use something like:

	stk11xx_write_regs(dev, table1);

There is a problem with this approach. There are reads every 3-5 writes and this can grow into many small tables.

Quoted 41 lines
Quoted 28 lines
drivers/media/video/stk11xx.c new file +27 -0
+/*
+ * STK-1135 API
+ */
+
+static int stk1135_load_microcode(struct stk11xx *dev)
+{
+	unsigned int i;
+	int retok, value;
 
+
+	const u8 values_204[] = {
+		0x17, 0x19, 0xb4, 0xa6, 0x12, 0x13, 0x1e, 0x21, 0x24, 0x32,
+		0x36, 0x39, 0x4d, 0x53, 0x5d, 0x5f, 0x60, 0x61, 0x62, 0x63,
+		0x64, 0x65, 0x66, 0x82, 0x83, 0x85, 0x86, 0x89, 0x97, 0x98,
+		0xad, 0xae, 0xb8, 0xb9, 0xba, 0xbb, 0xbc, 0xbf, 0x48, 0xd8,
+		0x76, 0x77, 0x78, 0x79, 0x7a, 0x7b, 0x7c, 0x7d, 0x7e, 0x7f,
+		0x80, 0x81, 0xd8, 0x76, 0x77, 0x78, 0x79, 0x7a, 0x7b, 0x7c,
+		0x7d, 0x7e, 0x7f, 0x80, 0x81, 0xd8, 0x76, 0x77, 0x78, 0x79,
+		0x7a, 0x7b, 0x7c, 0x7d, 0x7e, 0x7f, 0x80, 0x81, 0x5c, 0xc0,
+		0x59, 0x5a, 0x5b, 0xd4, 0x8e, 0x8f, 0x90, 0x91, 0x92, 0x93,
+		0x94, 0x95, 0x96, 0xb3, 0x73, 0x06, 0x07, 0x0b, 0x15, 0x20,
+		0x4e, 0x4f, 0x49, 0x4a, 0x4b, 0x4c, 0x46, 0x06, 0x07, 0xb9,
+		0xba, 0xbb, 0xbc, 0x61, 0x62, 0x65, 0x66
+	};
+	const u8 values_205[] = {
+		0x41, 0x41, 0x03, 0x06, 0x06, 0x08, 0x06, 0x00, 0x02, 0x69,
+		0x35, 0x60, 0xfe, 0x1c, 0x04, 0x08, 0x08, 0x08, 0x08, 0x00,
+		0x00, 0x10, 0x14, 0x01, 0x80, 0x0c, 0xb6, 0x00, 0x25, 0x25,

drivers/media/video/stk11xx.c line 1484

Quoted 10 lines
drivers/media/video/stk11xx.c new file +10 -0
+		0x3f, 0x24, 0x10, 0x07, 0xcc, 0x1f, 0x30, 0x02, 0x9c, 0x80,
+		0x00, 0x0d, 0x18, 0x22, 0x2c, 0x3e, 0x4f, 0x6f, 0x8e, 0xac,
+		0xc8, 0xe5, 0xa0, 0x00, 0x0d, 0x18, 0x22, 0x2c, 0x3e, 0x4f,
+		0x6f, 0x8e, 0xac, 0xc8, 0xe5, 0xc0, 0x00, 0x0d, 0x18, 0x22,
+		0x2c, 0x3e, 0x4f, 0x6f, 0x8e, 0xac, 0xc8, 0xe5, 0x70, 0x18,
+		0x09, 0x07, 0x07, 0x3c, 0x3d, 0x95, 0x88, 0x89, 0x47, 0x9c,
+		0x81, 0x9c, 0x3d, 0x76, 0x76, 0x01, 0xf3, 0x05, 0x00, 0x44,
+		0x06, 0x0a, 0x96, 0x00, 0x7d, 0x00, 0x20, 0x01, 0xf3, 0x04,
+		0xe4, 0x09, 0xc8, 0x08, 0x08, 0x10, 0x14
+	};

Same as commented before about microcode.

Quoted 3 lines

You may also consider writing a separate c file for stk1135. Having a large .c file is not very nice. The better is to split the code into a few parts.

I don't like many files for one driver and finding little pieces of code in each file separately -- 1125 + 1235 will be small pieces. Not considering the static functions and warning about unused code. But it's up to you, it's your subtree, make a decision.

Quoted 4 lines
Quoted 1 line

+static void *stk11xx_rvmalloc(unsigned long size)

Another rvmalloc implementation? You should consider using the one already at kernel.

What's the name, I can't find it?

The rest of comments has been applied, thanks,

Signature · 4 lines
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E
#34 lore raw
Quoted 9 lines
Quoted 4 lines
Quoted 2 lines

+ * Copyright (C) Nicolas VIVIEN

It would be interesting to have Nicolas SOB as well, if possible.

I don't think he ever knows about this version of the driver. I got his GPL driver, cleaned up -- coding style, v4l1 and v4l2 ioctl conversion to v4l2 functions, some bug fixes and so on... If you still want him to sign this of, I'll try my best to catch him but can't guarantee any results.

It would be nice. I can accept it without his ack, but it would be better to have it, if possible.

Quoted 11 lines
Quoted 10 lines

drivers/media/video/stk11xx.c line 72

Quoted 7 lines
drivers/media/video/stk11xx.c new file +6 -0
+
+#ifndef CONFIG_STK11XX_DEBUG_STREAM
+#define CONFIG_STK11XX_DEBUG_STREAM	0
+#endif
+
+#if CONFIG_STK11XX_DEBUG_STREAM

I would instead use:
#ifdef CONFIG_STK11XX_DEBUG_STREAM

Quoted 4 lines

Hmm, no, I would rather get rid of CONFIG_ thing, it may make things unclear, beacuse there is (will be) no option in Kconfig for this, because this is the most verbose option for the driver mainly used for algorithms debugging.

Seems ok to me.

Quoted 6 lines
Quoted 2 lines

We don't do format conversions in kernel. Instead, you should return a proper Bayer Fourcc format (like V4L2_PIX_FMT_SBGGR8).

Ok, there is a debate about this, I will do the changes after some decision will be made.

As you wish.

Quoted 14 lines
Quoted 3 lines

Please use instead the load_firmware routines. It is not a good idea to have firmware inside the kernel. Also, this might rise some legal issues due to licensing models.

Markus wrote:
<cite>
Jiri, are you allowed to include that microcode, did you get any information about this from the manufacturer which could allow the inclusion?
The sequences are rather small not putting it into extra firmware files would make life much easier for some users, on the other side if it raises legal issues Mauro's right with loading it from a file </cite>

Quoted 2 lines

This seems to be a reverse engineered driver, I think, all those values are intercepted, so there are no licensing issues.

Are those a code, or just another internal driver configuration (for example, maybe some register initialization inside the sensors)? We should take care to avoid adding material here that can be later complained.

Quoted 7 lines
Quoted 4 lines

Instead of using all those write, you should consider creating a table of values and use something like:

	stk11xx_write_regs(dev, table1);

There is a problem with this approach. There are reads every 3-5 writes and this can grow into many small tables.

Maybe you can do this then just for the bigger tables.

Quoted 8 lines
Quoted 3 lines

You may also consider writing a separate c file for stk1135. Having a large .c file is not very nice. The better is to split the code into a few parts.

I don't like many files for one driver and finding little pieces of code in each file separately -- 1125 + 1235 will be small pieces. Not considering the static functions and warning about unused code. But it's up to you, it's your subtree, make a decision.

Your driver have about 3600 lines. We target to keep newer files with a maximum of about 1000 lines on the same file (unfortunately, some drivers are bigger than that), separating the driver into logical pieces. I think it would be interesting to split it into two or tree files.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

+static void *stk11xx_rvmalloc(unsigned long size)

Another rvmalloc implementation? You should consider using the one already at kernel.

What's the name, I can't find it?

There are some rvmalloc on cpia, cpia2, em28xx, ...

What the current drivers are doing is to replace it to vmalloc_32:

#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,15)
                if ((buff = rvmalloc(dev->num_frames * imagesize))) {
#else
                if ((buff = vmalloc_32(dev->num_frames * imagesize))) {

#endif

Quoted 2 lines

The rest of comments has been applied, thanks,

You're welcome.

Signature · 2 lines
Cheers,
Mauro