[PATCH v3 1/2] [media] imx214: device tree binding

[PATCH v3 1/2] [media] imx214: device tree binding

[Ricardo Ribalda Delgado]

Document bindings for imx214 camera sensor

Cc: devicetree@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
Changelog from v2:
Laurent Pinchart:

-Spell checks
-Remove frequency
-Identation
-Data lanes order

Thanks!

 .../devicetree/bindings/media/i2c/imx214.txt  | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/imx214.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/imx214.txt b/Documentation/devicetree/bindings/media/i2c/imx214.txt
new file mode 100644
index 000000000000..bf3cac731eca
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/imx214.txt
@@ -0,0 +1,52 @@
+* Sony 1/3.06-Inch 13.13Mp CMOS Digital Image Sensor
+
+The Sony imx214 is a 1/3.06-inch CMOS active pixel digital image sensor with
+an active array size of 4224H x 3200V. It is programmable through an I2C
+interface. The I2C address can be configured to to 0x1a or 0x10, depending on
+how the hardware is wired.
+Image data is sent through MIPI CSI-2, which is configured as 4 lanes
+at 1440 Mbps.
+
+
+Required Properties:
+- compatible: value should be "sony,imx214" for imx214 sensor
+- reg: I2C bus address of the device
+- enable-gpios: GPIO descriptor for the enable pin.
+- vdddo-supply: Chip digital IO regulator (1.8V).
+- vdda-supply: Chip analog regulator (2.7V).
+- vddd-supply: Chip digital core regulator (1.12V).
+- clocks = Reference to the xclk clock.
+- clock-names = Clock name, e.g. "xclk".
+- clock-frequency = Frequency of the xclk clock. (Currently the
+	driver only supports <24000000>).
+
+Optional Properties:
+- flash-leds: See ../video-interfaces.txt
+- lens-focus: See ../video-interfaces.txt
+
+The imx274 device node should contain one 'port' child node with
+an 'endpoint' subnode. For further reading on port node refer to
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+	camera_rear@1a {
+		compatible = "sony,imx214";
+		reg = <0x1a>;
+		vdddo-supply = <&pm8994_lvs1>;
+		vddd-supply = <&camera_vddd_1v12>;
+		vdda-supply = <&pm8994_l17>;
+		lens-focus = <&ad5820>;
+		enable-gpios = <&msmgpio 25 GPIO_ACTIVE_HIGH>;
+		clocks = <&mmcc CAMSS_MCLK0_CLK>;
+		clock-names = "xclk";
+		clock-frequency = <24000000>;
+		port {
+			imx214_ep: endpoint {
+				clock-lanes = <0>;
+				data-lanes = <1 2 3 4>;
+				link-frequencies = /bits/ 64 <480000000>;
+				remote-endpoint = <&csiphy0_ep>;
+			};
+		};
+	};
-- 
2.19.0

Re: [PATCH v3 1/2] [media] imx214: device tree binding

[Philippe De Muyter]

On Tue, Oct 02, 2018 at 03:46:48PM +0200, Ricardo Ribalda Delgado wrote:
> Document bindings for imx214 camera sensor
 ...
> +Optional Properties:
> +- flash-leds: See ../video-interfaces.txt
> +- lens-focus: See ../video-interfaces.txt
> +
> +The imx274 device node should contain one 'port' child node with

imx214 

Re: [PATCH v3 1/2] [media] imx214: device tree binding

[Laurent Pinchart]

Hi Sakari,

On Tuesday, 2 October 2018 16:57:38 EEST Sakari Ailus wrote:
> On Tue, Oct 02, 2018 at 04:47:55PM +0300, Laurent Pinchart wrote:
> > On Tuesday, 2 October 2018 16:30:57 EEST Ricardo Ribalda Delgado wrote:
> ...
> 
> > > +- clock-frequency = Frequency of the xclk clock. (Currently the
> > > +	driver only supports <24000000>).
> > 
> > Please don't mention drivers in DT bindings. I would drop the reference to
> > the 24 MHz limitation.
> > 
> > I would actually drop the property completely :-) I don't see why you need
> > it, and you don't make use of it in the driver.
> 
> Would you rely on assigned-clock-rates or what? There's no guarantee it'll
> actually be the desired frequency. That said, few (or no) drivers checks
> what they get when they set the frequency.

No necessarily. I would either really on assigned-clock-rates + clock rate 
check in the driver, or on no DT property and a clock rate set in the driver. 
Patch 2/2 sets the clock rate in the driver to 24 MHz regardless of the clock-
frequency property in DT.

