All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add ov772x driver
@ 2008-10-15 10:52 Kuninori Morimoto
  2008-10-15 23:41 ` Guennadi Liakhovetski
  0 siblings, 1 reply; 22+ messages in thread
From: Kuninori Morimoto @ 2008-10-15 10:52 UTC (permalink / raw)
  To: V4L

This patch adds ov772x driver that use soc_camera framework.
It was tested on SH Migo-r board.

Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
---
 drivers/media/video/Kconfig  |    6 +
 drivers/media/video/Makefile |    1 +
 drivers/media/video/ov772x.c |  957 ++++++++++++++++++++++++++++++++++++++++++
 include/media/ov772x.h       |   13 +
 4 files changed, 977 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/video/ov772x.c
 create mode 100644 include/media/ov772x.h

diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 47102c2..d0c4152 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -750,6 +750,12 @@ config SOC_CAMERA_PLATFORM
 	help
 	  This is a generic SoC camera platform driver, useful for testing
 
+config SOC_CAMERA_OV772X
+	tristate "ov772x camera support"
+	depends on SOC_CAMERA
+	help
+	  This is a ov772x camera platform driver on soc framework
+
 config VIDEO_PXA27x
 	tristate "PXA27x Quick Capture Interface driver"
 	depends on VIDEO_DEV && PXA27x && SOC_CAMERA
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index 16962f3..d281330 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -134,6 +134,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9M001)	+= mt9m001.o
 obj-$(CONFIG_SOC_CAMERA_MT9M111)	+= mt9m111.o
 obj-$(CONFIG_SOC_CAMERA_MT9V022)	+= mt9v022.o
 obj-$(CONFIG_SOC_CAMERA_PLATFORM)	+= soc_camera_platform.o
+obj-$(CONFIG_SOC_CAMERA_OV772X)		+= ov772x.o
 
 obj-$(CONFIG_VIDEO_AU0828) += au0828/
 
diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
new file mode 100644
index 0000000..0db0319
--- /dev/null
+++ b/drivers/media/video/ov772x.c
@@ -0,0 +1,957 @@
+/*
+ * ov772x Camera Driver
+ *
+ * Copyright (C) 2008 Renesas Solutions Corp.
+ * Kuninori Morimoto <morimoto.kuninori@renesas.com>
+ *
+ * Based on ov7670 and soc_camera_platform driver,
+ *
+ * Copyright 2006-7 Jonathan Corbet <corbet@lwn.net>
+ * Copyright (C) 2008 Magnus Damm
+ * Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/videodev2.h>
+#include <media/v4l2-common.h>
+#include <media/soc_camera.h>
+#include <media/ov772x.h>
+
+/*
+ * bit defines
+ */
+#define ZERO 0x00
+#define B0   0x01
+#define B1   0x02
+#define B2   0x04
+#define B3   0x08
+
+#define B4   0x10
+#define B5   0x20
+#define B6   0x40
+#define B7   0x80
+
+/*
+ * register offset
+ */
+#define GAIN        0x00 /* AGC - Gain control gain setting */
+#define BLUE        0x01 /* AWB - Blue channel gain setting */
+#define RED         0x02 /* AWB - Red   channel gain setting */
+#define GREEN       0x03 /* AWB - Green channel gain setting */
+#define COM1        0x04 /* Common control 1 */
+#define BAVG        0x05 /* U/B Average Level */
+#define GAVG        0x06 /* Y/Gb Average Level */
+#define RAVG        0x07 /* V/R Average Level */
+#define AECH        0x08 /* Exposure Value - AEC MSBs */
+#define COM2        0x09 /* Common control 2 */
+#define PID         0x0A /* Product ID Number MSB */
+#define VER         0x0B /* Product ID Number LSB */
+#define COM3        0x0C /* Common control 3 */
+#define COM4        0x0D /* Common control 4 */
+#define COM5        0x0E /* Common control 5 */
+#define COM6        0x0F /* Common control 6 */
+#define AEC         0x10 /* Exposure Value */
+#define CLKRC       0x11 /* Internal clock */
+#define COM7        0x12 /* Common control 7 */
+#define COM8        0x13 /* Common control 8 */
+#define COM9        0x14 /* Common control 9 */
+#define COM10       0x15 /* Common control 10 */
+#define HSTART      0x17 /* Horizontal sensor size */
+#define HSIZE       0x18 /* Horizontal frame (HREF column) end high 8-bit */
+#define VSTART      0x19 /* Vertical frame (row) start high 8-bit */
+#define VSIZE       0x1A /* Vertical sensor size */
+#define PSHFT       0x1B /* Data format - pixel delay select */
+#define MIDH        0x1C /* Manufacturer ID byte - high */
+#define MIDL        0x1D /* Manufacturer ID byte - low  */
+#define LAEC        0x1F /* Fine AEC value */
+#define COM11       0x20 /* Common control 11 */
+#define BDBASE      0x22 /* Banding filter Minimum AEC value */
+#define DBSTEP      0x23 /* Banding filter Maximum Setp */
+#define AEW         0x24 /* AGC/AEC - Stable operating region (upper limit) */
+#define AEB         0x25 /* AGC/AEC - Stable operating region (lower limit) */
+#define VPT         0x26 /* AGC/AEC Fast mode operating region */
+#define HOUTSIZE    0x29 /* Horizontal data output size MSBs */
+#define EXHCH       0x2A /* Dummy pixel insert MSB */
+#define EXHCL       0x2B /* Dummy pixel insert LSB */
+#define VOUTSIZE    0x2C /* Vertical data output size MSBs */
+#define ADVFL       0x2D /* LSB of insert dummy lines in Vertical direction */
+#define ADVFH       0x2E /* MSG of insert dummy lines in Vertical direction */
+#define YAVE        0x2F /* Y/G Channel Average value */
+#define LUMHTH      0x30 /* Histogram AEC/AGC Luminance high level threshold */
+#define LUMLTH      0x31 /* Histogram AEC/AGC Luminance low  level threshold */
+#define HREF        0x32 /* Image start and size control */
+#define DM_LNL      0x33 /* Dummy line low  8 bits */
+#define DM_LNH      0x34 /* Dummy line high 8 bits */
+#define ADOFF_B     0x35 /* AD offset compensation value for B  channel */
+#define ADOFF_R     0x36 /* AD offset compensation value for R  channel */
+#define ADOFF_GB    0x37 /* AD offset compensation value for Gb channel */
+#define ADOFF_GR    0x38 /* AD offset compensation value for Gr channel */
+#define OFF_B       0x39 /* Analog process B  channel offset value */
+#define OFF_R       0x3A /* Analog process R  channel offset value */
+#define OFF_GB      0x3B /* Analog process Gb channel offset value */
+#define OFF_GR      0x3C /* Analog process Gr channel offset value */
+#define COM12       0x3D /* Common control 12 */
+#define COM13       0x3E /* Common control 13 */
+#define COM14       0x3F /* Common control 14 */
+#define COM15       0x40 /* Common control 15*/
+#define COM16       0x41 /* Common control 16 */
+#define TGT_B       0x42 /* BLC blue channel target value */
+#define TGT_R       0x43 /* BLC red  channel target value */
+#define TGT_GB      0x44 /* BLC Gb   channel target value */
+#define TGT_GR      0x45 /* BLC Gr   channel target value */
+#define LCC0        0x46 /* Lens correction control 0 */
+#define LCC1        0x47 /* Lens correction option 1 - X coordinate */
+#define LCC2        0x48 /* Lens correction option 2 - Y coordinate */
+#define LCC3        0x49 /* Lens correction option 3 */
+#define LCC4        0x4A /* Lens correction option 4 - radius of the circular */
+#define LCC5        0x4B /* Lens correction option 5 */
+#define LCC6        0x4C /* Lens correction option 6 */
+#define FIXGAIN     0x4D /* Analog fix gain amplifer */
+#define AREF0       0x4E /* Sensor reference control */
+#define AREF1       0x4F /* Sensor reference current control */
+#define AREF2       0x50 /* Analog reference control */
+#define AREF3       0x51 /* ADC    reference control */
+#define AREF4       0x52 /* ADC    reference control */
+#define AREF5       0x53 /* ADC    reference control */
+#define AREF6       0x54 /* Analog reference control */
+#define AREF7       0x55 /* Analog reference control */
+#define UFIX        0x60 /* U channel fixed value output */
+#define VFIX        0x61 /* V channel fixed value output */
+#define AW_BB_BLK   0x62 /* AWB option for advanced AWB */
+#define AW_B_CTRL0  0x63 /* AWB control byte 0 */
+#define DSP_CTRL1   0x64 /* DSP control byte 1 */
+#define DSP_CTRL2   0x65 /* DSP control byte 2 */
+#define DSP_CTRL3   0x66 /* DSP control byte 3 */
+#define DSP_CTRL4   0x67 /* DSP control byte 4 */
+#define AW_B_BIAS   0x68 /* AWB BLC level clip */
+#define AW_BCTRL1   0x69 /* AWB control  1 */
+#define AW_BCTRL2   0x6A /* AWB control  2 */
+#define AW_BCTRL3   0x6B /* AWB control  3 */
+#define AW_BCTRL4   0x6C /* AWB control  4 */
+#define AW_BCTRL5   0x6D /* AWB control  5 */
+#define AW_BCTRL6   0x6E /* AWB control  6 */
+#define AW_BCTRL7   0x6F /* AWB control  7 */
+#define AW_BCTRL8   0x70 /* AWB control  8 */
+#define AW_BCTRL9   0x71 /* AWB control  9 */
+#define AW_BCTRL10  0x72 /* AWB control 10 */
+#define AW_BCTRL11  0x73 /* AWB control 11 */
+#define AW_BCTRL12  0x74 /* AWB control 12 */
+#define AW_BCTRL13  0x75 /* AWB control 13 */
+#define AW_BCTRL14  0x76 /* AWB control 14 */
+#define AW_BCTRL15  0x77 /* AWB control 15 */
+#define AW_BCTRL16  0x78 /* AWB control 16 */
+#define AW_BCTRL17  0x79 /* AWB control 17 */
+#define AW_BCTRL18  0x7A /* AWB control 18 */
+#define AW_BCTRL19  0x7B /* AWB control 19 */
+#define AW_BCTRL20  0x7C /* AWB control 20 */
+#define AW_BCTRL21  0x7D /* AWB control 21 */
+#define GAM1        0x7E /* Gamma Curve  1st segment input end point */
+#define GAM2        0x7F /* Gamma Curve  2nd segment input end point */
+#define GAM3        0x80 /* Gamma Curve  3rd segment input end point */
+#define GAM4        0x81 /* Gamma Curve  4th segment input end point */
+#define GAM5        0x82 /* Gamma Curve  5th segment input end point */
+#define GAM6        0x83 /* Gamma Curve  6th segment input end point */
+#define GAM7        0x84 /* Gamma Curve  7th segment input end point */
+#define GAM8        0x85 /* Gamma Curve  8th segment input end point */
+#define GAM9        0x86 /* Gamma Curve  9th segment input end point */
+#define GAM10       0x87 /* Gamma Curve 10th segment input end point */
+#define GAM11       0x88 /* Gamma Curve 11th segment input end point */
+#define GAM12       0x89 /* Gamma Curve 12th segment input end point */
+#define GAM13       0x8A /* Gamma Curve 13th segment input end point */
+#define GAM14       0x8B /* Gamma Curve 14th segment input end point */
+#define GAM15       0x8C /* Gamma Curve 15th segment input end point */
+#define SLOP        0x8D /* Gamma curve highest segment slope */
+#define DNSTH       0x8E /* De-noise threshold */
+#define EDGE0       0x8F /* Edge enhancement control 0 */
+#define EDGE1       0x90 /* Edge enhancement control 1 */
+#define DNSOFF      0x91 /* Auto De-noise threshold control */
+#define EDGE2       0x92 /* Edge enhancement strength low  point control */
+#define EDGE3       0x93 /* Edge enhancement strength high point control */
+#define MTX1        0x94 /* Matrix coefficient 1 */
+#define MTX2        0x95 /* Matrix coefficient 2 */
+#define MTX3        0x96 /* Matrix coefficient 3 */
+#define MTX4        0x97 /* Matrix coefficient 4 */
+#define MTX5        0x98 /* Matrix coefficient 5 */
+#define MTX6        0x99 /* Matrix coefficient 6 */
+#define MTX_CTRL    0x9A /* Matrix control */
+#define BRIGHT      0x9B /* Brightness control */
+#define CNTRST      0x9C /* Contrast contrast */
+#define CNTRST_CTRL 0x9D /* Contrast contrast center */
+#define UVAD_J0     0x9E /* Auto UV adjust contrast 0 */
+#define UVAD_J1     0x9F /* Auto UV adjust contrast 1 */
+#define SCAL0       0xA0 /* Scaling control 0 */
+#define SCAL1       0xA1 /* Scaling control 1 */
+#define SCAL2       0xA2 /* Scaling control 2 */
+#define FIFODLYM    0xA3 /* FIFO manual mode delay control */
+#define FIFODLYA    0xA4 /* FIFO auto   mode delay control */
+#define SDE         0xA6 /* Special digital effect control */
+#define USAT        0xA7 /* U component saturation control */
+#define VSAT        0xA8 /* V component saturation control */
+#define HUE0        0xA9 /* Hue control 0 */
+#define HUE1        0xAA /* Hue control 1 */
+#define SIGN        0xAB /* Sign bit for Hue and contrast */
+#define DSPAUTO     0xAC /* DSP auto function ON/OFF control */
+
+/*
+ * register detail
+ */
+
+/* COM2 */
+#define SOFT_SLEEP_MODE (B4)    /* Soft sleep mode */
+				/* Output drive capability */
+#define OCAP_1x         (ZERO)	/* 1x */
+#define OCAP_2x         (B0)	/* 2x */
+#define OCAP_3x         (B1)	/* 3x */
+#define OCAP_4x         (B0|B1)	/* 4x */
+
+/* COM3 */
+#define SWAP_MASK       (B5|B4|B3)
+
+#define VFIMG_ON_OFF    (B7)	/* Vertical flip image ON/OFF selection */
+#define HMIMG_ON_OFF    (B6)	/* Horizontal mirror image ON/OFF selection */
+#define SWAP_RGB        (B5)	/* Swap B/R  output sequence in RGB mode */
+#define SWAP_YUV        (B4)	/* Swap Y/UV output sequence in YUV mode */
+#define SWAP_ML         (B3)	/* Swap output MSB/LSB */
+				/* Tri-state option for output clock */
+#define NOTRI_CLOCK     (B2)	/*   0: Tri-state    at this period */
+				/*   1: No tri-state at this period */
+				/* Tri-state option for output data */
+#define NOTRI_DATA      (B1)	/*   0: Tri-state    at this period */
+				/*   1: No tri-state at this period */
+#define SCOLOR_TEST     (B0)	/* Sensor color bar test pattern */
+
+/* COM4 */
+				/* PLL frequency control */
+#define PLL_BYPASS      (ZERO)	/*  00: Bypass PLL */
+#define PLL_4x          (B6)	/*  01: PLL 4x */
+#define PLL_6x          (B7)	/*  10: PLL 6x */
+#define PLL_8x          (B7|B6)	/*  11: PLL 8x */
+				/* AEC evaluate window */
+#define AEC_FULL        (ZERO)	/*  00: Full window */
+#define AEC_1p2         (B4)	/*  01: 1/2  window */
+#define AEC_1p4         (B5)	/*  10: 1/4  window */
+#define AEC_2p3         (B5|B4)	/*  11: Low 2/3 window */
+
+/* COM5 */
+#define AFR_ON_OFF      (B7)	/* Auto frame rate control ON/OFF selection */
+#define AFR_SPPED       (B6)	/* Auto frame rate control speed slection */
+				/* Auto frame rate max rate control */
+#define AFR_NO_RATE     (ZERO)	/*   No  reduction of frame rate */
+#define AFR_1p2         (B4)	/*   Max reduction to 1/2 frame rate */
+#define AFR_1p4         (B5)	/*   Max reduction to 1/4 frame rate */
+#define AFR_1p8         (B5|B4)	/*   Max reduction to 1/8 frame rate */
+				/* Auto frame rate active point control */
+#define AF_2x           (ZERO)	/*   Add frame when AGC reaches  2x gain */
+#define AF_4x           (B2)	/*   Add frame when AGC reaches  4x gain */
+#define AF_8x           (B3)	/*   Add frame when AGC reaches  8x gain */
+#define AF_16x          (B2|B3)	/*   Add frame when AGC reaches 16x gain */
+				/* AEC max step control */
+#define AEC_NO_LIMIT    (B0)	/*   0 : AEC incease step has limit */
+				/*   1 : No limit to AEC increase step */
+
+/* COM7 */
+				/* SCCB Register Reset */
+#define SCCB_RESET      (B7)	/*   0 : No change */
+				/*   1 : Resets all registers to default */
+				/* Resolution selection */
+#define SLCT_MASK       (B6)	/*   Mask of VGA or QVGA */
+#define SLCT_VGA        (ZERO)	/*   0 : VGA */
+#define SLCT_QVGA       (B6)	/*   1 : QVGA */
+#define ITU656_ON_OFF   (B5)	/* ITU656 protocol ON/OFF selection */
+				/* RGB output format control */
+#define FMT_GBR422      (ZERO)	/*   00 : GBR 4:2:2 */
+#define FMT_RGB565      (B2)	/*   01 : RGB 565 */
+#define FMT_RGB555      (B3)	/*   10 : RGB 555 */
+#define FMT_RGB444      (B3|B2)	/*   11 : RGB 444 */
+				/* Output format control */
+#define OFMT_YUV        (ZERO)	/*   00 : YUV */
+#define OFMT_P_BRAW     (B0)	/*   01 : Processed Bayer RAW */
+#define OFMT_RGB        (B1)	/*   10 : RGB */
+#define OFMT_BRAW       (B1|B0)	/*   11 : Bayer RAW */
+
+/* COM8 */
+#define FAST_ALGO       (B7)	/* Enable fast AGC/AEC algorithm */
+				/* AEC Setp size limit */
+#define UNLMT_STEP      (B6)	/*   0 : Step size is limited */
+				/*   1 : Unlimited step size */
+#define BNDF_ON_OFF     (B5)	/* Banding filter ON/OFF */
+#define AEC_BND         (B4)	/* Enable AEC below banding value */
+#define AEC_ON_OFF      (B3)	/* Fine AEC ON/OFF control */
+#define AGC_ON          (B2)	/* AGC Enable */
+#define AWB_ON          (B1)	/* AWB Enable */
+#define AEC_ON          (B0)	/* AEC Enable */
+
+/* COM9 */
+#define BASE_AECAGC     (B7)	/* Histogram or average based AEC/AGC */
+				/* Automatic gain ceiling - maximum AGC value */
+#define GAIN_2x         (ZERO)	/*   000 :   2x */
+#define GAIN_4x         (B4)	/*   001 :   4x */
+#define GAIN_8x         (B5)	/*   010 :   8x */
+#define GAIN_16x        (B4|B5)	/*   011 :  16x */
+#define GAIN_32x        (B6)	/*   100 :  32x */
+#define GAIN_64x        (B6|B4)	/*   101 :  64x */
+#define GAIN_128x       (B6|B5)	/*   110 : 128x */
+#define DROP_VSYNC      (B2)	/* Drop VSYNC output of corrupt frame */
+#define DROP_HREF       (B1)	/* Drop HREF  output of corrupt frame */
+
+/* COM11 */
+#define SGLF_ON_OFF     (B1)	/* Single frame ON/OFF selection */
+#define SGLF_TRIG       (B0)	/* Single frame transfer trigger */
+
+/* EXHCH */
+#define VSIZE_LSB       (B2)	/* Vertical data output size LSB */
+
+/* DSP_CTRL1 */
+#define FIFO_ON         (B7)	/* FIFO enable/disable selection */
+#define UV_ON_OFF       (B6)	/* UV adjust function ON/OFF selection */
+#define YUV444_2_422    (B5)	/* YUV444 to 422 UV channel option selection */
+#define CLR_MTRX_ON_OFF (B4)	/* Color matrix ON/OFF selection */
+#define INTPLT_ON_OFF   (B3)	/* Interpolation ON/OFF selection */
+#define GMM_ON_OFF      (B2)	/* Gamma function ON/OFF selection */
+#define AUTO_BLK_ON_OFF (B1)	/* Black defect auto correction ON/OFF */
+#define AUTO_WHT_ON_OFF (B0)	/* White define auto correction ON/OFF */
+
+/* DSP_CTRL3 */
+#define UV_MASK         (B7)	/* UV output sequence option */
+#define UV_ON           (B7)	/*   ON */
+#define UV_OFF          (ZERO)	/*   OFF */
+#define CBAR_MASK       (B5)	/* DSP Color bar mask */
+#define CBAR_ON         (B5)	/*   ON */
+#define CBAR_OFF        (ZERO)	/*   OFF */
+
+/* HSTART */
+#define HST_VGA  0x23
+#define HST_QVGA 0x3F
+
+/* HSIZE */
+#define HSZ_VGA  0xA0
+#define HSZ_QVGA 0x50
+
+/* VSTART */
+#define VST_VGA  0x07
+#define VST_QVGA 0x03
+
+/* VSIZE */
+#define VSZ_VGA  0xF0
+#define VSZ_QVGA 0x78
+
+/* HOUTSIZE */
+#define HOSZ_VGA   0xA0
+#define HOSZ_QVGA  0x50
+
+/* VOUTSIZE */
+#define VOSZ_VGA  0xF0
+#define VOSZ_QVGA 0x78
+
+/*
+ * bit configure (32 bit)
+ * this is used in struct ov772x_color_format :: option
+ */
+#define OP_UV       0x00000001
+#define OP_SWAP_RGB 0x00000002
+
+/*
+ * struct
+ */
+struct regval_list {
+	unsigned char reg_num;
+	unsigned char value;
+};
+
+struct ov772x_color_format {
+	char                 *name;
+	__u32                 fourcc;
+	struct regval_list   *regs;
+	unsigned int          option;
+};
+
+struct ov772x_win_size {
+	char               *name;
+	__u32               width;
+	__u32               height;
+	unsigned char       com7_bit;
+	struct regval_list *regs;
+};
+
+struct ov772x_priv {
+	struct ov772x_camera_info     *info;
+	struct i2c_client             *client;
+	struct soc_camera_device       icd;
+	struct ov772x_color_format    *fmt;
+	struct ov772x_win_size        *win;
+};
+
+#define ENDMARKER { 0xff, 0xff }
+
+static struct regval_list ov772x_default_regs[] =
+{
+	{ COM3,  ZERO },
+	{ COM4,  PLL_4x | 0x01 },
+	{ 0x16,  0x00 },  /* Mystery */
+	{ COM11, B4 },    /* Mystery */
+	{ 0x28,  0x00 },  /* Mystery */
+	{ HREF,  0x00 },
+	{ COM13, 0xe2 },  /* Mystery */
+	{ AREF0, 0xef },
+	{ AREF2, 0x60 },
+	{ AREF6, 0x7a },
+	ENDMARKER,
+};
+
+/*
+ * register setting for color format
+ */
+static struct regval_list ov772x_RGB555_regs[] = {
+	{ COM7, FMT_RGB555 | OFMT_RGB },
+	ENDMARKER,
+};
+
+static struct regval_list ov772x_RGB565_regs[] = {
+	{ COM7, FMT_RGB565 | OFMT_RGB },
+	ENDMARKER,
+};
+
+static struct regval_list ov772x_YYUV_regs[] = {
+	{ COM3, SWAP_YUV },
+	{ COM7, OFMT_YUV },
+	ENDMARKER,
+};
+
+static struct regval_list ov772x_UVYY_regs[] = {
+	{ COM7, OFMT_YUV },
+	ENDMARKER,
+};
+
+
+/*
+ * register setting for window size
+ */
+static struct regval_list ov772x_qvga_regs[] = {
+	{ HSTART,   HST_QVGA },
+	{ HSIZE,    HSZ_QVGA },
+	{ VSTART,   VST_QVGA },
+	{ VSIZE,    VSZ_QVGA  },
+	{ HOUTSIZE, HOSZ_QVGA },
+	{ VOUTSIZE, VOSZ_QVGA },
+	ENDMARKER,
+};
+
+static struct regval_list ov772x_vga_regs[] = {
+	{ HSTART,   HST_VGA },
+	{ HSIZE,    HSZ_VGA },
+	{ VSTART,   VST_VGA },
+	{ VSIZE,    VSZ_VGA },
+	{ HOUTSIZE, HOSZ_VGA },
+	{ VOUTSIZE, VOSZ_VGA },
+	ENDMARKER ,
+};
+
+/*
+ * supported format list
+ */
+
+#define SETFOURCC(type) .name = (#type), .fourcc = (V4L2_PIX_FMT_ ## type)
+static const struct soc_camera_data_format ov772x_fmt_lists[] = {
+	{
+		SETFOURCC(YUYV),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_JPEG,
+	},
+	{
+		SETFOURCC(YVYU),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_JPEG,
+	},
+	{
+		SETFOURCC(UYVY),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_JPEG,
+	},
+	{
+		SETFOURCC(RGB555),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+	},
+	{
+		SETFOURCC(RGB555X),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+	},
+	{
+		SETFOURCC(RGB565),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+	},
+	{
+		SETFOURCC(RGB565X),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+	},
+};
+
+/*
+ * color format list
+ */
+#define T_YUYV 0
+static struct ov772x_color_format ov772x_cfmts[] = {
+	[T_YUYV] = {
+		SETFOURCC(YUYV),
+		.regs   = ov772x_YYUV_regs,
+	},
+	{
+		SETFOURCC(YVYU),
+		.regs   = ov772x_YYUV_regs,
+		.option = OP_UV,
+	},
+	{
+		SETFOURCC(UYVY),
+		.regs   = ov772x_UVYY_regs,
+	},
+	{
+		SETFOURCC(RGB555),
+		.regs   = ov772x_RGB555_regs,
+		.option = OP_SWAP_RGB,
+	},
+	{
+		SETFOURCC(RGB555X),
+		.regs   = ov772x_RGB555_regs,
+	},
+	{
+		SETFOURCC(RGB565),
+		.regs   = ov772x_RGB565_regs,
+		.option = OP_SWAP_RGB,
+	},
+	{
+		SETFOURCC(RGB565X),
+		.regs   = ov772x_RGB565_regs,
+	},
+};
+
+
+/*
+ * window size list
+ */
+#define VGA_WIDTH   640
+#define VGA_HEIGHT  480
+#define QVGA_WIDTH  320
+#define QVGA_HEIGHT 240
+#define MAX_WIDTH   VGA_WIDTH
+#define MAX_HEIGHT  VGA_HEIGHT
+
+static struct ov772x_win_size ov772x_win_vga = {
+	.name     = "VGA",
+	.width    = VGA_WIDTH,
+	.height   = VGA_HEIGHT,
+	.com7_bit = SLCT_VGA,
+	.regs     = ov772x_vga_regs,
+};
+
+static struct ov772x_win_size ov772x_win_qvga = {
+	.name     = "QVGA",
+	.width    = QVGA_WIDTH,
+	.height   = QVGA_HEIGHT,
+	.com7_bit = SLCT_QVGA,
+	.regs     = ov772x_qvga_regs,
+};
+
+
+/*
+ * general function
+ */
+
+static int
+ov772x_write_array(struct i2c_client  *client,
+		   struct regval_list *vals)
+{
+	while (vals->reg_num != 0xff || vals->value != 0xff) {
+		int ret = i2c_smbus_write_byte_data(client,
+						    vals->reg_num,
+						    vals->value);
+		if (ret < 0)
+			return ret;
+		vals++;
+	}
+	return 0;
+}
+
+static int
+ov772x_mask_set(struct i2c_client *client,
+			       u8  command,
+			       u8  mask,
+			       u8  set)
+{
+	s32 val = i2c_smbus_read_byte_data(client, command);
+	val &= ~mask;
+	val |=  set;
+
+	return i2c_smbus_write_byte_data(client, command, val);
+}
+
+static int ov772x_reset(struct i2c_client *client)
+{
+	int ret = i2c_smbus_write_byte_data(client, COM7, SCCB_RESET);
+	msleep(1);
+	return ret;
+}
+
+/*
+ * soc_camera_ops function
+ */
+
+static int ov772x_init(struct soc_camera_device *icd)
+{
+	struct ov772x_priv     *priv = container_of(icd, struct ov772x_priv, icd);
+
+	if (priv->info->power)
+		priv->info->power(1);
+
+	ov772x_reset(priv->client);
+	return ov772x_write_array(priv->client, ov772x_default_regs);
+}
+
+static int ov772x_release(struct soc_camera_device *icd)
+{
+	struct ov772x_priv     *priv = container_of(icd, struct ov772x_priv, icd);
+
+	if (priv->info->power)
+		priv->info->power(0);
+
+	ov772x_reset(priv->client);
+	return 0;
+}
+
+static int ov772x_start_capture(struct soc_camera_device *icd)
+{
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+	int    ret ;
+
+
+	if (!priv->win)
+		priv->win = &ov772x_win_vga;
+	if (!priv->fmt)
+		priv->fmt = &ov772x_cfmts[T_YUYV];
+
+	/*
+	 * reset hardware
+	 */
+	ov772x_reset(priv->client);
+	ret = ov772x_write_array(priv->client, ov772x_default_regs);
+	if (ret < 0)
+		goto start_end;
+
+	/*
+	 * set color format
+	 */
+	ret = ov772x_write_array(priv->client, priv->fmt->regs);
+	if (ret < 0)
+		goto start_end;
+
+	/*
+	 * set size format
+	 */
+	ret = ov772x_write_array(priv->client, priv->win->regs);
+	if (ret < 0)
+		goto start_end;
+
+	/*
+	 * set COM7 bit ( QVGA or VGA )
+	 */
+	ret = ov772x_mask_set(priv->client,
+			      COM7, SLCT_MASK, priv->win->com7_bit);
+	if (ret < 0)
+		goto start_end;
+
+	/*
+	 * set UV setting
+	 */
+	if (priv->fmt->option & OP_UV) {
+		ret = ov772x_mask_set(priv->client,
+				      DSP_CTRL3, UV_MASK , UV_ON);
+		if (ret < 0)
+			goto start_end;
+	}
+
+	/*
+	 * set SWAP setting
+	 */
+	if (priv->fmt->option & OP_SWAP_RGB) {
+		ret = ov772x_mask_set(priv->client,
+				      COM3, SWAP_MASK , SWAP_RGB);
+		if (ret < 0)
+			goto start_end;
+	}
+
+	/*
+	 * color bar mode
+	 */
+	if (priv->info->color_bar) {
+		ret = ov772x_mask_set(priv->client,
+				DSP_CTRL3, CBAR_MASK  , CBAR_ON);
+		if (ret < 0)
+			goto start_end;
+	}
+
+	dev_info(&icd->dev,
+		 "format %s, win %s\n" , priv->fmt->name, priv->win->name);
+
+start_end:
+	priv->fmt = NULL;
+	priv->win = NULL;
+
+	return ret;
+}
+
+static int ov772x_stop_capture(struct soc_camera_device *icd)
+{
+	return 0;
+}
+
+static int ov772x_set_bus_param(struct soc_camera_device *icd,
+				unsigned long		  flags)
+{
+	int ret = 0;
+
+	/*
+	 * check bus width
+	 */
+	switch (flags & SOCAM_DATAWIDTH_MASK) {
+	case SOCAM_DATAWIDTH_10:
+	case SOCAM_DATAWIDTH_8:
+		break;
+	default:
+		dev_err(&icd->dev, "it is not 8 or 10 bit bus width\n");
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static unsigned long
+ov772x_query_bus_param(struct soc_camera_device *icd)
+{
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+
+	return  SOCAM_PCLK_SAMPLE_RISING |
+		SOCAM_HSYNC_ACTIVE_HIGH  |
+		SOCAM_VSYNC_ACTIVE_HIGH  |
+		SOCAM_MASTER             |
+		priv->info->buswidth;
+}
+
+static int ov772x_set_fmt_cap(struct soc_camera_device *icd   ,
+			      __u32                     pixfmt,
+			      struct v4l2_rect         *rect)
+{
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+	int ret = -EINVAL;
+	int i   = 0;
+
+	/*
+	 * select format
+	 */
+	priv->fmt = NULL;
+	for (i = 0; i < ARRAY_SIZE(ov772x_cfmts); i++) {
+		if (pixfmt == ov772x_cfmts[i].fourcc) {
+			priv->fmt = ov772x_cfmts + i;
+			ret = 0;
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static int ov772x_try_fmt_cap(struct soc_camera_device *icd,
+			      struct v4l2_format       *f)
+{
+	struct ov772x_priv     *priv = NULL;
+	struct v4l2_pix_format *pix  = &f->fmt.pix;
+
+	priv = container_of(icd, struct ov772x_priv, icd);
+
+	/* QVGA */
+	if ((pix->width  <= ov772x_win_qvga.width) ||
+	    (pix->height <= ov772x_win_qvga.height)) {
+		priv->win   = &ov772x_win_qvga;
+		pix->width  =  ov772x_win_qvga.width;
+		pix->height =  ov772x_win_qvga.height;
+	}
+
+	/* VGA */
+	else if ((pix->width  <= ov772x_win_vga.width) ||
+		 (pix->height <= ov772x_win_vga.height)) {
+		priv->win   = &ov772x_win_vga;
+		pix->width  =  ov772x_win_vga.width;
+		pix->height =  ov772x_win_vga.height;
+	}
+
+	pix->field = V4L2_FIELD_NONE;
+
+	return 0;
+}
+
+static int ov772x_video_probe(struct soc_camera_device *icd)
+{
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+
+	/*
+	 * We must have a parent by now. And it cannot be a wrong one.
+	 * So this entire test is completely redundant.
+	 */
+	if (!icd->dev.parent ||
+	    to_soc_camera_host(icd->dev.parent)->nr != icd->iface)
+		return -ENODEV;
+
+	/*
+	 * ov772x only use 8 or 10 bit bus width
+	 */
+	if ((SOCAM_DATAWIDTH_10 != priv->info->buswidth) &&
+	    (SOCAM_DATAWIDTH_8  != priv->info->buswidth)) {
+		dev_err(&icd->dev, "bus width error\n");
+		return -ENODEV;
+	}
+
+	/*
+	 * show product ID and manufacturer ID
+	 */
+	dev_info(&icd->dev,
+		 "ov772x Product ID %0x:%0x Manufacturer ID %x:%x\n" ,
+		 i2c_smbus_read_byte_data(priv->client, PID),
+		 i2c_smbus_read_byte_data(priv->client, VER),
+		 i2c_smbus_read_byte_data(priv->client, MIDH),
+		 i2c_smbus_read_byte_data(priv->client, MIDL));
+
+	icd->formats     = ov772x_fmt_lists;
+	icd->num_formats = ARRAY_SIZE(ov772x_fmt_lists);
+
+	return soc_camera_video_start(icd);
+}
+
+static void ov772x_video_remove(struct soc_camera_device *icd)
+{
+	soc_camera_video_stop(icd);
+}
+
+static struct soc_camera_ops ov772x_ops = {
+	.owner			= THIS_MODULE,
+	.probe			= ov772x_video_probe,
+	.remove			= ov772x_video_remove,
+	.init			= ov772x_init,
+	.release		= ov772x_release,
+	.start_capture		= ov772x_start_capture,
+	.stop_capture		= ov772x_stop_capture,
+	.set_fmt_cap		= ov772x_set_fmt_cap,
+	.try_fmt_cap		= ov772x_try_fmt_cap,
+	.set_bus_param		= ov772x_set_bus_param,
+	.query_bus_param	= ov772x_query_bus_param,
+};
+
+/*
+ * i2c_driver function
+ */
+
+static int ov772x_probe(struct i2c_client          *client,
+			const struct i2c_device_id *did)
+
+{
+	struct ov772x_priv         *priv = NULL;
+	struct ov772x_camera_info  *info = NULL;
+	struct soc_camera_device   *icd  = NULL;
+	int                         ret  = -ENODEV;
+
+	info = client->dev.platform_data;
+	if (!info)
+		return -EINVAL;
+
+	if (!i2c_check_functionality(to_i2c_adapter(client->dev.parent),
+				     I2C_FUNC_SMBUS_BYTE_DATA)) {
+		dev_err(&client->dev ,
+			"I2C-Adapter doesn't support I2C_FUNC_SMBUS_BYTE\n");
+		return -EIO;
+	}
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->info   = info;
+	priv->client = client;
+	priv->win    = NULL;
+	priv->fmt    = NULL;
+	i2c_set_clientdata(client, priv);
+
+	icd = &priv->icd;
+	icd->ops        = &ov772x_ops;
+	icd->control    = &client->dev;
+	icd->width_min  = 0;
+	icd->width_max  = MAX_WIDTH;
+	icd->height_min = 0;
+	icd->height_max = MAX_HEIGHT;
+	icd->y_skip_top = 0;
+	icd->iface      = priv->info->iface;
+
+
+	ret = soc_camera_device_register(icd);
+
+	if (ret)
+		kfree(priv);
+
+	return ret;
+}
+
+static int ov772x_remove(struct i2c_client *client)
+{
+	struct ov772x_priv *priv = i2c_get_clientdata(client);
+
+	soc_camera_device_unregister(&priv->icd);
+	kfree(priv);
+	return 0;
+}
+
+static const struct i2c_device_id ov772x_id[] = {
+	{"ov772x", 0},
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ov772x_id);
+
+
+static struct i2c_driver ov772x_i2c_driver = {
+	.driver = {
+		.name = "ov772x",
+	},
+	.probe    = ov772x_probe,
+	.remove   = ov772x_remove,
+	.id_table = ov772x_id,
+};
+
+/*
+ * module function
+ */
+
+static int __init ov772x_module_init(void)
+{
+	printk(KERN_NOTICE "ov772x driver (soc framework)\n");
+	return i2c_add_driver(&ov772x_i2c_driver);
+}
+
+static void __exit ov772x_module_exit(void)
+{
+	i2c_del_driver(&ov772x_i2c_driver);
+}
+
+module_init(ov772x_module_init);
+module_exit(ov772x_module_exit);
+
+MODULE_DESCRIPTION("SoC Camera Platform driver for ov772x");
+MODULE_AUTHOR("Kuninori Morimoto");
+MODULE_LICENSE("GPL v2");
diff --git a/include/media/ov772x.h b/include/media/ov772x.h
new file mode 100644
index 0000000..2fe3a1a
--- /dev/null
+++ b/include/media/ov772x.h
@@ -0,0 +1,13 @@
+#ifndef __SOC_CAMERA_H__
+#define __SOC_CAMERA_H__
+
+#include <linux/videodev2.h>
+
+struct ov772x_camera_info {
+	int             iface;
+	unsigned long   buswidth;
+	int             color_bar;
+	void (*power)(int);
+};
+
+#endif /* __SOC_CAMERA_H__ */
-- 
1.5.4.3

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-15 10:52 [PATCH] Add ov772x driver Kuninori Morimoto
@ 2008-10-15 23:41 ` Guennadi Liakhovetski
  2008-10-16  1:28   ` morimoto.kuninori
  2008-10-16  2:21   ` Magnus Damm
  0 siblings, 2 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2008-10-15 23:41 UTC (permalink / raw)
  To: Kuninori Morimoto; +Cc: V4L

