Hi Andrew,

On Friday 24 June 2011 01:19:41 ac...@nvidia.com wrote:
> From: Andrew Chew <ac...@nvidia.com>
> 
> Based on vendor feedback, should issue a software reset at start of day.
> 
> Also, OV9740_ANALOG_CTRL15 needs to be changed so the sensor does not begin
> streaming until it is ready (otherwise, results in a nonsense frame for the
> initial frame).
> 
> Added a comment on using discontinuous clock.
> 
> Finally, OV9740_ISP_CTRL19 needs to be changed to really use YUYV ordering
> (the previous value was for VYUY).

I've never liked those magic values. Fixing the whole driver to document 
everything in one go might be too much work, but what about documenting new 
registers, and especially their contents, as you add support for them ?

> Signed-off-by: Andrew Chew <ac...@nvidia.com>
> ---
>  drivers/media/video/ov9740.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/ov9740.c b/drivers/media/video/ov9740.c
> index d5c9061..72c6ac1d 100644
> --- a/drivers/media/video/ov9740.c
> +++ b/drivers/media/video/ov9740.c
> @@ -68,6 +68,7 @@
>  #define OV9740_ANALOG_CTRL04         0x3604
>  #define OV9740_ANALOG_CTRL10         0x3610
>  #define OV9740_ANALOG_CTRL12         0x3612
> +#define OV9740_ANALOG_CTRL15         0x3615
>  #define OV9740_ANALOG_CTRL20         0x3620
>  #define OV9740_ANALOG_CTRL21         0x3621
>  #define OV9740_ANALOG_CTRL22         0x3622
> @@ -222,6 +223,9 @@ struct ov9740_priv {
>  };
> 
>  static const struct ov9740_reg ov9740_defaults[] = {
> +     /* Software Reset */
> +     { OV9740_SOFTWARE_RESET,        0x01 },
> +
>       /* Banding Filter */
>       { OV9740_AEC_B50_STEP_HI,       0x00 },
>       { OV9740_AEC_B50_STEP_LO,       0xe8 },
> @@ -333,6 +337,7 @@ static const struct ov9740_reg ov9740_defaults[] = {
>       { OV9740_ANALOG_CTRL10,         0xa1 },
>       { OV9740_ANALOG_CTRL12,         0x24 },
>       { OV9740_ANALOG_CTRL22,         0x9f },
> +     { OV9740_ANALOG_CTRL15,         0xf0 },
> 
>       /* Sensor Control */
>       { OV9740_SENSOR_CTRL03,         0x42 },
> @@ -385,7 +390,7 @@ static const struct ov9740_reg ov9740_defaults[] = {
>       { OV9740_LN_LENGTH_PCK_LO,      0x62 },
> 
>       /* MIPI Control */
> -     { OV9740_MIPI_CTRL00,           0x44 },
> +     { OV9740_MIPI_CTRL00,           0x44 }, /* 0x64 for discontinuous clk */

You should instead add

#define OV9740_MIPI_CTRL00_DISC_CLK             0x20

just below the OV9740_MIPI_CTRL00 definition, with a comment to explain what 
it's for (and rename CTRL00 to something more meaningful if possible). Same 
for the other registers you modify.

>       { OV9740_MIPI_3837,             0x01 },
>       { OV9740_MIPI_CTRL01,           0x0f },
>       { OV9740_MIPI_CTRL03,           0x05 },
> @@ -393,6 +398,9 @@ static const struct ov9740_reg ov9740_defaults[] = {
>       { OV9740_VFIFO_RD_CTRL,         0x16 },
>       { OV9740_MIPI_CTRL_3012,        0x70 },
>       { OV9740_SC_CMMM_MIPI_CTR,      0x01 },
> +
> +     /* YUYV order */
> +     { OV9740_ISP_CTRL19,            0x02 },
>  };
> 
>  static const struct ov9740_reg ov9740_regs_vga[] = {

-- 
Regards,

Laurent Pinchart
--
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