Syntek webcams and out-of-tree driver

Hello,
the in-kernel stkwebcam driver (by Jaime Velasco Juan and Nicolas VIVIEN) supports only two webcam types (USB IDs 0x174f:0xa311 and 0x05e1:0x0501). There are many other Syntek webcam types that are not supported by this driver (such as 0x174f:0x6a31 in Asus F5RL laptop).

There is an out-of-tree GPL driver called stk11xx (by Martin Roos and also Nicolas VIVIEN) at http://sourceforge.net/projects/syntekdriver/ which supports more webcams. It can be even compiled for the latest kernels using the patch below and seems to work somehow (slow and buggy but better than nothing) with the Asus F5RL.

Is there any possibility that this driver could be merged into the kernel? The code could probably be simplified a lot and integrated into gspca.

diff -urp syntekdriver-code-107-trunk-orig/driver/stk11xx.h syntekdriver-code-107-trunk//driver/stk11xx.h

/driver/stk11xx.h renamed from driver/stk11xx.h +2 -0

@@ -33,6 +33,7 @@
 
  #ifndef STK11XX_H
  #define STK11XX_H
+#include <media/v4l2-device.h>
 
  #define DRIVER_NAME					"stk11xx"					/**< Name of this driver */
  #define DRIVER_VERSION				"v3.0.0"					/**< Version of this driver */

@@ -316,6 +317,7 @@ struct stk11xx_video {
   * @struct usb_stk11xx
   */
  struct usb_stk11xx {
+	struct v4l2_device v4l2_dev;
  	struct video_device *vdev; 			/**< Pointer on a V4L2 video device */
  	struct usb_device *udev;			/**< Pointer on a USB device */
  	struct usb_interface *interface;	/**< Pointer on a USB interface */

diff -urp syntekdriver-code-107-trunk-orig/driver/stk11xx-v4l.c syntekdriver-code-107-trunk//driver/stk11xx-v4l.c

/driver/stk11xx-v4l.c renamed from driver/stk11xx-v4l.c +10 -1

@@ -1498,9 +1498,17 @@ int v4l_stk11xx_register_video_device(st
  {
  	int err;
 
+	err = v4l2_device_register(&dev->interface->dev, &dev->v4l2_dev);
+	if (err < 0) {
+		STK_ERROR("couldn't register v4l2_device\n");
+		kfree(dev);
+		return err;
+	}
+
  	strcpy(dev->vdev->name, DRIVER_DESC);
 
-	dev->vdev->parent = &dev->interface->dev;
+//	dev->vdev->parent = &dev->interface->dev;
+	dev->vdev->v4l2_dev = &dev->v4l2_dev;
  	dev->vdev->fops = &v4l_stk11xx_fops;
  	dev->vdev->release = video_device_release;
  	dev->vdev->minor = -1;

@@ -1533,6 +1541,7 @@ int v4l_stk11xx_unregister_video_device(
 
  	video_set_drvdata(dev->vdev, NULL);
  	video_unregister_device(dev->vdev);
+	v4l2_device_unregister(&dev->v4l2_dev);
 
  	return 0;
  }

Hi,

Quoted 11 lines · Ondrej Zary

Hello,
the in-kernel stkwebcam driver (by Jaime Velasco Juan and Nicolas VIVIEN) supports only two webcam types (USB IDs 0x174f:0xa311 and 0x05e1:0x0501). There are many other Syntek webcam types that are not supported by this driver (such as 0x174f:0x6a31 in Asus F5RL laptop).

There is an out-of-tree GPL driver called stk11xx (by Martin Roos and also Nicolas VIVIEN) at http://sourceforge.net/projects/syntekdriver/
which supports more webcams. It can be even compiled for the latest kernels using the patch below and seems to work somehow (slow and buggy but better than nothing) with the Asus F5RL.

I took a quick look and there are a number of issues with this driver:

1) It conflicts usb-id wise with the new stk1160 driver (which supports usb-id 05e1:0408) so support for that usb-id, and any code only used for that id will need to be removed

2) "seems to work somehow (slow and buggy)" is not really the quality we aim for with in kernel drivers. We definitely will want to remove any usb-ids, and any code only used for those ids, where there is overlap with the existing stkwebcam driver, to avoid regressions

3) It does in kernel bayer decoding, this is not acceptable, it needs to be modified to produce buffers with raw bayer data (libv4l will take care of the bater decoding in userspace).

4) It is not using any of the new kernel infrastructure we have been adding over time, like the control-framework, videobuf2, etc. It would be best to convert this to a gspca sub driver (of which there are many already, which can serve as examples), so that it will use all the existing framework code.

As a minimum issues 1-3 needs to be addressed before this can be merged. An alternative / better approach might be to simply only lift the code for your camera, and add a new gspca driver supporting only your camera.

