Add support for Omnivision OV5647
Subsystems (4)
v4 cover Add support for Omnivision OV5647
Hello,
This patch adds support for the Omnivision OV5647 sensor.
At the moment it only supports 640x480 in Raw 8.
This is the fourth version of the OV5647 camera driver patchset.
v4:
- Add correct license
- Revert debugging info to generic infrastructure
- Turn defines into enums
- Correct code style issues
- Remove unused defines
- Make sure all errors where being handled
- Rename some functions to make code more readable
- Add some debugging info
v3:
- No changes. Re-submitted due to lack of responses
v2:
- Corrections in DT documentation
Ramiro Oliveira (2):
Add OV5647 device tree documentation
Add support for OV5647 sensor
.../devicetree/bindings/media/i2c/ov5647.txt | 19 + MAINTAINERS | 7 + drivers/media/i2c/Kconfig | 12 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/ov5647.c | 861 +++++++++++++++++++++ 5 files changed, 900 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt create mode 100644 drivers/media/i2c/ov5647.c
Signature · 1 line
2.10.2
PATCH v4 1/2 Add OV5647 device tree documentation
Add device tree documentation.
--- .../devicetree/bindings/media/i2c/ov5647.txt | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
Documentation/devicetree/bindings/media/i2c/ov5647.txt new file +19 -0
@@ -0,0 +1,19 @@ +Omnivision OV5647 raw image sensor +--------------------------------- + +OV5647 is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces +and CCI (I2C compatible) control bus. + +Required properties: + +- compatible : "ovti,ov5647"; +- reg : I2C slave address of the sensor; + +The common video interfaces bindings (see video-interfaces.txt) should be +used to specify link to the image data receiver. The OV5647 device +node should contain one 'port' child node with an 'endpoint' subnode. + +Following properties are valid for the endpoint node: + +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in + video-interfaces.txt. The sensor supports only two data lanes.
Signature · 1 line
2.10.2
Re: [PATCH v4 1/2] Add OV5647 device tree documentation
Quoted 7 lines · Ramiro Oliveira
Add device tree documentation.
--- .../devicetree/bindings/media/i2c/ov5647.txt | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 2/2 Add support for OV5647 sensor
In reply to Ramiro OliveiraAdd support for OV5647 sensor.
Modes supported:
- 640x480 RAW 8
--- MAINTAINERS | 7 + drivers/media/i2c/Kconfig | 12 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/ov5647.c | 861 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 881 insertions(+) create mode 100644 drivers/media/i2c/ov5647.c
MAINTAINERS +7 -0
@@ -8895,6 +8895,13 @@ M: Harald Welte <[email protected]> S: Maintained F: drivers/char/pcmcia/cm4040_cs.* +OMNIVISION OV5647 SENSOR DRIVER +M: Ramiro Oliveira <[email protected]> +L: [email protected] +T: git git://linuxtv.org/media_tree.git +S: Maintained +F: drivers/media/i2c/ov5647.c + OMNIVISION OV7670 SENSOR DRIVER M: Jonathan Corbet <[email protected]> L: [email protected]
drivers/media/i2c/Kconfig +12 -0
@@ -531,6 +531,18 @@ config VIDEO_OV2659 To compile this driver as a module, choose M here: the module will be called ov2659. +config VIDEO_OV5647 + tristate "OmniVision OV5647 sensor support" + depends on OF + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API + depends on MEDIA_CAMERA_SUPPORT + ---help--- + This is a Video4Linux2 sensor-level driver for the OmniVision + OV5647 camera. + + To compile this driver as a module, choose M here: the + module will be called ov5647. + config VIDEO_OV7640 tristate "OmniVision OV7640 sensor support" depends on I2C && VIDEO_V4L2
drivers/media/i2c/Makefile +1 -0
@@ -82,3 +82,4 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o obj-$(CONFIG_VIDEO_OV2659) += ov2659.o obj-$(CONFIG_VIDEO_TC358743) += tc358743.o +obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
drivers/media/i2c/ov5647.c new file +861 -0
861 lines not on this page · raw message
Signature · 1 line
2.10.2
Re: [PATCH v4 2/2] Add support for OV5647 sensor
Hi!
Quoted 7 lines
Add support for OV5647 sensor.
drivers/media/i2c/ov5647.c new file +4
-0+static int ov5647_write (struct v4l2_subdev *sd,u16 reg,u8 val) +{ +int ret; +unsigned char data[3 ] = { reg >>8 , reg &0xff , val};
drivers/media/i2c/ov5647.c line 238
Quoted 10 lines
drivers/media/i2c/ov5647.c new file +10
-0+struct i2c_client *client =v4l2_get_subdevdata (sd); + + ret =i2c_master_send (client, data,3 ); +if (ret !=3 ) { +dev_dbg (&client->dev,"%s: i2c write error, reg: %x\n" , + __func__, reg); +return ret <0 ? ret : -EIO ; + } +return 0 ; +}
Sorry, this is wrong. It should something <0 any time error is detected.
Quoted 8 lines
drivers/media/i2c/ov5647.c new file +8
-0+static int ov5647_write_array (struct v4l2_subdev *sd, +struct regval_list *regs,int array_size) +{ +int i =0 ; +int ret =0 ; + +if (!regs) +return -EINVAL ;
drivers/media/i2c/ov5647.c line 290
Quoted 10 lines
drivers/media/i2c/ov5647.c new file +10
-0+ +while (i < array_size) { + ret =ov5647_write (sd, regs->addr, regs->data); +if (ret <0 ) +return ret; + i++; + regs++; + } +return 0 ; +}
For example this expects <0 on error.
Quoted 5 lines
drivers/media/i2c/ov5647.c new file +5
-0+static int set_sw_standby (struct v4l2_subdev *sd,bool standby) +{ +int ret; +unsigned char rdval; +
drivers/media/i2c/ov5647.c line 341
Quoted 10 lines
drivers/media/i2c/ov5647.c new file +10
-0+ ret =ov5647_read (sd,0x0100 , &rdval); +if (ret !=0 ) +return ret; + +if (standby) + ret =ov5647_write (sd,0x0100 , rdval&0xfe ); +else + ret =ov5647_write (sd,0x0100 , rdval|0x01 ); + +return ret;
if (standby)
rdval &= 0xfe;
else
rdval |= 0x01;ret = ov5647_write(sd, 0x0100, rdval);
?
drivers/media/i2c/ov5647.c line 349
Quoted 6 lines
drivers/media/i2c/ov5647.c new file +6
-0+/** +* @short Store information about the video data format. +*/ +static struct sensor_format_struct { + __u8 *desc; + u32 mbus_code;
u8 is suitable here.
drivers/media/i2c/ov5647.c line 398
Quoted 2 lines
drivers/media/i2c/ov5647.c new file +2
-0+ov5647_read (sd,0x0100 , &resetval); +if (!resetval&0x01 ) {
add ()s here.
Quoted 24 lines
drivers/media/i2c/ov5647.c new file +24
-0+static int sensor_power (struct v4l2_subdev *sd,int on) +{ +int ret; +struct ov5647 *ov5647 =to_state (sd); +struct i2c_client *client =v4l2_get_subdevdata (sd); + + ret =0 ; +mutex_lock (&ov5647->lock); + +if (on) { +dev_dbg (&client->dev,"OV5647 power on!\n" ); + + ret =ov5647_write_array (sd, sensor_oe_enable_regs, +ARRAY_SIZE (sensor_oe_enable_regs)); + + ret =__sensor_init (sd); + +if (ret <0 ) +dev_err (&client->dev, +"Camera not available! Check Power!\n" ); + }else { +dev_dbg (&client->dev,"OV5647 power off!\n" ); + +dev_dbg (&client->dev,"disable oe\n" );
drivers/media/i2c/ov5647.c line 448
Quoted 10 lines
drivers/media/i2c/ov5647.c new file +10
-0+ ret =ov5647_write_array (sd, sensor_oe_disable_regs, +ARRAY_SIZE (sensor_oe_disable_regs)); + +if (ret <0 ) +dev_dbg (&client->dev,"disable oe failed!\n" ); + + ret =set_sw_standby (sd, true); + +if (ret <0 ) +dev_dbg (&client->dev,"soft stby failed!\n" );
dev_err for errors? Little less "!"s in the output?
drivers/media/i2c/ov5647.c line 474
Quoted 11 lines
drivers/media/i2c/ov5647.c new file +11
-0+static int sensor_get_register (struct v4l2_subdev *sd, +struct v4l2_dbg_register *reg) +{ +unsigned char val =0 ; +int ret; + + ret =ov5647_read (sd, reg->reg &0xff , &val); + reg->val = val; + reg->size =1 ; +return ret; +}
Filling reg->* when read failed is strange.
drivers/media/i2c/ov5647.c line 487
Quoted 6 lines
drivers/media/i2c/ov5647.c new file +6
-0+static int sensor_set_register (struct v4l2_subdev *sd, +const struct v4l2_dbg_register *reg) +{ +ov5647_write (sd, reg->reg &0xff , reg->val &0xff ); +return 0 ; +}
error handling?
Best regards,
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH v4 2/2] Add support for OV5647 sensor
Quoted 5 lines · Pavel Machek
Hi!
Quoted 2 lines
Add support for OV5647 sensor.
Quoted 17 lines
Quoted 4 lines
drivers/media/i2c/ov5647.c new file +4
-0+static int ov5647_write (struct v4l2_subdev *sd,u16 reg,u8 val) +{ +int ret; +unsigned char data[3 ] = { reg >>8 , reg &0xff , val};drivers/media/i2c/ov5647.c line 238
Quoted 10 lines
drivers/media/i2c/ov5647.c new file +10
-0+struct i2c_client *client =v4l2_get_subdevdata (sd); + + ret =i2c_master_send (client, data,3 ); +if (ret !=3 ) { +dev_dbg (&client->dev,"%s: i2c write error, reg: %x\n" , + __func__, reg); +return ret <0 ? ret : -EIO ; + } +return 0 ; +}Sorry, this is wrong. It should something <0 any time error is detected.
It seems to me that it does return a value < 0 each time an error is detected.
Guenter
Re: [PATCH v4 2/2] Add support for OV5647 sensor
Quoted 24 lines · Guenter Roeck
Quoted 5 lines · Pavel Machek
Hi!
Quoted 2 lines
Add support for OV5647 sensor.
Quoted 17 lines
Quoted 4 lines
drivers/media/i2c/ov5647.c new file +4
-0+static int ov5647_write (struct v4l2_subdev *sd,u16 reg,u8 val) +{ +int ret; +unsigned char data[3 ] = { reg >>8 , reg &0xff , val};drivers/media/i2c/ov5647.c line 238
Quoted 10 lines
drivers/media/i2c/ov5647.c new file +10
-0+struct i2c_client *client =v4l2_get_subdevdata (sd); + + ret =i2c_master_send (client, data,3 ); +if (ret !=3 ) { +dev_dbg (&client->dev,"%s: i2c write error, reg: %x\n" , + __func__, reg); +return ret <0 ? ret : -EIO ; + } +return 0 ; +}Sorry, this is wrong. It should something <0 any time error is detected.
Quoted 1 line
It seems to me that it does return a value < 0 each time an error is detected.
Yep, you are right, sorry, I misparsed the code.
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH v4 2/2] Add support for OV5647 sensor
In reply to Pavel MachekHi Pavel!
Quoted 144 lines · Pavel Machek
Hi!
Quoted 7 lines
Add support for OV5647 sensor.
drivers/media/i2c/ov5647.c new file +4
-0+static int ov5647_write (struct v4l2_subdev *sd,u16 reg,u8 val) +{ +int ret; +unsigned char data[3 ] = { reg >>8 , reg &0xff , val};drivers/media/i2c/ov5647.c line 238
Quoted 10 lines
drivers/media/i2c/ov5647.c new file +10
-0+struct i2c_client *client =v4l2_get_subdevdata (sd); + + ret =i2c_master_send (client, data,3 ); +if (ret !=3 ) { +dev_dbg (&client->dev,"%s: i2c write error, reg: %x\n" , + __func__, reg); +return ret <0 ? ret : -EIO ; + } +return 0 ; +}Sorry, this is wrong. It should something <0 any time error is detected.
Quoted 8 lines
drivers/media/i2c/ov5647.c new file +8
-0+static int ov5647_write_array (struct v4l2_subdev *sd, +struct regval_list *regs,int array_size) +{ +int i =0 ; +int ret =0 ; + +if (!regs) +return -EINVAL ;drivers/media/i2c/ov5647.c line 290
Quoted 10 lines
drivers/media/i2c/ov5647.c new file +10
-0+ +while (i < array_size) { + ret =ov5647_write (sd, regs->addr, regs->data); +if (ret <0 ) +return ret; + i++; + regs++; + } +return 0 ; +}For example this expects <0 on error.
Quoted 5 lines
drivers/media/i2c/ov5647.c new file +5
-0+static int set_sw_standby (struct v4l2_subdev *sd,bool standby) +{ +int ret; +unsigned char rdval; +drivers/media/i2c/ov5647.c line 341
Quoted 10 lines
drivers/media/i2c/ov5647.c new file +10
-0+ ret =ov5647_read (sd,0x0100 , &rdval); +if (ret !=0 ) +return ret; + +if (standby) + ret =ov5647_write (sd,0x0100 , rdval&0xfe ); +else + ret =ov5647_write (sd,0x0100 , rdval|0x01 ); + +return ret;if (standby)
rdval &= 0xfe; else rdval |= 0x01;ret =
ov5647_write(sd, 0x0100, rdval);?
drivers/media/i2c/ov5647.c line 349
Quoted 6 lines
drivers/media/i2c/ov5647.c new file +6
-0+/** +* @short Store information about the video data format. +*/ +static struct sensor_format_struct { + __u8 *desc; + u32 mbus_code;u8 is suitable here.
drivers/media/i2c/ov5647.c line 398
Quoted 2 lines
drivers/media/i2c/ov5647.c new file +2
-0+ov5647_read (sd,0x0100 , &resetval); +if (!resetval&0x01 ) {add ()s here.
Quoted 24 lines
drivers/media/i2c/ov5647.c new file +24
-0+static int sensor_power (struct v4l2_subdev *sd,int on) +{ +int ret; +struct ov5647 *ov5647 =to_state (sd); +struct i2c_client *client =v4l2_get_subdevdata (sd); + + ret =0 ; +mutex_lock (&ov5647->lock); + +if (on) { +dev_dbg (&client->dev,"OV5647 power on!\n" ); + + ret =ov5647_write_array (sd, sensor_oe_enable_regs, +ARRAY_SIZE (sensor_oe_enable_regs)); + + ret =__sensor_init (sd); + +if (ret <0 ) +dev_err (&client->dev, +"Camera not available! Check Power!\n" ); + }else { +dev_dbg (&client->dev,"OV5647 power off!\n" ); + +dev_dbg (&client->dev,"disable oe\n" );drivers/media/i2c/ov5647.c line 448
Quoted 10 lines
drivers/media/i2c/ov5647.c new file +10
-0+ ret =ov5647_write_array (sd, sensor_oe_disable_regs, +ARRAY_SIZE (sensor_oe_disable_regs)); + +if (ret <0 ) +dev_dbg (&client->dev,"disable oe failed!\n" ); + + ret =set_sw_standby (sd, true); + +if (ret <0 ) +dev_dbg (&client->dev,"soft stby failed!\n" );
dev_errfor errors? Little less "!"s in the output?drivers/media/i2c/ov5647.c line 474
Quoted 11 lines
drivers/media/i2c/ov5647.c new file +11
-0+static int sensor_get_register (struct v4l2_subdev *sd, +struct v4l2_dbg_register *reg) +{ +unsigned char val =0 ; +int ret; + + ret =ov5647_read (sd, reg->reg &0xff , &val); + reg->val = val; + reg->size =1 ; +return ret; +}Filling reg->* when read failed is strange.
drivers/media/i2c/ov5647.c line 487
Quoted 6 lines
drivers/media/i2c/ov5647.c new file +6
-0+static int sensor_set_register (struct v4l2_subdev *sd, +const struct v4l2_dbg_register *reg) +{ +ov5647_write (sd, reg->reg &0xff , reg->val &0xff ); +return 0 ; +}error handling?
Quoted 3 lines
Best regards,
Pavel
Thanks for the feedback, I've corrected all the issues you pointed out, except the one regarding the error handling in the ov5647_write function.
Best Regards,
Ramiro
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html