Hi, and thanks for the patch! A couple of notes:

(Magnus, see a note for you at the bottom:-))

first, you didn't add an ID for your sensor to 
include/media/v4l2-chip-ident.h, it is not reqlly required for your driver 
functionality, but you can use it to provide as a reply to the 
.vidioc_g_chip_ident request (.get_chip_id in struct soc_camera_ops).

On Wed, 15 Oct 2008, Kuninori Morimoto wrote:

> This patch adds ov772x driver that use soc_camera framework.
> It was tested on SH Migo-r board.
> 
> Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
> ---
>  drivers/media/video/Kconfig  |    6 +
>  drivers/media/video/Makefile |    1 +
>  drivers/media/video/ov772x.c |  957 ++++++++++++++++++++++++++++++++++++++++++
>  include/media/ov772x.h       |   13 +
>  4 files changed, 977 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/media/video/ov772x.c
>  create mode 100644 include/media/ov772x.h
> 
> diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> index 47102c2..d0c4152 100644
> --- a/drivers/media/video/Kconfig
> +++ b/drivers/media/video/Kconfig
> @@ -750,6 +750,12 @@ config SOC_CAMERA_PLATFORM
>  	help
>  	  This is a generic SoC camera platform driver, useful for testing
>  
> +config SOC_CAMERA_OV772X
> +	tristate "ov772x camera support"
> +	depends on SOC_CAMERA

