diff mbox series

[v4] media: dt-bindings: Add OV5642

Message ID 20230913192811.1946423-1-festevam@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v4] media: dt-bindings: Add OV5642 | expand

Commit Message

Fabio Estevam Sept. 13, 2023, 7:28 p.m. UTC
From: Fabio Estevam <festevam@denx.de>

As explained in the description text from trivial-devices.yaml:

"This is a list of trivial I2C and SPI devices that have simple device tree
bindings, consisting only of a compatible field, an address and possibly an
interrupt line."

A camera device does not fall into this category as it needs other
properties such as regulators, reset and powerdown GPIOs, clocks,
media endpoint.

Remove the OV5642 entry from trivial-devices.yaml and add its own
ovti,ov5642.yaml.

Signed-off-by: Fabio Estevam <festevam@denx.de>
---
Changes since v3:
- Make propeties conditional on parallel verus MIPI CSI-2. (Sakari)
- Remove clock-names as there is a single clock.
- Pass bus-type.

 .../bindings/media/i2c/ovti,ov5642.yaml       | 141 ++++++++++++++++++
 .../devicetree/bindings/trivial-devices.yaml  |   2 -
 2 files changed, 141 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5642.yaml

Comments

Rob Herring Sept. 14, 2023, 2:59 p.m. UTC | #1
On Wed, Sep 13, 2023 at 04:28:11PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@denx.de>
> 
> As explained in the description text from trivial-devices.yaml:
> 
> "This is a list of trivial I2C and SPI devices that have simple device tree
> bindings, consisting only of a compatible field, an address and possibly an
> interrupt line."
> 
> A camera device does not fall into this category as it needs other
> properties such as regulators, reset and powerdown GPIOs, clocks,
> media endpoint.
> 
> Remove the OV5642 entry from trivial-devices.yaml and add its own
> ovti,ov5642.yaml.

Many of these Omnivision sensors have the same supplies and same GPIO 
connections, so why can't they share the binding? There are some 
differences I've seen that boil down to just what the driver decided to 
require or not, but that shouldn't really vary. See commit 44ade291b77c 
("media: dt-bindings: Merge OV5695 into OV5693 binding") for example.

In any case, the binding looks fine and any merging can be done after.

Reviewed-by: Rob Herring <robh@kernel.org>

> 
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---
> Changes since v3:
> - Make propeties conditional on parallel verus MIPI CSI-2. (Sakari)
> - Remove clock-names as there is a single clock.
> - Pass bus-type.
> 
>  .../bindings/media/i2c/ovti,ov5642.yaml       | 141 ++++++++++++++++++
>  .../devicetree/bindings/trivial-devices.yaml  |   2 -
>  2 files changed, 141 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5642.yaml
Sakari Ailus Sept. 15, 2023, 11:04 a.m. UTC | #2
Hi Rob,

On Thu, Sep 14, 2023 at 09:59:35AM -0500, Rob Herring wrote:
> On Wed, Sep 13, 2023 at 04:28:11PM -0300, Fabio Estevam wrote:
> > From: Fabio Estevam <festevam@denx.de>
> > 
> > As explained in the description text from trivial-devices.yaml:
> > 
> > "This is a list of trivial I2C and SPI devices that have simple device tree
> > bindings, consisting only of a compatible field, an address and possibly an
> > interrupt line."
> > 
> > A camera device does not fall into this category as it needs other
> > properties such as regulators, reset and powerdown GPIOs, clocks,
> > media endpoint.
> > 
> > Remove the OV5642 entry from trivial-devices.yaml and add its own
> > ovti,ov5642.yaml.
> 
> Many of these Omnivision sensors have the same supplies and same GPIO 
> connections, so why can't they share the binding? There are some 
> differences I've seen that boil down to just what the driver decided to 
> require or not, but that shouldn't really vary. See commit 44ade291b77c 
> ("media: dt-bindings: Merge OV5695 into OV5693 binding") for example.
> 
> In any case, the binding looks fine and any merging can be done after.

