On Wednesday 02 September 2009 00:54:58 santiago.nu...@ridgerun.com wrote:
> From: Santiago Nunez-Corrales <santiago.nu...@ridgerun.com>
> 
> This patch provides the implementation of the TVP7002 decoder
> driver for DM365. Implemented revisions by Vaibhav Hiremath
> and Hans Verkuil. Removed most of controls, cleared up logic,
> cleaned up code.
> 
> Signed-off-by: Santiago Nunez-Corrales <santiago.nu...@ridgerun.com>
> ---
>  drivers/media/video/tvp7002.c | 1493 
> +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 1493 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/media/video/tvp7002.c
> 
> diff --git a/drivers/media/video/tvp7002.c b/drivers/media/video/tvp7002.c
> new file mode 100644
> index 0000000..e8cd77e
> --- /dev/null
> +++ b/drivers/media/video/tvp7002.c
> @@ -0,0 +1,1493 @@
> +/* Texas Instruments Triple 8-/10-BIT 165-/110-MSPS Video and Graphics
> + * Digitizer with Horizontal PLL registers
> + *
> + * Copyright (C) 2009 Texas Instruments Inc
> + * Author: Santiago Nunez-Corrales <santiago.nu...@ridgerun.com>
> + *
> + * This code is partially based upon the TVP5150 driver
> + * written by Mauro Carvalho Chehab (mche...@infradead.org)
> + * and the TVP514x driver written by Vaibhav Hiremath <hvaib...@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; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/videodev2.h>
> +#include <linux/delay.h>
> +#include <media/v4l2-device.h>
> +#include <media/tvp7002.h>
> +#include <media/v4l2-i2c-drv.h>
> +#include <media/v4l2-chip-ident.h>
> +#include <media/davinci/videohd.h>
> +#include "tvp7002_reg.h"
> +
> +MODULE_DESCRIPTION("TI TVP7002 Video and Graphics Digitizer driver");
> +MODULE_AUTHOR("Santiago Nunez-Corrales <santiago.nu...@ridgerun.com>");
> +MODULE_LICENSE("GPL");
> +
> +/* I2C retry attempts */
> +#define I2C_RETRY_COUNT                 (5)
> +
> +/* Debugging information */
> +
> +static int debug;
> +module_param(debug, int, 0);
> +MODULE_PARM_DESC(debug, "Debug level (0-2)");
> +
> +/* Reference to tvp7002_platform_data */
> +extern struct tvp7002_platform_data tvp7002_pdata;

No, no, don't do this. This will fail if you have multiple tvp7002 devices in
one system. Just pass the platform data in through the board_info. I also
suggest renaming this struct to tvp7002_config or something like that.
platform_data suggests that this device is platform specific, but it can also
be used in PCI or USB boards.

> +
> +/* Struct for handling resolutions and associate register values */
> +struct tvp7002_resol {
> +     v4l2_std_id id;
> +     int hres;
> +     int vres;
> +     int frate;
> +     int lrate;
> +     int prate;
> +     u8 reg01;
> +     u8 reg02;
> +     u8 reg03;
> +     u8 reg04;
> +};
> +
> +/* Struct for handling register values */
> +struct i2c_reg_value {
> +     u8 reg;
> +     u8 value;
> +};
> +
> +/* Register default values (according to tvp7002 datasheet) */
> +static const struct i2c_reg_value tvp7002_init_default[] = {
> +     /* 0x00: read only */
> +     { TVP7002_HPLL_FDBK_DIV_MSBS, 0x67 },
> +     { TVP7002_HPLL_FDBK_DIV_LSBS, 0x20 },
> +     { TVP7002_HPLL_CRTL, 0xa8 },
> +     { TVP7002_HPLL_PHASE_SEL, 0x80 },
> +     { TVP7002_CLAMP_START, 0x32 },
> +     { TVP7002_CLAMP_W, 0x20 },
> +     { TVP7002_HSYNC_OUT_W, 0x20 },
> +     { TVP7002_B_FINE_GAIN, 0x00 },
> +     { TVP7002_G_FINE_GAIN, 0x00 },
> +     { TVP7002_R_FINE_GAIN, 0x00 },
> +     { TVP7002_B_FINE_OFF_MSBS, 0x80 },
> +     { TVP7002_G_FINE_OFF_MSBS, 0x80 },
> +     { TVP7002_R_FINE_OFF_MSBS, 0x80 },
> +     { TVP7002_SYNC_CTL_1, 0x5b },
> +     { TVP7002_HPLL_AND_CLAMP_CTL, 0x2e },
> +     { TVP7002_SYNC_ON_G_THRS, 0x5d },
> +     { TVP7002_SYNC_SEPARATOR_THRS, 0x20 },
> +     { TVP7002_HPLL_PRE_COAST, 0x00 },
> +     { TVP7002_HPLL_POST_COAST, 0x00 },
> +     /* 0x14: read only */
> +     { TVP7002_OUT_FORMATTER, 0x00 },
> +     { TVP7002_MISC_CTL_1, 0x11 },
> +     { TVP7002_MISC_CTL_2, 0x03 },
> +     { TVP7002_MISC_CTL_3, 0x00 },
> +     { TVP7002_IN_MUX_SEL_1, 0x00 },
> +     { TVP7002_IN_MUX_SEL_2, 0xc2 },
> +     { TVP7002_B_AND_G_COARSE_GAIN, 0x77 },
> +     { TVP7002_R_COARSE_GAIN, 0x07 },
> +     { TVP7002_COARSE_CLAMP_CTL, 0x00 },
> +     { TVP7002_FINE_OFF_LSBS, 0x00 },
> +     { TVP7002_B_COARSE_OFF, 0x10 },
> +     { TVP7002_G_COARSE_OFF, 0x10 },
> +     { TVP7002_R_COARSE_OFF, 0x10 },
> +     { TVP7002_HSOUT_OUT_START, 0x0d },
> +     { TVP7002_MISC_CTL_4, 0x0d },
> +     /* 0x23: read only */
> +     /* 0x24: read only */
> +     /* 0x25: read only */
> +     { TVP7002_AUTO_LVL_CTL_ENABLE, 0x80 },
> +     /* 0x27: read only */
> +     { TVP7002_AUTO_LVL_CTL_FILTER, 0x53 },
> +     { TVP7002_FINE_CLAMP_CTL, 0x07 },
> +     { TVP7002_PWR_CTL, 0x00 },
> +     { TVP7002_ADC_SETUP, 0x50 },
> +     { TVP7002_COARSE_CLAMP_CTL, 0x00 },
> +     { TVP7002_SOG_CLAMP, 0x80 },
> +     { TVP7002_RGB_COARSE_CLAMP_CTL, 0x8c },
> +     { TVP7002_SOG_COARSE_CLAMP_CTL, 0x04 },
> +     { TVP7002_ALC_PLACEMENT, 0x5a },
> +     { TVP7002_MVIS_STRIPPER_W, 0x03 },
> +     { TVP7002_VSYNC_ALGN, 0x10 },
> +     { TVP7002_SYNC_BYPASS, 0x00 },
> +     /* 0x37: read only */
> +     /* 0x38: read only */
> +     /* 0x39: read only */
> +     /* 0x3a: read only */
> +     /* 0x3b: read only */
> +     /* 0x3c: read only */
> +     { TVP7002_L_LENGTH_TOL, 0x03 },
> +     { TVP7002_VIDEO_BWTH_CTL, 0x00 },
> +     { TVP7002_AVID_START_PIXEL_LSBS, 0x01 },
> +     { TVP7002_AVID_START_PIXEL_MSBS, 0x2c },
> +     { TVP7002_AVID_STOP_PIXEL_LSBS, 0x06 },
> +     { TVP7002_AVID_STOP_PIXEL_MSBS, 0x2c },
> +     { TVP7002_VBLK_F_0_START_L_OFF, 0x05 },
> +     { TVP7002_VBLK_F_1_START_L_OFF, 0x05 },
> +     { TVP7002_VBLK_F_0_DURATION, 0x1e },
> +     { TVP7002_VBLK_F_1_DURATION, 0x1e },
> +     { TVP7002_FBIT_F_0_START_L_OFF, 0x00 },
> +     { TVP7002_FBIT_F_1_START_L_OFF, 0x00 },
> +     { TVP7002_YUV_Y_G_COEF_LSBS, 0xe3 },
> +     { TVP7002_YUV_Y_G_COEF_MSBS, 0x16 },
> +     { TVP7002_YUV_Y_B_COEF_LSBS, 0x4f },
> +     { TVP7002_YUV_Y_B_COEF_MSBS, 0x02 },
> +     { TVP7002_YUV_Y_R_COEF_LSBS, 0xce },
> +     { TVP7002_YUV_Y_R_COEF_MSBS, 0x06 },
> +     { TVP7002_YUV_U_G_COEF_LSBS, 0xab },
> +     { TVP7002_YUV_U_G_COEF_MSBS, 0xf3 },
> +     { TVP7002_YUV_U_B_COEF_LSBS, 0x00 },
> +     { TVP7002_YUV_U_B_COEF_MSBS, 0x10 },
> +     { TVP7002_YUV_U_R_COEF_LSBS, 0x55 },
> +     { TVP7002_YUV_U_R_COEF_MSBS, 0xfc },
> +     { TVP7002_YUV_V_G_COEF_LSBS, 0x78 },
> +     { TVP7002_YUV_V_G_COEF_MSBS, 0xf1 },
> +     { TVP7002_YUV_V_B_COEF_LSBS, 0x88 },
> +     { TVP7002_YUV_V_B_COEF_MSBS, 0xfe },
> +     { TVP7002_YUV_V_R_COEF_LSBS, 0x00 },
> +     { TVP7002_YUV_V_R_COEF_MSBS, 0x10 },
> +     { 0x5c, 0x00 }  /* end of registers */
> +};
> +
> +/* Available resolutions */
> +static struct tvp7002_resol tvp7002_resolutions[] = {
> +     {
> +             .id = V4L2_STD_525I_60,
> +             .hres = 720,
> +             .vres = 480,
> +             .frate = 30,
> +             .lrate = 15,
> +             .prate = 14,
> +             .reg01 = 0x35,
> +             .reg02 = 0xa0,
> +             .reg03 = TVP7002_VCO_RANGE_ULOW | 0x18,
> +             .reg04 = 0x80,
> +     },
> +     {
> +             .id = V4L2_STD_625I_50,
> +             .hres = 720,
> +             .vres = 576,
> +             .frate = 25,
> +             .lrate = 16,
> +             .prate = 14,
> +             .reg01 = 0x36,
> +             .reg02 = 0x00,
> +             .reg03 = TVP7002_VCO_RANGE_ULOW | 0x18,
> +             .reg04 = 0x80,
> +     },
> +     {
> +             .id = V4L2_STD_525P_60,
> +             .hres = 720,
> +             .vres = 480,
> +             .frate = 60,
> +             .lrate = 31,
> +             .prate = 27,
> +             .reg01 = 0x35,
> +             .reg02 = 0xa0,
> +             .reg03 = TVP7002_VCO_RANGE_ULOW | 0x18,
> +             .reg04 = 0x80,
> +     },
> +     {
> +             .id = V4L2_STD_625P_50,
> +             .hres = 720,
> +             .vres = 576,
> +             .frate = 50,
> +             .lrate = 31,
> +             .prate = 27,
> +             .reg01 = 0x36,
> +             .reg02 = 0x00,
> +             .reg03 = TVP7002_VCO_RANGE_ULOW | 0x18,
> +             .reg04 = 0x80,
> +     },
> +     {
> +             .id = V4L2_STD_720P_60,
> +             .hres = 1280,
> +             .vres = 720,
> +             .frate = 60,
> +             .lrate = 45,
> +             .prate = 74,
> +             .reg01 = 0x67,
> +             .reg02 = 0x20,
> +             .reg03 = TVP7002_VCO_RANGE_MED | 0x20,
> +             .reg04 = 0x80,
> +     },
> +     {
> +             .id = V4L2_STD_720P_50,
> +             .hres = 1280,
> +             .vres = 720,
> +             .frate = 50,
> +             .lrate = 38,
> +             .prate = 74,
> +             .reg01 = 0x7b,
> +             .reg02 = 0xc0,
> +             .reg03 = TVP7002_VCO_RANGE_MED | 0x18,
> +             .reg04 = 0x80,
> +     },
> +     {
> +             .id = V4L2_STD_1080I_60,
> +             .hres = 1920,
> +             .vres = 1080,
> +             .frate = 60,
> +             .lrate = 34,
> +             .prate = 74,
> +             .reg01 = 0x89,
> +             .reg02 = 0x80,
> +             .reg03 = TVP7002_VCO_RANGE_MED | 0x18,
> +             .reg04 = 0x80,
> +     },
> +     {
> +             .id = V4L2_STD_1080I_50,
> +             .hres = 1920,
> +             .vres = 1080,
> +             .frate = 50,
> +             .lrate = 28,
> +             .prate = 74,
> +             .reg01 = 0xa5,
> +             .reg02 = 0x00,
> +             .reg03 = TVP7002_VCO_RANGE_MED | 0x10,
> +             .reg04 = 0x80,
> +     },
> +     {
> +             .id = V4L2_STD_1080P_60,
> +             .hres = 1920,
> +             .vres = 1080,
> +             .frate = 60,
> +             .lrate = 68,
> +             .prate = 149,
> +             .reg01 = 0x89,
> +             .reg02 = 0x80,
> +             .reg03 = TVP7002_VCO_RANGE_HIGH | 0x20,
> +             .reg04 = 0x80,
> +     },
> +     {
> +             .id = V4L2_STD_1080P_50,
> +             .hres = 1920,
> +             .vres = 1080,
> +             .frate = 50,
> +             .lrate = 56,
> +             .prate = 149,
> +             .reg01 = 0xa5,
> +             .reg02 = 0x00,
> +             .reg03 = TVP7002_VCO_RANGE_HIGH | 0x18,
> +             .reg04 = 0x80,
> +     },
> +};
> +
> +/*
> + * tvp7002_from_std - Map video standard to register information
> + * @std: v4l2_std_id (u64) integer
> + *
> + * Returns index of std or -1 in case of error.
> + */
> +int tvp7002_from_std(v4l2_std_id std)
> +{
> +     int res;
> +
> +     switch (std) {
> +     case V4L2_STD_525P_60:
> +             res = TVP7002_STD_480P;
> +             break;
> +     case V4L2_STD_525I_60:
> +             res = TVP7002_STD_480I;
> +             break;
> +     case V4L2_STD_625P_50:
> +             res = TVP7002_STD_576P;
> +             break;
> +     case V4L2_STD_625I_50:
> +             res = TVP7002_STD_576I;
> +             break;
> +     case V4L2_STD_720P_50:
> +             res = TVP7002_STD_720P_50;
> +             break;
> +     case V4L2_STD_720P_60:
> +             res = TVP7002_STD_720P_60;
> +             break;
> +     case V4L2_STD_1080I_50:
> +             res = TVP7002_STD_1080I_50;
> +             break;
> +     case V4L2_STD_1080I_60:
> +             res = TVP7002_STD_1080I_60;
> +             break;
> +     case V4L2_STD_1080P_50:
> +             res = TVP7002_STD_1080P_50;
> +             break;
> +     case V4L2_STD_1080P_60:
> +             res = TVP7002_STD_1080P_60;
> +             break;
> +     default:
> +             res = -1;
> +             break;
> +     }
> +
> +     return res;
> +}
> +
> +/* Device definition */
> +struct tvp7002 {
> +     struct v4l2_subdev sd;
> +     v4l2_std_id video_mode;
> +     struct v4l2_pix_format pix;
> +     int streaming;
> +};
> +
> +/* Supported controls */
> +static struct v4l2_queryctrl tvp7002_qctrl[] = {
> +     {
> +             /* This gain control uses fine grain in TVP7002 */
> +             .id = V4L2_CID_GAIN,
> +             .type = V4L2_CTRL_TYPE_INTEGER,
> +             .name = "Gain for RGB channels",
> +             .minimum = 0,
> +             .maximum = 255,
> +             .step = 1,
> +             .default_value = 0,
> +             .flags = 0,
> +     },
> +};
> +
> +/*
> + * to_tvp7002 - Obtain device handler TVP7002
> + * @sd: ptr to v4l2_subdev struct
> + *
> + * Returns device handler tvp7002.
> + */
> +static struct tvp7002 *to_tvp7002(struct v4l2_subdev *sd)
> +{
> +     return container_of(sd, struct tvp7002, sd);
> +}
> +
> +/*
> + * tvp7002_read - Read a value from a register in an TVP7002
> + * @sd: ptr to v4l2_subdev struct
> + * @reg: TVP7002 register address
> + *
> + * Returns value read if successful, or non-zero (-1) otherwise.
> + */
> +static int tvp7002_read(struct v4l2_subdev *sd, unsigned char addr)
> +{
> +     struct i2c_client *c = v4l2_get_subdevdata(sd);
> +     int retry;
> +     int error;
> +
> +     for (retry = 0; retry < I2C_RETRY_COUNT; retry++) {
> +             error = i2c_smbus_read_byte_data(c, addr);
> +
> +             if (error >= 0)
> +                     return error;
> +             msleep_interruptible(10);
> +     }
> +     v4l2_err(sd, "TVP7002 read error %d\n", error);
> +     return error;
> +}
> +
> +/*
> + * tvp7002_write() - Write a value to a register in TVP7002
> + * @sd: ptr to v4l2_subdev struct
> + * @addr: TVP7002 register address
> + * @value: value to be written to the register
> + *
> + * Write a value to a register in an TVP7002 decoder device.
> + * Returns zero if successful, or non-zero otherwise.
> + */
> +static int tvp7002_write(struct v4l2_subdev *sd, u8 addr, u8 value)
> +{
> +     struct i2c_client *c = v4l2_get_subdevdata(sd);
> +     int retry;
> +     int error;
> +
> +     for (retry = 0; retry < I2C_RETRY_COUNT; retry++) {
> +             error = i2c_smbus_write_byte_data(c, addr, value);
> +
> +             if (error >= 0)
> +                     return error;
> +             msleep_interruptible(10);
> +     }
> +     v4l2_err(sd, "TVP7002 write error %d\n", error);
> +     return error;
> +}
> +
> +/*
> + * dump_reg_range() - Dump information from TVP7002 registers
> + * @sd: ptr to v4l2_subdev struct
> + * @init: TVP7002 start address
> + * @end: TVP7002 end address
> + *
> + * Dump values at a specified register range
> + * Returns nothing.
> + */
> +static void dump_reg_range(struct v4l2_subdev *sd, u8 init, const u8 end)
> +{
> +     int i = 0;
> +     int result;
> +
> +     while (init != (u8)(end + 1)) {
> +             result = tvp7002_read(sd, init);
> +
> +             if (result < 0)
> +                     v4l2_err(sd, "tvp7002: reg 0x%02x unreadable\n", i);

As I mentioned before: don't add the tvp7002 prefix here, v4l2_err/warn/info
does that for you.

> +             else
> +                     v4l2_info(sd, "tvp7002: @0x%02x = %02x\n", i, result);
> +
> +             init++;
> +             i++;
> +     }
> +}
> +
> +/*
> + * Macro for handling reading error conditions in tvp7002_log_status
> + */
> +#define TVP7002_LOG_CHK(reg, message, res) \
> +     do {\
> +             (res) = tvp7002_read(sd, (reg));\
> +             \
> +             if ((res) >= 0)\
> +                     v4l2_info(sd, "%s = 0x%02x\n", (message), (res));\
> +     } while (0)