better

+	depends on SOC_CAMERA && I2C

?

> +	help
> +	  This is a ov772x camera platform driver on soc framework

This is not a platform driver, it's a i2c and an soc-camera (device) 
driver.

> +
>  config VIDEO_PXA27x
>  	tristate "PXA27x Quick Capture Interface driver"
>  	depends on VIDEO_DEV && PXA27x && SOC_CAMERA
> diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
> index 16962f3..d281330 100644
> --- a/drivers/media/video/Makefile
> +++ b/drivers/media/video/Makefile
> @@ -134,6 +134,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9M001)	+= mt9m001.o
>  obj-$(CONFIG_SOC_CAMERA_MT9M111)	+= mt9m111.o
>  obj-$(CONFIG_SOC_CAMERA_MT9V022)	+= mt9v022.o
>  obj-$(CONFIG_SOC_CAMERA_PLATFORM)	+= soc_camera_platform.o
> +obj-$(CONFIG_SOC_CAMERA_OV772X)		+= ov772x.o
>  
>  obj-$(CONFIG_VIDEO_AU0828) += au0828/
>  
> diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
> new file mode 100644
> index 0000000..0db0319
> --- /dev/null
> +++ b/drivers/media/video/ov772x.c
> @@ -0,0 +1,957 @@
> +/*
> + * ov772x Camera Driver
> + *
> + * Copyright (C) 2008 Renesas Solutions Corp.
> + * Kuninori Morimoto <morimoto.kuninori@renesas.com>
> + *
> + * Based on ov7670 and soc_camera_platform driver,
> + *
> + * Copyright 2006-7 Jonathan Corbet <corbet@lwn.net>
> + * Copyright (C) 2008 Magnus Damm
> + * Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/platform_device.h>

I don't think you need to include platform_device.h

[snip]

> +static struct regval_list ov772x_default_regs[] =
> +{
> +	{ COM3,  ZERO },
> +	{ COM4,  PLL_4x | 0x01 },
> +	{ 0x16,  0x00 },  /* Mystery */
> +	{ COM11, B4 },    /* Mystery */
> +	{ 0x28,  0x00 },  /* Mystery */
> +	{ HREF,  0x00 },
> +	{ COM13, 0xe2 },  /* Mystery */

Those "Mystery" register - are they not documented, or has the driver been 
reverse-engineered?

