diff mbox series

[v2,1/2] media: dt-bindings: media: i2c: document OV4689 DT bindings

Message ID 20220911200147.375198-2-mike.rudenko@gmail.com
State Accepted
Commit 6cbd33e75ec829a2c95ba1f4403c804cf1d85c06
Headers show
Series Add Omnivision OV4689 image sensor driver | expand

Commit Message

Mikhail Rudenko Sept. 11, 2022, 8:01 p.m. UTC 
Add device-tree binding documentation for OV4689 image sensor driver,
and the relevant MAINTAINERS entries.

Signed-off-by: Mikhail Rudenko <mike.rudenko@gmail.com>
---
 .../bindings/media/i2c/ovti,ov4689.yaml       | 141 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 148 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml

Comments

Mikhail Rudenko Sept. 15, 2022, 8:11 p.m. UTC | #1 
Hi Tommaso,

On 2022-09-13 at 16:05 +02, Tommaso Merciai <tommaso.merciai@amarulasolutions.com> wrote:
> Hi Mikhail,
>
> On Sun, Sep 11, 2022 at 11:01:34PM +0300, Mikhail Rudenko wrote:
>> Add device-tree binding documentation for OV4689 image sensor driver,
>> and the relevant MAINTAINERS entries.
>>
>> Signed-off-by: Mikhail Rudenko <mike.rudenko@gmail.com>
>> ---
>>  .../bindings/media/i2c/ovti,ov4689.yaml       | 141 ++++++++++++++++++
>>  MAINTAINERS                                   |   7 +
>>  2 files changed, 148 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
>> new file mode 100644
>> index 000000000000..376330b5572a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
>> @@ -0,0 +1,141 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov4689.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Omnivision OV4689 CMOS
>> +
>> +maintainers:
>> +  - Mikhail Rudenko <mike.rudenko@gmail.com>
>> +
>> +description: |
>> +  The Omnivision OV4689 is a high performance, 1/3-inch, 4 megapixel
>> +  image sensor. Ihis chip supports high frame rate speeds up to 90 fps
>> +  at 2688x1520 resolution. It is programmable through an I2C
>> +  interface, and sensor output is sent via 1/2/4 lane MIPI CSI-2
>> +  connection.
>> +
>> +allOf:
>> +  - $ref: /schemas/media/video-interface-devices.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: ovti,ov4689
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    description:
>> +      External clock (XVCLK) for the sensor, 6-64 MHz
>> +    maxItems: 1
>> +
>> +  clock-names: true
>> +
>> +  dovdd-supply:
>> +    description:
>> +      Digital I/O voltage supply, 1.7-3.0 V
>> +
>> +  avdd-supply:
>> +    description:
>> +      Analog voltage supply, 2.6-3.0 V
>> +
>> +  dvdd-supply:
>> +    description:
>> +      Digital core voltage supply, 1.1-1.3 V
>> +
>> +  powerdown-gpios:
>> +    maxItems: 1
>> +    description:
>> +      GPIO connected to the powerdown pin (active low)
>> +
>> +  reset-gpios:
>> +    maxItems: 1
>> +    description:
>> +      GPIO connected to the reset pin (active low)
>> +
>> +  orientation: true
>> +
>> +  rotation: true
>> +
>> +  port:
>> +    $ref: /schemas/graph.yaml#/$defs/port-base
>> +    additionalProperties: false
>> +    description:
>> +      Output port node, single endpoint describing the CSI-2 transmitter
>> +
>> +    properties:
>> +      endpoint:
>> +        $ref: /schemas/media/video-interfaces.yaml#
>> +        unevaluatedProperties: false
>> +
>> +        properties:
>> +          data-lanes:
>> +            oneOf:
>> +              - items:
>> +                  - const: 1
>> +                  - const: 2
>> +                  - const: 3
>> +                  - const: 4
>> +              - items:
>> +                  - const: 1
>> +                  - const: 2
>> +              - items:
>> +                  - const: 1
>> +          link-frequencies: true
>> +
>> +        required:
>> +          - data-lanes
>> +          - link-frequencies
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +  - dovdd-supply
>> +  - avdd-supply
>> +  - dvdd-supply
>> +  - powerdown-gpios
>> +  - reset-gpios
>> +  - port
>
> I think we don't need all of these entries as required.
> The only let me say "really" required are:
>
> - compatible
> - reg
> - clocks
> - port

Thanks for the review! I agree that the driver may be modified to work
without powerdown and reset gpios and they are not required for sensor
operation. On contrary, supplies are obviously required. Of course, linux
provides dummy regulators if supplies are missing from device tree, but
I though the intention was to document hardware, not implementation
details. What do think of this?

