Hi Laurent,

On 08/17/2016 02:44 PM, Laurent Pinchart wrote:

Hi Hans,

Thank you for the patch.

On Wednesday 17 Aug 2016 08:29:41 Hans Verkuil wrote:

quoted

From: Hans Verkuil <redacted>

Add DT support. Use it to get the reset and pwdn pins (if there are any).
Tested with one sensor requiring reset/pwdn and one sensor that doesn't
have reset/pwdn pins.

Signed-off-by: Hans Verkuil <redacted>
---
 .../devicetree/bindings/media/i2c/ov7670.txt       | 44 +++++++++++++++++
 MAINTAINERS                                        |  1 +
 drivers/media/i2c/ov7670.c                         | 51 +++++++++++++++++++
 3 files changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7670.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov7670.txt

b/Documentation/devicetree/bindings/media/i2c/ov7670.txt new file mode
100644
index 0000000..3231c47

--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov7670.txt

@@ -0,0 +1,44 @@
+* Omnivision OV7670 CMOS sensor
+
+The Omnivision OV7670 sensor support multiple resolutions output, such as

s/support/supports/

quoted

+CIF, SVGA, UXGA. It also can support YUV422/420, RGB565/555 or raw RGB
+output format.

s/format/formats/ (and possibly s/can support/can support the/)

quoted

+
+Required Properties:
+- compatible: should be "ovti,ov7670"
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- resetb-gpios: reference to the GPIO connected to the resetb pin, if any.
+- pwdn-gpios: reference to the GPIO connected to the pwdn pin, if any.
+
+The device node must contain one 'port' child node for its digital output
+video port, in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+	i2c1: i2c@f0018000 {
+		status = "okay";
+
+		ov7670: camera@0x21 {
+			compatible = "ovti,ov7670";
+			reg = <0x21>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_pck0_as_isi_mck
&pinctrl_sensor_power
&pinctrl_sensor_reset>;

The pinctrl properties should be part of the clock provider DT node.

Do you have examples of that?

I just copied this from existing atmel dts code (arch/arm/boot/dts/sama5d3xmb.dtsi).

quoted

+			resetb-gpios = <&pioE 11 GPIO_ACTIVE_LOW>;
+			pwdn-gpios = <&pioE 13 GPIO_ACTIVE_HIGH>;
+			clocks = <&pck0>;
+			clock-names = "xvclk";
+			assigned-clocks = <&pck0>;
+			assigned-clock-rates = <24000000>;

You should compute and set the clock rate dynamically in the driver, not 
hardcode it in DT.

Do you have an example of that? Again, I just copied this from the same sama5d3xmb.dtsi.

Regards,

	Hans