> +static int ov772x_start_capture(struct soc_camera_device *icd)
> +{
> +	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
> +	int    ret ;

Please, no space before ";" - this is just a single occurrence, must by a 
typo.

> +
> +
> +	if (!priv->win)
> +		priv->win = &ov772x_win_vga;
> +	if (!priv->fmt)
> +		priv->fmt = &ov772x_cfmts[T_YUYV];
> +
> +	/*
> +	 * reset hardware
> +	 */
> +	ov772x_reset(priv->client);
> +	ret = ov772x_write_array(priv->client, ov772x_default_regs);
> +	if (ret < 0)
> +		goto start_end;
> +
> +	/*
> +	 * set color format
> +	 */
> +	ret = ov772x_write_array(priv->client, priv->fmt->regs);
> +	if (ret < 0)
> +		goto start_end;
> +
> +	/*
> +	 * set size format
> +	 */
> +	ret = ov772x_write_array(priv->client, priv->win->regs);
> +	if (ret < 0)
> +		goto start_end;
> +
> +	/*
> +	 * set COM7 bit ( QVGA or VGA )
> +	 */
> +	ret = ov772x_mask_set(priv->client,
> +			      COM7, SLCT_MASK, priv->win->com7_bit);
> +	if (ret < 0)
> +		goto start_end;
> +
> +	/*
> +	 * set UV setting
> +	 */
> +	if (priv->fmt->option & OP_UV) {
> +		ret = ov772x_mask_set(priv->client,
> +				      DSP_CTRL3, UV_MASK , UV_ON);
> +		if (ret < 0)
> +			goto start_end;
> +	}
> +
> +	/*
> +	 * set SWAP setting
> +	 */
> +	if (priv->fmt->option & OP_SWAP_RGB) {
> +		ret = ov772x_mask_set(priv->client,
> +				      COM3, SWAP_MASK , SWAP_RGB);
> +		if (ret < 0)
> +			goto start_end;
> +	}
> +
> +	/*
> +	 * color bar mode
> +	 */
> +	if (priv->info->color_bar) {
> +		ret = ov772x_mask_set(priv->client,
> +				DSP_CTRL3, CBAR_MASK  , CBAR_ON);
> +		if (ret < 0)
> +			goto start_end;
> +	}

What is this "color bar mode" and why do you think you need it to be 
specified by the platform data (also see below)?

> +
> +	dev_info(&icd->dev,
> +		 "format %s, win %s\n" , priv->fmt->name, priv->win->name);
> +
> +start_end:
> +	priv->fmt = NULL;
> +	priv->win = NULL;
> +
> +	return ret;
> +}
> +
> +static int ov772x_stop_capture(struct soc_camera_device *icd)
> +{
> +	return 0;
> +}
> +
> +static int ov772x_set_bus_param(struct soc_camera_device *icd,
> +				unsigned long		  flags)
> +{
> +	int ret = 0;
> +
> +	/*
> +	 * check bus width
> +	 */
> +	switch (flags & SOCAM_DATAWIDTH_MASK) {
> +	case SOCAM_DATAWIDTH_10:
> +	case SOCAM_DATAWIDTH_8:

How does it work in both 8- and 10-bit modes without any reconfiguration? 
Are then just the upper 8 bits connected to the interface? Is it 
hard-wired, or software-switchable at runtime?

> +		break;
> +	default:
> +		dev_err(&icd->dev, "it is not 8 or 10 bit bus width\n");
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static unsigned long
> +ov772x_query_bus_param(struct soc_camera_device *icd)

In most of your function definitions you put the type and the name on one 
line, but a few you split, even though it wouldn't exceed 80 characters on 
one line either. Please put them all on one line.

> +{
> +	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
> +
> +	return  SOCAM_PCLK_SAMPLE_RISING |
> +		SOCAM_HSYNC_ACTIVE_HIGH  |
> +		SOCAM_VSYNC_ACTIVE_HIGH  |
> +		SOCAM_MASTER             |
> +		priv->info->buswidth;
> +}
> +
> +static int ov772x_set_fmt_cap(struct soc_camera_device *icd   ,

Please, no spaces before commas - there are multiple places like this, 
please, fix them all.

> +			      __u32                     pixfmt,
> +			      struct v4l2_rect         *rect)
> +{
> +	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
> +	int ret = -EINVAL;
> +	int i   = 0;

Superfluous initialisation of "i", please, remove. Same in 
ov772x_try_fmt_cap() for "priv".

[snip]

> +static int ov772x_probe(struct i2c_client          *client,
> +			const struct i2c_device_id *did)
> +
> +{
> +	struct ov772x_priv         *priv = NULL;
> +	struct ov772x_camera_info  *info = NULL;
> +	struct soc_camera_device   *icd  = NULL;
> +	int                         ret  = -ENODEV;

Ditto - for all four variables.

> +static int __init ov772x_module_init(void)
> +{
> +	printk(KERN_NOTICE "ov772x driver (soc framework)\n");

KERN_INFO would be quite enough:-) And it's soc-camera, just "soc" is too 
generic, and would have to be written SoC.

> +	return i2c_add_driver(&ov772x_i2c_driver);
> +}
> +
> +static void __exit ov772x_module_exit(void)
> +{
> +	i2c_del_driver(&ov772x_i2c_driver);
> +}
> +
> +module_init(ov772x_module_init);
> +module_exit(ov772x_module_exit);
> +
> +MODULE_DESCRIPTION("SoC Camera Platform driver for ov772x");

No "Platform".

Also in multiple places like

> +	if ((pix->width  <= ov772x_win_qvga.width) ||
> +	    (pix->height <= ov772x_win_qvga.height)) {

the internal parenthesis are not needed, but that's not really critical, 
might be characterised as a matter of personal preference:-)

> +MODULE_AUTHOR("Kuninori Morimoto");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/media/ov772x.h b/include/media/ov772x.h
> new file mode 100644
> index 0000000..2fe3a1a
> --- /dev/null
> +++ b/include/media/ov772x.h
> @@ -0,0 +1,13 @@
> +#ifndef __SOC_CAMERA_H__
> +#define __SOC_CAMERA_H__

Wrong protective __SOC_CAMERA_H__ macro... You're lucky I didn't use that 
many underscores:-) Also Copyright / license header missing.

> +
> +#include <linux/videodev2.h>

Include not needed.

> +
> +struct ov772x_camera_info {
> +	int             iface;
> +	unsigned long   buswidth;
> +	int             color_bar;
> +	void (*power)(int);
> +};

Now, this one. Please, use struct soc_camera_link. It also provides bus_id 
(your iface), power, ok, I admit, the inclusion of the "gpio" member in it 
was a mistake of mine, it is too specific, we might remove it at some 
point. I am not sure you really need color_bar and bus_width. I think, 
cameras are more or less exchangeable parts, and if they need some 
parameters, that cannot be autoprobed and do not belong to the camera 
itself, it might be better to make them module parameters, like the 
sensor_type parameter in mt9v022. Even if in your case the sensor chip is 
soldered on the board, in another configuration it might not be.

Magnus, I think, we should switch soc_camera_platform to use 
soc_camera_link too.

In both ov772x and soc_camera_platform cases, if you do need extra 
platform information, just embed soc_camera_link in your struct.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-15 23:41 ` Guennadi Liakhovetski
@ 2008-10-16  1:28   ` morimoto.kuninori
  2008-10-16  2:21   ` Magnus Damm
  1 sibling, 0 replies; 22+ messages in thread
From: morimoto.kuninori @ 2008-10-16  1:28 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: V4L


Hi Guennadi Liakhovetski

> first, you didn't add an ID for your sensor to 
> include/media/v4l2-chip-ident.h, it is not reqlly required for your driver 
> functionality, but you can use it to provide as a reply to the 
> .vidioc_g_chip_ident request (.get_chip_id in struct soc_camera_ops).
(snip)
> Now, this one. Please, use struct soc_camera_link. It also provides bus_id 
> (your iface), power, ok, I admit, the inclusion of the "gpio" member in it 
> was a mistake of mine, it is too specific, we might remove it at some 
> point. I am not sure you really need color_bar and bus_width. I think, 
> cameras are more or less exchangeable parts, and if they need some 
> parameters, that cannot be autoprobed and do not belong to the camera 
> itself, it might be better to make them module parameters, like the 
> sensor_type parameter in mt9v022. Even if in your case the sensor chip is 
> soldered on the board, in another configuration it might not be.

Thanks a lot.

I'll fix it up and send again.

Thanks
Morimoto

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-15 23:41 ` Guennadi Liakhovetski
  2008-10-16  1:28   ` morimoto.kuninori
@ 2008-10-16  2:21   ` Magnus Damm
  2008-10-16  6:24     ` Guennadi Liakhovetski
  1 sibling, 1 reply; 22+ messages in thread
From: Magnus Damm @ 2008-10-16  2:21 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: V4L

Hi Guennadi,

On Thu, Oct 16, 2008 at 8:41 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Hi, and thanks for the patch! A couple of notes:
>
> (Magnus, see a note for you at the bottom:-))
>
> first, you didn't add an ID for your sensor to
> include/media/v4l2-chip-ident.h, it is not reqlly required for your driver
> functionality, but you can use it to provide as a reply to the
> .vidioc_g_chip_ident request (.get_chip_id in struct soc_camera_ops).
>
> On Wed, 15 Oct 2008, Kuninori Morimoto wrote:
>
>> This patch adds ov772x driver that use soc_camera framework.
>> It was tested on SH Migo-r board.
>>
>> Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>

[snip]

>> +static struct regval_list ov772x_default_regs[] =
>> +{
>> +     { COM3,  ZERO },
>> +     { COM4,  PLL_4x | 0x01 },
>> +     { 0x16,  0x00 },  /* Mystery */
>> +     { COM11, B4 },    /* Mystery */
>> +     { 0x28,  0x00 },  /* Mystery */
>> +     { HREF,  0x00 },
>> +     { COM13, 0xe2 },  /* Mystery */
>
> Those "Mystery" register - are they not documented, or has the driver been
> reverse-engineered?

The data sheet does not cover them, but I think they come from the
magic init sequence in the Migo-R board code.

>> +     /*
>> +      * color bar mode
>> +      */
>> +     if (priv->info->color_bar) {
>> +             ret = ov772x_mask_set(priv->client,
>> +                             DSP_CTRL3, CBAR_MASK  , CBAR_ON);
>> +             if (ret < 0)
>> +                     goto start_end;
>> +     }
>
> What is this "color bar mode" and why do you think you need it to be
> specified by the platform data (also see below)?

The color bar mode is a camera test mode where color bars similar to
the vivi driver are output instead of the camera image. It's very
useful for testing and getting byte order issues resolved. Ideally I'd
like to have it as a second output, but I have to extend the SoC
camera framework first to get that in place.

I'm not sure if platform data is the best place to enable this, but I
guess it's good enough.

>> +static int ov772x_set_bus_param(struct soc_camera_device *icd,
>> +                             unsigned long             flags)
>> +{
>> +     int ret = 0;
>> +
>> +     /*
>> +      * check bus width
>> +      */
>> +     switch (flags & SOCAM_DATAWIDTH_MASK) {
>> +     case SOCAM_DATAWIDTH_10:
>> +     case SOCAM_DATAWIDTH_8:
>
> How does it work in both 8- and 10-bit modes without any reconfiguration?
> Are then just the upper 8 bits connected to the interface? Is it
> hard-wired, or software-switchable at runtime?

The device has 10 data pins but we use 8 on Migo-R. I think some extra
pixel formats are available with 10-bit interface but I'm not sure.
The data sheet is not very clear.

>> +
>> +struct ov772x_camera_info {
>> +     int             iface;
>> +     unsigned long   buswidth;
>> +     int             color_bar;
>> +     void (*power)(int);
>> +};
>
> Now, this one. Please, use struct soc_camera_link. It also provides bus_id
> (your iface), power, ok, I admit, the inclusion of the "gpio" member in it
> was a mistake of mine, it is too specific, we might remove it at some
> point. I am not sure you really need color_bar and bus_width. I think,
> cameras are more or less exchangeable parts, and if they need some
> parameters, that cannot be autoprobed and do not belong to the camera
> itself, it might be better to make them module parameters, like the
> sensor_type parameter in mt9v022. Even if in your case the sensor chip is
> soldered on the board, in another configuration it might not be.

Using soc_camera_link sounds like a good idea. I don't agree with you
regarding the module parameters - doing that removes the
per-camera-instance configuration that the platform data gives us.

> Magnus, I think, we should switch soc_camera_platform to use
> soc_camera_link too.

Sure.

> In both ov772x and soc_camera_platform cases, if you do need extra
> platform information, just embed soc_camera_link in your struct.

That seems the best way forward.

Cheers,

/ magnus

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH] Add ov772x driver
@ 2008-10-16  4:28 Kuninori Morimoto
  2008-10-16  8:27 ` Antonio Ospite
  0 siblings, 1 reply; 22+ messages in thread
From: Kuninori Morimoto @ 2008-10-16  4:28 UTC (permalink / raw)
  To: V4L; +Cc: mchehab

This patch adds ov772x driver that use soc_camera framework.
It was tested on SH Migo-r board.

Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
---
 drivers/media/video/Kconfig     |    6 +
 drivers/media/video/Makefile    |    1 +
 drivers/media/video/ov772x.c    |  960 +++++++++++++++++++++++++++++++++++++++
 include/media/ov772x.h          |   12 +
 include/media/v4l2-chip-ident.h |    1 +
 5 files changed, 980 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/video/ov772x.c
 create mode 100644 include/media/ov772x.h

diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 47102c2..7b363da 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -750,6 +750,12 @@ config SOC_CAMERA_PLATFORM
 	help
 	  This is a generic SoC camera platform driver, useful for testing
 
+config SOC_CAMERA_OV772X
+	tristate "ov772x camera support"
+	depends on SOC_CAMERA && I2C
+	help
+	  This is a ov772x camera driver
+
 config VIDEO_PXA27x
 	tristate "PXA27x Quick Capture Interface driver"
 	depends on VIDEO_DEV && PXA27x && SOC_CAMERA
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index 16962f3..d281330 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -134,6 +134,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9M001)	+= mt9m001.o
 obj-$(CONFIG_SOC_CAMERA_MT9M111)	+= mt9m111.o
 obj-$(CONFIG_SOC_CAMERA_MT9V022)	+= mt9v022.o
 obj-$(CONFIG_SOC_CAMERA_PLATFORM)	+= soc_camera_platform.o
+obj-$(CONFIG_SOC_CAMERA_OV772X)		+= ov772x.o
 
 obj-$(CONFIG_VIDEO_AU0828) += au0828/
 
diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
new file mode 100644
index 0000000..be4735b
--- /dev/null
+++ b/drivers/media/video/ov772x.c
@@ -0,0 +1,960 @@
+/*
+ * ov772x Camera Driver
+ *
+ * Copyright (C) 2008 Renesas Solutions Corp.
+ * Kuninori Morimoto <morimoto.kuninori@renesas.com>
+ *
+ * Based on ov7670 and soc_camera_platform driver,
+ *
+ * Copyright 2006-7 Jonathan Corbet <corbet@lwn.net>
+ * Copyright (C) 2008 Magnus Damm
+ * Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <media/v4l2-chip-ident.h>
+#include <linux/videodev2.h>
+#include <media/v4l2-common.h>
+#include <media/soc_camera.h>
+#include <media/ov772x.h>
+
+/*
+ * bit defines
+ */
+#define ZERO 0x00
+#define B0   0x01
+#define B1   0x02
+#define B2   0x04
+#define B3   0x08
+
+#define B4   0x10
+#define B5   0x20
+#define B6   0x40
+#define B7   0x80
+
+/*
+ * register offset
+ */
+#define GAIN        0x00 /* AGC - Gain control gain setting */
+#define BLUE        0x01 /* AWB - Blue channel gain setting */
+#define RED         0x02 /* AWB - Red   channel gain setting */
+#define GREEN       0x03 /* AWB - Green channel gain setting */
+#define COM1        0x04 /* Common control 1 */
+#define BAVG        0x05 /* U/B Average Level */
+#define GAVG        0x06 /* Y/Gb Average Level */
+#define RAVG        0x07 /* V/R Average Level */
+#define AECH        0x08 /* Exposure Value - AEC MSBs */
+#define COM2        0x09 /* Common control 2 */
+#define PID         0x0A /* Product ID Number MSB */
+#define VER         0x0B /* Product ID Number LSB */
+#define COM3        0x0C /* Common control 3 */
+#define COM4        0x0D /* Common control 4 */
+#define COM5        0x0E /* Common control 5 */
+#define COM6        0x0F /* Common control 6 */
+#define AEC         0x10 /* Exposure Value */
+#define CLKRC       0x11 /* Internal clock */
+#define COM7        0x12 /* Common control 7 */
+#define COM8        0x13 /* Common control 8 */
+#define COM9        0x14 /* Common control 9 */
+#define COM10       0x15 /* Common control 10 */
+#define HSTART      0x17 /* Horizontal sensor size */
+#define HSIZE       0x18 /* Horizontal frame (HREF column) end high 8-bit */
+#define VSTART      0x19 /* Vertical frame (row) start high 8-bit */
+#define VSIZE       0x1A /* Vertical sensor size */
+#define PSHFT       0x1B /* Data format - pixel delay select */
+#define MIDH        0x1C /* Manufacturer ID byte - high */
+#define MIDL        0x1D /* Manufacturer ID byte - low  */
+#define LAEC        0x1F /* Fine AEC value */
+#define COM11       0x20 /* Common control 11 */
+#define BDBASE      0x22 /* Banding filter Minimum AEC value */
+#define DBSTEP      0x23 /* Banding filter Maximum Setp */
+#define AEW         0x24 /* AGC/AEC - Stable operating region (upper limit) */
+#define AEB         0x25 /* AGC/AEC - Stable operating region (lower limit) */
+#define VPT         0x26 /* AGC/AEC Fast mode operating region */
+#define HOUTSIZE    0x29 /* Horizontal data output size MSBs */
+#define EXHCH       0x2A /* Dummy pixel insert MSB */
+#define EXHCL       0x2B /* Dummy pixel insert LSB */
+#define VOUTSIZE    0x2C /* Vertical data output size MSBs */
+#define ADVFL       0x2D /* LSB of insert dummy lines in Vertical direction */
+#define ADVFH       0x2E /* MSG of insert dummy lines in Vertical direction */
+#define YAVE        0x2F /* Y/G Channel Average value */
+#define LUMHTH      0x30 /* Histogram AEC/AGC Luminance high level threshold */
+#define LUMLTH      0x31 /* Histogram AEC/AGC Luminance low  level threshold */
+#define HREF        0x32 /* Image start and size control */
+#define DM_LNL      0x33 /* Dummy line low  8 bits */
+#define DM_LNH      0x34 /* Dummy line high 8 bits */
+#define ADOFF_B     0x35 /* AD offset compensation value for B  channel */
+#define ADOFF_R     0x36 /* AD offset compensation value for R  channel */
+#define ADOFF_GB    0x37 /* AD offset compensation value for Gb channel */
+#define ADOFF_GR    0x38 /* AD offset compensation value for Gr channel */
+#define OFF_B       0x39 /* Analog process B  channel offset value */
+#define OFF_R       0x3A /* Analog process R  channel offset value */
+#define OFF_GB      0x3B /* Analog process Gb channel offset value */
+#define OFF_GR      0x3C /* Analog process Gr channel offset value */
+#define COM12       0x3D /* Common control 12 */
+#define COM13       0x3E /* Common control 13 */
+#define COM14       0x3F /* Common control 14 */
+#define COM15       0x40 /* Common control 15*/
+#define COM16       0x41 /* Common control 16 */
+#define TGT_B       0x42 /* BLC blue channel target value */
+#define TGT_R       0x43 /* BLC red  channel target value */
+#define TGT_GB      0x44 /* BLC Gb   channel target value */
+#define TGT_GR      0x45 /* BLC Gr   channel target value */
+#define LCC0        0x46 /* Lens correction control 0 */
+#define LCC1        0x47 /* Lens correction option 1 - X coordinate */
+#define LCC2        0x48 /* Lens correction option 2 - Y coordinate */
+#define LCC3        0x49 /* Lens correction option 3 */
+#define LCC4        0x4A /* Lens correction option 4 - radius of the circular */
+#define LCC5        0x4B /* Lens correction option 5 */
+#define LCC6        0x4C /* Lens correction option 6 */
+#define FIXGAIN     0x4D /* Analog fix gain amplifer */
+#define AREF0       0x4E /* Sensor reference control */
+#define AREF1       0x4F /* Sensor reference current control */
+#define AREF2       0x50 /* Analog reference control */
+#define AREF3       0x51 /* ADC    reference control */
+#define AREF4       0x52 /* ADC    reference control */
+#define AREF5       0x53 /* ADC    reference control */
+#define AREF6       0x54 /* Analog reference control */
+#define AREF7       0x55 /* Analog reference control */
+#define UFIX        0x60 /* U channel fixed value output */
+#define VFIX        0x61 /* V channel fixed value output */
+#define AW_BB_BLK   0x62 /* AWB option for advanced AWB */
+#define AW_B_CTRL0  0x63 /* AWB control byte 0 */
+#define DSP_CTRL1   0x64 /* DSP control byte 1 */
+#define DSP_CTRL2   0x65 /* DSP control byte 2 */
+#define DSP_CTRL3   0x66 /* DSP control byte 3 */
+#define DSP_CTRL4   0x67 /* DSP control byte 4 */
+#define AW_B_BIAS   0x68 /* AWB BLC level clip */
+#define AW_BCTRL1   0x69 /* AWB control  1 */
+#define AW_BCTRL2   0x6A /* AWB control  2 */
+#define AW_BCTRL3   0x6B /* AWB control  3 */
+#define AW_BCTRL4   0x6C /* AWB control  4 */
+#define AW_BCTRL5   0x6D /* AWB control  5 */
+#define AW_BCTRL6   0x6E /* AWB control  6 */
+#define AW_BCTRL7   0x6F /* AWB control  7 */
+#define AW_BCTRL8   0x70 /* AWB control  8 */
+#define AW_BCTRL9   0x71 /* AWB control  9 */
+#define AW_BCTRL10  0x72 /* AWB control 10 */
+#define AW_BCTRL11  0x73 /* AWB control 11 */
+#define AW_BCTRL12  0x74 /* AWB control 12 */
+#define AW_BCTRL13  0x75 /* AWB control 13 */
+#define AW_BCTRL14  0x76 /* AWB control 14 */
+#define AW_BCTRL15  0x77 /* AWB control 15 */
+#define AW_BCTRL16  0x78 /* AWB control 16 */
+#define AW_BCTRL17  0x79 /* AWB control 17 */
+#define AW_BCTRL18  0x7A /* AWB control 18 */
+#define AW_BCTRL19  0x7B /* AWB control 19 */
+#define AW_BCTRL20  0x7C /* AWB control 20 */
+#define AW_BCTRL21  0x7D /* AWB control 21 */
+#define GAM1        0x7E /* Gamma Curve  1st segment input end point */
+#define GAM2        0x7F /* Gamma Curve  2nd segment input end point */
+#define GAM3        0x80 /* Gamma Curve  3rd segment input end point */
+#define GAM4        0x81 /* Gamma Curve  4th segment input end point */
+#define GAM5        0x82 /* Gamma Curve  5th segment input end point */
+#define GAM6        0x83 /* Gamma Curve  6th segment input end point */
+#define GAM7        0x84 /* Gamma Curve  7th segment input end point */
+#define GAM8        0x85 /* Gamma Curve  8th segment input end point */
+#define GAM9        0x86 /* Gamma Curve  9th segment input end point */
+#define GAM10       0x87 /* Gamma Curve 10th segment input end point */
+#define GAM11       0x88 /* Gamma Curve 11th segment input end point */
+#define GAM12       0x89 /* Gamma Curve 12th segment input end point */
+#define GAM13       0x8A /* Gamma Curve 13th segment input end point */
+#define GAM14       0x8B /* Gamma Curve 14th segment input end point */
+#define GAM15       0x8C /* Gamma Curve 15th segment input end point */
+#define SLOP        0x8D /* Gamma curve highest segment slope */
+#define DNSTH       0x8E /* De-noise threshold */
+#define EDGE0       0x8F /* Edge enhancement control 0 */
+#define EDGE1       0x90 /* Edge enhancement control 1 */
+#define DNSOFF      0x91 /* Auto De-noise threshold control */
+#define EDGE2       0x92 /* Edge enhancement strength low  point control */
+#define EDGE3       0x93 /* Edge enhancement strength high point control */
+#define MTX1        0x94 /* Matrix coefficient 1 */
+#define MTX2        0x95 /* Matrix coefficient 2 */
+#define MTX3        0x96 /* Matrix coefficient 3 */
+#define MTX4        0x97 /* Matrix coefficient 4 */
+#define MTX5        0x98 /* Matrix coefficient 5 */
+#define MTX6        0x99 /* Matrix coefficient 6 */
+#define MTX_CTRL    0x9A /* Matrix control */
+#define BRIGHT      0x9B /* Brightness control */
+#define CNTRST      0x9C /* Contrast contrast */
+#define CNTRST_CTRL 0x9D /* Contrast contrast center */
+#define UVAD_J0     0x9E /* Auto UV adjust contrast 0 */
+#define UVAD_J1     0x9F /* Auto UV adjust contrast 1 */
+#define SCAL0       0xA0 /* Scaling control 0 */
+#define SCAL1       0xA1 /* Scaling control 1 */
+#define SCAL2       0xA2 /* Scaling control 2 */
+#define FIFODLYM    0xA3 /* FIFO manual mode delay control */
+#define FIFODLYA    0xA4 /* FIFO auto   mode delay control */
+#define SDE         0xA6 /* Special digital effect control */
+#define USAT        0xA7 /* U component saturation control */
+#define VSAT        0xA8 /* V component saturation control */
+#define HUE0        0xA9 /* Hue control 0 */
+#define HUE1        0xAA /* Hue control 1 */
+#define SIGN        0xAB /* Sign bit for Hue and contrast */
+#define DSPAUTO     0xAC /* DSP auto function ON/OFF control */
+
+/*
+ * register detail
+ */
+
+/* COM2 */
+#define SOFT_SLEEP_MODE (B4)    /* Soft sleep mode */
+				/* Output drive capability */
+#define OCAP_1x         (ZERO)	/* 1x */
+#define OCAP_2x         (B0)	/* 2x */
+#define OCAP_3x         (B1)	/* 3x */
+#define OCAP_4x         (B0|B1)	/* 4x */
+
+/* COM3 */
+#define SWAP_MASK       (B5|B4|B3)
+
+#define VFIMG_ON_OFF    (B7)	/* Vertical flip image ON/OFF selection */
+#define HMIMG_ON_OFF    (B6)	/* Horizontal mirror image ON/OFF selection */
+#define SWAP_RGB        (B5)	/* Swap B/R  output sequence in RGB mode */
+#define SWAP_YUV        (B4)	/* Swap Y/UV output sequence in YUV mode */
+#define SWAP_ML         (B3)	/* Swap output MSB/LSB */
+				/* Tri-state option for output clock */
+#define NOTRI_CLOCK     (B2)	/*   0: Tri-state    at this period */
+				/*   1: No tri-state at this period */
+				/* Tri-state option for output data */
+#define NOTRI_DATA      (B1)	/*   0: Tri-state    at this period */
+				/*   1: No tri-state at this period */
+#define SCOLOR_TEST     (B0)	/* Sensor color bar test pattern */
+
+/* COM4 */
+				/* PLL frequency control */
+#define PLL_BYPASS      (ZERO)	/*  00: Bypass PLL */
+#define PLL_4x          (B6)	/*  01: PLL 4x */
+#define PLL_6x          (B7)	/*  10: PLL 6x */
+#define PLL_8x          (B7|B6)	/*  11: PLL 8x */
+				/* AEC evaluate window */
+#define AEC_FULL        (ZERO)	/*  00: Full window */
+#define AEC_1p2         (B4)	/*  01: 1/2  window */
+#define AEC_1p4         (B5)	/*  10: 1/4  window */
+#define AEC_2p3         (B5|B4)	/*  11: Low 2/3 window */
+
+/* COM5 */
+#define AFR_ON_OFF      (B7)	/* Auto frame rate control ON/OFF selection */
+#define AFR_SPPED       (B6)	/* Auto frame rate control speed slection */
+				/* Auto frame rate max rate control */
+#define AFR_NO_RATE     (ZERO)	/*   No  reduction of frame rate */
+#define AFR_1p2         (B4)	/*   Max reduction to 1/2 frame rate */
+#define AFR_1p4         (B5)	/*   Max reduction to 1/4 frame rate */
+#define AFR_1p8         (B5|B4)	/*   Max reduction to 1/8 frame rate */
+				/* Auto frame rate active point control */
+#define AF_2x           (ZERO)	/*   Add frame when AGC reaches  2x gain */
+#define AF_4x           (B2)	/*   Add frame when AGC reaches  4x gain */
+#define AF_8x           (B3)	/*   Add frame when AGC reaches  8x gain */
+#define AF_16x          (B2|B3)	/*   Add frame when AGC reaches 16x gain */
+				/* AEC max step control */
+#define AEC_NO_LIMIT    (B0)	/*   0 : AEC incease step has limit */
+				/*   1 : No limit to AEC increase step */
+
+/* COM7 */
+				/* SCCB Register Reset */
+#define SCCB_RESET      (B7)	/*   0 : No change */
+				/*   1 : Resets all registers to default */
+				/* Resolution selection */
+#define SLCT_MASK       (B6)	/*   Mask of VGA or QVGA */
+#define SLCT_VGA        (ZERO)	/*   0 : VGA */
+#define SLCT_QVGA       (B6)	/*   1 : QVGA */
+#define ITU656_ON_OFF   (B5)	/* ITU656 protocol ON/OFF selection */
+				/* RGB output format control */
+#define FMT_GBR422      (ZERO)	/*   00 : GBR 4:2:2 */
+#define FMT_RGB565      (B2)	/*   01 : RGB 565 */
+#define FMT_RGB555      (B3)	/*   10 : RGB 555 */
+#define FMT_RGB444      (B3|B2)	/*   11 : RGB 444 */
+				/* Output format control */
+#define OFMT_YUV        (ZERO)	/*   00 : YUV */
+#define OFMT_P_BRAW     (B0)	/*   01 : Processed Bayer RAW */
+#define OFMT_RGB        (B1)	/*   10 : RGB */
+#define OFMT_BRAW       (B1|B0)	/*   11 : Bayer RAW */
+
+/* COM8 */
+#define FAST_ALGO       (B7)	/* Enable fast AGC/AEC algorithm */
+				/* AEC Setp size limit */
+#define UNLMT_STEP      (B6)	/*   0 : Step size is limited */
+				/*   1 : Unlimited step size */
+#define BNDF_ON_OFF     (B5)	/* Banding filter ON/OFF */
+#define AEC_BND         (B4)	/* Enable AEC below banding value */
+#define AEC_ON_OFF      (B3)	/* Fine AEC ON/OFF control */
+#define AGC_ON          (B2)	/* AGC Enable */
+#define AWB_ON          (B1)	/* AWB Enable */
+#define AEC_ON          (B0)	/* AEC Enable */
+
+/* COM9 */
+#define BASE_AECAGC     (B7)	/* Histogram or average based AEC/AGC */
+				/* Automatic gain ceiling - maximum AGC value */
+#define GAIN_2x         (ZERO)	/*   000 :   2x */
+#define GAIN_4x         (B4)	/*   001 :   4x */
+#define GAIN_8x         (B5)	/*   010 :   8x */
+#define GAIN_16x        (B4|B5)	/*   011 :  16x */
+#define GAIN_32x        (B6)	/*   100 :  32x */
+#define GAIN_64x        (B6|B4)	/*   101 :  64x */
+#define GAIN_128x       (B6|B5)	/*   110 : 128x */
+#define DROP_VSYNC      (B2)	/* Drop VSYNC output of corrupt frame */
+#define DROP_HREF       (B1)	/* Drop HREF  output of corrupt frame */
+
+/* COM11 */
+#define SGLF_ON_OFF     (B1)	/* Single frame ON/OFF selection */
+#define SGLF_TRIG       (B0)	/* Single frame transfer trigger */
+
+/* EXHCH */
+#define VSIZE_LSB       (B2)	/* Vertical data output size LSB */
+
+/* DSP_CTRL1 */
+#define FIFO_ON         (B7)	/* FIFO enable/disable selection */
+#define UV_ON_OFF       (B6)	/* UV adjust function ON/OFF selection */
+#define YUV444_2_422    (B5)	/* YUV444 to 422 UV channel option selection */
+#define CLR_MTRX_ON_OFF (B4)	/* Color matrix ON/OFF selection */
+#define INTPLT_ON_OFF   (B3)	/* Interpolation ON/OFF selection */
+#define GMM_ON_OFF      (B2)	/* Gamma function ON/OFF selection */
+#define AUTO_BLK_ON_OFF (B1)	/* Black defect auto correction ON/OFF */
+#define AUTO_WHT_ON_OFF (B0)	/* White define auto correction ON/OFF */
+
+/* DSP_CTRL3 */
+#define UV_MASK         (B7)	/* UV output sequence option */
+#define UV_ON           (B7)	/*   ON */
+#define UV_OFF          (ZERO)	/*   OFF */
+#define CBAR_MASK       (B5)	/* DSP Color bar mask */
+#define CBAR_ON         (B5)	/*   ON */
+#define CBAR_OFF        (ZERO)	/*   OFF */
+
+/* HSTART */
+#define HST_VGA  0x23
+#define HST_QVGA 0x3F
+
+/* HSIZE */
+#define HSZ_VGA  0xA0
+#define HSZ_QVGA 0x50
+
+/* VSTART */
+#define VST_VGA  0x07
+#define VST_QVGA 0x03
+
+/* VSIZE */
+#define VSZ_VGA  0xF0
+#define VSZ_QVGA 0x78
+
+/* HOUTSIZE */
+#define HOSZ_VGA   0xA0
+#define HOSZ_QVGA  0x50
+
+/* VOUTSIZE */
+#define VOSZ_VGA  0xF0
+#define VOSZ_QVGA 0x78
+
+/*
+ * bit configure (32 bit)
+ * this is used in struct ov772x_color_format :: option
+ */
+#define OP_UV       0x00000001
+#define OP_SWAP_RGB 0x00000002
+
+/*
+ * struct
+ */
+struct regval_list {
+	unsigned char reg_num;
+	unsigned char value;
+};
+
+struct ov772x_color_format {
+	char                 *name;
+	__u32                 fourcc;
+	struct regval_list   *regs;
+	unsigned int          option;
+};
+
+struct ov772x_win_size {
+	char               *name;
+	__u32               width;
+	__u32               height;
+	unsigned char       com7_bit;
+	struct regval_list *regs;
+};
+
+struct ov772x_priv {
+	struct ov772x_camera_info   *info;
+	struct i2c_client           *client;
+	struct soc_camera_device     icd;
+	struct ov772x_color_format  *fmt;
+	struct ov772x_win_size      *win;
+};
+
+#define ENDMARKER { 0xff, 0xff }
+
+static struct regval_list ov772x_default_regs[] =
+{
+	{ COM3,  ZERO },
+	{ COM4,  PLL_4x | 0x01 },
+	{ 0x16,  0x00 },  /* Mystery */
+	{ COM11, B4 },    /* Mystery */
+	{ 0x28,  0x00 },  /* Mystery */
+	{ HREF,  0x00 },
+	{ COM13, 0xe2 },  /* Mystery */
+	{ AREF0, 0xef },
+	{ AREF2, 0x60 },
+	{ AREF6, 0x7a },
+	ENDMARKER,
+};
+
+/*
+ * register setting for color format
+ */
+static struct regval_list ov772x_RGB555_regs[] = {
+	{ COM7, FMT_RGB555 | OFMT_RGB },
+	ENDMARKER,
+};
+
+static struct regval_list ov772x_RGB565_regs[] = {
+	{ COM7, FMT_RGB565 | OFMT_RGB },
+	ENDMARKER,
+};
+
+static struct regval_list ov772x_YYUV_regs[] = {
+	{ COM3, SWAP_YUV },
+	{ COM7, OFMT_YUV },
+	ENDMARKER,
+};
+
+static struct regval_list ov772x_UVYY_regs[] = {
+	{ COM7, OFMT_YUV },
+	ENDMARKER,
+};
+
+
+/*
+ * register setting for window size
+ */
+static struct regval_list ov772x_qvga_regs[] = {
+	{ HSTART,   HST_QVGA },
+	{ HSIZE,    HSZ_QVGA },
+	{ VSTART,   VST_QVGA },
+	{ VSIZE,    VSZ_QVGA  },
+	{ HOUTSIZE, HOSZ_QVGA },
+	{ VOUTSIZE, VOSZ_QVGA },
+	ENDMARKER,
+};
+
+static struct regval_list ov772x_vga_regs[] = {
+	{ HSTART,   HST_VGA },
+	{ HSIZE,    HSZ_VGA },
+	{ VSTART,   VST_VGA },
+	{ VSIZE,    VSZ_VGA },
+	{ HOUTSIZE, HOSZ_VGA },
+	{ VOUTSIZE, VOSZ_VGA },
+	ENDMARKER ,
+};
+
+/*
+ * supported format list
+ */
+
+#define SETFOURCC(type) .name = (#type), .fourcc = (V4L2_PIX_FMT_ ## type)
+static const struct soc_camera_data_format ov772x_fmt_lists[] = {
+	{
+		SETFOURCC(YUYV),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_JPEG,
+	},
+	{
+		SETFOURCC(YVYU),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_JPEG,
+	},
+	{
+		SETFOURCC(UYVY),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_JPEG,
+	},
+	{
+		SETFOURCC(RGB555),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+	},
+	{
+		SETFOURCC(RGB555X),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+	},
+	{
+		SETFOURCC(RGB565),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+	},
+	{
+		SETFOURCC(RGB565X),
+		.depth      = 16,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+	},
+};
+
+/*
+ * color format list
+ */
+#define T_YUYV 0
+static struct ov772x_color_format ov772x_cfmts[] = {
+	[T_YUYV] = {
+		SETFOURCC(YUYV),
+		.regs   = ov772x_YYUV_regs,
+	},
+	{
+		SETFOURCC(YVYU),
+		.regs   = ov772x_YYUV_regs,
+		.option = OP_UV,
+	},
+	{
+		SETFOURCC(UYVY),
+		.regs   = ov772x_UVYY_regs,
+	},
+	{
+		SETFOURCC(RGB555),
+		.regs   = ov772x_RGB555_regs,
+		.option = OP_SWAP_RGB,
+	},
+	{
+		SETFOURCC(RGB555X),
+		.regs   = ov772x_RGB555_regs,
+	},
+	{
+		SETFOURCC(RGB565),
+		.regs   = ov772x_RGB565_regs,
+		.option = OP_SWAP_RGB,
+	},
+	{
+		SETFOURCC(RGB565X),
+		.regs   = ov772x_RGB565_regs,
+	},
+};
+
+
+/*
+ * window size list
+ */
+#define VGA_WIDTH   640
+#define VGA_HEIGHT  480
+#define QVGA_WIDTH  320
+#define QVGA_HEIGHT 240
+#define MAX_WIDTH   VGA_WIDTH
+#define MAX_HEIGHT  VGA_HEIGHT
+
+static struct ov772x_win_size ov772x_win_vga = {
+	.name     = "VGA",
+	.width    = VGA_WIDTH,
+	.height   = VGA_HEIGHT,
+	.com7_bit = SLCT_VGA,
+	.regs     = ov772x_vga_regs,
+};
+
+static struct ov772x_win_size ov772x_win_qvga = {
+	.name     = "QVGA",
+	.width    = QVGA_WIDTH,
+	.height   = QVGA_HEIGHT,
+	.com7_bit = SLCT_QVGA,
+	.regs     = ov772x_qvga_regs,
+};
+
+
+/*
+ * general function
+ */
+
+static int ov772x_write_array(struct i2c_client  *client,
+			      struct regval_list *vals)
+{
+	while (vals->reg_num != 0xff || vals->value != 0xff) {
+		int ret = i2c_smbus_write_byte_data(client,
+						    vals->reg_num,
+						    vals->value);
+		if (ret < 0)
+			return ret;
+		vals++;
+	}
+	return 0;
+}
+
+static int ov772x_mask_set(struct i2c_client *client,
+					  u8  command,
+					  u8  mask,
+					  u8  set)
+{
+	s32 val = i2c_smbus_read_byte_data(client, command);
+	val &= ~mask;
+	val |=  set;
+
+	return i2c_smbus_write_byte_data(client, command, val);
+}
+
+static int ov772x_reset(struct i2c_client *client)
+{
+	int ret = i2c_smbus_write_byte_data(client, COM7, SCCB_RESET);
+	msleep(1);
+	return ret;
+}
+
+/*
+ * soc_camera_ops function
+ */
+
+static int ov772x_init(struct soc_camera_device *icd)
+{
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+
+	if (priv->info->link.power)
+		priv->info->link.power(&priv->client->dev, 1);
+
+	ov772x_reset(priv->client);
+	return ov772x_write_array(priv->client, ov772x_default_regs);
+}
+
+static int ov772x_release(struct soc_camera_device *icd)
+{
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+
+	if (priv->info->link.power)
+		priv->info->link.power(&priv->client->dev, 0);
+
+	ov772x_reset(priv->client);
+	return 0;
+}
+
+static int ov772x_start_capture(struct soc_camera_device *icd)
+{
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+	int                 ret;
+
+
+	if (!priv->win)
+		priv->win = &ov772x_win_vga;
+	if (!priv->fmt)
+		priv->fmt = &ov772x_cfmts[T_YUYV];
+
+	/*
+	 * reset hardware
+	 */
+	ov772x_reset(priv->client);
+	ret = ov772x_write_array(priv->client, ov772x_default_regs);
+	if (ret < 0)
+		goto start_end;
+
+	/*
+	 * set color format
+	 */
+	ret = ov772x_write_array(priv->client, priv->fmt->regs);
+	if (ret < 0)
+		goto start_end;
+
+	/*
+	 * set size format
+	 */
+	ret = ov772x_write_array(priv->client, priv->win->regs);
+	if (ret < 0)
+		goto start_end;
+
+	/*
+	 * set COM7 bit ( QVGA or VGA )
+	 */
+	ret = ov772x_mask_set(priv->client,
+			      COM7, SLCT_MASK, priv->win->com7_bit);
+	if (ret < 0)
+		goto start_end;
+
+	/*
+	 * set UV setting
+	 */
+	if (priv->fmt->option & OP_UV) {
+		ret = ov772x_mask_set(priv->client,
+				      DSP_CTRL3, UV_MASK, UV_ON);
+		if (ret < 0)
+			goto start_end;
+	}
+
+	/*
+	 * set SWAP setting
+	 */
+	if (priv->fmt->option & OP_SWAP_RGB) {
+		ret = ov772x_mask_set(priv->client,
+				      COM3, SWAP_MASK, SWAP_RGB);
+		if (ret < 0)
+			goto start_end;
+	}
+
+	/*
+	 * color bar mode
+	 */
+	if (priv->info->color_bar) {
+		ret = ov772x_mask_set(priv->client,
+				      DSP_CTRL3, CBAR_MASK, CBAR_ON);
+		if (ret < 0)
+			goto start_end;
+	}
+
+
+	dev_info(&icd->dev,
+		 "format %s, win %s\n" , priv->fmt->name, priv->win->name);
+
+start_end:
+	priv->fmt = NULL;
+	priv->win = NULL;
+
+	return ret;
+}
+
+static int ov772x_stop_capture(struct soc_camera_device *icd)
+{
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+	ov772x_reset(priv->client);
+	return 0;
+}
+
+static int ov772x_set_bus_param(struct soc_camera_device *icd,
+				unsigned long		  flags)
+{
+	return 0;
+}
+
+static unsigned long ov772x_query_bus_param(struct soc_camera_device *icd)
+{
+
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+
+	return  SOCAM_PCLK_SAMPLE_RISING |
+		SOCAM_HSYNC_ACTIVE_HIGH  |
+		SOCAM_VSYNC_ACTIVE_HIGH  |
+		SOCAM_MASTER             |
+		priv->info->buswidth;
+}
+
+static int ov772x_get_chip_id(struct soc_camera_device *icd,
+			      struct v4l2_chip_ident   *id)
+{
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+
+	if (id->match_type != V4L2_CHIP_MATCH_I2C_ADDR)
+		return -EINVAL;
+
+	if (id->match_chip != priv->client->addr)
+		return -ENODEV;
+
+	id->ident    = V4L2_IDENT_OV772X;
+	id->revision = 0;
+
+	return 0;
+}
+
+static int ov772x_set_fmt_cap(struct soc_camera_device *icd,
+			      __u32                     pixfmt,
+			      struct v4l2_rect         *rect)
+{
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+	int ret = -EINVAL;
+	int i;
+
+	/*
+	 * select format
+	 */
+	priv->fmt = NULL;
+	for (i = 0; i < ARRAY_SIZE(ov772x_cfmts); i++) {
+		if (pixfmt == ov772x_cfmts[i].fourcc) {
+			priv->fmt = ov772x_cfmts + i;
+			ret = 0;
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static int ov772x_try_fmt_cap(struct soc_camera_device *icd,
+			      struct v4l2_format       *f)
+{
+	struct v4l2_pix_format *pix  = &f->fmt.pix;
+	struct ov772x_priv     *priv;
+
+	priv = container_of(icd, struct ov772x_priv, icd);
+
+	/* QVGA */
+	if (pix->width  <= ov772x_win_qvga.width ||
+	    pix->height <= ov772x_win_qvga.height) {
+		priv->win   = &ov772x_win_qvga;
+		pix->width  =  ov772x_win_qvga.width;
+		pix->height =  ov772x_win_qvga.height;
+	}
+
+	/* VGA */
+	else if (pix->width  <= ov772x_win_vga.width ||
+		 pix->height <= ov772x_win_vga.height) {
+		priv->win   = &ov772x_win_vga;
+		pix->width  =  ov772x_win_vga.width;
+		pix->height =  ov772x_win_vga.height;
+	}
+
+	pix->field = V4L2_FIELD_NONE;
+
+	return 0;
+}
+
+static int ov772x_video_probe(struct soc_camera_device *icd)
+{
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+
+	/*
+	 * We must have a parent by now. And it cannot be a wrong one.
+	 * So this entire test is completely redundant.
+	 */
+	if (!icd->dev.parent ||
+	    to_soc_camera_host(icd->dev.parent)->nr != icd->iface)
+		return -ENODEV;
+
+	/*
+	 * ov772x only use 8 or 10 bit bus width
+	 */
+	if (SOCAM_DATAWIDTH_10 != priv->info->buswidth &&
+	    SOCAM_DATAWIDTH_8  != priv->info->buswidth) {
+		dev_err(&icd->dev, "bus width error\n");
+		return -ENODEV;
+	}
+
+	/*
+	 * show product ID and manufacturer ID
+	 */
+	dev_info(&icd->dev,
+		 "ov772x Product ID %0x:%0x Manufacturer ID %x:%x\n" ,
+		 i2c_smbus_read_byte_data(priv->client, PID),
+		 i2c_smbus_read_byte_data(priv->client, VER),
+		 i2c_smbus_read_byte_data(priv->client, MIDH),
+		 i2c_smbus_read_byte_data(priv->client, MIDL));
+
+	icd->formats     = ov772x_fmt_lists;
+	icd->num_formats = ARRAY_SIZE(ov772x_fmt_lists);
+
+	return soc_camera_video_start(icd);
+}
+
+static void ov772x_video_remove(struct soc_camera_device *icd)
+{
+	soc_camera_video_stop(icd);
+}
+
+static struct soc_camera_ops ov772x_ops = {
+	.owner			= THIS_MODULE,
+	.probe			= ov772x_video_probe,
+	.remove			= ov772x_video_remove,
+	.init			= ov772x_init,
+	.release		= ov772x_release,
+	.start_capture		= ov772x_start_capture,
+	.stop_capture		= ov772x_stop_capture,
+	.set_fmt_cap		= ov772x_set_fmt_cap,
+	.try_fmt_cap		= ov772x_try_fmt_cap,
+	.set_bus_param		= ov772x_set_bus_param,
+	.query_bus_param	= ov772x_query_bus_param,
+	.get_chip_id		= ov772x_get_chip_id,
+};
+
+/*
+ * i2c_driver function
+ */
+
+static int ov772x_probe(struct i2c_client          *client,
+			const struct i2c_device_id *did)
+
+{
+	struct ov772x_priv         *priv;
+	struct ov772x_camera_info  *info;
+	struct soc_camera_device   *icd;
+	int                         ret;
+
+	info = client->dev.platform_data;
+	if (!info)
+		return -EINVAL;
+
+	if (!i2c_check_functionality(to_i2c_adapter(client->dev.parent),
+				     I2C_FUNC_SMBUS_BYTE_DATA)) {
+		dev_err(&client->dev ,
+			"I2C-Adapter doesn't support I2C_FUNC_SMBUS_BYTE\n");
+		return -EIO;
+	}
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->info   = info;
+	priv->client = client;
+	priv->win    = NULL;
+	priv->fmt    = NULL;
+	i2c_set_clientdata(client, priv);
+
+	icd = &priv->icd;
+	icd->ops        = &ov772x_ops;
+	icd->control    = &client->dev;
+	icd->width_min  = 0;
+	icd->width_max  = MAX_WIDTH;
+	icd->height_min = 0;
+	icd->height_max = MAX_HEIGHT;
+	icd->y_skip_top = 0;
+	icd->iface      = priv->info->link.bus_id;
+
+	ret = soc_camera_device_register(icd);
+
+	if (ret)
+		kfree(priv);
+
+	return ret;
+}
+
+static int ov772x_remove(struct i2c_client *client)
+{
+	struct ov772x_priv *priv = i2c_get_clientdata(client);
+
+	soc_camera_device_unregister(&priv->icd);
+	kfree(priv);
+	return 0;
+}
+
+static const struct i2c_device_id ov772x_id[] = {
+	{"ov772x", 0},
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ov772x_id);
+
+
+static struct i2c_driver ov772x_i2c_driver = {
+	.driver = {
+		.name = "ov772x",
+	},
+	.probe    = ov772x_probe,
+	.remove   = ov772x_remove,
+	.id_table = ov772x_id,
+};
+
+/*
+ * module function
+ */
+
+static int __init ov772x_module_init(void)
+{
+	printk(KERN_INFO "ov772x driver\n");
+	return i2c_add_driver(&ov772x_i2c_driver);
+}
+
+static void __exit ov772x_module_exit(void)
+{
+	i2c_del_driver(&ov772x_i2c_driver);
+}
+
+module_init(ov772x_module_init);
+module_exit(ov772x_module_exit);
+
+MODULE_DESCRIPTION("SoC Camera driver for ov772x");
+MODULE_AUTHOR("Kuninori Morimoto");
+MODULE_LICENSE("GPL v2");
diff --git a/include/media/ov772x.h b/include/media/ov772x.h
new file mode 100644
index 0000000..e966657
--- /dev/null
+++ b/include/media/ov772x.h
@@ -0,0 +1,12 @@
+#ifndef __OV772X_H__
+#define __OV772X_H__
+
+#include <media/soc_camera.h>
+
+struct ov772x_camera_info {
+	unsigned long   buswidth;
+	int             color_bar;
+	struct soc_camera_link link;
+};
+
+#endif /* __OV772X_H__ */
diff --git a/include/media/v4l2-chip-ident.h b/include/media/v4l2-chip-ident.h
index d73a8e9..bfe5142 100644
--- a/include/media/v4l2-chip-ident.h
+++ b/include/media/v4l2-chip-ident.h
@@ -60,6 +60,7 @@ enum {
 
 	/* OmniVision sensors: reserved range 250-299 */
 	V4L2_IDENT_OV7670 = 250,
+	V4L2_IDENT_OV772X = 251,
 
 	/* Conexant MPEG encoder/decoders: reserved range 410-420 */
 	V4L2_IDENT_CX23415 = 415,
-- 
1.5.4.3

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-16  2:21   ` Magnus Damm
@ 2008-10-16  6:24     ` Guennadi Liakhovetski
  2008-10-16  6:35       ` Robert Jarzmik
  2008-10-16  6:49       ` Magnus Damm
  0 siblings, 2 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2008-10-16  6:24 UTC (permalink / raw)
  To: Magnus Damm; +Cc: V4L

Hi Magnus

On Thu, 16 Oct 2008, Magnus Damm wrote:

> >> +     /*
> >> +      * color bar mode
> >> +      */
> >> +     if (priv->info->color_bar) {
> >> +             ret = ov772x_mask_set(priv->client,
> >> +                             DSP_CTRL3, CBAR_MASK  , CBAR_ON);
> >> +             if (ret < 0)
> >> +                     goto start_end;
> >> +     }
> >
> > What is this "color bar mode" and why do you think you need it to be
> > specified by the platform data (also see below)?
> 
> The color bar mode is a camera test mode where color bars similar to
> the vivi driver are output instead of the camera image. It's very
> useful for testing and getting byte order issues resolved. Ideally I'd
> like to have it as a second output, but I have to extend the SoC
> camera framework first to get that in place.
> 
> I'm not sure if platform data is the best place to enable this, but I
> guess it's good enough.

Hm, so, to test your camera you have to modify your source and rebuild 
your kernel... And same again to switch back to normal operation. Does not 
sound very convenient to me. OTOH, making it a module parameter makes it 
much easier. In fact, maybe it would be a good idea to add a new 
camera-class control for this mode. Yet another possibility is to enable 
debug register-access in the driver and use that to manually set the test 
mode from user-space. A new v4l-control seems best to me, not sure what 
others will say about this. As you probably know, many other cameras also 
have this "test pattern" mode, some even several of them. So, this becomes 
a control with a parameter then.

> > Now, this one. Please, use struct soc_camera_link. It also provides bus_id
> > (your iface), power, ok, I admit, the inclusion of the "gpio" member in it
> > was a mistake of mine, it is too specific, we might remove it at some
> > point. I am not sure you really need color_bar and bus_width. I think,
> > cameras are more or less exchangeable parts, and if they need some
> > parameters, that cannot be autoprobed and do not belong to the camera
> > itself, it might be better to make them module parameters, like the
> > sensor_type parameter in mt9v022. Even if in your case the sensor chip is
> > soldered on the board, in another configuration it might not be.
> 
> Using soc_camera_link sounds like a good idea. I don't agree with you
> regarding the module parameters - doing that removes the
> per-camera-instance configuration that the platform data gives us.

Then a new control or raw register access would be a better way, I think.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-16  6:24     ` Guennadi Liakhovetski
@ 2008-10-16  6:35       ` Robert Jarzmik
  2008-10-16  6:46         ` Magnus Damm
  2008-10-16  6:49       ` Magnus Damm
  1 sibling, 1 reply; 22+ messages in thread
From: Robert Jarzmik @ 2008-10-16  6:35 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: V4L

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> Hm, so, to test your camera you have to modify your source and rebuild 
> your kernel... And same again to switch back to normal operation. Does not 
> sound very convenient to me. OTOH, making it a module parameter makes it 
> much easier. In fact, maybe it would be a good idea to add a new 
> camera-class control for this mode. Yet another possibility is to enable 
> debug register-access in the driver and use that to manually set the test 
> mode from user-space. A new v4l-control seems best to me, not sure what 
> others will say about this. As you probably know, many other cameras also 
> have this "test pattern" mode, some even several of them. So, this becomes 
> a control with a parameter then.

Personnaly I'm rather inclined for the debug registers solutions.

When developping a camera driver, the test pattern alone is not enough. You have
to tweak the registers, see if the specification is correct, then understand the
specification, and then change your driver code. My experience tells me you
never understand correctly are camera setup from the first time.

So IMHO the registers are enough here.

> Then a new control or raw register access would be a better way, I think.
So do I.

Cheers.

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-16  6:35       ` Robert Jarzmik
@ 2008-10-16  6:46         ` Magnus Damm
  2008-10-16  6:55           ` Hans Verkuil
  0 siblings, 1 reply; 22+ messages in thread
From: Magnus Damm @ 2008-10-16  6:46 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: V4L, Guennadi Liakhovetski

On Thu, Oct 16, 2008 at 3:35 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
>
>> Hm, so, to test your camera you have to modify your source and rebuild
>> your kernel... And same again to switch back to normal operation. Does not
>> sound very convenient to me. OTOH, making it a module parameter makes it
>> much easier. In fact, maybe it would be a good idea to add a new
>> camera-class control for this mode. Yet another possibility is to enable
>> debug register-access in the driver and use that to manually set the test
>> mode from user-space. A new v4l-control seems best to me, not sure what
>> others will say about this. As you probably know, many other cameras also
>> have this "test pattern" mode, some even several of them. So, this becomes
>> a control with a parameter then.
>
> Personnaly I'm rather inclined for the debug registers solutions.
>
> When developping a camera driver, the test pattern alone is not enough. You have
> to tweak the registers, see if the specification is correct, then understand the
> specification, and then change your driver code. My experience tells me you
> never understand correctly are camera setup from the first time.

One thing is when people write their driver, but the scenario that I'm
thinking about is more when people take an already existing soc_camera
sensor driver and hook it up to some soc_camera host. There are all
sorts of endian and swapping issues that need to be worked out. They
depend on soc_camera host driver, endian setting and userspace. Having
a test pattern available would surely help there in my opinion.

> So IMHO the registers are enough here.
>
>> Then a new control or raw register access would be a better way, I think.
> So do I.

I dislike the register access option since it requires the developer
to have some user space tool that most likely won't ship with the
kernel. I think seeing it as yet another video input source is pretty
clean. Or maybe it isn't very useful at all, I'm not sure. =)

/ magnus

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-16  6:24     ` Guennadi Liakhovetski
  2008-10-16  6:35       ` Robert Jarzmik
@ 2008-10-16  6:49       ` Magnus Damm
  1 sibling, 0 replies; 22+ messages in thread
From: Magnus Damm @ 2008-10-16  6:49 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: V4L

Hi Guennadi,

On Thu, Oct 16, 2008 at 3:24 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> On Thu, 16 Oct 2008, Magnus Damm wrote:
>> Using soc_camera_link sounds like a good idea. I don't agree with you
>> regarding the module parameters - doing that removes the
>> per-camera-instance configuration that the platform data gives us.
>
> Then a new control or raw register access would be a better way, I think.

What about wrapping the color bar mode in #ifdef DEBUG for now? At
least that's simple and requires no infrastructure.

/ magnus

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-16  6:46         ` Magnus Damm
@ 2008-10-16  6:55           ` Hans Verkuil
  2008-10-16  6:58             ` Magnus Damm
  0 siblings, 1 reply; 22+ messages in thread
From: Hans Verkuil @ 2008-10-16  6:55 UTC (permalink / raw)
  To: video4linux-list; +Cc: Guennadi Liakhovetski

On Thursday 16 October 2008 08:46:31 Magnus Damm wrote:
> On Thu, Oct 16, 2008 at 3:35 PM, Robert Jarzmik 
<robert.jarzmik@free.fr> wrote:
> > Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> >> Hm, so, to test your camera you have to modify your source and
> >> rebuild your kernel... And same again to switch back to normal
> >> operation. Does not sound very convenient to me. OTOH, making it a
> >> module parameter makes it much easier. In fact, maybe it would be
> >> a good idea to add a new camera-class control for this mode. Yet
> >> another possibility is to enable debug register-access in the
> >> driver and use that to manually set the test mode from user-space.
> >> A new v4l-control seems best to me, not sure what others will say
> >> about this. As you probably know, many other cameras also have
> >> this "test pattern" mode, some even several of them. So, this
> >> becomes a control with a parameter then.
> >
> > Personnaly I'm rather inclined for the debug registers solutions.
> >
> > When developping a camera driver, the test pattern alone is not
> > enough. You have to tweak the registers, see if the specification
> > is correct, then understand the specification, and then change your
> > driver code. My experience tells me you never understand correctly
> > are camera setup from the first time.
>
> One thing is when people write their driver, but the scenario that
> I'm thinking about is more when people take an already existing
> soc_camera sensor driver and hook it up to some soc_camera host.
> There are all sorts of endian and swapping issues that need to be
> worked out. They depend on soc_camera host driver, endian setting and
> userspace. Having a test pattern available would surely help there in
> my opinion.
>
> > So IMHO the registers are enough here.
> >
> >> Then a new control or raw register access would be a better way, I
> >> think.
> >
> > So do I.
>
> I dislike the register access option since it requires the developer
> to have some user space tool that most likely won't ship with the
> kernel. I think seeing it as yet another video input source is pretty
> clean. Or maybe it isn't very useful at all, I'm not sure. =)

Just to give some background: register access can be done via the 
v4l2-dbg utility (see v4l2-apps/util) which uses the 
VIDIOC_DBG_G/S_REGISTER ioctls which are only compiled into the driver 
when the CONFIG_VIDEO_ADV_DEBUG option is set. This is the standard way 
of accessing registers.

An alternative for selecting a test pattern could be to have two inputs: 
one is the camera and another one is the test pattern. Here too you 
could enable the test pattern input only if CONFIG_VIDEO_ADV_DEBUG is 
set.

Just some ideas for you.

Regards,

	Hans

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-16  6:55           ` Hans Verkuil
@ 2008-10-16  6:58             ` Magnus Damm
  0 siblings, 0 replies; 22+ messages in thread
From: Magnus Damm @ 2008-10-16  6:58 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: video4linux-list, Guennadi Liakhovetski

On Thu, Oct 16, 2008 at 3:55 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On Thursday 16 October 2008 08:46:31 Magnus Damm wrote:
>> I dislike the register access option since it requires the developer
>> to have some user space tool that most likely won't ship with the
>> kernel. I think seeing it as yet another video input source is pretty
>> clean. Or maybe it isn't very useful at all, I'm not sure. =)
>
> Just to give some background: register access can be done via the
> v4l2-dbg utility (see v4l2-apps/util) which uses the
> VIDIOC_DBG_G/S_REGISTER ioctls which are only compiled into the driver
> when the CONFIG_VIDEO_ADV_DEBUG option is set. This is the standard way
> of accessing registers.

Thanks for pointing that out!

> An alternative for selecting a test pattern could be to have two inputs:
> one is the camera and another one is the test pattern. Here too you
> could enable the test pattern input only if CONFIG_VIDEO_ADV_DEBUG is
> set.
>
> Just some ideas for you.

Yeah, maybe a combination of both register access and separate test
pattern input would be useful. I guess register access is a good first
step at least.

Thank you!

/ magnus

> Regards,
>
>        Hans
>

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-16  4:28 Kuninori Morimoto
@ 2008-10-16  8:27 ` Antonio Ospite
  2008-10-16  9:53   ` morimoto.kuninori
  2008-10-16 19:23   ` Guennadi Liakhovetski
  0 siblings, 2 replies; 22+ messages in thread
From: Antonio Ospite @ 2008-10-16  8:27 UTC (permalink / raw)
  To: video4linux-list


[-- Attachment #1.1: Type: text/plain, Size: 810 bytes --]

On Thu, 16 Oct 2008 13:28:52 +0900
Kuninori Morimoto <morimoto.kuninori@renesas.com> wrote:

> This patch adds ov772x driver that use soc_camera framework.

Hi, this sensor is used also in some usb cameras (Playstation Eye, for
instance), and this code can be reused to improve the previously posted
ov534 driver.

The question is: is there any mechanism to share sensor code between
usb and i2c drivers or we have to copy and paste?

Thanks, and sorry for the noise.

Regards,
   Antonio Ospite

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

  Web site: http://www.studenti.unina.it/~ospite
Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 164 bytes --]

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-16  8:27 ` Antonio Ospite
@ 2008-10-16  9:53   ` morimoto.kuninori
  2008-10-16 10:36     ` Magnus Damm
  2008-10-16 19:23   ` Guennadi Liakhovetski
  1 sibling, 1 reply; 22+ messages in thread
From: morimoto.kuninori @ 2008-10-16  9:53 UTC (permalink / raw)
  To: Antonio Ospite; +Cc: V4L


Hi Antonio Ospite

# I'm sorry that I have only poor Enlish.

> The question is: is there any mechanism to share sensor code between
> usb and i2c drivers or we have to copy and paste?

I think it can be possible to reuse.
It's just a matter of glue code.

Regards,
    Morimoto

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-16  9:53   ` morimoto.kuninori
@ 2008-10-16 10:36     ` Magnus Damm
  0 siblings, 0 replies; 22+ messages in thread
From: Magnus Damm @ 2008-10-16 10:36 UTC (permalink / raw)
  To: morimoto.kuninori; +Cc: V4L

On Thu, Oct 16, 2008 at 6:53 PM,  <morimoto.kuninori@renesas.com> wrote:
>
> Hi Antonio Ospite
>
> # I'm sorry that I have only poor Enlish.
>
>> The question is: is there any mechanism to share sensor code between
>> usb and i2c drivers or we have to copy and paste?
>
> I think it can be possible to reuse.
> It's just a matter of glue code.

Right. If the usb driver would be a soc_camera host driver then it
could make use of the soc_camera camera/sensor drivers right out of
the box. I'm not sure if the soc_camera framework is the best option
for you though, but for us using the sh_mobile_ceu driver it's a good
choice. =)

/ magnus

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-16  8:27 ` Antonio Ospite
  2008-10-16  9:53   ` morimoto.kuninori
@ 2008-10-16 19:23   ` Guennadi Liakhovetski
  2008-10-16 20:58     ` Hans Verkuil
  1 sibling, 1 reply; 22+ messages in thread
From: Guennadi Liakhovetski @ 2008-10-16 19:23 UTC (permalink / raw)
  To: Antonio Ospite; +Cc: video4linux-list

On Thu, 16 Oct 2008, Antonio Ospite wrote:

> On Thu, 16 Oct 2008 13:28:52 +0900
> Kuninori Morimoto <morimoto.kuninori@renesas.com> wrote:
> 
> > This patch adds ov772x driver that use soc_camera framework.
> 
> Hi, this sensor is used also in some usb cameras (Playstation Eye, for
> instance), and this code can be reused to improve the previously posted
> ov534 driver.
> 
> The question is: is there any mechanism to share sensor code between
> usb and i2c drivers or we have to copy and paste?

Well, this is not the first time this idea / question appears... I think, 
it might be possible to re-use soc-camera sensor drivers with USB-cameras, 
at least with those, that export their internal i2c bus to the system. 
Then, as Magnus Damm noticed, you have to register your USB driver as a 
soc-camera host driver, and you will have to register the sensor as a 
normal i2c device with the i2c subsystem. That's about it:-) Hans Verkuil 
will, probably, notice, that soc-camera is not universal enough for many 
video applications, but it might well be enough for the cideo part of a 
simple USB web-camera, I think. OTOH, Hans is working on a new API, that 
should unify the host / device interface in v4l applications, at which 
time soc-camera drivers will have to be converted, as well as multiple 
other currently existing APIs.

Just out of curiosity, I would be interested to see a USB camera driver, 
using the soc-camera framework:-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-16 19:23   ` Guennadi Liakhovetski
@ 2008-10-16 20:58     ` Hans Verkuil
  2008-10-17  2:47       ` Magnus Damm
  2008-10-17  7:41       ` Antonio Ospite
  0 siblings, 2 replies; 22+ messages in thread
From: Hans Verkuil @ 2008-10-16 20:58 UTC (permalink / raw)
  To: video4linux-list; +Cc: Guennadi Liakhovetski

On Thursday 16 October 2008 21:23:59 Guennadi Liakhovetski wrote:
> On Thu, 16 Oct 2008, Antonio Ospite wrote:
> > On Thu, 16 Oct 2008 13:28:52 +0900
> >
> > Kuninori Morimoto <morimoto.kuninori@renesas.com> wrote:
> > > This patch adds ov772x driver that use soc_camera framework.
> >
> > Hi, this sensor is used also in some usb cameras (Playstation Eye,
> > for instance), and this code can be reused to improve the
> > previously posted ov534 driver.
> >
> > The question is: is there any mechanism to share sensor code
> > between usb and i2c drivers or we have to copy and paste?
>
> Well, this is not the first time this idea / question appears... I
> think, it might be possible to re-use soc-camera sensor drivers with
> USB-cameras, at least with those, that export their internal i2c bus
> to the system. Then, as Magnus Damm noticed, you have to register
> your USB driver as a soc-camera host driver, and you will have to
> register the sensor as a normal i2c device with the i2c subsystem.
> That's about it:-) 

Yes, that's what happens with for example pvrusb2. Both pvrusb2 (USB 
driver) and ivtv (PCI driver) use the same i2c modules such as saa7115, 
msp3400, wm8775 or cx25840.

> Hans Verkuil will, probably, notice, that 
> soc-camera is not universal enough for many video applications, but
> it might well be enough for the cideo part of a simple USB
> web-camera, I think. OTOH, Hans is working on a new API, that should
> unify the host / device interface in v4l applications, at which time
> soc-camera drivers will have to be converted, as well as multiple
> other currently existing APIs.

I'm planning to start on this this weekend. If all goes well the basic 
framework should go into v4l-dvb soon after the 2.6.28 windows closes, 
and it should end up in 2.6.29.

Regards,

          Hans

> Just out of curiosity, I would be interested to see a USB camera
> driver, using the soc-camera framework:-)
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
>
> --
> video4linux-list mailing list
> Unsubscribe
> mailto:video4linux-list-request@redhat.com?subject=unsubscribe
> https://www.redhat.com/mailman/listinfo/video4linux-list


--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-16 20:58     ` Hans Verkuil
@ 2008-10-17  2:47       ` Magnus Damm
  2008-10-17  6:50         ` Guennadi Liakhovetski
  2008-10-17  7:41       ` Antonio Ospite
  1 sibling, 1 reply; 22+ messages in thread
From: Magnus Damm @ 2008-10-17  2:47 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: video4linux-list, Guennadi Liakhovetski

[-- Attachment #1: Type: text/plain, Size: 1689 bytes --]

On Fri, Oct 17, 2008 at 5:58 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On Thursday 16 October 2008 21:23:59 Guennadi Liakhovetski wrote:
>> Hans Verkuil will, probably, notice, that
>> soc-camera is not universal enough for many video applications, but
>> it might well be enough for the cideo part of a simple USB
>> web-camera, I think. OTOH, Hans is working on a new API, that should
>> unify the host / device interface in v4l applications, at which time
>> soc-camera drivers will have to be converted, as well as multiple
>> other currently existing APIs.
>
> I'm planning to start on this this weekend. If all goes well the basic
> framework should go into v4l-dvb soon after the 2.6.28 windows closes,
> and it should end up in 2.6.29.

Hans,  any chance of that framework including pixel format helper
code? I've hacked a bit on using a bitmap to represent pixel formats
supported by a certain driver. The attached very rough patch maybe
shows what i'm trying to do.

Basically, I need a simple way to determine if a camera sensor
supports a certain pixel format, and if so then i'd like to add a
bunch of pixel formats supported by the soc_camera host.

The SuperH CEU "Capture Engine Unit" has a mode where it accepts
interleaved YUV pixel formats as input from the camera sensor and
converts them to one of NV12, NV21 or a less common 4:2:2 format maybe
known as NV16, NV61. So I need a simple way to check and add pixel
formats by the soc_camera host driver.

Also, having a centralized place for pixel format strings, color depth
and colorspaces associated with each of the pixel formats makes sense
to me. Any thoughts? Want me to hack something up?

Cheers,

/ magnus

[-- Attachment #2: linux-2.6.28-rc-video-pixel_formats-20081015.patch --]
[-- Type: application/octet-stream, Size: 13019 bytes --]

--- 0001/drivers/media/video/Makefile
+++ work/drivers/media/video/Makefile	2008-10-15 20:45:47.000000000 +0900
@@ -12,7 +12,7 @@ videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o
 
 obj-$(CONFIG_VIDEO_DEV) += videodev.o compat_ioctl32.o v4l2-int-device.o
 
-obj-$(CONFIG_VIDEO_V4L2_COMMON) += v4l2-common.o
+obj-$(CONFIG_VIDEO_V4L2_COMMON) += v4l2-common.o pixel_format.o
 
 ifeq ($(CONFIG_VIDEO_V4L1_COMPAT),y)
   obj-$(CONFIG_VIDEO_DEV) += v4l1-compat.o
--- /dev/null
+++ work/drivers/media/video/pixel_format.c	2008-10-15 22:23:50.000000000 +0900
@@ -0,0 +1,69 @@
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/videodev2.h>
+#include <linux/bitops.h>
+#include <media/pixel_format.h>
+
+static __u32 fourccs[NR_PIXEL_FORMATS] = {
+	[PIXEL_FORMAT_RGB332] = V4L2_PIX_FMT_RGB332,
+
+	[PIXEL_FORMAT_RGB565] = V4L2_PIX_FMT_RGB565,
+};
+
+static __u8 descriptions[NR_PIXEL_FORMATS][32] = {
+	[PIXEL_FORMAT_RGB332] = "RGB-3-3-2 [RGB1]",
+	[PIXEL_FORMAT_RGB565] = "RGB-5-6-5 [RGBP]",
+};
+
+int fourcc_to_pixel_format(__u32 fourcc, struct pixel_formats *supported)
+{
+	unsigned int k;
+
+	for (k = 0; k < NR_PIXEL_FORMATS; k++)
+		if (fourccs[k] == fourcc)
+			if (!supported || test_bit(k, supported->bits))
+				return k;
+
+	return -EINVAL;
+}
+
+int default_pixel_format(struct pixel_formats *supported)
+{
+	int pixel_format;
+	
+	pixel_format = find_first_bit(supported->bits, NR_PIXEL_FORMATS);
+	BUG_ON(pixel_format >= NR_PIXEL_FORMATS);
+
+	return pixel_format;
+}
+
+int get_pixel_format_bpp(enum v4l2_pixel_format pixel_format)
+{
+	BUG_ON(pixel_format >= NR_PIXEL_FORMATS);
+	return 16;
+}
+
+int get_pixel_format_fourcc(enum v4l2_pixel_format pixel_format)
+{
+	BUG_ON(pixel_format >= NR_PIXEL_FORMATS);
+	return fourccs[pixel_format];
+}
+
+int pixel_format_enum_fmtdesc(struct v4l2_fmtdesc *f,
+			      struct pixel_formats *supported)
+{
+	unsigned long bit;
+	int index;
+
+	for (bit = 0, index = 0;
+	     bit < NR_PIXEL_FORMATS && index <= f->index;
+	     bit++, index++)
+		bit = find_next_bit(supported->bits, NR_PIXEL_FORMATS, bit);
+
+	if (bit >= NR_PIXEL_FORMATS)
+		return -EINVAL;
+
+	strlcpy(f->description, descriptions[bit], sizeof(f->description));
+	f->pixelformat = fourccs[bit];
+	return 0;
+}
--- 0005/drivers/media/video/sh_mobile_ceu_camera.c
+++ work/drivers/media/video/sh_mobile_ceu_camera.c	2008-10-15 21:50:31.000000000 +0900
@@ -37,6 +37,7 @@
 #include <media/soc_camera.h>
 #include <media/sh_mobile_ceu.h>
 #include <media/videobuf-dma-contig.h>
+#include <media/pixel_format.h>
 
 /* register offsets for sh7722 / sh7723 */
 
@@ -79,7 +80,7 @@ static DEFINE_MUTEX(camera_lock);
 /* per video frame buffer */
 struct sh_mobile_ceu_buffer {
 	struct videobuf_buffer vb; /* v4l buffer must be first */
-	const struct soc_camera_data_format *fmt;
+	enum v4l2_pixel_format format;
 };
 
 struct sh_mobile_ceu_dev {
@@ -121,7 +122,8 @@ static int sh_mobile_ceu_videobuf_setup(
 	struct soc_camera_device *icd = vq->priv_data;
 	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct sh_mobile_ceu_dev *pcdev = ici->priv;
-	int bytes_per_pixel = (icd->current_fmt->depth + 7) >> 3;
+	int bpp = get_pixel_format_bpp(icd->current_format);
+	int bytes_per_pixel = (bpp + 7) >> 3;
 
 	*size = PAGE_ALIGN(icd->width * icd->height * bytes_per_pixel);
 
@@ -177,7 +179,7 @@ static int sh_mobile_ceu_videobuf_prepar
 {
 	struct soc_camera_device *icd = vq->priv_data;
 	struct sh_mobile_ceu_buffer *buf;
-	int ret;
+	int bpp, ret;
 
 	buf = container_of(vb, struct sh_mobile_ceu_buffer, vb);
 
@@ -193,20 +195,21 @@ static int sh_mobile_ceu_videobuf_prepar
 	memset((void *)vb->baddr, 0xaa, vb->bsize);
 #endif
 
-	BUG_ON(NULL == icd->current_fmt);
+	BUG_ON(icd->current_format >= NR_PIXEL_FORMATS);
 
-	if (buf->fmt	!= icd->current_fmt ||
+	if (buf->format	!= icd->current_format ||
 	    vb->width	!= icd->width ||
 	    vb->height	!= icd->height ||
 	    vb->field	!= field) {
-		buf->fmt	= icd->current_fmt;
+		buf->format	= icd->current_format;
 		vb->width	= icd->width;
 		vb->height	= icd->height;
 		vb->field	= field;
 		vb->state	= VIDEOBUF_NEEDS_INIT;
 	}
 
-	vb->size = vb->width * vb->height * ((buf->fmt->depth + 7) >> 3);
+	bpp = get_pixel_format_bpp(buf->format);
+	vb->size = vb->width * vb->height * ((bpp + 7) >> 3);
 	if (0 != vb->baddr && vb->bsize < vb->size) {
 		ret = -EINVAL;
 		goto out;
@@ -394,7 +397,7 @@ static int sh_mobile_ceu_set_bus_param(s
 
 	mdelay(1);
 
-	width = icd->width * (icd->current_fmt->depth / 8);
+	width = icd->width * (get_pixel_format_bpp(icd->current_format) / 8);
 	width = (buswidth == 16) ? width / 2 : width;
 	cfszr_width = (buswidth == 8) ? width / 2 : width;
 	cdwdr_width = (buswidth == 16) ? width * 2 : width;
--- 0001/drivers/media/video/soc_camera.c
+++ work/drivers/media/video/soc_camera.c	2008-10-15 21:44:12.000000000 +0900
@@ -35,17 +35,6 @@ static LIST_HEAD(devices);
 static DEFINE_MUTEX(list_lock);
 static DEFINE_MUTEX(video_lock);
 
-const static struct soc_camera_data_format*
-format_by_fourcc(struct soc_camera_device *icd, unsigned int fourcc)
-{
-	unsigned int i;
-
-	for (i = 0; i < icd->num_formats; i++)
-		if (icd->formats[i].fourcc == fourcc)
-			return icd->formats + i;
-	return NULL;
-}
-
 static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
 				  struct v4l2_format *f)
 {
@@ -54,19 +43,20 @@ static int soc_camera_try_fmt_vid_cap(st
 	struct soc_camera_host *ici =
 		to_soc_camera_host(icd->dev.parent);
 	enum v4l2_field field;
-	const struct soc_camera_data_format *fmt;
+	int pixel_format;
 	int ret;
 
 	WARN_ON(priv != file->private_data);
 
-	fmt = format_by_fourcc(icd, f->fmt.pix.pixelformat);
-	if (!fmt) {
+	pixel_format = fourcc_to_pixel_format(f->fmt.pix.pixelformat,
+					      &icd->supported_formats);
+	if (pixel_format < 0) {
 		dev_dbg(&icd->dev, "invalid format 0x%08x\n",
 			f->fmt.pix.pixelformat);
 		return -EINVAL;
 	}
 
-	dev_dbg(&icd->dev, "fmt: 0x%08x\n", fmt->fourcc);
+	dev_dbg(&icd->dev, "fmt: 0x%08x\n", f->fmt.pix.pixelformat);
 
 	field = f->fmt.pix.field;
 
@@ -88,7 +78,7 @@ static int soc_camera_try_fmt_vid_cap(st
 	/* calculate missing fields */
 	f->fmt.pix.field = field;
 	f->fmt.pix.bytesperline =
-		(f->fmt.pix.width * fmt->depth) >> 3;
+		(f->fmt.pix.width * get_pixel_format_bpp(pixel_format)) >> 3;
 	f->fmt.pix.sizeimage =
 		f->fmt.pix.height * f->fmt.pix.bytesperline;
 
@@ -332,16 +322,17 @@ static int soc_camera_s_fmt_vid_cap(stru
 		to_soc_camera_host(icd->dev.parent);
 	int ret;
 	struct v4l2_rect rect;
-	const static struct soc_camera_data_format *data_fmt;
+	int pixel_format;
 
 	WARN_ON(priv != file->private_data);
 
-	data_fmt = format_by_fourcc(icd, f->fmt.pix.pixelformat);
-	if (!data_fmt)
+	pixel_format = fourcc_to_pixel_format(f->fmt.pix.pixelformat,
+					      &icd->supported_formats);
+	if (pixel_format < 0)
 		return -EINVAL;
 
 	/* buswidth may be further adjusted by the ici */
-	icd->buswidth = data_fmt->depth;
+	icd->buswidth = get_pixel_format_bpp(pixel_format);
 
 	ret = soc_camera_try_fmt_vid_cap(file, icf, f);
 	if (ret < 0)
@@ -355,7 +346,7 @@ static int soc_camera_s_fmt_vid_cap(stru
 	if (ret < 0)
 		return ret;
 
-	icd->current_fmt	= data_fmt;
+	icd->current_format	= pixel_format;
 	icd->width		= rect.width;
 	icd->height		= rect.height;
 	icf->vb_vidq.field	= f->fmt.pix.field;
@@ -375,18 +366,10 @@ static int soc_camera_enum_fmt_vid_cap(s
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
-	const struct soc_camera_data_format *format;
 
 	WARN_ON(priv != file->private_data);
 
-	if (f->index >= icd->num_formats)
-		return -EINVAL;
-
-	format = &icd->formats[f->index];
-
-	strlcpy(f->description, format->name, sizeof(f->description));
-	f->pixelformat = format->fourcc;
-	return 0;
+	return pixel_format_enum_fmtdesc(f, &icd->supported_formats);
 }
 
 static int soc_camera_g_fmt_vid_cap(struct file *file, void *priv,
@@ -394,19 +377,20 @@ static int soc_camera_g_fmt_vid_cap(stru
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
+	int depth = get_pixel_format_bpp(icd->current_format);
 
 	WARN_ON(priv != file->private_data);
 
 	f->fmt.pix.width	= icd->width;
 	f->fmt.pix.height	= icd->height;
 	f->fmt.pix.field	= icf->vb_vidq.field;
-	f->fmt.pix.pixelformat	= icd->current_fmt->fourcc;
+	f->fmt.pix.pixelformat	= get_pixel_format_fourcc(icd->current_format);
 	f->fmt.pix.bytesperline	=
-		(f->fmt.pix.width * icd->current_fmt->depth) >> 3;
+		(f->fmt.pix.width * depth) >> 3;
 	f->fmt.pix.sizeimage	=
 		f->fmt.pix.height * f->fmt.pix.bytesperline;
 	dev_dbg(&icd->dev, "current_fmt->fourcc: 0x%08x\n",
-		icd->current_fmt->fourcc);
+		f->fmt.pix.pixelformat);
 	return 0;
 }
 
@@ -939,9 +923,9 @@ int soc_camera_video_start(struct soc_ca
 	vdev->ioctl_ops		= &soc_camera_ioctl_ops;
 	vdev->release		= video_device_release;
 	vdev->minor		= -1;
-	vdev->tvnorms		= V4L2_STD_UNKNOWN,
+	vdev->tvnorms		= V4L2_STD_UNKNOWN;
 
-	icd->current_fmt = &icd->formats[0];
+	icd->current_format = default_pixel_format(&icd->supported_formats);
 
 	err = video_register_device(vdev, VFL_TYPE_GRABBER, vdev->minor);
 	if (err < 0) {
--- 0002/drivers/media/video/soc_camera_platform.c
+++ work/drivers/media/video/soc_camera_platform.c	2008-10-15 22:10:19.000000000 +0900
@@ -23,7 +23,6 @@
 struct soc_camera_platform_priv {
 	struct soc_camera_platform_info *info;
 	struct soc_camera_device icd;
-	struct soc_camera_data_format format;
 };
 
 static struct soc_camera_platform_info *
@@ -98,15 +97,14 @@ static int soc_camera_platform_try_fmt_c
 static int soc_camera_platform_video_probe(struct soc_camera_device *icd)
 {
 	struct soc_camera_platform_priv *priv;
-	priv = container_of(icd, struct soc_camera_platform_priv, icd);
+	int pixel_format;
 
-	priv->format.name = priv->info->format_name;
-	priv->format.depth = priv->info->format_depth;
-	priv->format.fourcc = priv->info->format.pixelformat;
-	priv->format.colorspace = priv->info->format.colorspace;
+	priv = container_of(icd, struct soc_camera_platform_priv, icd);
 
-	icd->formats = &priv->format;
-	icd->num_formats = 1;
+	pixel_format = fourcc_to_pixel_format(priv->info->format.pixelformat,
+					      NULL);
+	BUG_ON((pixel_format < 0) || (pixel_format >= NR_PIXEL_FORMATS));
+	set_bit(pixel_format, icd->supported_formats.bits);
 
 	return soc_camera_video_start(icd);
 }
--- /dev/null
+++ work/include/media/pixel_format.h	2008-10-15 21:48:22.000000000 +0900
@@ -0,0 +1,71 @@
+
+#ifndef __PIXEL_FORMAT_H__
+#define __PIXEL_FORMAT_H__
+
+#include <linux/init.h>
+#include <linux/bitmap.h>
+
+enum v4l2_pixel_format {
+	PIXEL_FORMAT_RGB332 = 0,
+	PIXEL_FORMAT_RGB444,
+	PIXEL_FORMAT_RGB555,
+	PIXEL_FORMAT_RGB565,
+	PIXEL_FORMAT_RGB555X,
+	PIXEL_FORMAT_RGB565X,
+	PIXEL_FORMAT_BGR24,
+	PIXEL_FORMAT_RGB24,
+	PIXEL_FORMAT_BGR32,
+	PIXEL_FORMAT_RGB32,
+	PIXEL_FORMAT_GREY,
+	PIXEL_FORMAT_Y16,
+	PIXEL_FORMAT_PAL8,
+	PIXEL_FORMAT_YVU410,
+	PIXEL_FORMAT_YVU420,
+	PIXEL_FORMAT_YUYV,
+	PIXEL_FORMAT_UYVY,
+	PIXEL_FORMAT_YUV422P,
+	PIXEL_FORMAT_YUV411P,
+	PIXEL_FORMAT_Y41P,
+	PIXEL_FORMAT_YUV444,
+	PIXEL_FORMAT_YUV555,
+	PIXEL_FORMAT_YUV565,
+	PIXEL_FORMAT_YUV32,
+	PIXEL_FORMAT_NV12,
+	PIXEL_FORMAT_NV21,
+	PIXEL_FORMAT_YUV410,
+	PIXEL_FORMAT_YUV420,
+	PIXEL_FORMAT_YYUV,
+	PIXEL_FORMAT_HI240,
+	PIXEL_FORMAT_HM12,
+	PIXEL_FORMAT_SBGGR8,
+	PIXEL_FORMAT_SGBRG8,
+	PIXEL_FORMAT_SBGGR16,
+	PIXEL_FORMAT_MJPEG,
+	PIXEL_FORMAT_JPEG,
+	PIXEL_FORMAT_DV,
+	PIXEL_FORMAT_MPEG,
+	PIXEL_FORMAT_WNVA,
+	PIXEL_FORMAT_SN9C10X,
+	PIXEL_FORMAT_PWC1,
+	PIXEL_FORMAT_PWC2,
+	PIXEL_FORMAT_ET61X251,
+	PIXEL_FORMAT_SPCA501,
+	PIXEL_FORMAT_SPCA505,
+	PIXEL_FORMAT_SPCA508,
+	PIXEL_FORMAT_SPCA561,
+	PIXEL_FORMAT_PAC207,
+	PIXEL_FORMAT_PJPG,
+	PIXEL_FORMAT_YVYU,
+	NR_PIXEL_FORMATS,
+};
+
+struct pixel_formats { DECLARE_BITMAP(bits, NR_PIXEL_FORMATS); };
+
+int fourcc_to_pixel_format(__u32 fourcc, struct pixel_formats *supported);
+int default_pixel_format(struct pixel_formats *supported);
+int get_pixel_format_bpp(enum v4l2_pixel_format pixel_format);
+int get_pixel_format_fourcc(enum v4l2_pixel_format pixel_format);
+int pixel_format_enum_fmtdesc(struct v4l2_fmtdesc *f,
+			      struct pixel_formats *supported);
+
+#endif /* __PIXEL_FORMAT_H__ */
--- 0001/include/media/soc_camera.h
+++ work/include/media/soc_camera.h	2008-10-15 21:38:26.000000000 +0900
@@ -14,6 +14,7 @@
 
 #include <linux/videodev2.h>
 #include <media/videobuf-core.h>
+#include <media/pixel_format.h>
 #include <linux/pm.h>
 
 struct soc_camera_device {
@@ -38,9 +39,8 @@ struct soc_camera_device {
 	unsigned char buswidth;		/* See comment in .c */
 	struct soc_camera_ops *ops;
 	struct video_device *vdev;
-	const struct soc_camera_data_format *current_fmt;
-	const struct soc_camera_data_format *formats;
-	int num_formats;
+	struct pixel_formats supported_formats;
+	enum v4l2_pixel_format current_format;
 	struct module *owner;
 	/* soc_camera.c private count. Only accessed with video_lock held */
 	int use_count;

[-- Attachment #3: Type: text/plain, Size: 164 bytes --]

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-17  2:47       ` Magnus Damm
@ 2008-10-17  6:50         ` Guennadi Liakhovetski
  2008-10-17  7:55           ` Magnus Damm
  0 siblings, 1 reply; 22+ messages in thread
From: Guennadi Liakhovetski @ 2008-10-17  6:50 UTC (permalink / raw)
  To: Magnus Damm; +Cc: video4linux-list

Hi Magnus,

On Fri, 17 Oct 2008, Magnus Damm wrote:

> Hans,  any chance of that framework including pixel format helper
> code? I've hacked a bit on using a bitmap to represent pixel formats
> supported by a certain driver. The attached very rough patch maybe
> shows what i'm trying to do.
> 
> Basically, I need a simple way to determine if a camera sensor
> supports a certain pixel format, and if so then i'd like to add a
> bunch of pixel formats supported by the soc_camera host.

Thanks for the code, but, unfortunately, I don't understand what you are 
trying to do there, and why the current soc-camera pixel-format 
enumeration code doesn't suit your needs.

I know there is a problem with it, it has been discussed before on this 
list, look at this thread: http://marc.info/?t=121767492900001&r=1&w=2 but 
that's a bit of a different problem from what you are trying to do in your 
patch, AFAICS. And why are you trying to switch to some multiple arrays 
and bitmaps instead of the curent array / list of structs?

As for the format negotation code we have been discussing in that thread, 
unfortunately, up to now I haven't found time to try and implement it, and 
now my schedule doesn't look better than then:-( I'll see if I can find 
some time during the 2.6.29 development time-frame (i.e., before 2.6.28 is 
released), but, unfortunately, cannot promise anything.

But that is not your problem anyway, or is it?

Thanks
Guennadi

> The SuperH CEU "Capture Engine Unit" has a mode where it accepts
> interleaved YUV pixel formats as input from the camera sensor and
> converts them to one of NV12, NV21 or a less common 4:2:2 format maybe
> known as NV16, NV61. So I need a simple way to check and add pixel
> formats by the soc_camera host driver.
> 
> Also, having a centralized place for pixel format strings, color depth
> and colorspaces associated with each of the pixel formats makes sense
> to me. Any thoughts? Want me to hack something up?
> 
> Cheers,
> 
> / magnus
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-16 20:58     ` Hans Verkuil
  2008-10-17  2:47       ` Magnus Damm
@ 2008-10-17  7:41       ` Antonio Ospite
  1 sibling, 0 replies; 22+ messages in thread
From: Antonio Ospite @ 2008-10-17  7:41 UTC (permalink / raw)
  To: video4linux-list


[-- Attachment #1.1: Type: text/plain, Size: 1907 bytes --]

On Thu, 16 Oct 2008 22:58:28 +0200
Hans Verkuil <hverkuil@xs4all.nl> wrote:

> On Thursday 16 October 2008 21:23:59 Guennadi Liakhovetski wrote:
> > On Thu, 16 Oct 2008, Antonio Ospite wrote:
> > > On Thu, 16 Oct 2008 13:28:52 +0900
> > >
> > > Kuninori Morimoto <morimoto.kuninori@renesas.com> wrote:
> > > > This patch adds ov772x driver that use soc_camera framework.
> > >
> > > Hi, this sensor is used also in some usb cameras (Playstation Eye,
> > > for instance), and this code can be reused to improve the
> > > previously posted ov534 driver.
> > >
> > > The question is: is there any mechanism to share sensor code
> > > between usb and i2c drivers or we have to copy and paste?
> >
> > Well, this is not the first time this idea / question appears...

[...]

> > Hans Verkuil will, probably, notice, that 
> > soc-camera is not universal enough for many video applications, but
> > it might well be enough for the cideo part of a simple USB
> > web-camera, I think. OTOH, Hans is working on a new API, that should
> > unify the host / device interface in v4l applications, at which time
> > soc-camera drivers will have to be converted, as well as multiple
> > other currently existing APIs.
> 
> I'm planning to start on this this weekend. If all goes well the basic 
> framework should go into v4l-dvb soon after the 2.6.28 windows closes, 
> and it should end up in 2.6.29.
>

Hans, does you work include splitting bridge/protocol code and sensor
related code? This will clearly lead to a combinatorial increase of
supported cameras.

Regards,
   Antonio

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

  Web site: http://www.studenti.unina.it/~ospite
Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 164 bytes --]

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-17  6:50         ` Guennadi Liakhovetski
@ 2008-10-17  7:55           ` Magnus Damm
  2008-10-17  8:38             ` Guennadi Liakhovetski
  0 siblings, 1 reply; 22+ messages in thread
From: Magnus Damm @ 2008-10-17  7:55 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: video4linux-list

Hi Guennadi,

Thanks for your feedback!

On Fri, Oct 17, 2008 at 3:50 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> On Fri, 17 Oct 2008, Magnus Damm wrote:
>> Hans,  any chance of that framework including pixel format helper
>> code? I've hacked a bit on using a bitmap to represent pixel formats
>> supported by a certain driver. The attached very rough patch maybe
>> shows what i'm trying to do.
>>
>> Basically, I need a simple way to determine if a camera sensor
>> supports a certain pixel format, and if so then i'd like to add a
>> bunch of pixel formats supported by the soc_camera host.
>
> Thanks for the code, but, unfortunately, I don't understand what you are
> trying to do there, and why the current soc-camera pixel-format
> enumeration code doesn't suit your needs.

I need to check for certain pixel formats in my sh_mobile_ceu driver,
and if available then i should add a set of NVxx pixel formats and
switch the CEU hardware block to a certain operating mode. The patch
doesn't contain that part though - only the ground work to manage
pixel formats represented as a bitmap.

I _can_ do what i want to do with the current list of structs
approach, but using lists to represent supported modes seems overly
complicated. A bitmap may be a good fit since the number of pixel
modes we support through V4L2 is finite. Such a change will most
likely simplify the code and on top of that also reduce the memory
footprint.

I don't know if it fits will with your plans though.

> I know there is a problem with it, it has been discussed before on this
> list, look at this thread: http://marc.info/?t=121767492900001&r=1&w=2 but
> that's a bit of a different problem from what you are trying to do in your
> patch, AFAICS. And why are you trying to switch to some multiple arrays
> and bitmaps instead of the curent array / list of structs?

Sorry about the rough code. =) The arrays should contain centralized
knowledge about the different formats.  And I'm just using a single
bitmap to represent the supported modes that both the sensor and the
host support. Doing set_bit()/clear_bit() on a bitmap to enable and
disable formats is much easier compared to searching and modifying
lists. It's also O(1), but I don't think scalability matters here. =)

> As for the format negotation code we have been discussing in that thread,
> unfortunately, up to now I haven't found time to try and implement it, and
> now my schedule doesn't look better than then:-( I'll see if I can find
> some time during the 2.6.29 development time-frame (i.e., before 2.6.28 is
> released), but, unfortunately, cannot promise anything.

That's ok. I understand you're busy. =)

> But that is not your problem anyway, or is it?

No? I need to add NVxx support to the CEU driver, and to add that
cleanly it would be great with a simple way to represent the pixel
formats that are supported. I think the lists of structs are overly
complicated, and if Hans is rewriting things maybe the bitmap strategy
fits well, I'm not sure.

Apart from that, I can probably convince my employer that I should
spend a bit of time on this. =)

Cheers,

/ magnus

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-17  7:55           ` Magnus Damm
@ 2008-10-17  8:38             ` Guennadi Liakhovetski
  2008-10-17  9:31               ` Magnus Damm
  0 siblings, 1 reply; 22+ messages in thread
From: Guennadi Liakhovetski @ 2008-10-17  8:38 UTC (permalink / raw)
  To: Magnus Damm; +Cc: video4linux-list, Michael Schimek

On Fri, 17 Oct 2008, Magnus Damm wrote:

> Hi Guennadi,
> 
> Thanks for your feedback!
> 
> On Fri, Oct 17, 2008 at 3:50 PM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > On Fri, 17 Oct 2008, Magnus Damm wrote:
> >> Hans,  any chance of that framework including pixel format helper
> >> code? I've hacked a bit on using a bitmap to represent pixel formats
> >> supported by a certain driver. The attached very rough patch maybe
> >> shows what i'm trying to do.
> >>
> >> Basically, I need a simple way to determine if a camera sensor
> >> supports a certain pixel format, and if so then i'd like to add a
> >> bunch of pixel formats supported by the soc_camera host.
> >
> > Thanks for the code, but, unfortunately, I don't understand what you are
> > trying to do there, and why the current soc-camera pixel-format
> > enumeration code doesn't suit your needs.
> 
> I need to check for certain pixel formats in my sh_mobile_ceu driver,
> and if available then i should add a set of NVxx pixel formats and
> switch the CEU hardware block to a certain operating mode. The patch
> doesn't contain that part though - only the ground work to manage
> pixel formats represented as a bitmap.
> 
> I _can_ do what i want to do with the current list of structs
> approach, but using lists to represent supported modes seems overly
> complicated. A bitmap may be a good fit since the number of pixel
> modes we support through V4L2 is finite. Such a change will most
> likely simplify the code and on top of that also reduce the memory
> footprint.
> 
> I don't know if it fits will with your plans though.

Hm, well, let's try to think about it a bit, if I haven't forgotten yet 
how one does this:-)

We have to be able to describe the kind data that is being transferred 
from the sensor to the host, and the way how it is being transferred.

The "kind of data" is a pixel format, right?

Now, the "way it is being transferred" includes bus width, byte order, and  
packing information.

For example, the sensor driver can specify, that the sensor supports 
RGB444, is connected in 8 bit parallel mode, is sending data in BE order, 
and is leaving the highest bits 0 (example taken from MT9M111).

Now, how do we represent this? ATM we have a V4L2_PIX_FMT_RGB444 fourcc 
format. Do we introduce one more format V4L2_PIX_FMT_RGB444X for lowest 4 
bits = 0, or do we use the same fourcc code and use an additional 
enumeration to specify its packing?

I would say, it's a different fourcc format, but I'd love to hear what 
others think, Michael?

Packing all this in a single enumeration to then maintain all the formats 
in bitmaps doesn't seem very optimal to me, sorry.

Let's decide, what information uniquely describes the data and its 
on-the-wire representation, then we can think about implementing a 
negotiation algorithm in soc-camera.

> Apart from that, I can probably convince my employer that I should
> spend a bit of time on this. =)

That'd be good, let's see what the decision regarding the format 
representation and negotiation is, then you can start implementing it, if 
you like:-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Add ov772x driver
  2008-10-17  8:38             ` Guennadi Liakhovetski
@ 2008-10-17  9:31               ` Magnus Damm
  0 siblings, 0 replies; 22+ messages in thread
From: Magnus Damm @ 2008-10-17  9:31 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: video4linux-list, Michael Schimek

On Fri, Oct 17, 2008 at 5:38 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> On Fri, 17 Oct 2008, Magnus Damm wrote:
>> On Fri, Oct 17, 2008 at 3:50 PM, Guennadi Liakhovetski
>> <g.liakhovetski@gmx.de> wrote:
>> > On Fri, 17 Oct 2008, Magnus Damm wrote:
>> >> Hans,  any chance of that framework including pixel format helper
>> >> code? I've hacked a bit on using a bitmap to represent pixel formats
>> >> supported by a certain driver. The attached very rough patch maybe
>> >> shows what i'm trying to do.
>> >>
>> >> Basically, I need a simple way to determine if a camera sensor
>> >> supports a certain pixel format, and if so then i'd like to add a
>> >> bunch of pixel formats supported by the soc_camera host.
>> >
>> > Thanks for the code, but, unfortunately, I don't understand what you are
>> > trying to do there, and why the current soc-camera pixel-format
>> > enumeration code doesn't suit your needs.
>>
>> I need to check for certain pixel formats in my sh_mobile_ceu driver,
>> and if available then i should add a set of NVxx pixel formats and
>> switch the CEU hardware block to a certain operating mode. The patch
>> doesn't contain that part though - only the ground work to manage
>> pixel formats represented as a bitmap.
>>
>> I _can_ do what i want to do with the current list of structs
>> approach, but using lists to represent supported modes seems overly
>> complicated. A bitmap may be a good fit since the number of pixel
>> modes we support through V4L2 is finite. Such a change will most
>> likely simplify the code and on top of that also reduce the memory
>> footprint.
>>
>> I don't know if it fits will with your plans though.
>
> Hm, well, let's try to think about it a bit, if I haven't forgotten yet
> how one does this:-)
>
> We have to be able to describe the kind data that is being transferred
> from the sensor to the host, and the way how it is being transferred.
>
> The "kind of data" is a pixel format, right?

That's how I see it too.

> Now, the "way it is being transferred" includes bus width, byte order, and
> packing information.
>
> For example, the sensor driver can specify, that the sensor supports
> RGB444, is connected in 8 bit parallel mode, is sending data in BE order,
> and is leaving the highest bits 0 (example taken from MT9M111).

The ov772x driver is written so byte 0 of all supported pixel formats
is outputted first.

> Now, how do we represent this? ATM we have a V4L2_PIX_FMT_RGB444 fourcc
> format. Do we introduce one more format V4L2_PIX_FMT_RGB444X for lowest 4
> bits = 0, or do we use the same fourcc code and use an additional
> enumeration to specify its packing?

I'd say a new fourcc. Just to compare with ov772x again, it exports
pixel formats such as RGB565, RGB565X, RGB555 and RGB55X to allow the
soc_camera host / user space select which format that is most
suitable. I think we can support one more YUV format if it would have
a fourcc.

> I would say, it's a different fourcc format, but I'd love to hear what
> others think, Michael?
>
> Packing all this in a single enumeration to then maintain all the formats
> in bitmaps doesn't seem very optimal to me, sorry.

I'm suggesting to simply use one bit per fourcc. No extra enumeration.
I'd like to have one bitmap per driver. Today we're talking about 50
fourccs so the bitmap much smaller than most list / array combinations
i can think of - even with a single fourcc supported.

> Let's decide, what information uniquely describes the data and its
> on-the-wire representation, then we can think about implementing a
> negotiation algorithm in soc-camera.

Sure, good idea.

>> Apart from that, I can probably convince my employer that I should
>> spend a bit of time on this. =)
>
> That'd be good, let's see what the decision regarding the format
> representation and negotiation is, then you can start implementing it, if
> you like:-)

Thanks. I'd like to stay away from endless iteration over arrays though. =)

/ magnus

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2008-10-17  9:31 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-15 10:52 [PATCH] Add ov772x driver Kuninori Morimoto
2008-10-15 23:41 ` Guennadi Liakhovetski
2008-10-16  1:28   ` morimoto.kuninori
2008-10-16  2:21   ` Magnus Damm
2008-10-16  6:24     ` Guennadi Liakhovetski
2008-10-16  6:35       ` Robert Jarzmik
2008-10-16  6:46         ` Magnus Damm
2008-10-16  6:55           ` Hans Verkuil
2008-10-16  6:58             ` Magnus Damm
2008-10-16  6:49       ` Magnus Damm
  -- strict thread matches above, loose matches on Subject: below --
2008-10-16  4:28 Kuninori Morimoto
2008-10-16  8:27 ` Antonio Ospite
2008-10-16  9:53   ` morimoto.kuninori
2008-10-16 10:36     ` Magnus Damm
2008-10-16 19:23   ` Guennadi Liakhovetski
2008-10-16 20:58     ` Hans Verkuil
2008-10-17  2:47       ` Magnus Damm
2008-10-17  6:50         ` Guennadi Liakhovetski
2008-10-17  7:55           ` Magnus Damm
2008-10-17  8:38             ` Guennadi Liakhovetski
2008-10-17  9:31               ` Magnus Damm
2008-10-17  7:41       ` Antonio Ospite

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.