Please make this a static inline function. In general macros like this should
be avoided if there is a decent alternative.

> +/*
> + * tvp7002_log_status() - Print information about register settings
> + * @sd: ptr to v4l2_subdev struct
> + *
> + * Log register values of a TVP7002 decoder device.
> + * Returns zero or -EINVAL if read operation fails.

Nobody cares about the return value. Always returning 0 is perfectly
acceptable here. This code is never involved in anything critical, it's just
used during debugging.

> + */
> +static int tvp7002_log_status(struct v4l2_subdev *sd)
> +{
> +     int rres;
> +
> +     TVP7002_LOG_CHK(TVP7002_CHIP_REV, "Chip revision number", rres);
> +     TVP7002_LOG_CHK(TVP7002_HPLL_FDBK_DIV_LSBS, "H-PLL feedback div LSB",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_HPLL_FDBK_DIV_MSBS, "H-PLL feedback div MSB",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_HPLL_FDBK_DIV_MSBS, "VCO freq range selector",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_HPLL_PHASE_SEL, "ADC sampling clk phase sel",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_CLAMP_START, "Clamp start", rres);
> +     TVP7002_LOG_CHK(TVP7002_CLAMP_W, "Clamp width", rres);
> +     TVP7002_LOG_CHK(TVP7002_HSYNC_OUT_W, "HSYNC output width", rres);
> +     TVP7002_LOG_CHK(TVP7002_B_FINE_GAIN, "Digital fine grain B ch", rres);
> +     TVP7002_LOG_CHK(TVP7002_G_FINE_GAIN, "Digital fine grain G ch", rres);
> +     TVP7002_LOG_CHK(TVP7002_R_FINE_GAIN, "Digital fine grain R ch", rres);
> +     TVP7002_LOG_CHK(TVP7002_B_FINE_OFF_MSBS, "Digital fine grain off B ch",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_G_FINE_OFF_MSBS, "Digital fine grain off G ch",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_R_FINE_OFF_MSBS, "Digital fine grain off R ch",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_FINE_OFF_LSBS, "Dig fine grain off LSBs",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_SYNC_CTL_1, "Sync control 1", rres);
> +     TVP7002_LOG_CHK(TVP7002_HPLL_AND_CLAMP_CTL, "H-PLL and clamp control",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_SYNC_ON_G_THRS, "Sync-On-Green threshold",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_SYNC_SEPARATOR_THRS, "Sync separator thrshold",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_HPLL_PRE_COAST, "H-PLL pre-coast", rres);
> +     TVP7002_LOG_CHK(TVP7002_HPLL_POST_COAST, "H-PLL post-coast", rres);
> +     TVP7002_LOG_CHK(TVP7002_SYNC_DETECT_STAT, "Sync detect status", rres);
> +     TVP7002_LOG_CHK(TVP7002_OUT_FORMATTER, "Output formatter", rres);
> +     TVP7002_LOG_CHK(TVP7002_MISC_CTL_1, "Miscelaneous control 1", rres);
> +     TVP7002_LOG_CHK(TVP7002_MISC_CTL_2, "Miscelaneous control 2", rres);
> +     TVP7002_LOG_CHK(TVP7002_MISC_CTL_3, "Miscelaneous control 3", rres);
> +     TVP7002_LOG_CHK(TVP7002_IN_MUX_SEL_1, "Input Mux Selector 1", rres);
> +     TVP7002_LOG_CHK(TVP7002_IN_MUX_SEL_2, "Input Mux Selector 2", rres);
> +     TVP7002_LOG_CHK(TVP7002_B_AND_G_COARSE_GAIN, "B and G coarse gain",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_R_COARSE_GAIN, "R coarse gain", rres);
> +     TVP7002_LOG_CHK(TVP7002_B_COARSE_OFF, "Coarse offset for B ch", rres);
> +     TVP7002_LOG_CHK(TVP7002_G_COARSE_OFF, "Coarse offset for G ch", rres);
> +     TVP7002_LOG_CHK(TVP7002_R_COARSE_OFF, "Coarse offset for R ch", rres);
> +     TVP7002_LOG_CHK(TVP7002_HSOUT_OUT_START, "HSYNC lead edge out start",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_MISC_CTL_4, "Miscelaneous control 4", rres);
> +     TVP7002_LOG_CHK(TVP7002_B_DGTL_ALC_OUT_LSBS, "Flt ALC out B ch LSBs",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_G_DGTL_ALC_OUT_LSBS, "Flt ALC out G ch LSBs",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_R_DGTL_ALC_OUT_LSBS, "Flt ALC out R ch LSBs",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_AUTO_LVL_CTL_ENABLE, "Auto level ctrl enable",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_DGTL_ALC_OUT_MSBS, "Filt ALC out RGB chs MSB",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_AUTO_LVL_CTL_FILTER, "Auto level ctrl filter",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_FINE_CLAMP_CTL, "Fine clamp control", rres);
> +     TVP7002_LOG_CHK(TVP7002_PWR_CTL, "Power control", rres);
> +     TVP7002_LOG_CHK(TVP7002_ADC_SETUP, "ADC setup", rres);
> +     TVP7002_LOG_CHK(TVP7002_COARSE_CLAMP_CTL, "Coarse clamp ctrl", rres);
> +     TVP7002_LOG_CHK(TVP7002_SOG_CLAMP, "Sync-On-Green clamp", rres);
> +     TVP7002_LOG_CHK(TVP7002_RGB_COARSE_CLAMP_CTL, "RGB coarse clamp ctrl",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_SOG_COARSE_CLAMP_CTL, "SOG coarse clamp ctrl",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_ALC_PLACEMENT, "ALC placement", rres);
> +     TVP7002_LOG_CHK(TVP7002_MVIS_STRIPPER_W, "Macrovision stripper width",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_SYNC_BYPASS, "Sync bypass", rres);
> +     TVP7002_LOG_CHK(TVP7002_L_FRAME_STAT_LSBS, "Lines p Frame status LSBs",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_L_FRAME_STAT_MSBS, "Lines p Frame status MSBs",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_CLK_L_STAT_LSBS, "Clks p line status LSBs",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_CLK_L_STAT_MSBS, "Clks p line status MSBs",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_HSYNC_W, "HSYNC width", rres);
> +     TVP7002_LOG_CHK(TVP7002_VSYNC_W, "VSYNC width", rres);
> +     TVP7002_LOG_CHK(TVP7002_L_LENGTH_TOL, "Line length tolerance", rres);
> +     TVP7002_LOG_CHK(TVP7002_VIDEO_BWTH_CTL, "Video bandwth control", rres);
> +     TVP7002_LOG_CHK(TVP7002_AVID_START_PIXEL_LSBS, "AVID start pixel LSBs",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_AVID_START_PIXEL_MSBS, "AVID start pixel MSBs",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_AVID_STOP_PIXEL_LSBS, "AVID stop pixel LSBs",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_AVID_STOP_PIXEL_MSBS, "AVID stop pixel MSBs",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_VBLK_F_0_START_L_OFF, "VBLK start line off 0",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_VBLK_F_1_START_L_OFF, "VBLK start line off 1",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_VBLK_F_0_DURATION, "VBLK duration 0", rres);
> +     TVP7002_LOG_CHK(TVP7002_VBLK_F_1_DURATION, "VBLK duration 1", rres);
> +     TVP7002_LOG_CHK(TVP7002_FBIT_F_0_START_L_OFF, "FBIT start line off 0",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_FBIT_F_1_START_L_OFF, "FBIT start line off 1",
> +                                                                     rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_Y_G_COEF_LSBS, "YUV Y G LSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_Y_G_COEF_MSBS, "YUV Y G MSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_Y_B_COEF_LSBS, "YUV Y B LSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_Y_B_COEF_MSBS, "YUV Y B MSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_Y_R_COEF_LSBS, "YUV Y R LSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_Y_R_COEF_MSBS, "YUV Y R MSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_U_G_COEF_LSBS, "YUV U G LSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_U_G_COEF_MSBS, "YUV U G MSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_U_B_COEF_LSBS, "YUV U B LSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_U_B_COEF_MSBS, "YUV U B MSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_U_R_COEF_LSBS, "YUV U R LSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_U_R_COEF_MSBS, "YUV U R MSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_V_G_COEF_LSBS, "YUV V G LSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_V_G_COEF_MSBS, "YUV V G MSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_V_B_COEF_LSBS, "YUV V B LSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_V_B_COEF_MSBS, "YUV V B MSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_V_R_COEF_LSBS, "YUV V R LSBs", rres);
> +     TVP7002_LOG_CHK(TVP7002_YUV_V_R_COEF_MSBS, "YUV V R MSBs", rres);
> +
> +     return rres;
> +}
> +
> +/*
> + * tvp7002_g_chip_ident() - Get chip identification number
> + * @sd: ptr to v4l2_subdev struct
> + * @chip: ptr to v4l2_dbg_chip_ident struct
> + *
> + * Obtains the chip's identification number.
> + * Returns zero or -EINVAL if read operation fails.
> + */
> +static int tvp7002_g_chip_ident(struct v4l2_subdev *sd,
> +                                     struct v4l2_dbg_chip_ident *chip)
> +{
> +     int rev;
> +     struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +     rev = tvp7002_read(sd, TVP7002_CHIP_REV);
> +
> +     if (rev < 0)
> +             return -EINVAL;
> +
> +     return v4l2_chip_ident_i2c_client(client, chip, V4L2_IDENT_TVP7002,
> +                                                                     rev);
> +}
> +
> +/*
> + * tvp7002_write_inittab() - Write initialization values
> + * @sd: ptr to v4l2_subdev struct
> + * @regs: ptr to i2c_reg_value struct
> + *
> + * Write initialization values.
> + * Returns zero or -EINVAL if read operation fails.
> + */
> +static int tvp7002_write_inittab(struct v4l2_subdev *sd,
> +                                     const struct i2c_reg_value *regs)
> +{
> +     int i;
> +     int error;

Please add an empty line here.

> +     /* Initialize the first (defined) registers */
> +     while (regs->reg != 0x5c) {
> +             error = tvp7002_write(sd, regs->reg, regs->value);
> +             if (error < 0)
> +                     return -EINVAL;
> +             regs++;
> +     }
> +     /* Initialize the last unnamed registers */
> +     for (i = 0x5c; i <= 0xff; i++) {
> +             error = tvp7002_write(sd, i, 0x00);
> +             if (error < 0)
> +                     return -EINVAL;
> +             regs++;
> +     }
> +
> +     return 0;
> +}
> +
> +/*
> + * tvp7002_set_video_mode()
> + * @sd: pointer to standard V4L2 sub-device structure
> + * @sdf: index to structure describing format
> + *
> + * Set video standard according to index
> + *
> + * Returns 0 if operation is successful or -EINVAL otherwise
> + */
> +static int tvp7002_set_video_mode(struct v4l2_subdev *sd, int sdf)
> +{
> +     int error;
> +
> +     if (sdf < TVP7002_STD_480I || sdf > TVP7002_STD_1080P_50) {
> +             v4l2_err(sd, "sf out of range\n");
> +             return -ERANGE;
> +     }
> +
> +     /* Print specific information about current format */
> +     v4l2_info(sd, "Setting standard display format...\n");
> +     v4l2_info(sd, "hres = %d vres=%d fr=%d lr=%d prate=%d\n",
> +                     tvp7002_resolutions[sdf].hres,
> +                     tvp7002_resolutions[sdf].vres,
> +                     tvp7002_resolutions[sdf].frate,
> +                     tvp7002_resolutions[sdf].lrate,
> +                     tvp7002_resolutions[sdf].prate);
> +     /* Set registers accordingly */
> +     error = tvp7002_write(sd, TVP7002_HPLL_FDBK_DIV_MSBS,
> +                                     tvp7002_resolutions[sdf].reg01);
> +     if (error < 0)
> +             return error;

Hmm, this is awkward coding. What about this:

static inline void tvp7002_write_err(..., int *err)
{
        if (!*err)
                *err = tvp7002_write(...);
}

Then you can just call tvp7002_write_err and at the end return 'err'.
Once an error occurs any subsequent writes won't do anything.

> +
> +     error = tvp7002_write(sd, TVP7002_HPLL_FDBK_DIV_LSBS,
> +                                     tvp7002_resolutions[sdf].reg02);
> +     if (error < 0)
> +             return error;
> +
> +     error = tvp7002_write(sd, TVP7002_HPLL_CRTL, 
> +                                     tvp7002_resolutions[sdf].reg03);
> +     if (error < 0)
> +             return error;
> +
> +     error =  tvp7002_write(sd, TVP7002_HPLL_PHASE_SEL,
> +                                     tvp7002_resolutions[sdf].reg04);
> +     if (error < 0)
> +             return error;
> +
> +     /* Set SD/HD mode registers */
> +
> +     if (sdf < TVP7002_STD_720P_60) {
> +             error = tvp7002_write(sd, TVP7002_CLAMP_START, 0x06);
> +             if (error < 0)
> +                     return error;
> +
> +             error = tvp7002_write(sd, TVP7002_CLAMP_W, 0x10);
> +             if (error < 0)
> +                     return error;
> +
> +             error = tvp7002_write(sd, TVP7002_HPLL_PRE_COAST, 0x03);
> +             if (error < 0)
> +                     return error;
> +
> +             error = tvp7002_write(sd, TVP7002_HPLL_POST_COAST, 0x03);
> +             if (error < 0)
> +                     return error;
> +
> +             error = tvp7002_write(sd, TVP7002_IN_MUX_SEL_2, 0x17);
> +             if (error < 0)
> +                     return error;
> +
> +             if (sdf < TVP7002_STD_480P) {
> +                     error = tvp7002_write(sd, TVP7002_HSOUT_OUT_START,
> +                                                                     0x0c);
> +                     if (error < 0)
> +                             return error;
> +
> +                     error = tvp7002_write(sd, TVP7002_MVIS_STRIPPER_W,
> +                                                                     0x24);
> +                     if (error < 0)
> +                             return error;
> +
> +             } else {
> +                     error = tvp7002_write(sd, TVP7002_HSOUT_OUT_START,
> +                                                                     0x0a);
> +                     if (error < 0)
> +                             return error;
> +
> +                     error = tvp7002_write(sd, TVP7002_MVIS_STRIPPER_W,
> +                                                                     0x12);
> +                     if (error < 0)
> +                             return error;
> +             }
> +             error = tvp7002_write(sd, TVP7002_MISC_CTL_4, 0x08);
> +             if (error < 0)
> +                     return error;
> +
> +             error = tvp7002_write(sd, TVP7002_ALC_PLACEMENT, 0x18);
> +             if (error < 0)
> +                     return error;
> +     } else {
> +             error = tvp7002_write(sd, TVP7002_CLAMP_START, 0x32);
> +             if (error < 0)
> +                     return error;
> +
> +             error = tvp7002_write(sd, TVP7002_CLAMP_W, 0x20);
> +             if (error < 0)
> +                     return error;
> +
> +             error = tvp7002_write(sd, TVP7002_HPLL_PRE_COAST, 0x01);
> +             if (error < 0)
> +                     return error;
> +
> +             error = tvp7002_write(sd, TVP7002_HPLL_POST_COAST, 0x00);
> +             if (error < 0)
> +                     return error;
> +
> +             error = tvp7002_write(sd, TVP7002_IN_MUX_SEL_2, 0xc7);
> +             if (error < 0)
> +                     return error;
> +
> +             if(sdf < TVP7002_STD_1080I_60) {
> +                     error = tvp7002_write(sd, TVP7002_HSOUT_OUT_START,
> +                                                                     0x35);
> +                     if (error < 0)
> +                             return error;
> +             } else {
> +                     error = tvp7002_write(sd, TVP7002_HSOUT_OUT_START,
> +                                                                     0x39);
> +                     if (error < 0)
> +                             return error;
> +             }
> +             error = tvp7002_write(sd, TVP7002_MISC_CTL_4, 0x00);
> +             if (error < 0)
> +                     return error;
> +
> +             error = tvp7002_write(sd, TVP7002_ALC_PLACEMENT, 0x5a);
> +             if (error < 0)
> +                     return error;
> +
> +             if(sdf < TVP7002_STD_1080P_60) {
> +                     error = tvp7002_write(sd, TVP7002_MVIS_STRIPPER_W,
> +                                                                     0x07);
> +                     if (error < 0)
> +                             return error;
> +             } else {
> +                     error = tvp7002_write(sd, TVP7002_MVIS_STRIPPER_W,
> +                                                                     0x03);
> +                     if (error < 0)
> +                             return error;
> +             }
> +     }
> +     if (sdf < TVP7002_STD_1080P_60) {
> +             error = tvp7002_write(sd, TVP7002_ADC_SETUP, 0x50);
> +             if (error < 0)
> +                     return error;
> +
> +             error = tvp7002_write(sd, TVP7002_VIDEO_BWTH_CTL, 0x0f);
> +             if (error < 0)
> +                     return error;
> +     } else {
> +             error = tvp7002_write(sd, TVP7002_ADC_SETUP, 0x80);
> +             if (error < 0)
> +                     return error;
> +
> +             error = tvp7002_write(sd, TVP7002_VIDEO_BWTH_CTL, 0x00);
> +             if (error < 0)
> +                     return error;
> +     }
> +     /* Set up registers that hold the same value regardless of the
> +      * SD mode
> +      */
> +     error = tvp7002_write(sd, TVP7002_SYNC_ON_G_THRS, 0x5d);
> +     if (error < 0)
> +             return error;
> +
> +     error = tvp7002_write(sd, TVP7002_SYNC_SEPARATOR_THRS, 0x40);
> +     if (error < 0)
> +             return error;
> +
> +     error = tvp7002_write(sd, TVP7002_MISC_CTL_2, 0x00);
> +     if (error < 0)
> +             return error;
> +
> +     error = tvp7002_write(sd, TVP7002_MISC_CTL_3, 0x01);
> +     if (error < 0)
> +             return error;
> +
> +     error = tvp7002_write(sd, TVP7002_IN_MUX_SEL_1, 0x00);
> +     if (error < 0)
> +             return error;
> +
> +     error = tvp7002_write(sd, TVP7002_VSYNC_ALGN, 0x00);
> +     if (error < 0)
> +             return error;
> +
> +     error = tvp7002_write(sd, TVP7002_L_LENGTH_TOL, 0x06);
> +     if (error < 0)
> +             return error;
> +
> +     error = tvp7002_write(sd, TVP7002_SYNC_SEPARATOR_THRS, 0x40);
> +     if (error < 0)
> +             return error;
> +
> +     error = tvp7002_write(sd, TVP7002_MISC_CTL_2, 0x00);
> +     if (error < 0)
> +             return error;
> +
> +     error = tvp7002_write(sd, TVP7002_MISC_CTL_3, 0x01);
> +     if (error < 0)
> +             return error;
> +
> +     error = tvp7002_write(sd, TVP7002_IN_MUX_SEL_1, 0x00);
> +     if (error < 0)
> +             return error;
> +
> +     error = tvp7002_write(sd, TVP7002_VSYNC_ALGN, 0x00);
> +     if (error < 0)
> +             return error;
> +
> +     error = tvp7002_write(sd, TVP7002_L_LENGTH_TOL, 0x06);
> +     return error;
> +}
> +
> +/*
> + * tvp7002_get_video_mode() - V4L2 decoder interface handler for querystd
> + * @sd: pointer to standard V4L2 sub-device structure
> + *
> + * Returns the current standard detected by TVP7002. If no active input is
> + * detected, returns -1
> + */
> +static v4l2_std_id tvp7002_get_video_mode(struct v4l2_subdev *sd)
> +{
> +     v4l2_std_id error;
> +     int reg01, reg02, reg03;

Please add an empty line after the declarations of the local variables.

> +     reg01 = tvp7002_read(sd, TVP7002_HPLL_FDBK_DIV_MSBS);
> +     reg02 = tvp7002_read(sd, TVP7002_HPLL_FDBK_DIV_LSBS);
> +     reg03 = tvp7002_read(sd, TVP7002_HPLL_CRTL);
> +
> +     if (reg01 < 0 || reg02 < 0 || reg03 < 0) {
> +             error = V4L2_STD_UNKNOWN;
> +             goto found_error;

Why not just 'return V4L2_STD_UNKNOWN;'? Same in the rest of the function.

> +     }
> +
> +     switch(reg01) {
> +     case 0x35:
> +             if (reg02 == 0xa0)
> +                     error = V4L2_STD_525I_60;
> +             else
> +                     error = V4L2_STD_625I_50;
> +     case 0x36:
> +             if (reg02 == 0xa0)
> +                     error = V4L2_STD_525P_60;
> +             else
> +                     error = V4L2_STD_625P_50;
> +             break;
> +     case 0x67:
> +             error = V4L2_STD_720P_60;
> +             break;
> +     case 0x7b:
> +             error = V4L2_STD_720P_50;
> +             break;
> +     case 0x89:
> +             if (reg03 == 0x98)
> +                     error = V4L2_STD_1080I_60;
> +             else
> +                     error = V4L2_STD_1080P_60;
> +             break;
> +     case 0xa5:
> +             if (reg03 == 0x90)
> +                     error = V4L2_STD_1080I_50;
> +             else
> +                     error = V4L2_STD_1080P_50;
> +             break;
> +     default:
> +             error = V4L2_STD_UNKNOWN;
> +             break;
> +     }
> +
> +found_error:
> +     return error;
> +}
> +
> +/*
> + * tvp7002_querystd() - V4L2 decoder interface handler for querystd
> + * @sd: pointer to standard V4L2 sub-device structure
> + * @std_id: standard V4L2 std_id ioctl enum
> + *
> + * Returns the current standard detected by TVP7002. If no active input is
> + * detected, returns -EINVAL
> + */
> +static int tvp7002_querystd(struct v4l2_subdev *sd, v4l2_std_id *std_id)
> +{
> +     struct tvp7002 *decoder = to_tvp7002(sd);
> +     v4l2_std_id current_std;
> +     u8 sync_lock_status;
> +     int res;
> +
> +     if (std_id == NULL)
> +             return -EINVAL;
> +
> +     /* get the current standard */
> +     res = tvp7002_get_video_mode(sd);
> +     if (res == V4L2_STD_UNKNOWN)
> +             return -EINVAL;
> +     current_std = res;
> +
> +     /* check whether signal is locked */
> +     sync_lock_status = tvp7002_read(sd, TVP7002_SYNC_DETECT_STAT);
> +
> +     if (0x02 != (sync_lock_status & 0xff))
> +             return -EINVAL; /* No input detected */
> +
> +     decoder->video_mode = current_std;

querystd only returns the detected std, it does NOT change any internal state!

> +     *std_id = current_std;
> +
> +     v4l2_info(sd, "Current STD: %d %d @ %d Hz\n",
> +             tvp7002_resolutions[tvp7002_from_std(current_std)].hres,
> +             tvp7002_resolutions[tvp7002_from_std(current_std)].vres,
> +             tvp7002_resolutions[tvp7002_from_std(current_std)].frate);

This should be v4l2_dbg. You don't want to see logging when calling normal
functions like this.

I suggest that you also look into implementing video_ops.g_input_status. This
should be used much more often than it currently is. It may need additional
support from the dm365 driver to actually call it.

> +     return 0;
> +}
> +
> +/*
> + * tvp7002_scanmode() - Returns whether format is progressive
> + * or interlaced
> + */
> +
> +
> +/*
> + * tvp7002_try_fmt_cap() - V4L2 decoder interface handler for try_fmt
> + * @sd: pointer to standard V4L2 sub-device structure
> + * @f: pointer to standard V4L2 VIDIOC_TRY_FMT ioctl structure
> + *
> + * Implement the VIDIOC_TRY_FMT ioctl for the CAPTURE buffer type. This
> + * ioctl is used to negotiate the image capture size and pixel format
> + * without actually making it take effect.
> + */
> +static int tvp7002_try_fmt_cap(struct v4l2_subdev *sd, struct v4l2_format *f)
> +{
> +     struct tvp7002 *decoder = to_tvp7002(sd);
> +     struct v4l2_pix_format *pix;
> +     v4l2_std_id current_std;
> +     int res;
> +
> +     if (f == NULL)
> +             return -EINVAL;
> +
> +     if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +             /* only capture is supported */
> +             f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;

Huh? It should return an -EINVAL if it isn't this type.

> +
> +     pix = &f->fmt.pix;
> +
> +     /* Calculate height and width based on current standard */
> +     res = tvp7002_get_video_mode(sd);
> +     if (res < 0)
> +             return -EINVAL;
> +     current_std = res;
> +
> +     decoder->video_mode = current_std;

Note: try_fmt_cap should never change the internal state of a driver. That is
why it is called 'try': just to see if it can be done, not actually do it :-)

