Add support for Omnivision OV5647
Subsystems (4)
v10 cover Add support for Omnivision OV5647
Hello,
This patchset adds support for the Omnivision OV5647 sensor.
At the moment it only supports 640x480 in RAW 8.
This is the tenth version of the OV5647 camera driver patchset.
v10:
- Add more error info to model check
- Remove empty callback
- Change error message in read function
- Change clock handling
- Add error checking where it was missing
- Remove debug messages
- Remove the need for clock name
- Code refactoring
v9:
- Remove unused struct
- Remove comments
- Refactor error handling in i2c r/w functions
- Change declarations to single line.
- Remove value assignment in variable declarion
- Refactor configurion write loop
- Change the variable type that received ov5647_read() read value
- Remove print from probe function
- Remove unused device struct
- Remove OF dependency from Kconfig
v8:
- Remove a part of the initialization procedure which wasn't doing anything
- Check for i2c read/writes return values
- Add stream_on/off functions
v7:
- Remove "0x" and leading 0 from DT documentation examples
v6:
- Add example to DT documentation
- Remove data-lanes and clock-lane property from DT
- Add external clock property to DT
- Order includes
- Remove unused variables and functions
- Add external clock handling
- Add power on counter
- Change from g/s_parm to g/s_frame_interval
v5:
- Refactor code
- Change comments
- Add missing error handling in some functions
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):
Documentation: DT: Add OV5647 bindings
media: i2c: Add support for OV5647 sensor.
.../devicetree/bindings/media/i2c/ov5647.txt | 35 ++ MAINTAINERS | 7 + drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/ov5647.c | 636 +++++++++++++++++++++ 5 files changed, 690 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt create mode 100644 drivers/media/i2c/ov5647.c
Signature · 1 line
2.11.0
PATCH v10 1/2 Documentation: DT: Add OV5647 bindings
Create device tree bindings documentation.
--- .../devicetree/bindings/media/i2c/ov5647.txt | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
Documentation/devicetree/bindings/media/i2c/ov5647.txt new file +35 -0
@@ -0,0 +1,35 @@ +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. +- clocks : Reference to the xclk clock. + +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. + +Endpoint node mandatory properties: + +- remote-endpoint: A phandle to the bus receiver's endpoint node. + +Example: + + i2c@2000 { + ... + ov: camera@36 { + compatible = "ovti,ov5647"; + reg = <0x36>; + clocks = <&camera_clk>; + port { + camera_1: endpoint { + remote-endpoint = <&csi1_ep1>; + }; + }; + }; + };
Signature · 1 line
2.11.0
Rob Herring
maintainerRe: [PATCH v10 1/2] Documentation: DT: Add OV5647 bindings
Quoted 7 lines · Ramiro Oliveira
Create device tree bindings documentation.
Signed-off-by Ramiro Oliveira <[email protected]>--- .../devicetree/bindings/media/i2c/ov5647.txt | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
There's no changelog here, so I can't tell if anything is changed, but I acked v7. Please add acks when sending new versions.
Re: [PATCH v10 1/2] Documentation: DT: Add OV5647 bindings
Hi Rob
Quoted 12 lines · Rob Herring
Quoted 7 lines · Ramiro Oliveira
Create device tree bindings documentation.
Signed-off-by Ramiro Oliveira <[email protected]>--- .../devicetree/bindings/media/i2c/ov5647.txt | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txtThere's no changelog here, so I can't tell if anything is changed, but I acked v7. Please add acks when sending new versions.
The changelog is in the cover letter, although I didn't specify which changes where made in the driver and which were made in the Documentation.
The only change was removing the clock name since there was only one clock used.
Should I keep your ack?
Signature · 4 lines
Best Regards Ramiro Oliveira [email protected]
Rob Herring
maintainerRe: [PATCH v10 1/2] Documentation: DT: Add OV5647 bindings
Quoted 16 lines · Ramiro Oliveira
Hi Rob
Quoted 12 lines · Rob Herring
Quoted 7 lines · Ramiro Oliveira
Create device tree bindings documentation.
--- .../devicetree/bindings/media/i2c/ov5647.txt | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txtThere's no changelog here, so I can't tell if anything is changed, but I acked v7. Please add acks when sending new versions.
Quoted 6 lines
The changelog is in the cover letter, although I didn't specify which changes where made in the driver and which were made in the Documentation.
The only change was removing the clock name since there was only one clock used.
Should I keep your ack?
Yes.
Rob
--
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
Re: [PATCH v10 1/2] Documentation: DT: Add OV5647 bindings
In reply to Ramiro OliveiraHi Ramiro,
Quoted 3 lines · Ramiro Oliveira
Create device tree bindings documentation.
Signed-off-by Ramiro Oliveira <[email protected]>
The device tree binding description looks perfect from my perspective.
--
With best wishes,
Vladimir
PATCH v10 2/2 media: i2c: Add support for OV5647 sensor.
In reply to Ramiro OliveiraThe OV5647 sensor from Omnivision supports up to 2592x1944 @ 15 fps, RAW 8 and RAW 10 output formats, and MIPI CSI-2 interface.
The driver adds support for 640x480 RAW 8.
--- MAINTAINERS | 7 + drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/ov5647.c | 636 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 655 insertions(+) create mode 100644 drivers/media/i2c/ov5647.c
MAINTAINERS +7 -0
@@ -9141,6 +9141,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 +11 -0
@@ -531,6 +531,17 @@ 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 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
@@ -83,3 +83,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 +636 -0
636 lines not on this page · raw message
Signature · 1 line
2.11.0
Re: [PATCH v10 2/2] media: i2c: Add support for OV5647 sensor.
Hi Ramiro,
On Mon, Mar 06, 2017 at 11:16:34AM +0000, Ramiro Oliveira wrote:
...
drivers/media/i2c/ov5647.c line 296
Quoted 7 lines
drivers/media/i2c/ov5647.c new file +7
-0+static int __sensor_init (struct v4l2_subdev *sd) +{ +int ret; +u8 resetval, rdval; +struct i2c_client *client =v4l2_get_subdevdata (sd); + +dev_dbg (&client->dev,"sensor init\n" );
This looks like a debugging time leftover. Please remove.
With that,
...
Quoted 6 lines
drivers/media/i2c/ov5647.c new file +6
-0+static int ov5647_parse_dt (struct device_node *np) +{ +struct v4l2_of_endpoint bus_cfg; +struct device_node *ep; + +int ret;
drivers/media/i2c/ov5647.c line 528
Quoted 10 lines
drivers/media/i2c/ov5647.c new file +10
-0+ + ep =of_graph_get_next_endpoint (np,NULL ); +if (!ep) +return -EINVAL ; + + ret =v4l2_of_parse_endpoint (ep, &bus_cfg); + +of_node_put (ep); +return ret; +}
This will conflict with my fwnode patchset. Let's see in which order the patches will be merged, one of the sets has to be changed. The work is trivial though.
Signature · 8 lines
Kind regards, Sakari Ailus e-mail: [email protected] XMPP: [email protected] -- 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
Re: [PATCH v10 2/2] media: i2c: Add support for OV5647 sensor.
Hi Sakari
Quoted 4 lines · Sakari Ailus
Hi Ramiro,
On Mon, Mar 06, 2017 at 11:16:34AM +0000, Ramiro Oliveira wrote:
...
Quoted 10 lines
drivers/media/i2c/ov5647.c line 296
Quoted 7 lines
drivers/media/i2c/ov5647.c new file +7
-0+static int __sensor_init (struct v4l2_subdev *sd) +{ +int ret; +u8 resetval, rdval; +struct i2c_client *client =v4l2_get_subdevdata (sd); + +dev_dbg (&client->dev,"sensor init\n" );This looks like a debugging time leftover. Please remove.
Should I send a v11 with this change?
Quoted 27 lines
With that,
Acked-by Sakari Ailus <[email protected]>...
Quoted 6 lines
drivers/media/i2c/ov5647.c new file +6
-0+static int ov5647_parse_dt (struct device_node *np) +{ +struct v4l2_of_endpoint bus_cfg; +struct device_node *ep; + +int ret;drivers/media/i2c/ov5647.c line 528
Quoted 10 lines
drivers/media/i2c/ov5647.c new file +10
-0+ + ep =of_graph_get_next_endpoint (np,NULL ); +if (!ep) +return -EINVAL ; + + ret =v4l2_of_parse_endpoint (ep, &bus_cfg); + +of_node_put (ep); +return ret; +}This will conflict with my fwnode patchset. Let's see in which order the patches will be merged, one of the sets has to be changed. The work is trivial though.
Signature · 4 lines
Best Regards Ramiro Oliveira [email protected]
Re: [PATCH v10 2/2] media: i2c: Add support for OV5647 sensor.
Hi Ramiro,
Quoted 18 lines · Ramiro Oliveira
Hi Sakari
Quoted 4 lines · Sakari Ailus
Hi Ramiro,
On Mon, Mar 06, 2017 at 11:16:34AM +0000, Ramiro Oliveira wrote:
...Quoted 10 lines
drivers/media/i2c/ov5647.c line 296
Quoted 7 lines
drivers/media/i2c/ov5647.c new file +7
-0+static int __sensor_init (struct v4l2_subdev *sd) +{ +int ret; +u8 resetval, rdval; +struct i2c_client *client =v4l2_get_subdevdata (sd); + +dev_dbg (&client->dev,"sensor init\n" );This looks like a debugging time leftover. Please remove.
Quoted 1 line
Should I send a v11 with this change?
Please do; you can add my ack on that one.
Quoted 7 lines
Quoted 7 lines
With that,
...
Signature · 8 lines
Regards, Sakari Ailus e-mail: [email protected] XMPP: [email protected] -- 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
Re: [PATCH v10 2/2] media: i2c: Add support for OV5647 sensor.
In reply to Ramiro OliveiraHi Ramiro,
Quoted 6 lines · Ramiro Oliveira
The OV5647 sensor from Omnivision supports up to 2592x1944 @ 15 fps, RAW 8 and RAW 10 output formats, and MIPI CSI-2 interface.
The driver adds support for 640x480 RAW 8.
All updates are fine, thank you. Feel free to add my
Quoted 5 lines
--- MAINTAINERS | 7 + drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/ov5647.c | 636 +++++++++++++++++++++++++++++++++++++++++++++
I see this version has 100 LoC less in comparison to v8, good result.
[snip]
drivers/media/i2c/ov5647.c line 447
Quoted 3 lines
drivers/media/i2c/ov5647.c new file +3
-0+ +static const struct v4l2_subdev_pad_ops ov5647_subdev_pad_ops = { + .enum_mbus_code = ov5647_enum_mbus_code,
Nitpicking, above it's better to swap tab and space symbols around '='.
drivers/media/i2c/ov5647.c line 455
Quoted 8 lines
drivers/media/i2c/ov5647.c new file +8
-0+}; + +static const struct v4l2_subdev_ops ov5647_subdev_ops = { + .core = &ov5647_subdev_core_ops, + .video = &ov5647_subdev_video_ops, + .pad = &ov5647_subdev_pad_ops, +}; +
--
With best wishes,
Vladimir
--
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