On Wed, 26 Sep 2012 11:47:55 +0200 Javier Martin <javier.martin@xxxxxxxxxxxxxxxxx> wrote: > According to the datasheet ov7675 uses a formula to achieve > the desired framerate that is different from the operations > done in the current code. > > In fact, this formula should apply to ov7670 too. This would > mean that current code is wrong but, in order to preserve > compatibility, the new formula will be used for ov7675 only. At this point I couldn't tell you what the real situation is; it's been a while and there's always a fair amount of black magic involved with ov7670 configuration. I do appreciate attention to not breaking existing users. > +static void ov7670_get_framerate(struct v4l2_subdev *sd, > + struct v4l2_fract *tpf) This bugs me, though. It's called ov7670_get_framerate() but it's getting the rate for the ov7675 - confusing. Meanwhile the real ov7670 code remains inline while ov7675 has its own function. Please make two functions, one of which is ov7675_get_framerate(), and call the right one for the model. Same for the "set" functions, obviously. Maybe what's really needed is a structure full of sensor-specific operations? The get_wsizes() function could go there too. That would take a lot of if statements out of the code. > + /* > + * The datasheet claims that clkrc = 0 will divide the input clock by 1 > + * but we've checked with an oscilloscope that it divides by 2 instead. > + * So, if clkrc = 0 just bypass the divider. > + */ Thanks for documenting this kind of thing. jon -- 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
