diff mbox

[2/3] Add driver for OmniVision OV9640 sensor

Message ID 200909042225.59000.marek.vasut@gmail.com (mailing list archive)
State RFC
Headers show

Commit Message

Marek Vasut Sept. 4, 2009, 8:25 p.m. UTC
Dne Po 24. srpna 2009 20:06:29 Guennadi Liakhovetski napsal(a):
> Hi Marek
>
> On Sat, 22 Aug 2009, Marek Vasut wrote:
> > From 479aafc9a6540efec8a691a84aff166eb0218a72 Mon Sep 17 00:00:00 2001
> > From: Marek Vasut <marek.vasut@gmail.com>
> > Date: Sat, 22 Aug 2009 05:14:23 +0200
> > Subject: [PATCH 2/3] Add driver for OmniVision OV9640 sensor
> >
> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
>
> Ok, I see, you rebased your patches on my soc-camera patch-stack, this is
> good, thanks. But you missed a couple of my comments - you still have a
> few static functions in the ov9640.c marked inline: ov9640_set_crop() is
> not used at all, ov9640_set_bus_param() gets assigned to a struct member,
> so, cannot be inline. ov9640_alter_regs() is indeed only called at one
> location, but see Chapter 15 in Documentation/CodingStyle. You left at
> least one multi-line comment wrongly formatted. You left some broken
> printk format lines, etc. You moved your header under drivers/... - that's
> good. But, please, address all my comments that I provided in a private
> review, or explain why you do not agree. Otherwise I feel like I wasted my
> time. Besides, your mailer has wrapped lines. Please, read
> Documentation/email-clients.txt on how to configure your email client to
> send patches properly. In the worst case, you can inline your patches
> while reviewing, and then attach them for a final submission. This is,
> however, discouraged, because it makes review and test of your
> intermediate patches with wrapped lines more difficult. Also, please check
> your patches with scripts/checkpatch.pl.
>