> Regards,
> Tommaso

--
Best regards,
Mikhail Rudenko
Mikhail Rudenko Sept. 16, 2022, 1:42 p.m. UTC | #2 
On 2022-09-16 at 15:15 +02, Tommaso Merciai <tommaso.merciai@amarulasolutions.com> wrote:
> Hi Mikhail,
>
> On Thu, Sep 15, 2022 at 11:11:57PM +0300, Mikhail Rudenko wrote:
>>
>> Hi Tommaso,
>>
>> On 2022-09-13 at 16:05 +02, Tommaso Merciai <tommaso.merciai@amarulasolutions.com> wrote:
>> > Hi Mikhail,
>> >
>> > On Sun, Sep 11, 2022 at 11:01:34PM +0300, Mikhail Rudenko wrote:
>> >> Add device-tree binding documentation for OV4689 image sensor driver,
>> >> and the relevant MAINTAINERS entries.
>> >>
>> >> Signed-off-by: Mikhail Rudenko <mike.rudenko@gmail.com>
>> >> ---
>> >>  .../bindings/media/i2c/ovti,ov4689.yaml       | 141 ++++++++++++++++++
>> >>  MAINTAINERS                                   |   7 +
>> >>  2 files changed, 148 insertions(+)
>> >>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
>> >> new file mode 100644
>> >> index 000000000000..376330b5572a
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
>> >> @@ -0,0 +1,141 @@
>> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> >> +%YAML 1.2
>> >> +---
>> >> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov4689.yaml#
>> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> >> +
>> >> +title: Omnivision OV4689 CMOS
>> >> +
>> >> +maintainers:
>> >> +  - Mikhail Rudenko <mike.rudenko@gmail.com>
>> >> +
>> >> +description: |
>> >> +  The Omnivision OV4689 is a high performance, 1/3-inch, 4 megapixel
>> >> +  image sensor. Ihis chip supports high frame rate speeds up to 90 fps
>> >> +  at 2688x1520 resolution. It is programmable through an I2C
>> >> +  interface, and sensor output is sent via 1/2/4 lane MIPI CSI-2
>> >> +  connection.
>> >> +
>> >> +allOf:
>> >> +  - $ref: /schemas/media/video-interface-devices.yaml#
>> >> +
>> >> +properties:
>> >> +  compatible:
>> >> +    const: ovti,ov4689
>> >> +
>> >> +  reg:
>> >> +    maxItems: 1
>> >> +
>> >> +  clocks:
>> >> +    description:
>> >> +      External clock (XVCLK) for the sensor, 6-64 MHz
>> >> +    maxItems: 1
>> >> +
>> >> +  clock-names: true
>> >> +
>> >> +  dovdd-supply:
>> >> +    description:
>> >> +      Digital I/O voltage supply, 1.7-3.0 V
>> >> +
>> >> +  avdd-supply:
>> >> +    description:
>> >> +      Analog voltage supply, 2.6-3.0 V
>> >> +
>> >> +  dvdd-supply:
>> >> +    description:
>> >> +      Digital core voltage supply, 1.1-1.3 V
>> >> +
>> >> +  powerdown-gpios:
>> >> +    maxItems: 1
>> >> +    description:
>> >> +      GPIO connected to the powerdown pin (active low)
>> >> +
>> >> +  reset-gpios:
>> >> +    maxItems: 1
>> >> +    description:
>> >> +      GPIO connected to the reset pin (active low)
>> >> +
>> >> +  orientation: true
>> >> +
>> >> +  rotation: true
>> >> +
>> >> +  port:
>> >> +    $ref: /schemas/graph.yaml#/$defs/port-base
>> >> +    additionalProperties: false
>> >> +    description:
>> >> +      Output port node, single endpoint describing the CSI-2 transmitter
>> >> +
>> >> +    properties:
>> >> +      endpoint:
>> >> +        $ref: /schemas/media/video-interfaces.yaml#
>> >> +        unevaluatedProperties: false
>> >> +
>> >> +        properties:
>> >> +          data-lanes:
>> >> +            oneOf:
>> >> +              - items:
>> >> +                  - const: 1
>> >> +                  - const: 2
>> >> +                  - const: 3
>> >> +                  - const: 4
>> >> +              - items:
>> >> +                  - const: 1
>> >> +                  - const: 2
>> >> +              - items:
>> >> +                  - const: 1
>> >> +          link-frequencies: true
>> >> +
>> >> +        required:
>> >> +          - data-lanes
>> >> +          - link-frequencies
>> >> +
>> >> +required:
>> >> +  - compatible
>> >> +  - reg
>> >> +  - clocks
>> >> +  - clock-names
>> >> +  - dovdd-supply
>> >> +  - avdd-supply
>> >> +  - dvdd-supply
>> >> +  - powerdown-gpios
>> >> +  - reset-gpios
>> >> +  - port
>> >
>> > I think we don't need all of these entries as required.
>> > The only let me say "really" required are:
>> >
>> > - compatible
>> > - reg
>> > - clocks
>> > - port
>>
>> Thanks for the review! I agree that the driver may be modified to work
>> without powerdown and reset gpios and they are not required for sensor
>> operation. On contrary, supplies are obviously required. Of course, linux
>> provides dummy regulators if supplies are missing from device tree, but
>> I though the intention was to document hardware, not implementation
>> details. What do think of this?
>
> We have already discuss on this on the following thread sometimes ago :)
>
> https://www.patchwork.linux-fancy.com/project/linux-fancy/patch/20220630134835.592521-6-tommaso.merciai@amarulasolutions.com/
>
> Take a look and let me know.