The sensors differ from interface capabilities (one or more of parallel,
CSI-2 D-PHY and CSI-2 C-PHY), number of lanes / data lines etc. For
historical reasons, many have different defaults for vsync and hsync
polarities, for instance.

I suppose some of these would reasonably be taken into account, if not in a
single file, in a couple of files. CSI-2 connected sensors are the most
common these days.

The device for which this patch added the bindings appears to be 15 years
old.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5642.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5642.yaml
new file mode 100644
index 000000000000..01f8b2b3fd17
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5642.yaml
@@ -0,0 +1,141 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ovti,ov5642.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: OmniVision OV5642 Image Sensor
+
+maintainers:
+  - Fabio Estevam <festevam@gmail.com>
+
+allOf:
+  - $ref: /schemas/media/video-interface-devices.yaml#
+
+properties:
+  compatible:
+    const: ovti,ov5642
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    description: XCLK Input Clock
+
+  AVDD-supply:
+    description: Analog voltage supply, 2.8V.
+
+  DVDD-supply:
+    description: Digital core voltage supply, 1.5V.
+
+  DOVDD-supply:
+    description: Digital I/O voltage supply, 1.8V.
+
+  powerdown-gpios:
+    maxItems: 1
+    description: Reference to the GPIO connected to the powerdown pin, if any.
+
+  reset-gpios:
+    maxItems: 1
+    description: Reference to the GPIO connected to the reset pin, if any.
+
+  port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    description: |
+      Video output port.
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          bus-type:
+            enum: [5, 6]
+
+          bus-width:
+            enum: [8, 10]
+            default: 10
+
+          data-shift:
+            enum: [0, 2]
+            default: 0
+
+          hsync-active:
+            enum: [0, 1]
+            default: 1
+
+          vsync-active:
+            enum: [0, 1]
+            default: 1
+
+          pclk-sample:
+            enum: [0, 1]
+            default: 1
+
+        allOf:
+          - if:
+              properties:
+                bus-type:
+                  const: 6
+            then:
+              properties:
+                hsync-active: false
+                vsync-active: false
+
+          - if:
+              properties:
+                bus-width:
+                  const: 10
+            then:
+              properties:
+                data-shift:
+                  const: 0
+
+        required:
+          - bus-type
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+      #include <dt-bindings/gpio/gpio.h>
+      #include <dt-bindings/media/video-interfaces.h>
+
+      i2c {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          camera@3c {
+              compatible = "ovti,ov5642";
+              reg = <0x3c>;
+              pinctrl-names = "default";
+              pinctrl-0 = <&pinctrl_ov5642>;
+              clocks = <&clk_ext_camera>;
+              DOVDD-supply = <&vgen4_reg>;
+              AVDD-supply = <&vgen3_reg>;
+              DVDD-supply = <&vgen2_reg>;
+              powerdown-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
+              reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
+
+              port {
+                  ov5642_to_parallel: endpoint {
+                      bus-type = <MEDIA_BUS_TYPE_PARALLEL>;
+                      remote-endpoint = <&parallel_from_ov5642>;
+                      bus-width = <8>;
+                      data-shift = <2>; /* lines 9:2 are used */
+                      hsync-active = <0>;
+                      vsync-active = <0>;
+                      pclk-sample = <1>;
+                  };
+              };
+          };
+      };
diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index cd58179ae337..c665018f55b9 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -311,8 +311,6 @@  properties:
           - nuvoton,w83773g
             # OKI ML86V7667 video decoder
           - oki,ml86v7667
-            # OV5642: Color CMOS QSXGA (5-megapixel) Image Sensor with OmniBSI and Embedded TrueFocus
-          - ovti,ov5642
             # 48-Lane, 12-Port PCI Express Gen 2 (5.0 GT/s) Switch
           - plx,pex8648
             # Pulsedlight LIDAR range-finding sensor