Fixed patch follows, my mailer is fixed as much as possible (working on getting 
git-email to work, but that's still to be done). Please consider applying, 
thanks.

Add driver for OmniVision OV9640 sensor

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/media/video/Kconfig     |    6 +
 drivers/media/video/Makefile    |    1 +
 drivers/media/video/ov9640.c    |  811 +++++++++++++++++++++++++++++++++++++++
 drivers/media/video/ov9640.h    |  206 ++++++++++
 include/media/v4l2-chip-ident.h |    1 +
 5 files changed, 1025 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/video/ov9640.c
 create mode 100644 drivers/media/video/ov9640.h

Comments

Guennadi Liakhovetski Sept. 11, 2009, 9:51 p.m. UTC | #1
On Fri, 4 Sep 2009, Marek Vasut wrote:

> Dne Po 24. srpna 2009 20:06:29 Guennadi Liakhovetski napsal(a):
> > Hi Marek
> >
> > On Sat, 22 Aug 2009, Marek Vasut wrote:
> > > From 479aafc9a6540efec8a691a84aff166eb0218a72 Mon Sep 17 00:00:00 2001
> > > From: Marek Vasut <marek.vasut@gmail.com>
> > > Date: Sat, 22 Aug 2009 05:14:23 +0200
> > > Subject: [PATCH 2/3] Add driver for OmniVision OV9640 sensor
> > >
> > > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> >
> > Ok, I see, you rebased your patches on my soc-camera patch-stack, this is
> > good, thanks. But you missed a couple of my comments - you still have a
> > few static functions in the ov9640.c marked inline: ov9640_set_crop() is
> > not used at all, ov9640_set_bus_param() gets assigned to a struct member,
> > so, cannot be inline. ov9640_alter_regs() is indeed only called at one
> > location, but see Chapter 15 in Documentation/CodingStyle. You left at
> > least one multi-line comment wrongly formatted. You left some broken
> > printk format lines, etc. You moved your header under drivers/... - that's
> > good. But, please, address all my comments that I provided in a private
> > review, or explain why you do not agree. Otherwise I feel like I wasted my
> > time. Besides, your mailer has wrapped lines. Please, read
> > Documentation/email-clients.txt on how to configure your email client to
> > send patches properly. In the worst case, you can inline your patches
> > while reviewing, and then attach them for a final submission. This is,
> > however, discouraged, because it makes review and test of your
> > intermediate patches with wrapped lines more difficult. Also, please check
> > your patches with scripts/checkpatch.pl.
> >
> 
> Fixed patch follows, my mailer is fixed as much as possible (working on getting 
> git-email to work, but that's still to be done). Please consider applying, 
> thanks.

Ok, a couple more simple questions / remarks, In principle, there's just 
one principle objection - if we agree, that the correct format is BGR565 
and RGB565X, then we should change that. There's no BGR565 format 
currently in the kernel, so, we'd have to add it (and the documentation 
for the mercurial tree).

Thanks
Guennadi

> 
> Add driver for OmniVision OV9640 sensor
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> ---
>  drivers/media/video/Kconfig     |    6 +
>  drivers/media/video/Makefile    |    1 +
>  drivers/media/video/ov9640.c    |  811 +++++++++++++++++++++++++++++++++++++++
>  drivers/media/video/ov9640.h    |  206 ++++++++++
>  include/media/v4l2-chip-ident.h |    1 +
>  5 files changed, 1025 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/media/video/ov9640.c
>  create mode 100644 drivers/media/video/ov9640.h
> 
> diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> index 84b6fc1..fddd654 100644
> --- a/drivers/media/video/Kconfig
> +++ b/drivers/media/video/Kconfig
> @@ -772,6 +772,12 @@ config SOC_CAMERA_OV772X
>  	help
>  	  This is a ov772x camera driver
>  
> +config SOC_CAMERA_OV9640
> +	tristate "ov9640 camera support"
> +	depends on SOC_CAMERA && I2C
> +	help
> +	  This is a ov9640 camera driver
> +
>  config MX1_VIDEO
>  	bool
>  
> diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
> index 9f2e321..e18efd5 100644
> --- a/drivers/media/video/Makefile
> +++ b/drivers/media/video/Makefile
> @@ -76,6 +76,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9M111)	+= mt9m111.o
>  obj-$(CONFIG_SOC_CAMERA_MT9T031)	+= mt9t031.o
>  obj-$(CONFIG_SOC_CAMERA_MT9V022)	+= mt9v022.o
>  obj-$(CONFIG_SOC_CAMERA_OV772X)		+= ov772x.o
> +obj-$(CONFIG_SOC_CAMERA_OV9640)		+= ov9640.o
>  obj-$(CONFIG_SOC_CAMERA_TW9910)		+= tw9910.o
>  
>  # And now the v4l2 drivers:
> diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c
> new file mode 100644
> index 0000000..289e82d
> --- /dev/null
> +++ b/drivers/media/video/ov9640.c
> @@ -0,0 +1,811 @@
> +/*
> + * OmniVision OV96xx Camera Driver
> + *
> + * Copyright (C) 2009 Marek Vasut <marek.vasut@gmail.com>
> + *
> + * Based on ov772x camera driver:
> + *
> + * Copyright (C) 2008 Renesas Solutions Corp.
> + * Kuninori Morimoto <morimoto.kuninori@renesas.com>
> + *
> + * Based on ov7670 and soc_camera_platform driver,
> + *
> + * Copyright 2006-7 Jonathan Corbet <corbet@lwn.net>
> + * Copyright (C) 2008 Magnus Damm
> + * Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/videodev2.h>
> +#include <media/v4l2-chip-ident.h>
> +#include <media/v4l2-common.h>
> +#include <media/soc_camera.h>
> +
> +#include "ov9640.h"
> +
> +/* default register setup */
> +static const struct ov9640_reg ov9640_regs_dflt[] = {
> +	{ OV9640_COM5,	OV9640_COM5_SYSCLK | OV9640_COM5_LONGEXP },
> +	{ OV9640_COM6,	OV9640_COM6_OPT_BLC | OV9640_COM6_ADBLC_BIAS |
> +			OV9640_COM6_FMT_RST | OV9640_COM6_ADBLC_OPTEN },
> +	{ OV9640_PSHFT,	OV9640_PSHFT_VAL(0x01) },
> +	{ OV9640_ACOM,	OV9640_ACOM_2X_ANALOG | OV9640_ACOM_RSVD },
> +	{ OV9640_COM16,	OV9640_COM16_RB_AVG },
> +
> +	/* Gamma curve P */
> +	{ 0x6c, 0x40 },	{ 0x6d, 0x30 },	{ 0x6e, 0x4b },	{ 0x6f, 0x60 },
> +	{ 0x70, 0x70 },	{ 0x71, 0x70 },	{ 0x72, 0x70 },	{ 0x73, 0x70 },
> +	{ 0x74, 0x60 },	{ 0x75, 0x60 },	{ 0x76, 0x50 },	{ 0x77, 0x48 },
> +	{ 0x78, 0x3a },	{ 0x79, 0x2e },	{ 0x7a, 0x28 },	{ 0x7b, 0x22 },
> +
> +	/* Gamma curve T */
> +	{ 0x7c, 0x04 },	{ 0x7d, 0x07 },	{ 0x7e, 0x10 },	{ 0x7f, 0x28 },
> +	{ 0x80, 0x36 },	{ 0x81, 0x44 },	{ 0x82, 0x52 },	{ 0x83, 0x60 },
> +	{ 0x84, 0x6c },	{ 0x85, 0x78 },	{ 0x86, 0x8c },	{ 0x87, 0x9e },
> +	{ 0x88, 0xbb },	{ 0x89, 0xd2 },	{ 0x8a, 0xe6 },
> +};
> +
> +/* Configurations
> + * NOTE: for YUV, alter the following registers:
> + * 		COM12 |= OV9640_COM12_YUV_AVG
> + *
> + *	 for RGB, alter the following registers:
> + *	 	COM7  |= OV9640_COM7_RGB
> + *	 	COM13 |= OV9640_COM13_RGB_AVG
> + *	 	COM15 |= proper RGB color encoding mode
> + */
> +static const struct ov9640_reg ov9640_regs_qqcif[] = {
> +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x0f) },
> +	{ OV9640_COM1,	OV9640_COM1_QQFMT | OV9640_COM1_HREF_2SKIP },
> +	{ OV9640_COM4,	OV9640_COM4_QQ_VP | OV9640_COM4_RSVD },
> +	{ OV9640_COM7,	OV9640_COM7_QCIF },
> +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> +};
> +
> +static const struct ov9640_reg ov9640_regs_qqvga[] = {
> +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x07) },
> +	{ OV9640_COM1,	OV9640_COM1_QQFMT | OV9640_COM1_HREF_2SKIP },
> +	{ OV9640_COM4,	OV9640_COM4_QQ_VP | OV9640_COM4_RSVD },
> +	{ OV9640_COM7,	OV9640_COM7_QVGA },
> +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> +};
> +
> +static const struct ov9640_reg ov9640_regs_qcif[] = {
> +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x07) },
> +	{ OV9640_COM4,	OV9640_COM4_QQ_VP | OV9640_COM4_RSVD },
> +	{ OV9640_COM7,	OV9640_COM7_QCIF },
> +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> +};
> +
> +static const struct ov9640_reg ov9640_regs_qvga[] = {
> +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x03) },
> +	{ OV9640_COM4,	OV9640_COM4_QQ_VP | OV9640_COM4_RSVD },
> +	{ OV9640_COM7,	OV9640_COM7_QVGA },
> +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> +};
> +
> +static const struct ov9640_reg ov9640_regs_cif[] = {
> +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x03) },
> +	{ OV9640_COM3,	OV9640_COM3_VP },
> +	{ OV9640_COM7,	OV9640_COM7_CIF },
> +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> +};
> +
> +static const struct ov9640_reg ov9640_regs_vga[] = {
> +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x01) },
> +	{ OV9640_COM3,	OV9640_COM3_VP },
> +	{ OV9640_COM7,	OV9640_COM7_VGA },
> +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> +};
> +
> +static const struct ov9640_reg ov9640_regs_sxga[] = {
> +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x01) },
> +	{ OV9640_COM3,	OV9640_COM3_VP },
> +	{ OV9640_COM7,	0 },
> +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> +};
> +
> +static const struct ov9640_reg ov9640_regs_yuv[] = {
> +	{ OV9640_MTX1,	0x50 },
> +	{ OV9640_MTX2,	0x43 },
> +	{ OV9640_MTX3,	0x0d },
> +	{ OV9640_MTX4,	0x19 },
> +	{ OV9640_MTX5,	0x4c },
> +	{ OV9640_MTX6,	0x65 },
> +	{ OV9640_MTX7,	0x40 },
> +	{ OV9640_MTX8,	0x40 },
> +	{ OV9640_MTX9,	0x40 },
> +	{ OV9640_MTXS,	0x0f },
> +};
> +
> +static const struct ov9640_reg ov9640_regs_rgb[] = {
> +	{ OV9640_MTX1,	0x71 },
> +	{ OV9640_MTX2,	0x3e },
> +	{ OV9640_MTX3,	0x0c },
> +	{ OV9640_MTX4,	0x33 },
> +	{ OV9640_MTX5,	0x72 },
> +	{ OV9640_MTX6,	0x00 },
> +	{ OV9640_MTX7,	0x2b },
> +	{ OV9640_MTX8,	0x66 },
> +	{ OV9640_MTX9,	0xd2 },
> +	{ OV9640_MTXS,	0x65 },
> +};
> +
> +/* NOTE: The RGB555X format still has issues, so it's left out. */
> +static const struct soc_camera_data_format ov9640_fmt_lists[] = {
> +{
> +	.name		= "VYUY",
> +	.fourcc		= V4L2_PIX_FMT_VYUY,
> +	.depth		= 16,
> +	.colorspace	= V4L2_COLORSPACE_JPEG,
> +},
> +{
> +	.name		= "RGB565X",
> +	.fourcc		= V4L2_PIX_FMT_RGB565X,

Hm, so, do we keep RGB565X here or do we add a BGR565? We'll anyway have 
to do that when converting to imagebus, so, better do it right straight 
away, to avoid having to modify user-space apps.

> +	.depth		= 16,
> +	.colorspace	= V4L2_COLORSPACE_SRGB,
> +}
> +};
> +
> +static const struct v4l2_queryctrl ov9640_controls[] = {
> +	{
> +		.id		= V4L2_CID_VFLIP,
> +		.type		= V4L2_CTRL_TYPE_BOOLEAN,
> +		.name		= "Flip Vertically",
> +		.minimum	= 0,
> +		.maximum	= 1,
> +		.step		= 1,
> +		.default_value	= 0,
> +	},
> +	{
> +		.id		= V4L2_CID_HFLIP,
> +		.type		= V4L2_CTRL_TYPE_BOOLEAN,
> +		.name		= "Flip Horizontally",
> +		.minimum	= 0,
> +		.maximum	= 1,
> +		.step		= 1,
> +		.default_value	= 0,
> +	},
> +};
> +
> +/* read a register */
> +static int ov9640_reg_read(struct i2c_client *client, u8 reg, u8 *val)
> +{
> +	int ret;
> +	u8 data = reg;
> +	struct i2c_msg msg = {
> +		.addr	= client->addr,
> +		.flags	= 0,
> +		.len	= 1,
> +		.buf	= &data,
> +	};
> +
> +	ret = i2c_transfer(client->adapter, &msg, 1);

Are there any advantages in using raw i2c operations over smbus calls? The 
latter look much simpler, cf., e.g., mt9m001:

static int reg_read(struct i2c_client *client, const u8 reg)
{
	s32 data = i2c_smbus_read_word_data(client, reg);
	return data < 0 ? data : swab16(data);
}

static int reg_write(struct i2c_client *client, const u8 reg,
		     const u16 data)
{
	return i2c_smbus_write_word_data(client, reg, swab16(data));
}

Ok, going through smbus layer takes a bit of time, but that's just done 
during configuration.

> +	if (ret < 0)
> +		goto err;
> +
> +	msg.flags = I2C_M_RD;
> +	ret = i2c_transfer(client->adapter, &msg, 1);
> +	if (ret < 0)
> +		goto err;
> +
> +	*val = data;
> +	return 0;
> +
> +err:
> +	dev_err(&client->dev, "Failed reading register 0x%02x!\n", reg);
> +	return ret;
> +}
> +
> +/* write a register */
> +static int ov9640_reg_write(struct i2c_client *client, u8 reg, u8 val)
> +{
> +	int ret;
> +	u8 _val;
> +	unsigned char data[2] = { reg, val };
> +	struct i2c_msg msg = {
> +		.addr	= client->addr,
> +		.flags	= 0,
> +		.len	= 2,
> +		.buf	= data,
> +	};
> +
> +	ret = i2c_transfer(client->adapter, &msg, 1);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Failed writing register 0x%02x!\n", reg);
> +		return ret;
> +	}
> +
> +	/* we have to read the register back ... no idea why, maybe HW bug */
> +	ret = ov9640_reg_read(client, reg, &_val);
> +	if (ret)
> +		dev_err(&client->dev,
> +			"Failed reading back register 0x%02x!\n", reg);
> +
> +	return 0;
> +}
> +
> +
> +/* Read a register, alter its bits, write it back */
> +static int ov9640_reg_rmw(struct i2c_client *client, u8 reg, u8 set, u8 unset)
> +{
> +	u8 val;
> +	int ret;
> +
> +	ret = ov9640_reg_read(client, reg, &val);
> +	if (ret) {
> +		dev_err(&client->dev,
> +			"[Read]-Modify-Write of register %02x failed!\n", reg);
> +		return val;
> +	}
> +
> +	val |= set;
> +	val &= ~unset;
> +
> +	ret = ov9640_reg_write(client, reg, val);
> +	if (ret)
> +		dev_err(&client->dev,
> +			"Read-Modify-[Write] of register %02x failed!\n", reg);
> +
> +	return ret;
> +}
> +
> +/* Soft reset the camera. This has nothing to do with the RESET pin! */
> +static int ov9640_reset(struct i2c_client *client)
> +{
> +	int ret;
> +
> +	ret = ov9640_reg_write(client, OV9640_COM7, OV9640_COM7_SCCB_RESET);
> +	if (ret)
> +		dev_err(&client->dev,
> +			"An error occured while entering soft reset!\n");
> +
> +	return ret;
> +}
> +
> +/* Start/Stop streaming from the device */
> +static int ov9640_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	return 0;
> +}
> +
> +/* Alter bus settings on camera side */
> +static int ov9640_set_bus_param(struct soc_camera_device *icd,
> +					unsigned long flags)
> +{
> +	return 0;
> +}
> +
> +/* Request bus settings on camera side */
> +static unsigned long ov9640_query_bus_param(struct soc_camera_device *icd)
> +{
> +	struct soc_camera_link *icl = to_soc_camera_link(icd);
> +
> +	/*
> +	 * REVISIT: the camera probably can do 10 bit transfers, but I don't
> +	 *          have those pins connected on my hardware.
> +	 */
> +	unsigned long flags = SOCAM_PCLK_SAMPLE_RISING | SOCAM_MASTER |
> +		SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_HSYNC_ACTIVE_HIGH |
> +		SOCAM_DATA_ACTIVE_HIGH | SOCAM_DATAWIDTH_8;
> +
> +	return soc_camera_apply_sensor_flags(icl, flags);
> +}
> +
> +/* Get status of additional camera capabilities */
> +static int ov9640_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
> +{
> +	struct i2c_client *client = sd->priv;
> +	struct ov9640_priv *priv = container_of(i2c_get_clientdata(client),
> +					struct ov9640_priv, subdev);
> +
> +	switch (ctrl->id) {
> +	case V4L2_CID_VFLIP:
> +		ctrl->value = priv->flag_vflip;
> +		break;
> +	case V4L2_CID_HFLIP:
> +		ctrl->value = priv->flag_hflip;
> +		break;
> +	}
> +	return 0;
> +}
> +
> +/* Set status of additional camera capabilities */
> +static int ov9640_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
> +{
> +	struct i2c_client *client = sd->priv;
> +	struct ov9640_priv *priv = container_of(i2c_get_clientdata(client),
> +					struct ov9640_priv, subdev);
> +
> +	int ret = 0;
> +
> +	switch (ctrl->id) {
> +	case V4L2_CID_VFLIP:
> +		priv->flag_vflip = ctrl->value;
> +		if (ctrl->value)
> +			ret = ov9640_reg_rmw(client, OV9640_MVFP,
> +							OV9640_MVFP_V, 0);
> +		else
> +			ret = ov9640_reg_rmw(client, OV9640_MVFP,
> +							0, OV9640_MVFP_V);
> +		break;
> +	case V4L2_CID_HFLIP:
> +		priv->flag_hflip = ctrl->value;
> +		if (ctrl->value)
> +			ret = ov9640_reg_rmw(client, OV9640_MVFP,
> +							OV9640_MVFP_H, 0);
> +		else
> +			ret = ov9640_reg_rmw(client, OV9640_MVFP,
> +							0, OV9640_MVFP_H);
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +/* Get chip identification */
> +static int ov9640_g_chip_ident(struct v4l2_subdev *sd,
> +				struct v4l2_dbg_chip_ident *id)
> +{
> +	struct i2c_client *client = sd->priv;
> +	struct ov9640_priv *priv = container_of(i2c_get_clientdata(client),
> +					struct ov9640_priv, subdev);
> +
> +	id->ident	= priv->model;
> +	id->revision	= priv->revision;
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> +static int ov9640_get_register(struct soc_camera_device *icd,
> +			       struct v4l2_dbg_register *reg)
> +{
> +	struct i2c_client *client = to_i2c_client(to_soc_camera_control(icd));
> +	struct ov9640_priv *priv = i2c_get_clientdata(client);
> +
> +	int ret;
> +	u8 val;
> +
> +	if (reg->reg & ~0xff)
> +		return -EINVAL;
> +
> +	reg->size = 1;
> +
> +	ret = ov9640_read_reg(priv->client, reg->reg, &val);
> +	if (ret)
> +		return ret;
> +
> +	reg->val = (__u64)val;
> +
> +	return 0;
> +}
> +
> +static int ov9640_set_register(struct soc_camera_device *icd,
> +			       struct v4l2_dbg_register *reg)
> +{
> +	struct i2c_client *client = to_i2c_client(to_soc_camera_control(icd));
> +	struct ov9640_priv *priv = i2c_get_clientdata(client);
> +
> +	if (reg->reg & ~0xff || reg->val & ~0xff)
> +		return -EINVAL;
> +
> +	return ov9640_write_reg(priv->client, reg->reg, reg->val);
> +}
> +#endif
> +
> +/* select nearest higher resolution for capture */
> +static void ov9640_res_roundup(u32 *width, u32 *height)
> +{
> +	int i;
> +	enum { QQCIF, QQVGA, QCIF, QVGA, CIF, VGA, SXGA };
> +	int res_x[] = { 88, 160, 176, 320, 352, 640, 1280 };
> +	int res_y[] = { 72, 120, 144, 240, 288, 480, 960 };
> +
> +	for (i = 0; i < ARRAY_SIZE(res_x); i++) {
> +		if (res_x[i] >= *width && res_y[i] >= *height) {
> +			*width = res_x[i];
> +			*height = res_y[i];
> +			return;
> +		}
> +	}
> +
> +	*width = res_x[SXGA];
> +	*height = res_y[SXGA];
> +}
> +
> +/* Prepare necessary register changes depending on color encoding */
> +static void ov9640_alter_regs(u32 pixfmt, struct ov9640_reg_alt *alt)
> +{
> +	switch (pixfmt) {
> +	case V4L2_PIX_FMT_VYUY:
> +		alt->com12	= OV9640_COM12_YUV_AVG;
> +		alt->com13	= OV9640_COM13_Y_DELAY_EN |
> +					OV9640_COM13_YUV_DLY(0x01);
> +		break;
> +	case V4L2_PIX_FMT_RGB555X:
> +		alt->com7	= OV9640_COM7_RGB;
> +		alt->com13	= OV9640_COM13_RGB_AVG;
> +		alt->com15	= OV9640_COM15_RGB_555;
> +		break;
> +	case V4L2_PIX_FMT_RGB565X:
> +		alt->com7	= OV9640_COM7_RGB;
> +		alt->com13	= OV9640_COM13_RGB_AVG;
> +		alt->com15	= OV9640_COM15_RGB_565;
> +		break;
> +	};
> +}
> +
> +/* Setup registers according to resolution and color encoding */
> +static int ov9640_write_regs(struct i2c_client *client,
> +		u32 width, u32 pixfmt, struct ov9640_reg_alt *alts)
> +{
> +	const struct ov9640_reg	*ov9640_regs, *matrix_regs;
> +	int			ov9640_regs_len, matrix_regs_len;
> +	int			i, ret;
> +	u8			val;
> +
> +	/* select register configuration for given resolution */
> +	switch (width) {
> +	case W_QQCIF:
> +		ov9640_regs	= ov9640_regs_qqcif;
> +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_qqcif);
> +		break;
> +	case W_QQVGA:
> +		ov9640_regs	= ov9640_regs_qqvga;
> +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_qqvga);
> +		break;
> +	case W_QCIF:
> +		ov9640_regs	= ov9640_regs_qcif;
> +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_qcif);
> +		break;
> +	case W_QVGA:
> +		ov9640_regs	= ov9640_regs_qvga;
> +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_qvga);
> +		break;
> +	case W_CIF:
> +		ov9640_regs	= ov9640_regs_cif;
> +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_cif);
> +		break;
> +	case W_VGA:
> +		ov9640_regs	= ov9640_regs_vga;
> +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_vga);
> +		break;
> +	case W_SXGA:
> +		ov9640_regs	= ov9640_regs_sxga;
> +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_sxga);
> +		break;
> +	default:
> +		dev_err(&client->dev, "Failed to select resolution!\n");
> +		return -EINVAL;
> +	}
> +
> +	/* select color matrix configuration for given color encoding */
> +	if (pixfmt == V4L2_PIX_FMT_VYUY) {
> +		matrix_regs	= ov9640_regs_yuv;
> +		matrix_regs_len	= ARRAY_SIZE(ov9640_regs_yuv);
> +	} else {
> +		matrix_regs	= ov9640_regs_rgb;
> +		matrix_regs_len	= ARRAY_SIZE(ov9640_regs_rgb);
> +	}
> +
> +	/* write register settings into the module */
> +	for (i = 0; i < ov9640_regs_len; i++) {
> +		val = ov9640_regs[i].val;
> +
> +		switch (ov9640_regs[i].reg) {
> +		case OV9640_COM7:
> +			val |= alts->com7;
> +			break;
> +		case OV9640_COM12:
> +			val |= alts->com12;
> +			break;
> +		case OV9640_COM13:
> +			val |= alts->com13;
> +			break;
> +		case OV9640_COM15:
> +			val |= alts->com15;
> +			break;
> +		}
> +
> +		ret = ov9640_reg_write(client, ov9640_regs[i].reg, val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* write color matrix configuration into the module */
> +	for (i = 0; i < matrix_regs_len; i++) {
> +		ret = ov9640_reg_write(client, matrix_regs[i].reg,
> +						matrix_regs[i].val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/* program default register values */
> +static int ov9640_prog_dflt(struct i2c_client *client)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(ov9640_regs_dflt); i++) {
> +		ret = ov9640_reg_write(client, ov9640_regs_dflt[i].reg,
> +						ov9640_regs_dflt[i].val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* wait for the changes to actually happen */
> +	mdelay(150);

I still think that's a lot. Have you tried any lower values? I would try 
50ms, if that works, try 10ms...

> +
> +	return 0;
> +}
> +
> +/* set the format we will capture in */
> +static int ov9640_s_fmt(struct v4l2_subdev *sd, struct v4l2_format *f)
> +{
> +	struct i2c_client *client = sd->priv;
> +	struct v4l2_pix_format *pix = &f->fmt.pix;
> +	struct ov9640_reg_alt alts = {0};
> +	int ret;
> +
> +	ov9640_res_roundup(&pix->width, &pix->height);
> +	ov9640_alter_regs(pix->pixelformat, &alts);
> +
> +	ov9640_reset(client);
> +
> +	ret = ov9640_prog_dflt(client);
> +	if (ret)
> +		return ret;
> +
> +	return ov9640_write_regs(client, pix->width, pix->pixelformat, &alts);
> +}
> +
> +static int ov9640_try_fmt(struct v4l2_subdev *sd, struct v4l2_format *f)
> +{
> +	struct v4l2_pix_format *pix = &f->fmt.pix;
> +
> +	ov9640_res_roundup(&pix->width, &pix->height);
> +	pix->field  = V4L2_FIELD_NONE;
> +
> +	return 0;
> +}
> +
> +static int ov9640_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
> +{
> +	a->c.left	= 0;
> +	a->c.top	= 0;
> +	a->c.width	= W_SXGA;
> +	a->c.height	= H_SXGA;
> +	a->type		= V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +
> +	return 0;
> +}
> +
> +static int ov9640_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
> +{
> +	a->bounds.left			= 0;
> +	a->bounds.top			= 0;
> +	a->bounds.width			= W_SXGA;
> +	a->bounds.height		= H_SXGA;
> +	a->defrect			= a->bounds;
> +	a->type				= V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +	a->pixelaspect.numerator	= 1;
> +	a->pixelaspect.denominator	= 1;
> +
> +	return 0;
> +}
> +
> +
> +
> +static int ov9640_video_probe(struct soc_camera_device *icd,
> +				struct i2c_client *client)
> +{
> +	struct ov9640_priv *priv = i2c_get_clientdata(client);
> +	u8		pid, ver, midh, midl;
> +	const char	*devname;
> +	int		ret = 0;
> +
> +	/*
> +	 * We must have a parent by now. And it cannot be a wrong one.
> +	 * So this entire test is completely redundant.
> +	 */
> +	if (!icd->dev.parent ||
> +	    to_soc_camera_host(icd->dev.parent)->nr != icd->iface) {
> +		dev_err(&icd->dev, "Parent missing or invalid!\n");

Please, see 
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=d680a88e84792f84310042919065c90c5eb87423

> +		ret = -ENODEV;
> +		goto err;
> +	}
> +
> +	icd->formats		= ov9640_fmt_lists;
> +	icd->num_formats	= ARRAY_SIZE(ov9640_fmt_lists);
> +
> +	/*
> +	 * check and show product ID and manufacturer ID
> +	 */
> +
> +	ret = ov9640_reg_read(client, OV9640_PID, &pid);
> +	if (ret)
> +		goto err;
> +
> +	ret = ov9640_reg_read(client, OV9640_VER, &ver);
> +	if (ret)
> +		goto err;
> +
> +	ret = ov9640_reg_read(client, OV9640_MIDH, &midh);
> +	if (ret)
> +		goto err;
> +
> +	ret = ov9640_reg_read(client, OV9640_MIDL, &midl);
> +	if (ret)
> +		goto err;
> +
> +	switch (VERSION(pid, ver)) {
> +	case OV9640_V2:
> +		devname		= "ov9640";
> +		priv->model	= V4L2_IDENT_OV9640;
> +		priv->revision	= 2;
> +	case OV9640_V3:
> +		devname		= "ov9640";
> +		priv->model	= V4L2_IDENT_OV9640;
> +		priv->revision	= 3;
> +		break;
> +	default:
> +		dev_err(&icd->dev, "Product ID error %x:%x\n", pid, ver);
> +		ret = -ENODEV;
> +		goto err;
> +	}
> +
> +	dev_info(&icd->dev, "%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
> +		 devname, pid, ver, midh, midl);
> +
> +err:
> +	return ret;
> +}
> +
> +static struct soc_camera_ops ov9640_ops = {
> +	.set_bus_param		= ov9640_set_bus_param,
> +	.query_bus_param	= ov9640_query_bus_param,
> +	.controls		= ov9640_controls,
> +	.num_controls		= ARRAY_SIZE(ov9640_controls),
> +};
> +
> +static struct v4l2_subdev_core_ops ov9640_core_ops = {
> +	.g_ctrl			= ov9640_g_ctrl,
> +	.s_ctrl			= ov9640_s_ctrl,
> +	.g_chip_ident		= ov9640_g_chip_ident,
> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> +	.g_register		= ov9640_get_register,
> +	.s_register		= ov9640_set_register,
> +#endif
> +
> +};
> +
> +static struct v4l2_subdev_video_ops ov9640_video_ops = {
> +	.s_stream		= ov9640_s_stream,
> +	.s_fmt			= ov9640_s_fmt,
> +	.try_fmt		= ov9640_try_fmt,
> +	.cropcap		= ov9640_cropcap,
> +	.g_crop			= ov9640_g_crop,
> +
> +};
> +
> +static struct v4l2_subdev_ops ov9640_subdev_ops = {
> +	.core	= &ov9640_core_ops,
> +	.video	= &ov9640_video_ops,
> +};
> +
> +/*
> + * i2c_driver function
> + */
> +static int ov9640_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *did)
> +{
> +	struct ov9640_priv *priv;
> +	struct i2c_adapter *adapter	= to_i2c_adapter(client->dev.parent);
> +	struct soc_camera_device *icd	= client->dev.platform_data;
> +	struct soc_camera_link *icl;
> +	int ret;
> +
> +	if (!icd) {
> +		dev_err(&client->dev, "Missing soc-camera data!\n");
> +		return -EINVAL;
> +	}
> +
> +	icl = to_soc_camera_link(icd);
> +	if (!icl) {
> +		dev_err(&client->dev, "Missing platform_data for driver\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {

You're not using smbus calls, so, don't need this check. Or switch to 
using them:-)

[no more comments below]

Thanks
Guennadi

> +		dev_err(&adapter->dev,
> +			"I2C adapter doesn't support I2C_FUNC_SMBUS_BYTE\n");
> +		return -EIO;
> +	}
> +
> +	priv = kzalloc(sizeof(struct ov9640_priv), GFP_KERNEL);
> +	if (!priv) {
> +		dev_err(&client->dev,
> +			"Failed to allocate memory for private data!\n");
> +		return -ENOMEM;
> +	}
> +
> +	v4l2_i2c_subdev_init(&priv->subdev, client, &ov9640_subdev_ops);
> +
> +	icd->ops	= &ov9640_ops;
> +
> +	ret = ov9640_video_probe(icd, client);
> +
> +	if (ret) {
> +		icd->ops = NULL;
> +		i2c_set_clientdata(client, NULL);
> +		kfree(priv);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ov9640_remove(struct i2c_client *client)
> +{
> +	struct ov9640_priv *priv = i2c_get_clientdata(client);
> +
> +	i2c_set_clientdata(client, NULL);
> +	kfree(priv);
> +	return 0;
> +}
> +
> +static const struct i2c_device_id ov9640_id[] = {
> +	{ "ov9640", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ov9640_id);
> +
> +static struct i2c_driver ov9640_i2c_driver = {
> +	.driver = {
> +		.name = "ov9640",
> +	},
> +	.probe    = ov9640_probe,
> +	.remove   = ov9640_remove,
> +	.id_table = ov9640_id,
> +};
> +
> +static int __init ov9640_module_init(void)
> +{
> +	return i2c_add_driver(&ov9640_i2c_driver);
> +}
> +
> +static void __exit ov9640_module_exit(void)
> +{
> +	i2c_del_driver(&ov9640_i2c_driver);
> +}
> +
> +module_init(ov9640_module_init);
> +module_exit(ov9640_module_exit);
> +
> +MODULE_DESCRIPTION("SoC Camera driver for OmniVision OV96xx");
> +MODULE_AUTHOR("Marek Vasut <marek.vasut@gmail.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/media/video/ov9640.h b/drivers/media/video/ov9640.h
> new file mode 100644
> index 0000000..6c39f1e
> --- /dev/null
> +++ b/drivers/media/video/ov9640.h
> @@ -0,0 +1,206 @@
> +/*
> + * OmniVision OV96xx Camera Header File
> + *
> + * Copyright (C) 2009 Marek Vasut <marek.vasut@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef	__DRIVERS_MEDIA_VIDEO_OV9640_H__
> +#define	__DRIVERS_MEDIA_VIDEO_OV9640_H__
> +
> +/* Register definitions */
> +#define	OV9640_GAIN	0x00
> +#define	OV9640_BLUE	0x01
> +#define	OV9640_RED	0x02
> +#define	OV9640_VFER	0x03
> +#define	OV9640_COM1	0x04
> +#define	OV9640_BAVE	0x05
> +#define	OV9640_GEAVE	0x06
> +#define	OV9640_RSID	0x07
> +#define	OV9640_RAVE	0x08
> +#define	OV9640_COM2	0x09
> +#define	OV9640_PID	0x0a
> +#define	OV9640_VER	0x0b
> +#define	OV9640_COM3	0x0c
> +#define	OV9640_COM4	0x0d
> +#define	OV9640_COM5	0x0e
> +#define	OV9640_COM6	0x0f
> +#define	OV9640_AECH	0x10
> +#define	OV9640_CLKRC	0x11
> +#define	OV9640_COM7	0x12
> +#define	OV9640_COM8	0x13
> +#define	OV9640_COM9	0x14
> +#define	OV9640_COM10	0x15
> +/* 0x16 - RESERVED */
> +#define	OV9640_HSTART	0x17
> +#define	OV9640_HSTOP	0x18
> +#define	OV9640_VSTART	0x19
> +#define	OV9640_VSTOP	0x1a
> +#define	OV9640_PSHFT	0x1b
> +#define	OV9640_MIDH	0x1c
> +#define	OV9640_MIDL	0x1d
> +#define	OV9640_MVFP	0x1e
> +#define	OV9640_LAEC	0x1f
> +#define	OV9640_BOS	0x20
> +#define	OV9640_GBOS	0x21
> +#define	OV9640_GROS	0x22
> +#define	OV9640_ROS	0x23
> +#define	OV9640_AEW	0x24
> +#define	OV9640_AEB	0x25
> +#define	OV9640_VPT	0x26
> +#define	OV9640_BBIAS	0x27
> +#define	OV9640_GBBIAS	0x28
> +/* 0x29 - RESERVED */
> +#define	OV9640_EXHCH	0x2a
> +#define	OV9640_EXHCL	0x2b
> +#define	OV9640_RBIAS	0x2c
> +#define	OV9640_ADVFL	0x2d
> +#define	OV9640_ADVFH	0x2e
> +#define	OV9640_YAVE	0x2f
> +#define	OV9640_HSYST	0x30
> +#define	OV9640_HSYEN	0x31
> +#define	OV9640_HREF	0x32
> +#define	OV9640_CHLF	0x33
> +#define	OV9640_ARBLM	0x34
> +/* 0x35..0x36 - RESERVED */
> +#define	OV9640_ADC	0x37
> +#define	OV9640_ACOM	0x38
> +#define	OV9640_OFON	0x39
> +#define	OV9640_TSLB	0x3a
> +#define	OV9640_COM11	0x3b
> +#define	OV9640_COM12	0x3c
> +#define	OV9640_COM13	0x3d
> +#define	OV9640_COM14	0x3e
> +#define	OV9640_EDGE	0x3f
> +#define	OV9640_COM15	0x40
> +#define	OV9640_COM16	0x41
> +#define	OV9640_COM17	0x42
> +/* 0x43..0x4e - RESERVED */
> +#define	OV9640_MTX1	0x4f
> +#define	OV9640_MTX2	0x50
> +#define	OV9640_MTX3	0x51
> +#define	OV9640_MTX4	0x52
> +#define	OV9640_MTX5	0x53
> +#define	OV9640_MTX6	0x54
> +#define	OV9640_MTX7	0x55
> +#define	OV9640_MTX8	0x56
> +#define	OV9640_MTX9	0x57
> +#define	OV9640_MTXS	0x58
> +/* 0x59..0x61 - RESERVED */
> +#define	OV9640_LCC1	0x62
> +#define	OV9640_LCC2	0x63
> +#define	OV9640_LCC3	0x64
> +#define	OV9640_LCC4	0x65
> +#define	OV9640_LCC5	0x66
> +#define	OV9640_MANU	0x67
> +#define	OV9640_MANV	0x68
> +#define	OV9640_HV	0x69
> +#define	OV9640_MBD	0x6a
> +#define	OV9640_DBLV	0x6b
> +#define	OV9640_GSP	0x6c	/* ... till 0x7b */
> +#define	OV9640_GST	0x7c	/* ... till 0x8a */
> +
> +#define	OV9640_CLKRC_DPLL_EN	0x80
> +#define	OV9640_CLKRC_DIRECT	0x40
> +#define	OV9640_CLKRC_DIV(x)	((x) & 0x3f)
> +
> +#define	OV9640_PSHFT_VAL(x)	((x) & 0xff)
> +
> +#define	OV9640_ACOM_2X_ANALOG	0x80
> +#define	OV9640_ACOM_RSVD	0x12
> +
> +#define	OV9640_MVFP_V		0x10
> +#define	OV9640_MVFP_H		0x20
> +
> +#define	OV9640_COM1_HREF_NOSKIP	0x00
> +#define	OV9640_COM1_HREF_2SKIP	0x04
> +#define	OV9640_COM1_HREF_3SKIP	0x08
> +#define	OV9640_COM1_QQFMT	0x20
> +
> +#define	OV9640_COM2_SSM		0x10
> +
> +#define	OV9640_COM3_VP		0x04
> +
> +#define	OV9640_COM4_QQ_VP	0x80
> +#define	OV9640_COM4_RSVD	0x40
> +
> +#define	OV9640_COM5_SYSCLK	0x80
> +#define	OV9640_COM5_LONGEXP	0x01
> +
> +#define	OV9640_COM6_OPT_BLC	0x40
> +#define	OV9640_COM6_ADBLC_BIAS	0x08
> +#define	OV9640_COM6_FMT_RST	0x82
> +#define	OV9640_COM6_ADBLC_OPTEN	0x01
> +
> +#define	OV9640_COM7_RAW_RGB	0x01
> +#define	OV9640_COM7_RGB		0x04
> +#define	OV9640_COM7_QCIF	0x08
> +#define	OV9640_COM7_QVGA	0x10
> +#define	OV9640_COM7_CIF		0x20
> +#define	OV9640_COM7_VGA		0x40
> +#define	OV9640_COM7_SCCB_RESET	0x80
> +
> +#define	OV9640_COM12_YUV_AVG	0x04
> +#define	OV9640_COM12_RSVD	0x40
> +
> +#define	OV9640_COM13_GAMMA_NONE	0x00
> +#define	OV9640_COM13_GAMMA_Y	0x40
> +#define	OV9640_COM13_GAMMA_RAW	0x80
> +#define	OV9640_COM13_RGB_AVG	0x20
> +#define	OV9640_COM13_MATRIX_EN	0x10
> +#define	OV9640_COM13_Y_DELAY_EN	0x08
> +#define	OV9640_COM13_YUV_DLY(x)	((x) & 0x07)
> +
> +#define	OV9640_COM15_OR_00FF	0x00
> +#define	OV9640_COM15_OR_01FE	0x40
> +#define	OV9640_COM15_OR_10F0	0xc0
> +#define	OV9640_COM15_RGB_NORM	0x00
> +#define	OV9640_COM15_RGB_565	0x10
> +#define	OV9640_COM15_RGB_555	0x30
> +
> +#define	OV9640_COM16_RB_AVG	0x01
> +
> +/* IDs */
> +#define	OV9640_V2		0x9648
> +#define	OV9640_V3		0x9649
> +#define	VERSION(pid, ver)	(((pid) << 8) | ((ver) & 0xFF))
> +
> +/* supported resolutions */
> +enum {
> +	W_QQCIF	= 88,
> +	W_QQVGA	= 160,
> +	W_QCIF	= 176,
> +	W_QVGA	= 320,
> +	W_CIF	= 352,
> +	W_VGA	= 640,
> +	W_SXGA	= 1280
> +};
> +#define	H_SXGA	960
> +
> +/* Misc. structures */
> +struct ov9640_reg_alt {
> +	u8	com7;
> +	u8	com12;
> +	u8	com13;
> +	u8	com15;
> +};
> +
> +struct ov9640_reg {
> +	u8	reg;
> +	u8	val;
> +};
> +
> +struct ov9640_priv {
> +	struct v4l2_subdev		subdev;
> +
> +	int				model;
> +	int				revision;
> +
> +	bool				flag_vflip;
> +	bool				flag_hflip;
> +};
> +
> +#endif	/* __DRIVERS_MEDIA_VIDEO_OV9640_H__ */
> diff --git a/include/media/v4l2-chip-ident.h b/include/media/v4l2-chip-ident.h
> index 11a4a2d..0892e52 100644
> --- a/include/media/v4l2-chip-ident.h
> +++ b/include/media/v4l2-chip-ident.h
> @@ -60,6 +60,7 @@ enum {
>  	V4L2_IDENT_OV7670 = 250,
>  	V4L2_IDENT_OV7720 = 251,
>  	V4L2_IDENT_OV7725 = 252,
> +	V4L2_IDENT_OV9640 = 253,
>  
>  	/* module saa7146: reserved range 300-309 */
>  	V4L2_IDENT_SAA7146 = 300,
> -- 
> 1.6.3.3
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Vasut Sept. 14, 2009, 2:26 p.m. UTC | #2
Dne Pá 11. září 2009 23:51:44 Guennadi Liakhovetski napsal(a):
> On Fri, 4 Sep 2009, Marek Vasut wrote:
> > Dne Po 24. srpna 2009 20:06:29 Guennadi Liakhovetski napsal(a):
> > > Hi Marek
> > >
> > > On Sat, 22 Aug 2009, Marek Vasut wrote:
> > > > From 479aafc9a6540efec8a691a84aff166eb0218a72 Mon Sep 17 00:00:00
> > > > 2001 From: Marek Vasut <marek.vasut@gmail.com>
> > > > Date: Sat, 22 Aug 2009 05:14:23 +0200
> > > > Subject: [PATCH 2/3] Add driver for OmniVision OV9640 sensor
> > > >
> > > > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> > >
> > > Ok, I see, you rebased your patches on my soc-camera patch-stack, this
> > > is good, thanks. But you missed a couple of my comments - you still
> > > have a few static functions in the ov9640.c marked inline:
> > > ov9640_set_crop() is not used at all, ov9640_set_bus_param() gets
> > > assigned to a struct member, so, cannot be inline. ov9640_alter_regs()
> > > is indeed only called at one location, but see Chapter 15 in
> > > Documentation/CodingStyle. You left at least one multi-line comment
> > > wrongly formatted. You left some broken printk format lines, etc. You
> > > moved your header under drivers/... - that's good. But, please, address
> > > all my comments that I provided in a private review, or explain why you
> > > do not agree. Otherwise I feel like I wasted my time. Besides, your
> > > mailer has wrapped lines. Please, read
> > > Documentation/email-clients.txt on how to configure your email client
> > > to send patches properly. In the worst case, you can inline your
> > > patches while reviewing, and then attach them for a final submission.
> > > This is, however, discouraged, because it makes review and test of your
> > > intermediate patches with wrapped lines more difficult. Also, please
> > > check your patches with scripts/checkpatch.pl.
> >
> > Fixed patch follows, my mailer is fixed as much as possible (working on
> > getting git-email to work, but that's still to be done). Please consider
> > applying, thanks.
> 
> Ok, a couple more simple questions / remarks, In principle, there's just
> one principle objection - if we agree, that the correct format is BGR565
> and RGB565X, then we should change that. There's no BGR565 format
> currently in the kernel, so, we'd have to add it (and the documentation
> for the mercurial tree).
> 
> Thanks
> Guennadi
> 
> > Add driver for OmniVision OV9640 sensor
> >
> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> > ---
> >  drivers/media/video/Kconfig     |    6 +
> >  drivers/media/video/Makefile    |    1 +
> >  drivers/media/video/ov9640.c    |  811
> > +++++++++++++++++++++++++++++++++++++++ drivers/media/video/ov9640.h    |
> >  206 ++++++++++
> >  include/media/v4l2-chip-ident.h |    1 +
> >  5 files changed, 1025 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/media/video/ov9640.c
> >  create mode 100644 drivers/media/video/ov9640.h
> >
> > diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> > index 84b6fc1..fddd654 100644
> > --- a/drivers/media/video/Kconfig
> > +++ b/drivers/media/video/Kconfig
> > @@ -772,6 +772,12 @@ config SOC_CAMERA_OV772X
> >  	help
> >  	  This is a ov772x camera driver
> >
> > +config SOC_CAMERA_OV9640
> > +	tristate "ov9640 camera support"
> > +	depends on SOC_CAMERA && I2C
> > +	help
> > +	  This is a ov9640 camera driver
> > +
> >  config MX1_VIDEO
> >  	bool
> >
> > diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
> > index 9f2e321..e18efd5 100644
> > --- a/drivers/media/video/Makefile
> > +++ b/drivers/media/video/Makefile
> > @@ -76,6 +76,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9M111)	+= mt9m111.o
> >  obj-$(CONFIG_SOC_CAMERA_MT9T031)	+= mt9t031.o
> >  obj-$(CONFIG_SOC_CAMERA_MT9V022)	+= mt9v022.o
> >  obj-$(CONFIG_SOC_CAMERA_OV772X)		+= ov772x.o
> > +obj-$(CONFIG_SOC_CAMERA_OV9640)		+= ov9640.o
> >  obj-$(CONFIG_SOC_CAMERA_TW9910)		+= tw9910.o
> >
> >  # And now the v4l2 drivers:
> > diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c
> > new file mode 100644
> > index 0000000..289e82d
> > --- /dev/null
> > +++ b/drivers/media/video/ov9640.c
> > @@ -0,0 +1,811 @@
> > +/*
> > + * OmniVision OV96xx Camera Driver
> > + *
> > + * Copyright (C) 2009 Marek Vasut <marek.vasut@gmail.com>
> > + *
> > + * Based on ov772x camera driver:
> > + *
> > + * Copyright (C) 2008 Renesas Solutions Corp.
> > + * Kuninori Morimoto <morimoto.kuninori@renesas.com>
> > + *
> > + * Based on ov7670 and soc_camera_platform driver,
> > + *
> > + * Copyright 2006-7 Jonathan Corbet <corbet@lwn.net>
> > + * Copyright (C) 2008 Magnus Damm
> > + * Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/i2c.h>
> > +#include <linux/slab.h>
> > +#include <linux/delay.h>
> > +#include <linux/videodev2.h>
> > +#include <media/v4l2-chip-ident.h>
> > +#include <media/v4l2-common.h>
> > +#include <media/soc_camera.h>
> > +
> > +#include "ov9640.h"
> > +
> > +/* default register setup */
> > +static const struct ov9640_reg ov9640_regs_dflt[] = {
> > +	{ OV9640_COM5,	OV9640_COM5_SYSCLK | OV9640_COM5_LONGEXP },
> > +	{ OV9640_COM6,	OV9640_COM6_OPT_BLC | OV9640_COM6_ADBLC_BIAS |
> > +			OV9640_COM6_FMT_RST | OV9640_COM6_ADBLC_OPTEN },
> > +	{ OV9640_PSHFT,	OV9640_PSHFT_VAL(0x01) },
> > +	{ OV9640_ACOM,	OV9640_ACOM_2X_ANALOG | OV9640_ACOM_RSVD },
> > +	{ OV9640_COM16,	OV9640_COM16_RB_AVG },
> > +
> > +	/* Gamma curve P */
> > +	{ 0x6c, 0x40 },	{ 0x6d, 0x30 },	{ 0x6e, 0x4b },	{ 0x6f, 0x60 },
> > +	{ 0x70, 0x70 },	{ 0x71, 0x70 },	{ 0x72, 0x70 },	{ 0x73, 0x70 },
> > +	{ 0x74, 0x60 },	{ 0x75, 0x60 },	{ 0x76, 0x50 },	{ 0x77, 0x48 },
> > +	{ 0x78, 0x3a },	{ 0x79, 0x2e },	{ 0x7a, 0x28 },	{ 0x7b, 0x22 },
> > +
> > +	/* Gamma curve T */
> > +	{ 0x7c, 0x04 },	{ 0x7d, 0x07 },	{ 0x7e, 0x10 },	{ 0x7f, 0x28 },
> > +	{ 0x80, 0x36 },	{ 0x81, 0x44 },	{ 0x82, 0x52 },	{ 0x83, 0x60 },
> > +	{ 0x84, 0x6c },	{ 0x85, 0x78 },	{ 0x86, 0x8c },	{ 0x87, 0x9e },
> > +	{ 0x88, 0xbb },	{ 0x89, 0xd2 },	{ 0x8a, 0xe6 },
> > +};
> > +
> > +/* Configurations
> > + * NOTE: for YUV, alter the following registers:
> > + * 		COM12 |= OV9640_COM12_YUV_AVG
> > + *
> > + *	 for RGB, alter the following registers:
> > + *	 	COM7  |= OV9640_COM7_RGB
> > + *	 	COM13 |= OV9640_COM13_RGB_AVG
> > + *	 	COM15 |= proper RGB color encoding mode
> > + */
> > +static const struct ov9640_reg ov9640_regs_qqcif[] = {
> > +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x0f) },
> > +	{ OV9640_COM1,	OV9640_COM1_QQFMT | OV9640_COM1_HREF_2SKIP },
> > +	{ OV9640_COM4,	OV9640_COM4_QQ_VP | OV9640_COM4_RSVD },
> > +	{ OV9640_COM7,	OV9640_COM7_QCIF },
> > +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> > +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> > +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> > +};
> > +
> > +static const struct ov9640_reg ov9640_regs_qqvga[] = {
> > +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x07) },
> > +	{ OV9640_COM1,	OV9640_COM1_QQFMT | OV9640_COM1_HREF_2SKIP },
> > +	{ OV9640_COM4,	OV9640_COM4_QQ_VP | OV9640_COM4_RSVD },
> > +	{ OV9640_COM7,	OV9640_COM7_QVGA },
> > +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> > +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> > +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> > +};
> > +
> > +static const struct ov9640_reg ov9640_regs_qcif[] = {
> > +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x07) },
> > +	{ OV9640_COM4,	OV9640_COM4_QQ_VP | OV9640_COM4_RSVD },
> > +	{ OV9640_COM7,	OV9640_COM7_QCIF },
> > +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> > +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> > +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> > +};
> > +
> > +static const struct ov9640_reg ov9640_regs_qvga[] = {
> > +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x03) },
> > +	{ OV9640_COM4,	OV9640_COM4_QQ_VP | OV9640_COM4_RSVD },
> > +	{ OV9640_COM7,	OV9640_COM7_QVGA },
> > +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> > +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> > +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> > +};
> > +
> > +static const struct ov9640_reg ov9640_regs_cif[] = {
> > +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x03) },
> > +	{ OV9640_COM3,	OV9640_COM3_VP },
> > +	{ OV9640_COM7,	OV9640_COM7_CIF },
> > +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> > +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> > +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> > +};
> > +
> > +static const struct ov9640_reg ov9640_regs_vga[] = {
> > +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x01) },
> > +	{ OV9640_COM3,	OV9640_COM3_VP },
> > +	{ OV9640_COM7,	OV9640_COM7_VGA },
> > +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> > +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> > +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> > +};
> > +
> > +static const struct ov9640_reg ov9640_regs_sxga[] = {
> > +	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x01) },
> > +	{ OV9640_COM3,	OV9640_COM3_VP },
> > +	{ OV9640_COM7,	0 },
> > +	{ OV9640_COM12,	OV9640_COM12_RSVD },
> > +	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
> > +	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
> > +};
> > +
> > +static const struct ov9640_reg ov9640_regs_yuv[] = {
> > +	{ OV9640_MTX1,	0x50 },
> > +	{ OV9640_MTX2,	0x43 },
> > +	{ OV9640_MTX3,	0x0d },
> > +	{ OV9640_MTX4,	0x19 },
> > +	{ OV9640_MTX5,	0x4c },
> > +	{ OV9640_MTX6,	0x65 },
> > +	{ OV9640_MTX7,	0x40 },
> > +	{ OV9640_MTX8,	0x40 },
> > +	{ OV9640_MTX9,	0x40 },
> > +	{ OV9640_MTXS,	0x0f },
> > +};
> > +
> > +static const struct ov9640_reg ov9640_regs_rgb[] = {
> > +	{ OV9640_MTX1,	0x71 },
> > +	{ OV9640_MTX2,	0x3e },
> > +	{ OV9640_MTX3,	0x0c },
> > +	{ OV9640_MTX4,	0x33 },
> > +	{ OV9640_MTX5,	0x72 },
> > +	{ OV9640_MTX6,	0x00 },
> > +	{ OV9640_MTX7,	0x2b },
> > +	{ OV9640_MTX8,	0x66 },
> > +	{ OV9640_MTX9,	0xd2 },
> > +	{ OV9640_MTXS,	0x65 },
> > +};
> > +
> > +/* NOTE: The RGB555X format still has issues, so it's left out. */
> > +static const struct soc_camera_data_format ov9640_fmt_lists[] = {
> > +{
> > +	.name		= "VYUY",
> > +	.fourcc		= V4L2_PIX_FMT_VYUY,
> > +	.depth		= 16,
> > +	.colorspace	= V4L2_COLORSPACE_JPEG,
> > +},
> > +{
> > +	.name		= "RGB565X",
> > +	.fourcc		= V4L2_PIX_FMT_RGB565X,
> 
> Hm, so, do we keep RGB565X here or do we add a BGR565? We'll anyway have
> to do that when converting to imagebus, so, better do it right straight
> away, to avoid having to modify user-space apps.

I changed this, but the RGB can still possibly be broken (can be fixed later). 
The yuv works fine (and with the register tweak I did it's even more standard-
conformant).
> 
> > +	.depth		= 16,
> > +	.colorspace	= V4L2_COLORSPACE_SRGB,
> > +}
> > +};
> > +
> > +static const struct v4l2_queryctrl ov9640_controls[] = {
> > +	{
> > +		.id		= V4L2_CID_VFLIP,
> > +		.type		= V4L2_CTRL_TYPE_BOOLEAN,
> > +		.name		= "Flip Vertically",
> > +		.minimum	= 0,
> > +		.maximum	= 1,
> > +		.step		= 1,
> > +		.default_value	= 0,
> > +	},
> > +	{
> > +		.id		= V4L2_CID_HFLIP,
> > +		.type		= V4L2_CTRL_TYPE_BOOLEAN,
> > +		.name		= "Flip Horizontally",
> > +		.minimum	= 0,
> > +		.maximum	= 1,
> > +		.step		= 1,
> > +		.default_value	= 0,
> > +	},
> > +};
> > +
> > +/* read a register */
> > +static int ov9640_reg_read(struct i2c_client *client, u8 reg, u8 *val)
> > +{
> > +	int ret;
> > +	u8 data = reg;
> > +	struct i2c_msg msg = {
> > +		.addr	= client->addr,
> > +		.flags	= 0,
> > +		.len	= 1,
> > +		.buf	= &data,
> > +	};
> > +
> > +	ret = i2c_transfer(client->adapter, &msg, 1);
> 
> Are there any advantages in using raw i2c operations over smbus calls? The
> latter look much simpler, cf., e.g., mt9m001:
> 
> static int reg_read(struct i2c_client *client, const u8 reg)
> {
> 	s32 data = i2c_smbus_read_word_data(client, reg);
> 	return data < 0 ? data : swab16(data);
> }
> 
> static int reg_write(struct i2c_client *client, const u8 reg,
> 		     const u16 data)
> {
> 	return i2c_smbus_write_word_data(client, reg, swab16(data));
> }
> 
> Ok, going through smbus layer takes a bit of time, but that's just done
> during configuration.

Yes, the sensor doesnt work with SMBUS calls, that's why those are there. It 
took me a while to figure it out.

> 
> > +	if (ret < 0)
> > +		goto err;
> > +
> > +	msg.flags = I2C_M_RD;
> > +	ret = i2c_transfer(client->adapter, &msg, 1);
> > +	if (ret < 0)
> > +		goto err;
> > +
> > +	*val = data;
> > +	return 0;
> > +
> > +err:
> > +	dev_err(&client->dev, "Failed reading register 0x%02x!\n", reg);
> > +	return ret;
> > +}
> > +
> > +/* write a register */
> > +static int ov9640_reg_write(struct i2c_client *client, u8 reg, u8 val)
> > +{
> > +	int ret;
> > +	u8 _val;
> > +	unsigned char data[2] = { reg, val };
> > +	struct i2c_msg msg = {
> > +		.addr	= client->addr,
> > +		.flags	= 0,
> > +		.len	= 2,
> > +		.buf	= data,
> > +	};
> > +
> > +	ret = i2c_transfer(client->adapter, &msg, 1);
> > +	if (ret < 0) {
> > +		dev_err(&client->dev, "Failed writing register 0x%02x!\n", reg);
> > +		return ret;
> > +	}
> > +
> > +	/* we have to read the register back ... no idea why, maybe HW bug */
> > +	ret = ov9640_reg_read(client, reg, &_val);
> > +	if (ret)
> > +		dev_err(&client->dev,
> > +			"Failed reading back register 0x%02x!\n", reg);
> > +
> > +	return 0;
> > +}
> > +
> > +
> > +/* Read a register, alter its bits, write it back */
> > +static int ov9640_reg_rmw(struct i2c_client *client, u8 reg, u8 set, u8
> > unset) +{
> > +	u8 val;
> > +	int ret;
> > +
> > +	ret = ov9640_reg_read(client, reg, &val);
> > +	if (ret) {
> > +		dev_err(&client->dev,
> > +			"[Read]-Modify-Write of register %02x failed!\n", reg);
> > +		return val;
> > +	}
> > +
> > +	val |= set;
> > +	val &= ~unset;
> > +
> > +	ret = ov9640_reg_write(client, reg, val);
> > +	if (ret)
> > +		dev_err(&client->dev,
> > +			"Read-Modify-[Write] of register %02x failed!\n", reg);
> > +
> > +	return ret;
> > +}
> > +
> > +/* Soft reset the camera. This has nothing to do with the RESET pin! */
> > +static int ov9640_reset(struct i2c_client *client)
> > +{
> > +	int ret;
> > +
> > +	ret = ov9640_reg_write(client, OV9640_COM7, OV9640_COM7_SCCB_RESET);
> > +	if (ret)
> > +		dev_err(&client->dev,
> > +			"An error occured while entering soft reset!\n");
> > +
> > +	return ret;
> > +}
> > +
> > +/* Start/Stop streaming from the device */
> > +static int ov9640_s_stream(struct v4l2_subdev *sd, int enable)
> > +{
> > +	return 0;
> > +}
> > +
> > +/* Alter bus settings on camera side */
> > +static int ov9640_set_bus_param(struct soc_camera_device *icd,
> > +					unsigned long flags)
> > +{
> > +	return 0;
> > +}
> > +
> > +/* Request bus settings on camera side */
> > +static unsigned long ov9640_query_bus_param(struct soc_camera_device
> > *icd) +{
> > +	struct soc_camera_link *icl = to_soc_camera_link(icd);
> > +
> > +	/*
> > +	 * REVISIT: the camera probably can do 10 bit transfers, but I don't
> > +	 *          have those pins connected on my hardware.
> > +	 */
> > +	unsigned long flags = SOCAM_PCLK_SAMPLE_RISING | SOCAM_MASTER |
> > +		SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_HSYNC_ACTIVE_HIGH |
> > +		SOCAM_DATA_ACTIVE_HIGH | SOCAM_DATAWIDTH_8;
> > +
> > +	return soc_camera_apply_sensor_flags(icl, flags);
> > +}
> > +
> > +/* Get status of additional camera capabilities */
> > +static int ov9640_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control
> > *ctrl) +{
> > +	struct i2c_client *client = sd->priv;
> > +	struct ov9640_priv *priv = container_of(i2c_get_clientdata(client),
> > +					struct ov9640_priv, subdev);
> > +
> > +	switch (ctrl->id) {
> > +	case V4L2_CID_VFLIP:
> > +		ctrl->value = priv->flag_vflip;
> > +		break;
> > +	case V4L2_CID_HFLIP:
> > +		ctrl->value = priv->flag_hflip;
> > +		break;
> > +	}
> > +	return 0;
> > +}
> > +
> > +/* Set status of additional camera capabilities */
> > +static int ov9640_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control
> > *ctrl) +{
> > +	struct i2c_client *client = sd->priv;
> > +	struct ov9640_priv *priv = container_of(i2c_get_clientdata(client),
> > +					struct ov9640_priv, subdev);
> > +
> > +	int ret = 0;
> > +
> > +	switch (ctrl->id) {
> > +	case V4L2_CID_VFLIP:
> > +		priv->flag_vflip = ctrl->value;
> > +		if (ctrl->value)
> > +			ret = ov9640_reg_rmw(client, OV9640_MVFP,
> > +							OV9640_MVFP_V, 0);
> > +		else
> > +			ret = ov9640_reg_rmw(client, OV9640_MVFP,
> > +							0, OV9640_MVFP_V);
> > +		break;
> > +	case V4L2_CID_HFLIP:
> > +		priv->flag_hflip = ctrl->value;
> > +		if (ctrl->value)
> > +			ret = ov9640_reg_rmw(client, OV9640_MVFP,
> > +							OV9640_MVFP_H, 0);
> > +		else
> > +			ret = ov9640_reg_rmw(client, OV9640_MVFP,
> > +							0, OV9640_MVFP_H);
> > +		break;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +/* Get chip identification */
> > +static int ov9640_g_chip_ident(struct v4l2_subdev *sd,
> > +				struct v4l2_dbg_chip_ident *id)
> > +{
> > +	struct i2c_client *client = sd->priv;
> > +	struct ov9640_priv *priv = container_of(i2c_get_clientdata(client),
> > +					struct ov9640_priv, subdev);
> > +
> > +	id->ident	= priv->model;
> > +	id->revision	= priv->revision;
> > +
> > +	return 0;
> > +}
> > +
> > +#ifdef CONFIG_VIDEO_ADV_DEBUG
> > +static int ov9640_get_register(struct soc_camera_device *icd,
> > +			       struct v4l2_dbg_register *reg)
> > +{
> > +	struct i2c_client *client = to_i2c_client(to_soc_camera_control(icd));
> > +	struct ov9640_priv *priv = i2c_get_clientdata(client);
> > +
> > +	int ret;
> > +	u8 val;
> > +
> > +	if (reg->reg & ~0xff)
> > +		return -EINVAL;
> > +
> > +	reg->size = 1;
> > +
> > +	ret = ov9640_read_reg(priv->client, reg->reg, &val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	reg->val = (__u64)val;
> > +
> > +	return 0;
> > +}
> > +
> > +static int ov9640_set_register(struct soc_camera_device *icd,
> > +			       struct v4l2_dbg_register *reg)
> > +{
> > +	struct i2c_client *client = to_i2c_client(to_soc_camera_control(icd));
> > +	struct ov9640_priv *priv = i2c_get_clientdata(client);
> > +
> > +	if (reg->reg & ~0xff || reg->val & ~0xff)
> > +		return -EINVAL;
> > +
> > +	return ov9640_write_reg(priv->client, reg->reg, reg->val);
> > +}
> > +#endif
> > +
> > +/* select nearest higher resolution for capture */
> > +static void ov9640_res_roundup(u32 *width, u32 *height)
> > +{
> > +	int i;
> > +	enum { QQCIF, QQVGA, QCIF, QVGA, CIF, VGA, SXGA };
> > +	int res_x[] = { 88, 160, 176, 320, 352, 640, 1280 };
> > +	int res_y[] = { 72, 120, 144, 240, 288, 480, 960 };
> > +
> > +	for (i = 0; i < ARRAY_SIZE(res_x); i++) {
> > +		if (res_x[i] >= *width && res_y[i] >= *height) {
> > +			*width = res_x[i];
> > +			*height = res_y[i];
> > +			return;
> > +		}
> > +	}
> > +
> > +	*width = res_x[SXGA];
> > +	*height = res_y[SXGA];
> > +}
> > +
> > +/* Prepare necessary register changes depending on color encoding */
> > +static void ov9640_alter_regs(u32 pixfmt, struct ov9640_reg_alt *alt)
> > +{
> > +	switch (pixfmt) {
> > +	case V4L2_PIX_FMT_VYUY:
> > +		alt->com12	= OV9640_COM12_YUV_AVG;
> > +		alt->com13	= OV9640_COM13_Y_DELAY_EN |
> > +					OV9640_COM13_YUV_DLY(0x01);
> > +		break;
> > +	case V4L2_PIX_FMT_RGB555X:
> > +		alt->com7	= OV9640_COM7_RGB;
> > +		alt->com13	= OV9640_COM13_RGB_AVG;
> > +		alt->com15	= OV9640_COM15_RGB_555;
> > +		break;
> > +	case V4L2_PIX_FMT_RGB565X:
> > +		alt->com7	= OV9640_COM7_RGB;
> > +		alt->com13	= OV9640_COM13_RGB_AVG;
> > +		alt->com15	= OV9640_COM15_RGB_565;
> > +		break;
> > +	};
> > +}
> > +
> > +/* Setup registers according to resolution and color encoding */
> > +static int ov9640_write_regs(struct i2c_client *client,
> > +		u32 width, u32 pixfmt, struct ov9640_reg_alt *alts)
> > +{
> > +	const struct ov9640_reg	*ov9640_regs, *matrix_regs;
> > +	int			ov9640_regs_len, matrix_regs_len;
> > +	int			i, ret;
> > +	u8			val;
> > +
> > +	/* select register configuration for given resolution */
> > +	switch (width) {
> > +	case W_QQCIF:
> > +		ov9640_regs	= ov9640_regs_qqcif;
> > +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_qqcif);
> > +		break;
> > +	case W_QQVGA:
> > +		ov9640_regs	= ov9640_regs_qqvga;
> > +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_qqvga);
> > +		break;
> > +	case W_QCIF:
> > +		ov9640_regs	= ov9640_regs_qcif;
> > +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_qcif);
> > +		break;
> > +	case W_QVGA:
> > +		ov9640_regs	= ov9640_regs_qvga;
> > +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_qvga);
> > +		break;
> > +	case W_CIF:
> > +		ov9640_regs	= ov9640_regs_cif;
> > +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_cif);
> > +		break;
> > +	case W_VGA:
> > +		ov9640_regs	= ov9640_regs_vga;
> > +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_vga);
> > +		break;
> > +	case W_SXGA:
> > +		ov9640_regs	= ov9640_regs_sxga;
> > +		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_sxga);
> > +		break;
> > +	default:
> > +		dev_err(&client->dev, "Failed to select resolution!\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* select color matrix configuration for given color encoding */
> > +	if (pixfmt == V4L2_PIX_FMT_VYUY) {
> > +		matrix_regs	= ov9640_regs_yuv;
> > +		matrix_regs_len	= ARRAY_SIZE(ov9640_regs_yuv);
> > +	} else {
> > +		matrix_regs	= ov9640_regs_rgb;
> > +		matrix_regs_len	= ARRAY_SIZE(ov9640_regs_rgb);
> > +	}
> > +
> > +	/* write register settings into the module */
> > +	for (i = 0; i < ov9640_regs_len; i++) {
> > +		val = ov9640_regs[i].val;
> > +
> > +		switch (ov9640_regs[i].reg) {
> > +		case OV9640_COM7:
> > +			val |= alts->com7;
> > +			break;
> > +		case OV9640_COM12:
> > +			val |= alts->com12;
> > +			break;
> > +		case OV9640_COM13:
> > +			val |= alts->com13;
> > +			break;
> > +		case OV9640_COM15:
> > +			val |= alts->com15;
> > +			break;
> > +		}
> > +
> > +		ret = ov9640_reg_write(client, ov9640_regs[i].reg, val);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	/* write color matrix configuration into the module */
> > +	for (i = 0; i < matrix_regs_len; i++) {
> > +		ret = ov9640_reg_write(client, matrix_regs[i].reg,
> > +						matrix_regs[i].val);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/* program default register values */
> > +static int ov9640_prog_dflt(struct i2c_client *client)
> > +{
> > +	int i, ret;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(ov9640_regs_dflt); i++) {
> > +		ret = ov9640_reg_write(client, ov9640_regs_dflt[i].reg,
> > +						ov9640_regs_dflt[i].val);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	/* wait for the changes to actually happen */
> > +	mdelay(150);
> 
> I still think that's a lot. Have you tried any lower values? I would try
> 50ms, if that works, try 10ms...

You are free to try, I'm not doing anything about this. It doesnt work with 
lower values, period.
> 
> > +
> > +	return 0;
> > +}
> > +
> > +/* set the format we will capture in */
> > +static int ov9640_s_fmt(struct v4l2_subdev *sd, struct v4l2_format *f)
> > +{
> > +	struct i2c_client *client = sd->priv;
> > +	struct v4l2_pix_format *pix = &f->fmt.pix;
> > +	struct ov9640_reg_alt alts = {0};
> > +	int ret;
> > +
> > +	ov9640_res_roundup(&pix->width, &pix->height);
> > +	ov9640_alter_regs(pix->pixelformat, &alts);
> > +
> > +	ov9640_reset(client);
> > +
> > +	ret = ov9640_prog_dflt(client);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return ov9640_write_regs(client, pix->width, pix->pixelformat, &alts);
> > +}
> > +
> > +static int ov9640_try_fmt(struct v4l2_subdev *sd, struct v4l2_format *f)
> > +{
> > +	struct v4l2_pix_format *pix = &f->fmt.pix;
> > +
> > +	ov9640_res_roundup(&pix->width, &pix->height);
> > +	pix->field  = V4L2_FIELD_NONE;
> > +
> > +	return 0;
> > +}
> > +
> > +static int ov9640_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
> > +{
> > +	a->c.left	= 0;
> > +	a->c.top	= 0;
> > +	a->c.width	= W_SXGA;
> > +	a->c.height	= H_SXGA;
> > +	a->type		= V4L2_BUF_TYPE_VIDEO_CAPTURE;
> > +
> > +	return 0;
> > +}
> > +
> > +static int ov9640_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap
> > *a) +{
> > +	a->bounds.left			= 0;
> > +	a->bounds.top			= 0;
> > +	a->bounds.width			= W_SXGA;
> > +	a->bounds.height		= H_SXGA;
> > +	a->defrect			= a->bounds;
> > +	a->type				= V4L2_BUF_TYPE_VIDEO_CAPTURE;
> > +	a->pixelaspect.numerator	= 1;
> > +	a->pixelaspect.denominator	= 1;
> > +
> > +	return 0;
> > +}
> > +
> > +
> > +
> > +static int ov9640_video_probe(struct soc_camera_device *icd,
> > +				struct i2c_client *client)
> > +{
> > +	struct ov9640_priv *priv = i2c_get_clientdata(client);
> > +	u8		pid, ver, midh, midl;
> > +	const char	*devname;
> > +	int		ret = 0;
> > +
> > +	/*
> > +	 * We must have a parent by now. And it cannot be a wrong one.
> > +	 * So this entire test is completely redundant.
> > +	 */
> > +	if (!icd->dev.parent ||
> > +	    to_soc_camera_host(icd->dev.parent)->nr != icd->iface) {
> > +		dev_err(&icd->dev, "Parent missing or invalid!\n");
> 
> Please, see
> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;
> h=d680a88e84792f84310042919065c90c5eb87423
> 
> > +		ret = -ENODEV;
> > +		goto err;
> > +	}
> > +
> > +	icd->formats		= ov9640_fmt_lists;
> > +	icd->num_formats	= ARRAY_SIZE(ov9640_fmt_lists);
> > +
> > +	/*
> > +	 * check and show product ID and manufacturer ID
> > +	 */
> > +
> > +	ret = ov9640_reg_read(client, OV9640_PID, &pid);
> > +	if (ret)
> > +		goto err;
> > +
> > +	ret = ov9640_reg_read(client, OV9640_VER, &ver);
> > +	if (ret)
> > +		goto err;
> > +
> > +	ret = ov9640_reg_read(client, OV9640_MIDH, &midh);
> > +	if (ret)
> > +		goto err;
> > +
> > +	ret = ov9640_reg_read(client, OV9640_MIDL, &midl);
> > +	if (ret)
> > +		goto err;
> > +
> > +	switch (VERSION(pid, ver)) {
> > +	case OV9640_V2:
> > +		devname		= "ov9640";
> > +		priv->model	= V4L2_IDENT_OV9640;
> > +		priv->revision	= 2;
> > +	case OV9640_V3:
> > +		devname		= "ov9640";
> > +		priv->model	= V4L2_IDENT_OV9640;
> > +		priv->revision	= 3;
> > +		break;
> > +	default:
> > +		dev_err(&icd->dev, "Product ID error %x:%x\n", pid, ver);
> > +		ret = -ENODEV;
> > +		goto err;
> > +	}
> > +
> > +	dev_info(&icd->dev, "%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
> > +		 devname, pid, ver, midh, midl);
> > +
> > +err:
> > +	return ret;
> > +}
> > +
> > +static struct soc_camera_ops ov9640_ops = {
> > +	.set_bus_param		= ov9640_set_bus_param,
> > +	.query_bus_param	= ov9640_query_bus_param,
> > +	.controls		= ov9640_controls,
> > +	.num_controls		= ARRAY_SIZE(ov9640_controls),
> > +};
> > +
> > +static struct v4l2_subdev_core_ops ov9640_core_ops = {
> > +	.g_ctrl			= ov9640_g_ctrl,
> > +	.s_ctrl			= ov9640_s_ctrl,
> > +	.g_chip_ident		= ov9640_g_chip_ident,
> > +#ifdef CONFIG_VIDEO_ADV_DEBUG
> > +	.g_register		= ov9640_get_register,
> > +	.s_register		= ov9640_set_register,
> > +#endif
> > +
> > +};
> > +
> > +static struct v4l2_subdev_video_ops ov9640_video_ops = {
> > +	.s_stream		= ov9640_s_stream,
> > +	.s_fmt			= ov9640_s_fmt,
> > +	.try_fmt		= ov9640_try_fmt,
> > +	.cropcap		= ov9640_cropcap,
> > +	.g_crop			= ov9640_g_crop,
> > +
> > +};
> > +
> > +static struct v4l2_subdev_ops ov9640_subdev_ops = {
> > +	.core	= &ov9640_core_ops,
> > +	.video	= &ov9640_video_ops,
> > +};
> > +
> > +/*
> > + * i2c_driver function
> > + */
> > +static int ov9640_probe(struct i2c_client *client,
> > +			 const struct i2c_device_id *did)
> > +{
> > +	struct ov9640_priv *priv;
> > +	struct i2c_adapter *adapter	= to_i2c_adapter(client->dev.parent);
> > +	struct soc_camera_device *icd	= client->dev.platform_data;
> > +	struct soc_camera_link *icl;
> > +	int ret;
> > +
> > +	if (!icd) {
> > +		dev_err(&client->dev, "Missing soc-camera data!\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	icl = to_soc_camera_link(icd);
> > +	if (!icl) {
> > +		dev_err(&client->dev, "Missing platform_data for driver\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
> 
> You're not using smbus calls, so, don't need this check. Or switch to
> using them:-)
> 
> [no more comments below]
> 
> Thanks
> Guennadi
> 
> > +		dev_err(&adapter->dev,
> > +			"I2C adapter doesn't support I2C_FUNC_SMBUS_BYTE\n");
> > +		return -EIO;
> > +	}
> > +
> > +	priv = kzalloc(sizeof(struct ov9640_priv), GFP_KERNEL);
> > +	if (!priv) {
> > +		dev_err(&client->dev,
> > +			"Failed to allocate memory for private data!\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	v4l2_i2c_subdev_init(&priv->subdev, client, &ov9640_subdev_ops);
> > +
> > +	icd->ops	= &ov9640_ops;
> > +
> > +	ret = ov9640_video_probe(icd, client);
> > +
> > +	if (ret) {
> > +		icd->ops = NULL;
> > +		i2c_set_clientdata(client, NULL);
> > +		kfree(priv);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int ov9640_remove(struct i2c_client *client)
> > +{
> > +	struct ov9640_priv *priv = i2c_get_clientdata(client);
> > +
> > +	i2c_set_clientdata(client, NULL);
> > +	kfree(priv);
> > +	return 0;
> > +}
> > +
> > +static const struct i2c_device_id ov9640_id[] = {
> > +	{ "ov9640", 0 },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, ov9640_id);
> > +
> > +static struct i2c_driver ov9640_i2c_driver = {
> > +	.driver = {
> > +		.name = "ov9640",
> > +	},
> > +	.probe    = ov9640_probe,
> > +	.remove   = ov9640_remove,
> > +	.id_table = ov9640_id,
> > +};
> > +
> > +static int __init ov9640_module_init(void)
> > +{
> > +	return i2c_add_driver(&ov9640_i2c_driver);
> > +}
> > +
> > +static void __exit ov9640_module_exit(void)
> > +{
> > +	i2c_del_driver(&ov9640_i2c_driver);
> > +}
> > +
> > +module_init(ov9640_module_init);
> > +module_exit(ov9640_module_exit);
> > +
> > +MODULE_DESCRIPTION("SoC Camera driver for OmniVision OV96xx");
> > +MODULE_AUTHOR("Marek Vasut <marek.vasut@gmail.com>");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/media/video/ov9640.h b/drivers/media/video/ov9640.h
> > new file mode 100644
> > index 0000000..6c39f1e
> > --- /dev/null
> > +++ b/drivers/media/video/ov9640.h
> > @@ -0,0 +1,206 @@
> > +/*
> > + * OmniVision OV96xx Camera Header File
> > + *
> > + * Copyright (C) 2009 Marek Vasut <marek.vasut@gmail.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef	__DRIVERS_MEDIA_VIDEO_OV9640_H__
> > +#define	__DRIVERS_MEDIA_VIDEO_OV9640_H__
> > +
> > +/* Register definitions */
> > +#define	OV9640_GAIN	0x00
> > +#define	OV9640_BLUE	0x01
> > +#define	OV9640_RED	0x02
> > +#define	OV9640_VFER	0x03
> > +#define	OV9640_COM1	0x04
> > +#define	OV9640_BAVE	0x05
> > +#define	OV9640_GEAVE	0x06
> > +#define	OV9640_RSID	0x07
> > +#define	OV9640_RAVE	0x08
> > +#define	OV9640_COM2	0x09
> > +#define	OV9640_PID	0x0a
> > +#define	OV9640_VER	0x0b
> > +#define	OV9640_COM3	0x0c
> > +#define	OV9640_COM4	0x0d
> > +#define	OV9640_COM5	0x0e
> > +#define	OV9640_COM6	0x0f
> > +#define	OV9640_AECH	0x10
> > +#define	OV9640_CLKRC	0x11
> > +#define	OV9640_COM7	0x12
> > +#define	OV9640_COM8	0x13
> > +#define	OV9640_COM9	0x14
> > +#define	OV9640_COM10	0x15
> > +/* 0x16 - RESERVED */
> > +#define	OV9640_HSTART	0x17
> > +#define	OV9640_HSTOP	0x18
> > +#define	OV9640_VSTART	0x19
> > +#define	OV9640_VSTOP	0x1a
> > +#define	OV9640_PSHFT	0x1b
> > +#define	OV9640_MIDH	0x1c
> > +#define	OV9640_MIDL	0x1d
> > +#define	OV9640_MVFP	0x1e
> > +#define	OV9640_LAEC	0x1f
> > +#define	OV9640_BOS	0x20
> > +#define	OV9640_GBOS	0x21
> > +#define	OV9640_GROS	0x22
> > +#define	OV9640_ROS	0x23
> > +#define	OV9640_AEW	0x24
> > +#define	OV9640_AEB	0x25
> > +#define	OV9640_VPT	0x26
> > +#define	OV9640_BBIAS	0x27
> > +#define	OV9640_GBBIAS	0x28
> > +/* 0x29 - RESERVED */
> > +#define	OV9640_EXHCH	0x2a
> > +#define	OV9640_EXHCL	0x2b
> > +#define	OV9640_RBIAS	0x2c
> > +#define	OV9640_ADVFL	0x2d
> > +#define	OV9640_ADVFH	0x2e
> > +#define	OV9640_YAVE	0x2f
> > +#define	OV9640_HSYST	0x30
> > +#define	OV9640_HSYEN	0x31
> > +#define	OV9640_HREF	0x32
> > +#define	OV9640_CHLF	0x33
> > +#define	OV9640_ARBLM	0x34
> > +/* 0x35..0x36 - RESERVED */
> > +#define	OV9640_ADC	0x37
> > +#define	OV9640_ACOM	0x38
> > +#define	OV9640_OFON	0x39
> > +#define	OV9640_TSLB	0x3a
> > +#define	OV9640_COM11	0x3b
> > +#define	OV9640_COM12	0x3c
> > +#define	OV9640_COM13	0x3d
> > +#define	OV9640_COM14	0x3e
> > +#define	OV9640_EDGE	0x3f
> > +#define	OV9640_COM15	0x40
> > +#define	OV9640_COM16	0x41
> > +#define	OV9640_COM17	0x42
> > +/* 0x43..0x4e - RESERVED */
> > +#define	OV9640_MTX1	0x4f
> > +#define	OV9640_MTX2	0x50
> > +#define	OV9640_MTX3	0x51
> > +#define	OV9640_MTX4	0x52
> > +#define	OV9640_MTX5	0x53
> > +#define	OV9640_MTX6	0x54
> > +#define	OV9640_MTX7	0x55
> > +#define	OV9640_MTX8	0x56
> > +#define	OV9640_MTX9	0x57
> > +#define	OV9640_MTXS	0x58
> > +/* 0x59..0x61 - RESERVED */
> > +#define	OV9640_LCC1	0x62
> > +#define	OV9640_LCC2	0x63
> > +#define	OV9640_LCC3	0x64
> > +#define	OV9640_LCC4	0x65
> > +#define	OV9640_LCC5	0x66
> > +#define	OV9640_MANU	0x67
> > +#define	OV9640_MANV	0x68
> > +#define	OV9640_HV	0x69
> > +#define	OV9640_MBD	0x6a
> > +#define	OV9640_DBLV	0x6b
> > +#define	OV9640_GSP	0x6c	/* ... till 0x7b */
> > +#define	OV9640_GST	0x7c	/* ... till 0x8a */
> > +
> > +#define	OV9640_CLKRC_DPLL_EN	0x80
> > +#define	OV9640_CLKRC_DIRECT	0x40
> > +#define	OV9640_CLKRC_DIV(x)	((x) & 0x3f)
> > +
> > +#define	OV9640_PSHFT_VAL(x)	((x) & 0xff)
> > +
> > +#define	OV9640_ACOM_2X_ANALOG	0x80
> > +#define	OV9640_ACOM_RSVD	0x12
> > +
> > +#define	OV9640_MVFP_V		0x10
> > +#define	OV9640_MVFP_H		0x20
> > +
> > +#define	OV9640_COM1_HREF_NOSKIP	0x00
> > +#define	OV9640_COM1_HREF_2SKIP	0x04
> > +#define	OV9640_COM1_HREF_3SKIP	0x08
> > +#define	OV9640_COM1_QQFMT	0x20
> > +
> > +#define	OV9640_COM2_SSM		0x10
> > +
> > +#define	OV9640_COM3_VP		0x04
> > +
> > +#define	OV9640_COM4_QQ_VP	0x80
> > +#define	OV9640_COM4_RSVD	0x40
> > +
> > +#define	OV9640_COM5_SYSCLK	0x80
> > +#define	OV9640_COM5_LONGEXP	0x01
> > +
> > +#define	OV9640_COM6_OPT_BLC	0x40
> > +#define	OV9640_COM6_ADBLC_BIAS	0x08
> > +#define	OV9640_COM6_FMT_RST	0x82
> > +#define	OV9640_COM6_ADBLC_OPTEN	0x01
> > +
> > +#define	OV9640_COM7_RAW_RGB	0x01
> > +#define	OV9640_COM7_RGB		0x04
> > +#define	OV9640_COM7_QCIF	0x08
> > +#define	OV9640_COM7_QVGA	0x10
> > +#define	OV9640_COM7_CIF		0x20
> > +#define	OV9640_COM7_VGA		0x40
> > +#define	OV9640_COM7_SCCB_RESET	0x80
> > +
> > +#define	OV9640_COM12_YUV_AVG	0x04
> > +#define	OV9640_COM12_RSVD	0x40
> > +
> > +#define	OV9640_COM13_GAMMA_NONE	0x00
> > +#define	OV9640_COM13_GAMMA_Y	0x40
> > +#define	OV9640_COM13_GAMMA_RAW	0x80
> > +#define	OV9640_COM13_RGB_AVG	0x20
> > +#define	OV9640_COM13_MATRIX_EN	0x10
> > +#define	OV9640_COM13_Y_DELAY_EN	0x08
> > +#define	OV9640_COM13_YUV_DLY(x)	((x) & 0x07)
> > +
> > +#define	OV9640_COM15_OR_00FF	0x00
> > +#define	OV9640_COM15_OR_01FE	0x40
> > +#define	OV9640_COM15_OR_10F0	0xc0
> > +#define	OV9640_COM15_RGB_NORM	0x00
> > +#define	OV9640_COM15_RGB_565	0x10
> > +#define	OV9640_COM15_RGB_555	0x30
> > +
> > +#define	OV9640_COM16_RB_AVG	0x01
> > +
> > +/* IDs */
> > +#define	OV9640_V2		0x9648
> > +#define	OV9640_V3		0x9649
> > +#define	VERSION(pid, ver)	(((pid) << 8) | ((ver) & 0xFF))
> > +
> > +/* supported resolutions */
> > +enum {
> > +	W_QQCIF	= 88,
> > +	W_QQVGA	= 160,
> > +	W_QCIF	= 176,
> > +	W_QVGA	= 320,
> > +	W_CIF	= 352,
> > +	W_VGA	= 640,
> > +	W_SXGA	= 1280
> > +};
> > +#define	H_SXGA	960
> > +
> > +/* Misc. structures */
> > +struct ov9640_reg_alt {
> > +	u8	com7;
> > +	u8	com12;
> > +	u8	com13;
> > +	u8	com15;
> > +};
> > +
> > +struct ov9640_reg {
> > +	u8	reg;
> > +	u8	val;
> > +};
> > +
> > +struct ov9640_priv {
> > +	struct v4l2_subdev		subdev;
> > +
> > +	int				model;
> > +	int				revision;
> > +
> > +	bool				flag_vflip;
> > +	bool				flag_hflip;
> > +};
> > +
> > +#endif	/* __DRIVERS_MEDIA_VIDEO_OV9640_H__ */
> > diff --git a/include/media/v4l2-chip-ident.h
> > b/include/media/v4l2-chip-ident.h index 11a4a2d..0892e52 100644
> > --- a/include/media/v4l2-chip-ident.h
> > +++ b/include/media/v4l2-chip-ident.h
> > @@ -60,6 +60,7 @@ enum {
> >  	V4L2_IDENT_OV7670 = 250,
> >  	V4L2_IDENT_OV7720 = 251,
> >  	V4L2_IDENT_OV7725 = 252,
> > +	V4L2_IDENT_OV9640 = 253,
> >
> >  	/* module saa7146: reserved range 300-309 */
> >  	V4L2_IDENT_SAA7146 = 300,
> 
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guennadi Liakhovetski Sept. 14, 2009, 2:41 p.m. UTC | #3
On Mon, 14 Sep 2009, Marek Vasut wrote:

[snip]

> > > +/* NOTE: The RGB555X format still has issues, so it's left out. */
> > > +static const struct soc_camera_data_format ov9640_fmt_lists[] = {
> > > +{
> > > +	.name		= "VYUY",
> > > +	.fourcc		= V4L2_PIX_FMT_VYUY,
> > > +	.depth		= 16,
> > > +	.colorspace	= V4L2_COLORSPACE_JPEG,
> > > +},
> > > +{
> > > +	.name		= "RGB565X",
> > > +	.fourcc		= V4L2_PIX_FMT_RGB565X,
> > 
> > Hm, so, do we keep RGB565X here or do we add a BGR565? We'll anyway have
> > to do that when converting to imagebus, so, better do it right straight
> > away, to avoid having to modify user-space apps.
> 
> I changed this, but the RGB can still possibly be broken (can be fixed later). 

Well, no, sorry, I do not think it's a good idea to commit code, that we 
believe does not work. Then, please, remove RGB codes and add a TODO comment.

> The yuv works fine (and with the register tweak I did it's even more standard-
> conformant).

Ok.

> > > +/* read a register */
> > > +static int ov9640_reg_read(struct i2c_client *client, u8 reg, u8 *val)
> > > +{
> > > +	int ret;
> > > +	u8 data = reg;
> > > +	struct i2c_msg msg = {
> > > +		.addr	= client->addr,
> > > +		.flags	= 0,
> > > +		.len	= 1,
> > > +		.buf	= &data,
> > > +	};
> > > +
> > > +	ret = i2c_transfer(client->adapter, &msg, 1);
> > 
> > Are there any advantages in using raw i2c operations over smbus calls? The
> > latter look much simpler, cf., e.g., mt9m001:
> > 
> > static int reg_read(struct i2c_client *client, const u8 reg)
> > {
> > 	s32 data = i2c_smbus_read_word_data(client, reg);
> > 	return data < 0 ? data : swab16(data);
> > }
> > 
> > static int reg_write(struct i2c_client *client, const u8 reg,
> > 		     const u16 data)
> > {
> > 	return i2c_smbus_write_word_data(client, reg, swab16(data));
> > }
> > 
> > Ok, going through smbus layer takes a bit of time, but that's just done
> > during configuration.
> 
> Yes, the sensor doesnt work with SMBUS calls, that's why those are there. It 
> took me a while to figure it out.

Ok, then we should remove the check for SMBUS in probing.

> > > +/* program default register values */
> > > +static int ov9640_prog_dflt(struct i2c_client *client)
> > > +{
> > > +	int i, ret;
> > > +
> > > +	for (i = 0; i < ARRAY_SIZE(ov9640_regs_dflt); i++) {
> > > +		ret = ov9640_reg_write(client, ov9640_regs_dflt[i].reg,
> > > +						ov9640_regs_dflt[i].val);
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > > +
> > > +	/* wait for the changes to actually happen */
> > > +	mdelay(150);
> > 
> > I still think that's a lot. Have you tried any lower values? I would try
> > 50ms, if that works, try 10ms...
> 
> You are free to try, I'm not doing anything about this. It doesnt work with 
> lower values, period.

So, you did try it? Ok, then at least add a comment specifying whichlargest 
value didn't work for you.

> > > +	/*
> > > +	 * We must have a parent by now. And it cannot be a wrong one.
> > > +	 * So this entire test is completely redundant.
> > > +	 */
> > > +	if (!icd->dev.parent ||
> > > +	    to_soc_camera_host(icd->dev.parent)->nr != icd->iface) {
> > > +		dev_err(&icd->dev, "Parent missing or invalid!\n");
> > 
> > Please, see
> > http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;
> > h=d680a88e84792f84310042919065c90c5eb87423

Please adjust your dev_{dbg,err,warn,info} calls according to that patch.

> > > +/*
> > > + * i2c_driver function
> > > + */
> > > +static int ov9640_probe(struct i2c_client *client,
> > > +			 const struct i2c_device_id *did)
> > > +{
> > > +	struct ov9640_priv *priv;
> > > +	struct i2c_adapter *adapter	= to_i2c_adapter(client->dev.parent);
> > > +	struct soc_camera_device *icd	= client->dev.platform_data;
> > > +	struct soc_camera_link *icl;
> > > +	int ret;
> > > +
> > > +	if (!icd) {
> > > +		dev_err(&client->dev, "Missing soc-camera data!\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	icl = to_soc_camera_link(icd);
> > > +	if (!icl) {
> > > +		dev_err(&client->dev, "Missing platform_data for driver\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
> > 
> > You're not using smbus calls, so, don't need this check. Or switch to
> > using them:-)

And remove this.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 84b6fc1..fddd654 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -772,6 +772,12 @@  config SOC_CAMERA_OV772X
 	help
 	  This is a ov772x camera driver
 
+config SOC_CAMERA_OV9640
+	tristate "ov9640 camera support"
+	depends on SOC_CAMERA && I2C
+	help
+	  This is a ov9640 camera driver
+
 config MX1_VIDEO
 	bool
 
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index 9f2e321..e18efd5 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -76,6 +76,7 @@  obj-$(CONFIG_SOC_CAMERA_MT9M111)	+= mt9m111.o
 obj-$(CONFIG_SOC_CAMERA_MT9T031)	+= mt9t031.o
 obj-$(CONFIG_SOC_CAMERA_MT9V022)	+= mt9v022.o
 obj-$(CONFIG_SOC_CAMERA_OV772X)		+= ov772x.o
+obj-$(CONFIG_SOC_CAMERA_OV9640)		+= ov9640.o
 obj-$(CONFIG_SOC_CAMERA_TW9910)		+= tw9910.o
 
 # And now the v4l2 drivers:
diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c
new file mode 100644
index 0000000..289e82d
--- /dev/null
+++ b/drivers/media/video/ov9640.c
@@ -0,0 +1,811 @@ 
+/*
+ * OmniVision OV96xx Camera Driver
+ *
+ * Copyright (C) 2009 Marek Vasut <marek.vasut@gmail.com>
+ *
+ * Based on ov772x camera driver:
+ *
+ * Copyright (C) 2008 Renesas Solutions Corp.
+ * Kuninori Morimoto <morimoto.kuninori@renesas.com>
+ *
+ * Based on ov7670 and soc_camera_platform driver,
+ *
+ * Copyright 2006-7 Jonathan Corbet <corbet@lwn.net>
+ * Copyright (C) 2008 Magnus Damm
+ * Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/videodev2.h>
+#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-common.h>
+#include <media/soc_camera.h>
+
+#include "ov9640.h"
+
+/* default register setup */
+static const struct ov9640_reg ov9640_regs_dflt[] = {
+	{ OV9640_COM5,	OV9640_COM5_SYSCLK | OV9640_COM5_LONGEXP },
+	{ OV9640_COM6,	OV9640_COM6_OPT_BLC | OV9640_COM6_ADBLC_BIAS |
+			OV9640_COM6_FMT_RST | OV9640_COM6_ADBLC_OPTEN },
+	{ OV9640_PSHFT,	OV9640_PSHFT_VAL(0x01) },
+	{ OV9640_ACOM,	OV9640_ACOM_2X_ANALOG | OV9640_ACOM_RSVD },
+	{ OV9640_COM16,	OV9640_COM16_RB_AVG },
+
+	/* Gamma curve P */
+	{ 0x6c, 0x40 },	{ 0x6d, 0x30 },	{ 0x6e, 0x4b },	{ 0x6f, 0x60 },
+	{ 0x70, 0x70 },	{ 0x71, 0x70 },	{ 0x72, 0x70 },	{ 0x73, 0x70 },
+	{ 0x74, 0x60 },	{ 0x75, 0x60 },	{ 0x76, 0x50 },	{ 0x77, 0x48 },
+	{ 0x78, 0x3a },	{ 0x79, 0x2e },	{ 0x7a, 0x28 },	{ 0x7b, 0x22 },
+
+	/* Gamma curve T */
+	{ 0x7c, 0x04 },	{ 0x7d, 0x07 },	{ 0x7e, 0x10 },	{ 0x7f, 0x28 },
+	{ 0x80, 0x36 },	{ 0x81, 0x44 },	{ 0x82, 0x52 },	{ 0x83, 0x60 },
+	{ 0x84, 0x6c },	{ 0x85, 0x78 },	{ 0x86, 0x8c },	{ 0x87, 0x9e },
+	{ 0x88, 0xbb },	{ 0x89, 0xd2 },	{ 0x8a, 0xe6 },
+};
+
+/* Configurations
+ * NOTE: for YUV, alter the following registers:
+ * 		COM12 |= OV9640_COM12_YUV_AVG
+ *
+ *	 for RGB, alter the following registers:
+ *	 	COM7  |= OV9640_COM7_RGB
+ *	 	COM13 |= OV9640_COM13_RGB_AVG
+ *	 	COM15 |= proper RGB color encoding mode
+ */
+static const struct ov9640_reg ov9640_regs_qqcif[] = {
+	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x0f) },
+	{ OV9640_COM1,	OV9640_COM1_QQFMT | OV9640_COM1_HREF_2SKIP },
+	{ OV9640_COM4,	OV9640_COM4_QQ_VP | OV9640_COM4_RSVD },
+	{ OV9640_COM7,	OV9640_COM7_QCIF },
+	{ OV9640_COM12,	OV9640_COM12_RSVD },
+	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
+	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
+};
+
+static const struct ov9640_reg ov9640_regs_qqvga[] = {
+	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x07) },
+	{ OV9640_COM1,	OV9640_COM1_QQFMT | OV9640_COM1_HREF_2SKIP },
+	{ OV9640_COM4,	OV9640_COM4_QQ_VP | OV9640_COM4_RSVD },
+	{ OV9640_COM7,	OV9640_COM7_QVGA },
+	{ OV9640_COM12,	OV9640_COM12_RSVD },
+	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
+	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
+};
+
+static const struct ov9640_reg ov9640_regs_qcif[] = {
+	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x07) },
+	{ OV9640_COM4,	OV9640_COM4_QQ_VP | OV9640_COM4_RSVD },
+	{ OV9640_COM7,	OV9640_COM7_QCIF },
+	{ OV9640_COM12,	OV9640_COM12_RSVD },
+	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
+	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
+};
+
+static const struct ov9640_reg ov9640_regs_qvga[] = {
+	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x03) },
+	{ OV9640_COM4,	OV9640_COM4_QQ_VP | OV9640_COM4_RSVD },
+	{ OV9640_COM7,	OV9640_COM7_QVGA },
+	{ OV9640_COM12,	OV9640_COM12_RSVD },
+	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
+	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
+};
+
+static const struct ov9640_reg ov9640_regs_cif[] = {
+	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x03) },
+	{ OV9640_COM3,	OV9640_COM3_VP },
+	{ OV9640_COM7,	OV9640_COM7_CIF },
+	{ OV9640_COM12,	OV9640_COM12_RSVD },
+	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
+	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
+};
+
+static const struct ov9640_reg ov9640_regs_vga[] = {
+	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x01) },
+	{ OV9640_COM3,	OV9640_COM3_VP },
+	{ OV9640_COM7,	OV9640_COM7_VGA },
+	{ OV9640_COM12,	OV9640_COM12_RSVD },
+	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
+	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
+};
+
+static const struct ov9640_reg ov9640_regs_sxga[] = {
+	{ OV9640_CLKRC,	OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x01) },
+	{ OV9640_COM3,	OV9640_COM3_VP },
+	{ OV9640_COM7,	0 },
+	{ OV9640_COM12,	OV9640_COM12_RSVD },
+	{ OV9640_COM13,	OV9640_COM13_GAMMA_RAW | OV9640_COM13_MATRIX_EN },
+	{ OV9640_COM15,	OV9640_COM15_OR_10F0 },
+};
+
+static const struct ov9640_reg ov9640_regs_yuv[] = {
+	{ OV9640_MTX1,	0x50 },
+	{ OV9640_MTX2,	0x43 },
+	{ OV9640_MTX3,	0x0d },
+	{ OV9640_MTX4,	0x19 },
+	{ OV9640_MTX5,	0x4c },
+	{ OV9640_MTX6,	0x65 },
+	{ OV9640_MTX7,	0x40 },
+	{ OV9640_MTX8,	0x40 },
+	{ OV9640_MTX9,	0x40 },
+	{ OV9640_MTXS,	0x0f },
+};
+
+static const struct ov9640_reg ov9640_regs_rgb[] = {
+	{ OV9640_MTX1,	0x71 },
+	{ OV9640_MTX2,	0x3e },
+	{ OV9640_MTX3,	0x0c },
+	{ OV9640_MTX4,	0x33 },
+	{ OV9640_MTX5,	0x72 },
+	{ OV9640_MTX6,	0x00 },
+	{ OV9640_MTX7,	0x2b },
+	{ OV9640_MTX8,	0x66 },
+	{ OV9640_MTX9,	0xd2 },
+	{ OV9640_MTXS,	0x65 },
+};
+
+/* NOTE: The RGB555X format still has issues, so it's left out. */
+static const struct soc_camera_data_format ov9640_fmt_lists[] = {
+{
+	.name		= "VYUY",
+	.fourcc		= V4L2_PIX_FMT_VYUY,
+	.depth		= 16,
+	.colorspace	= V4L2_COLORSPACE_JPEG,
+},
+{
+	.name		= "RGB565X",
+	.fourcc		= V4L2_PIX_FMT_RGB565X,
+	.depth		= 16,
+	.colorspace	= V4L2_COLORSPACE_SRGB,
+}
+};
+
+static const struct v4l2_queryctrl ov9640_controls[] = {
+	{
+		.id		= V4L2_CID_VFLIP,
+		.type		= V4L2_CTRL_TYPE_BOOLEAN,
+		.name		= "Flip Vertically",
+		.minimum	= 0,
+		.maximum	= 1,
+		.step		= 1,
+		.default_value	= 0,
+	},
+	{
+		.id		= V4L2_CID_HFLIP,
+		.type		= V4L2_CTRL_TYPE_BOOLEAN,
+		.name		= "Flip Horizontally",
+		.minimum	= 0,
+		.maximum	= 1,
+		.step		= 1,
+		.default_value	= 0,
+	},
+};
+
+/* read a register */
+static int ov9640_reg_read(struct i2c_client *client, u8 reg, u8 *val)
+{
+	int ret;
+	u8 data = reg;
+	struct i2c_msg msg = {
+		.addr	= client->addr,
+		.flags	= 0,
+		.len	= 1,
+		.buf	= &data,
+	};
+
+	ret = i2c_transfer(client->adapter, &msg, 1);
+	if (ret < 0)
+		goto err;
+
+	msg.flags = I2C_M_RD;
+	ret = i2c_transfer(client->adapter, &msg, 1);
+	if (ret < 0)
+		goto err;
+
+	*val = data;
+	return 0;
+
+err:
+	dev_err(&client->dev, "Failed reading register 0x%02x!\n", reg);
+	return ret;
+}
+
+/* write a register */
+static int ov9640_reg_write(struct i2c_client *client, u8 reg, u8 val)
+{
+	int ret;
+	u8 _val;
+	unsigned char data[2] = { reg, val };
+	struct i2c_msg msg = {
+		.addr	= client->addr,
+		.flags	= 0,
+		.len	= 2,
+		.buf	= data,
+	};
+
+	ret = i2c_transfer(client->adapter, &msg, 1);
+	if (ret < 0) {
+		dev_err(&client->dev, "Failed writing register 0x%02x!\n", reg);
+		return ret;
+	}
+
+	/* we have to read the register back ... no idea why, maybe HW bug */
+	ret = ov9640_reg_read(client, reg, &_val);
+	if (ret)
+		dev_err(&client->dev,
+			"Failed reading back register 0x%02x!\n", reg);
+
+	return 0;
+}
+
+
+/* Read a register, alter its bits, write it back */
+static int ov9640_reg_rmw(struct i2c_client *client, u8 reg, u8 set, u8 unset)
+{
+	u8 val;
+	int ret;
+
+	ret = ov9640_reg_read(client, reg, &val);
+	if (ret) {
+		dev_err(&client->dev,
+			"[Read]-Modify-Write of register %02x failed!\n", reg);
+		return val;
+	}
+
+	val |= set;
+	val &= ~unset;
+
+	ret = ov9640_reg_write(client, reg, val);
+	if (ret)
+		dev_err(&client->dev,
+			"Read-Modify-[Write] of register %02x failed!\n", reg);
+
+	return ret;
+}
+
+/* Soft reset the camera. This has nothing to do with the RESET pin! */
+static int ov9640_reset(struct i2c_client *client)
+{
+	int ret;
+
+	ret = ov9640_reg_write(client, OV9640_COM7, OV9640_COM7_SCCB_RESET);
+	if (ret)
+		dev_err(&client->dev,
+			"An error occured while entering soft reset!\n");
+
+	return ret;
+}
+
+/* Start/Stop streaming from the device */
+static int ov9640_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	return 0;
+}
+
+/* Alter bus settings on camera side */
+static int ov9640_set_bus_param(struct soc_camera_device *icd,
+					unsigned long flags)
+{
+	return 0;
+}
+
+/* Request bus settings on camera side */
+static unsigned long ov9640_query_bus_param(struct soc_camera_device *icd)
+{
+	struct soc_camera_link *icl = to_soc_camera_link(icd);
+
+	/*
+	 * REVISIT: the camera probably can do 10 bit transfers, but I don't
+	 *          have those pins connected on my hardware.
+	 */
+	unsigned long flags = SOCAM_PCLK_SAMPLE_RISING | SOCAM_MASTER |
+		SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_HSYNC_ACTIVE_HIGH |
+		SOCAM_DATA_ACTIVE_HIGH | SOCAM_DATAWIDTH_8;
+
+	return soc_camera_apply_sensor_flags(icl, flags);
+}
+
+/* Get status of additional camera capabilities */
+static int ov9640_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+{
+	struct i2c_client *client = sd->priv;
+	struct ov9640_priv *priv = container_of(i2c_get_clientdata(client),
+					struct ov9640_priv, subdev);
+
+	switch (ctrl->id) {
+	case V4L2_CID_VFLIP:
+		ctrl->value = priv->flag_vflip;
+		break;
+	case V4L2_CID_HFLIP:
+		ctrl->value = priv->flag_hflip;
+		break;
+	}
+	return 0;
+}
+
+/* Set status of additional camera capabilities */
+static int ov9640_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+{
+	struct i2c_client *client = sd->priv;
+	struct ov9640_priv *priv = container_of(i2c_get_clientdata(client),
+					struct ov9640_priv, subdev);
+
+	int ret = 0;
+
+	switch (ctrl->id) {
+	case V4L2_CID_VFLIP:
+		priv->flag_vflip = ctrl->value;
+		if (ctrl->value)
+			ret = ov9640_reg_rmw(client, OV9640_MVFP,
+							OV9640_MVFP_V, 0);
+		else
+			ret = ov9640_reg_rmw(client, OV9640_MVFP,
+							0, OV9640_MVFP_V);
+		break;
+	case V4L2_CID_HFLIP:
+		priv->flag_hflip = ctrl->value;
+		if (ctrl->value)
+			ret = ov9640_reg_rmw(client, OV9640_MVFP,
+							OV9640_MVFP_H, 0);
+		else
+			ret = ov9640_reg_rmw(client, OV9640_MVFP,
+							0, OV9640_MVFP_H);
+		break;
+	}
+
+	return ret;
+}
+
+/* Get chip identification */
+static int ov9640_g_chip_ident(struct v4l2_subdev *sd,
+				struct v4l2_dbg_chip_ident *id)
+{
+	struct i2c_client *client = sd->priv;
+	struct ov9640_priv *priv = container_of(i2c_get_clientdata(client),
+					struct ov9640_priv, subdev);
+
+	id->ident	= priv->model;
+	id->revision	= priv->revision;
+
+	return 0;
+}
+
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+static int ov9640_get_register(struct soc_camera_device *icd,
+			       struct v4l2_dbg_register *reg)
+{
+	struct i2c_client *client = to_i2c_client(to_soc_camera_control(icd));
+	struct ov9640_priv *priv = i2c_get_clientdata(client);
+
+	int ret;
+	u8 val;
+
+	if (reg->reg & ~0xff)
+		return -EINVAL;
+
+	reg->size = 1;
+
+	ret = ov9640_read_reg(priv->client, reg->reg, &val);
+	if (ret)
+		return ret;
+
+	reg->val = (__u64)val;
+
+	return 0;
+}
+
+static int ov9640_set_register(struct soc_camera_device *icd,
+			       struct v4l2_dbg_register *reg)
+{
+	struct i2c_client *client = to_i2c_client(to_soc_camera_control(icd));
+	struct ov9640_priv *priv = i2c_get_clientdata(client);
+
+	if (reg->reg & ~0xff || reg->val & ~0xff)
+		return -EINVAL;
+
+	return ov9640_write_reg(priv->client, reg->reg, reg->val);
+}
+#endif
+
+/* select nearest higher resolution for capture */
+static void ov9640_res_roundup(u32 *width, u32 *height)
+{
+	int i;
+	enum { QQCIF, QQVGA, QCIF, QVGA, CIF, VGA, SXGA };
+	int res_x[] = { 88, 160, 176, 320, 352, 640, 1280 };
+	int res_y[] = { 72, 120, 144, 240, 288, 480, 960 };
+
+	for (i = 0; i < ARRAY_SIZE(res_x); i++) {
+		if (res_x[i] >= *width && res_y[i] >= *height) {
+			*width = res_x[i];
+			*height = res_y[i];
+			return;
+		}
+	}
+
+	*width = res_x[SXGA];
+	*height = res_y[SXGA];
+}
+
+/* Prepare necessary register changes depending on color encoding */
+static void ov9640_alter_regs(u32 pixfmt, struct ov9640_reg_alt *alt)
+{
+	switch (pixfmt) {
+	case V4L2_PIX_FMT_VYUY:
+		alt->com12	= OV9640_COM12_YUV_AVG;
+		alt->com13	= OV9640_COM13_Y_DELAY_EN |
+					OV9640_COM13_YUV_DLY(0x01);
+		break;
+	case V4L2_PIX_FMT_RGB555X:
+		alt->com7	= OV9640_COM7_RGB;
+		alt->com13	= OV9640_COM13_RGB_AVG;
+		alt->com15	= OV9640_COM15_RGB_555;
+		break;
+	case V4L2_PIX_FMT_RGB565X:
+		alt->com7	= OV9640_COM7_RGB;
+		alt->com13	= OV9640_COM13_RGB_AVG;
+		alt->com15	= OV9640_COM15_RGB_565;
+		break;
+	};
+}
+
+/* Setup registers according to resolution and color encoding */
+static int ov9640_write_regs(struct i2c_client *client,
+		u32 width, u32 pixfmt, struct ov9640_reg_alt *alts)
+{
+	const struct ov9640_reg	*ov9640_regs, *matrix_regs;
+	int			ov9640_regs_len, matrix_regs_len;
+	int			i, ret;
+	u8			val;
+
+	/* select register configuration for given resolution */
+	switch (width) {
+	case W_QQCIF:
+		ov9640_regs	= ov9640_regs_qqcif;
+		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_qqcif);
+		break;
+	case W_QQVGA:
+		ov9640_regs	= ov9640_regs_qqvga;
+		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_qqvga);
+		break;
+	case W_QCIF:
+		ov9640_regs	= ov9640_regs_qcif;
+		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_qcif);
+		break;
+	case W_QVGA:
+		ov9640_regs	= ov9640_regs_qvga;
+		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_qvga);
+		break;
+	case W_CIF:
+		ov9640_regs	= ov9640_regs_cif;
+		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_cif);
+		break;
+	case W_VGA:
+		ov9640_regs	= ov9640_regs_vga;
+		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_vga);
+		break;
+	case W_SXGA:
+		ov9640_regs	= ov9640_regs_sxga;
+		ov9640_regs_len	= ARRAY_SIZE(ov9640_regs_sxga);
+		break;
+	default:
+		dev_err(&client->dev, "Failed to select resolution!\n");
+		return -EINVAL;
+	}
+
+	/* select color matrix configuration for given color encoding */
+	if (pixfmt == V4L2_PIX_FMT_VYUY) {
+		matrix_regs	= ov9640_regs_yuv;
+		matrix_regs_len	= ARRAY_SIZE(ov9640_regs_yuv);
+	} else {
+		matrix_regs	= ov9640_regs_rgb;
+		matrix_regs_len	= ARRAY_SIZE(ov9640_regs_rgb);
+	}
+
+	/* write register settings into the module */
+	for (i = 0; i < ov9640_regs_len; i++) {
+		val = ov9640_regs[i].val;
+
+		switch (ov9640_regs[i].reg) {
+		case OV9640_COM7:
+			val |= alts->com7;
+			break;
+		case OV9640_COM12:
+			val |= alts->com12;
+			break;
+		case OV9640_COM13:
+			val |= alts->com13;
+			break;
+		case OV9640_COM15:
+			val |= alts->com15;
+			break;
+		}
+
+		ret = ov9640_reg_write(client, ov9640_regs[i].reg, val);
+		if (ret)
+			return ret;
+	}
+
+	/* write color matrix configuration into the module */
+	for (i = 0; i < matrix_regs_len; i++) {
+		ret = ov9640_reg_write(client, matrix_regs[i].reg,
+						matrix_regs[i].val);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/* program default register values */
+static int ov9640_prog_dflt(struct i2c_client *client)
+{
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(ov9640_regs_dflt); i++) {
+		ret = ov9640_reg_write(client, ov9640_regs_dflt[i].reg,
+						ov9640_regs_dflt[i].val);
+		if (ret)
+			return ret;
+	}
+
+	/* wait for the changes to actually happen */
+	mdelay(150);
+
+	return 0;
+}
+
+/* set the format we will capture in */
+static int ov9640_s_fmt(struct v4l2_subdev *sd, struct v4l2_format *f)
+{
+	struct i2c_client *client = sd->priv;
+	struct v4l2_pix_format *pix = &f->fmt.pix;
+	struct ov9640_reg_alt alts = {0};
+	int ret;
+
+	ov9640_res_roundup(&pix->width, &pix->height);
+	ov9640_alter_regs(pix->pixelformat, &alts);
+
+	ov9640_reset(client);
+
+	ret = ov9640_prog_dflt(client);
+	if (ret)
+		return ret;
+
+	return ov9640_write_regs(client, pix->width, pix->pixelformat, &alts);
+}
+
+static int ov9640_try_fmt(struct v4l2_subdev *sd, struct v4l2_format *f)
+{
+	struct v4l2_pix_format *pix = &f->fmt.pix;
+
+	ov9640_res_roundup(&pix->width, &pix->height);
+	pix->field  = V4L2_FIELD_NONE;
+
+	return 0;
+}
+
+static int ov9640_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
+{
+	a->c.left	= 0;
+	a->c.top	= 0;
+	a->c.width	= W_SXGA;
+	a->c.height	= H_SXGA;
+	a->type		= V4L2_BUF_TYPE_VIDEO_CAPTURE;
+
+	return 0;
+}
+
+static int ov9640_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
+{
+	a->bounds.left			= 0;
+	a->bounds.top			= 0;
+	a->bounds.width			= W_SXGA;
+	a->bounds.height		= H_SXGA;
+	a->defrect			= a->bounds;
+	a->type				= V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	a->pixelaspect.numerator	= 1;
+	a->pixelaspect.denominator	= 1;
+
+	return 0;
+}
+
+
+
+static int ov9640_video_probe(struct soc_camera_device *icd,
+				struct i2c_client *client)
+{
+	struct ov9640_priv *priv = i2c_get_clientdata(client);
+	u8		pid, ver, midh, midl;
+	const char	*devname;
+	int		ret = 0;
+
+	/*
+	 * We must have a parent by now. And it cannot be a wrong one.
+	 * So this entire test is completely redundant.
+	 */
+	if (!icd->dev.parent ||
+	    to_soc_camera_host(icd->dev.parent)->nr != icd->iface) {
+		dev_err(&icd->dev, "Parent missing or invalid!\n");
+		ret = -ENODEV;
+		goto err;
+	}
+
+	icd->formats		= ov9640_fmt_lists;
+	icd->num_formats	= ARRAY_SIZE(ov9640_fmt_lists);
+
+	/*
+	 * check and show product ID and manufacturer ID
+	 */
+
+	ret = ov9640_reg_read(client, OV9640_PID, &pid);
+	if (ret)
+		goto err;
+
+	ret = ov9640_reg_read(client, OV9640_VER, &ver);
+	if (ret)
+		goto err;
+
+	ret = ov9640_reg_read(client, OV9640_MIDH, &midh);
+	if (ret)
+		goto err;
+
+	ret = ov9640_reg_read(client, OV9640_MIDL, &midl);
+	if (ret)
+		goto err;
+
+	switch (VERSION(pid, ver)) {
+	case OV9640_V2:
+		devname		= "ov9640";
+		priv->model	= V4L2_IDENT_OV9640;
+		priv->revision	= 2;
+	case OV9640_V3:
+		devname		= "ov9640";
+		priv->model	= V4L2_IDENT_OV9640;
+		priv->revision	= 3;
+		break;
+	default:
+		dev_err(&icd->dev, "Product ID error %x:%x\n", pid, ver);
+		ret = -ENODEV;
+		goto err;
+	}
+
+	dev_info(&icd->dev, "%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
+		 devname, pid, ver, midh, midl);
+
+err:
+	return ret;
+}
+
+static struct soc_camera_ops ov9640_ops = {
+	.set_bus_param		= ov9640_set_bus_param,
+	.query_bus_param	= ov9640_query_bus_param,
+	.controls		= ov9640_controls,
+	.num_controls		= ARRAY_SIZE(ov9640_controls),
+};
+
+static struct v4l2_subdev_core_ops ov9640_core_ops = {
+	.g_ctrl			= ov9640_g_ctrl,
+	.s_ctrl			= ov9640_s_ctrl,
+	.g_chip_ident		= ov9640_g_chip_ident,
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+	.g_register		= ov9640_get_register,
+	.s_register		= ov9640_set_register,
+#endif
+
+};
+
+static struct v4l2_subdev_video_ops ov9640_video_ops = {
+	.s_stream		= ov9640_s_stream,
+	.s_fmt			= ov9640_s_fmt,
+	.try_fmt		= ov9640_try_fmt,
+	.cropcap		= ov9640_cropcap,
+	.g_crop			= ov9640_g_crop,
+
+};
+
+static struct v4l2_subdev_ops ov9640_subdev_ops = {
+	.core	= &ov9640_core_ops,
+	.video	= &ov9640_video_ops,
+};
+
+/*
+ * i2c_driver function
+ */
+static int ov9640_probe(struct i2c_client *client,
+			 const struct i2c_device_id *did)
+{
+	struct ov9640_priv *priv;
+	struct i2c_adapter *adapter	= to_i2c_adapter(client->dev.parent);
+	struct soc_camera_device *icd	= client->dev.platform_data;
+	struct soc_camera_link *icl;
+	int ret;
+
+	if (!icd) {
+		dev_err(&client->dev, "Missing soc-camera data!\n");
+		return -EINVAL;
+	}
+
+	icl = to_soc_camera_link(icd);
+	if (!icl) {
+		dev_err(&client->dev, "Missing platform_data for driver\n");
+		return -EINVAL;
+	}
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
+		dev_err(&adapter->dev,
+			"I2C adapter doesn't support I2C_FUNC_SMBUS_BYTE\n");
+		return -EIO;
+	}
+
+	priv = kzalloc(sizeof(struct ov9640_priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(&client->dev,
+			"Failed to allocate memory for private data!\n");
+		return -ENOMEM;
+	}
+
+	v4l2_i2c_subdev_init(&priv->subdev, client, &ov9640_subdev_ops);
+
+	icd->ops	= &ov9640_ops;
+
+	ret = ov9640_video_probe(icd, client);
+
+	if (ret) {
+		icd->ops = NULL;
+		i2c_set_clientdata(client, NULL);
+		kfree(priv);
+	}
+
+	return ret;
+}
+
+static int ov9640_remove(struct i2c_client *client)
+{
+	struct ov9640_priv *priv = i2c_get_clientdata(client);
+
+	i2c_set_clientdata(client, NULL);
+	kfree(priv);
+	return 0;
+}
+
+static const struct i2c_device_id ov9640_id[] = {
+	{ "ov9640", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ov9640_id);
+
+static struct i2c_driver ov9640_i2c_driver = {
+	.driver = {
+		.name = "ov9640",
+	},
+	.probe    = ov9640_probe,
+	.remove   = ov9640_remove,
+	.id_table = ov9640_id,
+};
+
+static int __init ov9640_module_init(void)
+{
+	return i2c_add_driver(&ov9640_i2c_driver);
+}
+
+static void __exit ov9640_module_exit(void)
+{
+	i2c_del_driver(&ov9640_i2c_driver);
+}
+
+module_init(ov9640_module_init);
+module_exit(ov9640_module_exit);
+
+MODULE_DESCRIPTION("SoC Camera driver for OmniVision OV96xx");
+MODULE_AUTHOR("Marek Vasut <marek.vasut@gmail.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/media/video/ov9640.h b/drivers/media/video/ov9640.h
new file mode 100644
index 0000000..6c39f1e
--- /dev/null
+++ b/drivers/media/video/ov9640.h
@@ -0,0 +1,206 @@ 
+/*
+ * OmniVision OV96xx Camera Header File
+ *
+ * Copyright (C) 2009 Marek Vasut <marek.vasut@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef	__DRIVERS_MEDIA_VIDEO_OV9640_H__
+#define	__DRIVERS_MEDIA_VIDEO_OV9640_H__
+
+/* Register definitions */
+#define	OV9640_GAIN	0x00
+#define	OV9640_BLUE	0x01
+#define	OV9640_RED	0x02
+#define	OV9640_VFER	0x03
+#define	OV9640_COM1	0x04
+#define	OV9640_BAVE	0x05
+#define	OV9640_GEAVE	0x06
+#define	OV9640_RSID	0x07
+#define	OV9640_RAVE	0x08
+#define	OV9640_COM2	0x09
+#define	OV9640_PID	0x0a
+#define	OV9640_VER	0x0b
+#define	OV9640_COM3	0x0c
+#define	OV9640_COM4	0x0d
+#define	OV9640_COM5	0x0e
+#define	OV9640_COM6	0x0f
+#define	OV9640_AECH	0x10
+#define	OV9640_CLKRC	0x11
+#define	OV9640_COM7	0x12
+#define	OV9640_COM8	0x13
+#define	OV9640_COM9	0x14
+#define	OV9640_COM10	0x15
+/* 0x16 - RESERVED */
+#define	OV9640_HSTART	0x17
+#define	OV9640_HSTOP	0x18
+#define	OV9640_VSTART	0x19
+#define	OV9640_VSTOP	0x1a
+#define	OV9640_PSHFT	0x1b
+#define	OV9640_MIDH	0x1c
+#define	OV9640_MIDL	0x1d
+#define	OV9640_MVFP	0x1e
+#define	OV9640_LAEC	0x1f
+#define	OV9640_BOS	0x20
+#define	OV9640_GBOS	0x21
+#define	OV9640_GROS	0x22
+#define	OV9640_ROS	0x23
+#define	OV9640_AEW	0x24
+#define	OV9640_AEB	0x25
+#define	OV9640_VPT	0x26
+#define	OV9640_BBIAS	0x27
+#define	OV9640_GBBIAS	0x28
+/* 0x29 - RESERVED */
+#define	OV9640_EXHCH	0x2a
+#define	OV9640_EXHCL	0x2b
+#define	OV9640_RBIAS	0x2c
+#define	OV9640_ADVFL	0x2d
+#define	OV9640_ADVFH	0x2e
+#define	OV9640_YAVE	0x2f
+#define	OV9640_HSYST	0x30
+#define	OV9640_HSYEN	0x31
+#define	OV9640_HREF	0x32
+#define	OV9640_CHLF	0x33
+#define	OV9640_ARBLM	0x34
+/* 0x35..0x36 - RESERVED */
+#define	OV9640_ADC	0x37
+#define	OV9640_ACOM	0x38
+#define	OV9640_OFON	0x39
+#define	OV9640_TSLB	0x3a
+#define	OV9640_COM11	0x3b
+#define	OV9640_COM12	0x3c
+#define	OV9640_COM13	0x3d
+#define	OV9640_COM14	0x3e
+#define	OV9640_EDGE	0x3f
+#define	OV9640_COM15	0x40
+#define	OV9640_COM16	0x41
+#define	OV9640_COM17	0x42
+/* 0x43..0x4e - RESERVED */
+#define	OV9640_MTX1	0x4f
+#define	OV9640_MTX2	0x50
+#define	OV9640_MTX3	0x51
+#define	OV9640_MTX4	0x52
+#define	OV9640_MTX5	0x53
+#define	OV9640_MTX6	0x54
+#define	OV9640_MTX7	0x55
+#define	OV9640_MTX8	0x56
+#define	OV9640_MTX9	0x57
+#define	OV9640_MTXS	0x58
+/* 0x59..0x61 - RESERVED */
+#define	OV9640_LCC1	0x62
+#define	OV9640_LCC2	0x63
+#define	OV9640_LCC3	0x64
+#define	OV9640_LCC4	0x65
+#define	OV9640_LCC5	0x66
+#define	OV9640_MANU	0x67
+#define	OV9640_MANV	0x68
+#define	OV9640_HV	0x69
+#define	OV9640_MBD	0x6a
+#define	OV9640_DBLV	0x6b
+#define	OV9640_GSP	0x6c	/* ... till 0x7b */
+#define	OV9640_GST	0x7c	/* ... till 0x8a */
+
+#define	OV9640_CLKRC_DPLL_EN	0x80
+#define	OV9640_CLKRC_DIRECT	0x40
+#define	OV9640_CLKRC_DIV(x)	((x) & 0x3f)
+
+#define	OV9640_PSHFT_VAL(x)	((x) & 0xff)
+
+#define	OV9640_ACOM_2X_ANALOG	0x80
+#define	OV9640_ACOM_RSVD	0x12
+
+#define	OV9640_MVFP_V		0x10
+#define	OV9640_MVFP_H		0x20
+
+#define	OV9640_COM1_HREF_NOSKIP	0x00
+#define	OV9640_COM1_HREF_2SKIP	0x04
+#define	OV9640_COM1_HREF_3SKIP	0x08
+#define	OV9640_COM1_QQFMT	0x20
+
+#define	OV9640_COM2_SSM		0x10
+
+#define	OV9640_COM3_VP		0x04
+
+#define	OV9640_COM4_QQ_VP	0x80
+#define	OV9640_COM4_RSVD	0x40
+
+#define	OV9640_COM5_SYSCLK	0x80
+#define	OV9640_COM5_LONGEXP	0x01
+
+#define	OV9640_COM6_OPT_BLC	0x40
+#define	OV9640_COM6_ADBLC_BIAS	0x08
+#define	OV9640_COM6_FMT_RST	0x82
+#define	OV9640_COM6_ADBLC_OPTEN	0x01
+
+#define	OV9640_COM7_RAW_RGB	0x01
+#define	OV9640_COM7_RGB		0x04
+#define	OV9640_COM7_QCIF	0x08
+#define	OV9640_COM7_QVGA	0x10
+#define	OV9640_COM7_CIF		0x20
+#define	OV9640_COM7_VGA		0x40
+#define	OV9640_COM7_SCCB_RESET	0x80
+
+#define	OV9640_COM12_YUV_AVG	0x04
+#define	OV9640_COM12_RSVD	0x40
+
+#define	OV9640_COM13_GAMMA_NONE	0x00
+#define	OV9640_COM13_GAMMA_Y	0x40
+#define	OV9640_COM13_GAMMA_RAW	0x80
+#define	OV9640_COM13_RGB_AVG	0x20
+#define	OV9640_COM13_MATRIX_EN	0x10
+#define	OV9640_COM13_Y_DELAY_EN	0x08
+#define	OV9640_COM13_YUV_DLY(x)	((x) & 0x07)
+
+#define	OV9640_COM15_OR_00FF	0x00
+#define	OV9640_COM15_OR_01FE	0x40
+#define	OV9640_COM15_OR_10F0	0xc0
+#define	OV9640_COM15_RGB_NORM	0x00
+#define	OV9640_COM15_RGB_565	0x10
+#define	OV9640_COM15_RGB_555	0x30
+
+#define	OV9640_COM16_RB_AVG	0x01
+
+/* IDs */
+#define	OV9640_V2		0x9648
+#define	OV9640_V3		0x9649
+#define	VERSION(pid, ver)	(((pid) << 8) | ((ver) & 0xFF))
+
+/* supported resolutions */
+enum {
+	W_QQCIF	= 88,
+	W_QQVGA	= 160,
+	W_QCIF	= 176,
+	W_QVGA	= 320,
+	W_CIF	= 352,
+	W_VGA	= 640,
+	W_SXGA	= 1280
+};
+#define	H_SXGA	960
+
+/* Misc. structures */
+struct ov9640_reg_alt {
+	u8	com7;
+	u8	com12;
+	u8	com13;
+	u8	com15;
+};
+
+struct ov9640_reg {
+	u8	reg;
+	u8	val;
+};
+
+struct ov9640_priv {
+	struct v4l2_subdev		subdev;
+
+	int				model;
+	int				revision;
+
+	bool				flag_vflip;
+	bool				flag_hflip;
+};
+
+#endif	/* __DRIVERS_MEDIA_VIDEO_OV9640_H__ */
diff --git a/include/media/v4l2-chip-ident.h b/include/media/v4l2-chip-ident.h
index 11a4a2d..0892e52 100644
--- a/include/media/v4l2-chip-ident.h
+++ b/include/media/v4l2-chip-ident.h
@@ -60,6 +60,7 @@  enum {
 	V4L2_IDENT_OV7670 = 250,
 	V4L2_IDENT_OV7720 = 251,
 	V4L2_IDENT_OV7725 = 252,
+	V4L2_IDENT_OV9640 = 253,
 
 	/* module saa7146: reserved range 300-309 */
 	V4L2_IDENT_SAA7146 = 300,