What todo with cams which have 2 drivers?

Hi All,

Now that gspca is part of the v4l-dvb tree, we have 2 drivers for several cams.

The "problem" is that various gspca subdrivers and various drivers written by Luca Risolia support the same controllers (and a subset of the same sensors too).

Currently this is solved by removing the usb-id's for any cams supported by Luca's drivers (which have been in the kernel for some time) from gspca when Luca's drivers are enabled.

Since the drivers written by Luca Risolia seem to be basicly unmaintained, for example luca has one last update to his sn9c102 driver on his website which he has never bother to merge upstream, and since Luca's drivers often have a poorer feature set then gspca (no exposure control for sn9c102 + tas5110 making it unusable in many lighting conditions for example), I would like to suggest to start removing usb-id's from Luca's drivers when gspca is enabled and leave them always enabled in gspca, atleast for those cams which have been thoroughly tested with gspca.

##

Another issue is that Luca's drivers claim to support usb-id's which they don't Luca uses probing to find out which sensor there is instead of using usb-id's and has added all usb-id's he could find for a controller, independend on wether or not his driver supports the sensor used in that usb-id.

For example the sn9c102 driver used to claim that it supports 0c45:6011 but it does not, as that uses an ov6650 sensor, which the sn9c102 driver does not support. I actually have such a cam and have tested it with Luca's driver and that only results in the following dmesg: "No supported image sensor detected for this bridge", a patch removing this usb-id has already been submitted (and accepted) through the gspca tree.

I've also noticed that Luca's zc0301 driver only supports 2 sensors the pas202 and the pb0330, where as the gspca zc3xx driver supports 18 different sensors!

If you look in zc0301_sensor.h you will find the following list:

         { ZC0301_USB_DEVICE(0x041e, 0x4017, 0xff), }, /* ICM105 */            \
         { ZC0301_USB_DEVICE(0x041e, 0x401c, 0xff), }, /* PAS106 */            \
         { ZC0301_USB_DEVICE(0x041e, 0x401e, 0xff), }, /* HV7131 */            \
         { ZC0301_USB_DEVICE(0x041e, 0x401f, 0xff), }, /* TAS5130 */           \
         { ZC0301_USB_DEVICE(0x041e, 0x4022, 0xff), },                         \
         { ZC0301_USB_DEVICE(0x041e, 0x4034, 0xff), }, /* PAS106 */            \
         { ZC0301_USB_DEVICE(0x041e, 0x4035, 0xff), }, /* PAS106 */            \
         { ZC0301_USB_DEVICE(0x041e, 0x4036, 0xff), }, /* HV7131 */            \
         { ZC0301_USB_DEVICE(0x041e, 0x403a, 0xff), }, /* HV7131 */            \
         { ZC0301_USB_DEVICE(0x0458, 0x7007, 0xff), }, /* TAS5130 */           \
         { ZC0301_USB_DEVICE(0x0458, 0x700c, 0xff), }, /* TAS5130 */           \
         { ZC0301_USB_DEVICE(0x0458, 0x700f, 0xff), }, /* TAS5130 */           \
         { ZC0301_USB_DEVICE(0x046d, 0x08ae, 0xff), }, /* PAS202 */            \
         { ZC0301_USB_DEVICE(0x055f, 0xd003, 0xff), }, /* TAS5130 */           \
         { ZC0301_USB_DEVICE(0x055f, 0xd004, 0xff), }, /* TAS5130 */           \
         { ZC0301_USB_DEVICE(0x0ac8, 0x0301, 0xff), },                         \
         { ZC0301_USB_DEVICE(0x0ac8, 0x301b, 0xff), }, /* PB-0330/HV7131 */    \
         { ZC0301_USB_DEVICE(0x0ac8, 0x303b, 0xff), }, /* PB-0330 */           \
         { ZC0301_USB_DEVICE(0x10fd, 0x0128, 0xff), }, /* TAS5130 */           \
         { ZC0301_USB_DEVICE(0x10fd, 0x8050, 0xff), }, /* TAS5130 */           \
         { ZC0301_USB_DEVICE(0x10fd, 0x804e, 0xff), }, /* TAS5130 */           \

