Re: [PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Hi Fabio,
On Tue 24 Apr 2018 at 15:53, Fabio Estevam wrote:

Hi Rui,

On Thu, Apr 19, 2018 at 8:00 AM, Rui Miguel Silva 
 wrote:
Add device tree binding documentation for the OV2680 camera 
sensor.


Reviewed-by: Rob Herring 
CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva 
---
 .../devicetree/bindings/media/i2c/ov2680.txt  | 40 
 +++

 1 file changed, 40 insertions(+)
 create mode 100644 
 Documentation/devicetree/bindings/media/i2c/ov2680.txt


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

new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".


You missed to pass the camera power supplies as required 
properties:


Urgh, yes, you are right, I will add this.

---
Cheers,
Rui



DOVDD-supply
AVDD-supply
DVDD-supply

Re: [PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Fabio Estevam]

Hi Rui,

On Thu, Apr 19, 2018 at 8:00 AM, Rui Miguel Silva  wrote:
> Add device tree binding documentation for the OV2680 camera sensor.
>
> Reviewed-by: Rob Herring 
> CC: devicet...@vger.kernel.org
> Signed-off-by: Rui Miguel Silva 
> ---
>  .../devicetree/bindings/media/i2c/ov2680.txt  | 40 +++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> new file mode 100644
> index ..0e29f1a113c0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> @@ -0,0 +1,40 @@
> +* Omnivision OV2680 MIPI CSI-2 sensor
> +
> +Required Properties:
> +- compatible: should be "ovti,ov2680".
> +- clocks: reference to the xvclk input clock.
> +- clock-names: should be "xvclk".

You missed to pass the camera power supplies as required properties:

DOVDD-supply
AVDD-supply
DVDD-supply

Re: [PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Fabio Estevam]

On Mon, Apr 23, 2018 at 12:09 PM, Rui Miguel Silva  wrote:

> Yes, you are correct, I will fix this, and the dts entry.

As this pin has both reset and powerdown functionalities, maybe you
can name the property as 'reset-gpios'

Thanks

Re: [PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Hi Fabio,
Thanks for the review.

On Mon 23 Apr 2018 at 14:11, Fabio Estevam wrote:

Hi Rui,

On Thu, Apr 19, 2018 at 8:00 AM, Rui Miguel Silva 
 wrote:



+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the 
powerdown pin,
+if any. This is an active high signal to 
the OV2680.


I looked at the OV2680 datasheet and I see a pin called XSHUTDN, 
which has

the following description:

XSHUTDN: reset and power down (active low with internal pull 
down resistor)


So it should be active low, not active high.


Yes, you are correct, I will fix this, and the dts entry.

Thanks.

---
Cheers,
Rui

Re: [PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Fabio Estevam]

Hi Rui,

On Thu, Apr 19, 2018 at 8:00 AM, Rui Miguel Silva  wrote:

> +Optional Properties:
> +- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
> +if any. This is an active high signal to the OV2680.

I looked at the OV2680 datasheet and I see a pin called XSHUTDN, which has
the following description:

XSHUTDN: reset and power down (active low with internal pull down resistor)

So it should be active low, not active high.

[PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Add device tree binding documentation for the OV2680 camera sensor.

Reviewed-by: Rob Herring 
CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva 
---
 .../devicetree/bindings/media/i2c/ov2680.txt  | 40 +++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+if any. This is an active high signal to the OV2680.
+
+The device node must contain one 'port' child node for its digital output
+video port, and this port must have a single endpoint in accordance with
+ the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Endpoint node required properties for CSI-2 connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint node.
+- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
+- data-lanes: should be set to <1> (one CSI-2 lane supported).
+ 
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+   remote-endpoint = <_sensor_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
2.17.0