diff mbox series

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

Message ID 20180703140803.19580-2-rui.silva@linaro.org
State Not Applicable, archived
Headers show
Series [v7,1/2] media: ov2680: dt: Add bindings for OV2680 | expand

Commit Message

Rui Miguel Silva July 3, 2018, 2:08 p.m. UTC 
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

Comments

Rob Herring July 3, 2018, 4:27 p.m. UTC | #1 
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jacopo Mondi July 4, 2018, 8:58 a.m. UTC | #2 
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
>
Rui Miguel Silva July 4, 2018, 3:51 p.m. UTC | #3 
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
>>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring July 5, 2018, 8:11 p.m. UTC | #4 
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

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>;
+			};
+		};
+	};
+};