> > > +Optional Properties:
> > > +- flash-leds: See ../video-interfaces.txt
> > > +- lens-focus: See ../video-interfaces.txt
> > > +
> > > +The imx274 device node should contain one 'port' child node with
> 
> s/should/shall/?
> 
> If there's a need to support no port nodes, then say "one or none" or such.
> Usually that's useful on the receiver side only though.
> 
> > > +Example:
> > > +
> > > +	camera_rear@1a {
> > > +		compatible = "sony,imx214";
> > > +		reg = <0x1a>;
> > > +		vdddo-supply = <&pm8994_lvs1>;
> > > +		vddd-supply = <&camera_vddd_1v12>;
> > > +		vdda-supply = <&pm8994_l17>;
> > > +		lens-focus = <&ad5820>;
> > > +		enable-gpios = <&msmgpio 25 GPIO_ACTIVE_HIGH>;
> > > +		clocks = <&mmcc CAMSS_MCLK0_CLK>;
> > > +		clock-names = "xclk";
> > > +		clock-frequency = <24000000>;
> > > +		port {
> > > +			imx214_ep: endpoint {
> > > +				clock-lanes = <0>;
> 
> I'd only put clock-lanes if the lane ordering is configurable.
> 
> > > +				data-lanes = <1 2 3 4>;
> > > +				link-frequencies = /bits/ 64 <480000000>;
> > > +				remote-endpoint = <&csiphy0_ep>;
> > > +			};
> > > +		};
> > > +	};

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 1/2] [media] imx214: device tree binding

[Sakari Ailus]

Hi Laurent,

On Tue, Oct 02, 2018 at 04:47:55PM +0300, Laurent Pinchart wrote:
> Hi Ricardo,
> 
> Thank you for the patch.
> 
> On Tuesday, 2 October 2018 16:30:57 EEST Ricardo Ribalda Delgado wrote:

...

> > +- clock-frequency = Frequency of the xclk clock. (Currently the
> > +	driver only supports <24000000>).
> 
> Please don't mention drivers in DT bindings. I would drop the reference to the 
> 24 MHz limitation.
> 
> I would actually drop the property completely :-) I don't see why you need it, 
> and you don't make use of it in the driver.

Would you rely on assigned-clock-rates or what? There's no guarantee it'll
actually be the desired frequency. That said, few (or no) drivers checks
what they get when they set the frequency.

> 
> > +Optional Properties:
> > +- flash-leds: See ../video-interfaces.txt
> > +- lens-focus: See ../video-interfaces.txt
> > +
> > +The imx274 device node should contain one 'port' child node with

s/should/shall/?

If there's a need to support no port nodes, then say "one or none" or such.
Usually that's useful on the receiver side only though.

> > +Example:
> > +
> > +	camera_rear@1a {
> > +		compatible = "sony,imx214";
> > +		reg = <0x1a>;
> > +		vdddo-supply = <&pm8994_lvs1>;
> > +		vddd-supply = <&camera_vddd_1v12>;
> > +		vdda-supply = <&pm8994_l17>;
> > +		lens-focus = <&ad5820>;
> > +		enable-gpios = <&msmgpio 25 GPIO_ACTIVE_HIGH>;
> > +		clocks = <&mmcc CAMSS_MCLK0_CLK>;
> > +		clock-names = "xclk";
> > +		clock-frequency = <24000000>;
> > +		port {
> > +			imx214_ep: endpoint {
> > +				clock-lanes = <0>;

I'd only put clock-lanes if the lane ordering is configurable.

> > +				data-lanes = <1 2 3 4>;
> > +				link-frequencies = /bits/ 64 <480000000>;
> > +				remote-endpoint = <&csiphy0_ep>;
> > +			};
> > +		};
> > +	};

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH v3 1/2] [media] imx214: device tree binding

[Laurent Pinchart]

Hi Ricardo,

Thank you for the patch.

On Tuesday, 2 October 2018 16:30:57 EEST Ricardo Ribalda Delgado wrote:
> Document bindings for imx214 camera sensor
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> Changelog v2:
> Laurent Pinchart:
> 
> -Spell checks
> -Remove frequency
> -Identation
> -Data lanes order
> 
> Thanks!
> 
>  .../devicetree/bindings/media/i2c/imx214.txt  | 52 +++++++++++++++++++
>  1 file changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/imx214.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/imx214.txt
> b/Documentation/devicetree/bindings/media/i2c/imx214.txt new file mode
> 100644
> index 000000000000..bf3cac731eca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/imx214.txt
> @@ -0,0 +1,52 @@
> +* Sony 1/3.06-Inch 13.13Mp CMOS Digital Image Sensor
> +
> +The Sony imx214 is a 1/3.06-inch CMOS active pixel digital image sensor
> with +an active array size of 4224H x 3200V. It is programmable through an
> I2C +interface. The I2C address can be configured to to 0x1a or 0x10,

s/to to/to/

> depending on +how the hardware is wired.
> +Image data is sent through MIPI CSI-2, which is configured as 4 lanes
> +at 1440 Mbps.

I suppose this is the maxium, with the actual frequency and number of lanes 
being configurable ? I would state it so explicitly then.

