[RFC,2/7] ARM: DaVinci: DM646x Video: ADV7343 video encoder driver

Commit Message

[e-mail] March 13, 2009, 9:08 a.m. UTC

From: Chaithrika U S <[e-mail]>

ADV7343 video encoder driver

Add ADV7473 I2C based video encoder driver. This follows the v4l2-subdev
framework.

Signed-off-by: Chaithrika U S <[e-mail]>
---
Applies to v4l-dvb repository located at
http://linuxtv.org/hg/v4l-dvb/rev/1fd54a62abde

 drivers/media/video/Kconfig   |    9 +
 drivers/media/video/Makefile  |    1 +
 drivers/media/video/adv7343.c |  730 +++++++++++++++++++++++++++++++++++++++++
 include/media/adv7343.h       |  373 +++++++++++++++++++++
 4 files changed, 1113 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/video/adv7343.c
 create mode 100644 include/media/adv7343.h

Comments

Hans Verkuil March 14, 2009, 2:13 p.m. UTC | #1

Hi Chaithrika,

Here is the review of this driver.

On Friday 13 March 2009 10:08:39 [e-mail] wrote:
> From: Chaithrika U S <[e-mail]>
> 
> ADV7343 video encoder driver
> 
> Add ADV7473 I2C based video encoder driver. This follows the v4l2-subdev
> framework.
> 
> Signed-off-by: Chaithrika U S <[e-mail]>
> ---
> Applies to v4l-dvb repository located at
> http://linuxtv.org/hg/v4l-dvb/rev/1fd54a62abde
> 
>  drivers/media/video/Kconfig   |    9 +
>  drivers/media/video/Makefile  |    1 +
>  drivers/media/video/adv7343.c |  730 +++++++++++++++++++++++++++++++++++++++++
>  include/media/adv7343.h       |  373 +++++++++++++++++++++
>  4 files changed, 1113 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/media/video/adv7343.c
>  create mode 100644 include/media/adv7343.h
> 
> diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> index 27f6397..16019e9 100644
> --- a/drivers/media/video/Kconfig
> +++ b/drivers/media/video/Kconfig
> @@ -426,6 +426,15 @@ config VIDEO_ADV7175
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called adv7175.
>  
> +config VIDEO_ADV7343
> +        tristate "ADV7343 video encoder"
> +        depends on I2C
> +        help
> +          Support for Analog Devices I2C bus based ADV7343 encoder.
> +
> +          To compile this driver as a module, choose M here: the
> +          module will be called adv7473.
> +
>  comment "Video improvement chips"
>  
>  config VIDEO_UPD64031A
> diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
> index 99b448e..7f9fc62 100644
> --- a/drivers/media/video/Makefile
> +++ b/drivers/media/video/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
>  obj-$(CONFIG_VIDEO_SAA7191) += saa7191.o
>  obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
>  obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
> +obj-$(CONFIG_VIDEO_ADV7343) += adv7343.o
>  obj-$(CONFIG_VIDEO_VPX3220) += vpx3220.o
>  obj-$(CONFIG_VIDEO_BT819) += bt819.o
>  obj-$(CONFIG_VIDEO_BT856) += bt856.o
> diff --git a/drivers/media/video/adv7343.c b/drivers/media/video/adv7343.c
> new file mode 100644
> index 0000000..c912f1d
> --- /dev/null
> +++ b/drivers/media/video/adv7343.c
> @@ -0,0 +1,730 @@
> +/*
> + * adv7343 - ADV7343 Video Encoder Driver
> + *
> + * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed .as is. WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/ctype.h>
> +#include <linux/i2c.h>
> +#include <linux/device.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/videodev2.h>
> +#include <linux/uaccess.h>
> +#include <linux/version.h>
> +
> +#include <media/adv7343.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-chip-ident.h>
> +#include <media/v4l2-i2c-drv.h>
> +
> +static int debug = 2;

Not a module option. See ths7303 review for more info.

> +static unsigned char reg00 = 0x80;	/* Power Mode register */
> +static unsigned char reg01 = 0x00;	/* MODE_SELECT_REG */
> +static unsigned char reg02 = 0x20;	/* MODE_REG0 */
> +static unsigned char reg30 = 0x3C;	/* HD_MODE_REG1 */
> +static unsigned char reg35 = 0x00;	/* HD_MODE_REG6 */
> +static unsigned char reg80 = ADV7343_SD_MODE_REG1_DEFAULT; /* SD_MODE_REG1 */
> +static unsigned char reg82 = ADV7343_SD_MODE_REG2_DEFAULT; /* SD_MODE_REG2 */
> +
> +struct adv7343_state {
> +	struct i2c_client *client;

Don't add this, obtain from v4l2_subdev instead.

> +	u32 ident;
> +	struct v4l2_subdev sd;
> +	v4l2_std_id std;
> +	int output;
> +	int enable;
> +	int bright;
> +	int hue;
> +	int gain;
> +	int initialized;
> +	int video_enable;
> +	int ch_id;
> +};
> +
> +static inline struct adv7343_state *to_state(struct v4l2_subdev *sd)
> +{
> +	return container_of(sd, struct adv7343_state, sd);
> +}
> +
> +static inline int adv7343_write(struct v4l2_subdev *sd, u8 reg, u8 value)
> +{
> +	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +	return i2c_smbus_write_byte_data(client, reg, value);
> +}
> +
> +struct adv7343_std_info
> +	adv7343_composite_std_info[ADV7343_COMPOSITE_NUM_STD] = {
> +	{
> +		ADV7343_SD_MODE_REG1, &reg80, SD_INPUT_MODE, (~(SD_STD_MASK)),
> +		SD_STD_NTSC, 0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> +		V4L2_STD_NTSC,
> +	},
> +	{
> +		ADV7343_SD_MODE_REG1, &reg80, SD_INPUT_MODE, (~(SD_STD_MASK)),
> +		SD_STD_PAL_BDGHI, 0x8C, 0xCB, 0x8D, 0x8A, 0x8E, 0x09, 0x8F,
> +		0x2A, V4L2_STD_PAL,
> +	},
> +};
> +
> +struct adv7343_std_info
> +	adv7343_component_std_info[ADV7343_COMPONENT_NUM_STD] = {
> +	{
> +		ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
> +		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
> +		(STD_MODE_720P << STD_MODE_SHIFT),
> +		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> +		V4L2_STD_720P_60,
> +	},
> +	{
> +		ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
> +		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
> +		(STD_MODE_720P_25 << STD_MODE_SHIFT),
> +		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> +		V4L2_STD_720P_25,
> +	},
> +	{
> +		ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
> +		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
> +		(STD_MODE_720P_30 << STD_MODE_SHIFT),
> +		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> +		V4L2_STD_720P_30,
> +	},
> +	{
> +		ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
> +		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
> +		(STD_MODE_720P_50 << STD_MODE_SHIFT),
> +		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> +		V4L2_STD_720P_50,
> +	},
> +	{
> +		ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
> +		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
> +		(STD_MODE_1080I << STD_MODE_SHIFT),
> +		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> +		V4L2_STD_1080I_30,
> +	},
> +	{
> +		ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
> +		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
> +		(STD_MODE_1080I_25fps << STD_MODE_SHIFT),
> +		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> +		V4L2_STD_1080I_25,
> +	},
> +	{
> +		ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
> +		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
> +		(STD_MODE_525P << STD_MODE_SHIFT),
> +		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> +		V4L2_STD_480P_60,
> +	},
> +	{
> +		ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
> +		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
> +		(STD_MODE_625P << STD_MODE_SHIFT),
> +		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> +		V4L2_STD_576P_50,
> +	},
> +	{
> +		ADV7343_SD_MODE_REG1, &reg80, SD_INPUT_MODE, (~(SD_STD_MASK)),
> +		SD_STD_NTSC, 0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> +		V4L2_STD_NTSC,
> +	},
> +	{
> +		ADV7343_SD_MODE_REG1, &reg80, SD_INPUT_MODE, (~(SD_STD_MASK)),
> +		SD_STD_PAL_BDGHI, 0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F,
> +		0x21, V4L2_STD_PAL,
> +	},
> +	{
> +		ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
> +		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
> +		(STD_MODE_1080P_24 << STD_MODE_SHIFT),
> +		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> +		V4L2_STD_1080P_24,
> +	},
> +	{
> +		ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
> +		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
> +		(STD_MODE_1080P_25 << STD_MODE_SHIFT),
> +		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> +		V4L2_STD_1080I_25,
> +	},
> +	{
> +		ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
> +		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
> +		(STD_MODE_1080P_30 << STD_MODE_SHIFT),
> +		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> +		V4L2_STD_1080P_30,
> +	},
> +};
> +
> +static struct adv7343_config adv7343_configuration[ADV7343_NUM_CHANNELS] = {
> +	{
> +		.no_of_outputs = ADV7343_MAX_NO_OUTPUTS,
> +		.output[0] = {
> +			.output_type	= ADV7343_COMPOSITE_ID,
> +			.output_name	= VID_ENC_OUTPUT_COMPOSITE,
> +			.no_of_standard	= ADV7343_COMPOSITE_NUM_STD,
> +			.def_std	= V4L2_STD_NTSC,
> +			.std_info	= (struct adv7343_std_info *)
> +						&adv7343_composite_std_info,
> +			.power_val	= ADV7343_COMPOSITE_POWER_VALUE,
> +		},
> +		.output[1] = {
> +			.output_type	= ADV7343_COMPONENT_ID,
> +			.output_name	= VID_ENC_OUTPUT_COMPONENT,
> +			.no_of_standard	= ADV7343_COMPONENT_NUM_STD,
> +			.def_std	= V4L2_STD_720P_60,
> +			.std_info	= (struct adv7343_std_info *)
> +						&adv7343_component_std_info,
> +			.power_val	= ADV7343_COMPONENT_POWER_VALUE,
> +		},
> +		.output[2] = {
> +			.output_type	= ADV7343_SVIDEO_ID,
> +			.output_name	= VID_ENC_OUTPUT_SVIDEO,
> +			.no_of_standard	= ADV7343_SVIDEO_NUM_STD,
> +			.def_std	= V4L2_STD_NTSC,
> +			.std_info	= (struct adv7343_std_info *)
> +						&adv7343_composite_std_info,
> +			.power_val	= ADV7343_SVIDEO_POWER_VALUE
> +		},
> +	},
> +};
> +
> +static struct adv7343_channel adv7343_channel_info[ADV7343_NUM_CHANNELS] = {
> +	{
> +		.current_output	= ADV7343_COMPOSITE_ID,
> +		.mode_info	= V4L2_STD_NTSC,
> +	}
> +};

These arrays might be mixing user and device level concepts. See my comments
on that at the end. It's not clear to me whether it is indeed the case, I'll
leave it to you to take action if it's wrong and I'll review it the next
round.