Note how most of these cams cannot work with Luca's driver as there is no support for the mentioned sensors. So I'll be submitting a patch (through the gspca tree) enabling these cams unconditional in gspca and removing their usb-ids from Luca's zc0301 driver, given the small number of supported cams then left (2), we should consider deprecating the zc0301 driver all together.

I'll do an audit of Luca's other drivers for similar issues (claiming unsupport usb-id's)

In general I am getting worried by the lack of a common standard to interface to sensor and controller drivers. I'm no expert on webcams, so correct me if I'm wrong, but it seems to me that the correct design would be to have a generic API to these devices that can be used by the various USB or platform drivers.

But for e.g. the mt9* sensors we have three that use the soc_camera interface, one using the sn9c102 interface and I know of two more that are not yet in v4l-dvb but that will use the v4l2-int-device.h interface. It's a mess in my opinion.

I think it would be interesting to discuss this further with you in Portland.

Quoted 7 lines · Hans Verkuil

In general I am getting worried by the lack of a common standard to interface to sensor and controller drivers. I'm no expert on webcams, so correct me if I'm wrong, but it seems to me that the correct design would be to have a generic API to these devices that can be used by the various USB or platform drivers.

Quoted 5 lines

But for e.g. the mt9* sensors we have three that use the soc_camera interface, one using the sn9c102 interface and I know of two more that are not yet in v4l-dvb but that will use the v4l2-int-device.h interface. It's a mess in my opinion.

It is indeed, in my experience so far the easiest solution is to see the bridge and sensor (and this the whole cam) as one device. 80% of the per sensor code in a bridge driver is setting up the bridge to talk to the cam (there are various connections between the 2 possible and most bridges support multiple connection types) and 80% of the sensor code is configuring the sensor for a specific bridge (same story). The remaining 20% of sensor code is rather boring
(poking registers to set things like conteast and brightness) and since the
sensor needs to be programmed to talk to the bridge in a bridge specific way there is little to gain from having seperate sensor drivers as those still need to be patched for a new bridge type before the sensor will work with a certain bridge.

If you for example look at the attempting to be generic ovchipcam drivers in the current kernel tree, which contain code for the ov6xxx and ov7xxx sensors, then the attach routine contains the following:

         switch (adap->id) {
         case I2C_HW_SMBUS_OV511:
         case I2C_HW_SMBUS_OV518:
         case I2C_HW_SMBUS_W9968CF:
                 PDEBUG(1, "Adapter ID 0x%06x accepted", adap->id);
                 break;
         default:
                 PDEBUG(1, "Adapter ID 0x%06x rejected", adap->id);
                 return -ENODEV;
         }

IOW this generic sensor code will only work with 3 know bridges and some of the code itself is bridge specific too, for example in ov6x30.c :

         if (win->format == VIDEO_PALETTE_GREY) {
                 if (c->adapter->id == I2C_HW_SMBUS_OV518) {
                         /* Do nothing - we're already in 8-bit mode */
                 } else {
                         ov_write_mask(c, 0x13, 0x20, 0x20);
                 }
         } else {
                 /* The OV518 needs special treatment. Although both the OV518
                  * and the OV6630 support a 16-bit video bus, only the 8 bit Y
                  * bus is actually used. The UV bus is tied to ground.
                  * Therefore, the OV6630 needs to be in 8-bit multiplexed
                  * output mode */

                 if (c->adapter->id == I2C_HW_SMBUS_OV518) {
                         /* Do nothing - we want to stay in 8-bit mode */
                         /* Warning: Messing with reg 0x13 breaks OV518 color */
                 } else {
                         ov_write_mask(c, 0x13, 0x00, 0x20);
                 }
         }

I've done a comparison of code size between trying to use generic sensor code (which isn't that generic at all see above) and just putting sensor specific code into each bridge driver separately, see:
http://marc.info/?l=linux-video&m=121645882518114&w=2

