2022-09-13 17:48:52

by Prabhakar

[permalink] [raw]
Subject: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema

From: Lad Prabhakar <[email protected]>

Convert the simple OV5645 Device Tree binding to json-schema.

Signed-off-by: Lad Prabhakar <[email protected]>
---
.../devicetree/bindings/media/i2c/ov5645.txt | 54 --------
.../bindings/media/i2c/ovti,ov5645.yaml | 119 ++++++++++++++++++
2 files changed, 119 insertions(+), 54 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5645.txt
create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
deleted file mode 100644
index 72ad992f77be..000000000000
--- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
+++ /dev/null
@@ -1,54 +0,0 @@
-* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
-
-The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image sensor with
-an active array size of 2592H x 1944V. It is programmable through a serial I2C
-interface.
-
-Required Properties:
-- compatible: Value should be "ovti,ov5645".
-- 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 PWDNB which is physically active low.
-- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
- the hardware pin RESETB.
-- vdddo-supply: Chip digital IO regulator.
-- vdda-supply: Chip analog regulator.
-- vddd-supply: Chip digital core regulator.
-
-The device node must contain one 'port' child node for its digital output
-video port, in accordance with the video interface bindings defined in
-Documentation/devicetree/bindings/media/video-interfaces.txt.
-
-Example:
-
- &i2c1 {
- ...
-
- ov5645: ov5645@3c {
- compatible = "ovti,ov5645";
- reg = <0x3c>;
-
- enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
- reset-gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
- pinctrl-names = "default";
- pinctrl-0 = <&camera_rear_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 {
- ov5645_ep: endpoint {
- clock-lanes = <1>;
- data-lanes = <0 2>;
- remote-endpoint = <&csi0_ep>;
- };
- };
- };
- };
diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
new file mode 100644
index 000000000000..7f407c988f87
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
@@ -0,0 +1,119 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ovti,ov5645.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: OmniVision OV5645 Image Sensor Device Tree Bindings
+
+maintainers:
+ - Lad Prabhakar <[email protected]>
+
+allOf:
+ - $ref: /schemas/media/video-interface-devices.yaml#
+
+properties:
+ compatible:
+ const: ovti,ov5645
+
+ 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.5 volts
+
+ vdddo-supply:
+ description: Digital I/O voltage supply, 1.8 volts
+
+ enable-gpios:
+ maxItems: 1
+ description:
+ Reference to the GPIO connected to the PWDNB pin, if any.
+
+ reset-gpios:
+ maxItems: 1
+ description:
+ Reference to the GPIO connected to the RESETB pin, if any.
+
+ 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:
+ const: 0
+
+ bus-type:
+ const: 4
+
+ data-lanes:
+ minItems: 1
+ maxItems: 2
+ items:
+ enum: [1, 2]
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - clock-frequency
+ - vdda-supply
+ - vddd-supply
+ - vdddo-supply
+ - enable-gpios
+ - reset-gpios
+ - port
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ camera@3c {
+ compatible = "ovti,ov5645";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_ov5645>;
+ reg = <0x3c>;
+ clocks = <&clks 1>;
+ clock-names = "xclk";
+ clock-frequency = <24000000>;
+ vdddo-supply = <&ov5645_vdddo_1v8>; /* 1.8v */
+ vdda-supply = <&ov5645_vdda_2v8>; /* 2.8v */
+ vddd-supply = <&ov5645_vddd_1v5>; /* 1.5v */
+ enable-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
+
+ port {
+ ov5645_ep: endpoint {
+ remote-endpoint = <&csi0_ep>;
+ clock-lanes = <0>;
+ data-lanes = <1 2>;
+ };
+ };
+ };
+ };
+...
--
2.25.1


2022-09-14 14:28:50

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema

