linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] dt-bindings: media: ov772x: Convert to json-schama
@ 2020-08-18 12:20 Jacopo Mondi
  2020-08-18 12:20 ` [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema Jacopo Mondi
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Jacopo Mondi @ 2020-08-18 12:20 UTC (permalink / raw)
  To: robh+dt, devicetree, linux-media, Lad, Prabhakar
  Cc: Jacopo Mondi, mchehab, sakari.ailus, hverkuil-cisco,
	laurent.pinchart, linux-renesas-soc

Just adding a missing signed-off-by to 2/3

Re-pasting here the cover letter for reference

--------------------------------------------------------------------------------
Convert to json-schema the ov772x bindings and add support for endpoint
properties to pave the way for  Prabhakar's support for BT.656 in the driver.

Prabhakar: could you confirm the properties defaults are sane according to your
setup?
--------------------------------------------------------------------------------

Jacopo Mondi (3):
  dt-bindings: media: ov772x: Convert to json-schema
  dt-bindings: media: ov772x: Make bus-type mandatory
  dt-bindings: media: ov772x: Document endpoint props

 .../devicetree/bindings/media/i2c/ov772x.txt  |  40 ------
 .../devicetree/bindings/media/i2c/ov772x.yaml | 134 ++++++++++++++++++
 MAINTAINERS                                   |   2 +-
 3 files changed, 135 insertions(+), 41 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.yaml

--
2.27.0


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema
  2020-08-18 12:20 [PATCH 0/3] dt-bindings: media: ov772x: Convert to json-schama Jacopo Mondi
@ 2020-08-18 12:20 ` Jacopo Mondi
  2020-08-19 13:52   ` Laurent Pinchart
                     ` (2 more replies)
  2020-08-18 12:20 ` [PATCH 2/3] dt-bindings: media: ov772x: Make bus-type mandatory Jacopo Mondi
  2020-08-18 12:20 ` [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props Jacopo Mondi
  2 siblings, 3 replies; 23+ messages in thread
From: Jacopo Mondi @ 2020-08-18 12:20 UTC (permalink / raw)
  To: robh+dt, devicetree, linux-media, Lad, Prabhakar
  Cc: Jacopo Mondi, mchehab, sakari.ailus, hverkuil-cisco,
	laurent.pinchart, linux-renesas-soc

Convert the ov772x binding document to json-schema and update
the MAINTAINERS file accordingly.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 .../devicetree/bindings/media/i2c/ov772x.txt  | 40 ---------
 .../devicetree/bindings/media/i2c/ov772x.yaml | 84 +++++++++++++++++++
 MAINTAINERS                                   |  2 +-
 3 files changed, 85 insertions(+), 41 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
deleted file mode 100644
index 0b3ede5b8e6a..000000000000
--- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
+++ /dev/null
@@ -1,40 +0,0 @@
-* Omnivision OV7720/OV7725 CMOS sensor
-
-The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
-such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
-support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
-
-Required Properties:
-- compatible: shall be one of
-	"ovti,ov7720"
-	"ovti,ov7725"
-- clocks: reference to the xclk input clock.
-
-Optional Properties:
-- reset-gpios: reference to the GPIO connected to the RSTB pin which is
-  active low, if any.
-- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
-  active high, if any.
-
-The device node shall contain one 'port' child node with one child '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:
-
-&i2c0 {
-	ov772x: camera@21 {
-		compatible = "ovti,ov7725";
-		reg = <0x21>;
-		reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
-		powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
-		clocks = <&xclk>;
-
-		port {
-			ov772x_0: endpoint {
-				remote-endpoint = <&vcap1_in0>;
-			};
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
new file mode 100644
index 000000000000..2b84fefeb4aa
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ov772x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title:  Omnivision OV7720/OV7725 CMOS sensor
+
+maintainers:
+  - Jacopo Mondi <jacopo@jmondi.org>
+
+description: -|
+  The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
+  such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
+  support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
+
+properties:
+  compatible:
+    enum:
+      - ovti,ov7720
+      - ovti,ov7725
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  reset-gpios:
+    description: -|
+      Reference to the GPIO connected to the RSTB pin which is active low.
+    maxItems: 1
+
+  powerdown-gpios:
+    description: -|
+      Reference to the GPIO connected to the PWDN pin which is active high.
+    maxItems: 1
+
+  port:
+    type: object
+    description: |
+      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.
+
+    properties:
+      endpoint:
+        type: object
+        properties:
+          remote-endpoint:
+            description: A phandle to the bus receiver's endpoint node.
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - reset-gpios
+  - powerdown-gpios
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        ov772x: camera@21 {
+            compatible = "ovti,ov7725";
+            reg = <0x21>;
+            reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
+            powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
+            clocks = <&xclk>;
+
+            port {
+                ov772x_0: endpoint {
+                    remote-endpoint = <&vcap1_in0>;
+                };
+            };
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index d1a6173d3b64..d0a20214eaaf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12666,7 +12666,7 @@ M:	Jacopo Mondi <jacopo@jmondi.org>
 L:	linux-media@vger.kernel.org
 S:	Odd fixes
 T:	git git://linuxtv.org/media_tree.git
-F:	Documentation/devicetree/bindings/media/i2c/ov772x.txt
+F:	Documentation/devicetree/bindings/media/i2c/ov772x.yaml
 F:	drivers/media/i2c/ov772x.c
 F:	include/media/i2c/ov772x.h
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/3] dt-bindings: media: ov772x: Make bus-type mandatory
  2020-08-18 12:20 [PATCH 0/3] dt-bindings: media: ov772x: Convert to json-schama Jacopo Mondi
  2020-08-18 12:20 ` [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema Jacopo Mondi
@ 2020-08-18 12:20 ` Jacopo Mondi
  2020-08-19 13:52   ` Laurent Pinchart
                     ` (2 more replies)
  2020-08-18 12:20 ` [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props Jacopo Mondi
  2 siblings, 3 replies; 23+ messages in thread
From: Jacopo Mondi @ 2020-08-18 12:20 UTC (permalink / raw)
  To: robh+dt, devicetree, linux-media, Lad, Prabhakar
  Cc: Jacopo Mondi, mchehab, sakari.ailus, hverkuil-cisco,
	laurent.pinchart, linux-renesas-soc

In order to establish required properties based on the selected
bus type, make the 'bus-type' property mandatory.

Binary compatibility with existing DTB is kept as the driver does not
enforce the property to be present, and shall fall-back to default
parallel bus configuration, which was the only supported bus type, if
the property is not specified.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 Documentation/devicetree/bindings/media/i2c/ov772x.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
index 2b84fefeb4aa..75dad40f70cc 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
@@ -47,9 +47,15 @@ properties:
       endpoint:
         type: object
         properties:
+          bus-type:
+            enum: [5, 6]
+
           remote-endpoint:
             description: A phandle to the bus receiver's endpoint node.
 
+        required:
+          - bus-type
+
     additionalProperties: false
 
 required:
@@ -75,6 +81,7 @@ examples:
 
             port {
                 ov772x_0: endpoint {
+                    bus-type = <5>;
                     remote-endpoint = <&vcap1_in0>;
                 };
             };
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props
  2020-08-18 12:20 [PATCH 0/3] dt-bindings: media: ov772x: Convert to json-schama Jacopo Mondi
  2020-08-18 12:20 ` [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema Jacopo Mondi
  2020-08-18 12:20 ` [PATCH 2/3] dt-bindings: media: ov772x: Make bus-type mandatory Jacopo Mondi
@ 2020-08-18 12:20 ` Jacopo Mondi
  2020-08-19 13:54   ` Laurent Pinchart
  2 siblings, 1 reply; 23+ messages in thread
From: Jacopo Mondi @ 2020-08-18 12:20 UTC (permalink / raw)
  To: robh+dt, devicetree, linux-media, Lad, Prabhakar
  Cc: Jacopo Mondi, mchehab, sakari.ailus, hverkuil-cisco,
	laurent.pinchart, linux-renesas-soc

Document endpoint properties for the parallel bus type and
add them to the example.

Specify a few constraints:
- If the bus type is BT.656 no hsync or vsycn polarities can be
  specified.
- If the bus width is 10 bits, not data-shift can be applied.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 .../devicetree/bindings/media/i2c/ov772x.yaml | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
index 75dad40f70cc..3fad5dffd19a 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
@@ -50,9 +50,47 @@ 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
+
           remote-endpoint:
             description: A phandle to the bus receiver's endpoint node.

+        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

@@ -82,6 +120,11 @@ examples:
             port {
                 ov772x_0: endpoint {
                     bus-type = <5>;
+                    vsync-active = <0>;
+                    hsync-active = <0>;
+                    pclk-sample = <0>;
+                    bus-width = <8>;
+                    data-shift = <0>;
                     remote-endpoint = <&vcap1_in0>;
                 };
             };
--
2.27.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema
  2020-08-18 12:20 ` [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema Jacopo Mondi
@ 2020-08-19 13:52   ` Laurent Pinchart
  2020-08-24  8:32     ` Jacopo Mondi
  2020-08-21 11:26   ` Lad, Prabhakar
  2020-08-25 21:03   ` Rob Herring
  2 siblings, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2020-08-19 13:52 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: robh+dt, devicetree, linux-media, Lad, Prabhakar, mchehab,
	sakari.ailus, hverkuil-cisco, linux-renesas-soc

Hi Jacopo,

Thank you for the patch.

On Tue, Aug 18, 2020 at 02:20:10PM +0200, Jacopo Mondi wrote:
> Convert the ov772x binding document to json-schema and update
> the MAINTAINERS file accordingly.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  .../devicetree/bindings/media/i2c/ov772x.txt  | 40 ---------
>  .../devicetree/bindings/media/i2c/ov772x.yaml | 84 +++++++++++++++++++

Could yuo rename this to ovti,ov772x.yaml ?

>  MAINTAINERS                                   |  2 +-
>  3 files changed, 85 insertions(+), 41 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> deleted file mode 100644
> index 0b3ede5b8e6a..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> +++ /dev/null
> @@ -1,40 +0,0 @@
> -* Omnivision OV7720/OV7725 CMOS sensor
> -
> -The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> -such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> -support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> -
> -Required Properties:
> -- compatible: shall be one of
> -	"ovti,ov7720"
> -	"ovti,ov7725"
> -- clocks: reference to the xclk input clock.
> -
> -Optional Properties:
> -- reset-gpios: reference to the GPIO connected to the RSTB pin which is
> -  active low, if any.
> -- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
> -  active high, if any.
> -
> -The device node shall contain one 'port' child node with one child '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:
> -
> -&i2c0 {
> -	ov772x: camera@21 {
> -		compatible = "ovti,ov7725";
> -		reg = <0x21>;
> -		reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> -		powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> -		clocks = <&xclk>;
> -
> -		port {
> -			ov772x_0: endpoint {
> -				remote-endpoint = <&vcap1_in0>;
> -			};
> -		};
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> new file mode 100644
> index 000000000000..2b84fefeb4aa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> @@ -0,0 +1,84 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ov772x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title:  Omnivision OV7720/OV7725 CMOS sensor
> +
> +maintainers:
> +  - Jacopo Mondi <jacopo@jmondi.org>
> +
> +description: -|
> +  The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> +  such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> +  support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ovti,ov7720
> +      - ovti,ov7725
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    description: -|
> +      Reference to the GPIO connected to the RSTB pin which is active low.
> +    maxItems: 1
> +
> +  powerdown-gpios:
> +    description: -|
> +      Reference to the GPIO connected to the PWDN pin which is active high.
> +    maxItems: 1
> +
> +  port:
> +    type: object
> +    description: |
> +      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.

You can simply write

      Digital input video port. See ../video-interfaces.txt.

> +
> +    properties:
> +      endpoint:
> +        type: object
> +        properties:
> +          remote-endpoint:
> +            description: A phandle to the bus receiver's endpoint node.

           required:
	     - remote-endpoint

           additionalProperties: false

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - reset-gpios
> +  - powerdown-gpios
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        ov772x: camera@21 {
> +            compatible = "ovti,ov7725";
> +            reg = <0x21>;
> +            reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> +            powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> +            clocks = <&xclk>;
> +
> +            port {
> +                ov772x_0: endpoint {
> +                    remote-endpoint = <&vcap1_in0>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d1a6173d3b64..d0a20214eaaf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12666,7 +12666,7 @@ M:	Jacopo Mondi <jacopo@jmondi.org>
>  L:	linux-media@vger.kernel.org
>  S:	Odd fixes
>  T:	git git://linuxtv.org/media_tree.git
> -F:	Documentation/devicetree/bindings/media/i2c/ov772x.txt
> +F:	Documentation/devicetree/bindings/media/i2c/ov772x.yaml
>  F:	drivers/media/i2c/ov772x.c
>  F:	include/media/i2c/ov772x.h
>  

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/3] dt-bindings: media: ov772x: Make bus-type mandatory
  2020-08-18 12:20 ` [PATCH 2/3] dt-bindings: media: ov772x: Make bus-type mandatory Jacopo Mondi
@ 2020-08-19 13:52   ` Laurent Pinchart
  2020-08-21 11:29   ` Lad, Prabhakar
  2020-08-25 21:06   ` Rob Herring
  2 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2020-08-19 13:52 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: robh+dt, devicetree, linux-media, Lad, Prabhakar, mchehab,
	sakari.ailus, hverkuil-cisco, linux-renesas-soc

Hi Jacopo,

Thank you for the patch.

On Tue, Aug 18, 2020 at 02:20:11PM +0200, Jacopo Mondi wrote:
> In order to establish required properties based on the selected
> bus type, make the 'bus-type' property mandatory.
> 
> Binary compatibility with existing DTB is kept as the driver does not
> enforce the property to be present, and shall fall-back to default
> parallel bus configuration, which was the only supported bus type, if
> the property is not specified.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  Documentation/devicetree/bindings/media/i2c/ov772x.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> index 2b84fefeb4aa..75dad40f70cc 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> @@ -47,9 +47,15 @@ properties:
>        endpoint:
>          type: object
>          properties:
> +          bus-type:
> +            enum: [5, 6]
> +
>            remote-endpoint:
>              description: A phandle to the bus receiver's endpoint node.
>  
> +        required:
> +          - bus-type
> +
>      additionalProperties: false
>  
>  required:
> @@ -75,6 +81,7 @@ examples:
>  
>              port {
>                  ov772x_0: endpoint {
> +                    bus-type = <5>;
>                      remote-endpoint = <&vcap1_in0>;
>                  };
>              };

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props
  2020-08-18 12:20 ` [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props Jacopo Mondi
@ 2020-08-19 13:54   ` Laurent Pinchart
  2020-08-21 11:37     ` Lad, Prabhakar
  0 siblings, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2020-08-19 13:54 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: robh+dt, devicetree, linux-media, Lad, Prabhakar, mchehab,
	sakari.ailus, hverkuil-cisco, linux-renesas-soc

Hi Jacopo,

Thank you for the patch.

On Tue, Aug 18, 2020 at 02:20:12PM +0200, Jacopo Mondi wrote:
> Document endpoint properties for the parallel bus type and
> add them to the example.
> 
> Specify a few constraints:
> - If the bus type is BT.656 no hsync or vsycn polarities can be
>   specified.
> - If the bus width is 10 bits, not data-shift can be applied.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  .../devicetree/bindings/media/i2c/ov772x.yaml | 43 +++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> index 75dad40f70cc..3fad5dffd19a 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> @@ -50,9 +50,47 @@ 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
> +
>            remote-endpoint:
>              description: A phandle to the bus receiver's endpoint node.
> 
> +        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

I'd add a blank line here.

>          required:
>            - bus-type

Should some of the properties be required ? Possibly conditioned on
bus-type ?

> 
> @@ -82,6 +120,11 @@ examples:
>              port {
>                  ov772x_0: endpoint {
>                      bus-type = <5>;
> +                    vsync-active = <0>;
> +                    hsync-active = <0>;
> +                    pclk-sample = <0>;
> +                    bus-width = <8>;
> +                    data-shift = <0>;
>                      remote-endpoint = <&vcap1_in0>;
>                  };
>              };

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema
  2020-08-18 12:20 ` [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema Jacopo Mondi
  2020-08-19 13:52   ` Laurent Pinchart
@ 2020-08-21 11:26   ` Lad, Prabhakar
  2020-08-25 21:03   ` Rob Herring
  2 siblings, 0 replies; 23+ messages in thread
From: Lad, Prabhakar @ 2020-08-21 11:26 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-media, Mauro Carvalho Chehab, Sakari Ailus, Hans Verkuil,
	Laurent Pinchart, Linux-Renesas

Hi Jacopo,

Thank you for the patch.

On Tue, Aug 18, 2020 at 1:16 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
>
> Convert the ov772x binding document to json-schema and update
> the MAINTAINERS file accordingly.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  .../devicetree/bindings/media/i2c/ov772x.txt  | 40 ---------
>  .../devicetree/bindings/media/i2c/ov772x.yaml | 84 +++++++++++++++++++
>  MAINTAINERS                                   |  2 +-
>  3 files changed, 85 insertions(+), 41 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.yaml
>

Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabbhakar

> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> deleted file mode 100644
> index 0b3ede5b8e6a..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> +++ /dev/null
> @@ -1,40 +0,0 @@
> -* Omnivision OV7720/OV7725 CMOS sensor
> -
> -The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> -such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> -support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> -
> -Required Properties:
> -- compatible: shall be one of
> -       "ovti,ov7720"
> -       "ovti,ov7725"
> -- clocks: reference to the xclk input clock.
> -
> -Optional Properties:
> -- reset-gpios: reference to the GPIO connected to the RSTB pin which is
> -  active low, if any.
> -- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
> -  active high, if any.
> -
> -The device node shall contain one 'port' child node with one child '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:
> -
> -&i2c0 {
> -       ov772x: camera@21 {
> -               compatible = "ovti,ov7725";
> -               reg = <0x21>;
> -               reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> -               powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> -               clocks = <&xclk>;
> -
> -               port {
> -                       ov772x_0: endpoint {
> -                               remote-endpoint = <&vcap1_in0>;
> -                       };
> -               };
> -       };
> -};
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> new file mode 100644
> index 000000000000..2b84fefeb4aa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> @@ -0,0 +1,84 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ov772x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title:  Omnivision OV7720/OV7725 CMOS sensor
> +
> +maintainers:
> +  - Jacopo Mondi <jacopo@jmondi.org>
> +
> +description: -|
> +  The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> +  such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> +  support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ovti,ov7720
> +      - ovti,ov7725
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    description: -|
> +      Reference to the GPIO connected to the RSTB pin which is active low.
> +    maxItems: 1
> +
> +  powerdown-gpios:
> +    description: -|
> +      Reference to the GPIO connected to the PWDN pin which is active high.
> +    maxItems: 1
> +
> +  port:
> +    type: object
> +    description: |
> +      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.
> +
> +    properties:
> +      endpoint:
> +        type: object
> +        properties:
> +          remote-endpoint:
> +            description: A phandle to the bus receiver's endpoint node.
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - reset-gpios
> +  - powerdown-gpios
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        ov772x: camera@21 {
> +            compatible = "ovti,ov7725";
> +            reg = <0x21>;
> +            reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> +            powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> +            clocks = <&xclk>;
> +
> +            port {
> +                ov772x_0: endpoint {
> +                    remote-endpoint = <&vcap1_in0>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d1a6173d3b64..d0a20214eaaf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12666,7 +12666,7 @@ M:      Jacopo Mondi <jacopo@jmondi.org>
>  L:     linux-media@vger.kernel.org
>  S:     Odd fixes
>  T:     git git://linuxtv.org/media_tree.git
> -F:     Documentation/devicetree/bindings/media/i2c/ov772x.txt
> +F:     Documentation/devicetree/bindings/media/i2c/ov772x.yaml
>  F:     drivers/media/i2c/ov772x.c
>  F:     include/media/i2c/ov772x.h
>
> --
> 2.27.0
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/3] dt-bindings: media: ov772x: Make bus-type mandatory
  2020-08-18 12:20 ` [PATCH 2/3] dt-bindings: media: ov772x: Make bus-type mandatory Jacopo Mondi
  2020-08-19 13:52   ` Laurent Pinchart
@ 2020-08-21 11:29   ` Lad, Prabhakar
  2020-08-25 21:06   ` Rob Herring
  2 siblings, 0 replies; 23+ messages in thread
From: Lad, Prabhakar @ 2020-08-21 11:29 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-media, Mauro Carvalho Chehab, Sakari Ailus, Hans Verkuil,
	Laurent Pinchart, Linux-Renesas

Hi Jacopo,

Thank you for the patch.

On Tue, Aug 18, 2020 at 1:16 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
>
> In order to establish required properties based on the selected
> bus type, make the 'bus-type' property mandatory.
>
> Binary compatibility with existing DTB is kept as the driver does not
> enforce the property to be present, and shall fall-back to default
> parallel bus configuration, which was the only supported bus type, if
> the property is not specified.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/i2c/ov772x.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> index 2b84fefeb4aa..75dad40f70cc 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> @@ -47,9 +47,15 @@ properties:
>        endpoint:
>          type: object
>          properties:
> +          bus-type:
> +            enum: [5, 6]
> +
>            remote-endpoint:
>              description: A phandle to the bus receiver's endpoint node.
>
> +        required:
> +          - bus-type
> +
>      additionalProperties: false
>
>  required:
> @@ -75,6 +81,7 @@ examples:
>
>              port {
>                  ov772x_0: endpoint {
> +                    bus-type = <5>;
>                      remote-endpoint = <&vcap1_in0>;
>                  };
>              };
> --
> 2.27.0
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props
  2020-08-19 13:54   ` Laurent Pinchart
@ 2020-08-21 11:37     ` Lad, Prabhakar
  2020-08-22  1:35       ` Laurent Pinchart
  2020-08-24  8:35       ` Jacopo Mondi
  0 siblings, 2 replies; 23+ messages in thread
From: Lad, Prabhakar @ 2020-08-21 11:37 UTC (permalink / raw)
  To: Laurent Pinchart, Jacopo Mondi
  Cc: Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-media, Mauro Carvalho Chehab, Sakari Ailus, Hans Verkuil,
	Linux-Renesas

Hi Laurent and Jacopo

On Wed, Aug 19, 2020 at 2:54 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Aug 18, 2020 at 02:20:12PM +0200, Jacopo Mondi wrote:
> > Document endpoint properties for the parallel bus type and
> > add them to the example.
> >
> > Specify a few constraints:
> > - If the bus type is BT.656 no hsync or vsycn polarities can be
> >   specified.
> > - If the bus width is 10 bits, not data-shift can be applied.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  .../devicetree/bindings/media/i2c/ov772x.yaml | 43 +++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > index 75dad40f70cc..3fad5dffd19a 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > @@ -50,9 +50,47 @@ 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
> > +
> >            remote-endpoint:
> >              description: A phandle to the bus receiver's endpoint node.
> >
> > +        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
>
> I'd add a blank line here.
>
> >          required:
> >            - bus-type
>
> Should some of the properties be required ? Possibly conditioned on
> bus-type ?
>
Agreed, would be interesting to know how this can be handled (split
out bus-type and add required properties for each) ?

Cheers,
Prabhakar

> >
> > @@ -82,6 +120,11 @@ examples:
> >              port {
> >                  ov772x_0: endpoint {
> >                      bus-type = <5>;
> > +                    vsync-active = <0>;
> > +                    hsync-active = <0>;
> > +                    pclk-sample = <0>;
> > +                    bus-width = <8>;
> > +                    data-shift = <0>;
> >                      remote-endpoint = <&vcap1_in0>;
> >                  };
> >              };
>
> --
> Regards,
>
> Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props
  2020-08-21 11:37     ` Lad, Prabhakar
@ 2020-08-22  1:35       ` Laurent Pinchart
  2020-08-26  8:42         ` Lad, Prabhakar
  2020-08-24  8:35       ` Jacopo Mondi
  1 sibling, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2020-08-22  1:35 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Jacopo Mondi, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-media, Mauro Carvalho Chehab, Sakari Ailus, Hans Verkuil,
	Linux-Renesas

Hi Prabhakar,

On Fri, Aug 21, 2020 at 12:37:35PM +0100, Lad, Prabhakar wrote:
> On Wed, Aug 19, 2020 at 2:54 PM Laurent Pinchart wrote:
> > On Tue, Aug 18, 2020 at 02:20:12PM +0200, Jacopo Mondi wrote:
> > > Document endpoint properties for the parallel bus type and
> > > add them to the example.
> > >
> > > Specify a few constraints:
> > > - If the bus type is BT.656 no hsync or vsycn polarities can be
> > >   specified.
> > > - If the bus width is 10 bits, not data-shift can be applied.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  .../devicetree/bindings/media/i2c/ov772x.yaml | 43 +++++++++++++++++++
> > >  1 file changed, 43 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > index 75dad40f70cc..3fad5dffd19a 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > @@ -50,9 +50,47 @@ 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
> > > +
> > >            remote-endpoint:
> > >              description: A phandle to the bus receiver's endpoint node.
> > >
> > > +        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
> >
> > I'd add a blank line here.
> >
> > >          required:
> > >            - bus-type
> >
> > Should some of the properties be required ? Possibly conditioned on
> > bus-type ?
>
> Agreed, would be interesting to know how this can be handled (split
> out bus-type and add required properties for each) ?

We can add required: statements to the above if/then/else.

> > > @@ -82,6 +120,11 @@ examples:
> > >              port {
> > >                  ov772x_0: endpoint {
> > >                      bus-type = <5>;
> > > +                    vsync-active = <0>;
> > > +                    hsync-active = <0>;
> > > +                    pclk-sample = <0>;
> > > +                    bus-width = <8>;
> > > +                    data-shift = <0>;
> > >                      remote-endpoint = <&vcap1_in0>;
> > >                  };
> > >              };

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema
  2020-08-19 13:52   ` Laurent Pinchart
@ 2020-08-24  8:32     ` Jacopo Mondi
  2020-08-24 11:34       ` Laurent Pinchart
  0 siblings, 1 reply; 23+ messages in thread
From: Jacopo Mondi @ 2020-08-24  8:32 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Jacopo Mondi, robh+dt, devicetree, linux-media, Lad, Prabhakar,
	mchehab, sakari.ailus, hverkuil-cisco, linux-renesas-soc

Hi Laurent,

On Wed, Aug 19, 2020 at 04:52:04PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Aug 18, 2020 at 02:20:10PM +0200, Jacopo Mondi wrote:
> > Convert the ov772x binding document to json-schema and update
> > the MAINTAINERS file accordingly.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  .../devicetree/bindings/media/i2c/ov772x.txt  | 40 ---------
> >  .../devicetree/bindings/media/i2c/ov772x.yaml | 84 +++++++++++++++++++
>
> Could yuo rename this to ovti,ov772x.yaml ?
>
> >  MAINTAINERS                                   |  2 +-
> >  3 files changed, 85 insertions(+), 41 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > deleted file mode 100644
> > index 0b3ede5b8e6a..000000000000
> > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > +++ /dev/null
> > @@ -1,40 +0,0 @@
> > -* Omnivision OV7720/OV7725 CMOS sensor
> > -
> > -The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> > -such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> > -support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> > -
> > -Required Properties:
> > -- compatible: shall be one of
> > -	"ovti,ov7720"
> > -	"ovti,ov7725"
> > -- clocks: reference to the xclk input clock.
> > -
> > -Optional Properties:
> > -- reset-gpios: reference to the GPIO connected to the RSTB pin which is
> > -  active low, if any.
> > -- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
> > -  active high, if any.
> > -
> > -The device node shall contain one 'port' child node with one child '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:
> > -
> > -&i2c0 {
> > -	ov772x: camera@21 {
> > -		compatible = "ovti,ov7725";
> > -		reg = <0x21>;
> > -		reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> > -		powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> > -		clocks = <&xclk>;
> > -
> > -		port {
> > -			ov772x_0: endpoint {
> > -				remote-endpoint = <&vcap1_in0>;
> > -			};
> > -		};
> > -	};
> > -};
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > new file mode 100644
> > index 000000000000..2b84fefeb4aa
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > @@ -0,0 +1,84 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/ov772x.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title:  Omnivision OV7720/OV7725 CMOS sensor
> > +
> > +maintainers:
> > +  - Jacopo Mondi <jacopo@jmondi.org>
> > +
> > +description: -|
> > +  The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> > +  such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> > +  support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - ovti,ov7720
> > +      - ovti,ov7725
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  reset-gpios:
> > +    description: -|
> > +      Reference to the GPIO connected to the RSTB pin which is active low.
> > +    maxItems: 1
> > +
> > +  powerdown-gpios:
> > +    description: -|
> > +      Reference to the GPIO connected to the PWDN pin which is active high.
> > +    maxItems: 1
> > +
> > +  port:
> > +    type: object
> > +    description: |
> > +      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.
>
> You can simply write
>
>       Digital input video port. See ../video-interfaces.txt.
>
> > +
> > +    properties:
> > +      endpoint:
> > +        type: object
> > +        properties:
> > +          remote-endpoint:
> > +            description: A phandle to the bus receiver's endpoint node.
>
>            required:
> 	     - remote-endpoint
>
>            additionalProperties: false

I receveied a reply to you on previous json-schema conversion attempt
where you suggested to not set remote-endpoint as required, as we
allow empty ones to be later filled in in, maybe with an overlay.

Which Laurent should I listen to ? I tend to agree with the one that
said to drop remote-endpoint from the required properties list.

Thanks
  j
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > +
> > +    additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - reset-gpios
> > +  - powerdown-gpios
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    i2c0 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        ov772x: camera@21 {
> > +            compatible = "ovti,ov7725";
> > +            reg = <0x21>;
> > +            reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> > +            powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> > +            clocks = <&xclk>;
> > +
> > +            port {
> > +                ov772x_0: endpoint {
> > +                    remote-endpoint = <&vcap1_in0>;
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index d1a6173d3b64..d0a20214eaaf 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -12666,7 +12666,7 @@ M:	Jacopo Mondi <jacopo@jmondi.org>
> >  L:	linux-media@vger.kernel.org
> >  S:	Odd fixes
> >  T:	git git://linuxtv.org/media_tree.git
> > -F:	Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > +F:	Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> >  F:	drivers/media/i2c/ov772x.c
> >  F:	include/media/i2c/ov772x.h
> >
>
> --
> Regards,
>
> Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props
  2020-08-21 11:37     ` Lad, Prabhakar
  2020-08-22  1:35       ` Laurent Pinchart
@ 2020-08-24  8:35       ` Jacopo Mondi
  2020-08-26  8:47         ` Lad, Prabhakar
  1 sibling, 1 reply; 23+ messages in thread
From: Jacopo Mondi @ 2020-08-24  8:35 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Laurent Pinchart, Jacopo Mondi, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-media, Mauro Carvalho Chehab, Sakari Ailus, Hans Verkuil,
	Linux-Renesas

Hi Laurent, Prabhakar,

On Fri, Aug 21, 2020 at 12:37:35PM +0100, Lad, Prabhakar wrote:
> Hi Laurent and Jacopo
>
> On Wed, Aug 19, 2020 at 2:54 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Hi Jacopo,
> >
> > Thank you for the patch.
> >
> > On Tue, Aug 18, 2020 at 02:20:12PM +0200, Jacopo Mondi wrote:
> > > Document endpoint properties for the parallel bus type and
> > > add them to the example.
> > >
> > > Specify a few constraints:
> > > - If the bus type is BT.656 no hsync or vsycn polarities can be
> > >   specified.
> > > - If the bus width is 10 bits, not data-shift can be applied.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  .../devicetree/bindings/media/i2c/ov772x.yaml | 43 +++++++++++++++++++
> > >  1 file changed, 43 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > index 75dad40f70cc..3fad5dffd19a 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > @@ -50,9 +50,47 @@ 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
> > > +
> > >            remote-endpoint:
> > >              description: A phandle to the bus receiver's endpoint node.
> > >
> > > +        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
> >
> > I'd add a blank line here.
> >
> > >          required:
> > >            - bus-type
> >
> > Should some of the properties be required ? Possibly conditioned on
> > bus-type ?
> >

I am not sure. They all have defaults, as reported here and as
supported by the driver. There's nothing -strictly- required, as long
as the here reported defaults are correct.

> Agreed, would be interesting to know how this can be handled (split
> out bus-type and add required properties for each) ?
>

That already happens with

+          - if:
+              properties:
+                bus-type:
+                  const: 6
+            then:
+                properties:
+                  hsync-active: false
+                  vsync-active: false

And could be expanded, if we want any of these to be required.

Thanks
  j

> Cheers,
> Prabhakar
>
> > >
> > > @@ -82,6 +120,11 @@ examples:
> > >              port {
> > >                  ov772x_0: endpoint {
> > >                      bus-type = <5>;
> > > +                    vsync-active = <0>;
> > > +                    hsync-active = <0>;
> > > +                    pclk-sample = <0>;
> > > +                    bus-width = <8>;
> > > +                    data-shift = <0>;
> > >                      remote-endpoint = <&vcap1_in0>;
> > >                  };
> > >              };
> >
> > --
> > Regards,
> >
> > Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema
  2020-08-24  8:32     ` Jacopo Mondi
@ 2020-08-24 11:34       ` Laurent Pinchart
  2020-08-24 12:15         ` Jacopo Mondi
  0 siblings, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2020-08-24 11:34 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: Jacopo Mondi, robh+dt, devicetree, linux-media, Lad, Prabhakar,
	mchehab, sakari.ailus, hverkuil-cisco, linux-renesas-soc

Hi Jacopo,

On Mon, Aug 24, 2020 at 10:32:11AM +0200, Jacopo Mondi wrote:
> On Wed, Aug 19, 2020 at 04:52:04PM +0300, Laurent Pinchart wrote:
> > On Tue, Aug 18, 2020 at 02:20:10PM +0200, Jacopo Mondi wrote:
> > > Convert the ov772x binding document to json-schema and update
> > > the MAINTAINERS file accordingly.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  .../devicetree/bindings/media/i2c/ov772x.txt  | 40 ---------
> > >  .../devicetree/bindings/media/i2c/ov772x.yaml | 84 +++++++++++++++++++
> >
> > Could yuo rename this to ovti,ov772x.yaml ?
> >
> > >  MAINTAINERS                                   |  2 +-
> > >  3 files changed, 85 insertions(+), 41 deletions(-)
> > >  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > deleted file mode 100644
> > > index 0b3ede5b8e6a..000000000000
> > > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > +++ /dev/null
> > > @@ -1,40 +0,0 @@
> > > -* Omnivision OV7720/OV7725 CMOS sensor
> > > -
> > > -The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> > > -such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> > > -support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> > > -
> > > -Required Properties:
> > > -- compatible: shall be one of
> > > -	"ovti,ov7720"
> > > -	"ovti,ov7725"
> > > -- clocks: reference to the xclk input clock.
> > > -
> > > -Optional Properties:
> > > -- reset-gpios: reference to the GPIO connected to the RSTB pin which is
> > > -  active low, if any.
> > > -- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
> > > -  active high, if any.
> > > -
> > > -The device node shall contain one 'port' child node with one child '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:
> > > -
> > > -&i2c0 {
> > > -	ov772x: camera@21 {
> > > -		compatible = "ovti,ov7725";
> > > -		reg = <0x21>;
> > > -		reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> > > -		powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> > > -		clocks = <&xclk>;
> > > -
> > > -		port {
> > > -			ov772x_0: endpoint {
> > > -				remote-endpoint = <&vcap1_in0>;
> > > -			};
> > > -		};
> > > -	};
> > > -};
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > new file mode 100644
> > > index 000000000000..2b84fefeb4aa
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > @@ -0,0 +1,84 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/i2c/ov772x.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title:  Omnivision OV7720/OV7725 CMOS sensor
> > > +
> > > +maintainers:
> > > +  - Jacopo Mondi <jacopo@jmondi.org>
> > > +
> > > +description: -|
> > > +  The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> > > +  such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> > > +  support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - ovti,ov7720
> > > +      - ovti,ov7725
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    maxItems: 1
> > > +
> > > +  reset-gpios:
> > > +    description: -|
> > > +      Reference to the GPIO connected to the RSTB pin which is active low.
> > > +    maxItems: 1
> > > +
> > > +  powerdown-gpios:
> > > +    description: -|
> > > +      Reference to the GPIO connected to the PWDN pin which is active high.
> > > +    maxItems: 1
> > > +
> > > +  port:
> > > +    type: object
> > > +    description: |
> > > +      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.
> >
> > You can simply write
> >
> >       Digital input video port. See ../video-interfaces.txt.
> >
> > > +
> > > +    properties:
> > > +      endpoint:
> > > +        type: object
> > > +        properties:
> > > +          remote-endpoint:
> > > +            description: A phandle to the bus receiver's endpoint node.
> >
> >            required:
> > 	     - remote-endpoint
> >
> >            additionalProperties: false
> 
> I receveied a reply to you on previous json-schema conversion attempt
> where you suggested to not set remote-endpoint as required, as we
> allow empty ones to be later filled in in, maybe with an overlay.
> 
> Which Laurent should I listen to ? I tend to agree with the one that
> said to drop remote-endpoint from the required properties list.

Maybe I recall incorrectly, didn't I say that endpoint shouldn't be
mandatory ? Ports should be mandatory as they describe the hardware,
endpoints describe a connection, and within a connection, I'm not sure
to see a use-case for not setting remote-endpoint. Maybe I need to look
better ? :-)

> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > > +
> > > +    additionalProperties: false
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - clocks
> > > +  - reset-gpios
> > > +  - powerdown-gpios
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/gpio/gpio.h>
> > > +
> > > +    i2c0 {
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +        ov772x: camera@21 {
> > > +            compatible = "ovti,ov7725";
> > > +            reg = <0x21>;
> > > +            reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> > > +            powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> > > +            clocks = <&xclk>;
> > > +
> > > +            port {
> > > +                ov772x_0: endpoint {
> > > +                    remote-endpoint = <&vcap1_in0>;
> > > +                };
> > > +            };
> > > +        };
> > > +    };
> > > +
> > > +...
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index d1a6173d3b64..d0a20214eaaf 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -12666,7 +12666,7 @@ M:	Jacopo Mondi <jacopo@jmondi.org>
> > >  L:	linux-media@vger.kernel.org
> > >  S:	Odd fixes
> > >  T:	git git://linuxtv.org/media_tree.git
> > > -F:	Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > +F:	Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > >  F:	drivers/media/i2c/ov772x.c
> > >  F:	include/media/i2c/ov772x.h
> > >

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema
  2020-08-24 12:15         ` Jacopo Mondi
@ 2020-08-24 12:14           ` Laurent Pinchart
  2020-08-25 20:55             ` Rob Herring
  0 siblings, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2020-08-24 12:14 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: Jacopo Mondi, robh+dt, devicetree, linux-media, Lad, Prabhakar,
	mchehab, sakari.ailus, hverkuil-cisco, linux-renesas-soc

Hi Jacopo,

On Mon, Aug 24, 2020 at 02:15:13PM +0200, Jacopo Mondi wrote:
> On Mon, Aug 24, 2020 at 02:34:40PM +0300, Laurent Pinchart wrote:
> > On Mon, Aug 24, 2020 at 10:32:11AM +0200, Jacopo Mondi wrote:
> > > On Wed, Aug 19, 2020 at 04:52:04PM +0300, Laurent Pinchart wrote:
> > > > On Tue, Aug 18, 2020 at 02:20:10PM +0200, Jacopo Mondi wrote:
> > > > > Convert the ov772x binding document to json-schema and update
> > > > > the MAINTAINERS file accordingly.
> > > > >
> > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > > > ---
> > > > >  .../devicetree/bindings/media/i2c/ov772x.txt  | 40 ---------
> > > > >  .../devicetree/bindings/media/i2c/ov772x.yaml | 84 +++++++++++++++++++
> > > >
> > > > Could yuo rename this to ovti,ov772x.yaml ?
> > > >
> > > > >  MAINTAINERS                                   |  2 +-
> > > > >  3 files changed, 85 insertions(+), 41 deletions(-)
> > > > >  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > > > deleted file mode 100644
> > > > > index 0b3ede5b8e6a..000000000000
> > > > > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > > > +++ /dev/null
> > > > > @@ -1,40 +0,0 @@
> > > > > -* Omnivision OV7720/OV7725 CMOS sensor
> > > > > -
> > > > > -The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> > > > > -such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> > > > > -support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> > > > > -
> > > > > -Required Properties:
> > > > > -- compatible: shall be one of
> > > > > -	"ovti,ov7720"
> > > > > -	"ovti,ov7725"
> > > > > -- clocks: reference to the xclk input clock.
> > > > > -
> > > > > -Optional Properties:
> > > > > -- reset-gpios: reference to the GPIO connected to the RSTB pin which is
> > > > > -  active low, if any.
> > > > > -- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
> > > > > -  active high, if any.
> > > > > -
> > > > > -The device node shall contain one 'port' child node with one child '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:
> > > > > -
> > > > > -&i2c0 {
> > > > > -	ov772x: camera@21 {
> > > > > -		compatible = "ovti,ov7725";
> > > > > -		reg = <0x21>;
> > > > > -		reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> > > > > -		powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> > > > > -		clocks = <&xclk>;
> > > > > -
> > > > > -		port {
> > > > > -			ov772x_0: endpoint {
> > > > > -				remote-endpoint = <&vcap1_in0>;
> > > > > -			};
> > > > > -		};
> > > > > -	};
> > > > > -};
> > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..2b84fefeb4aa
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > > @@ -0,0 +1,84 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/media/i2c/ov772x.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title:  Omnivision OV7720/OV7725 CMOS sensor
> > > > > +
> > > > > +maintainers:
> > > > > +  - Jacopo Mondi <jacopo@jmondi.org>
> > > > > +
> > > > > +description: -|
> > > > > +  The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> > > > > +  such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> > > > > +  support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    enum:
> > > > > +      - ovti,ov7720
> > > > > +      - ovti,ov7725
> > > > > +
> > > > > +  reg:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  clocks:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  reset-gpios:
> > > > > +    description: -|
> > > > > +      Reference to the GPIO connected to the RSTB pin which is active low.
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  powerdown-gpios:
> > > > > +    description: -|
> > > > > +      Reference to the GPIO connected to the PWDN pin which is active high.
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  port:
> > > > > +    type: object
> > > > > +    description: |
> > > > > +      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.
> > > >
> > > > You can simply write
> > > >
> > > >       Digital input video port. See ../video-interfaces.txt.
> > > >
> > > > > +
> > > > > +    properties:
> > > > > +      endpoint:
> > > > > +        type: object
> > > > > +        properties:
> > > > > +          remote-endpoint:
> > > > > +            description: A phandle to the bus receiver's endpoint node.
> > > >
> > > >            required:
> > > > 	     - remote-endpoint
> > > >
> > > >            additionalProperties: false
> > >
> > > I receveied a reply to you on previous json-schema conversion attempt
> > > where you suggested to not set remote-endpoint as required, as we
> > > allow empty ones to be later filled in in, maybe with an overlay.
> > >
> > > Which Laurent should I listen to ? I tend to agree with the one that
> > > said to drop remote-endpoint from the required properties list.
> >
> > Maybe I recall incorrectly, didn't I say that endpoint shouldn't be
> > mandatory ? Ports should be mandatory as they describe the hardware,
> > endpoints describe a connection, and within a connection, I'm not sure
> > to see a use-case for not setting remote-endpoint. Maybe I need to look
> > better ? :-)
> >
> 
> I might be confused as well, but to me port and endpoint should be
> there as they represent the available endpoints of the devices connections.
> Connections to external devices that can be established (or overwritten)
> by applying an overlay, and such are not mandatory.
> 
> As I see it:
> - port/endpoints: establish the available device connection endpoitns
>   and shall be mandatory (also to give a known place where to 'plug'
>   the connections)
> 
> - remote-endpoints: data connections to external devices, which might
>   depend on the board assembly or installed 'capes' and expansions. As
>   such, they can be modeled as an overlay fragment to be applied on the
>   (known layout of the) device.

Only the port represents a connection point. The endpoint node is part
of the representation of the link, it doesn't map to a particular
hardware resource on the port side.

> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > > > +
> > > > > +    additionalProperties: false
> > > > > +
> > > > > +required:
> > > > > +  - compatible
> > > > > +  - reg
> > > > > +  - clocks
> > > > > +  - reset-gpios
> > > > > +  - powerdown-gpios
> > > > > +
> > > > > +examples:
> > > > > +  - |
> > > > > +    #include <dt-bindings/gpio/gpio.h>
> > > > > +
> > > > > +    i2c0 {
> > > > > +        #address-cells = <1>;
> > > > > +        #size-cells = <0>;
> > > > > +        ov772x: camera@21 {
> > > > > +            compatible = "ovti,ov7725";
> > > > > +            reg = <0x21>;
> > > > > +            reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> > > > > +            powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> > > > > +            clocks = <&xclk>;
> > > > > +
> > > > > +            port {
> > > > > +                ov772x_0: endpoint {
> > > > > +                    remote-endpoint = <&vcap1_in0>;
> > > > > +                };
> > > > > +            };
> > > > > +        };
> > > > > +    };
> > > > > +
> > > > > +...
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > index d1a6173d3b64..d0a20214eaaf 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -12666,7 +12666,7 @@ M:	Jacopo Mondi <jacopo@jmondi.org>
> > > > >  L:	linux-media@vger.kernel.org
> > > > >  S:	Odd fixes
> > > > >  T:	git git://linuxtv.org/media_tree.git
> > > > > -F:	Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > > > +F:	Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > >  F:	drivers/media/i2c/ov772x.c
> > > > >  F:	include/media/i2c/ov772x.h
> > > > >

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema
  2020-08-24 11:34       ` Laurent Pinchart
@ 2020-08-24 12:15         ` Jacopo Mondi
  2020-08-24 12:14           ` Laurent Pinchart
  0 siblings, 1 reply; 23+ messages in thread
From: Jacopo Mondi @ 2020-08-24 12:15 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Jacopo Mondi, robh+dt, devicetree, linux-media, Lad, Prabhakar,
	mchehab, sakari.ailus, hverkuil-cisco, linux-renesas-soc

Hi Laurent,

On Mon, Aug 24, 2020 at 02:34:40PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Mon, Aug 24, 2020 at 10:32:11AM +0200, Jacopo Mondi wrote:
> > On Wed, Aug 19, 2020 at 04:52:04PM +0300, Laurent Pinchart wrote:
> > > On Tue, Aug 18, 2020 at 02:20:10PM +0200, Jacopo Mondi wrote:
> > > > Convert the ov772x binding document to json-schema and update
> > > > the MAINTAINERS file accordingly.
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > > ---
> > > >  .../devicetree/bindings/media/i2c/ov772x.txt  | 40 ---------
> > > >  .../devicetree/bindings/media/i2c/ov772x.yaml | 84 +++++++++++++++++++
> > >
> > > Could yuo rename this to ovti,ov772x.yaml ?
> > >
> > > >  MAINTAINERS                                   |  2 +-
> > > >  3 files changed, 85 insertions(+), 41 deletions(-)
> > > >  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > > deleted file mode 100644
> > > > index 0b3ede5b8e6a..000000000000
> > > > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > > +++ /dev/null
> > > > @@ -1,40 +0,0 @@
> > > > -* Omnivision OV7720/OV7725 CMOS sensor
> > > > -
> > > > -The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> > > > -such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> > > > -support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> > > > -
> > > > -Required Properties:
> > > > -- compatible: shall be one of
> > > > -	"ovti,ov7720"
> > > > -	"ovti,ov7725"
> > > > -- clocks: reference to the xclk input clock.
> > > > -
> > > > -Optional Properties:
> > > > -- reset-gpios: reference to the GPIO connected to the RSTB pin which is
> > > > -  active low, if any.
> > > > -- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
> > > > -  active high, if any.
> > > > -
> > > > -The device node shall contain one 'port' child node with one child '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:
> > > > -
> > > > -&i2c0 {
> > > > -	ov772x: camera@21 {
> > > > -		compatible = "ovti,ov7725";
> > > > -		reg = <0x21>;
> > > > -		reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> > > > -		powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> > > > -		clocks = <&xclk>;
> > > > -
> > > > -		port {
> > > > -			ov772x_0: endpoint {
> > > > -				remote-endpoint = <&vcap1_in0>;
> > > > -			};
> > > > -		};
> > > > -	};
> > > > -};
> > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > new file mode 100644
> > > > index 000000000000..2b84fefeb4aa
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > @@ -0,0 +1,84 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/media/i2c/ov772x.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title:  Omnivision OV7720/OV7725 CMOS sensor
> > > > +
> > > > +maintainers:
> > > > +  - Jacopo Mondi <jacopo@jmondi.org>
> > > > +
> > > > +description: -|
> > > > +  The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> > > > +  such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> > > > +  support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - ovti,ov7720
> > > > +      - ovti,ov7725
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  clocks:
> > > > +    maxItems: 1
> > > > +
> > > > +  reset-gpios:
> > > > +    description: -|
> > > > +      Reference to the GPIO connected to the RSTB pin which is active low.
> > > > +    maxItems: 1
> > > > +
> > > > +  powerdown-gpios:
> > > > +    description: -|
> > > > +      Reference to the GPIO connected to the PWDN pin which is active high.
> > > > +    maxItems: 1
> > > > +
> > > > +  port:
> > > > +    type: object
> > > > +    description: |
> > > > +      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.
> > >
> > > You can simply write
> > >
> > >       Digital input video port. See ../video-interfaces.txt.
> > >
> > > > +
> > > > +    properties:
> > > > +      endpoint:
> > > > +        type: object
> > > > +        properties:
> > > > +          remote-endpoint:
> > > > +            description: A phandle to the bus receiver's endpoint node.
> > >
> > >            required:
> > > 	     - remote-endpoint
> > >
> > >            additionalProperties: false
> >
> > I receveied a reply to you on previous json-schema conversion attempt
> > where you suggested to not set remote-endpoint as required, as we
> > allow empty ones to be later filled in in, maybe with an overlay.
> >
> > Which Laurent should I listen to ? I tend to agree with the one that
> > said to drop remote-endpoint from the required properties list.
>
> Maybe I recall incorrectly, didn't I say that endpoint shouldn't be
> mandatory ? Ports should be mandatory as they describe the hardware,
> endpoints describe a connection, and within a connection, I'm not sure
> to see a use-case for not setting remote-endpoint. Maybe I need to look
> better ? :-)
>

I might be confused as well, but to me port and endpoint should be
there as they represent the available endpoints of the devices connections.
Connections to external devices that can be established (or overwritten)
by applying an overlay, and such are not mandatory.

As I see it:
- port/endpoints: establish the available device connection endpoitns
  and shall be mandatory (also to give a known place where to 'plug'
  the connections)

- remote-endpoints: data connections to external devices, which might
  depend on the board assembly or installed 'capes' and expansions. As
  such, they can be modeled as an overlay fragment to be applied on the
  (known layout of the) device.


> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >
> > > > +
> > > > +    additionalProperties: false
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - clocks
> > > > +  - reset-gpios
> > > > +  - powerdown-gpios
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    #include <dt-bindings/gpio/gpio.h>
> > > > +
> > > > +    i2c0 {
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <0>;
> > > > +        ov772x: camera@21 {
> > > > +            compatible = "ovti,ov7725";
> > > > +            reg = <0x21>;
> > > > +            reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> > > > +            powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> > > > +            clocks = <&xclk>;
> > > > +
> > > > +            port {
> > > > +                ov772x_0: endpoint {
> > > > +                    remote-endpoint = <&vcap1_in0>;
> > > > +                };
> > > > +            };
> > > > +        };
> > > > +    };
> > > > +
> > > > +...
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index d1a6173d3b64..d0a20214eaaf 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -12666,7 +12666,7 @@ M:	Jacopo Mondi <jacopo@jmondi.org>
> > > >  L:	linux-media@vger.kernel.org
> > > >  S:	Odd fixes
> > > >  T:	git git://linuxtv.org/media_tree.git
> > > > -F:	Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > > +F:	Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > >  F:	drivers/media/i2c/ov772x.c
> > > >  F:	include/media/i2c/ov772x.h
> > > >
>
> --
> Regards,
>
> Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema
  2020-08-24 12:14           ` Laurent Pinchart
@ 2020-08-25 20:55             ` Rob Herring
  2020-08-25 21:41               ` Laurent Pinchart
  0 siblings, 1 reply; 23+ messages in thread
From: Rob Herring @ 2020-08-25 20:55 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Jacopo Mondi, Jacopo Mondi, devicetree, linux-media, Lad,
	Prabhakar, mchehab, sakari.ailus, hverkuil-cisco,
	linux-renesas-soc

On Mon, Aug 24, 2020 at 03:14:57PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> On Mon, Aug 24, 2020 at 02:15:13PM +0200, Jacopo Mondi wrote:
> > On Mon, Aug 24, 2020 at 02:34:40PM +0300, Laurent Pinchart wrote:
> > > On Mon, Aug 24, 2020 at 10:32:11AM +0200, Jacopo Mondi wrote:
> > > > On Wed, Aug 19, 2020 at 04:52:04PM +0300, Laurent Pinchart wrote:
> > > > > On Tue, Aug 18, 2020 at 02:20:10PM +0200, Jacopo Mondi wrote:
> > > > > > Convert the ov772x binding document to json-schema and update
> > > > > > the MAINTAINERS file accordingly.
> > > > > >
> > > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > > > > ---
> > > > > >  .../devicetree/bindings/media/i2c/ov772x.txt  | 40 ---------
> > > > > >  .../devicetree/bindings/media/i2c/ov772x.yaml | 84 +++++++++++++++++++
> > > > >
> > > > > Could yuo rename this to ovti,ov772x.yaml ?
> > > > >
> > > > > >  MAINTAINERS                                   |  2 +-
> > > > > >  3 files changed, 85 insertions(+), 41 deletions(-)
> > > > > >  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > > > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > > > > deleted file mode 100644
> > > > > > index 0b3ede5b8e6a..000000000000
> > > > > > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > > > > +++ /dev/null
> > > > > > @@ -1,40 +0,0 @@
> > > > > > -* Omnivision OV7720/OV7725 CMOS sensor
> > > > > > -
> > > > > > -The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> > > > > > -such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> > > > > > -support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> > > > > > -
> > > > > > -Required Properties:
> > > > > > -- compatible: shall be one of
> > > > > > -	"ovti,ov7720"
> > > > > > -	"ovti,ov7725"
> > > > > > -- clocks: reference to the xclk input clock.
> > > > > > -
> > > > > > -Optional Properties:
> > > > > > -- reset-gpios: reference to the GPIO connected to the RSTB pin which is
> > > > > > -  active low, if any.
> > > > > > -- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
> > > > > > -  active high, if any.
> > > > > > -
> > > > > > -The device node shall contain one 'port' child node with one child '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:
> > > > > > -
> > > > > > -&i2c0 {
> > > > > > -	ov772x: camera@21 {
> > > > > > -		compatible = "ovti,ov7725";
> > > > > > -		reg = <0x21>;
> > > > > > -		reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> > > > > > -		powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> > > > > > -		clocks = <&xclk>;
> > > > > > -
> > > > > > -		port {
> > > > > > -			ov772x_0: endpoint {
> > > > > > -				remote-endpoint = <&vcap1_in0>;
> > > > > > -			};
> > > > > > -		};
> > > > > > -	};
> > > > > > -};
> > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..2b84fefeb4aa
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > > > @@ -0,0 +1,84 @@
> > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id: http://devicetree.org/schemas/media/i2c/ov772x.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title:  Omnivision OV7720/OV7725 CMOS sensor
> > > > > > +
> > > > > > +maintainers:
> > > > > > +  - Jacopo Mondi <jacopo@jmondi.org>
> > > > > > +
> > > > > > +description: -|
> > > > > > +  The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> > > > > > +  such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> > > > > > +  support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> > > > > > +
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +    enum:
> > > > > > +      - ovti,ov7720
> > > > > > +      - ovti,ov7725
> > > > > > +
> > > > > > +  reg:
> > > > > > +    maxItems: 1
> > > > > > +
> > > > > > +  clocks:
> > > > > > +    maxItems: 1
> > > > > > +
> > > > > > +  reset-gpios:
> > > > > > +    description: -|
> > > > > > +      Reference to the GPIO connected to the RSTB pin which is active low.
> > > > > > +    maxItems: 1
> > > > > > +
> > > > > > +  powerdown-gpios:
> > > > > > +    description: -|
> > > > > > +      Reference to the GPIO connected to the PWDN pin which is active high.
> > > > > > +    maxItems: 1
> > > > > > +
> > > > > > +  port:
> > > > > > +    type: object
> > > > > > +    description: |
> > > > > > +      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.
> > > > >
> > > > > You can simply write
> > > > >
> > > > >       Digital input video port. See ../video-interfaces.txt.
> > > > >
> > > > > > +
> > > > > > +    properties:
> > > > > > +      endpoint:
> > > > > > +        type: object
> > > > > > +        properties:
> > > > > > +          remote-endpoint:
> > > > > > +            description: A phandle to the bus receiver's endpoint node.
> > > > >
> > > > >            required:
> > > > > 	     - remote-endpoint
> > > > >
> > > > >            additionalProperties: false
> > > >
> > > > I receveied a reply to you on previous json-schema conversion attempt
> > > > where you suggested to not set remote-endpoint as required, as we
> > > > allow empty ones to be later filled in in, maybe with an overlay.
> > > >
> > > > Which Laurent should I listen to ? I tend to agree with the one that
> > > > said to drop remote-endpoint from the required properties list.
> > >
> > > Maybe I recall incorrectly, didn't I say that endpoint shouldn't be
> > > mandatory ? Ports should be mandatory as they describe the hardware,
> > > endpoints describe a connection, and within a connection, I'm not sure
> > > to see a use-case for not setting remote-endpoint. Maybe I need to look
> > > better ? :-)
> > >
> > 
> > I might be confused as well, but to me port and endpoint should be
> > there as they represent the available endpoints of the devices connections.
> > Connections to external devices that can be established (or overwritten)
> > by applying an overlay, and such are not mandatory.
> > 
> > As I see it:
> > - port/endpoints: establish the available device connection endpoitns
> >   and shall be mandatory (also to give a known place where to 'plug'
> >   the connections)
> > 
> > - remote-endpoints: data connections to external devices, which might
> >   depend on the board assembly or installed 'capes' and expansions. As
> >   such, they can be modeled as an overlay fragment to be applied on the
> >   (known layout of the) device.
> 
> Only the port represents a connection point. The endpoint node is part
> of the representation of the link, it doesn't map to a particular
> hardware resource on the port side.

I think all of 'endpoint' should be dropped if you only have 
'remote-endpoint' (and no other properties) and you don't have multiple 
endpoints (typically only if the h/w has some sort of built-in mux or 
connection selector). 

Once we have a generic graph schema, it can enforce that 'port' has 
'endpoint' and 'endpoint' has 'remote-endpoint'. Doing this hasn't been 
too high on my list simply because dtc already does all or most of that.

Rob

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema
  2020-08-18 12:20 ` [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema Jacopo Mondi
  2020-08-19 13:52   ` Laurent Pinchart
  2020-08-21 11:26   ` Lad, Prabhakar
@ 2020-08-25 21:03   ` Rob Herring
  2 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2020-08-25 21:03 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: devicetree, linux-media, Lad, Prabhakar, mchehab, sakari.ailus,
	hverkuil-cisco, laurent.pinchart, linux-renesas-soc

On Tue, Aug 18, 2020 at 02:20:10PM +0200, Jacopo Mondi wrote:
> Convert the ov772x binding document to json-schema and update
> the MAINTAINERS file accordingly.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  .../devicetree/bindings/media/i2c/ov772x.txt  | 40 ---------
>  .../devicetree/bindings/media/i2c/ov772x.yaml | 84 +++++++++++++++++++
>  MAINTAINERS                                   |  2 +-
>  3 files changed, 85 insertions(+), 41 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> deleted file mode 100644
> index 0b3ede5b8e6a..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> +++ /dev/null
> @@ -1,40 +0,0 @@
> -* Omnivision OV7720/OV7725 CMOS sensor
> -
> -The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> -such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> -support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> -
> -Required Properties:
> -- compatible: shall be one of
> -	"ovti,ov7720"
> -	"ovti,ov7725"
> -- clocks: reference to the xclk input clock.
> -
> -Optional Properties:
> -- reset-gpios: reference to the GPIO connected to the RSTB pin which is
> -  active low, if any.
> -- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
> -  active high, if any.
> -
> -The device node shall contain one 'port' child node with one child '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:
> -
> -&i2c0 {
> -	ov772x: camera@21 {
> -		compatible = "ovti,ov7725";
> -		reg = <0x21>;
> -		reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> -		powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> -		clocks = <&xclk>;
> -
> -		port {
> -			ov772x_0: endpoint {
> -				remote-endpoint = <&vcap1_in0>;
> -			};
> -		};
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> new file mode 100644
> index 000000000000..2b84fefeb4aa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> @@ -0,0 +1,84 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ov772x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title:  Omnivision OV7720/OV7725 CMOS sensor
> +
> +maintainers:
> +  - Jacopo Mondi <jacopo@jmondi.org>
> +
> +description: -|
> +  The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> +  such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> +  support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ovti,ov7720
> +      - ovti,ov7725
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    description: -|

You can drop '-|' as there's not any formatting to preserve here.

> +      Reference to the GPIO connected to the RSTB pin which is active low.
> +    maxItems: 1
> +
> +  powerdown-gpios:
> +    description: -|
> +      Reference to the GPIO connected to the PWDN pin which is active high.
> +    maxItems: 1
> +
> +  port:
> +    type: object
> +    description: |
> +      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.
> +
> +    properties:
> +      endpoint:
> +        type: object
> +        properties:
> +          remote-endpoint:
> +            description: A phandle to the bus receiver's endpoint node.
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - reset-gpios
> +  - powerdown-gpios

port not required?

Add:

additionalProperties: false

> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        ov772x: camera@21 {
> +            compatible = "ovti,ov7725";
> +            reg = <0x21>;
> +            reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> +            powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> +            clocks = <&xclk>;
> +
> +            port {
> +                ov772x_0: endpoint {
> +                    remote-endpoint = <&vcap1_in0>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d1a6173d3b64..d0a20214eaaf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12666,7 +12666,7 @@ M:	Jacopo Mondi <jacopo@jmondi.org>
>  L:	linux-media@vger.kernel.org
>  S:	Odd fixes
>  T:	git git://linuxtv.org/media_tree.git
> -F:	Documentation/devicetree/bindings/media/i2c/ov772x.txt
> +F:	Documentation/devicetree/bindings/media/i2c/ov772x.yaml
>  F:	drivers/media/i2c/ov772x.c
>  F:	include/media/i2c/ov772x.h
>  
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/3] dt-bindings: media: ov772x: Make bus-type mandatory
  2020-08-18 12:20 ` [PATCH 2/3] dt-bindings: media: ov772x: Make bus-type mandatory Jacopo Mondi
  2020-08-19 13:52   ` Laurent Pinchart
  2020-08-21 11:29   ` Lad, Prabhakar
@ 2020-08-25 21:06   ` Rob Herring
  2 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2020-08-25 21:06 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: robh+dt, devicetree, Lad, Prabhakar, linux-renesas-soc,
	sakari.ailus, mchehab, linux-media, hverkuil-cisco,
	laurent.pinchart

On Tue, 18 Aug 2020 14:20:11 +0200, Jacopo Mondi wrote:
> In order to establish required properties based on the selected
> bus type, make the 'bus-type' property mandatory.
> 
> Binary compatibility with existing DTB is kept as the driver does not
> enforce the property to be present, and shall fall-back to default
> parallel bus configuration, which was the only supported bus type, if
> the property is not specified.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/i2c/ov772x.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema
  2020-08-25 20:55             ` Rob Herring
@ 2020-08-25 21:41               ` Laurent Pinchart
  0 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2020-08-25 21:41 UTC (permalink / raw)
  To: Rob Herring
  Cc: Jacopo Mondi, Jacopo Mondi, devicetree, linux-media, Lad,
	Prabhakar, mchehab, sakari.ailus, hverkuil-cisco,
	linux-renesas-soc

Hi Rob,

On Tue, Aug 25, 2020 at 02:55:31PM -0600, Rob Herring wrote:
> On Mon, Aug 24, 2020 at 03:14:57PM +0300, Laurent Pinchart wrote:
> > On Mon, Aug 24, 2020 at 02:15:13PM +0200, Jacopo Mondi wrote:
> > > On Mon, Aug 24, 2020 at 02:34:40PM +0300, Laurent Pinchart wrote:
> > > > On Mon, Aug 24, 2020 at 10:32:11AM +0200, Jacopo Mondi wrote:
> > > > > On Wed, Aug 19, 2020 at 04:52:04PM +0300, Laurent Pinchart wrote:
> > > > > > On Tue, Aug 18, 2020 at 02:20:10PM +0200, Jacopo Mondi wrote:
> > > > > > > Convert the ov772x binding document to json-schema and update
> > > > > > > the MAINTAINERS file accordingly.
> > > > > > >
> > > > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > > > > > ---
> > > > > > >  .../devicetree/bindings/media/i2c/ov772x.txt  | 40 ---------
> > > > > > >  .../devicetree/bindings/media/i2c/ov772x.yaml | 84 +++++++++++++++++++
> > > > > >
> > > > > > Could yuo rename this to ovti,ov772x.yaml ?
> > > > > >
> > > > > > >  MAINTAINERS                                   |  2 +-
> > > > > > >  3 files changed, 85 insertions(+), 41 deletions(-)
> > > > > > >  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > > > > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > > > >
> > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > > > > > deleted file mode 100644
> > > > > > > index 0b3ede5b8e6a..000000000000
> > > > > > > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> > > > > > > +++ /dev/null
> > > > > > > @@ -1,40 +0,0 @@
> > > > > > > -* Omnivision OV7720/OV7725 CMOS sensor
> > > > > > > -
> > > > > > > -The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> > > > > > > -such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> > > > > > > -support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> > > > > > > -
> > > > > > > -Required Properties:
> > > > > > > -- compatible: shall be one of
> > > > > > > -	"ovti,ov7720"
> > > > > > > -	"ovti,ov7725"
> > > > > > > -- clocks: reference to the xclk input clock.
> > > > > > > -
> > > > > > > -Optional Properties:
> > > > > > > -- reset-gpios: reference to the GPIO connected to the RSTB pin which is
> > > > > > > -  active low, if any.
> > > > > > > -- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
> > > > > > > -  active high, if any.
> > > > > > > -
> > > > > > > -The device node shall contain one 'port' child node with one child '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:
> > > > > > > -
> > > > > > > -&i2c0 {
> > > > > > > -	ov772x: camera@21 {
> > > > > > > -		compatible = "ovti,ov7725";
> > > > > > > -		reg = <0x21>;
> > > > > > > -		reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
> > > > > > > -		powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
> > > > > > > -		clocks = <&xclk>;
> > > > > > > -
> > > > > > > -		port {
> > > > > > > -			ov772x_0: endpoint {
> > > > > > > -				remote-endpoint = <&vcap1_in0>;
> > > > > > > -			};
> > > > > > > -		};
> > > > > > > -	};
> > > > > > > -};
> > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > > > > new file mode 100644
> > > > > > > index 000000000000..2b84fefeb4aa
> > > > > > > --- /dev/null
> > > > > > > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > > > > @@ -0,0 +1,84 @@
> > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > > +%YAML 1.2
> > > > > > > +---
> > > > > > > +$id: http://devicetree.org/schemas/media/i2c/ov772x.yaml#
> > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > > +
> > > > > > > +title:  Omnivision OV7720/OV7725 CMOS sensor
> > > > > > > +
> > > > > > > +maintainers:
> > > > > > > +  - Jacopo Mondi <jacopo@jmondi.org>
> > > > > > > +
> > > > > > > +description: -|
> > > > > > > +  The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> > > > > > > +  such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> > > > > > > +  support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> > > > > > > +
> > > > > > > +properties:
> > > > > > > +  compatible:
> > > > > > > +    enum:
> > > > > > > +      - ovti,ov7720
> > > > > > > +      - ovti,ov7725
> > > > > > > +
> > > > > > > +  reg:
> > > > > > > +    maxItems: 1
> > > > > > > +
> > > > > > > +  clocks:
> > > > > > > +    maxItems: 1
> > > > > > > +
> > > > > > > +  reset-gpios:
> > > > > > > +    description: -|
> > > > > > > +      Reference to the GPIO connected to the RSTB pin which is active low.
> > > > > > > +    maxItems: 1
> > > > > > > +
> > > > > > > +  powerdown-gpios:
> > > > > > > +    description: -|
> > > > > > > +      Reference to the GPIO connected to the PWDN pin which is active high.
> > > > > > > +    maxItems: 1
> > > > > > > +
> > > > > > > +  port:
> > > > > > > +    type: object
> > > > > > > +    description: |
> > > > > > > +      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.
> > > > > >
> > > > > > You can simply write
> > > > > >
> > > > > >       Digital input video port. See ../video-interfaces.txt.
> > > > > >
> > > > > > > +
> > > > > > > +    properties:
> > > > > > > +      endpoint:
> > > > > > > +        type: object
> > > > > > > +        properties:
> > > > > > > +          remote-endpoint:
> > > > > > > +            description: A phandle to the bus receiver's endpoint node.
> > > > > >
> > > > > >            required:
> > > > > > 	     - remote-endpoint
> > > > > >
> > > > > >            additionalProperties: false
> > > > >
> > > > > I receveied a reply to you on previous json-schema conversion attempt
> > > > > where you suggested to not set remote-endpoint as required, as we
> > > > > allow empty ones to be later filled in in, maybe with an overlay.
> > > > >
> > > > > Which Laurent should I listen to ? I tend to agree with the one that
> > > > > said to drop remote-endpoint from the required properties list.
> > > >
> > > > Maybe I recall incorrectly, didn't I say that endpoint shouldn't be
> > > > mandatory ? Ports should be mandatory as they describe the hardware,
> > > > endpoints describe a connection, and within a connection, I'm not sure
> > > > to see a use-case for not setting remote-endpoint. Maybe I need to look
> > > > better ? :-)
> > > >
> > > 
> > > I might be confused as well, but to me port and endpoint should be
> > > there as they represent the available endpoints of the devices connections.
> > > Connections to external devices that can be established (or overwritten)
> > > by applying an overlay, and such are not mandatory.
> > > 
> > > As I see it:
> > > - port/endpoints: establish the available device connection endpoitns
> > >   and shall be mandatory (also to give a known place where to 'plug'
> > >   the connections)
> > > 
> > > - remote-endpoints: data connections to external devices, which might
> > >   depend on the board assembly or installed 'capes' and expansions. As
> > >   such, they can be modeled as an overlay fragment to be applied on the
> > >   (known layout of the) device.
> > 
> > Only the port represents a connection point. The endpoint node is part
> > of the representation of the link, it doesn't map to a particular
> > hardware resource on the port side.
> 
> I think all of 'endpoint' should be dropped if you only have 
> 'remote-endpoint' (and no other properties) and you don't have multiple 
> endpoints (typically only if the h/w has some sort of built-in mux or 
> connection selector). 

Makes sense to me, I'll keep this in mind during review.

> Once we have a generic graph schema, it can enforce that 'port' has 
> 'endpoint' and 'endpoint' has 'remote-endpoint'.

I think 'endpoint' should be optional though, as a port can exist but
not be connected. 'remote-endpoint' should be mandatory.

> Doing this hasn't been 
> too high on my list simply because dtc already does all or most of that.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props
  2020-08-22  1:35       ` Laurent Pinchart
@ 2020-08-26  8:42         ` Lad, Prabhakar
  0 siblings, 0 replies; 23+ messages in thread
From: Lad, Prabhakar @ 2020-08-26  8:42 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Jacopo Mondi, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-media, Mauro Carvalho Chehab, Sakari Ailus, Hans Verkuil,
	Linux-Renesas

Hi Laurent,

On Sat, Aug 22, 2020 at 2:36 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Prabhakar,
>
> On Fri, Aug 21, 2020 at 12:37:35PM +0100, Lad, Prabhakar wrote:
> > On Wed, Aug 19, 2020 at 2:54 PM Laurent Pinchart wrote:
> > > On Tue, Aug 18, 2020 at 02:20:12PM +0200, Jacopo Mondi wrote:
> > > > Document endpoint properties for the parallel bus type and
> > > > add them to the example.
> > > >
> > > > Specify a few constraints:
> > > > - If the bus type is BT.656 no hsync or vsycn polarities can be
> > > >   specified.
> > > > - If the bus width is 10 bits, not data-shift can be applied.
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > > ---
> > > >  .../devicetree/bindings/media/i2c/ov772x.yaml | 43 +++++++++++++++++++
> > > >  1 file changed, 43 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > index 75dad40f70cc..3fad5dffd19a 100644
> > > > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > @@ -50,9 +50,47 @@ 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
> > > > +
> > > >            remote-endpoint:
> > > >              description: A phandle to the bus receiver's endpoint node.
> > > >
> > > > +        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
> > >
> > > I'd add a blank line here.
> > >
> > > >          required:
> > > >            - bus-type
> > >
> > > Should some of the properties be required ? Possibly conditioned on
> > > bus-type ?
> >
> > Agreed, would be interesting to know how this can be handled (split
> > out bus-type and add required properties for each) ?
>
> We can add required: statements to the above if/then/else.
>
Aha thanks for pointing out. (I hadn't come across such cases)

Cheers,
Prabhakar

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props
  2020-08-24  8:35       ` Jacopo Mondi
@ 2020-08-26  8:47         ` Lad, Prabhakar
  0 siblings, 0 replies; 23+ messages in thread
From: Lad, Prabhakar @ 2020-08-26  8:47 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: Laurent Pinchart, Jacopo Mondi, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-media, Mauro Carvalho Chehab, Sakari Ailus, Hans Verkuil,
	Linux-Renesas

Hi Jacopo

On Mon, Aug 24, 2020 at 9:32 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi Laurent, Prabhakar,
>
> On Fri, Aug 21, 2020 at 12:37:35PM +0100, Lad, Prabhakar wrote:
> > Hi Laurent and Jacopo
> >
> > On Wed, Aug 19, 2020 at 2:54 PM Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> > >
> > > Hi Jacopo,
> > >
> > > Thank you for the patch.
> > >
> > > On Tue, Aug 18, 2020 at 02:20:12PM +0200, Jacopo Mondi wrote:
> > > > Document endpoint properties for the parallel bus type and
> > > > add them to the example.
> > > >
> > > > Specify a few constraints:
> > > > - If the bus type is BT.656 no hsync or vsycn polarities can be
> > > >   specified.
> > > > - If the bus width is 10 bits, not data-shift can be applied.
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > > ---
> > > >  .../devicetree/bindings/media/i2c/ov772x.yaml | 43 +++++++++++++++++++
> > > >  1 file changed, 43 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > index 75dad40f70cc..3fad5dffd19a 100644
> > > > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
> > > > @@ -50,9 +50,47 @@ 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
> > > > +
> > > >            remote-endpoint:
> > > >              description: A phandle to the bus receiver's endpoint node.
> > > >
> > > > +        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
> > >
> > > I'd add a blank line here.
> > >
> > > >          required:
> > > >            - bus-type
> > >
> > > Should some of the properties be required ? Possibly conditioned on
> > > bus-type ?
> > >
>
> I am not sure. They all have defaults, as reported here and as
> supported by the driver. There's nothing -strictly- required, as long
> as the here reported defaults are correct.
>
Agreed, ran dt_binding_check and looks good so,

Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> > Agreed, would be interesting to know how this can be handled (split
> > out bus-type and add required properties for each) ?
> >
>
> That already happens with
>
> +          - if:
> +              properties:
> +                bus-type:
> +                  const: 6
> +            then:
> +                properties:
> +                  hsync-active: false
> +                  vsync-active: false
>
> And could be expanded, if we want any of these to be required.
>
> Thanks
>   j
>
> > Cheers,
> > Prabhakar
> >
> > > >
> > > > @@ -82,6 +120,11 @@ examples:
> > > >              port {
> > > >                  ov772x_0: endpoint {
> > > >                      bus-type = <5>;
> > > > +                    vsync-active = <0>;
> > > > +                    hsync-active = <0>;
> > > > +                    pclk-sample = <0>;
> > > > +                    bus-width = <8>;
> > > > +                    data-shift = <0>;
> > > >                      remote-endpoint = <&vcap1_in0>;
> > > >                  };
> > > >              };
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema
  2020-08-17 15:59 [PATCH 0/3] dt-bindings: media: ov772x: Convert to json-schama Jacopo Mondi
@ 2020-08-17 15:59 ` Jacopo Mondi
  0 siblings, 0 replies; 23+ messages in thread
From: Jacopo Mondi @ 2020-08-17 15:59 UTC (permalink / raw)
  To: robh+dt, devicetree, linux-media, Lad, Prabhakar
  Cc: Jacopo Mondi, mchehab, sakari.ailus, hverkuil-cisco,
	laurent.pinchart, linux-renesas-soc

Convert the ov772x binding document to json-schema and update
the MAINTAINERS file accordingly.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 .../devicetree/bindings/media/i2c/ov772x.txt  | 40 ---------
 .../devicetree/bindings/media/i2c/ov772x.yaml | 84 +++++++++++++++++++
 MAINTAINERS                                   |  2 +-
 3 files changed, 85 insertions(+), 41 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
deleted file mode 100644
index 0b3ede5b8e6a..000000000000
--- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
+++ /dev/null
@@ -1,40 +0,0 @@
-* Omnivision OV7720/OV7725 CMOS sensor
-
-The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
-such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
-support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
-
-Required Properties:
-- compatible: shall be one of
-	"ovti,ov7720"
-	"ovti,ov7725"
-- clocks: reference to the xclk input clock.
-
-Optional Properties:
-- reset-gpios: reference to the GPIO connected to the RSTB pin which is
-  active low, if any.
-- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
-  active high, if any.
-
-The device node shall contain one 'port' child node with one child '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:
-
-&i2c0 {
-	ov772x: camera@21 {
-		compatible = "ovti,ov7725";
-		reg = <0x21>;
-		reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
-		powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
-		clocks = <&xclk>;
-
-		port {
-			ov772x_0: endpoint {
-				remote-endpoint = <&vcap1_in0>;
-			};
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.yaml b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
new file mode 100644
index 000000000000..2b84fefeb4aa
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov772x.yaml
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ov772x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title:  Omnivision OV7720/OV7725 CMOS sensor
+
+maintainers:
+  - Jacopo Mondi <jacopo@jmondi.org>
+
+description: -|
+  The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
+  such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
+  support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
+
+properties:
+  compatible:
+    enum:
+      - ovti,ov7720
+      - ovti,ov7725
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  reset-gpios:
+    description: -|
+      Reference to the GPIO connected to the RSTB pin which is active low.
+    maxItems: 1
+
+  powerdown-gpios:
+    description: -|
+      Reference to the GPIO connected to the PWDN pin which is active high.
+    maxItems: 1
+
+  port:
+    type: object
+    description: |
+      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.
+
+    properties:
+      endpoint:
+        type: object
+        properties:
+          remote-endpoint:
+            description: A phandle to the bus receiver's endpoint node.
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - reset-gpios
+  - powerdown-gpios
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        ov772x: camera@21 {
+            compatible = "ovti,ov7725";
+            reg = <0x21>;
+            reset-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
+            powerdown-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
+            clocks = <&xclk>;
+
+            port {
+                ov772x_0: endpoint {
+                    remote-endpoint = <&vcap1_in0>;
+                };
+            };
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index d1a6173d3b64..d0a20214eaaf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12666,7 +12666,7 @@ M:	Jacopo Mondi <jacopo@jmondi.org>
 L:	linux-media@vger.kernel.org
 S:	Odd fixes
 T:	git git://linuxtv.org/media_tree.git
-F:	Documentation/devicetree/bindings/media/i2c/ov772x.txt
+F:	Documentation/devicetree/bindings/media/i2c/ov772x.yaml
 F:	drivers/media/i2c/ov772x.c
 F:	include/media/i2c/ov772x.h
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2020-08-26  8:47 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-18 12:20 [PATCH 0/3] dt-bindings: media: ov772x: Convert to json-schama Jacopo Mondi
2020-08-18 12:20 ` [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema Jacopo Mondi
2020-08-19 13:52   ` Laurent Pinchart
2020-08-24  8:32     ` Jacopo Mondi
2020-08-24 11:34       ` Laurent Pinchart
2020-08-24 12:15         ` Jacopo Mondi
2020-08-24 12:14           ` Laurent Pinchart
2020-08-25 20:55             ` Rob Herring
2020-08-25 21:41               ` Laurent Pinchart
2020-08-21 11:26   ` Lad, Prabhakar
2020-08-25 21:03   ` Rob Herring
2020-08-18 12:20 ` [PATCH 2/3] dt-bindings: media: ov772x: Make bus-type mandatory Jacopo Mondi
2020-08-19 13:52   ` Laurent Pinchart
2020-08-21 11:29   ` Lad, Prabhakar
2020-08-25 21:06   ` Rob Herring
2020-08-18 12:20 ` [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props Jacopo Mondi
2020-08-19 13:54   ` Laurent Pinchart
2020-08-21 11:37     ` Lad, Prabhakar
2020-08-22  1:35       ` Laurent Pinchart
2020-08-26  8:42         ` Lad, Prabhakar
2020-08-24  8:35       ` Jacopo Mondi
2020-08-26  8:47         ` Lad, Prabhakar
  -- strict thread matches above, loose matches on Subject: below --
2020-08-17 15:59 [PATCH 0/3] dt-bindings: media: ov772x: Convert to json-schama Jacopo Mondi
2020-08-17 15:59 ` [PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema Jacopo Mondi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).