> +Required Properties:
> +- compatible: value should be "sony,imx214" for imx214 sensor
> +- reg: I2C bus address of the device
> +- enable-gpios: GPIO descriptor for the enable pin.
> +- vdddo-supply: Chip digital IO regulator (1.8V).
> +- vdda-supply: Chip analog regulator (2.7V).
> +- vddd-supply: Chip digital core regulator (1.12V).
> +- clocks = Reference to the xclk clock.

s/ = /: /

Same below.

> +- clock-names = Clock name, e.g. "xclk".

As the name "xclk" is mandatory I wouldn't call it an example. You can just 
say

- clock-names: Shall be "xclk".

> +- clock-frequency = Frequency of the xclk clock. (Currently the
> +	driver only supports <24000000>).

Please don't mention drivers in DT bindings. I would drop the reference to the 
24 MHz limitation.

I would actually drop the property completely :-) I don't see why you need it, 
and you don't make use of it in the driver.

> +Optional Properties:
> +- flash-leds: See ../video-interfaces.txt
> +- lens-focus: See ../video-interfaces.txt
> +
> +The imx274 device node should contain one 'port' child node with
> +an 'endpoint' subnode. For further reading on port node refer to
> +Documentation/devicetree/bindings/media/video-interfaces.txt.

Please also document the properties of the endpoint node. You can just list 
the ones that are required and the ones that are optional, and reference the 
same document for their definition.

> +Example:
> +
> +	camera_rear@1a {
> +		compatible = "sony,imx214";
> +		reg = <0x1a>;
> +		vdddo-supply = <&pm8994_lvs1>;
> +		vddd-supply = <&camera_vddd_1v12>;
> +		vdda-supply = <&pm8994_l17>;
> +		lens-focus = <&ad5820>;
> +		enable-gpios = <&msmgpio 25 GPIO_ACTIVE_HIGH>;
> +		clocks = <&mmcc CAMSS_MCLK0_CLK>;
> +		clock-names = "xclk";
> +		clock-frequency = <24000000>;
> +		port {
> +			imx214_ep: endpoint {
> +				clock-lanes = <0>;
> +				data-lanes = <1 2 3 4>;
> +				link-frequencies = /bits/ 64 <480000000>;
> +				remote-endpoint = <&csiphy0_ep>;
> +			};
> +		};
> +	};

-- 
Regards,

Laurent Pinchart

[PATCH v3 1/2] [media] imx214: device tree binding

[Ricardo Ribalda Delgado]

Document bindings for imx214 camera sensor

Cc: devicetree@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
Changelog v2:
Laurent Pinchart:

-Spell checks
-Remove frequency
-Identation
-Data lanes order

Thanks!

 .../devicetree/bindings/media/i2c/imx214.txt  | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/imx214.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/imx214.txt b/Documentation/devicetree/bindings/media/i2c/imx214.txt
new file mode 100644
index 000000000000..bf3cac731eca
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/imx214.txt
@@ -0,0 +1,52 @@
+* Sony 1/3.06-Inch 13.13Mp CMOS Digital Image Sensor
+
+The Sony imx214 is a 1/3.06-inch CMOS active pixel digital image sensor with
+an active array size of 4224H x 3200V. It is programmable through an I2C
+interface. The I2C address can be configured to to 0x1a or 0x10, depending on
+how the hardware is wired.
+Image data is sent through MIPI CSI-2, which is configured as 4 lanes
+at 1440 Mbps.
+
+
+Required Properties:
+- compatible: value should be "sony,imx214" for imx214 sensor
+- reg: I2C bus address of the device
+- enable-gpios: GPIO descriptor for the enable pin.
+- vdddo-supply: Chip digital IO regulator (1.8V).
+- vdda-supply: Chip analog regulator (2.7V).
+- vddd-supply: Chip digital core regulator (1.12V).
+- clocks = Reference to the xclk clock.
+- clock-names = Clock name, e.g. "xclk".
+- clock-frequency = Frequency of the xclk clock. (Currently the
+	driver only supports <24000000>).
+
+Optional Properties:
+- flash-leds: See ../video-interfaces.txt
+- lens-focus: See ../video-interfaces.txt
+
+The imx274 device node should contain one 'port' child node with
+an 'endpoint' subnode. For further reading on port node refer to
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+	camera_rear@1a {
+		compatible = "sony,imx214";
+		reg = <0x1a>;
+		vdddo-supply = <&pm8994_lvs1>;
+		vddd-supply = <&camera_vddd_1v12>;
+		vdda-supply = <&pm8994_l17>;
+		lens-focus = <&ad5820>;
+		enable-gpios = <&msmgpio 25 GPIO_ACTIVE_HIGH>;
+		clocks = <&mmcc CAMSS_MCLK0_CLK>;
+		clock-names = "xclk";
+		clock-frequency = <24000000>;
+		port {
+			imx214_ep: endpoint {
+				clock-lanes = <0>;
+				data-lanes = <1 2 3 4>;
+				link-frequencies = /bits/ 64 <480000000>;
+				remote-endpoint = <&csiphy0_ep>;
+			};
+		};
+	};
-- 
2.19.0