Okay, if there already is a consensus regarding this matter, I'll make
the regulators optional in v3.

--
Best regards,
Mikhail Rudenko
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
new file mode 100644
index 000000000000..376330b5572a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
@@ -0,0 +1,141 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ovti,ov4689.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Omnivision OV4689 CMOS
+
+maintainers:
+  - Mikhail Rudenko <mike.rudenko@gmail.com>
+
+description: |
+  The Omnivision OV4689 is a high performance, 1/3-inch, 4 megapixel
+  image sensor. Ihis chip supports high frame rate speeds up to 90 fps
+  at 2688x1520 resolution. It is programmable through an I2C
+  interface, and sensor output is sent via 1/2/4 lane MIPI CSI-2
+  connection.
+
+allOf:
+  - $ref: /schemas/media/video-interface-devices.yaml#
+
+properties:
+  compatible:
+    const: ovti,ov4689
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    description:
+      External clock (XVCLK) for the sensor, 6-64 MHz
+    maxItems: 1
+
+  clock-names: true
+
+  dovdd-supply:
+    description:
+      Digital I/O voltage supply, 1.7-3.0 V
+
+  avdd-supply:
+    description:
+      Analog voltage supply, 2.6-3.0 V
+
+  dvdd-supply:
+    description:
+      Digital core voltage supply, 1.1-1.3 V
+
+  powerdown-gpios:
+    maxItems: 1
+    description:
+      GPIO connected to the powerdown pin (active low)
+
+  reset-gpios:
+    maxItems: 1
+    description:
+      GPIO connected to the reset pin (active low)
+
+  orientation: true
+
+  rotation: true
+
+  port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    additionalProperties: false
+    description:
+      Output port node, single endpoint describing the CSI-2 transmitter
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          data-lanes:
+            oneOf:
+              - items:
+                  - const: 1
+                  - const: 2
+                  - const: 3
+                  - const: 4
+              - items:
+                  - const: 1
+                  - const: 2
+              - items:
+                  - const: 1
+          link-frequencies: true
+
+        required:
+          - data-lanes
+          - link-frequencies
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - dovdd-supply
+  - avdd-supply
+  - dvdd-supply
+  - powerdown-gpios
+  - reset-gpios
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ov4689: camera@36 {
+            compatible = "ovti,ov4689";
+            reg = <0x36>;
+
+            clocks = <&ov4689_clk>;
+            clock-names = "xvclk";
+
+            avdd-supply = <&ov4689_avdd>;
+            dovdd-supply = <&ov4689_dovdd>;
+            dvdd-supply = <&ov4689_dvdd>;
+
+            powerdown-gpios = <&pio 107 GPIO_ACTIVE_LOW>;
+            reset-gpios = <&pio 109 GPIO_ACTIVE_LOW>;
+
+            orientation = <2>;
+            rotation = <0>;
+
+            port {
+                wcam_out: endpoint {
+                    remote-endpoint = <&mipi_in_wcam>;
+                    data-lanes = <1 2 3 4>;
+                    link-frequencies = /bits/ 64 <500000000>;
+                };
+            };
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index f468864fd268..63c4844f26e6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14523,6 +14523,13 @@  S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
 F:	drivers/media/i2c/ov2740.c
 
+OMNIVISION OV4689 SENSOR DRIVER
+M:	Mikhail Rudenko <mike.rudenko@gmail.com>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+T:	git git://linuxtv.org/media_tree.git
+F:	Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
+
 OMNIVISION OV5640 SENSOR DRIVER
 M:	Steve Longerbeam <slongerbeam@gmail.com>
 L:	linux-media@vger.kernel.org