On Wed, 26 Sep 2012 11:47:55 +0200
Javier Martin <javier.mar...@vista-silicon.com> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to