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 chaithrika@xxxxxx wrote: > > From: Chaithrika U S <chaithrika@xxxxxx> > > > > ADV7343 video encoder driver > > > > Add ADV7473 I2C based video encoder driver. This follows the v4l2- > subdev > > framework. > > > > Signed-off-by: Chaithrika U S <chaithrika@xxxxxx> > > --- > > 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, ®80, SD_INPUT_MODE, > (~(SD_STD_MASK)), > > + SD_STD_NTSC, 0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, > 0x21, > > + V4L2_STD_NTSC, > > + }, > > + { > > + ADV7343_SD_MODE_REG1, ®80, 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, ®30, 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, ®30, 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, ®30, 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, ®30, 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, ®30, 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, ®30, 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, ®30, 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, ®30, 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, ®80, SD_INPUT_MODE, > (~(SD_STD_MASK)), > > + SD_STD_NTSC, 0x8C, 0x1F, 0x8D, 0x7C, 0x8E, 0xF0, 0x8F, > 0x21, > > + V4L2_STD_NTSC, > > + }, > > + { > > + ADV7343_SD_MODE_REG1, ®80, 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, ®30, 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, ®30, 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, ®30, 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