> +static int adv7343_setstd(struct v4l2_subdev *sd, v4l2_std_id std)
> +{
> +	int err = 0;
> +	int i = 0;
> +	struct adv7343_std_info *std_info;
> +	int output_idx;
> +	u8 reg, val;
> +
> +	int ch_id = (to_state(sd))->ch_id;
> +	struct adv7343_config *config = &adv7343_configuration[ch_id];
> +
> +	v4l2_dbg(1, debug, sd, "Start of adv7343_setstd..\n");
> +	output_idx = adv7343_channel_info[ch_id].current_output;
> +	v4l2_dbg(1, debug, sd, "the output index is %d\n", output_idx);
> +
> +	for (i = 0; i < config->output[output_idx].no_of_standard; i++) {
> +		std_info = &config->output[output_idx].std_info[i];
> +		if (std_info->stdid ==  std)
> +			break;
> +	}
> +
> +	if (i == config->output[output_idx].no_of_standard) {
> +		v4l2_err(sd, "Invalid id...\n");
> +		return -EINVAL;
> +	}
> +
> +	val = *(config->output[output_idx].std_info[i].value);
> +	val &= config->output[output_idx].std_info[i].standard_val2;
> +	val |= config->output[output_idx].std_info[i].standard_val3;
> +	err |= adv7343_write(sd,
> +		config->output[output_idx].std_info[i].set_std_register, val);
> +	if (err < 0) {
> +		v4l2_err(sd, "Set standard failed\n");
> +		return err;
> +	}
> +	*(config->output[output_idx].std_info[i].value) = val;
> +
> +	val = reg01;
> +	val &= (~((u8) INPUT_MODE_MASK));
> +	val |= config->output[output_idx].std_info[i].outputmode_val1;
> +	err |= adv7343_write(sd, ADV7343_MODE_SELECT_REG, val);
> +	if (err < 0) {
> +		v4l2_err(sd, "Set standard failed\n");
> +		return err;
> +	}
> +	reg01 = val;
> +
> +	/* Store the standard in global object of adv7343 */
> +	adv7343_channel_info[ch_id].mode_info =
> +				config->output[output_idx].std_info[i].stdid;
> +
> +	reg = config->output[output_idx].std_info[i].fsc0_reg;
> +	val = config->output[output_idx].std_info[i].fsc0_val;
> +	err |= adv7343_write(sd, reg, val);
> +
> +	reg = config->output[output_idx].std_info[i].fsc1_reg;
> +	val = config->output[output_idx].std_info[i].fsc1_val;
> +	err |= adv7343_write(sd, reg, val);
> +
> +	reg = config->output[output_idx].std_info[i].fsc2_reg;
> +	val = config->output[output_idx].std_info[i].fsc2_val;
> +	err |= adv7343_write(sd, reg, val);
> +
> +	reg = config->output[output_idx].std_info[i].fsc3_reg;
> +	val = config->output[output_idx].std_info[i].fsc3_val;
> +	err |= adv7343_write(sd, reg, val);
> +
> +	val = reg80;
> +
> +	if (std == V4L2_STD_NTSC)
> +		val &= 0x03;
> +	else if (std ==  V4L2_STD_PAL)
> +		val |= 0x04;
> +
> +	err |= adv7343_write(sd, ADV7343_SD_MODE_REG1, val);
> +
> +	reg80 = val;
> +
> +	v4l2_dbg(1, debug, sd, "</adv7343_setstd>\n");
> +
> +	return err;
> +}
> +
> +/* Following function is used to set output format in ADV7343 device. The index
> +   of the output format is  passed as the argument to this function. */
> +static int adv7343_setoutput(struct v4l2_subdev *sd, int output_type)
> +{
> +	unsigned char val;
> +	int i;
> +	int index;
> +	int err = 0;
> +	int ch_id = (to_state(sd))->ch_id;
> +	struct adv7343_config *config = &adv7343_configuration[ch_id];
> +
> +	v4l2_dbg(1, debug, sd, "Start of set output function.\n");
> +
> +	for (i = 0; i < config->no_of_outputs; i++) {
> +		if (output_type == config->output[i].output_type)
> +			break;
> +	}
> +
> +	if (i == config->no_of_outputs) {
> +		v4l2_err(sd, "Invalid output\n");
> +		return -EINVAL;
> +	}
> +	index = i;
> +
> +	/* Enable Appropriate DAC */
> +	val = reg00;
> +	val &= 0x03;
> +	val |= config->output[index].power_val;
> +	err = adv7343_write(sd, ADV7343_POWER_MODE_REG, val);
> +
> +	reg00 = val;
> +
> +	/* Enable YUV output */
> +	val = reg02;
> +	val |= YUV_OUTPUT_SELECT;
> +	err |= adv7343_write(sd, ADV7343_MODE_REG0, val);
> +
> +	reg02 = val;
> +
> +	/* configure SD DAC Output 2 and SD DAC Output 1 bit to zero */
> +	val = reg82;
> +	val &= (SD_DAC_1_DI & SD_DAC_2_DI);
> +	err |= adv7343_write(sd, ADV7343_SD_MODE_REG2, val);
> +	if (err < 0)
> +		return err;
> +	reg82 = val;
> +
> +	/* configure ED/HD Color DAC Swap and ED/HD RGB Input Enable bit to
> +	 * zero */
> +	val = reg35;
> +	val &= (HD_RGB_INPUT_DI & HD_DAC_SWAP_DI);
> +	err |= adv7343_write(sd, ADV7343_HD_MODE_REG6, val);
> +	if (err < 0)
> +		return err;
> +	reg35 = val;
> +
> +	adv7343_channel_info[ch_id].current_output = index;
> +	adv7343_channel_info[ch_id].mode_info = config->output[index].def_std;
> +
> +	err |= adv7343_setstd(sd, adv7343_channel_info[ch_id].mode_info);
> +
> +	if (err < 0)
> +		return err;
> +
> +	v4l2_dbg(1, debug, sd, "</adv7343_setoutput>\n");
> +
> +	return err;
> +}
> +
> +static int adv7343_getstd(struct v4l2_subdev *sd,
> +				v4l2_std_id *stdid)
> +{
> +	int err = 0;
> +	int ch_id =  (to_state(sd))->ch_id;
> +	int output_idx;
> +
> +	v4l2_dbg(1, debug, sd, "In getstd function.\n");
> +	output_idx = adv7343_channel_info[ch_id].current_output;
> +
> +	*stdid = adv7343_channel_info[ch_id].mode_info;
> +
> +	return err;
> +}
> +
> +static int adv7343_log_status(struct v4l2_subdev *sd)
> +{
> +	struct adv7343_state *state = to_state(sd);
> +
> +	v4l2_info(sd, "Standard: %llu\n", state->std);
> +	v4l2_info(sd, "Output: %s\n", (state->output) ? "COMPOSITE" :
> +							"COMPONENT");
> +	v4l2_info(sd, "Channel: %d\n", state->ch_id);
> +
> +	return 0;
> +}
> +
> +static int adv7343_initialize(struct v4l2_subdev *sd)
> +{
> +	int err = 0;
> +	int ch_id = (to_state(sd))->ch_id; /* for now */
> +	err |= adv7343_write(sd, ADV7343_SOFT_RESET,
> +				ADV7343_SOFT_RESET_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_POWER_MODE_REG,
> +				ADV7343_POWER_MODE_REG_DEFAULT);
> +
> +	err |= adv7343_write(sd, ADV7343_HD_MODE_REG1,
> +				ADV7343_HD_MODE_REG1_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_HD_MODE_REG2,
> +				 ADV7343_HD_MODE_REG2_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_HD_MODE_REG3,
> +				ADV7343_HD_MODE_REG3_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_HD_MODE_REG4,
> +				ADV7343_HD_MODE_REG4_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_HD_MODE_REG5,
> +				ADV7343_HD_MODE_REG5_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_HD_MODE_REG6,
> +				ADV7343_HD_MODE_REG6_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_HD_MODE_REG7,
> +				ADV7343_HD_MODE_REG7_DEFAULT);
> +
> +	err |= adv7343_write(sd, ADV7343_SD_MODE_REG1,
> +				ADV7343_SD_MODE_REG1_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_SD_MODE_REG2,
> +				ADV7343_SD_MODE_REG2_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_SD_MODE_REG3,
> +				ADV7343_SD_MODE_REG3_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_SD_MODE_REG4,
> +				ADV7343_SD_MODE_REG4_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_SD_MODE_REG5,
> +				ADV7343_SD_MODE_REG5_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_SD_MODE_REG6,
> +				ADV7343_SD_MODE_REG6_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_SD_MODE_REG7,
> +				ADV7343_SD_MODE_REG7_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_SD_MODE_REG8,
> +				ADV7343_SD_MODE_REG8_DEFAULT);
> +
> +	err |= adv7343_write(sd, ADV7343_SD_HUE_REG,
> +				ADV7343_SD_HUE_REG_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_SD_CGMS_WSS0,
> +				ADV7343_SD_CGMS_WSS0_DEFAULT);
> +	err |= adv7343_write(sd, ADV7343_SD_BRIGHTNESS_WSS,
> +				ADV7343_SD_BRIGHTNESS_WSS_DEFAULT);
> +
> +	if (err < 0) {
> +		v4l2_err(sd, "Error in initializing!\n");
> +		err = -EINVAL;
> +		goto adv7343_init_exit;
> +	}
> +
> +	adv7343_channel_info[ch_id].current_output = 0;
> +	adv7343_channel_info[ch_id].mode_info =
> +		    adv7343_composite_std_info[0].stdid;
> +
> +	/* Configure for default video standard */
> +	err |= adv7343_setoutput(sd, adv7343_configuration[ch_id].
> +					output[0].output_type);
> +	err |= adv7343_setstd(sd, adv7343_configuration[ch_id].
> +					output[0].def_std);
> +
> +	if (err < 0) {
> +		err = -EINVAL;
> +		goto adv7343_init_exit;
> +	}
> +
> +	v4l2_dbg(1, debug, sd, "</adv7343_initialize>\n");
> +
> +adv7343_init_exit:
> +	return err;
> +}
> +
> +static int adv7343_reset(struct v4l2_subdev *sd, u32 val)
> +{
> +	v4l2_dbg(1, debug, sd, "Reset\n");
> +	return adv7343_initialize(sd);
> +}
> +
> +static int adv7343_init(struct v4l2_subdev *sd, u32 val)
> +{
> +	struct adv7343_state *state = to_state(sd);
> +	if (!state->initialized) {
> +		state->initialized = 1;
> +		v4l2_dbg(1, debug, sd, "Initializing Encoder\n");
> +		return adv7343_initialize(sd);
> +	}
> +
> +	return 0;
> +}
> +
> +static int adv7343_queryctrl(struct v4l2_subdev *sd, struct v4l2_queryctrl *qc)
> +{
> +	switch (qc->id) {
> +	case V4L2_CID_BRIGHTNESS:
> +	case V4L2_CID_HUE:
> +		return v4l2_ctrl_query_fill_std(qc);

Use v4l2_ctrl_query_fill instead. The fill_std function has been removed
(that was a bad idea).

> +	default:
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int adv7343_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
> +{
> +	struct adv7343_state *state = to_state(sd);
> +	int err = 0;
> +
> +	switch (ctrl->id) {
> +	case V4L2_CID_BRIGHTNESS:
> +		if (ctrl->value < 0 || ctrl->value > 127) {
> +			v4l2_err(sd, "invalid brightness setting %d\n",
> +					ctrl->value);

Recommend that you use v4l2_dbg for this instead.

> +			return -ERANGE;
> +		}
> +
> +		state->bright = ctrl->value;
> +		err = adv7343_write(sd, ADV7343_SD_BRIGHTNESS_WSS,
> +					state->bright);
> +		break;
> +
> +	case V4L2_CID_HUE:
> +		if (ctrl->value < 0 || ctrl->value > 255) {
> +			v4l2_err(sd, "invalid hue settings %d\n", ctrl->value);
> +			return -ERANGE;
> +		}
> +
> +		state->hue = ctrl->value;
> +		err = adv7343_write(sd, ADV7343_SD_HUE_REG, state->hue);
> +		break;
> +
> +	case V4L2_CID_GAIN:

Why is there no V4L2_CID_GAIN case in queryctrl above?

> +		if (ctrl->value < 0 || ctrl->value > 255) {
> +			v4l2_err(sd, "invalid gain settings %d\n", ctrl->value);
> +			return -ERANGE;
> +		}
> +
> +		if ((ctrl->value > POSITIVE_GAIN_MAX) &&
> +			(ctrl->value < NEGATIVE_GAIN_MIN)) {
> +			v4l2_err(sd, "gain settings not within \
> +					the specified range\n");
> +			return -ERANGE;
> +		} else {
> +			state->gain = ctrl->value;
> +			err = adv7343_write(sd, ADV7343_DAC2_OUTPUT_LEVEL,
> +					state->gain);
> +		}
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +	if (err < 0)
> +		v4l2_err(sd, "Failed tp set the encoder controls\n");

Typo: tp -> to

> +
> +	return 0;
> +}
> +
> +static int adv7343_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
> +{
> +	struct adv7343_state *state = to_state(sd);
> +
> +	switch (ctrl->id) {
> +	case V4L2_CID_BRIGHTNESS:
> +		ctrl->value = state->bright;
> +		break;
> +
> +	case V4L2_CID_HUE:
> +		ctrl->value = state->hue;
> +		break;
> +
> +	case V4L2_CID_GAIN:
> +		ctrl->value = state->gain;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int adv7343_g_chip_ident(struct v4l2_subdev *sd,
> +				struct v4l2_dbg_chip_ident *chip)
> +{
> +	struct adv7343_state *state = to_state(sd);
> +	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +	return v4l2_chip_ident_i2c_client(client, chip, state->ident, 0);
> +}
> +
> +static long adv7343_ioctl(struct v4l2_subdev *sd, unsigned cmd, void *arg)
> +{
> +	int err = 0;
> +	v4l2_dbg(1, debug, sd, "ioctl\n");
> +	switch (cmd) {
> +	case ENCODER_GET_MODE:
> +		err = adv7343_getstd(sd, (v4l2_std_id *)arg);
> +		break;

Not a good idea. The v4l-dvb master repository adds a .querystd callback that
you should use instead. It's a recent addition that appears in 2.6.30.

> +
> +	default:
> +		break;
> +	}
> +
> +	return err;
> +}
> +
> +static const struct v4l2_subdev_core_ops adv7343_core_ops = {
> +	.log_status	= adv7343_log_status,
> +	.g_chip_ident	= adv7343_g_chip_ident,
> +	.g_ctrl		= adv7343_g_ctrl,
> +	.s_ctrl		= adv7343_s_ctrl,
> +	.queryctrl	= adv7343_queryctrl,
> +	.reset		= adv7343_reset,

Do you really need a reset?

> +	.init		= adv7343_init,

Do you really need an init? Better to init in the probe() function.
See also my comments about init in the ths7303 review.

> +	.ioctl		= adv7343_ioctl,
> +};
> +
> +static int adv7343_s_std_output(struct v4l2_subdev *sd, v4l2_std_id std)
> +{
> +	struct adv7343_state *state = to_state(sd);
> +	int err = 0;
> +
> +	if (state->std == std)
> +		return 0;
> +
> +	err = adv7343_setstd(sd, std);
> +
> +	if (!err)
> +		state->std = std;
> +	else
> +		v4l2_err(sd, "s_std failed\n");
> +
> +	return err;
> +}
> +
> +static int adv7343_s_routing(struct v4l2_subdev *sd,
> +				const struct v4l2_routing *route)
> +{
> +	struct adv7343_state *state = to_state(sd);
> +
> +	int err = 0;
> +
> +	if (state->output == route->output)
> +		return 0;
> +
> +	err = adv7343_setoutput(sd, route->output);
> +	if (err)
> +		v4l2_err(sd, "Error setting output\n");
> +	else
> +		state->output = route->output;
> +
> +	return err;
> +}
> +
> +static const struct v4l2_subdev_video_ops adv7343_video_ops = {
> +	.s_std_output	= adv7343_s_std_output,
> +	.s_routing	= adv7343_s_routing,
> +};
> +
> +static const struct v4l2_subdev_ops adv7343_ops = {
> +	.core	= &adv7343_core_ops,
> +	.video	= &adv7343_video_ops,
> +};
> +
> +static int adv7343_command(struct i2c_client *client, unsigned cmd, void *arg)
> +{
> +	return v4l2_subdev_command(i2c_get_clientdata(client), cmd, arg);
> +}

Not needed, see my ths7303 review.

> +
> +static int adv7343_probe(struct i2c_client *client,
> +				const struct i2c_device_id *id)
> +{
> +	struct adv7343_state *state;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> +		return -ENODEV;
> +
> +	v4l2_info(client, "chip found @ 0x%x (%s)\n",
> +			client->addr << 1, client->adapter->name);

use v4l_info here.

> +
> +	state = kzalloc(sizeof(struct adv7343_state), GFP_KERNEL);
> +	if (state == NULL)
> +		return -ENOMEM;
> +
> +	state->client	= client;
> +	state->enable	= 1;
> +	state->ch_id	= 0;
> +	state->output	= -1;
> +	state->initialized = 0;
> +	state->ident = 0;
> +	v4l2_i2c_subdev_init(&state->sd, client, &adv7343_ops);
> +	v4l2_dbg(1, debug, client, "Registered the encoder\n");

This v4l2_dbg doesn't add anything useful that v4l_info didn't already say.

> +
> +	return 0;
> +}
> +
> +static int adv7343_remove(struct i2c_client *client)
> +{
> +	struct v4l2_subdev *sd = i2c_get_clientdata(client);
> +
> +	v4l2_device_unregister_subdev(sd);
> +	kfree(to_state(sd));
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id adv7343_id[] = {
> +	{ADV7343_NAME, 0},
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, adv7343_id);
> +
> +static struct v4l2_i2c_driver_data v4l2_i2c_data = {
> +	.name		= ADV7343_NAME,
> +	.command	= adv7343_command,
> +	.probe		= adv7343_probe,
> +	.remove		= adv7343_remove,
> +	.legacy_class	= I2C_CLASS_TV_ANALOG | I2C_CLASS_TV_DIGITAL,
> +	.id_table	= adv7343_id,
> +};
> +
> +static __init int init_adv7343(void)
> +{
> +	return 0;
> +}
> +
> +static __exit void exit_adv7343(void)
> +{
> +
> +}

See my comments in the ths7303 review.

> +
> +module_init(init_adv7343);
> +module_exit(exit_adv7343);
> +MODULE_LICENSE("GPL");
> diff --git a/include/media/adv7343.h b/include/media/adv7343.h
> new file mode 100644
> index 0000000..b7da4a6
> --- /dev/null
> +++ b/include/media/adv7343.h
> @@ -0,0 +1,373 @@
> +/*
> + * ADV7343 header file
> + *
> + * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed .as is. WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef ADV7343_H
> +#define ADV7343_H
> +
> +#ifdef __KERNEL__

Not needed. Files in include/media are internal to the kernel only.

I'm not doing an in-depth review of this header. You first need to split
it up in two parts: one adv7343_regs.h header that is included by the adv7343.c
driver and is in the same directory as that driver: that header contains all
the internal datastructures and defines (register addresses, etc.) that
it uses.

The media/adv7343.h contains only the defines that other drivers need to
interface with the adv7343 driver: in particular the routing information
for the s_routing ops goes there.

Once that's done I'll review it again.

One thing to keep in mind at all times is that you do not mix user-level
input/output descriptions and device-level input/output descriptions.

E.g. say that pin X is connected to the "Composite 1" output connector
on the board. The i2c driver has no knowledge of that. All it needs to
know is that pin X is the output pin. The platform driver, however, is
the one that has access to the actual board layout and should be
the driver that associates the "Composite 1" connector (a user-level
description) with pin X in the i2c device (a device-level description).

The s_routing operations only deal with the device-level. How you encode
the pins is purely device specific and goes into the media/adv7343.h
header. Depending on the i2c device you may have to specify both input and
output pins. E.g. the input pin would be the pin that the videoport sends
its data to, and the output pin is the pin that is connector to the physical
connector on the board.

Note that when the conversion of the legacy drivers is finished I want to
modify the s_routing function a bit: instead of using the awkward
v4l2_routing struct you can just specify the input and output fields directly
as arguments. And I think that I'll also add a config argument since
sometimes you need to specify addition configuration information. The
media/saa7115.h is an example of that where the output field is abused to
pass config information.

Regards,

        Hans

> +
> +/* Kernel Header files */
> +#include <linux/i2c.h>
> +#include <linux/device.h>
> +#include <linux/videodev2.h>
> +
> +#endif				/* __KERNEL__ */
> +
> +#define ADV7343_NAME	"adv7343"
> +
> +/** VID_ENC_NAME_MAX_CHARS
> + *
> + * Description:
> + * MAX characters in the name.
> + */
> +#define VID_ENC_NAME_MAX_CHARS	30
> +
> +/*
> + * constant strings for output names.
> + */
> +#define VID_ENC_OUTPUT_COMPOSITE	"COMPOSITE"
> +#define VID_ENC_OUTPUT_SVIDEO		"SVIDEO"
> +#define	VID_ENC_OUTPUT_COMPONENT	"COMPONENT"
> +
> +/* Internal IOCTL defines */
> +#define ENCODER_GET_MODE	_IOR('e', BASE_VIDIOC_PRIVATE + 1, v4l2_std_id*)
> +
> +/* Macros */
> +#define ADV7343_MAX_GAMMA_COEFFS	(10)	/* Maximum Gamma Coefficients */
> +
> +#ifdef __KERNEL__
> +
> +#define ADV7343_COMPOSITE_OUTPUT_NAME	"COMPOSITE"
> +#define ADV7343_COMPONENT_OUTPUT_NAME	"COMPONENT"
> +#define ADV7343_SVIDEO_OUTPUT_NAME	"SVIDEO"
> +#define ADV7343_COMPOSITE_ID	(0)
> +#define ADV7343_COMPONENT_ID	(1)
> +#define ADV7343_SVIDEO_ID	(2)
> +
> +#define ADV7343_COMPOSITE_NO_CONTROLS	(3)
> +#define ADV7343_COMPONENT_NO_CONTROLS	(1)
> +#define ADV7343_SVIDEO_NO_CONTROLS	(3)
> +
> +#define ADV7343_NUM_CHANNELS		(1)
> +
> +/* encoder standard related strctures */
> +#define ADV7343_MAX_NO_OUTPUTS		(3)
> +#define ADV7343_COMPOSITE_NUM_STD	(2)
> +#define ADV7343_COMPONENT_NUM_STD	(7+3+3)
> +#define ADV7343_SVIDEO_NUM_STD		(2)
> +#define ADV7343_MAX_NO_CONTROLS		(3)
> +#define ADV7343_VBI_NUM_SERVICES	(3)
> +
> +struct adv7343_std_info {
> +	unsigned char set_std_register;
> +	unsigned char *value;
> +	u32 outputmode_val1;
> +	u32 standard_val2;
> +	u32 standard_val3;
> +	u8 fsc0_reg, fsc0_val;
> +	u8 fsc1_reg, fsc1_val;
> +	u8 fsc2_reg, fsc2_val;
> +	u8 fsc3_reg, fsc3_val;
> +	v4l2_std_id stdid;
> +};
> +
> +struct adv7343_config {
> +	int no_of_outputs;
> +	struct {
> +		unsigned char power_val;
> +		int output_type;
> +		char output_name[VID_ENC_NAME_MAX_CHARS];
> +		int no_of_standard;
> +		v4l2_std_id def_std;
> +		struct adv7343_std_info *std_info;
> +	} output[ADV7343_MAX_NO_OUTPUTS];
> +	unsigned short services_set;
> +	u8 num_services;
> +};
> +
> +struct adv7343_channel {
> +	u8 current_output;
> +	v4l2_std_id mode_info;
> +	unsigned short services_set;
> +};
> +
> +#define ADV7343_VALID_FEATURE_VAL(val)	((ADV7343_DISABLE == (val)) || \
> +					(ADV7343_ENABLE == (val)))
> +#define ADV7343_VALID_GAMMA_CURVE(val)	((ADV7343_GAMMA_CURVE_A == (val)) || \
> +					(ADV7343_GAMMA_CURVE_B == (val)))
> +
> +/* Register offset macros */
> +#define ADV7343_POWER_MODE_REG		(0x00)
> +#define ADV7343_MODE_SELECT_REG		(0x01)
> +#define ADV7343_MODE_REG0		(0x02)
> +#define ADV7343_CSC_MATRIX0		(0x03)
> +#define ADV7343_CSC_MATRIX1		(0x04)
> +#define ADV7343_CSC_MATRIX2		(0x05)
> +#define ADV7343_CSC_MATRIX3		(0x06)
> +#define ADV7343_CSC_MATRIX4		(0x07)
> +#define ADV7343_CSC_MATRIX5		(0x08)
> +#define ADV7343_CSC_MATRIX6		(0x09)
> +#define ADV7343_DAC1_OUTPUT_LEVEL	(0x0a)
> +#define ADV7343_DAC2_OUTPUT_LEVEL	(0x0b)
> +#define ADV7343_DAC_POWER_MODE		(0x0d)
> +#define ADV7343_CABLE_DETECTION		(0x10)
> +#define ADV7343_SBUS_READ		(0x12)
> +#define ADV7343_YBUS_READ		(0x13)
> +#define ADV7343_CBUS_READ		(0x14)
> +#define ADV7343_CONTROL_READ		(0x16)
> +#define ADV7343_SOFT_RESET		(0x17)
> +#define ADV7343_HD_MODE_REG1		(0x30)
> +#define ADV7343_HD_MODE_REG2		(0x31)
> +#define ADV7343_HD_MODE_REG3		(0x32)
> +#define ADV7343_HD_MODE_REG4		(0x33)
> +#define ADV7343_HD_MODE_REG5		(0x34)
> +#define ADV7343_HD_MODE_REG6		(0x35)
> +#define ADV7343_HD_Y_LEVEL		(0x36)
> +#define ADV7343_HD_CR_LEVEL		(0x37)
> +#define ADV7343_HD_CB_LEVEL		(0x38)
> +#define ADV7343_HD_MODE_REG7		(0x39)
> +#define ADV7343_HD_SHARPNESS_FLTR_GAIN	(0x40)
> +#define ADV7343_HD_CGMS_DATA_0		(0x41)
> +#define ADV7343_HD_CGMS_DATA_1		(0x42)
> +#define ADV7343_HD_CGMS_DATA_2		(0x43)
> +#define ADV7343_HD_GAMMA_A0		(0x44)
> +#define ADV7343_HD_GAMMA_A1		(0x45)
> +#define ADV7343_HD_GAMMA_A2		(0x46)
> +#define ADV7343_HD_GAMMA_A3		(0x47)
> +#define ADV7343_HD_GAMMA_A4		(0x48)
> +#define ADV7343_HD_GAMMA_A5		(0x49)
> +#define ADV7343_HD_GAMMA_A6		(0x4a)
> +#define ADV7343_HD_GAMMA_A7		(0x4b)
> +#define ADV7343_HD_GAMMA_A8		(0x4c)
> +#define ADV7343_HD_GAMMA_A9		(0x4d)
> +#define ADV7343_HD_GAMMA_B0		(0x4E)
> +#define ADV7343_HD_GAMMA_B1		(0x4F)
> +#define ADV7343_HD_GAMMA_B2		(0x50)
> +#define ADV7343_HD_GAMMA_B3		(0x51)
> +#define ADV7343_HD_GAMMA_B4		(0x52)
> +#define ADV7343_HD_GAMMA_B5		(0x53)
> +#define ADV7343_HD_GAMMA_B6		(0x54)
> +#define ADV7343_HD_GAMMA_B7		(0x55)
> +#define ADV7343_HD_GAMMA_B8		(0x56)
> +#define ADV7343_HD_GAMMA_B9		(0x57)
> +#define ADV7343_HD_ADPT_FLTR_GAIN1	(0x58)
> +#define ADV7343_HD_ADPT_FLTR_GAIN2	(0x59)
> +#define ADV7343_HD_ADPT_FLTR_GAIN3	(0x5a)
> +#define ADV7343_HD_ADPT_FLTR_THRLDA	(0x5b)
> +#define ADV7343_HD_ADPT_FLTR_THRLDB	(0x5c)
> +#define ADV7343_HD_ADPT_FLTR_THRLDC	(0x5d)
> +#define ADV7343_HD_CGMS_B0		(0x5E)
> +#define ADV7343_HD_CGMS_B1		(0x5F)
> +#define ADV7343_HD_CGMS_B2		(0x60)
> +#define ADV7343_HD_CGMS_B3		(0x61)
> +#define ADV7343_HD_CGMS_B4		(0x62)
> +#define ADV7343_HD_CGMS_B5		(0x63)
> +#define ADV7343_HD_CGMS_B6		(0x64)
> +#define ADV7343_HD_CGMS_B7		(0x65)
> +#define ADV7343_HD_CGMS_B8		(0x66)
> +#define ADV7343_HD_CGMS_B9		(0x67)
> +#define ADV7343_HD_CGMS_B10		(0x68)
> +#define ADV7343_HD_CGMS_B11		(0x69)
> +#define ADV7343_HD_CGMS_B12		(0x6A)
> +#define ADV7343_HD_CGMS_B13		(0x6B)
> +#define ADV7343_HD_CGMS_B14		(0x6C)
> +#define ADV7343_HD_CGMS_B15		(0x6D)
> +#define ADV7343_HD_CGMS_B16		(0x6E)
> +
> +#define ADV7343_SD_MODE_REG1		(0x80)
> +#define ADV7343_SD_MODE_REG2		(0x82)
> +#define ADV7343_SD_MODE_REG3		(0x83)
> +#define ADV7343_SD_MODE_REG4		(0x84)
> +#define ADV7343_SD_MODE_REG5		(0x86)
> +#define ADV7343_SD_MODE_REG6		(0x87)
> +#define ADV7343_SD_MODE_REG7		(0x88)
> +#define ADV7343_SD_MODE_REG8		(0x89)
> +#define ADV7343_SD_TIMING_REG0		(0x8A)
> +#define ADV7343_SD_TIMING_REG1		(0x8B)
> +#define ADV7343_SD_FSC_REG0		(0x8C)
> +#define ADV7343_SD_FSC_REG1		(0x8D)
> +#define ADV7343_SD_FSC_REG2		(0x8E)
> +#define ADV7343_SD_FSC_REG3		(0x8F)
> +#define ADV7343_SD_FSC_PHASE		(0x90)
> +#define ADV7343_SD_CLOSE_CAPTION_EVEN0	(0x91)
> +#define ADV7343_SD_CLOSE_CAPTION_EVEN1	(0x92)
> +#define ADV7343_SD_CLOSE_CAPTION_ODD0	(0x93)
> +#define ADV7343_SD_CLOSE_CAPTION_ODD1	(0x94)
> +#define ADV7343_SD_PEDESTAL_REG0	(0x95)
> +#define ADV7343_SD_PEDESTAL_REG1	(0x96)
> +#define ADV7343_SD_PEDESTAL_REG2	(0x97)
> +#define ADV7343_SD_PEDESTAL_REG3	(0x98)
> +#define ADV7343_SD_CGMS_WSS0		(0x99)
> +#define ADV7343_SD_CGMS_WSS1		(0x9A)
> +#define ADV7343_SD_CGMS_WSS2		(0x9B)
> +
> +#define ADV7343_SD_SCALE_LSB		(0x9C)
> +#define ADV7343_SD_Y_SCALE		(0x9D)
> +#define ADV7343_SD_CB_SCALE		(0x9E)
> +#define ADV7343_SD_CR_SCALE		(0x9F)
> +
> +#define ADV7343_SD_HUE_REG		(0xA0)
> +#define ADV7343_SD_BRIGHTNESS_WSS	(0xA1)
> +#define ADV7343_SD_LUMA_SSAF		(0xA2)
> +#define ADV7343_SD_DNR0			(0xA3)
> +#define ADV7343_SD_DNR1			(0xA4)
> +#define ADV7343_SD_DNR2			(0xA5)
> +
> +#define ADV7343_SD_GAMMA_A0		(0xA6)
> +#define ADV7343_SD_GAMMA_A1		(0xA7)
> +#define ADV7343_SD_GAMMA_A2		(0xA8)
> +#define ADV7343_SD_GAMMA_A3		(0xA9)
> +#define ADV7343_SD_GAMMA_A4		(0xAA)
> +#define ADV7343_SD_GAMMA_A5		(0xAB)
> +#define ADV7343_SD_GAMMA_A6		(0xAC)
> +#define ADV7343_SD_GAMMA_A7		(0xAD)
> +#define ADV7343_SD_GAMMA_A8		(0xAE)
> +#define ADV7343_SD_GAMMA_A9		(0xAF)
> +#define ADV7343_SD_GAMMA_B0		(0xB0)
> +#define ADV7343_SD_GAMMA_B1		(0xB1)
> +#define ADV7343_SD_GAMMA_B2		(0xB2)
> +#define ADV7343_SD_GAMMA_B3		(0xB3)
> +#define ADV7343_SD_GAMMA_B4		(0xB4)
> +#define ADV7343_SD_GAMMA_B5		(0xB5)
> +#define ADV7343_SD_GAMMA_B6		(0xB6)
> +#define ADV7343_SD_GAMMA_B7		(0xB7)
> +#define ADV7343_SD_GAMMA_B8		(0xB8)
> +#define ADV7343_SD_GAMMA_B9		(0xB9)
> +#define ADV7343_SD_BRIGHTNESS_DETECT	(0xBA)
> +#define ADV7343_FIELD_COUNT_REG		(0xBB)
> +#define ADV7343_10_BIT_INPUT		(0x7C)
> +
> +/* Default values for the registers */
> +#define ADV7343_POWER_MODE_REG_DEFAULT		(0x10)
> +#define ADV7343_HD_MODE_REG1_DEFAULT		(0x3C)	/* Changed Default
> +							   720p EAVSAV code*/
> +#define ADV7343_HD_MODE_REG2_DEFAULT		(0x01)	/* Changed Pixel data
> +							   valid */
> +#define ADV7343_HD_MODE_REG3_DEFAULT		(0x00)	/* Color delay 0 clks */
> +#define ADV7343_HD_MODE_REG4_DEFAULT		(0xE8)	/* Changed */
> +#define ADV7343_HD_MODE_REG5_DEFAULT		(0x08)
> +#define ADV7343_HD_MODE_REG6_DEFAULT		(0x00)
> +#define ADV7343_HD_MODE_REG7_DEFAULT		(0x00)
> +
> +#define ADV7343_SD_MODE_REG1_DEFAULT		(0x00)
> +#define ADV7343_SD_MODE_REG2_DEFAULT		(0xC9)
> +#define ADV7343_SD_MODE_REG3_DEFAULT		(0x10)
> +#define ADV7343_SD_MODE_REG4_DEFAULT		(0x01)
> +#define ADV7343_SD_MODE_REG5_DEFAULT		(0x02)
> +#define ADV7343_SD_MODE_REG6_DEFAULT		(0x0C)
> +#define ADV7343_SD_MODE_REG7_DEFAULT		(0x04)
> +#define ADV7343_SD_MODE_REG8_DEFAULT		(0x00)
> +#define ADV7343_SOFT_RESET_DEFAULT		(0x02)
> +#define ADV7343_COMPOSITE_POWER_VALUE		(0x80)
> +#define ADV7343_COMPONENT_POWER_VALUE		(0x1C)
> +#define ADV7343_SVIDEO_POWER_VALUE		(0x60)
> +#define ADV7343_SD_HUE_REG_DEFAULT		(127)
> +#define ADV7343_SD_BRIGHTNESS_WSS_DEFAULT	(0x03)
> +#define ADV7343_SD_CGMS_WSS0_DEFAULT		(0x10)
> +
> +/* Macros for the Mode Select Register */
> +#ifdef GENERATE_MASK
> +#undef GENERATE_MASK
> +#endif
> +#define GENERATE_MASK(bits, pos)	((((0xFF) << (8 - bits)) >> \
> +					(8 - bits)) << pos)
> +
> +/* Bit masks for Mode Select Register */
> +#define INPUT_MODE_MASK			(0x70)
> +#define SD_INPUT_MODE			(0x00)
> +#define HD_720P_INPUT_MODE		(0x10)
> +#define HD_1080I_INPUT_MODE		(0x10)
> +
> +/* Bit masks for Mode Register 0 */
> +#define TEST_PATTERN_BLACK_BAR_EN	(0x04)
> +#define YUV_OUTPUT_SELECT		(0x20)
> +#define RGB_OUTPUT_SELECT		(0xDF)
> +
> +/* Bit masks for DAC output levels */
> +#define DAC_OUTPUT_LEVEL_MASK		(0xFF)
> +#define POSITIVE_GAIN_MAX		(0x40)
> +#define POSITIVE_GAIN_MIN		(0x00)
> +#define NEGATIVE_GAIN_MAX		(0xFF)
> +#define NEGATIVE_GAIN_MIN		(0xC0)
> +
> +/* Bit masks for soft reset register */
> +#define SOFT_RESET			(0x02)
> +
> +/* Bit masks for HD Mode Register 1 */
> +#define OUTPUT_STD_MASK			(0x03)
> +#define OUTPUT_STD_SHIFT		(0)
> +#define OUTPUT_STD_EIA0_2		(0x00)
> +#define OUTPUT_STD_EIA0_1		(0x01)
> +#define OUTPUT_STD_FULL			(0x02)
> +#define EMBEDDED_SYNC			(0x04)
> +#define EXTERNAL_SYNC			(0xFB)
> +#define STD_MODE_SHIFT			(3)
> +#define STD_MODE_MASK			(0x1F)
> +#define STD_MODE_720P			(0x05)
> +#define STD_MODE_720P_25		(0x08)
> +#define STD_MODE_720P_30		(0x07)
> +#define STD_MODE_720P_50		(0x06)
> +#define STD_MODE_1080I			(0x0D)
> +#define STD_MODE_1080I_25fps		(0x0E)
> +#define STD_MODE_1080P_24		(0x12)
> +#define STD_MODE_1080P_25		(0x10)
> +#define STD_MODE_1080P_30		(0x0F)
> +#define STD_MODE_525P			(0x00)
> +#define STD_MODE_625P			(0x03)
> +
> +/* Bit masks for SD Mode Register 1 */
> +#define SD_STD_MASK			(0x03)
> +#define SD_STD_NTSC			(0x00)
> +#define SD_STD_PAL_BDGHI		(0x01)
> +#define SD_STD_PAL_M			(0x02)
> +#define SD_STD_PAL_N			(0x03)
> +#define SD_LUMA_FLTR_MASK		(0x7)
> +#define SD_LUMA_FLTR_SHIFT		(0x2)
> +#define SD_CHROMA_FLTR_MASK		(0x7)
> +#define SD_CHROMA_FLTR_SHIFT		(0x5)
> +
> +/* Bit masks for SD Mode Register 2 */
> +#define SD_PBPR_SSAF_EN			(0x01)
> +#define SD_PBPR_SSAF_DI			(0xFE)
> +#define SD_DAC_1_DI			(0xFD)
> +#define SD_DAC_2_DI			(0xFB)
> +#define SD_PEDESTAL_EN			(0x08)
> +#define SD_PEDESTAL_DI			(0xF7)
> +#define SD_SQUARE_PIXEL_EN		(0x10)
> +#define SD_SQUARE_PIXEL_DI		(0xEF)
> +#define SD_PIXEL_DATA_VALID		(0x40)
> +#define SD_ACTIVE_EDGE_EN		(0x80)
> +#define SD_ACTIVE_EDGE_DI		(0x7F)
> +
> +/* Bit masks for HD Mode Register 6 */
> +#define HD_RGB_INPUT_EN			(0x02)
> +#define HD_RGB_INPUT_DI			(0xFD)
> +#define HD_PBPR_SYNC_EN			(0x04)
> +#define HD_PBPR_SYNC_DI			(0xFB)
> +#define HD_DAC_SWAP_EN			(0x08)
> +#define HD_DAC_SWAP_DI			(0xF7)
> +#define HD_GAMMA_CURVE_A		(0xEF)
> +#define HD_GAMMA_CURVE_B		(0x10)
> +#define HD_GAMMA_EN			(0x20)
> +#define HD_GAMMA_DI			(0xDF)
> +#define HD_ADPT_FLTR_MODEB		(0x40)
> +#define HD_ADPT_FLTR_MODEA		(0xBF)
> +#define HD_ADPT_FLTR_EN			(0x80)
> +#define HD_ADPT_FLTR_DI			(0x7F)
> +
> +#endif				/* End of #ifdef __KERNEL__ */
> +
> +#endif				/* End of #ifndef ADV7343_H */

[e-mail] March 17, 2009, 5:55 a.m. UTC | #2

Hans,

Please see my response inline. For the comments common to THS7303
and ADV7343 driver, I will do the changes to both the drivers.

>  Hi Chaithrika,
>
> Here is the review of this driver.
>
> On Friday 13 March 2009 10:08:39 [e-mail] wrote:
> > From: Chaithrika U S <[e-mail]>
> >
> > ADV7343 video encoder driver
> >
> > Add ADV7473 I2C based video encoder driver. This follows the v4l2-
> subdev
> > framework.
> >
> > Signed-off-by: Chaithrika U S <[e-mail]>
> > ---
> > Applies to v4l-dvb repository located at
> > http://linuxtv.org/hg/v4l-dvb/rev/1fd54a62abde
> >
> >  drivers/media/video/Kconfig   |    9 +
> >  drivers/media/video/Makefile  |    1 +
> >  drivers/media/video/adv7343.c |  730
> +++++++++++++++++++++++++++++++++++++++++
> >  include/media/adv7343.h       |  373 +++++++++++++++++++++
> >  4 files changed, 1113 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/media/video/adv7343.c
> >  create mode 100644 include/media/adv7343.h
> >
> > diff --git a/drivers/media/video/Kconfig
> b/drivers/media/video/Kconfig
> > index 27f6397..16019e9 100644
> > --- a/drivers/media/video/Kconfig
> > +++ b/drivers/media/video/Kconfig
> > @@ -426,6 +426,15 @@ config VIDEO_ADV7175
> >          To compile this driver as a module, choose M here: the
> >          module will be called adv7175.
> >
> > +config VIDEO_ADV7343
> > +        tristate "ADV7343 video encoder"
> > +        depends on I2C
> > +        help
> > +          Support for Analog Devices I2C bus based ADV7343 encoder.
> > +
> > +          To compile this driver as a module, choose M here: the
> > +          module will be called adv7473.
> > +
> >  comment "Video improvement chips"
> >
> >  config VIDEO_UPD64031A
> > diff --git a/drivers/media/video/Makefile
> b/drivers/media/video/Makefile
> > index 99b448e..7f9fc62 100644
> > --- a/drivers/media/video/Makefile
> > +++ b/drivers/media/video/Makefile
> > @@ -49,6 +49,7 @@ obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
> >  obj-$(CONFIG_VIDEO_SAA7191) += saa7191.o
> >  obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
> >  obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
> > +obj-$(CONFIG_VIDEO_ADV7343) += adv7343.o
> >  obj-$(CONFIG_VIDEO_VPX3220) += vpx3220.o
> >  obj-$(CONFIG_VIDEO_BT819) += bt819.o
> >  obj-$(CONFIG_VIDEO_BT856) += bt856.o
> > diff --git a/drivers/media/video/adv7343.c
> b/drivers/media/video/adv7343.c
> > new file mode 100644
> > index 0000000..c912f1d
> > --- /dev/null
> > +++ b/drivers/media/video/adv7343.c
> > @@ -0,0 +1,730 @@
> > +/*
> > + * adv7343 - ADV7343 Video Encoder Driver
> > + *
> > + * Copyright (C) 2009 Texas Instruments Incorporated -
> http://www.ti.com/
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation version 2.
> > + *
> > + * This program is distributed .as is. WITHOUT ANY WARRANTY of any
> > + * kind, whether express or implied; without even the implied
> warranty
> > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/init.h>
> > +#include <linux/ctype.h>
> > +#include <linux/i2c.h>
> > +#include <linux/device.h>
> > +#include <linux/delay.h>
> > +#include <linux/module.h>
> > +#include <linux/videodev2.h>
> > +#include <linux/uaccess.h>
> > +#include <linux/version.h>
> > +
> > +#include <media/adv7343.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-chip-ident.h>
> > +#include <media/v4l2-i2c-drv.h>
> > +
> > +static int debug = 2;
>
> Not a module option. See ths7303 review for more info.
I will update this.

>
> > +static unsigned char reg00 = 0x80;   /* Power Mode register */
> > +static unsigned char reg01 = 0x00;   /* MODE_SELECT_REG */
> > +static unsigned char reg02 = 0x20;   /* MODE_REG0 */
> > +static unsigned char reg30 = 0x3C;   /* HD_MODE_REG1 */
> > +static unsigned char reg35 = 0x00;   /* HD_MODE_REG6 */
> > +static unsigned char reg80 = ADV7343_SD_MODE_REG1_DEFAULT; /*
> SD_MODE_REG1 */
> > +static unsigned char reg82 = ADV7343_SD_MODE_REG2_DEFAULT; /*
> SD_MODE_REG2 */
> > +
> > +struct adv7343_state {
> > +     struct i2c_client *client;
>
> Don't add this, obtain from v4l2_subdev instead.
OK.

>
> > +     u32 ident;
> > +     struct v4l2_subdev sd;
> > +     v4l2_std_id std;
> > +     int output;
> > +     int enable;
> > +     int bright;
> > +     int hue;
> > +     int gain;
> > +     int initialized;
> > +     int video_enable;
> > +     int ch_id;
> > +};
> > +
> > +static inline struct adv7343_state *to_state(struct v4l2_subdev *sd)
> > +{
> > +     return container_of(sd, struct adv7343_state, sd);
> > +}
> > +
> > +static inline int adv7343_write(struct v4l2_subdev *sd, u8 reg, u8
> value)
> > +{
> > +     struct i2c_client *client = v4l2_get_subdevdata(sd);
> > +
> > +     return i2c_smbus_write_byte_data(client, reg, value);
> > +}
> > +
> > +struct adv7343_std_info
> > +     adv7343_composite_std_info[ADV7343_COMPOSITE_NUM_STD] = {
> > +     {
> > +             ADV7343_SD_MODE_REG1, &reg80, SD_INPUT_MODE,
> (~(SD_STD_MASK)),
> > +             SD_STD_NTSC, 0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F,
> 0x21,
> > +             V4L2_STD_NTSC,
> > +     },
> > +     {
> > +             ADV7343_SD_MODE_REG1, &reg80, SD_INPUT_MODE,
> (~(SD_STD_MASK)),
> > +             SD_STD_PAL_BDGHI, 0x8C, 0xCB, 0x8D, 0x8A, 0x8E, 0x09,
> 0x8F,
> > +             0x2A, V4L2_STD_PAL,
> > +     },
> > +};
> > +
> > +struct adv7343_std_info
> > +     adv7343_component_std_info[ADV7343_COMPONENT_NUM_STD] = {
> > +     {
> > +             ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
> > +             (~(STD_MODE_MASK << STD_MODE_SHIFT)),
> > +             (STD_MODE_720P << STD_MODE_SHIFT),
> > +             0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> > +             V4L2_STD_720P_60,
> > +     },
> > +     {
> > +             ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
> > +             (~(STD_MODE_MASK << STD_MODE_SHIFT)),
> > +             (STD_MODE_720P_25 << STD_MODE_SHIFT),
> > +             0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> > +             V4L2_STD_720P_25,
> > +     },
> > +     {
> > +             ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
> > +             (~(STD_MODE_MASK << STD_MODE_SHIFT)),
> > +             (STD_MODE_720P_30 << STD_MODE_SHIFT),
> > +             0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> > +             V4L2_STD_720P_30,
> > +     },
> > +     {
> > +             ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
> > +             (~(STD_MODE_MASK << STD_MODE_SHIFT)),
> > +             (STD_MODE_720P_50 << STD_MODE_SHIFT),
> > +             0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> > +             V4L2_STD_720P_50,
> > +     },
> > +     {
> > +             ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
> > +             (~(STD_MODE_MASK << STD_MODE_SHIFT)),
> > +             (STD_MODE_1080I << STD_MODE_SHIFT),
> > +             0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> > +             V4L2_STD_1080I_30,
> > +     },
> > +     {
> > +             ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
> > +             (~(STD_MODE_MASK << STD_MODE_SHIFT)),
> > +             (STD_MODE_1080I_25fps << STD_MODE_SHIFT),
> > +             0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> > +             V4L2_STD_1080I_25,
> > +     },
> > +     {
> > +             ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
> > +             (~(STD_MODE_MASK << STD_MODE_SHIFT)),
> > +             (STD_MODE_525P << STD_MODE_SHIFT),
> > +             0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> > +             V4L2_STD_480P_60,
> > +     },
> > +     {
> > +             ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
> > +             (~(STD_MODE_MASK << STD_MODE_SHIFT)),
> > +             (STD_MODE_625P << STD_MODE_SHIFT),
> > +             0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> > +             V4L2_STD_576P_50,
> > +     },
> > +     {
> > +             ADV7343_SD_MODE_REG1, &reg80, SD_INPUT_MODE,
> (~(SD_STD_MASK)),
> > +             SD_STD_NTSC, 0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F,
> 0x21,
> > +             V4L2_STD_NTSC,
> > +     },
> > +     {
> > +             ADV7343_SD_MODE_REG1, &reg80, SD_INPUT_MODE,
> (~(SD_STD_MASK)),
> > +             SD_STD_PAL_BDGHI, 0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0,
> 0x8F,
> > +             0x21, V4L2_STD_PAL,
> > +     },
> > +     {
> > +             ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
> > +             (~(STD_MODE_MASK << STD_MODE_SHIFT)),
> > +             (STD_MODE_1080P_24 << STD_MODE_SHIFT),
> > +             0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> > +             V4L2_STD_1080P_24,
> > +     },
> > +     {
> > +             ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
> > +             (~(STD_MODE_MASK << STD_MODE_SHIFT)),
> > +             (STD_MODE_1080P_25 << STD_MODE_SHIFT),
> > +             0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> > +             V4L2_STD_1080I_25,
> > +     },
> > +     {
> > +             ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
> > +             (~(STD_MODE_MASK << STD_MODE_SHIFT)),
> > +             (STD_MODE_1080P_30 << STD_MODE_SHIFT),
> > +             0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
> > +             V4L2_STD_1080P_30,
> > +     },
> > +};
> > +
> > +static struct adv7343_config
> adv7343_configuration[ADV7343_NUM_CHANNELS] = {
> > +     {
> > +             .no_of_outputs = ADV7343_MAX_NO_OUTPUTS,
> > +             .output[0] = {
> > +                     .output_type    = ADV7343_COMPOSITE_ID,
> > +                     .output_name    = VID_ENC_OUTPUT_COMPOSITE,
> > +                     .no_of_standard = ADV7343_COMPOSITE_NUM_STD,
> > +                     .def_std        = V4L2_STD_NTSC,
> > +                     .std_info       = (struct adv7343_std_info *)
> > +
> &adv7343_composite_std_info,
> > +                     .power_val      =
> ADV7343_COMPOSITE_POWER_VALUE,
> > +             },
> > +             .output[1] = {
> > +                     .output_type    = ADV7343_COMPONENT_ID,
> > +                     .output_name    = VID_ENC_OUTPUT_COMPONENT,
> > +                     .no_of_standard = ADV7343_COMPONENT_NUM_STD,
> > +                     .def_std        = V4L2_STD_720P_60,
> > +                     .std_info       = (struct adv7343_std_info *)
> > +
> &adv7343_component_std_info,
> > +                     .power_val      =
> ADV7343_COMPONENT_POWER_VALUE,
> > +             },
> > +             .output[2] = {
> > +                     .output_type    = ADV7343_SVIDEO_ID,
> > +                     .output_name    = VID_ENC_OUTPUT_SVIDEO,
> > +                     .no_of_standard = ADV7343_SVIDEO_NUM_STD,
> > +                     .def_std        = V4L2_STD_NTSC,
> > +                     .std_info       = (struct adv7343_std_info *)
> > +
> &adv7343_composite_std_info,
> > +                     .power_val      = ADV7343_SVIDEO_POWER_VALUE
> > +             },
> > +     },
> > +};
> > +
> > +static struct adv7343_channel
> adv7343_channel_info[ADV7343_NUM_CHANNELS] = {
> > +     {
> > +             .current_output = ADV7343_COMPOSITE_ID,
> > +             .mode_info      = V4L2_STD_NTSC,
> > +     }
> > +};
>
> These arrays might be mixing user and device level concepts. See my
> comments
> on that at the end. It's not clear to me whether it is indeed the case,
> I'll
> leave it to you to take action if it's wrong and I'll review it the
> next
> round.
>
> > +static int adv7343_setstd(struct v4l2_subdev *sd, v4l2_std_id std)
> > +{
> > +     int err = 0;
> > +     int i = 0;
> > +     struct adv7343_std_info *std_info;
> > +     int output_idx;
> > +     u8 reg, val;
> > +
> > +     int ch_id = (to_state(sd))->ch_id;
> > +     struct adv7343_config *config = &adv7343_configuration[ch_id];
> > +
> > +     v4l2_dbg(1, debug, sd, "Start of adv7343_setstd..\n");
> > +     output_idx = adv7343_channel_info[ch_id].current_output;
> > +     v4l2_dbg(1, debug, sd, "the output index is %d\n", output_idx);
> > +
> > +     for (i = 0; i < config->output[output_idx].no_of_standard; i++)
> {
> > +             std_info = &config->output[output_idx].std_info[i];
> > +             if (std_info->stdid ==  std)
> > +                     break;
> > +     }
> > +
> > +     if (i == config->output[output_idx].no_of_standard) {
> > +             v4l2_err(sd, "Invalid id...\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     val = *(config->output[output_idx].std_info[i].value);
> > +     val &= config->output[output_idx].std_info[i].standard_val2;
> > +     val |= config->output[output_idx].std_info[i].standard_val3;
> > +     err |= adv7343_write(sd,
> > +             config-
> >output[output_idx].std_info[i].set_std_register, val);
> > +     if (err < 0) {
> > +             v4l2_err(sd, "Set standard failed\n");
> > +             return err;
> > +     }
> > +     *(config->output[output_idx].std_info[i].value) = val;
> > +
> > +     val = reg01;
> > +     val &= (~((u8) INPUT_MODE_MASK));
> > +     val |= config->output[output_idx].std_info[i].outputmode_val1;
> > +     err |= adv7343_write(sd, ADV7343_MODE_SELECT_REG, val);
> > +     if (err < 0) {
> > +             v4l2_err(sd, "Set standard failed\n");
> > +             return err;
> > +     }
> > +     reg01 = val;
> > +
> > +     /* Store the standard in global object of adv7343 */
> > +     adv7343_channel_info[ch_id].mode_info =
> > +                             config-
> >output[output_idx].std_info[i].stdid;
> > +
> > +     reg = config->output[output_idx].std_info[i].fsc0_reg;
> > +     val = config->output[output_idx].std_info[i].fsc0_val;
> > +     err |= adv7343_write(sd, reg, val);
> > +
> > +     reg = config->output[output_idx].std_info[i].fsc1_reg;
> > +     val = config->output[output_idx].std_info[i].fsc1_val;
> > +     err |= adv7343_write(sd, reg, val);
> > +
> > +     reg = config->output[output_idx].std_info[i].fsc2_reg;
> > +     val = config->output[output_idx].std_info[i].fsc2_val;
> > +     err |= adv7343_write(sd, reg, val);
> > +
> > +     reg = config->output[output_idx].std_info[i].fsc3_reg;
> > +     val = config->output[output_idx].std_info[i].fsc3_val;
> > +     err |= adv7343_write(sd, reg, val);
> > +
> > +     val = reg80;
> > +
> > +     if (std == V4L2_STD_NTSC)
> > +             val &= 0x03;
> > +     else if (std ==  V4L2_STD_PAL)
> > +             val |= 0x04;
> > +
> > +     err |= adv7343_write(sd, ADV7343_SD_MODE_REG1, val);
> > +
> > +     reg80 = val;
> > +
> > +     v4l2_dbg(1, debug, sd, "</adv7343_setstd>\n");
> > +
> > +     return err;
> > +}
> > +
> > +/* Following function is used to set output format in ADV7343
> device. The index
> > +   of the output format is  passed as the argument to this function.
> */
> > +static int adv7343_setoutput(struct v4l2_subdev *sd, int
> output_type)
> > +{
> > +     unsigned char val;
> > +     int i;
> > +     int index;
> > +     int err = 0;
> > +     int ch_id = (to_state(sd))->ch_id;
> > +     struct adv7343_config *config = &adv7343_configuration[ch_id];
> > +
> > +     v4l2_dbg(1, debug, sd, "Start of set output function.\n");
> > +
> > +     for (i = 0; i < config->no_of_outputs; i++) {
> > +             if (output_type == config->output[i].output_type)
> > +                     break;
> > +     }
> > +
> > +     if (i == config->no_of_outputs) {
> > +             v4l2_err(sd, "Invalid output\n");
> > +             return -EINVAL;
> > +     }
> > +     index = i;
> > +
> > +     /* Enable Appropriate DAC */
> > +     val = reg00;
> > +     val &= 0x03;
> > +     val |= config->output[index].power_val;
> > +     err = adv7343_write(sd, ADV7343_POWER_MODE_REG, val);
> > +
> > +     reg00 = val;
> > +
> > +     /* Enable YUV output */
> > +     val = reg02;
> > +     val |= YUV_OUTPUT_SELECT;
> > +     err |= adv7343_write(sd, ADV7343_MODE_REG0, val);
> > +
> > +     reg02 = val;
> > +
> > +     /* configure SD DAC Output 2 and SD DAC Output 1 bit to zero */
> > +     val = reg82;
> > +     val &= (SD_DAC_1_DI & SD_DAC_2_DI);
> > +     err |= adv7343_write(sd, ADV7343_SD_MODE_REG2, val);
> > +     if (err < 0)
> > +             return err;
> > +     reg82 = val;
> > +
> > +     /* configure ED/HD Color DAC Swap and ED/HD RGB Input Enable
> bit to
> > +      * zero */
> > +     val = reg35;
> > +     val &= (HD_RGB_INPUT_DI & HD_DAC_SWAP_DI);
> > +     err |= adv7343_write(sd, ADV7343_HD_MODE_REG6, val);
> > +     if (err < 0)
> > +             return err;
> > +     reg35 = val;
> > +
> > +     adv7343_channel_info[ch_id].current_output = index;
> > +     adv7343_channel_info[ch_id].mode_info = config-
> >output[index].def_std;
> > +
> > +     err |= adv7343_setstd(sd,
> adv7343_channel_info[ch_id].mode_info);
> > +
> > +     if (err < 0)
> > +             return err;
> > +
> > +     v4l2_dbg(1, debug, sd, "</adv7343_setoutput>\n");
> > +
> > +     return err;
> > +}
> > +
> > +static int adv7343_getstd(struct v4l2_subdev *sd,
> > +                             v4l2_std_id *stdid)
> > +{
> > +     int err = 0;
> > +     int ch_id =  (to_state(sd))->ch_id;
> > +     int output_idx;
> > +
> > +     v4l2_dbg(1, debug, sd, "In getstd function.\n");
> > +     output_idx = adv7343_channel_info[ch_id].current_output;
> > +
> > +     *stdid = adv7343_channel_info[ch_id].mode_info;
> > +
> > +     return err;
> > +}
> > +
> > +static int adv7343_log_status(struct v4l2_subdev *sd)
> > +{
> > +     struct adv7343_state *state = to_state(sd);
> > +
> > +     v4l2_info(sd, "Standard: %llu\n", state->std);
> > +     v4l2_info(sd, "Output: %s\n", (state->output) ? "COMPOSITE" :
> > +                                                     "COMPONENT");
> > +     v4l2_info(sd, "Channel: %d\n", state->ch_id);
> > +
> > +     return 0;
> > +}
> > +
> > +static int adv7343_initialize(struct v4l2_subdev *sd)
> > +{
> > +     int err = 0;
> > +     int ch_id = (to_state(sd))->ch_id; /* for now */
> > +     err |= adv7343_write(sd, ADV7343_SOFT_RESET,
> > +                             ADV7343_SOFT_RESET_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_POWER_MODE_REG,
> > +                             ADV7343_POWER_MODE_REG_DEFAULT);
> > +
> > +     err |= adv7343_write(sd, ADV7343_HD_MODE_REG1,
> > +                             ADV7343_HD_MODE_REG1_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_HD_MODE_REG2,
> > +                              ADV7343_HD_MODE_REG2_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_HD_MODE_REG3,
> > +                             ADV7343_HD_MODE_REG3_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_HD_MODE_REG4,
> > +                             ADV7343_HD_MODE_REG4_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_HD_MODE_REG5,
> > +                             ADV7343_HD_MODE_REG5_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_HD_MODE_REG6,
> > +                             ADV7343_HD_MODE_REG6_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_HD_MODE_REG7,
> > +                             ADV7343_HD_MODE_REG7_DEFAULT);
> > +
> > +     err |= adv7343_write(sd, ADV7343_SD_MODE_REG1,
> > +                             ADV7343_SD_MODE_REG1_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_SD_MODE_REG2,
> > +                             ADV7343_SD_MODE_REG2_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_SD_MODE_REG3,
> > +                             ADV7343_SD_MODE_REG3_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_SD_MODE_REG4,
> > +                             ADV7343_SD_MODE_REG4_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_SD_MODE_REG5,
> > +                             ADV7343_SD_MODE_REG5_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_SD_MODE_REG6,
> > +                             ADV7343_SD_MODE_REG6_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_SD_MODE_REG7,
> > +                             ADV7343_SD_MODE_REG7_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_SD_MODE_REG8,
> > +                             ADV7343_SD_MODE_REG8_DEFAULT);
> > +
> > +     err |= adv7343_write(sd, ADV7343_SD_HUE_REG,
> > +                             ADV7343_SD_HUE_REG_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_SD_CGMS_WSS0,
> > +                             ADV7343_SD_CGMS_WSS0_DEFAULT);
> > +     err |= adv7343_write(sd, ADV7343_SD_BRIGHTNESS_WSS,
> > +                             ADV7343_SD_BRIGHTNESS_WSS_DEFAULT);
> > +
> > +     if (err < 0) {
> > +             v4l2_err(sd, "Error in initializing!\n");
> > +             err = -EINVAL;
> > +             goto adv7343_init_exit;
> > +     }
> > +
> > +     adv7343_channel_info[ch_id].current_output = 0;
> > +     adv7343_channel_info[ch_id].mode_info =
> > +                 adv7343_composite_std_info[0].stdid;
> > +
> > +     /* Configure for default video standard */
> > +     err |= adv7343_setoutput(sd, adv7343_configuration[ch_id].
> > +                                     output[0].output_type);
> > +     err |= adv7343_setstd(sd, adv7343_configuration[ch_id].
> > +                                     output[0].def_std);
> > +
> > +     if (err < 0) {
> > +             err = -EINVAL;
> > +             goto adv7343_init_exit;
> > +     }
> > +
> > +     v4l2_dbg(1, debug, sd, "</adv7343_initialize>\n");
> > +
> > +adv7343_init_exit:
> > +     return err;
> > +}
> > +
> > +static int adv7343_reset(struct v4l2_subdev *sd, u32 val)
> > +{
> > +     v4l2_dbg(1, debug, sd, "Reset\n");
> > +     return adv7343_initialize(sd);
> > +}
> > +
> > +static int adv7343_init(struct v4l2_subdev *sd, u32 val)
> > +{
> > +     struct adv7343_state *state = to_state(sd);
> > +     if (!state->initialized) {
> > +             state->initialized = 1;
> > +             v4l2_dbg(1, debug, sd, "Initializing Encoder\n");
> > +             return adv7343_initialize(sd);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int adv7343_queryctrl(struct v4l2_subdev *sd, struct
> v4l2_queryctrl *qc)
> > +{
> > +     switch (qc->id) {
> > +     case V4L2_CID_BRIGHTNESS:
> > +     case V4L2_CID_HUE:
> > +             return v4l2_ctrl_query_fill_std(qc);
>
> Use v4l2_ctrl_query_fill instead. The fill_std function has been
> removed
> (that was a bad idea).
>
OK.

> > +     default:
> > +             break;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int adv7343_s_ctrl(struct v4l2_subdev *sd, struct
> v4l2_control *ctrl)
> > +{
> > +     struct adv7343_state *state = to_state(sd);
> > +     int err = 0;
> > +
> > +     switch (ctrl->id) {
> > +     case V4L2_CID_BRIGHTNESS:
> > +             if (ctrl->value < 0 || ctrl->value > 127) {
> > +                     v4l2_err(sd, "invalid brightness setting %d\n",
> > +                                     ctrl->value);
>
> Recommend that you use v4l2_dbg for this instead.
OK.

>
> > +                     return -ERANGE;
> > +             }
> > +
> > +             state->bright = ctrl->value;
> > +             err = adv7343_write(sd, ADV7343_SD_BRIGHTNESS_WSS,
> > +                                     state->bright);
> > +             break;
> > +
> > +     case V4L2_CID_HUE:
> > +             if (ctrl->value < 0 || ctrl->value > 255) {
> > +                     v4l2_err(sd, "invalid hue settings %d\n", ctrl-
> >value);
> > +                     return -ERANGE;
> > +             }
> > +
> > +             state->hue = ctrl->value;
> > +             err = adv7343_write(sd, ADV7343_SD_HUE_REG, state-
> >hue);
> > +             break;
> > +
> > +     case V4L2_CID_GAIN:
>
> Why is there no V4L2_CID_GAIN case in queryctrl above?
>
> > +             if (ctrl->value < 0 || ctrl->value > 255) {
> > +                     v4l2_err(sd, "invalid gain settings %d\n",
> ctrl->value);
> > +                     return -ERANGE;
> > +             }
> > +
> > +             if ((ctrl->value > POSITIVE_GAIN_MAX) &&
> > +                     (ctrl->value < NEGATIVE_GAIN_MIN)) {
> > +                     v4l2_err(sd, "gain settings not within \
> > +                                     the specified range\n");
> > +                     return -ERANGE;
> > +             } else {
> > +                     state->gain = ctrl->value;
> > +                     err = adv7343_write(sd,
> ADV7343_DAC2_OUTPUT_LEVEL,
> > +                                     state->gain);
> > +             }
> > +             break;
> > +
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +     if (err < 0)
> > +             v4l2_err(sd, "Failed tp set the encoder controls\n");
>
> Typo: tp -> to
Will be corrected.

>
> > +
> > +     return 0;
> > +}
> > +
> > +static int adv7343_g_ctrl(struct v4l2_subdev *sd, struct
> v4l2_control *ctrl)
> > +{
> > +     struct adv7343_state *state = to_state(sd);
> > +
> > +     switch (ctrl->id) {
> > +     case V4L2_CID_BRIGHTNESS:
> > +             ctrl->value = state->bright;
> > +             break;
> > +
> > +     case V4L2_CID_HUE:
> > +             ctrl->value = state->hue;
> > +             break;
> > +
> > +     case V4L2_CID_GAIN:
> > +             ctrl->value = state->gain;
> > +             break;
> > +
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int adv7343_g_chip_ident(struct v4l2_subdev *sd,
> > +                             struct v4l2_dbg_chip_ident *chip)
> > +{
> > +     struct adv7343_state *state = to_state(sd);
> > +     struct i2c_client *client = v4l2_get_subdevdata(sd);
> > +
> > +     return v4l2_chip_ident_i2c_client(client, chip, state->ident,
> 0);
> > +}
> > +
> > +static long adv7343_ioctl(struct v4l2_subdev *sd, unsigned cmd, void
> *arg)
> > +{
> > +     int err = 0;
> > +     v4l2_dbg(1, debug, sd, "ioctl\n");
> > +     switch (cmd) {
> > +     case ENCODER_GET_MODE:
> > +             err = adv7343_getstd(sd, (v4l2_std_id *)arg);
> > +             break;
>
> Not a good idea. The v4l-dvb master repository adds a .querystd
> callback that
> you should use instead. It's a recent addition that appears in 2.6.30.
>
OK, will do the necessary modifications.

> > +
> > +     default:
> > +             break;
> > +     }
> > +
> > +     return err;
> > +}
> > +
> > +static const struct v4l2_subdev_core_ops adv7343_core_ops = {
> > +     .log_status     = adv7343_log_status,
> > +     .g_chip_ident   = adv7343_g_chip_ident,
> > +     .g_ctrl         = adv7343_g_ctrl,
> > +     .s_ctrl         = adv7343_s_ctrl,
> > +     .queryctrl      = adv7343_queryctrl,
> > +     .reset          = adv7343_reset,
>
> Do you really need a reset?
I don't think it is needed.

>
> > +     .init           = adv7343_init,
>
> Do you really need an init? Better to init in the probe() function.
> See also my comments about init in the ths7303 review.
>
OK.

> > +     .ioctl          = adv7343_ioctl,
> > +};
> > +
> > +static int adv7343_s_std_output(struct v4l2_subdev *sd, v4l2_std_id
> std)
> > +{
> > +     struct adv7343_state *state = to_state(sd);
> > +     int err = 0;
> > +
> > +     if (state->std == std)
> > +             return 0;
> > +
> > +     err = adv7343_setstd(sd, std);
> > +
> > +     if (!err)
> > +             state->std = std;
> > +     else
> > +             v4l2_err(sd, "s_std failed\n");
> > +
> > +     return err;
> > +}
> > +
> > +static int adv7343_s_routing(struct v4l2_subdev *sd,
> > +                             const struct v4l2_routing *route)
> > +{
> > +     struct adv7343_state *state = to_state(sd);
> > +
> > +     int err = 0;
> > +
> > +     if (state->output == route->output)
> > +             return 0;
> > +
> > +     err = adv7343_setoutput(sd, route->output);
> > +     if (err)
> > +             v4l2_err(sd, "Error setting output\n");
> > +     else
> > +             state->output = route->output;
> > +
> > +     return err;
> > +}
> > +
> > +static const struct v4l2_subdev_video_ops adv7343_video_ops = {
> > +     .s_std_output   = adv7343_s_std_output,
> > +     .s_routing      = adv7343_s_routing,
> > +};
> > +
> > +static const struct v4l2_subdev_ops adv7343_ops = {
> > +     .core   = &adv7343_core_ops,
> > +     .video  = &adv7343_video_ops,
> > +};
> > +
> > +static int adv7343_command(struct i2c_client *client, unsigned cmd,
> void *arg)
> > +{
> > +     return v4l2_subdev_command(i2c_get_clientdata(client), cmd,
> arg);
> > +}
>
> Not needed, see my ths7303 review.
OK.

>
> > +
> > +static int adv7343_probe(struct i2c_client *client,
> > +                             const struct i2c_device_id *id)
> > +{
> > +     struct adv7343_state *state;
> > +
> > +     if (!i2c_check_functionality(client->adapter,
> I2C_FUNC_SMBUS_BYTE_DATA))
> > +             return -ENODEV;
> > +
> > +     v4l2_info(client, "chip found @ 0x%x (%s)\n",
> > +                     client->addr << 1, client->adapter->name);
>
> use v4l_info here.
>
OK.

> > +
> > +     state = kzalloc(sizeof(struct adv7343_state), GFP_KERNEL);
> > +     if (state == NULL)
> > +             return -ENOMEM;
> > +
> > +     state->client   = client;
> > +     state->enable   = 1;
> > +     state->ch_id    = 0;
> > +     state->output   = -1;
> > +     state->initialized = 0;
> > +     state->ident = 0;
> > +     v4l2_i2c_subdev_init(&state->sd, client, &adv7343_ops);
> > +     v4l2_dbg(1, debug, client, "Registered the encoder\n");
>
> This v4l2_dbg doesn't add anything useful that v4l_info didn't already
> say.
>
OK.

> > +
> > +     return 0;
> > +}
> > +
> > +static int adv7343_remove(struct i2c_client *client)
> > +{
> > +     struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > +
> > +     v4l2_device_unregister_subdev(sd);
> > +     kfree(to_state(sd));
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct i2c_device_id adv7343_id[] = {
> > +     {ADV7343_NAME, 0},
> > +     {},
> > +};
> > +
> > +MODULE_DEVICE_TABLE(i2c, adv7343_id);
> > +
> > +static struct v4l2_i2c_driver_data v4l2_i2c_data = {
> > +     .name           = ADV7343_NAME,
> > +     .command        = adv7343_command,
> > +     .probe          = adv7343_probe,
> > +     .remove         = adv7343_remove,
> > +     .legacy_class   = I2C_CLASS_TV_ANALOG | I2C_CLASS_TV_DIGITAL,
> > +     .id_table       = adv7343_id,
> > +};
> > +
> > +static __init int init_adv7343(void)
> > +{
> > +     return 0;
> > +}
> > +
> > +static __exit void exit_adv7343(void)
> > +{
> > +
> > +}
>
> See my comments in the ths7303 review.
>
> > +
> > +module_init(init_adv7343);
> > +module_exit(exit_adv7343);
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/media/adv7343.h b/include/media/adv7343.h
> > new file mode 100644
> > index 0000000..b7da4a6
> > --- /dev/null
> > +++ b/include/media/adv7343.h
> > @@ -0,0 +1,373 @@
> > +/*
> > + * ADV7343 header file
> > + *
> > + * Copyright (C) 2009 Texas Instruments Incorporated -
> http://www.ti.com/
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation version 2.
> > + *
> > + * This program is distributed .as is. WITHOUT ANY WARRANTY of any
> > + * kind, whether express or implied; without even the implied
> warranty
> > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#ifndef ADV7343_H
> > +#define ADV7343_H
> > +
> > +#ifdef __KERNEL__
>
> Not needed. Files in include/media are internal to the kernel only.
>
> I'm not doing an in-depth review of this header. You first need to
> split
> it up in two parts: one adv7343_regs.h header that is included by the
> adv7343.c
> driver and is in the same directory as that driver: that header
> contains all
> the internal datastructures and defines (register addresses, etc.) that
> it uses.
>
> The media/adv7343.h contains only the defines that other drivers need
> to
> interface with the adv7343 driver: in particular the routing
> information
> for the s_routing ops goes there.
>
> Once that's done I'll review it again.
>
OK, I understood your point, will modify the header files and its
location.

> One thing to keep in mind at all times is that you do not mix user-
> level
> input/output descriptions and device-level input/output descriptions.
>
> E.g. say that pin X is connected to the "Composite 1" output connector
> on the board. The i2c driver has no knowledge of that. All it needs to
> know is that pin X is the output pin. The platform driver, however, is
> the one that has access to the actual board layout and should be
> the driver that associates the "Composite 1" connector (a user-level
> description) with pin X in the i2c device (a device-level description).
>
> The s_routing operations only deal with the device-level. How you
> encode
> the pins is purely device specific and goes into the media/adv7343.h
> header. Depending on the i2c device you may have to specify both input
> and
> output pins. E.g. the input pin would be the pin that the videoport
> sends
> its data to, and the output pin is the pin that is connector to the
> physical
> connector on the board.
>
> Note that when the conversion of the legacy drivers is finished I want
> to
> modify the s_routing function a bit: instead of using the awkward
> v4l2_routing struct you can just specify the input and output fields
> directly
> as arguments. And I think that I'll also add a config argument since
> sometimes you need to specify addition configuration information. The
> media/saa7115.h is an example of that where the output field is abused
> to
> pass config information.
>
The main purpose of the arrays is to set the relevant registers of the encoder
based on the format being displayed. The adv7343_std_info structure is set of
register values to be modified based on the standard.  The adv7343_config
structure is a collection of output types supported and the corresponding register
settings. The above registers are specific to the encoder only and the platform driver
is not aware of these arrays. Hope I have understood your above explanation properly.
I am of the opinion that the arrays are not mixing user and device level concepts.

Thanks,
Chaithrika

> Regards,
>
>         Hans
>
> > +
> > +/* Kernel Header files */
> > +#include <linux/i2c.h>
> > +#include <linux/device.h>
> > +#include <linux/videodev2.h>
> > +
> > +#endif                               /* __KERNEL__ */
> > +
> > +#define ADV7343_NAME "adv7343"
> > +
> > +/** VID_ENC_NAME_MAX_CHARS
> > + *
> > + * Description:
> > + * MAX characters in the name.
> > + */
> > +#define VID_ENC_NAME_MAX_CHARS       30
> > +
> > +/*
> > + * constant strings for output names.
> > + */
> > +#define VID_ENC_OUTPUT_COMPOSITE     "COMPOSITE"
> > +#define VID_ENC_OUTPUT_SVIDEO                "SVIDEO"
> > +#define      VID_ENC_OUTPUT_COMPONENT        "COMPONENT"
> > +
> > +/* Internal IOCTL defines */
> > +#define ENCODER_GET_MODE     _IOR('e', BASE_VIDIOC_PRIVATE + 1,
> v4l2_std_id*)
> > +
> > +/* Macros */
> > +#define ADV7343_MAX_GAMMA_COEFFS     (10)    /* Maximum Gamma
> Coefficients */
> > +
> > +#ifdef __KERNEL__
> > +
> > +#define ADV7343_COMPOSITE_OUTPUT_NAME        "COMPOSITE"
> > +#define ADV7343_COMPONENT_OUTPUT_NAME        "COMPONENT"
> > +#define ADV7343_SVIDEO_OUTPUT_NAME   "SVIDEO"
> > +#define ADV7343_COMPOSITE_ID (0)
> > +#define ADV7343_COMPONENT_ID (1)
> > +#define ADV7343_SVIDEO_ID    (2)
> > +
> > +#define ADV7343_COMPOSITE_NO_CONTROLS        (3)
> > +#define ADV7343_COMPONENT_NO_CONTROLS        (1)
> > +#define ADV7343_SVIDEO_NO_CONTROLS   (3)
> > +
> > +#define ADV7343_NUM_CHANNELS         (1)
> > +
> > +/* encoder standard related strctures */
> > +#define ADV7343_MAX_NO_OUTPUTS               (3)
> > +#define ADV7343_COMPOSITE_NUM_STD    (2)
> > +#define ADV7343_COMPONENT_NUM_STD    (7+3+3)
> > +#define ADV7343_SVIDEO_NUM_STD               (2)
> > +#define ADV7343_MAX_NO_CONTROLS              (3)
> > +#define ADV7343_VBI_NUM_SERVICES     (3)
> > +
> > +struct adv7343_std_info {
> > +     unsigned char set_std_register;
> > +     unsigned char *value;
> > +     u32 outputmode_val1;
> > +     u32 standard_val2;
> > +     u32 standard_val3;
> > +     u8 fsc0_reg, fsc0_val;
> > +     u8 fsc1_reg, fsc1_val;
> > +     u8 fsc2_reg, fsc2_val;
> > +     u8 fsc3_reg, fsc3_val;
> > +     v4l2_std_id stdid;
> > +};
> > +
> > +struct adv7343_config {
> > +     int no_of_outputs;
> > +     struct {
> > +             unsigned char power_val;
> > +             int output_type;
> > +             char output_name[VID_ENC_NAME_MAX_CHARS];
> > +             int no_of_standard;
> > +             v4l2_std_id def_std;
> > +             struct adv7343_std_info *std_info;
> > +     } output[ADV7343_MAX_NO_OUTPUTS];
> > +     unsigned short services_set;
> > +     u8 num_services;
> > +};
> > +
> > +struct adv7343_channel {
> > +     u8 current_output;
> > +     v4l2_std_id mode_info;
> > +     unsigned short services_set;
> > +};
> > +
> > +#define ADV7343_VALID_FEATURE_VAL(val)       ((ADV7343_DISABLE ==
> (val)) || \
> > +                                     (ADV7343_ENABLE == (val)))
> > +#define ADV7343_VALID_GAMMA_CURVE(val)       ((ADV7343_GAMMA_CURVE_A
> == (val)) || \
> > +                                     (ADV7343_GAMMA_CURVE_B ==
> (val)))
> > +
> > +/* Register offset macros */
> > +#define ADV7343_POWER_MODE_REG               (0x00)
> > +#define ADV7343_MODE_SELECT_REG              (0x01)
> > +#define ADV7343_MODE_REG0            (0x02)
> > +#define ADV7343_CSC_MATRIX0          (0x03)
> > +#define ADV7343_CSC_MATRIX1          (0x04)
> > +#define ADV7343_CSC_MATRIX2          (0x05)
> > +#define ADV7343_CSC_MATRIX3          (0x06)
> > +#define ADV7343_CSC_MATRIX4          (0x07)
> > +#define ADV7343_CSC_MATRIX5          (0x08)
> > +#define ADV7343_CSC_MATRIX6          (0x09)
> > +#define ADV7343_DAC1_OUTPUT_LEVEL    (0x0a)
> > +#define ADV7343_DAC2_OUTPUT_LEVEL    (0x0b)
> > +#define ADV7343_DAC_POWER_MODE               (0x0d)
> > +#define ADV7343_CABLE_DETECTION              (0x10)
> > +#define ADV7343_SBUS_READ            (0x12)
> > +#define ADV7343_YBUS_READ            (0x13)
> > +#define ADV7343_CBUS_READ            (0x14)
> > +#define ADV7343_CONTROL_READ         (0x16)
> > +#define ADV7343_SOFT_RESET           (0x17)
> > +#define ADV7343_HD_MODE_REG1         (0x30)
> > +#define ADV7343_HD_MODE_REG2         (0x31)
> > +#define ADV7343_HD_MODE_REG3         (0x32)
> > +#define ADV7343_HD_MODE_REG4         (0x33)
> > +#define ADV7343_HD_MODE_REG5         (0x34)
> > +#define ADV7343_HD_MODE_REG6         (0x35)
> > +#define ADV7343_HD_Y_LEVEL           (0x36)
> > +#define ADV7343_HD_CR_LEVEL          (0x37)
> > +#define ADV7343_HD_CB_LEVEL          (0x38)
> > +#define ADV7343_HD_MODE_REG7         (0x39)
> > +#define ADV7343_HD_SHARPNESS_FLTR_GAIN       (0x40)
> > +#define ADV7343_HD_CGMS_DATA_0               (0x41)
> > +#define ADV7343_HD_CGMS_DATA_1               (0x42)
> > +#define ADV7343_HD_CGMS_DATA_2               (0x43)
> > +#define ADV7343_HD_GAMMA_A0          (0x44)
> > +#define ADV7343_HD_GAMMA_A1          (0x45)
> > +#define ADV7343_HD_GAMMA_A2          (0x46)
> > +#define ADV7343_HD_GAMMA_A3          (0x47)
> > +#define ADV7343_HD_GAMMA_A4          (0x48)
> > +#define ADV7343_HD_GAMMA_A5          (0x49)
> > +#define ADV7343_HD_GAMMA_A6          (0x4a)
> > +#define ADV7343_HD_GAMMA_A7          (0x4b)
> > +#define ADV7343_HD_GAMMA_A8          (0x4c)
> > +#define ADV7343_HD_GAMMA_A9          (0x4d)
> > +#define ADV7343_HD_GAMMA_B0          (0x4E)
> > +#define ADV7343_HD_GAMMA_B1          (0x4F)
> > +#define ADV7343_HD_GAMMA_B2          (0x50)
> > +#define ADV7343_HD_GAMMA_B3          (0x51)
> > +#define ADV7343_HD_GAMMA_B4          (0x52)
> > +#define ADV7343_HD_GAMMA_B5          (0x53)
> > +#define ADV7343_HD_GAMMA_B6          (0x54)
> > +#define ADV7343_HD_GAMMA_B7          (0x55)
> > +#define ADV7343_HD_GAMMA_B8          (0x56)
> > +#define ADV7343_HD_GAMMA_B9          (0x57)
> > +#define ADV7343_HD_ADPT_FLTR_GAIN1   (0x58)
> > +#define ADV7343_HD_ADPT_FLTR_GAIN2   (0x59)
> > +#define ADV7343_HD_ADPT_FLTR_GAIN3   (0x5a)
> > +#define ADV7343_HD_ADPT_FLTR_THRLDA  (0x5b)
> > +#define ADV7343_HD_ADPT_FLTR_THRLDB  (0x5c)
> > +#define ADV7343_HD_ADPT_FLTR_THRLDC  (0x5d)
> > +#define ADV7343_HD_CGMS_B0           (0x5E)
> > +#define ADV7343_HD_CGMS_B1           (0x5F)
> > +#define ADV7343_HD_CGMS_B2           (0x60)
> > +#define ADV7343_HD_CGMS_B3           (0x61)
> > +#define ADV7343_HD_CGMS_B4           (0x62)
> > +#define ADV7343_HD_CGMS_B5           (0x63)
> > +#define ADV7343_HD_CGMS_B6           (0x64)
> > +#define ADV7343_HD_CGMS_B7           (0x65)
> > +#define ADV7343_HD_CGMS_B8           (0x66)
> > +#define ADV7343_HD_CGMS_B9           (0x67)
> > +#define ADV7343_HD_CGMS_B10          (0x68)
> > +#define ADV7343_HD_CGMS_B11          (0x69)
> > +#define ADV7343_HD_CGMS_B12          (0x6A)
> > +#define ADV7343_HD_CGMS_B13          (0x6B)
> > +#define ADV7343_HD_CGMS_B14          (0x6C)
> > +#define ADV7343_HD_CGMS_B15          (0x6D)
> > +#define ADV7343_HD_CGMS_B16          (0x6E)
> > +
> > +#define ADV7343_SD_MODE_REG1         (0x80)
> > +#define ADV7343_SD_MODE_REG2         (0x82)
> > +#define ADV7343_SD_MODE_REG3         (0x83)
> > +#define ADV7343_SD_MODE_REG4         (0x84)
> > +#define ADV7343_SD_MODE_REG5         (0x86)
> > +#define ADV7343_SD_MODE_REG6         (0x87)
> > +#define ADV7343_SD_MODE_REG7         (0x88)
> > +#define ADV7343_SD_MODE_REG8         (0x89)
> > +#define ADV7343_SD_TIMING_REG0               (0x8A)
> > +#define ADV7343_SD_TIMING_REG1               (0x8B)
> > +#define ADV7343_SD_FSC_REG0          (0x8C)
> > +#define ADV7343_SD_FSC_REG1          (0x8D)
> > +#define ADV7343_SD_FSC_REG2          (0x8E)
> > +#define ADV7343_SD_FSC_REG3          (0x8F)
> > +#define ADV7343_SD_FSC_PHASE         (0x90)
> > +#define ADV7343_SD_CLOSE_CAPTION_EVEN0       (0x91)
> > +#define ADV7343_SD_CLOSE_CAPTION_EVEN1       (0x92)
> > +#define ADV7343_SD_CLOSE_CAPTION_ODD0        (0x93)
> > +#define ADV7343_SD_CLOSE_CAPTION_ODD1        (0x94)
> > +#define ADV7343_SD_PEDESTAL_REG0     (0x95)
> > +#define ADV7343_SD_PEDESTAL_REG1     (0x96)
> > +#define ADV7343_SD_PEDESTAL_REG2     (0x97)
> > +#define ADV7343_SD_PEDESTAL_REG3     (0x98)
> > +#define ADV7343_SD_CGMS_WSS0         (0x99)
> > +#define ADV7343_SD_CGMS_WSS1         (0x9A)
> > +#define ADV7343_SD_CGMS_WSS2         (0x9B)
> > +
> > +#define ADV7343_SD_SCALE_LSB         (0x9C)
> > +#define ADV7343_SD_Y_SCALE           (0x9D)
> > +#define ADV7343_SD_CB_SCALE          (0x9E)
> > +#define ADV7343_SD_CR_SCALE          (0x9F)
> > +
> > +#define ADV7343_SD_HUE_REG           (0xA0)
> > +#define ADV7343_SD_BRIGHTNESS_WSS    (0xA1)
> > +#define ADV7343_SD_LUMA_SSAF         (0xA2)
> > +#define ADV7343_SD_DNR0                      (0xA3)
> > +#define ADV7343_SD_DNR1                      (0xA4)
> > +#define ADV7343_SD_DNR2                      (0xA5)
> > +
> > +#define ADV7343_SD_GAMMA_A0          (0xA6)
> > +#define ADV7343_SD_GAMMA_A1          (0xA7)
> > +#define ADV7343_SD_GAMMA_A2          (0xA8)
> > +#define ADV7343_SD_GAMMA_A3          (0xA9)
> > +#define ADV7343_SD_GAMMA_A4          (0xAA)
> > +#define ADV7343_SD_GAMMA_A5          (0xAB)
> > +#define ADV7343_SD_GAMMA_A6          (0xAC)
> > +#define ADV7343_SD_GAMMA_A7          (0xAD)
> > +#define ADV7343_SD_GAMMA_A8          (0xAE)
> > +#define ADV7343_SD_GAMMA_A9          (0xAF)
> > +#define ADV7343_SD_GAMMA_B0          (0xB0)
> > +#define ADV7343_SD_GAMMA_B1          (0xB1)
> > +#define ADV7343_SD_GAMMA_B2          (0xB2)
> > +#define ADV7343_SD_GAMMA_B3          (0xB3)
> > +#define ADV7343_SD_GAMMA_B4          (0xB4)
> > +#define ADV7343_SD_GAMMA_B5          (0xB5)
> > +#define ADV7343_SD_GAMMA_B6          (0xB6)
> > +#define ADV7343_SD_GAMMA_B7          (0xB7)
> > +#define ADV7343_SD_GAMMA_B8          (0xB8)
> > +#define ADV7343_SD_GAMMA_B9          (0xB9)
> > +#define ADV7343_SD_BRIGHTNESS_DETECT (0xBA)
> > +#define ADV7343_FIELD_COUNT_REG              (0xBB)
> > +#define ADV7343_10_BIT_INPUT         (0x7C)
> > +
> > +/* Default values for the registers */
> > +#define ADV7343_POWER_MODE_REG_DEFAULT               (0x10)
> > +#define ADV7343_HD_MODE_REG1_DEFAULT         (0x3C)  /* Changed
> Default
> > +                                                        720p EAVSAV
> code*/
> > +#define ADV7343_HD_MODE_REG2_DEFAULT         (0x01)  /* Changed
> Pixel data
> > +                                                        valid */
> > +#define ADV7343_HD_MODE_REG3_DEFAULT         (0x00)  /* Color delay
> 0 clks */
> > +#define ADV7343_HD_MODE_REG4_DEFAULT         (0xE8)  /* Changed */
> > +#define ADV7343_HD_MODE_REG5_DEFAULT         (0x08)
> > +#define ADV7343_HD_MODE_REG6_DEFAULT         (0x00)
> > +#define ADV7343_HD_MODE_REG7_DEFAULT         (0x00)
> > +
> > +#define ADV7343_SD_MODE_REG1_DEFAULT         (0x00)
> > +#define ADV7343_SD_MODE_REG2_DEFAULT         (0xC9)
> > +#define ADV7343_SD_MODE_REG3_DEFAULT         (0x10)
> > +#define ADV7343_SD_MODE_REG4_DEFAULT         (0x01)
> > +#define ADV7343_SD_MODE_REG5_DEFAULT         (0x02)
> > +#define ADV7343_SD_MODE_REG6_DEFAULT         (0x0C)
> > +#define ADV7343_SD_MODE_REG7_DEFAULT         (0x04)
> > +#define ADV7343_SD_MODE_REG8_DEFAULT         (0x00)
> > +#define ADV7343_SOFT_RESET_DEFAULT           (0x02)
> > +#define ADV7343_COMPOSITE_POWER_VALUE                (0x80)
> > +#define ADV7343_COMPONENT_POWER_VALUE                (0x1C)
> > +#define ADV7343_SVIDEO_POWER_VALUE           (0x60)
> > +#define ADV7343_SD_HUE_REG_DEFAULT           (127)
> > +#define ADV7343_SD_BRIGHTNESS_WSS_DEFAULT    (0x03)
> > +#define ADV7343_SD_CGMS_WSS0_DEFAULT         (0x10)
> > +
> > +/* Macros for the Mode Select Register */
> > +#ifdef GENERATE_MASK
> > +#undef GENERATE_MASK
> > +#endif
> > +#define GENERATE_MASK(bits, pos)     ((((0xFF) << (8 - bits)) >> \
> > +                                     (8 - bits)) << pos)
> > +
> > +/* Bit masks for Mode Select Register */
> > +#define INPUT_MODE_MASK                      (0x70)
> > +#define SD_INPUT_MODE                        (0x00)
> > +#define HD_720P_INPUT_MODE           (0x10)
> > +#define HD_1080I_INPUT_MODE          (0x10)
> > +
> > +/* Bit masks for Mode Register 0 */
> > +#define TEST_PATTERN_BLACK_BAR_EN    (0x04)
> > +#define YUV_OUTPUT_SELECT            (0x20)
> > +#define RGB_OUTPUT_SELECT            (0xDF)
> > +
> > +/* Bit masks for DAC output levels */
> > +#define DAC_OUTPUT_LEVEL_MASK                (0xFF)
> > +#define POSITIVE_GAIN_MAX            (0x40)
> > +#define POSITIVE_GAIN_MIN            (0x00)
> > +#define NEGATIVE_GAIN_MAX            (0xFF)
> > +#define NEGATIVE_GAIN_MIN            (0xC0)
> > +
> > +/* Bit masks for soft reset register */
> > +#define SOFT_RESET                   (0x02)
> > +
> > +/* Bit masks for HD Mode Register 1 */
> > +#define OUTPUT_STD_MASK                      (0x03)
> > +#define OUTPUT_STD_SHIFT             (0)
> > +#define OUTPUT_STD_EIA0_2            (0x00)
> > +#define OUTPUT_STD_EIA0_1            (0x01)
> > +#define OUTPUT_STD_FULL                      (0x02)
> > +#define EMBEDDED_SYNC                        (0x04)
> > +#define EXTERNAL_SYNC                        (0xFB)
> > +#define STD_MODE_SHIFT                       (3)
> > +#define STD_MODE_MASK                        (0x1F)
> > +#define STD_MODE_720P                        (0x05)
> > +#define STD_MODE_720P_25             (0x08)
> > +#define STD_MODE_720P_30             (0x07)
> > +#define STD_MODE_720P_50             (0x06)
> > +#define STD_MODE_1080I                       (0x0D)
> > +#define STD_MODE_1080I_25fps         (0x0E)
> > +#define STD_MODE_1080P_24            (0x12)
> > +#define STD_MODE_1080P_25            (0x10)
> > +#define STD_MODE_1080P_30            (0x0F)
> > +#define STD_MODE_525P                        (0x00)
> > +#define STD_MODE_625P                        (0x03)
> > +
> > +/* Bit masks for SD Mode Register 1 */
> > +#define SD_STD_MASK                  (0x03)
> > +#define SD_STD_NTSC                  (0x00)
> > +#define SD_STD_PAL_BDGHI             (0x01)
> > +#define SD_STD_PAL_M                 (0x02)
> > +#define SD_STD_PAL_N                 (0x03)
> > +#define SD_LUMA_FLTR_MASK            (0x7)
> > +#define SD_LUMA_FLTR_SHIFT           (0x2)
> > +#define SD_CHROMA_FLTR_MASK          (0x7)
> > +#define SD_CHROMA_FLTR_SHIFT         (0x5)
> > +
> > +/* Bit masks for SD Mode Register 2 */
> > +#define SD_PBPR_SSAF_EN                      (0x01)
> > +#define SD_PBPR_SSAF_DI                      (0xFE)
> > +#define SD_DAC_1_DI                  (0xFD)
> > +#define SD_DAC_2_DI                  (0xFB)
> > +#define SD_PEDESTAL_EN                       (0x08)
> > +#define SD_PEDESTAL_DI                       (0xF7)
> > +#define SD_SQUARE_PIXEL_EN           (0x10)
> > +#define SD_SQUARE_PIXEL_DI           (0xEF)
> > +#define SD_PIXEL_DATA_VALID          (0x40)
> > +#define SD_ACTIVE_EDGE_EN            (0x80)
> > +#define SD_ACTIVE_EDGE_DI            (0x7F)
> > +
> > +/* Bit masks for HD Mode Register 6 */
> > +#define HD_RGB_INPUT_EN                      (0x02)
> > +#define HD_RGB_INPUT_DI                      (0xFD)
> > +#define HD_PBPR_SYNC_EN                      (0x04)
> > +#define HD_PBPR_SYNC_DI                      (0xFB)
> > +#define HD_DAC_SWAP_EN                       (0x08)
> > +#define HD_DAC_SWAP_DI                       (0xF7)
> > +#define HD_GAMMA_CURVE_A             (0xEF)
> > +#define HD_GAMMA_CURVE_B             (0x10)
> > +#define HD_GAMMA_EN                  (0x20)
> > +#define HD_GAMMA_DI                  (0xDF)
> > +#define HD_ADPT_FLTR_MODEB           (0x40)
> > +#define HD_ADPT_FLTR_MODEA           (0xBF)
> > +#define HD_ADPT_FLTR_EN                      (0x80)
> > +#define HD_ADPT_FLTR_DI                      (0x7F)
> > +
> > +#endif                               /* End of #ifdef __KERNEL__ */
> > +
> > +#endif                               /* End of #ifndef ADV7343_H */
>
>
>
> --
> Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 27f6397..16019e9 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -426,6 +426,15 @@  config VIDEO_ADV7175
 	  To compile this driver as a module, choose M here: the
 	  module will be called adv7175.
 
+config VIDEO_ADV7343
+        tristate "ADV7343 video encoder"
+        depends on I2C
+        help
+          Support for Analog Devices I2C bus based ADV7343 encoder.
+
+          To compile this driver as a module, choose M here: the
+          module will be called adv7473.
+
 comment "Video improvement chips"
 
 config VIDEO_UPD64031A
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index 99b448e..7f9fc62 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -49,6 +49,7 @@  obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
 obj-$(CONFIG_VIDEO_SAA7191) += saa7191.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
+obj-$(CONFIG_VIDEO_ADV7343) += adv7343.o
 obj-$(CONFIG_VIDEO_VPX3220) += vpx3220.o
 obj-$(CONFIG_VIDEO_BT819) += bt819.o
 obj-$(CONFIG_VIDEO_BT856) += bt856.o
diff --git a/drivers/media/video/adv7343.c b/drivers/media/video/adv7343.c
new file mode 100644
index 0000000..c912f1d
--- /dev/null
+++ b/drivers/media/video/adv7343.c
@@ -0,0 +1,730 @@ 
+/*
+ * adv7343 - ADV7343 Video Encoder Driver
+ *
+ * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed .as is. WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/ctype.h>
+#include <linux/i2c.h>
+#include <linux/device.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/videodev2.h>
+#include <linux/uaccess.h>
+#include <linux/version.h>
+
+#include <media/adv7343.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-i2c-drv.h>
+
+static int debug = 2;
+static unsigned char reg00 = 0x80;	/* Power Mode register */
+static unsigned char reg01 = 0x00;	/* MODE_SELECT_REG */
+static unsigned char reg02 = 0x20;	/* MODE_REG0 */
+static unsigned char reg30 = 0x3C;	/* HD_MODE_REG1 */
+static unsigned char reg35 = 0x00;	/* HD_MODE_REG6 */
+static unsigned char reg80 = ADV7343_SD_MODE_REG1_DEFAULT; /* SD_MODE_REG1 */
+static unsigned char reg82 = ADV7343_SD_MODE_REG2_DEFAULT; /* SD_MODE_REG2 */
+
+struct adv7343_state {
+	struct i2c_client *client;
+	u32 ident;
+	struct v4l2_subdev sd;
+	v4l2_std_id std;
+	int output;
+	int enable;
+	int bright;
+	int hue;
+	int gain;
+	int initialized;
+	int video_enable;
+	int ch_id;
+};
+
+static inline struct adv7343_state *to_state(struct v4l2_subdev *sd)
+{
+	return container_of(sd, struct adv7343_state, sd);
+}
+
+static inline int adv7343_write(struct v4l2_subdev *sd, u8 reg, u8 value)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+	return i2c_smbus_write_byte_data(client, reg, value);
+}
+
+struct adv7343_std_info
+	adv7343_composite_std_info[ADV7343_COMPOSITE_NUM_STD] = {
+	{
+		ADV7343_SD_MODE_REG1, &reg80, SD_INPUT_MODE, (~(SD_STD_MASK)),
+		SD_STD_NTSC, 0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
+		V4L2_STD_NTSC,
+	},
+	{
+		ADV7343_SD_MODE_REG1, &reg80, SD_INPUT_MODE, (~(SD_STD_MASK)),
+		SD_STD_PAL_BDGHI, 0x8C, 0xCB, 0x8D, 0x8A, 0x8E, 0x09, 0x8F,
+		0x2A, V4L2_STD_PAL,
+	},
+};
+
+struct adv7343_std_info
+	adv7343_component_std_info[ADV7343_COMPONENT_NUM_STD] = {
+	{
+		ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
+		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
+		(STD_MODE_720P << STD_MODE_SHIFT),
+		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
+		V4L2_STD_720P_60,
+	},
+	{
+		ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
+		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
+		(STD_MODE_720P_25 << STD_MODE_SHIFT),
+		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
+		V4L2_STD_720P_25,
+	},
+	{
+		ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
+		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
+		(STD_MODE_720P_30 << STD_MODE_SHIFT),
+		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
+		V4L2_STD_720P_30,
+	},
+	{
+		ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
+		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
+		(STD_MODE_720P_50 << STD_MODE_SHIFT),
+		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
+		V4L2_STD_720P_50,
+	},
+	{
+		ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
+		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
+		(STD_MODE_1080I << STD_MODE_SHIFT),
+		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
+		V4L2_STD_1080I_30,
+	},
+	{
+		ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
+		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
+		(STD_MODE_1080I_25fps << STD_MODE_SHIFT),
+		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
+		V4L2_STD_1080I_25,
+	},
+	{
+		ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
+		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
+		(STD_MODE_525P << STD_MODE_SHIFT),
+		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
+		V4L2_STD_480P_60,
+	},
+	{
+		ADV7343_HD_MODE_REG1, &reg30, HD_720P_INPUT_MODE,
+		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
+		(STD_MODE_625P << STD_MODE_SHIFT),
+		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
+		V4L2_STD_576P_50,
+	},
+	{
+		ADV7343_SD_MODE_REG1, &reg80, SD_INPUT_MODE, (~(SD_STD_MASK)),
+		SD_STD_NTSC, 0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
+		V4L2_STD_NTSC,
+	},
+	{
+		ADV7343_SD_MODE_REG1, &reg80, SD_INPUT_MODE, (~(SD_STD_MASK)),
+		SD_STD_PAL_BDGHI, 0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F,
+		0x21, V4L2_STD_PAL,
+	},
+	{
+		ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
+		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
+		(STD_MODE_1080P_24 << STD_MODE_SHIFT),
+		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
+		V4L2_STD_1080P_24,
+	},
+	{
+		ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
+		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
+		(STD_MODE_1080P_25 << STD_MODE_SHIFT),
+		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
+		V4L2_STD_1080I_25,
+	},
+	{
+		ADV7343_HD_MODE_REG1, &reg30, HD_1080I_INPUT_MODE,
+		(~(STD_MODE_MASK << STD_MODE_SHIFT)),
+		(STD_MODE_1080P_30 << STD_MODE_SHIFT),
+		0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, 0x21,
+		V4L2_STD_1080P_30,
+	},
+};
+
+static struct adv7343_config adv7343_configuration[ADV7343_NUM_CHANNELS] = {
+	{
+		.no_of_outputs = ADV7343_MAX_NO_OUTPUTS,
+		.output[0] = {
+			.output_type	= ADV7343_COMPOSITE_ID,
+			.output_name	= VID_ENC_OUTPUT_COMPOSITE,
+			.no_of_standard	= ADV7343_COMPOSITE_NUM_STD,
+			.def_std	= V4L2_STD_NTSC,
+			.std_info	= (struct adv7343_std_info *)
+						&adv7343_composite_std_info,
+			.power_val	= ADV7343_COMPOSITE_POWER_VALUE,
+		},
+		.output[1] = {
+			.output_type	= ADV7343_COMPONENT_ID,
+			.output_name	= VID_ENC_OUTPUT_COMPONENT,
+			.no_of_standard	= ADV7343_COMPONENT_NUM_STD,
+			.def_std	= V4L2_STD_720P_60,
+			.std_info	= (struct adv7343_std_info *)
+						&adv7343_component_std_info,
+			.power_val	= ADV7343_COMPONENT_POWER_VALUE,
+		},
+		.output[2] = {
+			.output_type	= ADV7343_SVIDEO_ID,
+			.output_name	= VID_ENC_OUTPUT_SVIDEO,
+			.no_of_standard	= ADV7343_SVIDEO_NUM_STD,
+			.def_std	= V4L2_STD_NTSC,
+			.std_info	= (struct adv7343_std_info *)
+						&adv7343_composite_std_info,
+			.power_val	= ADV7343_SVIDEO_POWER_VALUE
+		},
+	},
+};
+
+static struct adv7343_channel adv7343_channel_info[ADV7343_NUM_CHANNELS] = {
+	{
+		.current_output	= ADV7343_COMPOSITE_ID,
+		.mode_info	= V4L2_STD_NTSC,
+	}
+};
+
+static int adv7343_setstd(struct v4l2_subdev *sd, v4l2_std_id std)
+{
+	int err = 0;
+	int i = 0;
+	struct adv7343_std_info *std_info;
+	int output_idx;
+	u8 reg, val;
+
+	int ch_id = (to_state(sd))->ch_id;
+	struct adv7343_config *config = &adv7343_configuration[ch_id];
+
+	v4l2_dbg(1, debug, sd, "Start of adv7343_setstd..\n");
+	output_idx = adv7343_channel_info[ch_id].current_output;
+	v4l2_dbg(1, debug, sd, "the output index is %d\n", output_idx);
+
+	for (i = 0; i < config->output[output_idx].no_of_standard; i++) {
+		std_info = &config->output[output_idx].std_info[i];
+		if (std_info->stdid ==  std)
+			break;
+	}
+
+	if (i == config->output[output_idx].no_of_standard) {
+		v4l2_err(sd, "Invalid id...\n");
+		return -EINVAL;
+	}
+
+	val = *(config->output[output_idx].std_info[i].value);
+	val &= config->output[output_idx].std_info[i].standard_val2;
+	val |= config->output[output_idx].std_info[i].standard_val3;
+	err |= adv7343_write(sd,
+		config->output[output_idx].std_info[i].set_std_register, val);
+	if (err < 0) {
+		v4l2_err(sd, "Set standard failed\n");
+		return err;
+	}
+	*(config->output[output_idx].std_info[i].value) = val;
+
+	val = reg01;
+	val &= (~((u8) INPUT_MODE_MASK));
+	val |= config->output[output_idx].std_info[i].outputmode_val1;
+	err |= adv7343_write(sd, ADV7343_MODE_SELECT_REG, val);
+	if (err < 0) {
+		v4l2_err(sd, "Set standard failed\n");
+		return err;
+	}
+	reg01 = val;
+
+	/* Store the standard in global object of adv7343 */
+	adv7343_channel_info[ch_id].mode_info =
+				config->output[output_idx].std_info[i].stdid;
+
+	reg = config->output[output_idx].std_info[i].fsc0_reg;
+	val = config->output[output_idx].std_info[i].fsc0_val;
+	err |= adv7343_write(sd, reg, val);
+
+	reg = config->output[output_idx].std_info[i].fsc1_reg;
+	val = config->output[output_idx].std_info[i].fsc1_val;
+	err |= adv7343_write(sd, reg, val);
+
+	reg = config->output[output_idx].std_info[i].fsc2_reg;
+	val = config->output[output_idx].std_info[i].fsc2_val;
+	err |= adv7343_write(sd, reg, val);
+
+	reg = config->output[output_idx].std_info[i].fsc3_reg;
+	val = config->output[output_idx].std_info[i].fsc3_val;
+	err |= adv7343_write(sd, reg, val);
+
+	val = reg80;
+
+	if (std == V4L2_STD_NTSC)
+		val &= 0x03;
+	else if (std ==  V4L2_STD_PAL)
+		val |= 0x04;
+
+	err |= adv7343_write(sd, ADV7343_SD_MODE_REG1, val);
+
+	reg80 = val;
+
+	v4l2_dbg(1, debug, sd, "</adv7343_setstd>\n");
+
+	return err;
+}
+
+/* Following function is used to set output format in ADV7343 device. The index
+   of the output format is  passed as the argument to this function. */
+static int adv7343_setoutput(struct v4l2_subdev *sd, int output_type)
+{
+	unsigned char val;
+	int i;
+	int index;
+	int err = 0;
+	int ch_id = (to_state(sd))->ch_id;
+	struct adv7343_config *config = &adv7343_configuration[ch_id];
+
+	v4l2_dbg(1, debug, sd, "Start of set output function.\n");
+
+	for (i = 0; i < config->no_of_outputs; i++) {
+		if (output_type == config->output[i].output_type)
+			break;
+	}
+
+	if (i == config->no_of_outputs) {
+		v4l2_err(sd, "Invalid output\n");
+		return -EINVAL;
+	}
+	index = i;
+
+	/* Enable Appropriate DAC */
+	val = reg00;
+	val &= 0x03;
+	val |= config->output[index].power_val;
+	err = adv7343_write(sd, ADV7343_POWER_MODE_REG, val);
+
+	reg00 = val;
+
+	/* Enable YUV output */
+	val = reg02;
+	val |= YUV_OUTPUT_SELECT;
+	err |= adv7343_write(sd, ADV7343_MODE_REG0, val);
+
+	reg02 = val;
+
+	/* configure SD DAC Output 2 and SD DAC Output 1 bit to zero */
+	val = reg82;
+	val &= (SD_DAC_1_DI & SD_DAC_2_DI);
+	err |= adv7343_write(sd, ADV7343_SD_MODE_REG2, val);
+	if (err < 0)
+		return err;
+	reg82 = val;
+
+	/* configure ED/HD Color DAC Swap and ED/HD RGB Input Enable bit to
+	 * zero */
+	val = reg35;
+	val &= (HD_RGB_INPUT_DI & HD_DAC_SWAP_DI);
+	err |= adv7343_write(sd, ADV7343_HD_MODE_REG6, val);
+	if (err < 0)
+		return err;
+	reg35 = val;
+
+	adv7343_channel_info[ch_id].current_output = index;
+	adv7343_channel_info[ch_id].mode_info = config->output[index].def_std;
+
+	err |= adv7343_setstd(sd, adv7343_channel_info[ch_id].mode_info);
+
+	if (err < 0)
+		return err;
+
+	v4l2_dbg(1, debug, sd, "</adv7343_setoutput>\n");
+
+	return err;
+}
+
+static int adv7343_getstd(struct v4l2_subdev *sd,
+				v4l2_std_id *stdid)
+{
+	int err = 0;
+	int ch_id =  (to_state(sd))->ch_id;
+	int output_idx;
+
+	v4l2_dbg(1, debug, sd, "In getstd function.\n");
+	output_idx = adv7343_channel_info[ch_id].current_output;
+
+	*stdid = adv7343_channel_info[ch_id].mode_info;
+
+	return err;
+}
+
+static int adv7343_log_status(struct v4l2_subdev *sd)
+{
+	struct adv7343_state *state = to_state(sd);
+
+	v4l2_info(sd, "Standard: %llu\n", state->std);
+	v4l2_info(sd, "Output: %s\n", (state->output) ? "COMPOSITE" :
+							"COMPONENT");
+	v4l2_info(sd, "Channel: %d\n", state->ch_id);
+
+	return 0;
+}
+
+static int adv7343_initialize(struct v4l2_subdev *sd)
+{
+	int err = 0;
+	int ch_id = (to_state(sd))->ch_id; /* for now */
+	err |= adv7343_write(sd, ADV7343_SOFT_RESET,
+				ADV7343_SOFT_RESET_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_POWER_MODE_REG,
+				ADV7343_POWER_MODE_REG_DEFAULT);
+
+	err |= adv7343_write(sd, ADV7343_HD_MODE_REG1,
+				ADV7343_HD_MODE_REG1_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_HD_MODE_REG2,
+				 ADV7343_HD_MODE_REG2_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_HD_MODE_REG3,
+				ADV7343_HD_MODE_REG3_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_HD_MODE_REG4,
+				ADV7343_HD_MODE_REG4_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_HD_MODE_REG5,
+				ADV7343_HD_MODE_REG5_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_HD_MODE_REG6,
+				ADV7343_HD_MODE_REG6_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_HD_MODE_REG7,
+				ADV7343_HD_MODE_REG7_DEFAULT);
+
+	err |= adv7343_write(sd, ADV7343_SD_MODE_REG1,
+				ADV7343_SD_MODE_REG1_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_SD_MODE_REG2,
+				ADV7343_SD_MODE_REG2_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_SD_MODE_REG3,
+				ADV7343_SD_MODE_REG3_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_SD_MODE_REG4,
+				ADV7343_SD_MODE_REG4_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_SD_MODE_REG5,
+				ADV7343_SD_MODE_REG5_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_SD_MODE_REG6,
+				ADV7343_SD_MODE_REG6_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_SD_MODE_REG7,
+				ADV7343_SD_MODE_REG7_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_SD_MODE_REG8,
+				ADV7343_SD_MODE_REG8_DEFAULT);
+
+	err |= adv7343_write(sd, ADV7343_SD_HUE_REG,
+				ADV7343_SD_HUE_REG_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_SD_CGMS_WSS0,
+				ADV7343_SD_CGMS_WSS0_DEFAULT);
+	err |= adv7343_write(sd, ADV7343_SD_BRIGHTNESS_WSS,
+				ADV7343_SD_BRIGHTNESS_WSS_DEFAULT);
+
+	if (err < 0) {
+		v4l2_err(sd, "Error in initializing!\n");
+		err = -EINVAL;
+		goto adv7343_init_exit;
+	}
+
+	adv7343_channel_info[ch_id].current_output = 0;
+	adv7343_channel_info[ch_id].mode_info =
+		    adv7343_composite_std_info[0].stdid;
+
+	/* Configure for default video standard */
+	err |= adv7343_setoutput(sd, adv7343_configuration[ch_id].
+					output[0].output_type);
+	err |= adv7343_setstd(sd, adv7343_configuration[ch_id].
+					output[0].def_std);
+
+	if (err < 0) {
+		err = -EINVAL;
+		goto adv7343_init_exit;
+	}
+
+	v4l2_dbg(1, debug, sd, "</adv7343_initialize>\n");
+
+adv7343_init_exit:
+	return err;
+}
+
+static int adv7343_reset(struct v4l2_subdev *sd, u32 val)
+{
+	v4l2_dbg(1, debug, sd, "Reset\n");
+	return adv7343_initialize(sd);
+}
+
+static int adv7343_init(struct v4l2_subdev *sd, u32 val)
+{
+	struct adv7343_state *state = to_state(sd);
+	if (!state->initialized) {
+		state->initialized = 1;
+		v4l2_dbg(1, debug, sd, "Initializing Encoder\n");
+		return adv7343_initialize(sd);
+	}
+
+	return 0;
+}
+
+static int adv7343_queryctrl(struct v4l2_subdev *sd, struct v4l2_queryctrl *qc)
+{
+	switch (qc->id) {
+	case V4L2_CID_BRIGHTNESS:
+	case V4L2_CID_HUE:
+		return v4l2_ctrl_query_fill_std(qc);
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int adv7343_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+{
+	struct adv7343_state *state = to_state(sd);
+	int err = 0;
+
+	switch (ctrl->id) {
+	case V4L2_CID_BRIGHTNESS:
+		if (ctrl->value < 0 || ctrl->value > 127) {
+			v4l2_err(sd, "invalid brightness setting %d\n",
+					ctrl->value);
+			return -ERANGE;
+		}
+
+		state->bright = ctrl->value;
+		err = adv7343_write(sd, ADV7343_SD_BRIGHTNESS_WSS,
+					state->bright);
+		break;
+
+	case V4L2_CID_HUE:
+		if (ctrl->value < 0 || ctrl->value > 255) {
+			v4l2_err(sd, "invalid hue settings %d\n", ctrl->value);
+			return -ERANGE;
+		}
+
+		state->hue = ctrl->value;
+		err = adv7343_write(sd, ADV7343_SD_HUE_REG, state->hue);
+		break;
+
+	case V4L2_CID_GAIN:
+		if (ctrl->value < 0 || ctrl->value > 255) {
+			v4l2_err(sd, "invalid gain settings %d\n", ctrl->value);
+			return -ERANGE;
+		}
+
+		if ((ctrl->value > POSITIVE_GAIN_MAX) &&
+			(ctrl->value < NEGATIVE_GAIN_MIN)) {
+			v4l2_err(sd, "gain settings not within \
+					the specified range\n");
+			return -ERANGE;
+		} else {
+			state->gain = ctrl->value;
+			err = adv7343_write(sd, ADV7343_DAC2_OUTPUT_LEVEL,
+					state->gain);
+		}
+		break;
+
+	default:
+		return -EINVAL;
+	}
+	if (err < 0)
+		v4l2_err(sd, "Failed tp set the encoder controls\n");
+
+	return 0;
+}
+
+static int adv7343_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+{
+	struct adv7343_state *state = to_state(sd);
+
+	switch (ctrl->id) {
+	case V4L2_CID_BRIGHTNESS:
+		ctrl->value = state->bright;
+		break;
+
+	case V4L2_CID_HUE:
+		ctrl->value = state->hue;
+		break;
+
+	case V4L2_CID_GAIN:
+		ctrl->value = state->gain;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int adv7343_g_chip_ident(struct v4l2_subdev *sd,
+				struct v4l2_dbg_chip_ident *chip)
+{
+	struct adv7343_state *state = to_state(sd);
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+	return v4l2_chip_ident_i2c_client(client, chip, state->ident, 0);
+}
+
+static long adv7343_ioctl(struct v4l2_subdev *sd, unsigned cmd, void *arg)
+{
+	int err = 0;
+	v4l2_dbg(1, debug, sd, "ioctl\n");
+	switch (cmd) {
+	case ENCODER_GET_MODE:
+		err = adv7343_getstd(sd, (v4l2_std_id *)arg);
+		break;
+
+	default:
+		break;
+	}
+
+	return err;
+}
+
+static const struct v4l2_subdev_core_ops adv7343_core_ops = {
+	.log_status	= adv7343_log_status,
+	.g_chip_ident	= adv7343_g_chip_ident,
+	.g_ctrl		= adv7343_g_ctrl,
+	.s_ctrl		= adv7343_s_ctrl,
+	.queryctrl	= adv7343_queryctrl,
+	.reset		= adv7343_reset,
+	.init		= adv7343_init,
+	.ioctl		= adv7343_ioctl,
+};
+
+static int adv7343_s_std_output(struct v4l2_subdev *sd, v4l2_std_id std)
+{
+	struct adv7343_state *state = to_state(sd);
+	int err = 0;
+
+	if (state->std == std)
+		return 0;
+
+	err = adv7343_setstd(sd, std);
+
+	if (!err)
+		state->std = std;
+	else
+		v4l2_err(sd, "s_std failed\n");
+
+	return err;
+}
+
+static int adv7343_s_routing(struct v4l2_subdev *sd,
+				const struct v4l2_routing *route)
+{
+	struct adv7343_state *state = to_state(sd);
+
+	int err = 0;
+
+	if (state->output == route->output)
+		return 0;
+
+	err = adv7343_setoutput(sd, route->output);
+	if (err)
+		v4l2_err(sd, "Error setting output\n");
+	else
+		state->output = route->output;
+
+	return err;
+}
+
+static const struct v4l2_subdev_video_ops adv7343_video_ops = {
+	.s_std_output	= adv7343_s_std_output,
+	.s_routing	= adv7343_s_routing,
+};
+
+static const struct v4l2_subdev_ops adv7343_ops = {
+	.core	= &adv7343_core_ops,
+	.video	= &adv7343_video_ops,
+};
+
+static int adv7343_command(struct i2c_client *client, unsigned cmd, void *arg)
+{
+	return v4l2_subdev_command(i2c_get_clientdata(client), cmd, arg);
+}
+
+static int adv7343_probe(struct i2c_client *client,
+				const struct i2c_device_id *id)
+{
+	struct adv7343_state *state;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		return -ENODEV;
+
+	v4l2_info(client, "chip found @ 0x%x (%s)\n",
+			client->addr << 1, client->adapter->name);
+
+	state = kzalloc(sizeof(struct adv7343_state), GFP_KERNEL);
+	if (state == NULL)
+		return -ENOMEM;
+
+	state->client	= client;
+	state->enable	= 1;
+	state->ch_id	= 0;
+	state->output	= -1;
+	state->initialized = 0;
+	state->ident = 0;
+	v4l2_i2c_subdev_init(&state->sd, client, &adv7343_ops);
+	v4l2_dbg(1, debug, client, "Registered the encoder\n");
+
+	return 0;
+}
+
+static int adv7343_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+
+	v4l2_device_unregister_subdev(sd);
+	kfree(to_state(sd));
+
+	return 0;
+}
+
+static const struct i2c_device_id adv7343_id[] = {
+	{ADV7343_NAME, 0},
+	{},
+};
+
+MODULE_DEVICE_TABLE(i2c, adv7343_id);
+
+static struct v4l2_i2c_driver_data v4l2_i2c_data = {
+	.name		= ADV7343_NAME,
+	.command	= adv7343_command,
+	.probe		= adv7343_probe,
+	.remove		= adv7343_remove,
+	.legacy_class	= I2C_CLASS_TV_ANALOG | I2C_CLASS_TV_DIGITAL,
+	.id_table	= adv7343_id,
+};
+
+static __init int init_adv7343(void)
+{
+	return 0;
+}
+
+static __exit void exit_adv7343(void)
+{
+
+}
+
+module_init(init_adv7343);
+module_exit(exit_adv7343);
+MODULE_LICENSE("GPL");
diff --git a/include/media/adv7343.h b/include/media/adv7343.h
new file mode 100644
index 0000000..b7da4a6
--- /dev/null
+++ b/include/media/adv7343.h
@@ -0,0 +1,373 @@ 
+/*
+ * ADV7343 header file
+ *
+ * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed .as is. WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef ADV7343_H
+#define ADV7343_H
+
+#ifdef __KERNEL__
+
+/* Kernel Header files */
+#include <linux/i2c.h>
+#include <linux/device.h>
+#include <linux/videodev2.h>
+
+#endif				/* __KERNEL__ */
+
+#define ADV7343_NAME	"adv7343"
+
+/** VID_ENC_NAME_MAX_CHARS
+ *
+ * Description:
+ * MAX characters in the name.
+ */
+#define VID_ENC_NAME_MAX_CHARS	30
+
+/*
+ * constant strings for output names.
+ */
+#define VID_ENC_OUTPUT_COMPOSITE	"COMPOSITE"
+#define VID_ENC_OUTPUT_SVIDEO		"SVIDEO"
+#define	VID_ENC_OUTPUT_COMPONENT	"COMPONENT"
+
+/* Internal IOCTL defines */
+#define ENCODER_GET_MODE	_IOR('e', BASE_VIDIOC_PRIVATE + 1, v4l2_std_id*)
+
+/* Macros */
+#define ADV7343_MAX_GAMMA_COEFFS	(10)	/* Maximum Gamma Coefficients */
+
+#ifdef __KERNEL__
+
+#define ADV7343_COMPOSITE_OUTPUT_NAME	"COMPOSITE"
+#define ADV7343_COMPONENT_OUTPUT_NAME	"COMPONENT"
+#define ADV7343_SVIDEO_OUTPUT_NAME	"SVIDEO"
+
+#define ADV7343_COMPOSITE_ID	(0)
+#define ADV7343_COMPONENT_ID	(1)
+#define ADV7343_SVIDEO_ID	(2)
+
+#define ADV7343_COMPOSITE_NO_CONTROLS	(3)
+#define ADV7343_COMPONENT_NO_CONTROLS	(1)
+#define ADV7343_SVIDEO_NO_CONTROLS	(3)
+
+#define ADV7343_NUM_CHANNELS		(1)
+
+/* encoder standard related strctures */
+#define ADV7343_MAX_NO_OUTPUTS		(3)
+#define ADV7343_COMPOSITE_NUM_STD	(2)
+#define ADV7343_COMPONENT_NUM_STD	(7+3+3)
+#define ADV7343_SVIDEO_NUM_STD		(2)
+#define ADV7343_MAX_NO_CONTROLS		(3)
+#define ADV7343_VBI_NUM_SERVICES	(3)
+
+struct adv7343_std_info {
+	unsigned char set_std_register;
+	unsigned char *value;
+	u32 outputmode_val1;
+	u32 standard_val2;
+	u32 standard_val3;
+	u8 fsc0_reg, fsc0_val;
+	u8 fsc1_reg, fsc1_val;
+	u8 fsc2_reg, fsc2_val;
+	u8 fsc3_reg, fsc3_val;
+	v4l2_std_id stdid;
+};
+
+struct adv7343_config {
+	int no_of_outputs;
+	struct {
+		unsigned char power_val;
+		int output_type;
+		char output_name[VID_ENC_NAME_MAX_CHARS];
+		int no_of_standard;
+		v4l2_std_id def_std;
+		struct adv7343_std_info *std_info;
+	} output[ADV7343_MAX_NO_OUTPUTS];
+	unsigned short services_set;
+	u8 num_services;
+};
+
+struct adv7343_channel {
+	u8 current_output;
+	v4l2_std_id mode_info;
+	unsigned short services_set;
+};
+
+#define ADV7343_VALID_FEATURE_VAL(val)	((ADV7343_DISABLE == (val)) || \
+					(ADV7343_ENABLE == (val)))
+#define ADV7343_VALID_GAMMA_CURVE(val)	((ADV7343_GAMMA_CURVE_A == (val)) || \
+					(ADV7343_GAMMA_CURVE_B == (val)))
+
+/* Register offset macros */
+#define ADV7343_POWER_MODE_REG		(0x00)
+#define ADV7343_MODE_SELECT_REG		(0x01)
+#define ADV7343_MODE_REG0		(0x02)
+#define ADV7343_CSC_MATRIX0		(0x03)
+#define ADV7343_CSC_MATRIX1		(0x04)
+#define ADV7343_CSC_MATRIX2		(0x05)
+#define ADV7343_CSC_MATRIX3		(0x06)
+#define ADV7343_CSC_MATRIX4		(0x07)
+#define ADV7343_CSC_MATRIX5		(0x08)
+#define ADV7343_CSC_MATRIX6		(0x09)
+#define ADV7343_DAC1_OUTPUT_LEVEL	(0x0a)
+#define ADV7343_DAC2_OUTPUT_LEVEL	(0x0b)
+#define ADV7343_DAC_POWER_MODE		(0x0d)
+#define ADV7343_CABLE_DETECTION		(0x10)
+#define ADV7343_SBUS_READ		(0x12)
+#define ADV7343_YBUS_READ		(0x13)
+#define ADV7343_CBUS_READ		(0x14)
+#define ADV7343_CONTROL_READ		(0x16)
+#define ADV7343_SOFT_RESET		(0x17)
+#define ADV7343_HD_MODE_REG1		(0x30)
+#define ADV7343_HD_MODE_REG2		(0x31)
+#define ADV7343_HD_MODE_REG3		(0x32)
+#define ADV7343_HD_MODE_REG4		(0x33)
+#define ADV7343_HD_MODE_REG5		(0x34)
+#define ADV7343_HD_MODE_REG6		(0x35)
+#define ADV7343_HD_Y_LEVEL		(0x36)
+#define ADV7343_HD_CR_LEVEL		(0x37)
+#define ADV7343_HD_CB_LEVEL		(0x38)
+#define ADV7343_HD_MODE_REG7		(0x39)
+#define ADV7343_HD_SHARPNESS_FLTR_GAIN	(0x40)
+#define ADV7343_HD_CGMS_DATA_0		(0x41)
+#define ADV7343_HD_CGMS_DATA_1		(0x42)
+#define ADV7343_HD_CGMS_DATA_2		(0x43)
+#define ADV7343_HD_GAMMA_A0		(0x44)
+#define ADV7343_HD_GAMMA_A1		(0x45)
+#define ADV7343_HD_GAMMA_A2		(0x46)
+#define ADV7343_HD_GAMMA_A3		(0x47)
+#define ADV7343_HD_GAMMA_A4		(0x48)
+#define ADV7343_HD_GAMMA_A5		(0x49)
+#define ADV7343_HD_GAMMA_A6		(0x4a)
+#define ADV7343_HD_GAMMA_A7		(0x4b)
+#define ADV7343_HD_GAMMA_A8		(0x4c)
+#define ADV7343_HD_GAMMA_A9		(0x4d)
+#define ADV7343_HD_GAMMA_B0		(0x4E)
+#define ADV7343_HD_GAMMA_B1		(0x4F)
+#define ADV7343_HD_GAMMA_B2		(0x50)
+#define ADV7343_HD_GAMMA_B3		(0x51)
+#define ADV7343_HD_GAMMA_B4		(0x52)
+#define ADV7343_HD_GAMMA_B5		(0x53)
+#define ADV7343_HD_GAMMA_B6		(0x54)
+#define ADV7343_HD_GAMMA_B7		(0x55)
+#define ADV7343_HD_GAMMA_B8		(0x56)
+#define ADV7343_HD_GAMMA_B9		(0x57)
+#define ADV7343_HD_ADPT_FLTR_GAIN1	(0x58)
+#define ADV7343_HD_ADPT_FLTR_GAIN2	(0x59)
+#define ADV7343_HD_ADPT_FLTR_GAIN3	(0x5a)
+#define ADV7343_HD_ADPT_FLTR_THRLDA	(0x5b)
+#define ADV7343_HD_ADPT_FLTR_THRLDB	(0x5c)
+#define ADV7343_HD_ADPT_FLTR_THRLDC	(0x5d)
+#define ADV7343_HD_CGMS_B0		(0x5E)
+#define ADV7343_HD_CGMS_B1		(0x5F)
+#define ADV7343_HD_CGMS_B2		(0x60)
+#define ADV7343_HD_CGMS_B3		(0x61)
+#define ADV7343_HD_CGMS_B4		(0x62)
+#define ADV7343_HD_CGMS_B5		(0x63)
+#define ADV7343_HD_CGMS_B6		(0x64)
+#define ADV7343_HD_CGMS_B7		(0x65)
+#define ADV7343_HD_CGMS_B8		(0x66)
+#define ADV7343_HD_CGMS_B9		(0x67)
+#define ADV7343_HD_CGMS_B10		(0x68)
+#define ADV7343_HD_CGMS_B11		(0x69)
+#define ADV7343_HD_CGMS_B12		(0x6A)
+#define ADV7343_HD_CGMS_B13		(0x6B)
+#define ADV7343_HD_CGMS_B14		(0x6C)
+#define ADV7343_HD_CGMS_B15		(0x6D)
+#define ADV7343_HD_CGMS_B16		(0x6E)
+
+#define ADV7343_SD_MODE_REG1		(0x80)
+#define ADV7343_SD_MODE_REG2		(0x82)
+#define ADV7343_SD_MODE_REG3		(0x83)
+#define ADV7343_SD_MODE_REG4		(0x84)
+#define ADV7343_SD_MODE_REG5		(0x86)
+#define ADV7343_SD_MODE_REG6		(0x87)
+#define ADV7343_SD_MODE_REG7		(0x88)
+#define ADV7343_SD_MODE_REG8		(0x89)
+#define ADV7343_SD_TIMING_REG0		(0x8A)
+#define ADV7343_SD_TIMING_REG1		(0x8B)
+#define ADV7343_SD_FSC_REG0		(0x8C)
+#define ADV7343_SD_FSC_REG1		(0x8D)
+#define ADV7343_SD_FSC_REG2		(0x8E)
+#define ADV7343_SD_FSC_REG3		(0x8F)
+#define ADV7343_SD_FSC_PHASE		(0x90)
+#define ADV7343_SD_CLOSE_CAPTION_EVEN0	(0x91)
+#define ADV7343_SD_CLOSE_CAPTION_EVEN1	(0x92)
+#define ADV7343_SD_CLOSE_CAPTION_ODD0	(0x93)
+#define ADV7343_SD_CLOSE_CAPTION_ODD1	(0x94)
+#define ADV7343_SD_PEDESTAL_REG0	(0x95)
+#define ADV7343_SD_PEDESTAL_REG1	(0x96)
+#define ADV7343_SD_PEDESTAL_REG2	(0x97)
+#define ADV7343_SD_PEDESTAL_REG3	(0x98)
+#define ADV7343_SD_CGMS_WSS0		(0x99)
+#define ADV7343_SD_CGMS_WSS1		(0x9A)
+#define ADV7343_SD_CGMS_WSS2		(0x9B)
+
+#define ADV7343_SD_SCALE_LSB		(0x9C)
+#define ADV7343_SD_Y_SCALE		(0x9D)
+#define ADV7343_SD_CB_SCALE		(0x9E)
+#define ADV7343_SD_CR_SCALE		(0x9F)
+
+#define ADV7343_SD_HUE_REG		(0xA0)
+#define ADV7343_SD_BRIGHTNESS_WSS	(0xA1)
+#define ADV7343_SD_LUMA_SSAF		(0xA2)
+#define ADV7343_SD_DNR0			(0xA3)
+#define ADV7343_SD_DNR1			(0xA4)
+#define ADV7343_SD_DNR2			(0xA5)
+
+#define ADV7343_SD_GAMMA_A0		(0xA6)
+#define ADV7343_SD_GAMMA_A1		(0xA7)
+#define ADV7343_SD_GAMMA_A2		(0xA8)
+#define ADV7343_SD_GAMMA_A3		(0xA9)
+#define ADV7343_SD_GAMMA_A4		(0xAA)
+#define ADV7343_SD_GAMMA_A5		(0xAB)
+#define ADV7343_SD_GAMMA_A6		(0xAC)
+#define ADV7343_SD_GAMMA_A7		(0xAD)
+#define ADV7343_SD_GAMMA_A8		(0xAE)
+#define ADV7343_SD_GAMMA_A9		(0xAF)
+#define ADV7343_SD_GAMMA_B0		(0xB0)
+#define ADV7343_SD_GAMMA_B1		(0xB1)
+#define ADV7343_SD_GAMMA_B2		(0xB2)
+#define ADV7343_SD_GAMMA_B3		(0xB3)
+#define ADV7343_SD_GAMMA_B4		(0xB4)
+#define ADV7343_SD_GAMMA_B5		(0xB5)
+#define ADV7343_SD_GAMMA_B6		(0xB6)
+#define ADV7343_SD_GAMMA_B7		(0xB7)
+#define ADV7343_SD_GAMMA_B8		(0xB8)
+#define ADV7343_SD_GAMMA_B9		(0xB9)
+#define ADV7343_SD_BRIGHTNESS_DETECT	(0xBA)
+#define ADV7343_FIELD_COUNT_REG		(0xBB)
+#define ADV7343_10_BIT_INPUT		(0x7C)
+
+/* Default values for the registers */
+#define ADV7343_POWER_MODE_REG_DEFAULT		(0x10)
+#define ADV7343_HD_MODE_REG1_DEFAULT		(0x3C)	/* Changed Default
+							   720p EAVSAV code*/
+#define ADV7343_HD_MODE_REG2_DEFAULT		(0x01)	/* Changed Pixel data
+							   valid */
+#define ADV7343_HD_MODE_REG3_DEFAULT		(0x00)	/* Color delay 0 clks */
+#define ADV7343_HD_MODE_REG4_DEFAULT		(0xE8)	/* Changed */
+#define ADV7343_HD_MODE_REG5_DEFAULT		(0x08)
+#define ADV7343_HD_MODE_REG6_DEFAULT		(0x00)
+#define ADV7343_HD_MODE_REG7_DEFAULT		(0x00)
+
+#define ADV7343_SD_MODE_REG1_DEFAULT		(0x00)
+#define ADV7343_SD_MODE_REG2_DEFAULT		(0xC9)
+#define ADV7343_SD_MODE_REG3_DEFAULT		(0x10)
+#define ADV7343_SD_MODE_REG4_DEFAULT		(0x01)
+#define ADV7343_SD_MODE_REG5_DEFAULT		(0x02)
+#define ADV7343_SD_MODE_REG6_DEFAULT		(0x0C)
+#define ADV7343_SD_MODE_REG7_DEFAULT		(0x04)
+#define ADV7343_SD_MODE_REG8_DEFAULT		(0x00)
+#define ADV7343_SOFT_RESET_DEFAULT		(0x02)
+#define ADV7343_COMPOSITE_POWER_VALUE		(0x80)
+#define ADV7343_COMPONENT_POWER_VALUE		(0x1C)
+#define ADV7343_SVIDEO_POWER_VALUE		(0x60)
+#define ADV7343_SD_HUE_REG_DEFAULT		(127)
+#define ADV7343_SD_BRIGHTNESS_WSS_DEFAULT	(0x03)
+#define ADV7343_SD_CGMS_WSS0_DEFAULT		(0x10)
+
+/* Macros for the Mode Select Register */
+#ifdef GENERATE_MASK
+#undef GENERATE_MASK
+#endif
+#define GENERATE_MASK(bits, pos)	((((0xFF) << (8 - bits)) >> \
+					(8 - bits)) << pos)
+
+/* Bit masks for Mode Select Register */
+#define INPUT_MODE_MASK			(0x70)
+#define SD_INPUT_MODE			(0x00)
+#define HD_720P_INPUT_MODE		(0x10)
+#define HD_1080I_INPUT_MODE		(0x10)
+
+/* Bit masks for Mode Register 0 */
+#define TEST_PATTERN_BLACK_BAR_EN	(0x04)
+#define YUV_OUTPUT_SELECT		(0x20)
+#define RGB_OUTPUT_SELECT		(0xDF)
+
+/* Bit masks for DAC output levels */
+#define DAC_OUTPUT_LEVEL_MASK		(0xFF)
+#define POSITIVE_GAIN_MAX		(0x40)
+#define POSITIVE_GAIN_MIN		(0x00)
+#define NEGATIVE_GAIN_MAX		(0xFF)
+#define NEGATIVE_GAIN_MIN		(0xC0)
+
+/* Bit masks for soft reset register */
+#define SOFT_RESET			(0x02)
+
+/* Bit masks for HD Mode Register 1 */
+#define OUTPUT_STD_MASK			(0x03)
+#define OUTPUT_STD_SHIFT		(0)
+#define OUTPUT_STD_EIA0_2		(0x00)
+#define OUTPUT_STD_EIA0_1		(0x01)
+#define OUTPUT_STD_FULL			(0x02)
+#define EMBEDDED_SYNC			(0x04)
+#define EXTERNAL_SYNC			(0xFB)
+#define STD_MODE_SHIFT			(3)
+#define STD_MODE_MASK			(0x1F)
+#define STD_MODE_720P			(0x05)
+#define STD_MODE_720P_25		(0x08)
+#define STD_MODE_720P_30		(0x07)
+#define STD_MODE_720P_50		(0x06)
+#define STD_MODE_1080I			(0x0D)
+#define STD_MODE_1080I_25fps		(0x0E)
+#define STD_MODE_1080P_24		(0x12)
+#define STD_MODE_1080P_25		(0x10)
+#define STD_MODE_1080P_30		(0x0F)
+#define STD_MODE_525P			(0x00)
+#define STD_MODE_625P			(0x03)
+
+/* Bit masks for SD Mode Register 1 */
+#define SD_STD_MASK			(0x03)
+#define SD_STD_NTSC			(0x00)
+#define SD_STD_PAL_BDGHI		(0x01)
+#define SD_STD_PAL_M			(0x02)
+#define SD_STD_PAL_N			(0x03)
+#define SD_LUMA_FLTR_MASK		(0x7)
+#define SD_LUMA_FLTR_SHIFT		(0x2)
+#define SD_CHROMA_FLTR_MASK		(0x7)
+#define SD_CHROMA_FLTR_SHIFT		(0x5)
+
+/* Bit masks for SD Mode Register 2 */
+#define SD_PBPR_SSAF_EN			(0x01)
+#define SD_PBPR_SSAF_DI			(0xFE)
+#define SD_DAC_1_DI			(0xFD)
+#define SD_DAC_2_DI			(0xFB)
+#define SD_PEDESTAL_EN			(0x08)
+#define SD_PEDESTAL_DI			(0xF7)
+#define SD_SQUARE_PIXEL_EN		(0x10)
+#define SD_SQUARE_PIXEL_DI		(0xEF)
+#define SD_PIXEL_DATA_VALID		(0x40)
+#define SD_ACTIVE_EDGE_EN		(0x80)
+#define SD_ACTIVE_EDGE_DI		(0x7F)
+
+/* Bit masks for HD Mode Register 6 */
+#define HD_RGB_INPUT_EN			(0x02)
+#define HD_RGB_INPUT_DI			(0xFD)
+#define HD_PBPR_SYNC_EN			(0x04)
+#define HD_PBPR_SYNC_DI			(0xFB)
+#define HD_DAC_SWAP_EN			(0x08)
+#define HD_DAC_SWAP_DI			(0xF7)
+#define HD_GAMMA_CURVE_A		(0xEF)
+#define HD_GAMMA_CURVE_B		(0x10)
+#define HD_GAMMA_EN			(0x20)
+#define HD_GAMMA_DI			(0xDF)
+#define HD_ADPT_FLTR_MODEB		(0x40)
+#define HD_ADPT_FLTR_MODEA		(0xBF)
+#define HD_ADPT_FLTR_EN			(0x80)
+#define HD_ADPT_FLTR_DI			(0x7F)
+
+#endif				/* End of #ifdef __KERNEL__ */
+
+#endif				/* End of #ifndef ADV7343_H */