OK, I'll try and answer a few questions...
Brian Johnson schrieb:
Ok I've been really busy recently but i have a few comments about the recent patches. First i may have missed this but what is the reasoning behind statically assigning the sensors based on device? I don't really mind statically assigning the sensor perse, I just perfer determing the sensor using a single method and am unsure what was decided was wrong with simply probing for all sensor types.
We had to fix the probing anyway since probing, assigning functions/parameters and initializing at the same time made life difficult. At that point we decided to simplify the process if we already knew the sensor.
That being said if we would rather go with the dual approach, you should not be defining another array of webcams. We already have one in the form the device ID table in microdia-usb.c. Each member of that table has a 32bit driver_info field which can be used to store various information. Currently its storing just the bridge id in the upper 16 bits which we probably don't need to do since we are only supporting sn9c20x bridge. Anyways, that field should probably be used to store a sensor id. You could also make special sensor id called SENSOR_PROBE that if used will cause the driver to do probing.
You are referring to "struct usb_device_id microdia_bla[]"? I agree.
Ok second issue, why were all the format/resolutions condensed into a single microdia_default_fmts array, instead of remaining separate per sensor? This has some problems that i can see already. First our driver currently lies about what formats are supported. Since if you don't have have a set_yuv422 function defined for your sensor you don't really support that format yet our driver will say you do. Second and i think more problematic is that if you have multiple attached webcams they will all be using a reference to the same structure, which is a problem since you then procede to modifiy that shared reference based on what senspor was attached to the currently probed webcam. This will cause all sorts of confusion if your multiple webcams use different sensors.
You're right that wasn't very elegant but I didn't want to stuff all these old arrays into the headers. BTW - I made the respective v4l fucntion (v4l_try_fmt_blabla?) check if there actually is a set_yuv422 function. But another problem will be that some sensors have different vstarts/hstarts for lower resolutions. And of course I never thought someone could use 2 sn9c20x webcams at the same time. You're right this needs to be done via memcpy or something similar.
On more minor thing the sn9c20x_write_i2c_data function was modified to allow for an extra byte to be passed in, yet this does not seem to be used anywhere, Is this necessary for some yet unimplemented feature? I do really like the new software auto exposure for the omni vision sensors though it works just fine on my 624f.
GWater
signature.asc
Description: OpenPGP digital signature