Either way since non of the v4l developers have a laptop which such a camera, you will need to do most of the work yourself, as we cannot test.

So congratulations, you've just become a v4l kernel developer :)

Regards,

Hans

Quoted 59 lines

Is there any possibility that this driver could be merged into the kernel? The code could probably be simplified a lot and integrated into gspca.

diff -urp syntekdriver-code-107-trunk-orig/driver/stk11xx.h syntekdriver-code-107-trunk//driver/stk11xx.h ---
syntekdriver-code-107-trunk-orig/driver/stk11xx.h 2012-03-10
10:03:12.000000000 +0100 +++
syntekdriver-code-107-trunk//driver/stk11xx.h 2013-08-05
22:50:00.000000000 +0200 @@ -33,6 +33,7 @@

  #ifndef STK11XX_H
  #define STK11XX_H

/driver/stk11xx.h renamed from driver/stk11xx.h +2 -0

+#include <media/v4l2-device.h>
 
  #define DRIVER_NAME					"stk11xx"					/**< Name of this driver */
  #define DRIVER_VERSION				"v3.0.0"					/**< Version of this driver */

@@ -316,6 +317,7 @@ struct stk11xx_video {
   * @struct usb_stk11xx
   */
  struct usb_stk11xx {
+	struct v4l2_device v4l2_dev;
  	struct video_device *vdev; 			/**< Pointer on a V4L2 video device */
  	struct usb_device *udev;			/**< Pointer on a USB device */
  	struct usb_interface *interface;	/**< Pointer on a USB interface */

diff -urp syntekdriver-code-107-trunk-orig/driver/stk11xx-v4l.c syntekdriver-code-107-trunk//driver/stk11xx-v4l.c ---
syntekdriver-code-107-trunk-orig/driver/stk11xx-v4l.c 2012-03-10
09:54:57.000000000 +0100 +++
syntekdriver-code-107-trunk//driver/stk11xx-v4l.c 2013-08-05
22:51:12.000000000 +0200 @@ -1498,9 +1498,17 @@ int v4l_stk11xx_register_video_device(st
{
int err;

/driver/stk11xx-v4l.c renamed from driver/stk11xx-v4l.c +10 -1

+	err = v4l2_device_register(&dev->interface->dev, &dev->v4l2_dev);
+	if (err < 0) {
+		STK_ERROR("couldn't register v4l2_device\n");
+		kfree(dev);
+		return err;
+	}
+
  	strcpy(dev->vdev->name, DRIVER_DESC);
 
-	dev->vdev->parent = &dev->interface->dev;
+//	dev->vdev->parent = &dev->interface->dev;
+	dev->vdev->v4l2_dev = &dev->v4l2_dev;
  	dev->vdev->fops = &v4l_stk11xx_fops;
  	dev->vdev->release = video_device_release;
  	dev->vdev->minor = -1;

@@ -1533,6 +1541,7 @@ int v4l_stk11xx_unregister_video_device(
 
  	video_set_drvdata(dev->vdev, NULL);
  	video_unregister_device(dev->vdev);
+	v4l2_device_unregister(&dev->v4l2_dev);
 
  	return 0;
  }

Hi,

Quoted 11 lines · Ondrej Zary

Hello,
the in-kernel stkwebcam driver (by Jaime Velasco Juan and Nicolas VIVIEN) supports only two webcam types (USB IDs 0x174f:0xa311 and 0x05e1:0x0501). There are many other Syntek webcam types that are not supported by this driver (such as 0x174f:0x6a31 in Asus F5RL laptop).

There is an out-of-tree GPL driver called stk11xx (by Martin Roos and also Nicolas VIVIEN) at http://sourceforge.net/projects/syntekdriver/
which supports more webcams. It can be even compiled for the latest kernels using the patch below and seems to work somehow (slow and buggy but better than nothing) with the Asus F5RL.

I took a quick look and there are a number of issues with this driver:

1) It conflicts usb-id wise with the new stk1160 driver (which supports usb-id 05e1:0408) so support for that usb-id, and any code only used for that id will need to be removed

2) "seems to work somehow (slow and buggy)" is not really the quality we aim for with in kernel drivers. We definitely will want to remove any usb-ids, and any code only used for those ids, where there is overlap with the existing stkwebcam driver, to avoid regressions

3) It does in kernel bayer decoding, this is not acceptable, it needs to be modified to produce buffers with raw bayer data (libv4l will take care of the bater decoding in userspace).

4) It is not using any of the new kernel infrastructure we have been adding over time, like the control-framework, videobuf2, etc. It would be best to convert this to a gspca sub driver (of which there are many already, which can serve as examples), so that it will use all the existing framework code.