diff mbox series

media: dt-bindings: Convert Omnivision OV7251 to DT schema

Message ID 20230707210646.868758-1-robh@kernel.org
State Superseded
Headers show
Series media: dt-bindings: Convert Omnivision OV7251 to DT schema | expand

Commit Message

Rob Herring July 7, 2023, 9:06 p.m. UTC
Convert the OmniVision OV7251 Image Sensor binding to DT schema format.

vddd-supply was listed as required, but the example and actual user
don't have it. Also, the data brief says it has an internal regulator,
so perhaps it is truly optional.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/media/i2c/ov7251.txt  |  52 ---------
 .../bindings/media/i2c/ovti,ov7251.yaml       | 105 ++++++++++++++++++
 2 files changed, 105 insertions(+), 52 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov7251.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml

Comments

Conor Dooley July 10, 2023, 6:20 p.m. UTC | #1
On Mon, Jul 10, 2023 at 12:03:38PM -0600, Rob Herring wrote:
> On Mon, Jul 10, 2023 at 11:57 AM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Fri, Jul 07, 2023 at 03:06:46PM -0600, Rob Herring wrote:
> > > Convert the OmniVision OV7251 Image Sensor binding to DT schema format.
> > >
> > > vddd-supply was listed as required, but the example and actual user
> > > don't have it. Also, the data brief says it has an internal regulator,
> > > so perhaps it is truly optional.
> >
> > ov7251.c:
> >         ov7251->core_regulator = devm_regulator_get(dev, "vddd");
> >         if (IS_ERR(ov7251->core_regulator)) {
> >                 dev_err(dev, "cannot get core regulator\n");
> >                 return PTR_ERR(ov7251->core_regulator);
> >         }
> >
> > Looks like the driver's probe function disagrees?
> 
> Doesn't the regulator framework return a dummy regulator if missing?

Huh, I think I misunderstood how _regulator_get() worked. Apologies for
the noise, I thought it was returning an error when it used a dummy.

> > I was going to ask how it worked, but the one user has
> > status = "disabled"...
> 
> Saw that too, but figured there's some other include with that
> overridden. We should really add a built .dts output target to avoid
> trying to manually walk includes.

Probably gonna sound like an eejit, but the user is in a dts - can an
include overwrite that status?
Sakari Ailus July 31, 2023, 11:29 a.m. UTC | #2
Hi Rob,

On Fri, Jul 07, 2023 at 03:06:46PM -0600, Rob Herring wrote:
> Convert the OmniVision OV7251 Image Sensor binding to DT schema format.
> 
> vddd-supply was listed as required, but the example and actual user
> don't have it. Also, the data brief says it has an internal regulator,
> so perhaps it is truly optional.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>

Same comment on this than the ov5693/5 bindings: we should have
link-frequency here. Although I guess it could be added by a separate
patch (I can write one if you like).

The driver already checks for this.