So my conclusion (for now) is that trying to separate the sensor code out of the bridge drivers is not worth it. I plan to rewrite the ov511/ov518 driver for v4l2 using gspca (so that libv4l can be used to decode the special JPEG-ish format making these cams actually work). For this rewrite I will probably remove the ovcamchip stuff and just put sensor init and ctrl code in the bridge driver, probably resulting in a smaller driver.

Quoted 1 line

I think it would be interesting to discuss this further with you in Portland.

Yes it would, I think we need to make an agenda what we want to discuss (both in the miniconf and outside of that).

Quoted 9 lines · Hans Verkuil

In general I am getting worried by the lack of a common standard to interface to sensor and controller drivers. I'm no expert on webcams, so correct me if I'm wrong, but it seems to me that the correct design would
be to have a generic API to these devices that can be used by the various
USB or platform drivers.

Quoted 6 lines

But for e.g. the mt9* sensors we have three that use the soc_camera interface, one using the sn9c102 interface and I know of two more that are not yet in v4l-dvb but that will use the v4l2-int-device.h interface.
It's a mess in my opinion.

It is indeed, in my experience so far the easiest solution is to see the bridge and sensor (and this the whole cam) as one device. 80% of the per sensor code in a bridge driver is setting up the bridge to talk to the cam (there are various connections between the 2 possible and most bridges support multiple connection types) and 80% of the sensor code is configuring the sensor for a specific bridge (same story). The remaining 20% of sensor code is rather boring (poking registers to set things like conteast and brightness) and since the sensor needs to be programmed to talk to the bridge in a bridge specific way there is little to gain from having seperate sensor drivers as those still need to be patched for a new bridge type before the sensor will work with a certain bridge.

If you for example look at the attempting to be generic ovchipcam drivers in the current kernel tree, which contain code for the ov6xxx and ov7xxx sensors, then the attach routine contains the following:

        switch (adap->id) {
        case I2C_HW_SMBUS_OV511:
        case I2C_HW_SMBUS_OV518:
        case I2C_HW_SMBUS_W9968CF:
                PDEBUG(1, "Adapter ID 0x%06x accepted", adap->id);
                break;
        default:
                PDEBUG(1, "Adapter ID 0x%06x rejected", adap->id);
                return -ENODEV;
        }

IOW this generic sensor code will only work with 3 know bridges and some of the code itself is bridge specific too, for example in ov6x30.c :

        if (win->format == VIDEO_PALETTE_GREY) {
                if (c->adapter->id == I2C_HW_SMBUS_OV518) {
                        /* Do nothing - we're already in 8-bit mode */
                } else {
                        ov_write_mask(c, 0x13, 0x20, 0x20);
                }
        } else {
                /* The OV518 needs special treatment. Although both

the OV518
* and the OV6630 support a 16-bit video bus, only the
8 bit Y
* bus is actually used. The UV bus is tied to ground.
* Therefore, the OV6630 needs to be in 8-bit multiplexed
* output mode */

if (c->adapter->id == I2C_HW_SMBUS_OV518) {
/* Do nothing - we want to stay in 8-bit mode */ /* Warning: Messing with reg 0x13 breaks OV518
color */

                } else {
                        ov_write_mask(c, 0x13, 0x00, 0x20);
                }
        }

I've done a comparison of code size between trying to use generic sensor code (which isn't that generic at all see above) and just putting sensor specific code into each bridge driver separately, see:
http://marc.info/?l=linux-video&m=121645882518114&w=2

So my conclusion (for now) is that trying to separate the sensor code out of the bridge drivers is not worth it. I plan to rewrite the ov511/ov518 driver for v4l2 using gspca (so that libv4l can be used to decode the special JPEG-ish format making these cams actually work). For this rewrite I will probably remove the ovcamchip stuff and just put sensor init and ctrl code in the bridge driver, probably resulting in a smaller driver.

Quoted 2 lines

I think it would be interesting to discuss this further with you in Portland.

Yes it would, I think we need to make an agenda what we want to discuss (both in the miniconf and outside of that).