> +     pix->width = tvp7002_resolutions[tvp7002_from_std(current_std)].hres;
> +     pix->height = tvp7002_resolutions[tvp7002_from_std(current_std)].vres;
> +
> +     pix->pixelformat = V4L2_PIX_FMT_UYVY;
> +
> +     pix->field = V4L2_FIELD_INTERLACED;
> +     pix->bytesperline = pix->width * 2;
> +     pix->sizeimage = pix->bytesperline * pix->height;
> +     pix->colorspace = V4L2_COLORSPACE_REC709;

Does the tvp7002 support SDTV? If so, then I believe those formats have a
slightly different colorspace (REC601 I think). I'm not 100% sure, so please
double-check before changing anything.

> +     pix->priv = 0;
> +
> +     v4l2_dbg(1, debug, sd, "Try FMT: pixelformat - %s, bytesperline - %d"
> +                     "Width - %d, Height - %d",
> +                     "8-bit UYVY 4:2:2 Format", pix->bytesperline,
> +                     pix->width, pix->height);
> +     return 0;
> +}
> +
> +/**
> + * tvp7002_s_fmt_cap() - V4L2 decoder interface handler for s_fmt
> + * @sd: pointer to standard V4L2 sub-device structure
> + * @f: pointer to standard V4L2 VIDIOC_S_FMT ioctl structure
> + *
> + * If the requested format is supported, configures the HW to use that
> + * format, returns error code if format not supported or HW can't be
> + * correctly configured.
> + */
> +static int tvp7002_s_fmt_cap(struct v4l2_subdev *sd, struct v4l2_format *f)
> +{
> +     struct tvp7002 *decoder = to_tvp7002(sd);
> +     struct v4l2_pix_format *pix;
> +     int rval;
> +
> +     if (f == NULL)
> +             return -EINVAL;
> +
> +     if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +             /* only capture is supported */
> +             return -EINVAL;
> +
> +     pix = &f->fmt.pix;
> +     rval = tvp7002_try_fmt_cap(sd, f);
> +     if (rval)
> +             return rval;
> +
> +             decoder->pix = *pix;

Indentation.

> +
> +     return rval;
> +}
> +
> +/**
> + * tvp7002_g_fmt() - V4L2 decoder interface handler for tvp7002_g_fmt
> + * @sd: pointer to standard V4L2 sub-device structure
> + * @f: pointer to standard V4L2 v4l2_format structure
> + *
> + * Returns the decoder's current pixel format in the v4l2_format
> + * parameter.
> + */
> +static int tvp7002_g_fmt(struct v4l2_subdev *sd, struct v4l2_format *f)
> +{
> +     struct tvp7002 *decoder = to_tvp7002(sd);
> +
> +     if (f == NULL)
> +             return -EINVAL;
> +
> +     if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +             /* only capture is supported */
> +             return -EINVAL;
> +
> +     f->fmt.pix = decoder->pix;
> +
> +     v4l2_info(sd, "Current FMT: bytesperline - %d"
> +                     "Width - %d, Height - %d",
> +                     decoder->pix.bytesperline,
> +                     decoder->pix.width, decoder->pix.height);

Use v4l2_dbg.

> +     return 0;
> +}
> +
> +/*
> + * tvp7002_s_ctrl() - Set a control
> + * @sd: ptr to v4l2_subdev struct
> + * @ctrl: ptr to v4l2_control struct
> + *
> + * Set a control for a TVP7002 decoder device.
> + * Returns zero when successful or -EINVAL if register access fails.
> + */
> +static int tvp7002_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
> +{
> +     struct tvp7002 *decoder = to_tvp7002(sd);
> +     int vmd = 0;
> +
> +     decoder->video_mode = std;
> +     vmd = tvp7002_from_std(std);
> +
> +     v4l2_info(sd, "Set video std mode to %d.\n", (int)std);

Use v4l2_dbg. Please check this source for similar cases.

And also use %llx when printing v4l2_std_id codes. It's a bitmask, and it's
hard to 'see' the bits when printing decimal.

> +
> +     return tvp7002_set_video_mode(sd, vmd);
> +}
> +
> +/*
> + * tvp7002_g_ctrl() - Get a control
> + * @sd: ptr to v4l2_subdev struct
> + * @ctrl: ptr to v4l2_control struct
> + *
> + * Get a control for a TVP7002 decoder device.
> + * Returns zero when successful or -EINVAL if register access fails.
> + */
> +static int tvp7002_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
> +{
> +     int rval, gval, bval;
> +     int res;
> +
> +     v4l2_info(sd, "g_ctrl called\n");
> +
> +     switch (ctrl->id) {
> +     case V4L2_CID_GAIN:
> +             rval = tvp7002_read(sd, TVP7002_R_FINE_GAIN);
> +             gval = tvp7002_read(sd, TVP7002_G_FINE_GAIN);
> +             bval = tvp7002_read(sd, TVP7002_B_FINE_GAIN);
> +
> +             if (rval < 0 || gval < 0 || bval < 0) {
> +                     res = -1;
> +             } else if (rval != gval || rval != bval) {
> +                     res = -1;
> +             } else {
> +                     ctrl->value = rval & 0x0F;
> +                     res = ctrl->value;
> +             }
> +             break;
> +     default:
> +             res = -1;
> +             break;
> +     }
> +
> +     return res < 0 ? res : 0;
> +}
> +
> +/*
> + * tvp7002_s_ctrl() - Set a control
> + * @sd: ptr to v4l2_subdev struct
> + * @ctrl: ptr to v4l2_control struct
> + *
> + * Set a control in TVP7002 decoder device.
> + * Returns zero when successful or -EINVAL if register access fails.
> + */
> +static int tvp7002_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
> +{
> +     int rval, gval, bval;
> +     int error;
> +     u8 i, n;
> +     n = ARRAY_SIZE(tvp7002_qctrl);
> +
> +     for (i = 0; i < n; i++)
> +             if (ctrl->id == tvp7002_qctrl[i].id)
> +                     break;
> +
> +     if (i == n)
> +             return -EINVAL;
> +
> +     if (ctrl->value < tvp7002_qctrl[i].minimum ||
> +                     ctrl->value > tvp7002_qctrl[i].maximum)
> +             return -ERANGE;
> +
> +     switch (ctrl->id) {
> +     case V4L2_CID_GAIN:
> +             rval = tvp7002_write(sd, TVP7002_R_FINE_GAIN,
> +                                                     ctrl->value & 0xff);
> +             gval = tvp7002_write(sd, TVP7002_G_FINE_GAIN,
> +                                                     ctrl->value & 0xff);
> +             bval = tvp7002_write(sd, TVP7002_B_FINE_GAIN,
> +                                                     ctrl->value & 0xff);
> +             if (rval < 0  || gval < 0 || bval < 0)
> +                     error = -1;
> +             else
> +                     error = rval;
> +             break;
> +     default:
> +             error = -1;
> +             break;
> +     }
> +
> +     if (error < 0)
> +             return -EINVAL;
> +     else
> +             return 0;
> +}
> +
> +/*
> + * tvp7002_g_register() - Get the value of a register
> + * @sd: ptr to v4l2_subdev struct
> + * @vreg: ptr to v4l2_dbg_register struct
> + *
> + * Get the value of a TVP7002 decoder device register.
> + * Returns zero when successful, -EINVAL if register read fails or
> + * access to I2C client fails, -EPERM if the call is not allowed
> + * by diabled CAP_SYS_ADMIN.
> + */
> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> +static int tvp7002_g_register(struct v4l2_subdev *sd,
> +                                             struct v4l2_dbg_register *reg)
> +{
> +     struct i2c_client *client = v4l2_get_subdevdata(sd);
> +     int error;
> +
> +     if (!v4l2_chip_match_i2c_client(client, &reg->match))
> +             return -EINVAL;
> +     if (!capable(CAP_SYS_ADMIN))
> +             return -EPERM;
> +
> +     reg->val = tvp7002_read(sd, reg->reg & 0xff);
> +     reg->size = 1;
> +
> +     if (reg->val < 0)
> +             error = -EINVAL;
> +     else
> +             error = 0;
> +
> +     return error;

return reg->val < 0 ? -EINVAL : 0;

> +}
> +
> +/*
> + * tvp7002_s_register() - set a control
> + * @sd: ptr to v4l2_subdev struct
> + * @ctrl: ptr to v4l2_control struct
> + *
> + * Get the value of a TVP7002 decoder device register.
> + * Returns zero when successful or -EINVAL if register read fails.
> + */
> +static int tvp7002_s_register(struct v4l2_subdev *sd,
> +                                             struct v4l2_dbg_register *reg)
> +{
> +     int error, wres;
> +     struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +     if (!v4l2_chip_match_i2c_client(client, &reg->match))
> +             return -EINVAL;
> +     if (!capable(CAP_SYS_ADMIN))
> +             return -EPERM;
> +
> +     wres = tvp7002_write(sd, reg->reg & 0xff, reg->val & 0xff);
> +
> +     if (wres < 0)
> +             error = -EINVAL;
> +     else
> +             error = 0;
> +
> +     return error;

Ditto.

> +}
> +#endif
> +
> +/*
> + * tvp7002_queryctrl() - Query a control
> + * @sd: ptr to v4l2_subdev struct
> + * @ctrl: ptr to v4l2_queryctrl struct
> + *
> + * Query a control of a TVP7002 decoder device.
> + * Returns zero when successful or -EINVAL if register read fails.
> + */
> +static int tvp7002_queryctrl(struct v4l2_subdev *sd, struct v4l2_queryctrl 
> *qc)
> +{
> +     int i, error;
> +     error = -EINVAL;
> +
> +     v4l2_info(sd, "queryctrl called\n");
> +
> +     for (i = 0; i < ARRAY_SIZE(tvp7002_qctrl); i++)
> +             if (qc->id && qc->id == tvp7002_qctrl[i].id) {
> +                     memcpy(qc, &(tvp7002_qctrl[i]), sizeof(*qc));
> +                     error = 0;
> +                     break;
> +             }

Please use the v4l2_ctrl_query_fill function from v4l2-common.h for this.
It will ensure consistent control names and it will be easier to convert to
the upcoming control handling framework that I am working on. It's also
shorter :-)