> ---
>  .../devicetree/bindings/media/i2c/ov7251.txt  |  52 ---------
>  .../bindings/media/i2c/ovti,ov7251.yaml       | 105 ++++++++++++++++++
>  2 files changed, 105 insertions(+), 52 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov7251.txt
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov7251.txt b/Documentation/devicetree/bindings/media/i2c/ov7251.txt
> deleted file mode 100644
> index 8281151f7493..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ov7251.txt
> +++ /dev/null
> @@ -1,52 +0,0 @@
> -* Omnivision 1/7.5-Inch B&W VGA CMOS Digital Image Sensor
> -
> -The Omnivision OV7251 is a 1/7.5-Inch CMOS active pixel digital image sensor
> -with an active array size of 640H x 480V. It is programmable through a serial
> -I2C interface.
> -
> -Required Properties:
> -- compatible: Value should be "ovti,ov7251".
> -- clocks: Reference to the xclk clock.
> -- clock-names: Should be "xclk".
> -- clock-frequency: Frequency of the xclk clock.
> -- enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> -  to the hardware pin XSHUTDOWN which is physically active low.
> -- vdddo-supply: Chip digital IO regulator.
> -- vdda-supply: Chip analog regulator.
> -- vddd-supply: Chip digital core regulator.
> -
> -The device node shall contain one 'port' child node with a single 'endpoint'
> -subnode for its digital output video port, in accordance with the video
> -interface bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Example:
> -
> -	&i2c1 {
> -		...
> -
> -		ov7251: camera-sensor@60 {
> -			compatible = "ovti,ov7251";
> -			reg = <0x60>;
> -
> -			enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> -			pinctrl-names = "default";
> -			pinctrl-0 = <&camera_bw_default>;
> -
> -			clocks = <&clks 200>;
> -			clock-names = "xclk";
> -			clock-frequency = <24000000>;
> -
> -			vdddo-supply = <&camera_dovdd_1v8>;
> -			vdda-supply = <&camera_avdd_2v8>;
> -			vddd-supply = <&camera_dvdd_1v2>;
> -
> -			port {
> -				ov7251_ep: endpoint {
> -					clock-lanes = <1>;
> -					data-lanes = <0>;
> -					remote-endpoint = <&csi0_ep>;
> -				};
> -			};
> -		};
> -	};
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml
> new file mode 100644
> index 000000000000..8d939858d950
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov7251.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: OmniVision OV7251 Image Sensor
> +
> +description:
> +  The Omnivision OV7251 is a 1/7.5-Inch CMOS active pixel digital image sensor
> +  with an active array size of 640H x 480V. It is programmable through a serial
> +  I2C interface.
> +
> +maintainers:
> +  - Todor Tomov <todor.too@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: ovti,ov7251
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    description: XCLK Input Clock
> +
> +  clock-names:
> +    const: xclk
> +
> +  clock-frequency:
> +    description: Frequency of the xclk clock in Hz.
> +
> +  vdda-supply:
> +    description: Analog voltage supply, 2.8 volts
> +
> +  vddd-supply:
> +    description: Digital core voltage supply, 1.2 volts
> +
> +  vdddo-supply:
> +    description: Digital I/O voltage supply, 1.8 volts
> +
> +  enable-gpios:
> +    maxItems: 1
> +    description:
> +      Reference to the GPIO connected to the XSHUTDOWN pin, if any. Polarity
> +      is GPIO_ACTIVE_HIGH.
> +
> +  port:
> +    description: Digital Output Port
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    additionalProperties: false
> +
> +    properties:
> +      endpoint:
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          clock-lanes:
> +            maximum: 1
> +
> +          data-lanes:
> +            maxItems: 1
> +
> +        required:
> +          - data-lanes
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - vdddo-supply
> +  - vdda-supply
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        camera@3c {
> +            compatible = "ovti,ov7251";
> +            reg = <0x3c>;
> +            clocks = <&clks 1>;
> +            clock-frequency = <24000000>;
> +            vdddo-supply = <&ov7251_vdddo_1v8>;
> +            vdda-supply = <&ov7251_vdda_2v8>;
> +            vddd-supply = <&ov7251_vddd_1v5>;
> +            enable-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
> +
> +            port {
> +                ov7251_ep: endpoint {
> +                    remote-endpoint = <&csi0_ep>;
> +                    clock-lanes = <1>;
> +                    data-lanes = <0>;
> +                };
> +            };
> +        };
> +    };
> +...
> -- 
> 2.40.1
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/i2c/ov7251.txt b/Documentation/devicetree/bindings/media/i2c/ov7251.txt
deleted file mode 100644
index 8281151f7493..000000000000
--- a/Documentation/devicetree/bindings/media/i2c/ov7251.txt
+++ /dev/null
@@ -1,52 +0,0 @@ 
-* Omnivision 1/7.5-Inch B&W VGA CMOS Digital Image Sensor
-
-The Omnivision OV7251 is a 1/7.5-Inch CMOS active pixel digital image sensor
-with an active array size of 640H x 480V. It is programmable through a serial
-I2C interface.
-
-Required Properties:
-- compatible: Value should be "ovti,ov7251".
-- clocks: Reference to the xclk clock.
-- clock-names: Should be "xclk".
-- clock-frequency: Frequency of the xclk clock.
-- enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
-  to the hardware pin XSHUTDOWN which is physically active low.
-- vdddo-supply: Chip digital IO regulator.
-- vdda-supply: Chip analog regulator.
-- vddd-supply: Chip digital core regulator.
-
-The device node shall contain one 'port' child node with a single 'endpoint'
-subnode for its digital output video port, in accordance with the video
-interface bindings defined in
-Documentation/devicetree/bindings/media/video-interfaces.txt.
-
-Example:
-
-	&i2c1 {
-		...
-
-		ov7251: camera-sensor@60 {
-			compatible = "ovti,ov7251";
-			reg = <0x60>;
-
-			enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
-			pinctrl-names = "default";
-			pinctrl-0 = <&camera_bw_default>;
-
-			clocks = <&clks 200>;
-			clock-names = "xclk";
-			clock-frequency = <24000000>;
-
-			vdddo-supply = <&camera_dovdd_1v8>;
-			vdda-supply = <&camera_avdd_2v8>;
-			vddd-supply = <&camera_dvdd_1v2>;
-
-			port {
-				ov7251_ep: endpoint {
-					clock-lanes = <1>;
-					data-lanes = <0>;
-					remote-endpoint = <&csi0_ep>;
-				};
-			};
-		};
-	};
diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml
new file mode 100644
index 000000000000..8d939858d950
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml
@@ -0,0 +1,105 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ovti,ov7251.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: OmniVision OV7251 Image Sensor
+
+description:
+  The Omnivision OV7251 is a 1/7.5-Inch CMOS active pixel digital image sensor
+  with an active array size of 640H x 480V. It is programmable through a serial
+  I2C interface.
+
+maintainers:
+  - Todor Tomov <todor.too@gmail.com>
+
+properties:
+  compatible:
+    const: ovti,ov7251
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    description: XCLK Input Clock
+
+  clock-names:
+    const: xclk
+
+  clock-frequency:
+    description: Frequency of the xclk clock in Hz.
+
+  vdda-supply:
+    description: Analog voltage supply, 2.8 volts
+
+  vddd-supply:
+    description: Digital core voltage supply, 1.2 volts
+
+  vdddo-supply:
+    description: Digital I/O voltage supply, 1.8 volts
+
+  enable-gpios:
+    maxItems: 1
+    description:
+      Reference to the GPIO connected to the XSHUTDOWN pin, if any. Polarity
+      is GPIO_ACTIVE_HIGH.
+
+  port:
+    description: Digital Output Port
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    additionalProperties: false
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          clock-lanes:
+            maximum: 1
+
+          data-lanes:
+            maxItems: 1
+
+        required:
+          - data-lanes
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - vdddo-supply
+  - vdda-supply
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        camera@3c {
+            compatible = "ovti,ov7251";
+            reg = <0x3c>;
+            clocks = <&clks 1>;
+            clock-frequency = <24000000>;
+            vdddo-supply = <&ov7251_vdddo_1v8>;
+            vdda-supply = <&ov7251_vdda_2v8>;
+            vddd-supply = <&ov7251_vddd_1v5>;
+            enable-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
+
+            port {
+                ov7251_ep: endpoint {
+                    remote-endpoint = <&csi0_ep>;
+                    clock-lanes = <1>;
+                    data-lanes = <0>;
+                };
+            };
+        };
+    };
+...