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

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

[Rui Miguel Silva]

Add device tree binding documentation for the OV2680 camera sensor.

CC: devicetree@vger.kernel.org
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
---
 .../devicetree/bindings/media/i2c/ov2680.txt  | 46 +++++++++++++++++++
 1 file changed, 46 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 000000000000..11e925ed9dad
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,46 @@
+* 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".
+- DOVDD-supply: Digital I/O voltage supply.
+- DVDD-supply: Digital core voltage supply.
+- AVDD-supply: Analog voltage supply.
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the powerdown/reset pin,
+               if any. This is an active low 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:
+
+&i2c2 {
+	ov2680: camera-sensor@36 {
+		compatible = "ovti,ov2680";
+		reg = <0x36>;
+		clocks = <&osc>;
+		clock-names = "xvclk";
+		reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
+		DOVDD-supply = <&sw2_reg>;
+		DVDD-supply = <&sw2_reg>;
+		AVDD-supply = <&reg_peri_3p15v>;
+
+		port {
+			ov2680_to_mipi: endpoint {
+				remote-endpoint = <&mipi_from_sensor>;
+				clock-lanes = <0>;
+				data-lanes = <1>;
+			};
+		};
+	};
+};
-- 
2.18.0

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

[Rob Herring]

On Tue, Jul 03, 2018 at 03:08:02PM +0100, Rui Miguel Silva wrote:
> Add device tree binding documentation for the OV2680 camera sensor.
> 
> CC: devicetree@vger.kernel.org
> Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
> ---
>  .../devicetree/bindings/media/i2c/ov2680.txt  | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

Please add acks/reviews when posting new versions.

Rob

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

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 2728 bytes --]

Hi Rui,
   sorry, I'm a bit late, you're already at v7 and I don't want to
slow down inclusion with a few minor comments.

Please bear with me and see below...

On Tue, Jul 03, 2018 at 03:08:02PM +0100, Rui Miguel Silva wrote:
> Add device tree binding documentation for the OV2680 camera sensor.
>
> CC: devicetree@vger.kernel.org
> Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
> ---
>  .../devicetree/bindings/media/i2c/ov2680.txt  | 46 +++++++++++++++++++
>  1 file changed, 46 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 000000000000..11e925ed9dad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> @@ -0,0 +1,46 @@
> +* 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".

Having a single clock source I think you can omit 'clock-names' (or at
least not marking it as required)

> +- DOVDD-supply: Digital I/O voltage supply.
> +- DVDD-supply: Digital core voltage supply.
> +- AVDD-supply: Analog voltage supply.
> +
> +Optional Properties:
> +- reset-gpios: reference to the GPIO connected to the powerdown/reset pin,
> +               if any. This is an active low 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).

What is the value of marking as required two properties which can only have
default values (the sensor does not support clock on different lanes,
nor it supports more than 1 data lane) ?

Thanks
   j

> +
> +Example:
> +
> +&i2c2 {
> +	ov2680: camera-sensor@36 {
> +		compatible = "ovti,ov2680";
> +		reg = <0x36>;
> +		clocks = <&osc>;
> +		clock-names = "xvclk";
> +		reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
> +		DOVDD-supply = <&sw2_reg>;
> +		DVDD-supply = <&sw2_reg>;
> +		AVDD-supply = <&reg_peri_3p15v>;
> +
> +		port {
> +			ov2680_to_mipi: endpoint {
> +				remote-endpoint = <&mipi_from_sensor>;
> +				clock-lanes = <0>;
> +				data-lanes = <1>;
> +			};
> +		};
> +	};
> +};
> --
> 2.18.0
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Rui Miguel Silva]

Hi Jacopo,
Hope your fine.
Thanks for the review.

On Wed 04 Jul 2018 at 09:58, jacopo mondi wrote:
> Hi Rui,
>    sorry, I'm a bit late, you're already at v7 and I don't want 
>    to
> slow down inclusion with a few minor comments.
>
> Please bear with me and see below...
>
> On Tue, Jul 03, 2018 at 03:08:02PM +0100, Rui Miguel Silva 
> wrote:
>> Add device tree binding documentation for the OV2680 camera 
>> sensor.
>>
>> CC: devicetree@vger.kernel.org
>> Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
>> ---
>>  .../devicetree/bindings/media/i2c/ov2680.txt  | 46 
>>  +++++++++++++++++++
>>  1 file changed, 46 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 000000000000..11e925ed9dad
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
>> @@ -0,0 +1,46 @@
>> +* 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".
>
> Having a single clock source I think you can omit 'clock-names' 
> (or at
> least not marking it as required)

yeah, I see you point, but really all other OV sensors share this 
and
the bellow clock/data-lanes properties as required, I will let Rob 
or
Sakari take a call in this one.

---
Cheers,
	Rui

>
>> +- DOVDD-supply: Digital I/O voltage supply.
>> +- DVDD-supply: Digital core voltage supply.
>> +- AVDD-supply: Analog voltage supply.
>> +
>> +Optional Properties:
>> +- reset-gpios: reference to the GPIO connected to the 
>> powerdown/reset pin,
>> +               if any. This is an active low 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).
>
> What is the value of marking as required two properties which 
> can only have
> default values (the sensor does not support clock on different 
> lanes,
> nor it supports more than 1 data lane) ?
>
> Thanks
>    j
>
>> +
>> +Example:
>> +
>> +&i2c2 {
>> +	ov2680: camera-sensor@36 {
>> +		compatible = "ovti,ov2680";
>> +		reg = <0x36>;
>> +		clocks = <&osc>;
>> +		clock-names = "xvclk";
>> +		reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
>> +		DOVDD-supply = <&sw2_reg>;
>> +		DVDD-supply = <&sw2_reg>;
>> +		AVDD-supply = <&reg_peri_3p15v>;
>> +
>> +		port {
>> +			ov2680_to_mipi: endpoint {
>> +				remote-endpoint = 
>> <&mipi_from_sensor>;
>> +				clock-lanes = <0>;
>> +				data-lanes = <1>;
>> +			};
>> +		};
>> +	};
>> +};
>> --
>> 2.18.0
>>

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

[Rob Herring]

On Wed, Jul 4, 2018 at 9:52 AM Rui Miguel Silva <rui.silva@linaro.org> wrote:
>
> Hi Jacopo,
> Hope your fine.
> Thanks for the review.
>
> On Wed 04 Jul 2018 at 09:58, jacopo mondi wrote:
> > Hi Rui,
> >    sorry, I'm a bit late, you're already at v7 and I don't want
> >    to
> > slow down inclusion with a few minor comments.
> >
> > Please bear with me and see below...
> >
> > On Tue, Jul 03, 2018 at 03:08:02PM +0100, Rui Miguel Silva
> > wrote:
> >> Add device tree binding documentation for the OV2680 camera
> >> sensor.
> >>
> >> CC: devicetree@vger.kernel.org
> >> Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
> >> ---
> >>  .../devicetree/bindings/media/i2c/ov2680.txt  | 46
> >>  +++++++++++++++++++
> >>  1 file changed, 46 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 000000000000..11e925ed9dad
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> >> @@ -0,0 +1,46 @@
> >> +* 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".
> >
> > Having a single clock source I think you can omit 'clock-names'
> > (or at
> > least not marking it as required)
>
> yeah, I see you point, but really all other OV sensors share this
> and
> the bellow clock/data-lanes properties as required, I will let Rob
> or
> Sakari take a call in this one.

I generally tell folks that *-names is not needed when there's only 1,
but having alignment across bindings is good too.

Rob