On Tue, Sep 13, 2022 at 05:02:24PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <[email protected]>
>
> Convert the simple OV5645 Device Tree binding to json-schema.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> .../devicetree/bindings/media/i2c/ov5645.txt | 54 --------
> .../bindings/media/i2c/ovti,ov5645.yaml | 119 ++++++++++++++++++
> 2 files changed, 119 insertions(+), 54 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5645.txt
> create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> deleted file mode 100644
> index 72ad992f77be..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> +++ /dev/null
> @@ -1,54 +0,0 @@
> -* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
> -
> -The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image sensor with
> -an active array size of 2592H x 1944V. It is programmable through a serial I2C
> -interface.
> -
> -Required Properties:
> -- compatible: Value should be "ovti,ov5645".
> -- 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 PWDNB which is physically active low.
> -- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> - the hardware pin RESETB.
> -- vdddo-supply: Chip digital IO regulator.
> -- vdda-supply: Chip analog regulator.
> -- vddd-supply: Chip digital core regulator.
> -
> -The device node must contain one 'port' child node for its digital output
> -video port, in accordance with the video interface bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Example:
> -
> - &i2c1 {
> - ...
> -
> - ov5645: ov5645@3c {
> - compatible = "ovti,ov5645";
> - reg = <0x3c>;
> -
> - enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> - reset-gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
> - pinctrl-names = "default";
> - pinctrl-0 = <&camera_rear_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 {
> - ov5645_ep: endpoint {
> - clock-lanes = <1>;
> - data-lanes = <0 2>;
> - remote-endpoint = <&csi0_ep>;
> - };
> - };
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> new file mode 100644
> index 000000000000..7f407c988f87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5645.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: OmniVision OV5645 Image Sensor Device Tree Bindings
> +
> +maintainers:
> + - Lad Prabhakar <[email protected]>
> +
> +allOf:
> + - $ref: /schemas/media/video-interface-devices.yaml#

What are you using from here?

(Answer below)

> +
> +properties:
> + compatible:
> + const: ovti,ov5645
> +
> + 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.5 volts
> +
> + vdddo-supply:
> + description: Digital I/O voltage supply, 1.8 volts
> +
> + enable-gpios:
> + maxItems: 1
> + description:
> + Reference to the GPIO connected to the PWDNB pin, if any.
> +
> + reset-gpios:
> + maxItems: 1
> + description:
> + Reference to the GPIO connected to the RESETB pin, if any.
> +
> + 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:
> + const: 0
> +
> + bus-type:
> + const: 4
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 2
> + items:
> + enum: [1, 2]
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - clock-frequency
> + - vdda-supply
> + - vddd-supply
> + - vdddo-supply
> + - enable-gpios
> + - reset-gpios
> + - port
> +
> +additionalProperties: false

You are not because this prevents it. Either remove the ref or use
unevaluatedProperties.

Rob

2022-09-14 22:00:35

by Tommaso Merciai

[permalink] [raw]
Subject: Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema

Hi Prabhakar,

On Tue, Sep 13, 2022 at 05:02:24PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <[email protected]>
>
> Convert the simple OV5645 Device Tree binding to json-schema.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> .../devicetree/bindings/media/i2c/ov5645.txt | 54 --------
> .../bindings/media/i2c/ovti,ov5645.yaml | 119 ++++++++++++++++++
> 2 files changed, 119 insertions(+), 54 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5645.txt
> create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> deleted file mode 100644
> index 72ad992f77be..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> +++ /dev/null
> @@ -1,54 +0,0 @@
> -* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
> -
> -The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image sensor with
> -an active array size of 2592H x 1944V. It is programmable through a serial I2C
> -interface.
> -
> -Required Properties:
> -- compatible: Value should be "ovti,ov5645".
> -- 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 PWDNB which is physically active low.
> -- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> - the hardware pin RESETB.
> -- vdddo-supply: Chip digital IO regulator.
> -- vdda-supply: Chip analog regulator.
> -- vddd-supply: Chip digital core regulator.
> -
> -The device node must contain one 'port' child node for its digital output
> -video port, in accordance with the video interface bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Example:
> -
> - &i2c1 {
> - ...
> -
> - ov5645: ov5645@3c {
> - compatible = "ovti,ov5645";
> - reg = <0x3c>;
> -
> - enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> - reset-gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
> - pinctrl-names = "default";
> - pinctrl-0 = <&camera_rear_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 {
> - ov5645_ep: endpoint {
> - clock-lanes = <1>;
> - data-lanes = <0 2>;
> - remote-endpoint = <&csi0_ep>;
> - };
> - };
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> new file mode 100644
> index 000000000000..7f407c988f87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5645.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: OmniVision OV5645 Image Sensor Device Tree Bindings
> +
> +maintainers:
> + - Lad Prabhakar <[email protected]>
> +
> +allOf:
> + - $ref: /schemas/media/video-interface-devices.yaml#
> +
> +properties:
> + compatible:
> + const: ovti,ov5645
> +
> + 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.5 volts
> +
> + vdddo-supply:
> + description: Digital I/O voltage supply, 1.8 volts
> +
> + enable-gpios:
> + maxItems: 1
> + description:
> + Reference to the GPIO connected to the PWDNB pin, if any.
> +
> + reset-gpios:
> + maxItems: 1
> + description:
> + Reference to the GPIO connected to the RESETB pin, if any.
> +
> + 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:
> + const: 0
> +
> + bus-type:
> + const: 4
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 2
> + items:
> + enum: [1, 2]
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - clock-frequency
> + - vdda-supply
> + - vddd-supply
> + - vdddo-supply
> + - enable-gpios
> + - reset-gpios
> + - port

I think we don't need all of these properties as required.
The only "really" required are:

- compatible
- reg
- clocks
- port

Regards,
Tommaso

> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + camera@3c {
> + compatible = "ovti,ov5645";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ov5645>;
> + reg = <0x3c>;
> + clocks = <&clks 1>;
> + clock-names = "xclk";
> + clock-frequency = <24000000>;
> + vdddo-supply = <&ov5645_vdddo_1v8>; /* 1.8v */
> + vdda-supply = <&ov5645_vdda_2v8>; /* 2.8v */
> + vddd-supply = <&ov5645_vddd_1v5>; /* 1.5v */
> + enable-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
> + reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
> +
> + port {
> + ov5645_ep: endpoint {
> + remote-endpoint = <&csi0_ep>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> + };
> + };
> + };
> + };
> +...
> --
> 2.25.1
>

--
Tommaso Merciai
Embedded Linux Engineer
[email protected]
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
[email protected]
http://www.amarulasolutions.com

2022-09-16 10:55:45

by Prabhakar

[permalink] [raw]
Subject: Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema

Hi Rob,

Thank you for the review.

On Wed, Sep 14, 2022 at 3:16 PM Rob Herring <[email protected]> wrote:
>
> On Tue, Sep 13, 2022 at 05:02:24PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > Convert the simple OV5645 Device Tree binding to json-schema.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > .../devicetree/bindings/media/i2c/ov5645.txt | 54 --------
> > .../bindings/media/i2c/ovti,ov5645.yaml | 119 ++++++++++++++++++
> > 2 files changed, 119 insertions(+), 54 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > deleted file mode 100644
> > index 72ad992f77be..000000000000
> > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > +++ /dev/null
> > @@ -1,54 +0,0 @@
> > -* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
> > -
> > -The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image sensor with
> > -an active array size of 2592H x 1944V. It is programmable through a serial I2C
> > -interface.
> > -
> > -Required Properties:
> > -- compatible: Value should be "ovti,ov5645".
> > -- 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 PWDNB which is physically active low.
> > -- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > - the hardware pin RESETB.
> > -- vdddo-supply: Chip digital IO regulator.
> > -- vdda-supply: Chip analog regulator.
> > -- vddd-supply: Chip digital core regulator.
> > -
> > -The device node must contain one 'port' child node for its digital output
> > -video port, in accordance with the video interface bindings defined in
> > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > -
> > -Example:
> > -
> > - &i2c1 {
> > - ...
> > -
> > - ov5645: ov5645@3c {
> > - compatible = "ovti,ov5645";
> > - reg = <0x3c>;
> > -
> > - enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> > - reset-gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
> > - pinctrl-names = "default";
> > - pinctrl-0 = <&camera_rear_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 {
> > - ov5645_ep: endpoint {
> > - clock-lanes = <1>;
> > - data-lanes = <0 2>;
> > - remote-endpoint = <&csi0_ep>;
> > - };
> > - };
> > - };
> > - };
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> > new file mode 100644
> > index 000000000000..7f407c988f87
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> > @@ -0,0 +1,119 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5645.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: OmniVision OV5645 Image Sensor Device Tree Bindings
> > +
> > +maintainers:
> > + - Lad Prabhakar <[email protected]>
> > +
> > +allOf:
> > + - $ref: /schemas/media/video-interface-devices.yaml#
>
> What are you using from here?
>
> (Answer below)
>
> > +
> > +properties:
<snip>
> > + - port
> > +
> > +additionalProperties: false
>
> You are not because this prevents it. Either remove the ref or use
> unevaluatedProperties.
>
I'll drop the ref while sending the v2.

Cheers,
Prabhakar

2022-09-16 11:00:04

by Prabhakar

[permalink] [raw]
Subject: Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema

Hi Tommaso,

Thank you for the review.

On Wed, Sep 14, 2022 at 10:41 PM Tommaso Merciai
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Tue, Sep 13, 2022 at 05:02:24PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > Convert the simple OV5645 Device Tree binding to json-schema.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > .../devicetree/bindings/media/i2c/ov5645.txt | 54 --------
> > .../bindings/media/i2c/ovti,ov5645.yaml | 119 ++++++++++++++++++
> > 2 files changed, 119 insertions(+), 54 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > deleted file mode 100644
> > index 72ad992f77be..000000000000
> > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > +++ /dev/null
> > @@ -1,54 +0,0 @@
> > -* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
> > -
> > -The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image sensor with
> > -an active array size of 2592H x 1944V. It is programmable through a serial I2C
> > -interface.
> > -
> > -Required Properties:
> > -- compatible: Value should be "ovti,ov5645".
> > -- 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 PWDNB which is physically active low.
> > -- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > - the hardware pin RESETB.
> > -- vdddo-supply: Chip digital IO regulator.
> > -- vdda-supply: Chip analog regulator.
> > -- vddd-supply: Chip digital core regulator.
> > -
> > -The device node must contain one 'port' child node for its digital output
> > -video port, in accordance with the video interface bindings defined in
> > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > -
> > -Example:
> > -
> > - &i2c1 {
> > - ...
> > -
> > - ov5645: ov5645@3c {
> > - compatible = "ovti,ov5645";
> > - reg = <0x3c>;
> > -
> > - enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> > - reset-gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
> > - pinctrl-names = "default";
> > - pinctrl-0 = <&camera_rear_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 {
> > - ov5645_ep: endpoint {
> > - clock-lanes = <1>;
> > - data-lanes = <0 2>;
> > - remote-endpoint = <&csi0_ep>;
> > - };
> > - };
> > - };
> > - };
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> > new file mode 100644
> > index 000000000000..7f407c988f87
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> > @@ -0,0 +1,119 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5645.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: OmniVision OV5645 Image Sensor Device Tree Bindings
> > +
> > +maintainers:
> > + - Lad Prabhakar <[email protected]>
> > +
> > +allOf:
> > + - $ref: /schemas/media/video-interface-devices.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: ovti,ov5645
> > +
> > + 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.5 volts
> > +
> > + vdddo-supply:
> > + description: Digital I/O voltage supply, 1.8 volts
> > +
> > + enable-gpios:
> > + maxItems: 1
> > + description:
> > + Reference to the GPIO connected to the PWDNB pin, if any.
> > +
> > + reset-gpios:
> > + maxItems: 1
> > + description:
> > + Reference to the GPIO connected to the RESETB pin, if any.
> > +
> > + 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:
> > + const: 0
> > +
> > + bus-type:
> > + const: 4
> > +
> > + data-lanes:
> > + minItems: 1
> > + maxItems: 2
> > + items:
> > + enum: [1, 2]
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - clocks
> > + - clock-names
> > + - clock-frequency
> > + - vdda-supply
> > + - vddd-supply
> > + - vdddo-supply
> > + - enable-gpios
> > + - reset-gpios
> > + - port
>
> I think we don't need all of these properties as required.
> The only "really" required are:
>
> - compatible
> - reg
> - clocks
> - port
>
Agreed, it's just the driver requirement hence the previous bindings
had marked them as required.

Cheers,
Prabhakar

2022-09-16 11:25:49

by Prabhakar

[permalink] [raw]
Subject: Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema

On Fri, Sep 16, 2022 at 11:28 AM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 16/09/2022 11:24, Lad, Prabhakar wrote:
> >>> +required:
> >>> + - compatible
> >>> + - reg
> >>> + - clocks
> >>> + - clock-names
> >>> + - clock-frequency
> >>> + - vdda-supply
> >>> + - vddd-supply
> >>> + - vdddo-supply
> >>> + - enable-gpios
> >>> + - reset-gpios
> >>> + - port
> >>
> >> I think we don't need all of these properties as required.
> >> The only "really" required are:
> >>
> >> - compatible
> >> - reg
> >> - clocks
> >> - port
> >>
> > Agreed, it's just the driver requirement hence the previous bindings
> > had marked them as required.
>
> Please mention the change from pure conversion (with reason) in the
> commit msg.
>
Sure will do.

Cheers,
Prabhakar

2022-09-16 11:25:58

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema

On 16/09/2022 11:24, Lad, Prabhakar wrote:
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - clocks
>>> + - clock-names
>>> + - clock-frequency
>>> + - vdda-supply
>>> + - vddd-supply
>>> + - vdddo-supply
>>> + - enable-gpios
>>> + - reset-gpios
>>> + - port
>>
>> I think we don't need all of these properties as required.
>> The only "really" required are:
>>
>> - compatible
>> - reg
>> - clocks
>> - port
>>
> Agreed, it's just the driver requirement hence the previous bindings
> had marked them as required.

Please mention the change from pure conversion (with reason) in the
commit msg.


Best regards,
Krzysztof

2022-09-16 12:23:51

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema

Hi Prabhakar,

On Tue, Sep 13, 2022 at 05:02:24PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <[email protected]>
>
> Convert the simple OV5645 Device Tree binding to json-schema.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> .../devicetree/bindings/media/i2c/ov5645.txt | 54 --------
> .../bindings/media/i2c/ovti,ov5645.yaml | 119 ++++++++++++++++++
> 2 files changed, 119 insertions(+), 54 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5645.txt
> create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> deleted file mode 100644
> index 72ad992f77be..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> +++ /dev/null
> @@ -1,54 +0,0 @@
> -* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
> -
> -The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image sensor with
> -an active array size of 2592H x 1944V. It is programmable through a serial I2C
> -interface.
> -
> -Required Properties:
> -- compatible: Value should be "ovti,ov5645".
> -- 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 PWDNB which is physically active low.
> -- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> - the hardware pin RESETB.
> -- vdddo-supply: Chip digital IO regulator.
> -- vdda-supply: Chip analog regulator.
> -- vddd-supply: Chip digital core regulator.
> -
> -The device node must contain one 'port' child node for its digital output
> -video port, in accordance with the video interface bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Example:
> -
> - &i2c1 {
> - ...
> -
> - ov5645: ov5645@3c {
> - compatible = "ovti,ov5645";
> - reg = <0x3c>;
> -
> - enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> - reset-gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
> - pinctrl-names = "default";
> - pinctrl-0 = <&camera_rear_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 {
> - ov5645_ep: endpoint {
> - clock-lanes = <1>;
> - data-lanes = <0 2>;
> - remote-endpoint = <&csi0_ep>;
> - };
> - };
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> new file mode 100644
> index 000000000000..7f407c988f87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5645.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: OmniVision OV5645 Image Sensor Device Tree Bindings
> +
> +maintainers:
> + - Lad Prabhakar <[email protected]>
> +
> +allOf:
> + - $ref: /schemas/media/video-interface-devices.yaml#
> +
> +properties:
> + compatible:
> + const: ovti,ov5645
> +
> + 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.5 volts
> +
> + vdddo-supply:
> + description: Digital I/O voltage supply, 1.8 volts
> +
> + enable-gpios:
> + maxItems: 1
> + description:
> + Reference to the GPIO connected to the PWDNB pin, if any.
> +
> + reset-gpios:
> + maxItems: 1
> + description:
> + Reference to the GPIO connected to the RESETB pin, if any.
> +
> + 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:
> + const: 0
> +
> + bus-type:
> + const: 4

Please drop the two, they provide no information to the driver that already
knows this. (Unless of course, the driver supports other bus types these
bindings do not document. Some OV sensors do.) clock-lanes needs to go in
any case, and data-lanes are dependent on bus-type being 4 I presume.

bus-type is also not present in the example.

> +
> + data-lanes:
> + minItems: 1
> + maxItems: 2
> + items:
> + enum: [1, 2]
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - clock-frequency
> + - vdda-supply
> + - vddd-supply
> + - vdddo-supply
> + - enable-gpios
> + - reset-gpios
> + - port
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + camera@3c {
> + compatible = "ovti,ov5645";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ov5645>;
> + reg = <0x3c>;
> + clocks = <&clks 1>;
> + clock-names = "xclk";
> + clock-frequency = <24000000>;
> + vdddo-supply = <&ov5645_vdddo_1v8>; /* 1.8v */
> + vdda-supply = <&ov5645_vdda_2v8>; /* 2.8v */
> + vddd-supply = <&ov5645_vddd_1v5>; /* 1.5v */
> + enable-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
> + reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
> +
> + port {
> + ov5645_ep: endpoint {
> + remote-endpoint = <&csi0_ep>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> + };
> + };
> + };
> + };
> +...

--
Kind regards,

Sakari Ailus

2022-09-16 12:58:55

by Prabhakar

[permalink] [raw]
Subject: Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema

Hi Sakari,

Thank you for the review.

On Fri, Sep 16, 2022 at 12:52 PM Sakari Ailus
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Tue, Sep 13, 2022 at 05:02:24PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > Convert the simple OV5645 Device Tree binding to json-schema.
<snip>
> > + $ref: /schemas/media/video-interfaces.yaml#
> > + unevaluatedProperties: false
> > +
> > + properties:
> > + clock-lanes:
> > + const: 0
> > +
> > + bus-type:
> > + const: 4
>
> Please drop the two, they provide no information to the driver that already
> knows this. (Unless of course, the driver supports other bus types these
> bindings do not document. Some OV sensors do.) clock-lanes needs to go in
> any case, and data-lanes are dependent on bus-type being 4 I presume.
>
Agreed, I will drop the above two and send a v2.

> bus-type is also not present in the example.
>
Ahh, I missed that.

Cheers,
Prabhakar