Re: [PATCH 1/1] gspca: Add sn9c20x subdriver

[Brian Johnson <brijohn@gmail.com>]

Hans,
Thanks for the feedback, I'll work on the changes and hopefully post the revised patch this weekend.
> Hi,
> 
> First of all many many thanks for doings this!
> There are 4 issues with this driver, 2 of which are blockers:
> 
> 1) The big one is the use of a custom debugging mechanism,
>     please use the v4l standard debugging mechanism
>     which is activated by the kernel config option
>     VIDEO_ADV_DEBUG, please use this define to
>     enable / disable the debugging features of this
>     driver and use the standard VIDIOC_DBG_G_REGISTER
>     and VIDIOC_DBG_S_REGISTER ioctl's instead of an
>     sysfs interface. Note I'm not very familiar with
>     these myself, please send any questions on this to the
>     list.
>
Ok I'll change the debugging code to use those ioctl's instead of debugfs.
> 2) :
> 
>> +	case SENSOR_OV7660:
>> +		if (ov7660_init_sensor(gspca_dev)<  0)
>> +			return -ENODEV;
>> +		info("OV7660 sensor detected");
> 
> You are missing a break here! Which I found out because
> my only sn9c20x cam has ab ov7660 sensor
Oops. 

> 
>> +	case SENSOR_OV7670:
>> +		if (ov7670_init_sensor(gspca_dev)<  0)
>> +			return -ENODEV;
>> +		info("OV7670 sensor detected");
>> +		break;
> 
> 3) My cam works a lot better with the standalone driver
> then with you're gspca version. With your version it shows
> a bayer pattern ish pattern over the whole picture as if
> the bayer pixel order is of, except that the colors are right
> so that is most likely not the cause. I'll investigate this
> further as time permits.
> 
Hmm, Hans can you see if disabling the code for hvflip on the ov7660 helps any?

> 4) The evdev device creation and handling realyl belongs in the
> gspca core, as we can (and should) handle the snapshot button
> in other drivers too, but this is something which can be fixed
> after merging.


Thanks,
Brian Johnson