[PATCH 0/3] dt-bindings: media: ov772x: Convert to json-schama

[PATCH 0/3] dt-bindings: media: ov772x: Convert to json-schama

[Jacopo Mondi]

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?

Thanks
  j

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

[PATCH 1/3] dt-bindings: media: ov772x: Convert to json-schema

[Jacopo Mondi]

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

[PATCH 2/3] dt-bindings: media: ov772x: Make bus-type mandatory

[Jacopo Mondi]

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.
---
 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

[PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props

[Jacopo Mondi]

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

Re: [PATCH 2/3] dt-bindings: media: ov772x: Make bus-type mandatory

[Sergei Shtylyov]

On 8/17/20 6:59 PM, 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?

[...]

MBR, Sergei

Re: [PATCH 2/3] dt-bindings: media: ov772x: Make bus-type mandatory

[Jacopo Mondi]

Hi Sergei

On Mon, Aug 17, 2020 at 11:14:04PM +0300, Sergei Shtylyov wrote:
> On 8/17/20 6:59 PM, 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?
>

Who didn't run checkpatch on this binding path: o/

Will re-send, sorry for the fuss

> [...]
>
> MBR, Sergei

Re: [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props

[Lad, Prabhakar]

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

Re: [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props

[Lad, Prabhakar]

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

Re: [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props

[Jacopo Mondi]

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

Re: [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props

[Laurent Pinchart]

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

Re: [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props

[Lad, Prabhakar]

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

Re: [PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props

[Laurent Pinchart]

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

[PATCH 3/3] dt-bindings: media: ov772x: Document endpoint props

[Jacopo Mondi]

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