> +
> +     return error;
> +}
> +
> +/*
> + * tvp7002_s_stream() - V4L2 decoder i/f handler for s_stream
> + * @sd: pointer to standard V4L2 sub-device structure
> + * @enable: streaming enable or disable
> + *
> + * Sets streaming to enable or disable, if possible.
> + */
> +static int tvp7002_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +     int err = 0;
> +     struct tvp7002 *decoder = to_tvp7002(sd);
> +
> +     if (decoder->streaming == enable)
> +             return 0;
> +
> +     if (enable) {
> +             /* Power Up Sequence */
> +             err = tvp7002_write(sd, TVP7002_PWR_CTL, 0x00);
> +             if (err) {
> +                     v4l2_err(sd, "Unable to turn on decoder\n");
> +                     err = -EINVAL;
> +             }
> +             err = tvp7002_write_inittab(sd, tvp7002_init_default);
> +             if (err < 0) {
> +                     v4l2_err(sd, "Unable to initialize\n");
> +                     err = -EINVAL;
> +             }
> +             /* Detect if not already detected */
> +             err = tvp7002_read(sd, TVP7002_CHIP_REV);
> +             if (err < 0) {
> +                     v4l2_err(sd, "Unable to detect decoder\n");
> +                     err = -EINVAL;
> +             }
> +             decoder->streaming = enable;
> +     } else {
> +             /* Power Down Sequence */
> +             err = tvp7002_write(sd, TVP7002_PWR_CTL, 0x40);
> +             if (err) {
> +                     v4l2_err(sd, "Unable to turn off decoder\n");
> +                     return err;
> +             }
> +             decoder->streaming = enable;
> +     }
> +
> +     return err;
> +}
> +
> +/* Specific video subsystem operation handlers */
> +static const struct v4l2_subdev_video_ops tvp7002_video_ops = {
> +     .querystd = tvp7002_querystd,
> +     .s_stream = tvp7002_s_stream,
> +     .g_fmt = tvp7002_g_fmt,
> +};
> +
> +/* V4L2 Operations handlers */
> +static const struct v4l2_subdev_core_ops tvp7002_core_ops = {
> +     .g_chip_ident = tvp7002_g_chip_ident,
> +     .log_status = tvp7002_log_status,
> +     .g_ctrl = tvp7002_g_ctrl,
> +     .s_ctrl = tvp7002_s_ctrl,
> +     .queryctrl = tvp7002_queryctrl,
> +     .s_std = tvp7002_s_std,
> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> +     .g_register = tvp7002_g_register,
> +     .s_register = tvp7002_s_register,
> +#endif
> +};
> +
> +static const struct v4l2_subdev_ops tvp7002_ops = {
> +     .core = &tvp7002_core_ops,
> +     .video = &tvp7002_video_ops,
> +};
> +
> +/*
> + * tvp7002_reset - Reset a TVP7002 device
> + * @sd: ptr to v4l2_subdev struct
> + * @val: unsigned integer (not used)
> + *
> + * Reset the TVP7002 device
> + * Returns zero when successful or -EINVAL if register read fails.
> + */
> +static int tvp7002_reset(struct v4l2_subdev *sd, u32 val)
> +{
> +     int polarity;
> +     int error;
> +
> +     error = tvp7002_read(sd, TVP7002_CHIP_REV);
> +     if (error < 0) {
> +             error = -EINVAL;
> +             goto found_error;
> +     }
> +
> +     if (error == 0x02) {
> +             v4l2_info(sd, "rev. %02x detected.\n", error);
> +     } else {
> +             v4l2_info(sd, "unknown revision detected.\n");
> +             v4l2_info(sd, "revision number is %02x\n", error);
> +     }
> +
> +     /* Set polarity information */
> +     polarity = tvp7002_pdata.clk_polarity & tvp7002_pdata.hs_polarity &
> +                     tvp7002_pdata.vs_polarity & tvp7002_pdata.fid_polarity;
> +     error = tvp7002_write(sd, TVP7002_MISC_CTL_2, polarity);
> +     if (error < 0) {
> +             error = -EINVAL;
> +             goto found_error;
> +     }
> +
> +     /* Initializes TVP7002 to its default values */
> +     error = tvp7002_write_inittab(sd, tvp7002_init_default);
> +     if (error < 0) {
> +             error = -EINVAL;
> +             goto found_error;
> +     }
> +
> +found_error:
> +     return error;
> +};
> +
> +/*
> + * tvp7002_probe - Reset a TVP7002 device
> + * @sd: ptr to v4l2_subdev struct
> + * @ctrl: ptr to i2c_device_id struct
> + *
> + * Reset the TVP7002 device
> + * Returns zero when successful or -EINVAL if register read fails.
> + */
> +static int tvp7002_probe(struct i2c_client *c, const struct i2c_device_id 
> *id)
> +{
> +     struct v4l2_subdev *sd;
> +     struct tvp7002 *core;
> +     int polarity;
> +     int error;
> +
> +     /* Check if the adapter supports the needed features */
> +     if (!i2c_check_functionality(c->adapter,
> +             I2C_FUNC_SMBUS_READ_BYTE | I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
> +             return -EIO;
> +
> +     core = kzalloc(sizeof(struct tvp7002), GFP_KERNEL);
> +
> +     if (!core)
> +             return -ENOMEM;
> +
> +     sd = &core->sd;
> +     v4l2_i2c_subdev_init(sd, c, &tvp7002_ops);
> +     v4l_info(c, "tvp7002 found @ 0x%02x (%s)\n",
> +                                     c->addr << 1, c->adapter->name);
> +
> +     /* Set polarity information */
> +     polarity = tvp7002_pdata.clk_polarity & tvp7002_pdata.hs_polarity &
> +                     tvp7002_pdata.vs_polarity & tvp7002_pdata.fid_polarity;
> +     error = tvp7002_write(sd, TVP7002_MISC_CTL_2, polarity);
> +     if (error < 0) {
> +             error = -EINVAL;
> +             goto found_error;
> +     }
> +
> +     /* Set video mode */
> +     core->video_mode = V4L2_STD_525P_60;
> +
> +     if (debug > 1)
> +             error = tvp7002_log_status(sd);

Ignore the error here. I am not actually sure how useful it is to have this
here as you can call log_status at any time (v4l2-ctl --log-status).

> +     else
> +             error = 0;
> +
> +found_error:
> +     if (error < 0)
> +             kfree(core);
> +
> +     return error;
> +}
> +
> +/*
> + * tvp7002_remove - Remove TVP7002 device support
> + * @c: ptr to i2c_client struct
> + *
> + * Reset the TVP7002 device
> + * Returns zero when successful or -EINVAL if register read fails.
> + */
> +static int tvp7002_remove(struct i2c_client *c)
> +{
> +     struct v4l2_subdev *sd = i2c_get_clientdata(c);
> +
> +     v4l2_dbg(1, debug, sd, "tvp7002.c: removing tvp7002 adapter"
> +                             "on address 0x%x\n", c->addr << 1);

Remove tvp7002 prefix.

> +
> +     v4l2_device_unregister_subdev(sd);
> +     kfree(to_tvp7002(sd));
> +     return 0;
> +}
> +
> +/* I2C Device ID table */
> +static const struct i2c_device_id tvp7002_id[] = {
> +     { "tvp7002", 0 },
> +     { }
> +};
> +MODULE_DEVICE_TABLE(i2c, tvp7002_id);
> +
> +/* I2C driver data */
> +static struct i2c_driver tvp7002_driver = {
> +     .driver = {
> +             .owner = THIS_MODULE,
> +             .name = "tvp7002",
> +     },
> +     .probe = tvp7002_probe,
> +     .remove = tvp7002_remove,
> +     .id_table = tvp7002_id,
> +};

I have no comments on the other patches, it's just this one that needs another
update. 

Regards,

        Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to