ov9740: Initial submit of OV9740 driver.
PATCH v3 1/1 ov9740: Initial submit of OV9740 driver.
From: Andrew Chew <[email protected]>
This soc_camera driver is for Omnivision's OV9740 sensor. This initial submission provides support for YUV422 output at 1280x720 (720p), which is the sensor's native resolution. 640x480 (VGA) is also supported, with cropping and scaling performed by the sensor's ISP.
This driver is heavily based off of the existing OV9640 driver.
---
Applied all of the suggestions from Guennadi Liakhovetski, to wit:
Renamed to_ov9740_sensor macro to to_ov9740.
Added spaces to the sides of comments.
Some renaming of 1280x720 to 720P, and 640x480 to VGA.
Added global array of supported resolutions and their extents. Fixed spacing problem with curly braces.
Removed register arrays for the start and stop streaming register pokes. Removed IS_ERR_VALUE uses.
Changed a set of tiny memcpy's to assignment operations.
Renamed "ary" to "array" in all cases.
Removed a comment accidentally carried over from the OV9640 regarding bus width. Renamed ov9740_set_res_code() to ov9740_set_res().
Changed ov9740_try_fmt() to not return an error if format is not supported. Removed __devinit and __devexit.
Fixed an endianness issue with reading the 16-bit model.
Removed unnecessary "devname" variable in probe call.
Implemented cropcap() and g_crop() methods.
drivers/media/video/Kconfig | 6 + drivers/media/video/Makefile | 1 + drivers/media/video/ov9740.c | 1013 +++++++++++++++++++++++++++++++++++++++ include/media/v4l2-chip-ident.h | 1 + 4 files changed, 1021 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/ov9740.c
drivers/media/video/Kconfig +6 -0
@@ -837,6 +837,12 @@ config SOC_CAMERA_OV9640 help This is a ov9640 camera driver +config SOC_CAMERA_OV9740 + tristate "ov9740 camera support" + depends on SOC_CAMERA && I2C + help + This is a ov9740 camera driver + config MX1_VIDEO bool
drivers/media/video/Makefile +1 -0
@@ -79,6 +79,7 @@ obj-$(CONFIG_SOC_CAMERA_OV2640) += ov2640.o obj-$(CONFIG_SOC_CAMERA_OV6650) += ov6650.o obj-$(CONFIG_SOC_CAMERA_OV772X) += ov772x.o obj-$(CONFIG_SOC_CAMERA_OV9640) += ov9640.o +obj-$(CONFIG_SOC_CAMERA_OV9740) += ov9740.o obj-$(CONFIG_SOC_CAMERA_RJ54N1) += rj54n1cb0c.o obj-$(CONFIG_SOC_CAMERA_TW9910) += tw9910.o
drivers/media/video/ov9740.c new file +1013 -0
1,013 lines not on this page · raw message
include/media/v4l2-chip-ident.h +1 -0
@@ -75,6 +75,7 @@ enum {V4L2_IDENT_OV9640 =257 ,V4L2_IDENT_OV6650 =258 ,V4L2_IDENT_OV2640 =259 , +V4L2_IDENT_OV9740 =260 ,/* module saa7146: reserved range 300-309 */ V4L2_IDENT_SAA7146 = 300,
Signature · 1 line
1.7.0.4
Hi Andrew
Thanks for an update, looks much better now, still, a couple of things _have_ to be corrected before we can push it:
Quoted 49 lines · On Wed, 16 Feb 2011, [email protected] wrote:
From: Andrew Chew <[email protected]>
This
soc_cameradriver is for Omnivision's OV9740 sensor. This initial submission provides support for YUV422 output at 1280x720 (720p), which is the sensor's native resolution. 640x480 (VGA) is also supported, with cropping and scaling performed by the sensor's ISP.This driver is heavily based off of the existing OV9640 driver.
Change-Id I2f3b731e667a8f16df44e03eb5187176d3abb1f4Signed-off-by Andrew Chew <[email protected]>---
Applied all of the suggestions from Guennadi Liakhovetski, to wit:Renamed
to_ov9740_sensormacro toto_ov9740.
Added spaces to the sides of comments.
Some renaming of 1280x720 to 720P, and 640x480 to VGA.
Added global array of supported resolutions and their extents. Fixed spacing problem with curly braces.
Removed register arrays for the start and stop streaming register pokes. RemovedIS_ERR_VALUEuses.
Changed a set of tiny memcpy's to assignment operations.
Renamed "ary" to "array" in all cases.
Removed a comment accidentally carried over from the OV9640 regarding bus width. Renamedov9740_set_res_code()toov9740_set_res().
Changedov9740_try_fmt()to not return an error if format is not supported. Removed__devinitand__devexit.
Fixed an endianness issue with reading the 16-bit model.
Removed unnecessary "devname" variable in probe call.
Implementedcropcap()andg_crop()methods.drivers/media/video/Kconfig | 6 + drivers/media/video/Makefile | 1 + drivers/media/video/ov9740.c | 1013 +++++++++++++++++++++++++++++++++++++++ include/media/v4l2-chip-ident.h | 1 + 4 files changed, 1021 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/ov9740.cdrivers/media/video/Kconfig +0
-0drivers/media/video/ov9740.c new file +0
-0@@ -0,0 +1,1013 @@
[snip]
Quoted 14 lines
drivers/media/video/ov9740.c new file +14
-0+/* Start/Stop streaming from the device */ +static int ov9740_s_stream (struct v4l2_subdev *sd,int enable) +{ +struct i2c_client *client =v4l2_get_subdevdata (sd); +struct ov9740_priv *priv =to_ov9740 (sd); +int ret; + +/* Program orientation register. */ +if (priv->flag_vflip) + ret =ov9740_reg_rmw (client,OV9740_IMAGE_ORT ,0x2 ,0 ); +else + ret =ov9740_reg_rmw (client,OV9740_IMAGE_ORT ,0 ,0x2 ); +if (ret <0 ) +return ret;
drivers/media/video/ov9740.c line 597
Quoted 10 lines
drivers/media/video/ov9740.c new file +10
-0+ +if (priv->flag_hflip) + ret =ov9740_reg_rmw (client,OV9740_IMAGE_ORT ,0x1 ,0 ); +else + ret =ov9740_reg_rmw (client,OV9740_IMAGE_ORT ,0 ,0x1 ); +if (ret <0 ) +return ret; + +if (enable) { +dev_info (&client->dev,"Enabling Streaming\n" );
This
drivers/media/video/ov9740.c line 601
Quoted 4 lines
drivers/media/video/ov9740.c new file +4
-0+ ret =ov9740_reg_write (client,OV9740_MODE_SELECT ,0x01 ); + + }else { +dev_info (&client->dev,"Disabling Streaming\n" );
and this dev_info() you might want to change to dev_dbg(). You don't want the driver to be too noisy. A couple more occasions below
drivers/media/video/ov9740.c line 605
Quoted 4 lines
drivers/media/video/ov9740.c new file +4
-0+ ret =ov9740_reg_write (client,OV9740_SOFTWARE_RESET ,0x01 ); +if (ret <0 ) +return ret; + ret =ov9740_reg_write (client,OV9740_MODE_SELECT ,0x00 );
In the previous version you had comments for streamoff::
+ /* Software Reset */
and
+ /* Setting Streaming to Standby */
you can still use them now. Besides, the following is a bit shorter and I find it slightly better readable:
+ ret = ov9740_reg_write(client, OV9740_SOFTWARE_RESET, 0x01);
+ if (!ret)
+ ret = ov9740_reg_write(client, OV9740_MODE_SELECT, 0x00);
Quoted 10 lines
drivers/media/video/ov9740.c new file +10
-0+ } + +return ret; +} + +/* Alter bus settings on camera side */ +static int ov9740_set_bus_param (struct soc_camera_device *icd, +unsigned long flags) +{ +return 0 ;
drivers/media/video/ov9740.c line 625
Quoted 10 lines
drivers/media/video/ov9740.c new file +10
-0+} + +/* Request bus settings on camera side */ +static unsigned long ov9740_query_bus_param (struct soc_camera_device *icd) +{ +struct soc_camera_link *icl =to_soc_camera_link (icd); + +unsigned long flags =SOCAM_PCLK_SAMPLE_RISING |SOCAM_MASTER | +SOCAM_VSYNC_ACTIVE_HIGH |SOCAM_HSYNC_ACTIVE_HIGH | +SOCAM_DATA_ACTIVE_HIGH |SOCAM_DATAWIDTH_8 ;
I asked you to use the platform data here... If you don't do that, then at least, please, preserve your comment from the original version:
+ /*
+ * REVISIT: the camera probably can do 10 bit transfers, but I don't
+ * have those pins connected on my hardware.
+ */
Quoted 3 lines
drivers/media/video/ov9740.c new file +3
-0+ +return soc_camera_apply_sensor_flags (icl, flags); +}
[snip]
drivers/media/video/ov9740.c line 734
Quoted 4 lines
drivers/media/video/ov9740.c new file +4
-0+/* Setup registers according to resolution and color encoding */ +static int ov9740_set_res (struct i2c_client *client,u32 width, +enum v4l2_mbus_pixelcode code) +{
Why don't you just remove the "code" parameter as I proposed in my previous review? I don't see why you need it, and if it is ever needed, it can always be re-added.
drivers/media/video/ov9740.c line 739
Quoted 5 lines
drivers/media/video/ov9740.c new file +5
-0+int ret; + +/* select register configuration for given resolution */ +if (width == ov9740_resolutions[OV9740_VGA ].width) { +dev_info (&client->dev,"Setting image size to 640x480\n" );
dev_dbg()
drivers/media/video/ov9740.c line 743
Quoted 4 lines
drivers/media/video/ov9740.c new file +4
-0+ ret =ov9740_reg_write_array (client, ov9740_regs_vga, +ARRAY_SIZE (ov9740_regs_vga)); + }else if (width == ov9740_resolutions[OV9740_720P ].width) { +dev_info (&client->dev,"Setting image size to 1280x720\n" );
dev_dbg()
Quoted 16 lines
drivers/media/video/ov9740.c new file +15
-0+ ret =ov9740_reg_write_array (client, ov9740_regs_720p, +ARRAY_SIZE (ov9740_regs_720p)); + }else { +dev_err (&client->dev,"Failed to select resolution!\n" ); +return -EINVAL ; + } + +return ret; +} +static int ov9740_try_fmt (struct v4l2_subdev *sd, +struct v4l2_mbus_framefmt *mf) +{ +ov9740_res_roundup (&mf->width, &mf->height); + + mf->field =V4L2_FIELD_NONE ;
drivers/media/video/ov9740.c line 803
Quoted 10 lines
drivers/media/video/ov9740.c new file +10
-0+ +switch (mf->code) { +case V4L2_MBUS_FMT_YUYV8_2X8 : + mf->colorspace =V4L2_COLORSPACE_SRGB ; +break ; +default : +break ; + } + +return 0 ;
Ok, this is the important one: sorry, this is not what I meant. You shouldn't fail try_fmt(), that's right, but you should _always_ return valid data from it. In your case you just can set mf->code and mf->colorspace always to your fixed values and remove the switch completely.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/