Am 20.08.2012 21:21, schrieb Mauro Carvalho Chehab: > Em 20-08-2012 10:02, Hans de Goede escreveu: >> Hi, >> >> On 08/20/2012 01:41 PM, Frank Schäfer wrote: >>> Hi, >>> >>> after a break of 2 1/2 kernel releases (sorry, I was busy with another >>> project), I would like to bring up again the question how to add support >>> for this device to the kernel. >>> See >>> http://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg44417.html >>> ("Move em27xx/em28xx webcams to a gspca subdriver ?") for the previous >>> discussion. >>> >>> Current status is, that I've reverse-engineered the Windows driver and >>> written a new gspca-subdriver for testing, which is feature complete and >>> working stable (will send a patch shortly !). >>> >>> The device uses an em2765-bridge, so my first idea was of course to >>> modify/extend the em28xx-driver. >>> But during the reverse-engineering-process, it turned out that writing a >>> new gspca-subdriver was much easier than modifying the em28xx-driver. >>> >>> The device has the following special characteristics: >>> - supports only bulk transfers (em28xx driver supports ISOC only) > Em28xx driver supports both isoc and bulk transfers, as bulk is > required by DVB. Hmm... are you 100% sure ? Must have been added recently then... I did a quick check of the current code, but can't find anything. Could you please give me a pointer to the code parts ? Btw, if I'm not understanding the code wrong, em28xx_usb_probe() still seems to return -ENODEV if no isoc-in endpoint is found, so bulk-ep-only devices should not work... > >>> - uses "proprietary" read/write procedures for the sensor > Sure about that? It doesn't use I2C? According to the datasheet of the OV2640 it should be SCCB. Anyway, I'm referring to how communication works on the USB level. Take a look at http://linuxtv.org/wiki/index.php/VAD_Laplace section "Reverse Engineering (evaluation of USB-logs)" to see how it is working. Interestingly, the standard I2C reads are used, too, for reading the EEPROM. So maybe there is a "physical" difference. "Proprietary" is probably not the best name, but I don't have e better one at the moment (suggestions ?). >>> - uses 16bit eeprom >>> - em25xx-eeprom with different layout > There are other supported chips with 16bit eeproms. Currently, > support for 16bit eeproms is disabled just because this weren't > needed so far, but I'm sure this is a need there. Yes, I've read the comment in em28xx_i2c_eeprom(): "...there is the risk that we could corrupt the eeprom (since a 16-bit read call is interpreted as a write call by 8-bit eeproms)..." How can we know if a device uses an 8bit or 16bit EEPROM ? Can we derive that from the uses em27xx/28xx-chip ? Anyway, this problem is common to both solutions (gspca or em28xx-driver). >>> - sensor OV2640 > There is a driver for it at: > drivers/media/i2c/soc_camera/ov2640.c > > The better is to use it (even if this got mapped via gspca). Yes, I know. This is already on my ToDo list. >>> - different frame processing >>> - 3 buttons (snapshot, mute, light) which need special treatment >>> (GPIO-polling, status-reseting, ...) > Need to see the code to better understand that. Take a look at the patch. http://www.spinics.net/lists/linux-media/msg52084.html I've sent it to the list 3 minutes after I started this thread and also CC'ed you. >>> Another important point to mention: you can see from the USB-logs >>> (sensor probing) that there must be at least 3 other webcam devices. > What do you mean? The Windows driver probes 3 other sensor addresses (using the same "proprietary" reads). I've included them in my patch. The INF-file does not contain any other USB IDs, but I think it is unlikely that they are used by this device. SpeedLink spent different USB IDs even for identical devices with different body colors, so I think they would have done they same for variants with different sensors. It is more likely that the Windows driver is a kind of universal em2765 driver. >>> Some pros and cons for both solutions: >>> >>> em28xx: >>> + one driver for all 25xx/26xx/27xx/28xx devices >>> + no duplicate code (bridge register defines, bridge read/write fcns) >>> + other devices COULD benefit from the new functions/code >>> - big task/lots of work >>> - code gets bloated with stuff, which is only needed by a few special >>> devices > It all depends on what's needed ;) > >>> gspca: >>> + driver already exists (see patch) > Which patch? See above. >>> + default driver for webcams >>> + much easier to understand and extend >>> + same or even less amount of new code lines >>> + keeps em28xx-code "simple" >>> - code duplication >>> - support for em28xx-webcams spread over to 2 different drivers > The spread of em27xx support on 2 different drivers can lead into > problems for the users to know what driver implements support for > their device. Ok, but that's what dmesg is for, isn't it ? And people willing to add support for a new device have to contact the linux-media ML anyway. Btw: there is another "non-technical" argument for a gspca subdriver: For the remaining unsupported em2xxx webcams, we probably depend on people doing reverse-engineering of their device. People which might not be that skilled (like me ;-) ). A gspca subdriver is MUCH easier to understand and to modify, than the em28xx-driver. I really tried and I think I didn't give up to early, but... > So, if we're going to do that, then the better is to move support > for all em27xx devices out of em28xx driver, but I think that this > will end by creating duplicated stuff. You are probably right. I don't like the idea of having two different drivers for the same webcam family, but I think separate drivers for webcams and TV-devices would be ok. Concerning code duplication: I think it isn't too much. We should also take the total number of code lines into account. For example, we could get rid of field is_webcam in the em28xx device struct, which saves some lines of code. But form your own opinion by comparing the code. > Btw, how is audio with your em2765 device? Is it provided via > snd-usb-audio, or does it require some code like the one inside > em28xx? It works out of the box with snd-usb-audio. But, of course, at least in theory, there could be webcams requiring the special code. I'm not sure how likely that is, given that the audio part of webcam is just a microphone. >>> I have no strong opinion whether support for this device should finally >>> be added to em28xx or gspca and >>> I'm willing to continue working on both solutions as much as my time >>> permits and as long as I'm having fun (I'm doing this as a hobby !). >>> Anyway, the em28xx driver is very complex and I really think it would >>> take several further kernel releases to get the job done... >>> I would also be willing to spend some time for moving the em28xx-webcam >>> code to a gspca subdriver, but I don't have any of these devices for >>> testing. >>> >>> What do you think ? >> I think given the special way this camera uses the bridge (not using >> standard i2c interface, weird button layout, etc.). That it is likely server >> better by a specialized driver. As the (new) gspca maintainer I'm fine with >> taking it as a gspca sub-driver, but given the code duplication issue, >> that is a call Mauro should make. >> >> Note that luckily these devices do use a unique USB id and not one of the >> generic em28xx ids so from that pov having a specialized driver for them >> is not an issue. > Hans, > > Not sure if all em2765 cameras will have unique USB id's: at em28xx, > the known em2710/em2750 cameras that don't have unique ID's; detecting > between them requires to probe for the type of sensor. Hmmm... I'm aware of non-unique-id problem, but is it really possible that an ID is used for a webcam and a for example a DVB-T-device ? Regards, Frank > Regards, > Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html