Re: [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX219 CMOS sensor binding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sakari,

Thank you for reviewing the patchset, and for the pointers on improving the driver (nokia,smia.txt etc)!
I'll write a separate email later, or just fix what you suggested in v3 (I agree with the proposed changes
I didn't comment on in this email).

Just few quick answers below.

Thanks,
Andrey

On 27.12.2019 17:17, Sakari Ailus wrote:
Hi Andrey,

Thanks for the patchset.

On Fri, Dec 27, 2019 at 03:21:13PM +0300, Andrey Konovalov wrote:
Add YAML device tree binding for IMX219 CMOS image sensor, and
the relevant MAINTAINERS entries.

Signed-off-by: Andrey Konovalov <andrey.konovalov@xxxxxxxxxx>
---
  .../devicetree/bindings/media/i2c/imx219.yaml | 134 ++++++++++++++++++
  MAINTAINERS                                   |   8 ++
  2 files changed, 142 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/i2c/imx219.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/imx219.yaml b/Documentation/devicetree/bindings/media/i2c/imx219.yaml
new file mode 100644
index 000000000000..b58aa49a7c03
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/imx219.yaml
@@ -0,0 +1,134 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/imx219.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sony 1/4.0-Inch 8Mpixel CMOS Digital Image Sensor
+
+maintainers:
+  - Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
+
+description: |-
+  The Sony imx219 is a 1/4.0-inch CMOS active pixel digital image sensor
+  with an active array size of 3280H x 2464V. It is programmable through
+  I2C interface. The I2C address is fixed to 0x10 as per sensor data sheet.
+  Image data is sent through MIPI CSI-2, which is configured as either 2 or
+  4 data lanes.
+
+properties:
+  compatible:
+    const: sony,imx219
+
+  reg:
+    description: I2C device address
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: xclk

There's a single clock. Does it need a name? I'd just omit it.

+
+  VDIG-supply:
+    description:
+      Digital I/O voltage supply, 1.8 volts
+
+  VANA-supply:
+    description:
+      Analog voltage supply, 2.8 volts
+
+  VDDL-supply:
+    description:
+      Digital core voltage supply, 1.2 volts
+
+  xclr-gpios:
+    description: |-
+      Reference to the GPIO connected to the xclr pin, if any.
+      Must be released (set high) after all supplies are applied.

A common name for this in camera sensors is xshutdown. I'd suggest to use
that.

Indeed, "xshutdown" is the pin name commonly used by OmniVision for their sensors.
(In older sensors they used "pwdn" which is similar, but the polarity is reversed.)

In their sensor datasheets Sony consistently use "xclr" for the pin and signal otherwise
very similar to OmniVision's "xshutdown".

Wouldn't using the signal name from the sensor by the different vendor just add more confusion
instead?

+
+  camera-clk:
+    type: object
+
+    description: Clock source for imx219
+
+    properties:
+      clock-frequency: true
+
+    required:
+      - clock-frequency

Hmm. The driver doesn't seem to use this for anything.

There are two approaches to this; either you can get and check the
frequency, or specify it in DT bindings, set and then check it.

See e.g. Documentation/devicetree/bindings/media/i2c/nokia,smia.txt (not in
YAML though).

+
+  # See ../video-interfaces.txt for more details
+  port:
+    type: object
+    properties:
+      endpoint:
+        type: object
+        properties:
+          clock-lanes:
+            const: 0

If the hardware does not support lane reordering, you can omit the
clock-lanes property as it provides no information.

+
+          data-lanes:
+            description: |-
+              Should be <1 2> for two-lane operation, or <1 2 3 4> for
+              four-lane operation.
+            oneOf:
+              - const: [[ 1, 2 ]]
+              - const: [[ 1, 2, 3, 4 ]]
+
+          clock-noncontinuous:
+            type: boolean
+            description: |-
+              Presence of this boolean property decides whether the MIPI CSI-2
+              clock is continuous or non-continuous.

How about: MIPI CSI-2 clock will be non-continuous if this property is
present, otherwise it's continuous.

This statement is more clear than the original. Thanks!

+
+        required:
+          - clock-lanes
+          - data-lanes
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - VANA-supply
+  - VDIG-supply
+  - VDDL-supply
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        imx219: sensor@10 {
+            compatible = "sony,imx219";
+            reg = <0x10>;
+            clocks = <&imx219_clk>;
+            clock-names = "xclk";
+            VANA-supply = <&imx219_vana>;   /* 2.8v */
+            VDIG-supply = <&imx219_vdig>;   /* 1.8v */
+            VDDL-supply = <&imx219_vddl>;   /* 1.2v */
+
+            imx219_clk: camera-clk {
+                compatible = "fixed-clock";
+                #clock-cells = <0>;
+                clock-frequency = <24000000>;
+            };
+
+            port {
+                imx219_0: endpoint {
+                    remote-endpoint = <&csi1_ep>;
+                    clock-lanes = <0>;
+                    data-lanes = <1 2>;
+                    clock-noncontinuous;
+                };
+            };
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index ffa3371bc750..f7b6c24ec081 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15350,6 +15350,14 @@ S:	Maintained
  F:	drivers/media/i2c/imx214.c
  F:	Documentation/devicetree/bindings/media/i2c/sony,imx214.txt
+SONY IMX219 SENSOR DRIVER
+M:	Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>

Is Dave aware of this? :-)

Yes, he is :)

But I forgot to add him to Cc this time. My bad..

Thanks,
Andrey

+L:	linux-media@xxxxxxxxxxxxxxx
+T:	git git://linuxtv.org/media_tree.git
+S:	Maintained
+F:	drivers/media/i2c/imx219.c
+F:	Documentation/devicetree/bindings/media/i2c/imx219.yaml
+
  SONY IMX258 SENSOR DRIVER
  M:	Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
  L:	linux-media@xxxxxxxxxxxxxxx




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux