[PATCH v2 0/3] dt-bindings: media: imx7 and ov2680 updates to yaml

[PATCH v2 0/3] dt-bindings: media: imx7 and ov2680 updates to yaml

[Rui Miguel Silva]

Update bindings documentation to json-schema yaml in imx7 csi, mipi csi and
ov2680 sensor media devices along the respective MAINTAINERS entry.

Cheers,
  Rui

v1 -> v2:
  Sakari Ailus - Patch 1/3:
  https://lore.kernel.org/linux-media/20201013160908.GC13341@paasikivi.fi.intel.com/
  - omit remote-endpoint
  - remove not needed clock-lanes and data-lanes


Rui Miguel Silva (3):
  dt-bindings: ov2680: convert bindings to yaml
  dt-bindings: imx7-csi: convert bindings to yaml
  dt-bindings: imx7-mipi-csi2: convert bindings to yaml

 .../devicetree/bindings/media/i2c/ov2680.txt  |  46 -----
 .../devicetree/bindings/media/i2c/ov2680.yaml | 109 +++++++++++
 .../devicetree/bindings/media/imx7-csi.txt    |  42 ----
 .../bindings/media/imx7-mipi-csi2.txt         |  90 ---------
 .../bindings/media/nxp,imx7-csi.yaml          |  84 ++++++++
 .../bindings/media/nxp,imx7-mipi-csi2.yaml    | 181 ++++++++++++++++++
 MAINTAINERS                                   |   6 +-
 7 files changed, 377 insertions(+), 181 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.yaml
 delete mode 100644 Documentation/devicetree/bindings/media/imx7-csi.txt
 delete mode 100644 Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
 create mode 100644 Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
 create mode 100644 Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml

-- 
2.28.0

[PATCH v2 1/3] dt-bindings: ov2680: convert bindings to yaml

[Rui Miguel Silva]

Convert ov2680 sensor bindings documentation to yaml schema, remove
the textual bindings document and update MAINTAINERS entry.

Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
  
v1 -> v2:
  Sakari Ailus - Patch 1/3:
  https://lore.kernel.org/linux-media/20201013160908.GC13341@paasikivi.fi.intel.com/
  - omit remote-endpoint
  - remove not needed clock-lanes and data-lanes

 .../devicetree/bindings/media/i2c/ov2680.txt  |  46 --------
 .../devicetree/bindings/media/i2c/ov2680.yaml | 109 ++++++++++++++++++
 MAINTAINERS                                   |   2 +-
 3 files changed, 110 insertions(+), 47 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
deleted file mode 100644
index 11e925ed9dad..000000000000
--- a/Documentation/devicetree/bindings/media/i2c/ov2680.txt
+++ /dev/null
@@ -1,46 +0,0 @@
-* Omnivision OV2680 MIPI CSI-2 sensor
-
-Required Properties:
-- compatible: should be "ovti,ov2680".
-- clocks: reference to the xvclk input clock.
-- clock-names: should be "xvclk".
-- DOVDD-supply: Digital I/O voltage supply.
-- DVDD-supply: Digital core voltage supply.
-- AVDD-supply: Analog voltage supply.
-
-Optional Properties:
-- reset-gpios: reference to the GPIO connected to the powerdown/reset pin,
-               if any. This is an active low signal to the OV2680.
-
-The device node must contain one 'port' child node for its digital output
-video port, and this port must have a single endpoint in accordance with
- the video interface bindings defined in
-Documentation/devicetree/bindings/media/video-interfaces.txt.
-
-Endpoint node required properties for CSI-2 connection are:
-- remote-endpoint: a phandle to the bus receiver's endpoint node.
-- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
-- data-lanes: should be set to <1> (one CSI-2 lane supported).
-
-Example:
-
-&i2c2 {
-	ov2680: camera-sensor@36 {
-		compatible = "ovti,ov2680";
-		reg = <0x36>;
-		clocks = <&osc>;
-		clock-names = "xvclk";
-		reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
-		DOVDD-supply = <&sw2_reg>;
-		DVDD-supply = <&sw2_reg>;
-		AVDD-supply = <&reg_peri_3p15v>;
-
-		port {
-			ov2680_to_mipi: endpoint {
-				remote-endpoint = <&mipi_from_sensor>;
-				clock-lanes = <0>;
-				data-lanes = <1>;
-			};
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.yaml b/Documentation/devicetree/bindings/media/i2c/ov2680.yaml
new file mode 100644
index 000000000000..ef2b45b03dcc
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.yaml
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ov2680.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Omnivision OV2680 CMOS Sensor
+
+maintainers:
+  - Rui Miguel Silva <rmfrfs@gmail.com>
+
+description: |-
+  The OV2680 color sensor is a low voltage, high performance 1/5 inch UXGA (2
+  megapixel) CMOS image sensor that provides a single-chip UXGA (1600 x 1200)
+  camera. It provides full-frame, sub-sampled, or windowed 10-bit images in
+  various formats via the control of the Serial Camera Control Bus (SCCB)
+  interface.  The OV2680 has an image array capable of operating at up to 30
+  frames per second (fps) in UXGA resolution.
+
+properties:
+  compatible:
+    const: ovti,ov2680
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    description:
+      Input clock for the sensor.
+    items:
+      - const: xvclk
+
+  reset-gpios:
+    description:
+      The phandle and specifier for the GPIO that controls sensor reset.
+      This corresponds to the hardware pin XSHUTDOWN which is physically
+      active low.
+
+  dovdd-supply:
+    description:
+      Definition of the regulator used as interface power supply.
+
+  avdd-supply:
+    description:
+      Definition of the regulator used as analog power supply.
+
+  dvdd-supply:
+    description:
+      Definition of the regulator used as digital power supply.
+
+  port:
+    type: object
+    additionalProperties: false
+    description:
+      A node containing an output port node with an endpoint definition
+      as documented in
+      Documentation/devicetree/bindings/media/video-interfaces.txt
+
+    properties:
+      endpoint:
+        type: object
+
+    required:
+      - endpoint
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - dovdd-supply
+  - avdd-supply
+  - dvdd-supply
+  - reset-gpios
+  - port
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ov2680: camera-sensor@36 {
+                compatible = "ovti,ov2680";
+                reg = <0x36>;
+                clocks = <&osc>;
+                clock-names = "xvclk";
+                reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
+
+                dovdd-supply = <&sw2_reg>;
+                dvdd-supply = <&sw2_reg>;
+                avdd-supply = <&reg_peri_3p15v>;
+
+                port {
+                        ov2680_to_mipi: endpoint {
+                                remote-endpoint = <&mipi_from_sensor>;
+                        };
+                };
+        };
+    };
+...
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 2e85e114c9c3..926dcdc4794c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12775,7 +12775,7 @@ M:	Rui Miguel Silva <rmfrfs@gmail.com>
 L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
-F:	Documentation/devicetree/bindings/media/i2c/ov2680.txt
+F:	Documentation/devicetree/bindings/media/i2c/ov2680.yaml
 F:	drivers/media/i2c/ov2680.c
 
 OMNIVISION OV2685 SENSOR DRIVER
-- 
2.28.0

Re: [PATCH v2 1/3] dt-bindings: ov2680: convert bindings to yaml

[Jacopo Mondi]

Hi Rui,

On Wed, Oct 14, 2020 at 03:27:57PM +0100, Rui Miguel Silva wrote:
> Convert ov2680 sensor bindings documentation to yaml schema, remove
> the textual bindings document and update MAINTAINERS entry.
>
> Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
> ---
>
> v1 -> v2:
>   Sakari Ailus - Patch 1/3:
>   https://lore.kernel.org/linux-media/20201013160908.GC13341@paasikivi.fi.intel.com/
>   - omit remote-endpoint
>   - remove not needed clock-lanes and data-lanes
>
>  .../devicetree/bindings/media/i2c/ov2680.txt  |  46 --------
>  .../devicetree/bindings/media/i2c/ov2680.yaml | 109 ++++++++++++++++++
>  MAINTAINERS                                   |   2 +-
>  3 files changed, 110 insertions(+), 47 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> deleted file mode 100644
> index 11e925ed9dad..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> +++ /dev/null
> @@ -1,46 +0,0 @@
> -* Omnivision OV2680 MIPI CSI-2 sensor
> -
> -Required Properties:
> -- compatible: should be "ovti,ov2680".
> -- clocks: reference to the xvclk input clock.
> -- clock-names: should be "xvclk".
> -- DOVDD-supply: Digital I/O voltage supply.
> -- DVDD-supply: Digital core voltage supply.
> -- AVDD-supply: Analog voltage supply.
> -
> -Optional Properties:
> -- reset-gpios: reference to the GPIO connected to the powerdown/reset pin,
> -               if any. This is an active low signal to the OV2680.
> -
> -The device node must contain one 'port' child node for its digital output
> -video port, and this port must have a single endpoint in accordance with
> - the video interface bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Endpoint node required properties for CSI-2 connection are:
> -- remote-endpoint: a phandle to the bus receiver's endpoint node.
> -- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
> -- data-lanes: should be set to <1> (one CSI-2 lane supported).
> -
> -Example:
> -
> -&i2c2 {
> -	ov2680: camera-sensor@36 {
> -		compatible = "ovti,ov2680";
> -		reg = <0x36>;
> -		clocks = <&osc>;
> -		clock-names = "xvclk";
> -		reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
> -		DOVDD-supply = <&sw2_reg>;
> -		DVDD-supply = <&sw2_reg>;
> -		AVDD-supply = <&reg_peri_3p15v>;
> -
> -		port {
> -			ov2680_to_mipi: endpoint {
> -				remote-endpoint = <&mipi_from_sensor>;
> -				clock-lanes = <0>;
> -				data-lanes = <1>;
> -			};
> -		};
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.yaml b/Documentation/devicetree/bindings/media/i2c/ov2680.yaml

Might this be a good occasion to rename the file to ovti,ov2680.yaml ?

> new file mode 100644
> index 000000000000..ef2b45b03dcc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ov2680.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Omnivision OV2680 CMOS Sensor
> +
> +maintainers:
> +  - Rui Miguel Silva <rmfrfs@gmail.com>
> +
> +description: |-
> +  The OV2680 color sensor is a low voltage, high performance 1/5 inch UXGA (2
> +  megapixel) CMOS image sensor that provides a single-chip UXGA (1600 x 1200)
> +  camera. It provides full-frame, sub-sampled, or windowed 10-bit images in
> +  various formats via the control of the Serial Camera Control Bus (SCCB)
> +  interface.  The OV2680 has an image array capable of operating at up to 30
                ^ double space

> +  frames per second (fps) in UXGA resolution.
> +
> +properties:
> +  compatible:
> +    const: ovti,ov2680
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    description:

I'll never get yaml right, doesn't breaking lines require '|' after
the semicolon ? The validator does not complain, so I guess not.

> +      Input clock for the sensor.
> +    items:
> +      - const: xvclk
> +
> +  reset-gpios:
> +    description:
> +      The phandle and specifier for the GPIO that controls sensor reset.
> +      This corresponds to the hardware pin XSHUTDOWN which is physically
> +      active low.
> +
> +  dovdd-supply:
> +    description:
> +      Definition of the regulator used as interface power supply.
> +
> +  avdd-supply:
> +    description:
> +      Definition of the regulator used as analog power supply.
> +
> +  dvdd-supply:
> +    description:
> +      Definition of the regulator used as digital power supply.
> +
> +  port:
> +    type: object
> +    additionalProperties: false
> +    description:
> +      A node containing an output port node with an endpoint definition
> +      as documented in
> +      Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +    properties:
> +      endpoint:
> +        type: object
> +
> +    required:
> +      - endpoint

If no endpoint properties are specified, the last 6 lines here can be
omitted. The rationale is that 'port' will be validated against a
forthcoming 'of-graph.yaml' schema. So just:

   port:
     type: object
     additionalProperties: false
     description:
       A node containing an output port node with an endpoint definition
       as documented in
       Documentation/devicetree/bindings/media/video-interfaces.txt

With 'port' listed as mandatory, as you do already.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - dovdd-supply
> +  - avdd-supply
> +  - dvdd-supply
> +  - reset-gpios
> +  - port
> +
> +unevaluatedProperties: false

'additionalProperties: false' too ?

> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ov2680: camera-sensor@36 {
> +                compatible = "ovti,ov2680";
> +                reg = <0x36>;
> +                clocks = <&osc>;
> +                clock-names = "xvclk";
> +                reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
> +
> +                dovdd-supply = <&sw2_reg>;
> +                dvdd-supply = <&sw2_reg>;
> +                avdd-supply = <&reg_peri_3p15v>;
> +
> +                port {
> +                        ov2680_to_mipi: endpoint {
> +                                remote-endpoint = <&mipi_from_sensor>;
> +                        };
> +                };
> +        };
> +    };
> +...
> +

Applying the patch gives me:
.git/rebase-apply/patch:182: new blank line at EOF.

I see most bindings have an empty line before '...'

With this small issues fixed:
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks
   j

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2e85e114c9c3..926dcdc4794c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12775,7 +12775,7 @@ M:	Rui Miguel Silva <rmfrfs@gmail.com>
>  L:	linux-media@vger.kernel.org
>  S:	Maintained
>  T:	git git://linuxtv.org/media_tree.git
> -F:	Documentation/devicetree/bindings/media/i2c/ov2680.txt
> +F:	Documentation/devicetree/bindings/media/i2c/ov2680.yaml
>  F:	drivers/media/i2c/ov2680.c
>
>  OMNIVISION OV2685 SENSOR DRIVER
> --
> 2.28.0
>

Re: [PATCH v2 1/3] dt-bindings: ov2680: convert bindings to yaml

[Rui Miguel Silva]

Hey Jacopo,
Thanks for the review.

On Thu, Oct 15, 2020 at 04:49:05PM +0200, Jacopo Mondi wrote:
> Hi Rui,
> 
> On Wed, Oct 14, 2020 at 03:27:57PM +0100, Rui Miguel Silva wrote:
> > Convert ov2680 sensor bindings documentation to yaml schema, remove
> > the textual bindings document and update MAINTAINERS entry.
> >
> > Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
> > ---
> >
> > v1 -> v2:
> >   Sakari Ailus - Patch 1/3:
> >   https://lore.kernel.org/linux-media/20201013160908.GC13341@paasikivi.fi.intel.com/
> >   - omit remote-endpoint
> >   - remove not needed clock-lanes and data-lanes
> >
> >  .../devicetree/bindings/media/i2c/ov2680.txt  |  46 --------
> >  .../devicetree/bindings/media/i2c/ov2680.yaml | 109 ++++++++++++++++++
> >  MAINTAINERS                                   |   2 +-
> >  3 files changed, 110 insertions(+), 47 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> > deleted file mode 100644
> > index 11e925ed9dad..000000000000
> > --- a/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> > +++ /dev/null
> > @@ -1,46 +0,0 @@
> > -* Omnivision OV2680 MIPI CSI-2 sensor
> > -
> > -Required Properties:
> > -- compatible: should be "ovti,ov2680".
> > -- clocks: reference to the xvclk input clock.
> > -- clock-names: should be "xvclk".
> > -- DOVDD-supply: Digital I/O voltage supply.
> > -- DVDD-supply: Digital core voltage supply.
> > -- AVDD-supply: Analog voltage supply.
> > -
> > -Optional Properties:
> > -- reset-gpios: reference to the GPIO connected to the powerdown/reset pin,
> > -               if any. This is an active low signal to the OV2680.
> > -
> > -The device node must contain one 'port' child node for its digital output
> > -video port, and this port must have a single endpoint in accordance with
> > - the video interface bindings defined in
> > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > -
> > -Endpoint node required properties for CSI-2 connection are:
> > -- remote-endpoint: a phandle to the bus receiver's endpoint node.
> > -- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
> > -- data-lanes: should be set to <1> (one CSI-2 lane supported).
> > -
> > -Example:
> > -
> > -&i2c2 {
> > -	ov2680: camera-sensor@36 {
> > -		compatible = "ovti,ov2680";
> > -		reg = <0x36>;
> > -		clocks = <&osc>;
> > -		clock-names = "xvclk";
> > -		reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
> > -		DOVDD-supply = <&sw2_reg>;
> > -		DVDD-supply = <&sw2_reg>;
> > -		AVDD-supply = <&reg_peri_3p15v>;
> > -
> > -		port {
> > -			ov2680_to_mipi: endpoint {
> > -				remote-endpoint = <&mipi_from_sensor>;
> > -				clock-lanes = <0>;
> > -				data-lanes = <1>;
> > -			};
> > -		};
> > -	};
> > -};
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.yaml b/Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> 
> Might this be a good occasion to rename the file to ovti,ov2680.yaml ?

Yeah, was just following your precedent in ov5647 and the other ov
already ported to yaml in the directory. But I agree that it makes
sense to rename the files.

> 
> > new file mode 100644
> > index 000000000000..ef2b45b03dcc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> > @@ -0,0 +1,109 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/ov2680.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Omnivision OV2680 CMOS Sensor
> > +
> > +maintainers:
> > +  - Rui Miguel Silva <rmfrfs@gmail.com>
> > +
> > +description: |-
> > +  The OV2680 color sensor is a low voltage, high performance 1/5 inch UXGA (2
> > +  megapixel) CMOS image sensor that provides a single-chip UXGA (1600 x 1200)
> > +  camera. It provides full-frame, sub-sampled, or windowed 10-bit images in
> > +  various formats via the control of the Serial Camera Control Bus (SCCB)
> > +  interface.  The OV2680 has an image array capable of operating at up to 30
>                 ^ double space
> 
> > +  frames per second (fps) in UXGA resolution.
> > +
> > +properties:
> > +  compatible:
> > +    const: ovti,ov2680
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    description:
> 
> I'll never get yaml right, doesn't breaking lines require '|' after
> the semicolon ? The validator does not complain, so I guess not.

I also had that idea, but looking also to other cases, and also in the
examlpe-schema where you have both cases, looks like it is not needed.

> 
> > +      Input clock for the sensor.
> > +    items:
> > +      - const: xvclk
> > +
> > +  reset-gpios:
> > +    description:
> > +      The phandle and specifier for the GPIO that controls sensor reset.
> > +      This corresponds to the hardware pin XSHUTDOWN which is physically
> > +      active low.
> > +
> > +  dovdd-supply:
> > +    description:
> > +      Definition of the regulator used as interface power supply.
> > +
> > +  avdd-supply:
> > +    description:
> > +      Definition of the regulator used as analog power supply.
> > +
> > +  dvdd-supply:
> > +    description:
> > +      Definition of the regulator used as digital power supply.
> > +
> > +  port:
> > +    type: object
> > +    additionalProperties: false
> > +    description:
> > +      A node containing an output port node with an endpoint definition
> > +      as documented in
> > +      Documentation/devicetree/bindings/media/video-interfaces.txt
> > +
> > +    properties:
> > +      endpoint:
> > +        type: object
> > +
> > +    required:
> > +      - endpoint
> 
> If no endpoint properties are specified, the last 6 lines here can be
> omitted. The rationale is that 'port' will be validated against a
> forthcoming 'of-graph.yaml' schema. So just:
> 
>    port:
>      type: object
>      additionalProperties: false
>      description:
>        A node containing an output port node with an endpoint definition
>        as documented in
>        Documentation/devicetree/bindings/media/video-interfaces.txt
> 
> With 'port' listed as mandatory, as you do already.

you are right, will fix this.

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - dovdd-supply
> > +  - avdd-supply
> > +  - dvdd-supply
> > +  - reset-gpios
> > +  - port
> > +
> > +unevaluatedProperties: false
> 
> 'additionalProperties: false' too ?

instead.

> 
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        ov2680: camera-sensor@36 {
> > +                compatible = "ovti,ov2680";
> > +                reg = <0x36>;
> > +                clocks = <&osc>;
> > +                clock-names = "xvclk";
> > +                reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
> > +
> > +                dovdd-supply = <&sw2_reg>;
> > +                dvdd-supply = <&sw2_reg>;
> > +                avdd-supply = <&reg_peri_3p15v>;
> > +
> > +                port {
> > +                        ov2680_to_mipi: endpoint {
> > +                                remote-endpoint = <&mipi_from_sensor>;
> > +                        };
> > +                };
> > +        };
> > +    };
> > +...
> > +
> 
> Applying the patch gives me:
> .git/rebase-apply/patch:182: new blank line at EOF.
> 
> I see most bindings have an empty line before '...'
> 
> With this small issues fixed:
> Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
 
 Thanks again Jacopo.

 Cheers,
    Rui
> 
> Thanks
>    j
> 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 2e85e114c9c3..926dcdc4794c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -12775,7 +12775,7 @@ M:	Rui Miguel Silva <rmfrfs@gmail.com>
> >  L:	linux-media@vger.kernel.org
> >  S:	Maintained
> >  T:	git git://linuxtv.org/media_tree.git
> > -F:	Documentation/devicetree/bindings/media/i2c/ov2680.txt
> > +F:	Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> >  F:	drivers/media/i2c/ov2680.c
> >
> >  OMNIVISION OV2685 SENSOR DRIVER
> > --
> > 2.28.0
> >

Re: [PATCH v2 1/3] dt-bindings: ov2680: convert bindings to yaml

[Rob Herring]

On Fri, Oct 16, 2020 at 03:42:04PM +0100, Rui Miguel Silva wrote:
> Hey Jacopo,
> Thanks for the review.
> 
> On Thu, Oct 15, 2020 at 04:49:05PM +0200, Jacopo Mondi wrote:
> > Hi Rui,
> > 
> > On Wed, Oct 14, 2020 at 03:27:57PM +0100, Rui Miguel Silva wrote:
> > > Convert ov2680 sensor bindings documentation to yaml schema, remove
> > > the textual bindings document and update MAINTAINERS entry.
> > >
> > > Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>

> > > +  clock-names:
> > > +    description:
> > 
> > I'll never get yaml right, doesn't breaking lines require '|' after
> > the semicolon ? The validator does not complain, so I guess not.
> 
> I also had that idea, but looking also to other cases, and also in the
> examlpe-schema where you have both cases, looks like it is not needed.

'|' will preserve line breaks and formatting. For a single line like 
this it doesn't really matter. Though ruamel's round trip will make it a 
single line when writing back out. 

> > 
> > > +      Input clock for the sensor.

Really, you can just drop the description. Doesn't really add anything 
specific for this device.

> > > +    items:
> > > +      - const: xvclk

Re: [PATCH v2 1/3] dt-bindings: ov2680: convert bindings to yaml

[Jacopo Mondi]

Hi Rob,

On Mon, Oct 19, 2020 at 03:33:59PM -0500, Rob Herring wrote:
> On Fri, Oct 16, 2020 at 03:42:04PM +0100, Rui Miguel Silva wrote:
> > Hey Jacopo,
> > Thanks for the review.
> >
> > On Thu, Oct 15, 2020 at 04:49:05PM +0200, Jacopo Mondi wrote:
> > > Hi Rui,
> > >
> > > On Wed, Oct 14, 2020 at 03:27:57PM +0100, Rui Miguel Silva wrote:
> > > > Convert ov2680 sensor bindings documentation to yaml schema, remove
> > > > the textual bindings document and update MAINTAINERS entry.
> > > >
> > > > Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
>
> > > > +  clock-names:
> > > > +    description:
> > >
> > > I'll never get yaml right, doesn't breaking lines require '|' after
> > > the semicolon ? The validator does not complain, so I guess not.
> >
> > I also had that idea, but looking also to other cases, and also in the
> > examlpe-schema where you have both cases, looks like it is not needed.
>
> '|' will preserve line breaks and formatting. For a single line like
> this it doesn't really matter. Though ruamel's round trip will make it a
> single line when writing back out.

Thanks for the explanation.

I'll take the occasion to ask the difference between '|', '|-' and
'-|' as I haven't find it documented anywhere.

Thanks
  j

>
> > >
> > > > +      Input clock for the sensor.
>
> Really, you can just drop the description. Doesn't really add anything
> specific for this device.
>
> > > > +    items:
> > > > +      - const: xvclk

Re: [PATCH v2 1/3] dt-bindings: ov2680: convert bindings to yaml

[Rob Herring]

On Wed, Oct 21, 2020 at 9:52 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi Rob,
>
> On Mon, Oct 19, 2020 at 03:33:59PM -0500, Rob Herring wrote:
> > On Fri, Oct 16, 2020 at 03:42:04PM +0100, Rui Miguel Silva wrote:
> > > Hey Jacopo,
> > > Thanks for the review.
> > >
> > > On Thu, Oct 15, 2020 at 04:49:05PM +0200, Jacopo Mondi wrote:
> > > > Hi Rui,
> > > >
> > > > On Wed, Oct 14, 2020 at 03:27:57PM +0100, Rui Miguel Silva wrote:
> > > > > Convert ov2680 sensor bindings documentation to yaml schema, remove
> > > > > the textual bindings document and update MAINTAINERS entry.
> > > > >
> > > > > Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
> >
> > > > > +  clock-names:
> > > > > +    description:
> > > >
> > > > I'll never get yaml right, doesn't breaking lines require '|' after
> > > > the semicolon ? The validator does not complain, so I guess not.
> > >
> > > I also had that idea, but looking also to other cases, and also in the
> > > examlpe-schema where you have both cases, looks like it is not needed.
> >
> > '|' will preserve line breaks and formatting. For a single line like
> > this it doesn't really matter. Though ruamel's round trip will make it a
> > single line when writing back out.
>
> Thanks for the explanation.
>
> I'll take the occasion to ask the difference between '|', '|-' and
> '-|' as I haven't find it documented anywhere.

https://yaml-multiline.info/

'-|' is not valid AFAIK. Did you mean '- |'?. That's a list entry
which then contains a multi-line string.

Rob

Re: [PATCH v2 1/3] dt-bindings: ov2680: convert bindings to yaml

[Rob Herring]

On Wed, Oct 14, 2020 at 03:27:57PM +0100, Rui Miguel Silva wrote:
> Convert ov2680 sensor bindings documentation to yaml schema, remove
> the textual bindings document and update MAINTAINERS entry.
> 
> Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
> ---
>   
> v1 -> v2:
>   Sakari Ailus - Patch 1/3:
>   https://lore.kernel.org/linux-media/20201013160908.GC13341@paasikivi.fi.intel.com/
>   - omit remote-endpoint
>   - remove not needed clock-lanes and data-lanes
> 
>  .../devicetree/bindings/media/i2c/ov2680.txt  |  46 --------
>  .../devicetree/bindings/media/i2c/ov2680.yaml | 109 ++++++++++++++++++
>  MAINTAINERS                                   |   2 +-
>  3 files changed, 110 insertions(+), 47 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> deleted file mode 100644
> index 11e925ed9dad..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> +++ /dev/null
> @@ -1,46 +0,0 @@
> -* Omnivision OV2680 MIPI CSI-2 sensor
> -
> -Required Properties:
> -- compatible: should be "ovti,ov2680".
> -- clocks: reference to the xvclk input clock.
> -- clock-names: should be "xvclk".
> -- DOVDD-supply: Digital I/O voltage supply.
> -- DVDD-supply: Digital core voltage supply.
> -- AVDD-supply: Analog voltage supply.
> -
> -Optional Properties:
> -- reset-gpios: reference to the GPIO connected to the powerdown/reset pin,
> -               if any. This is an active low signal to the OV2680.
> -
> -The device node must contain one 'port' child node for its digital output
> -video port, and this port must have a single endpoint in accordance with
> - the video interface bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Endpoint node required properties for CSI-2 connection are:
> -- remote-endpoint: a phandle to the bus receiver's endpoint node.
> -- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
> -- data-lanes: should be set to <1> (one CSI-2 lane supported).
> -
> -Example:
> -
> -&i2c2 {
> -	ov2680: camera-sensor@36 {
> -		compatible = "ovti,ov2680";
> -		reg = <0x36>;
> -		clocks = <&osc>;
> -		clock-names = "xvclk";
> -		reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
> -		DOVDD-supply = <&sw2_reg>;
> -		DVDD-supply = <&sw2_reg>;
> -		AVDD-supply = <&reg_peri_3p15v>;
> -
> -		port {
> -			ov2680_to_mipi: endpoint {
> -				remote-endpoint = <&mipi_from_sensor>;
> -				clock-lanes = <0>;
> -				data-lanes = <1>;
> -			};
> -		};
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.yaml b/Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> new file mode 100644
> index 000000000000..ef2b45b03dcc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ov2680.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Omnivision OV2680 CMOS Sensor
> +
> +maintainers:
> +  - Rui Miguel Silva <rmfrfs@gmail.com>
> +
> +description: |-
> +  The OV2680 color sensor is a low voltage, high performance 1/5 inch UXGA (2
> +  megapixel) CMOS image sensor that provides a single-chip UXGA (1600 x 1200)
> +  camera. It provides full-frame, sub-sampled, or windowed 10-bit images in
> +  various formats via the control of the Serial Camera Control Bus (SCCB)
> +  interface.  The OV2680 has an image array capable of operating at up to 30
> +  frames per second (fps) in UXGA resolution.
> +
> +properties:
> +  compatible:
> +    const: ovti,ov2680
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    description:
> +      Input clock for the sensor.
> +    items:
> +      - const: xvclk
> +
> +  reset-gpios:

How many? (maxItems: 1)

> +    description:
> +      The phandle and specifier for the GPIO that controls sensor reset.
> +      This corresponds to the hardware pin XSHUTDOWN which is physically
> +      active low.
> +
> +  dovdd-supply:
> +    description:
> +      Definition of the regulator used as interface power supply.
> +
> +  avdd-supply:
> +    description:
> +      Definition of the regulator used as analog power supply.
> +
> +  dvdd-supply:
> +    description:
> +      Definition of the regulator used as digital power supply.
> +
> +  port:
> +    type: object
> +    additionalProperties: false
> +    description:
> +      A node containing an output port node with an endpoint definition
> +      as documented in
> +      Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +    properties:
> +      endpoint:
> +        type: object
> +
> +    required:
> +      - endpoint

Just need a description of the data/direction for 'port'. Drop the rest.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - dovdd-supply
> +  - avdd-supply
> +  - dvdd-supply
> +  - reset-gpios
> +  - port
> +
> +unevaluatedProperties: false

You can use 'additionalProperties: false' here instead.

> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ov2680: camera-sensor@36 {
> +                compatible = "ovti,ov2680";
> +                reg = <0x36>;
> +                clocks = <&osc>;
> +                clock-names = "xvclk";
> +                reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
> +
> +                dovdd-supply = <&sw2_reg>;
> +                dvdd-supply = <&sw2_reg>;
> +                avdd-supply = <&reg_peri_3p15v>;
> +
> +                port {
> +                        ov2680_to_mipi: endpoint {
> +                                remote-endpoint = <&mipi_from_sensor>;
> +                        };
> +                };
> +        };
> +    };
> +...
> +
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2e85e114c9c3..926dcdc4794c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12775,7 +12775,7 @@ M:	Rui Miguel Silva <rmfrfs@gmail.com>
>  L:	linux-media@vger.kernel.org
>  S:	Maintained
>  T:	git git://linuxtv.org/media_tree.git
> -F:	Documentation/devicetree/bindings/media/i2c/ov2680.txt
> +F:	Documentation/devicetree/bindings/media/i2c/ov2680.yaml
>  F:	drivers/media/i2c/ov2680.c
>  
>  OMNIVISION OV2685 SENSOR DRIVER
> -- 
> 2.28.0
> 

Re: [PATCH v2 1/3] dt-bindings: ov2680: convert bindings to yaml

[Rui Miguel Silva]

Hey Rob,
On Mon, Oct 19, 2020 at 03:39:10PM -0500, Rob Herring wrote:
> On Wed, Oct 14, 2020 at 03:27:57PM +0100, Rui Miguel Silva wrote:
> > Convert ov2680 sensor bindings documentation to yaml schema, remove
> > the textual bindings document and update MAINTAINERS entry.
> > 
> > Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
> > ---
> >   
> > v1 -> v2:
> >   Sakari Ailus - Patch 1/3:
> >   https://lore.kernel.org/linux-media/20201013160908.GC13341@paasikivi.fi.intel.com/
> >   - omit remote-endpoint
> >   - remove not needed clock-lanes and data-lanes

There was already a V3 on the list when you reviewed this version,
Jacopo had already made some of the same comments as you. So,
some are already solved in that version.

I will send a v4 please just jump to that version.
Many thanks,

------
Cheers,
     Rui
> > 
> >  .../devicetree/bindings/media/i2c/ov2680.txt  |  46 --------
> >  .../devicetree/bindings/media/i2c/ov2680.yaml | 109 ++++++++++++++++++
> >  MAINTAINERS                                   |   2 +-
> >  3 files changed, 110 insertions(+), 47 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> > deleted file mode 100644
> > index 11e925ed9dad..000000000000
> > --- a/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> > +++ /dev/null
> > @@ -1,46 +0,0 @@
> > -* Omnivision OV2680 MIPI CSI-2 sensor
> > -
> > -Required Properties:
> > -- compatible: should be "ovti,ov2680".
> > -- clocks: reference to the xvclk input clock.
> > -- clock-names: should be "xvclk".
> > -- DOVDD-supply: Digital I/O voltage supply.
> > -- DVDD-supply: Digital core voltage supply.
> > -- AVDD-supply: Analog voltage supply.
> > -
> > -Optional Properties:
> > -- reset-gpios: reference to the GPIO connected to the powerdown/reset pin,
> > -               if any. This is an active low signal to the OV2680.
> > -
> > -The device node must contain one 'port' child node for its digital output
> > -video port, and this port must have a single endpoint in accordance with
> > - the video interface bindings defined in
> > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > -
> > -Endpoint node required properties for CSI-2 connection are:
> > -- remote-endpoint: a phandle to the bus receiver's endpoint node.
> > -- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
> > -- data-lanes: should be set to <1> (one CSI-2 lane supported).
> > -
> > -Example:
> > -
> > -&i2c2 {
> > -	ov2680: camera-sensor@36 {
> > -		compatible = "ovti,ov2680";
> > -		reg = <0x36>;
> > -		clocks = <&osc>;
> > -		clock-names = "xvclk";
> > -		reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
> > -		DOVDD-supply = <&sw2_reg>;
> > -		DVDD-supply = <&sw2_reg>;
> > -		AVDD-supply = <&reg_peri_3p15v>;
> > -
> > -		port {
> > -			ov2680_to_mipi: endpoint {
> > -				remote-endpoint = <&mipi_from_sensor>;
> > -				clock-lanes = <0>;
> > -				data-lanes = <1>;
> > -			};
> > -		};
> > -	};
> > -};
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.yaml b/Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> > new file mode 100644
> > index 000000000000..ef2b45b03dcc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> > @@ -0,0 +1,109 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/ov2680.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Omnivision OV2680 CMOS Sensor
> > +
> > +maintainers:
> > +  - Rui Miguel Silva <rmfrfs@gmail.com>
> > +
> > +description: |-
> > +  The OV2680 color sensor is a low voltage, high performance 1/5 inch UXGA (2
> > +  megapixel) CMOS image sensor that provides a single-chip UXGA (1600 x 1200)
> > +  camera. It provides full-frame, sub-sampled, or windowed 10-bit images in
> > +  various formats via the control of the Serial Camera Control Bus (SCCB)
> > +  interface.  The OV2680 has an image array capable of operating at up to 30
> > +  frames per second (fps) in UXGA resolution.
> > +
> > +properties:
> > +  compatible:
> > +    const: ovti,ov2680
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    description:
> > +      Input clock for the sensor.
> > +    items:
> > +      - const: xvclk
> > +
> > +  reset-gpios:
> 
> How many? (maxItems: 1)
> 
> > +    description:
> > +      The phandle and specifier for the GPIO that controls sensor reset.
> > +      This corresponds to the hardware pin XSHUTDOWN which is physically
> > +      active low.
> > +
> > +  dovdd-supply:
> > +    description:
> > +      Definition of the regulator used as interface power supply.
> > +
> > +  avdd-supply:
> > +    description:
> > +      Definition of the regulator used as analog power supply.
> > +
> > +  dvdd-supply:
> > +    description:
> > +      Definition of the regulator used as digital power supply.
> > +
> > +  port:
> > +    type: object
> > +    additionalProperties: false
> > +    description:
> > +      A node containing an output port node with an endpoint definition
> > +      as documented in
> > +      Documentation/devicetree/bindings/media/video-interfaces.txt
> > +
> > +    properties:
> > +      endpoint:
> > +        type: object
> > +
> > +    required:
> > +      - endpoint
> 
> Just need a description of the data/direction for 'port'. Drop the rest.
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - dovdd-supply
> > +  - avdd-supply
> > +  - dvdd-supply
> > +  - reset-gpios
> > +  - port
> > +
> > +unevaluatedProperties: false
> 
> You can use 'additionalProperties: false' here instead.
> 
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        ov2680: camera-sensor@36 {
> > +                compatible = "ovti,ov2680";
> > +                reg = <0x36>;
> > +                clocks = <&osc>;
> > +                clock-names = "xvclk";
> > +                reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
> > +
> > +                dovdd-supply = <&sw2_reg>;
> > +                dvdd-supply = <&sw2_reg>;
> > +                avdd-supply = <&reg_peri_3p15v>;
> > +
> > +                port {
> > +                        ov2680_to_mipi: endpoint {
> > +                                remote-endpoint = <&mipi_from_sensor>;
> > +                        };
> > +                };
> > +        };
> > +    };
> > +...
> > +
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 2e85e114c9c3..926dcdc4794c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -12775,7 +12775,7 @@ M:	Rui Miguel Silva <rmfrfs@gmail.com>
> >  L:	linux-media@vger.kernel.org
> >  S:	Maintained
> >  T:	git git://linuxtv.org/media_tree.git
> > -F:	Documentation/devicetree/bindings/media/i2c/ov2680.txt
> > +F:	Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> >  F:	drivers/media/i2c/ov2680.c
> >  
> >  OMNIVISION OV2685 SENSOR DRIVER
> > -- 
> > 2.28.0
> > 

[PATCH v2 2/3] dt-bindings: imx7-csi: convert bindings to yaml

[Rui Miguel Silva]

Convert imx7-csi bindings documentation to yaml schema, remove the
textual bindings document and update MAINTAINERS entry.

Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 .../devicetree/bindings/media/imx7-csi.txt    | 42 ----------
 .../bindings/media/nxp,imx7-csi.yaml          | 84 +++++++++++++++++++
 MAINTAINERS                                   |  2 +-
 3 files changed, 85 insertions(+), 43 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/imx7-csi.txt
 create mode 100644 Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml

diff --git a/Documentation/devicetree/bindings/media/imx7-csi.txt b/Documentation/devicetree/bindings/media/imx7-csi.txt
deleted file mode 100644
index d80ceefa0c00..000000000000
--- a/Documentation/devicetree/bindings/media/imx7-csi.txt
+++ /dev/null
@@ -1,42 +0,0 @@
-Freescale i.MX7 CMOS Sensor Interface
-=====================================
-
-csi node
---------
-
-This is device node for the CMOS Sensor Interface (CSI) which enables the chip
-to connect directly to external CMOS image sensors.
-
-Required properties:
-
-- compatible    : "fsl,imx7-csi" or "fsl,imx6ul-csi";
-- reg           : base address and length of the register set for the device;
-- interrupts    : should contain CSI interrupt;
-- clocks        : list of clock specifiers, see
-        Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
-- clock-names   : must contain "mclk";
-
-The device node shall contain one 'port' child node with one child 'endpoint'
-node, according to the bindings defined in:
-Documentation/devicetree/bindings/media/video-interfaces.txt.
-
-In the following example a remote endpoint is a video multiplexer.
-
-example:
-
-                csi: csi@30710000 {
-                        #address-cells = <1>;
-                        #size-cells = <0>;
-
-                        compatible = "fsl,imx7-csi";
-                        reg = <0x30710000 0x10000>;
-                        interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
-                        clocks = <&clks IMX7D_CSI_MCLK_ROOT_CLK>;
-                        clock-names = "mclk";
-
-                        port {
-                                csi_from_csi_mux: endpoint {
-                                        remote-endpoint = <&csi_mux_to_csi>;
-                                };
-                        };
-                };
diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
new file mode 100644
index 000000000000..9fe064dd5ba3
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/nxp,imx7-csi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX7 CMOS Sensor Interface
+
+maintainers:
+  - Rui Miguel Silva <rmfrfs@gmail.com>
+
+description: |
+  This is device node for the CMOS Sensor Interface (CSI) which enables the
+  chip to connect directly to external CMOS image sensors.
+
+properties:
+  compatible:
+    enum:
+      - fsl,imx7-csi
+      - fsl,imx6ul-csi
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: mclk
+
+  port:
+    type: object
+    additionalProperties: false
+    description:
+      A node containing input port nodes with endpoint definitions as documented
+      in Documentation/devicetree/bindings/media/video-interfaces.txt
+
+    properties:
+      endpoint:
+        type: object
+        additionalProperties: false
+
+        properties:
+          remote-endpoint: true
+
+        required:
+          - remote-endpoint
+
+    required:
+      - endpoint
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - port
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx7d-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    csi: csi@30710000 {
+            compatible = "fsl,imx7-csi";
+            reg = <0x30710000 0x10000>;
+            interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&clks IMX7D_CSI_MCLK_ROOT_CLK>;
+            clock-names = "mclk";
+
+            port {
+                    csi_from_csi_mux: endpoint {
+                            remote-endpoint = <&csi_mux_to_csi>;
+                    };
+            };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 926dcdc4794c..b7f7f14cd85b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10773,8 +10773,8 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
 F:	Documentation/admin-guide/media/imx7.rst
-F:	Documentation/devicetree/bindings/media/imx7-csi.txt
 F:	Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
+F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
 F:	drivers/staging/media/imx/imx7-media-csi.c
 F:	drivers/staging/media/imx/imx7-mipi-csis.c
 
-- 
2.28.0

Re: [PATCH v2 2/3] dt-bindings: imx7-csi: convert bindings to yaml

[Jacopo Mondi]

Hi Rui,

On Wed, Oct 14, 2020 at 03:27:58PM +0100, Rui Miguel Silva wrote:
> Convert imx7-csi bindings documentation to yaml schema, remove the
> textual bindings document and update MAINTAINERS entry.
>
> Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
> ---
>  .../devicetree/bindings/media/imx7-csi.txt    | 42 ----------
>  .../bindings/media/nxp,imx7-csi.yaml          | 84 +++++++++++++++++++
>  MAINTAINERS                                   |  2 +-
>  3 files changed, 85 insertions(+), 43 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/imx7-csi.txt
>  create mode 100644 Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/imx7-csi.txt b/Documentation/devicetree/bindings/media/imx7-csi.txt
> deleted file mode 100644
> index d80ceefa0c00..000000000000
> --- a/Documentation/devicetree/bindings/media/imx7-csi.txt
> +++ /dev/null
> @@ -1,42 +0,0 @@
> -Freescale i.MX7 CMOS Sensor Interface
> -=====================================
> -
> -csi node
> ---------
> -
> -This is device node for the CMOS Sensor Interface (CSI) which enables the chip
> -to connect directly to external CMOS image sensors.
> -
> -Required properties:
> -
> -- compatible    : "fsl,imx7-csi" or "fsl,imx6ul-csi";
> -- reg           : base address and length of the register set for the device;
> -- interrupts    : should contain CSI interrupt;
> -- clocks        : list of clock specifiers, see
> -        Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
> -- clock-names   : must contain "mclk";
> -
> -The device node shall contain one 'port' child node with one child 'endpoint'
> -node, according to the bindings defined in:
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -In the following example a remote endpoint is a video multiplexer.
> -
> -example:
> -
> -                csi: csi@30710000 {
> -                        #address-cells = <1>;
> -                        #size-cells = <0>;
> -
> -                        compatible = "fsl,imx7-csi";
> -                        reg = <0x30710000 0x10000>;
> -                        interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> -                        clocks = <&clks IMX7D_CSI_MCLK_ROOT_CLK>;
> -                        clock-names = "mclk";
> -
> -                        port {
> -                                csi_from_csi_mux: endpoint {
> -                                        remote-endpoint = <&csi_mux_to_csi>;
> -                                };
> -                        };
> -                };
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> new file mode 100644
> index 000000000000..9fe064dd5ba3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> @@ -0,0 +1,84 @@
> +# SPDX-License-Identifier: GPL-2.0

Shouldn't bindings be dual licensed ?

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/nxp,imx7-csi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: i.MX7 CMOS Sensor Interface
> +
> +maintainers:
> +  - Rui Miguel Silva <rmfrfs@gmail.com>
> +
> +description: |
> +  This is device node for the CMOS Sensor Interface (CSI) which enables the
> +  chip to connect directly to external CMOS image sensors.

Pretty cryptic, not your fault as it was there already. Is NXP using
CSI as a name but it's not really MIPI CSI-2 ? This seems to be a
bridge, right ?

> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,imx7-csi
> +      - fsl,imx6ul-csi
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: mclk
> +
> +  port:
> +    type: object
> +    additionalProperties: false
> +    description:
> +      A node containing input port nodes with endpoint definitions as documented
> +      in Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +    properties:
> +      endpoint:
> +        type: object
> +        additionalProperties: false
> +
> +        properties:
> +          remote-endpoint: true
> +
> +        required:
> +          - remote-endpoint
> +
> +    required:
> +      - endpoint

As per the comment on ov2680, this last part can be removed

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - port
> +
> +unevaluatedProperties: false

additionalProperties: false ?

This apart
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks
  j

> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx7d-clock.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    csi: csi@30710000 {
> +            compatible = "fsl,imx7-csi";
> +            reg = <0x30710000 0x10000>;
> +            interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&clks IMX7D_CSI_MCLK_ROOT_CLK>;
> +            clock-names = "mclk";
> +
> +            port {
> +                    csi_from_csi_mux: endpoint {
> +                            remote-endpoint = <&csi_mux_to_csi>;
> +                    };
> +            };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 926dcdc4794c..b7f7f14cd85b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10773,8 +10773,8 @@ L:	linux-media@vger.kernel.org
>  S:	Maintained
>  T:	git git://linuxtv.org/media_tree.git
>  F:	Documentation/admin-guide/media/imx7.rst
> -F:	Documentation/devicetree/bindings/media/imx7-csi.txt
>  F:	Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> +F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
>  F:	drivers/staging/media/imx/imx7-media-csi.c
>  F:	drivers/staging/media/imx/imx7-mipi-csis.c
>
> --
> 2.28.0
>

Re: [PATCH v2 2/3] dt-bindings: imx7-csi: convert bindings to yaml

[Rui Miguel Silva]

Hi Jacopo,

On Thu, Oct 15, 2020 at 05:25:46PM +0200, Jacopo Mondi wrote:
> Hi Rui,
> 
> On Wed, Oct 14, 2020 at 03:27:58PM +0100, Rui Miguel Silva wrote:
> > Convert imx7-csi bindings documentation to yaml schema, remove the
> > textual bindings document and update MAINTAINERS entry.
> >
> > Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
> > ---
> >  .../devicetree/bindings/media/imx7-csi.txt    | 42 ----------
> >  .../bindings/media/nxp,imx7-csi.yaml          | 84 +++++++++++++++++++
> >  MAINTAINERS                                   |  2 +-
> >  3 files changed, 85 insertions(+), 43 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/media/imx7-csi.txt
> >  create mode 100644 Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/imx7-csi.txt b/Documentation/devicetree/bindings/media/imx7-csi.txt
> > deleted file mode 100644
> > index d80ceefa0c00..000000000000
> > --- a/Documentation/devicetree/bindings/media/imx7-csi.txt
> > +++ /dev/null
> > @@ -1,42 +0,0 @@
> > -Freescale i.MX7 CMOS Sensor Interface
> > -=====================================
> > -
> > -csi node
> > ---------
> > -
> > -This is device node for the CMOS Sensor Interface (CSI) which enables the chip
> > -to connect directly to external CMOS image sensors.
> > -
> > -Required properties:
> > -
> > -- compatible    : "fsl,imx7-csi" or "fsl,imx6ul-csi";
> > -- reg           : base address and length of the register set for the device;
> > -- interrupts    : should contain CSI interrupt;
> > -- clocks        : list of clock specifiers, see
> > -        Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
> > -- clock-names   : must contain "mclk";
> > -
> > -The device node shall contain one 'port' child node with one child 'endpoint'
> > -node, according to the bindings defined in:
> > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > -
> > -In the following example a remote endpoint is a video multiplexer.
> > -
> > -example:
> > -
> > -                csi: csi@30710000 {
> > -                        #address-cells = <1>;
> > -                        #size-cells = <0>;
> > -
> > -                        compatible = "fsl,imx7-csi";
> > -                        reg = <0x30710000 0x10000>;
> > -                        interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> > -                        clocks = <&clks IMX7D_CSI_MCLK_ROOT_CLK>;
> > -                        clock-names = "mclk";
> > -
> > -                        port {
> > -                                csi_from_csi_mux: endpoint {
> > -                                        remote-endpoint = <&csi_mux_to_csi>;
> > -                                };
> > -                        };
> > -                };
> > diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> > new file mode 100644
> > index 000000000000..9fe064dd5ba3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> > @@ -0,0 +1,84 @@
> > +# SPDX-License-Identifier: GPL-2.0
> 
> Shouldn't bindings be dual licensed ?

right.

> 
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/nxp,imx7-csi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: i.MX7 CMOS Sensor Interface
> > +
> > +maintainers:
> > +  - Rui Miguel Silva <rmfrfs@gmail.com>
> > +
> > +description: |
> > +  This is device node for the CMOS Sensor Interface (CSI) which enables the
> > +  chip to connect directly to external CMOS image sensors.
> 
> Pretty cryptic, not your fault as it was there already. Is NXP using
> CSI as a name but it's not really MIPI CSI-2 ? This seems to be a
> bridge, right ?

Correct, original naming issues.

> 
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - fsl,imx7-csi
> > +      - fsl,imx6ul-csi
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    items:
> > +      - const: mclk
> > +
> > +  port:
> > +    type: object
> > +    additionalProperties: false
> > +    description:
> > +      A node containing input port nodes with endpoint definitions as documented
> > +      in Documentation/devicetree/bindings/media/video-interfaces.txt
> > +
> > +    properties:
> > +      endpoint:
> > +        type: object
> > +        additionalProperties: false
> > +
> > +        properties:
> > +          remote-endpoint: true
> > +
> > +        required:
> > +          - remote-endpoint
> > +
> > +    required:
> > +      - endpoint
> 
> As per the comment on ov2680, this last part can be removed
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - port
> > +
> > +unevaluatedProperties: false
> 
> additionalProperties: false ?
> 
> This apart
> Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks,
Cheers,
   Rui
> 
> Thanks
>   j
> 
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/imx7d-clock.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    csi: csi@30710000 {
> > +            compatible = "fsl,imx7-csi";
> > +            reg = <0x30710000 0x10000>;
> > +            interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> > +            clocks = <&clks IMX7D_CSI_MCLK_ROOT_CLK>;
> > +            clock-names = "mclk";
> > +
> > +            port {
> > +                    csi_from_csi_mux: endpoint {
> > +                            remote-endpoint = <&csi_mux_to_csi>;
> > +                    };
> > +            };
> > +    };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 926dcdc4794c..b7f7f14cd85b 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -10773,8 +10773,8 @@ L:	linux-media@vger.kernel.org
> >  S:	Maintained
> >  T:	git git://linuxtv.org/media_tree.git
> >  F:	Documentation/admin-guide/media/imx7.rst
> > -F:	Documentation/devicetree/bindings/media/imx7-csi.txt
> >  F:	Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> > +F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> >  F:	drivers/staging/media/imx/imx7-media-csi.c
> >  F:	drivers/staging/media/imx/imx7-mipi-csis.c
> >
> > --
> > 2.28.0
> >

[PATCH v2 3/3] dt-bindings: imx7-mipi-csi2: convert bindings to yaml

[Rui Miguel Silva]

Convert imx7 mipi csi2 bindings documentation to yaml schema, remove
the textual document and update MAINTAINERS entry.

Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 .../bindings/media/imx7-mipi-csi2.txt         |  90 ---------
 .../bindings/media/nxp,imx7-mipi-csi2.yaml    | 181 ++++++++++++++++++
 MAINTAINERS                                   |   2 +-
 3 files changed, 182 insertions(+), 91 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
 create mode 100644 Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml

diff --git a/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt b/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
deleted file mode 100644
index 71fd74ed3ec8..000000000000
--- a/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
+++ /dev/null
@@ -1,90 +0,0 @@
-Freescale i.MX7 Mipi CSI2
-=========================
-
-mipi_csi2 node
---------------
-
-This is the device node for the MIPI CSI-2 receiver core in i.MX7 SoC. It is
-compatible with previous version of Samsung D-phy.
-
-Required properties:
-
-- compatible    : "fsl,imx7-mipi-csi2";
-- reg           : base address and length of the register set for the device;
-- interrupts    : should contain MIPI CSIS interrupt;
-- clocks        : list of clock specifiers, see
-        Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
-- clock-names   : must contain "pclk", "wrap" and "phy" entries, matching
-                  entries in the clock property;
-- power-domains : a phandle to the power domain, see
-          Documentation/devicetree/bindings/power/power_domain.txt for details.
-- reset-names   : should include following entry "mrst";
-- resets        : a list of phandle, should contain reset entry of
-                  reset-names;
-- phy-supply    : from the generic phy bindings, a phandle to a regulator that
-	          provides power to MIPI CSIS core;
-
-Optional properties:
-
-- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
-		    value when this property is not specified is 166 MHz;
-- fsl,csis-hs-settle : differential receiver (HS-RX) settle time;
-
-The device node should contain two 'port' child nodes with one child 'endpoint'
-node, according to the bindings defined in:
- Documentation/devicetree/bindings/ media/video-interfaces.txt.
- The following are properties specific to those nodes.
-
-port node
----------
-
-- reg		  : (required) can take the values 0 or 1, where 0 shall be
-                     related to the sink port and port 1 shall be the source
-                     one;
-
-endpoint node
--------------
-
-- data-lanes    : (required) an array specifying active physical MIPI-CSI2
-		    data input lanes and their mapping to logical lanes; this
-                    shall only be applied to port 0 (sink port), the array's
-                    content is unused only its length is meaningful,
-                    in this case the maximum length supported is 2;
-
-example:
-
-        mipi_csi: mipi-csi@30750000 {
-                #address-cells = <1>;
-                #size-cells = <0>;
-
-                compatible = "fsl,imx7-mipi-csi2";
-                reg = <0x30750000 0x10000>;
-                interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
-                clocks = <&clks IMX7D_IPG_ROOT_CLK>,
-                                <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
-                                <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
-                clock-names = "pclk", "wrap", "phy";
-                clock-frequency = <166000000>;
-                power-domains = <&pgc_mipi_phy>;
-                phy-supply = <&reg_1p0d>;
-                resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
-                reset-names = "mrst";
-                fsl,csis-hs-settle = <3>;
-
-                port@0 {
-                        reg = <0>;
-
-                        mipi_from_sensor: endpoint {
-                                remote-endpoint = <&ov2680_to_mipi>;
-                                data-lanes = <1>;
-                        };
-                };
-
-                port@1 {
-                        reg = <1>;
-
-                        mipi_vc0_to_csi_mux: endpoint {
-                                remote-endpoint = <&csi_mux_from_mipi_vc0>;
-                        };
-                };
-        };
diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
new file mode 100644
index 000000000000..0438b28232ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
@@ -0,0 +1,181 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/nxp,imx7-mipi-csi2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX7 Mipi CSI2
+
+maintainers:
+  - Rui Miguel Silva <rmfrfs@gmail.com>
+
+description: |
+  this is the device node for the mipi csi-2 receiver core in i.mx7 soc. it is
+  compatible with previous version of samsung d-phy.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - fsl,imx7-mipi-csi2
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 3
+
+  clock-names:
+    items:
+      - const: pclk
+      - const: wrap
+      - const: phy
+
+  power-domains:
+    maxItems: 1
+
+  phy-supply:
+    description:
+      Phandle to a regulator that provides power to the PHY. This
+      regulator will be managed during the PHY power on/off sequence.
+
+  resets:
+    maxItems: 1
+
+  reset-names:
+    const: mrst
+
+  clock-frequency:
+    description:
+      The IP main (system bus) clock frequency in Hertz
+    default: 166000000
+
+  fsl,csis-hs-settle:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      differential receiver (HS-RX) settle time
+
+  ports:
+    type: object
+    description:
+      A node containing input and output port nodes with endpoint definitions
+      as documented in
+      Documentation/devicetree/bindings/media/video-interfaces.txt
+
+    properties:
+      '#address-cells':
+        const: 1
+
+      '#size-cells':
+        const: 0
+
+      port@0:
+        type: object
+        description:
+          Input port node, single endpoint describing the CSI-2 transmitter.
+
+        properties:
+          reg:
+            const: 0
+
+          endpoint:
+            type: object
+
+            properties:
+              data-lanes:
+                maxItems: 1
+
+              remote-endpoint: true
+
+            required:
+              - data-lanes
+              - remote-endpoint
+
+            additionalProperties: false
+
+        additionalProperties: false
+
+      port@1:
+        type: object
+        description:
+          Output port node,
+
+        properties:
+          reg:
+            const: 1
+
+          endpoint:
+              type: object
+
+              properties:
+                remote-endpoint: true
+
+              required:
+                - remote-endpoint
+
+              additionalProperties: false
+
+        additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+  - reset-names
+  - ports
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx7d-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/reset/imx7-reset.h>
+
+    mipi_csi: mipi-csi@30750000 {
+            compatible = "fsl,imx7-mipi-csi2";
+            reg = <0x30750000 0x10000>;
+            interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
+  
+            clocks = <&clks IMX7D_IPG_ROOT_CLK>,
+                     <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
+                     <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
+            clock-names = "pclk", "wrap", "phy";
+            clock-frequency = <166000000>;
+
+            power-domains = <&pgc_mipi_phy>;
+            phy-supply = <&reg_1p0d>;
+            resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
+            reset-names = "mrst";
+            fsl,csis-hs-settle = <3>;
+
+            ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    port@0 {
+                            reg = <0>;
+
+                            mipi_from_sensor: endpoint {
+                                    remote-endpoint = <&ov2680_to_mipi>;
+                                    data-lanes = <1>;
+                            };
+                    };
+
+                    port@1 {
+                            reg = <1>;
+
+                            mipi_vc0_to_csi_mux: endpoint {
+                                    remote-endpoint = <&csi_mux_from_mipi_vc0>;
+                            };
+                    };
+            };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index b7f7f14cd85b..9da67222b0c7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10773,8 +10773,8 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
 F:	Documentation/admin-guide/media/imx7.rst
-F:	Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
 F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
+F:	Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
 F:	drivers/staging/media/imx/imx7-media-csi.c
 F:	drivers/staging/media/imx/imx7-mipi-csis.c
 
-- 
2.28.0

Re: [PATCH v2 3/3] dt-bindings: imx7-mipi-csi2: convert bindings to yaml

[Jacopo Mondi]

Hi Rui,

On Wed, Oct 14, 2020 at 03:27:59PM +0100, Rui Miguel Silva wrote:
> Convert imx7 mipi csi2 bindings documentation to yaml schema, remove
> the textual document and update MAINTAINERS entry.
>
> Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
> ---
>  .../bindings/media/imx7-mipi-csi2.txt         |  90 ---------
>  .../bindings/media/nxp,imx7-mipi-csi2.yaml    | 181 ++++++++++++++++++
>  MAINTAINERS                                   |   2 +-
>  3 files changed, 182 insertions(+), 91 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
>  create mode 100644 Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt b/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> deleted file mode 100644
> index 71fd74ed3ec8..000000000000
> --- a/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> +++ /dev/null
> @@ -1,90 +0,0 @@
> -Freescale i.MX7 Mipi CSI2
> -=========================
> -
> -mipi_csi2 node
> ---------------
> -
> -This is the device node for the MIPI CSI-2 receiver core in i.MX7 SoC. It is
> -compatible with previous version of Samsung D-phy.
> -
> -Required properties:
> -
> -- compatible    : "fsl,imx7-mipi-csi2";
> -- reg           : base address and length of the register set for the device;
> -- interrupts    : should contain MIPI CSIS interrupt;
> -- clocks        : list of clock specifiers, see
> -        Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
> -- clock-names   : must contain "pclk", "wrap" and "phy" entries, matching
> -                  entries in the clock property;
> -- power-domains : a phandle to the power domain, see
> -          Documentation/devicetree/bindings/power/power_domain.txt for details.
> -- reset-names   : should include following entry "mrst";
> -- resets        : a list of phandle, should contain reset entry of
> -                  reset-names;
> -- phy-supply    : from the generic phy bindings, a phandle to a regulator that
> -	          provides power to MIPI CSIS core;
> -
> -Optional properties:
> -
> -- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
> -		    value when this property is not specified is 166 MHz;
> -- fsl,csis-hs-settle : differential receiver (HS-RX) settle time;
> -
> -The device node should contain two 'port' child nodes with one child 'endpoint'
> -node, according to the bindings defined in:
> - Documentation/devicetree/bindings/ media/video-interfaces.txt.
> - The following are properties specific to those nodes.
> -
> -port node
> ----------
> -
> -- reg		  : (required) can take the values 0 or 1, where 0 shall be
> -                     related to the sink port and port 1 shall be the source
> -                     one;
> -
> -endpoint node
> --------------
> -
> -- data-lanes    : (required) an array specifying active physical MIPI-CSI2
> -		    data input lanes and their mapping to logical lanes; this
> -                    shall only be applied to port 0 (sink port), the array's
> -                    content is unused only its length is meaningful,
> -                    in this case the maximum length supported is 2;
> -
> -example:
> -
> -        mipi_csi: mipi-csi@30750000 {
> -                #address-cells = <1>;
> -                #size-cells = <0>;
> -
> -                compatible = "fsl,imx7-mipi-csi2";
> -                reg = <0x30750000 0x10000>;
> -                interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> -                clocks = <&clks IMX7D_IPG_ROOT_CLK>,
> -                                <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
> -                                <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
> -                clock-names = "pclk", "wrap", "phy";
> -                clock-frequency = <166000000>;
> -                power-domains = <&pgc_mipi_phy>;
> -                phy-supply = <&reg_1p0d>;
> -                resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
> -                reset-names = "mrst";
> -                fsl,csis-hs-settle = <3>;
> -
> -                port@0 {
> -                        reg = <0>;
> -
> -                        mipi_from_sensor: endpoint {
> -                                remote-endpoint = <&ov2680_to_mipi>;
> -                                data-lanes = <1>;
> -                        };
> -                };
> -
> -                port@1 {
> -                        reg = <1>;
> -
> -                        mipi_vc0_to_csi_mux: endpoint {
> -                                remote-endpoint = <&csi_mux_from_mipi_vc0>;
> -                        };
> -                };
> -        };
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> new file mode 100644
> index 000000000000..0438b28232ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> @@ -0,0 +1,181 @@
> +# SPDX-License-Identifier: GPL-2.0-only

Same question about dual licensing

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/nxp,imx7-mipi-csi2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX7 Mipi CSI2
> +
> +maintainers:
> +  - Rui Miguel Silva <rmfrfs@gmail.com>
> +
> +description: |
> +  this is the device node for the mipi csi-2 receiver core in i.mx7 soc. it is
> +  compatible with previous version of samsung d-phy.

nit: missing a few captial letters (beginning and after a full stop).
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - fsl,imx7-mipi-csi2

Should enum with a single item be expressed as a const ?

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 3
> +
> +  clock-names:
> +    items:
> +      - const: pclk
> +      - const: wrap
> +      - const: phy
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  phy-supply:
> +    description:
> +      Phandle to a regulator that provides power to the PHY. This
> +      regulator will be managed during the PHY power on/off sequence.
> +
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    const: mrst
> +
> +  clock-frequency:

This was here already, but shouldn't this property be assigned in the
clock provider and you should have here a reference to it ? Otherwise,
is this really a custom property hijacking a standard property name ?

I see it is mentioned in the example-schema.yaml, so it's probably
correct.

> +    description:
> +      The IP main (system bus) clock frequency in Hertz
> +    default: 166000000
> +
> +  fsl,csis-hs-settle:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      differential receiver (HS-RX) settle time

You have used capital letters at the beginning of documentation blocks
before.

> +
> +  ports:
> +    type: object
> +    description:
> +      A node containing input and output port nodes with endpoint definitions
> +      as documented in
> +      Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +    properties:
> +      '#address-cells':
> +        const: 1
> +
> +      '#size-cells':
> +        const: 0
> +
> +      port@0:
> +        type: object
> +        description:
> +          Input port node, single endpoint describing the CSI-2 transmitter.
> +
> +        properties:
> +          reg:
> +            const: 0
> +
> +          endpoint:
> +            type: object
> +
> +            properties:
> +              data-lanes:
> +                maxItems: 1

How many data lanes does this receiver supports ? I see in the txt
bindingsd:
    data-lanes    : (required) an array specifying active physical MIPI-CSI2
                    data input lanes and their mapping to logical lanes; this
                    shall only be applied to port 0 (sink port), the array's
                    content is unused only its length is meaningful,
                    in this case the maximum length supported is 2;

I'm not sure I fully get this. Does this mean up to 2 data lanes are
supported ?

> +
> +              remote-endpoint: true
> +
> +            required:
> +              - data-lanes
> +              - remote-endpoint
> +
> +            additionalProperties: false
> +
> +        additionalProperties: false
> +
> +      port@1:
> +        type: object
> +        description:
> +          Output port node,

Rougue ',' at the end. You probably want a full stop or
> +
> +        properties:
> +          reg:
> +            const: 1
> +
> +          endpoint:
> +              type: object
> +
> +              properties:
> +                remote-endpoint: true
> +
> +              required:
> +                - remote-endpoint
> +
> +              additionalProperties: false

As no other endpoint property is specified, the whole 'endpoint' block
can probably be omitted ?

> +
> +        additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - power-domains
> +  - resets
> +  - reset-names
> +  - ports

phy-supply was listed as required in the txt bindings

> +
> +unevaluatedProperties: false

additionalProperties: false

Thanks
  j

> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx7d-clock.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/reset/imx7-reset.h>
> +
> +    mipi_csi: mipi-csi@30750000 {
> +            compatible = "fsl,imx7-mipi-csi2";
> +            reg = <0x30750000 0x10000>;
> +            interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> +
> +            clocks = <&clks IMX7D_IPG_ROOT_CLK>,
> +                     <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
> +                     <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
> +            clock-names = "pclk", "wrap", "phy";
> +            clock-frequency = <166000000>;
> +
> +            power-domains = <&pgc_mipi_phy>;
> +            phy-supply = <&reg_1p0d>;
> +            resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
> +            reset-names = "mrst";
> +            fsl,csis-hs-settle = <3>;
> +
> +            ports {
> +                    #address-cells = <1>;
> +                    #size-cells = <0>;
> +
> +                    port@0 {
> +                            reg = <0>;
> +
> +                            mipi_from_sensor: endpoint {
> +                                    remote-endpoint = <&ov2680_to_mipi>;
> +                                    data-lanes = <1>;
> +                            };
> +                    };
> +
> +                    port@1 {
> +                            reg = <1>;
> +
> +                            mipi_vc0_to_csi_mux: endpoint {
> +                                    remote-endpoint = <&csi_mux_from_mipi_vc0>;
> +                            };
> +                    };
> +            };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b7f7f14cd85b..9da67222b0c7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10773,8 +10773,8 @@ L:	linux-media@vger.kernel.org
>  S:	Maintained
>  T:	git git://linuxtv.org/media_tree.git
>  F:	Documentation/admin-guide/media/imx7.rst
> -F:	Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
>  F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> +F:	Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
>  F:	drivers/staging/media/imx/imx7-media-csi.c
>  F:	drivers/staging/media/imx/imx7-mipi-csis.c
>
> --
> 2.28.0
>

Re: [PATCH v2 3/3] dt-bindings: imx7-mipi-csi2: convert bindings to yaml

[Rui Miguel Silva]

Hi again Jacopo,
On Thu, Oct 15, 2020 at 05:52:51PM +0200, Jacopo Mondi wrote:
> Hi Rui,
> 
> On Wed, Oct 14, 2020 at 03:27:59PM +0100, Rui Miguel Silva wrote:
> > Convert imx7 mipi csi2 bindings documentation to yaml schema, remove
> > the textual document and update MAINTAINERS entry.
> >
> > Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
> > ---
> >  .../bindings/media/imx7-mipi-csi2.txt         |  90 ---------
> >  .../bindings/media/nxp,imx7-mipi-csi2.yaml    | 181 ++++++++++++++++++
> >  MAINTAINERS                                   |   2 +-
> >  3 files changed, 182 insertions(+), 91 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> >  create mode 100644 Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt b/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> > deleted file mode 100644
> > index 71fd74ed3ec8..000000000000
> > --- a/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> > +++ /dev/null
> > @@ -1,90 +0,0 @@
> > -Freescale i.MX7 Mipi CSI2
> > -=========================
> > -
> > -mipi_csi2 node
> > ---------------
> > -
> > -This is the device node for the MIPI CSI-2 receiver core in i.MX7 SoC. It is
> > -compatible with previous version of Samsung D-phy.
> > -
> > -Required properties:
> > -
> > -- compatible    : "fsl,imx7-mipi-csi2";
> > -- reg           : base address and length of the register set for the device;
> > -- interrupts    : should contain MIPI CSIS interrupt;
> > -- clocks        : list of clock specifiers, see
> > -        Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
> > -- clock-names   : must contain "pclk", "wrap" and "phy" entries, matching
> > -                  entries in the clock property;
> > -- power-domains : a phandle to the power domain, see
> > -          Documentation/devicetree/bindings/power/power_domain.txt for details.
> > -- reset-names   : should include following entry "mrst";
> > -- resets        : a list of phandle, should contain reset entry of
> > -                  reset-names;
> > -- phy-supply    : from the generic phy bindings, a phandle to a regulator that
> > -	          provides power to MIPI CSIS core;
> > -
> > -Optional properties:
> > -
> > -- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
> > -		    value when this property is not specified is 166 MHz;
> > -- fsl,csis-hs-settle : differential receiver (HS-RX) settle time;
> > -
> > -The device node should contain two 'port' child nodes with one child 'endpoint'
> > -node, according to the bindings defined in:
> > - Documentation/devicetree/bindings/ media/video-interfaces.txt.
> > - The following are properties specific to those nodes.
> > -
> > -port node
> > ----------
> > -
> > -- reg		  : (required) can take the values 0 or 1, where 0 shall be
> > -                     related to the sink port and port 1 shall be the source
> > -                     one;
> > -
> > -endpoint node
> > --------------
> > -
> > -- data-lanes    : (required) an array specifying active physical MIPI-CSI2
> > -		    data input lanes and their mapping to logical lanes; this
> > -                    shall only be applied to port 0 (sink port), the array's
> > -                    content is unused only its length is meaningful,
> > -                    in this case the maximum length supported is 2;
> > -
> > -example:
> > -
> > -        mipi_csi: mipi-csi@30750000 {
> > -                #address-cells = <1>;
> > -                #size-cells = <0>;
> > -
> > -                compatible = "fsl,imx7-mipi-csi2";
> > -                reg = <0x30750000 0x10000>;
> > -                interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> > -                clocks = <&clks IMX7D_IPG_ROOT_CLK>,
> > -                                <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
> > -                                <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
> > -                clock-names = "pclk", "wrap", "phy";
> > -                clock-frequency = <166000000>;
> > -                power-domains = <&pgc_mipi_phy>;
> > -                phy-supply = <&reg_1p0d>;
> > -                resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
> > -                reset-names = "mrst";
> > -                fsl,csis-hs-settle = <3>;
> > -
> > -                port@0 {
> > -                        reg = <0>;
> > -
> > -                        mipi_from_sensor: endpoint {
> > -                                remote-endpoint = <&ov2680_to_mipi>;
> > -                                data-lanes = <1>;
> > -                        };
> > -                };
> > -
> > -                port@1 {
> > -                        reg = <1>;
> > -
> > -                        mipi_vc0_to_csi_mux: endpoint {
> > -                                remote-endpoint = <&csi_mux_from_mipi_vc0>;
> > -                        };
> > -                };
> > -        };
> > diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> > new file mode 100644
> > index 000000000000..0438b28232ed
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> > @@ -0,0 +1,181 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> 
> Same question about dual licensing
> 
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/nxp,imx7-mipi-csi2.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP i.MX7 Mipi CSI2
> > +
> > +maintainers:
> > +  - Rui Miguel Silva <rmfrfs@gmail.com>
> > +
> > +description: |
> > +  this is the device node for the mipi csi-2 receiver core in i.mx7 soc. it is
> > +  compatible with previous version of samsung d-phy.
> 
> nit: missing a few captial letters (beginning and after a full stop).

will fix along all document.

> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - fsl,imx7-mipi-csi2
> 
> Should enum with a single item be expressed as a const ?

agree.

> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    minItems: 3
> > +
> > +  clock-names:
> > +    items:
> > +      - const: pclk
> > +      - const: wrap
> > +      - const: phy
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  phy-supply:
> > +    description:
> > +      Phandle to a regulator that provides power to the PHY. This
> > +      regulator will be managed during the PHY power on/off sequence.
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  reset-names:
> > +    const: mrst
> > +
> > +  clock-frequency:
> 
> This was here already, but shouldn't this property be assigned in the
> clock provider and you should have here a reference to it ? Otherwise,
> is this really a custom property hijacking a standard property name ?
> 
> I see it is mentioned in the example-schema.yaml, so it's probably
> correct.

I think limits and default can be assigned here.

> 
> > +    description:
> > +      The IP main (system bus) clock frequency in Hertz
> > +    default: 166000000
> > +
> > +  fsl,csis-hs-settle:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      differential receiver (HS-RX) settle time
> 
> You have used capital letters at the beginning of documentation blocks
> before.
> 
> > +
> > +  ports:
> > +    type: object
> > +    description:
> > +      A node containing input and output port nodes with endpoint definitions
> > +      as documented in
> > +      Documentation/devicetree/bindings/media/video-interfaces.txt
> > +
> > +    properties:
> > +      '#address-cells':
> > +        const: 1
> > +
> > +      '#size-cells':
> > +        const: 0
> > +
> > +      port@0:
> > +        type: object
> > +        description:
> > +          Input port node, single endpoint describing the CSI-2 transmitter.
> > +
> > +        properties:
> > +          reg:
> > +            const: 0
> > +
> > +          endpoint:
> > +            type: object
> > +
> > +            properties:
> > +              data-lanes:
> > +                maxItems: 1
> 
> How many data lanes does this receiver supports ? I see in the txt
> bindingsd:
>     data-lanes    : (required) an array specifying active physical MIPI-CSI2
>                     data input lanes and their mapping to logical lanes; this
>                     shall only be applied to port 0 (sink port), the array's
>                     content is unused only its length is meaningful,
>                     in this case the maximum length supported is 2;
> 
> I'm not sure I fully get this. Does this mean up to 2 data lanes are
> supported ?

yeah, it supports up to 2 data lanes. Will fix

> 
> > +
> > +              remote-endpoint: true
> > +
> > +            required:
> > +              - data-lanes
> > +              - remote-endpoint
> > +
> > +            additionalProperties: false
> > +
> > +        additionalProperties: false
> > +
> > +      port@1:
> > +        type: object
> > +        description:
> > +          Output port node,
> 
> Rougue ',' at the end. You probably want a full stop or
> > +
> > +        properties:
> > +          reg:
> > +            const: 1
> > +
> > +          endpoint:
> > +              type: object
> > +
> > +              properties:
> > +                remote-endpoint: true
> > +
> > +              required:
> > +                - remote-endpoint
> > +
> > +              additionalProperties: false
> 
> As no other endpoint property is specified, the whole 'endpoint' block
> can probably be omitted ?
> 
> > +
> > +        additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - power-domains
> > +  - resets
> > +  - reset-names
> > +  - ports
> 
> phy-supply was listed as required in the txt bindings

good catch.

> 
> > +
> > +unevaluatedProperties: false
> 
> additionalProperties: false
> 

Thanks again for your review.

Cheers,
   Rui

> Thanks
>   j
> 
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/imx7d-clock.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/reset/imx7-reset.h>
> > +
> > +    mipi_csi: mipi-csi@30750000 {
> > +            compatible = "fsl,imx7-mipi-csi2";
> > +            reg = <0x30750000 0x10000>;
> > +            interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > +            clocks = <&clks IMX7D_IPG_ROOT_CLK>,
> > +                     <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
> > +                     <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
> > +            clock-names = "pclk", "wrap", "phy";
> > +            clock-frequency = <166000000>;
> > +
> > +            power-domains = <&pgc_mipi_phy>;
> > +            phy-supply = <&reg_1p0d>;
> > +            resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
> > +            reset-names = "mrst";
> > +            fsl,csis-hs-settle = <3>;
> > +
> > +            ports {
> > +                    #address-cells = <1>;
> > +                    #size-cells = <0>;
> > +
> > +                    port@0 {
> > +                            reg = <0>;
> > +
> > +                            mipi_from_sensor: endpoint {
> > +                                    remote-endpoint = <&ov2680_to_mipi>;
> > +                                    data-lanes = <1>;
> > +                            };
> > +                    };
> > +
> > +                    port@1 {
> > +                            reg = <1>;
> > +
> > +                            mipi_vc0_to_csi_mux: endpoint {
> > +                                    remote-endpoint = <&csi_mux_from_mipi_vc0>;
> > +                            };
> > +                    };
> > +            };
> > +    };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index b7f7f14cd85b..9da67222b0c7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -10773,8 +10773,8 @@ L:	linux-media@vger.kernel.org
> >  S:	Maintained
> >  T:	git git://linuxtv.org/media_tree.git
> >  F:	Documentation/admin-guide/media/imx7.rst
> > -F:	Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> >  F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> > +F:	Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> >  F:	drivers/staging/media/imx/imx7-media-csi.c
> >  F:	drivers/staging/media/imx/imx7-mipi-csis.c
> >
> > --
> > 2.28.0
> >

Re: [PATCH v2 3/3] dt-bindings: imx7-mipi-csi2: convert bindings to yaml

[Jacopo Mondi]

Hi Rui,

On Fri, Oct 16, 2020 at 03:51:09PM +0100, Rui Miguel Silva wrote:
> Hi again Jacopo,
> On Thu, Oct 15, 2020 at 05:52:51PM +0200, Jacopo Mondi wrote:
> > Hi Rui,
> >
> > On Wed, Oct 14, 2020 at 03:27:59PM +0100, Rui Miguel Silva wrote:
> > > Convert imx7 mipi csi2 bindings documentation to yaml schema, remove
> > > the textual document and update MAINTAINERS entry.
> > >
> > > Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
> > > ---
> > >  .../bindings/media/imx7-mipi-csi2.txt         |  90 ---------
> > >  .../bindings/media/nxp,imx7-mipi-csi2.yaml    | 181 ++++++++++++++++++
> > >  MAINTAINERS                                   |   2 +-
> > >  3 files changed, 182 insertions(+), 91 deletions(-)
> > >  delete mode 100644 Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> > >  create mode 100644 Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt b/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> > > deleted file mode 100644
> > > index 71fd74ed3ec8..000000000000
> > > --- a/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> > > +++ /dev/null
> > > @@ -1,90 +0,0 @@
> > > -Freescale i.MX7 Mipi CSI2
> > > -=========================
> > > -
> > > -mipi_csi2 node
> > > ---------------
> > > -
> > > -This is the device node for the MIPI CSI-2 receiver core in i.MX7 SoC. It is
> > > -compatible with previous version of Samsung D-phy.
> > > -
> > > -Required properties:
> > > -
> > > -- compatible    : "fsl,imx7-mipi-csi2";
> > > -- reg           : base address and length of the register set for the device;
> > > -- interrupts    : should contain MIPI CSIS interrupt;
> > > -- clocks        : list of clock specifiers, see
> > > -        Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
> > > -- clock-names   : must contain "pclk", "wrap" and "phy" entries, matching
> > > -                  entries in the clock property;
> > > -- power-domains : a phandle to the power domain, see
> > > -          Documentation/devicetree/bindings/power/power_domain.txt for details.
> > > -- reset-names   : should include following entry "mrst";
> > > -- resets        : a list of phandle, should contain reset entry of
> > > -                  reset-names;
> > > -- phy-supply    : from the generic phy bindings, a phandle to a regulator that
> > > -	          provides power to MIPI CSIS core;
> > > -
> > > -Optional properties:
> > > -
> > > -- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
> > > -		    value when this property is not specified is 166 MHz;
> > > -- fsl,csis-hs-settle : differential receiver (HS-RX) settle time;
> > > -
> > > -The device node should contain two 'port' child nodes with one child 'endpoint'
> > > -node, according to the bindings defined in:
> > > - Documentation/devicetree/bindings/ media/video-interfaces.txt.
> > > - The following are properties specific to those nodes.
> > > -
> > > -port node
> > > ----------
> > > -
> > > -- reg		  : (required) can take the values 0 or 1, where 0 shall be
> > > -                     related to the sink port and port 1 shall be the source
> > > -                     one;
> > > -
> > > -endpoint node
> > > --------------
> > > -
> > > -- data-lanes    : (required) an array specifying active physical MIPI-CSI2
> > > -		    data input lanes and their mapping to logical lanes; this
> > > -                    shall only be applied to port 0 (sink port), the array's
> > > -                    content is unused only its length is meaningful,
> > > -                    in this case the maximum length supported is 2;
> > > -
> > > -example:
> > > -
> > > -        mipi_csi: mipi-csi@30750000 {
> > > -                #address-cells = <1>;
> > > -                #size-cells = <0>;
> > > -
> > > -                compatible = "fsl,imx7-mipi-csi2";
> > > -                reg = <0x30750000 0x10000>;
> > > -                interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> > > -                clocks = <&clks IMX7D_IPG_ROOT_CLK>,
> > > -                                <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
> > > -                                <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
> > > -                clock-names = "pclk", "wrap", "phy";
> > > -                clock-frequency = <166000000>;
> > > -                power-domains = <&pgc_mipi_phy>;
> > > -                phy-supply = <&reg_1p0d>;
> > > -                resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
> > > -                reset-names = "mrst";
> > > -                fsl,csis-hs-settle = <3>;
> > > -
> > > -                port@0 {
> > > -                        reg = <0>;
> > > -
> > > -                        mipi_from_sensor: endpoint {
> > > -                                remote-endpoint = <&ov2680_to_mipi>;
> > > -                                data-lanes = <1>;
> > > -                        };
> > > -                };
> > > -
> > > -                port@1 {
> > > -                        reg = <1>;
> > > -
> > > -                        mipi_vc0_to_csi_mux: endpoint {
> > > -                                remote-endpoint = <&csi_mux_from_mipi_vc0>;
> > > -                        };
> > > -                };
> > > -        };
> > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> > > new file mode 100644
> > > index 000000000000..0438b28232ed
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> > > @@ -0,0 +1,181 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> >
> > Same question about dual licensing
> >
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/nxp,imx7-mipi-csi2.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: NXP i.MX7 Mipi CSI2
> > > +
> > > +maintainers:
> > > +  - Rui Miguel Silva <rmfrfs@gmail.com>
> > > +
> > > +description: |
> > > +  this is the device node for the mipi csi-2 receiver core in i.mx7 soc. it is
> > > +  compatible with previous version of samsung d-phy.
> >
> > nit: missing a few captial letters (beginning and after a full stop).
>
> will fix along all document.
>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - enum:
> > > +          - fsl,imx7-mipi-csi2
> >
> > Should enum with a single item be expressed as a const ?
>
> agree.
>
> >
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    minItems: 3
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: pclk
> > > +      - const: wrap
> > > +      - const: phy
> > > +
> > > +  power-domains:
> > > +    maxItems: 1
> > > +
> > > +  phy-supply:
> > > +    description:
> > > +      Phandle to a regulator that provides power to the PHY. This
> > > +      regulator will be managed during the PHY power on/off sequence.
> > > +
> > > +  resets:
> > > +    maxItems: 1
> > > +
> > > +  reset-names:
> > > +    const: mrst
> > > +
> > > +  clock-frequency:
> >
> > This was here already, but shouldn't this property be assigned in the
> > clock provider and you should have here a reference to it ? Otherwise,
> > is this really a custom property hijacking a standard property name ?
> >
> > I see it is mentioned in the example-schema.yaml, so it's probably
> > correct.
>
> I think limits and default can be assigned here.
>
> >
> > > +    description:
> > > +      The IP main (system bus) clock frequency in Hertz
> > > +    default: 166000000
> > > +
> > > +  fsl,csis-hs-settle:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description:
> > > +      differential receiver (HS-RX) settle time
> >
> > You have used capital letters at the beginning of documentation blocks
> > before.
> >
> > > +
> > > +  ports:
> > > +    type: object
> > > +    description:
> > > +      A node containing input and output port nodes with endpoint definitions
> > > +      as documented in
> > > +      Documentation/devicetree/bindings/media/video-interfaces.txt
> > > +
> > > +    properties:
> > > +      '#address-cells':
> > > +        const: 1
> > > +
> > > +      '#size-cells':
> > > +        const: 0
> > > +
> > > +      port@0:
> > > +        type: object
> > > +        description:
> > > +          Input port node, single endpoint describing the CSI-2 transmitter.
> > > +
> > > +        properties:
> > > +          reg:
> > > +            const: 0
> > > +
> > > +          endpoint:
> > > +            type: object
> > > +
> > > +            properties:
> > > +              data-lanes:
> > > +                maxItems: 1
> >
> > How many data lanes does this receiver supports ? I see in the txt
> > bindingsd:
> >     data-lanes    : (required) an array specifying active physical MIPI-CSI2
> >                     data input lanes and their mapping to logical lanes; this
> >                     shall only be applied to port 0 (sink port), the array's
> >                     content is unused only its length is meaningful,
> >                     in this case the maximum length supported is 2;
> >
> > I'm not sure I fully get this. Does this mean up to 2 data lanes are
> > supported ?
>
> yeah, it supports up to 2 data lanes. Will fix
>

This one might get nasty then. For reference, this is what I've been
suggested to use by Laurent and Rob in the imx214 conversion (not yet
landed afaict) to express that <1 2> and <1 2 3 4> where allowed:

+        properties:
+          data-lanes:
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            description: See ../video-interfaces.txt
+            anyOf:
+              - items:
+                - const: 1
+                - const: 2
+              - items:
+                - const: 1
+                - const: 2
+                - const: 3
+                - const: 4

Hope it helps.

Thanks
  j

> >
> > > +
> > > +              remote-endpoint: true
> > > +
> > > +            required:
> > > +              - data-lanes
> > > +              - remote-endpoint
> > > +
> > > +            additionalProperties: false
> > > +
> > > +        additionalProperties: false
> > > +
> > > +      port@1:
> > > +        type: object
> > > +        description:
> > > +          Output port node,
> >
> > Rougue ',' at the end. You probably want a full stop or
> > > +
> > > +        properties:
> > > +          reg:
> > > +            const: 1
> > > +
> > > +          endpoint:
> > > +              type: object
> > > +
> > > +              properties:
> > > +                remote-endpoint: true
> > > +
> > > +              required:
> > > +                - remote-endpoint
> > > +
> > > +              additionalProperties: false
> >
> > As no other endpoint property is specified, the whole 'endpoint' block
> > can probably be omitted ?
> >
> > > +
> > > +        additionalProperties: false
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +  - clocks
> > > +  - clock-names
> > > +  - power-domains
> > > +  - resets
> > > +  - reset-names
> > > +  - ports
> >
> > phy-supply was listed as required in the txt bindings
>
> good catch.
>
> >
> > > +
> > > +unevaluatedProperties: false
> >
> > additionalProperties: false
> >
>
> Thanks again for your review.
>
> Cheers,
>    Rui
>
> > Thanks
> >   j
> >
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/clock/imx7d-clock.h>
> > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +    #include <dt-bindings/interrupt-controller/irq.h>
> > > +    #include <dt-bindings/reset/imx7-reset.h>
> > > +
> > > +    mipi_csi: mipi-csi@30750000 {
> > > +            compatible = "fsl,imx7-mipi-csi2";
> > > +            reg = <0x30750000 0x10000>;
> > > +            interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> > > +
> > > +            clocks = <&clks IMX7D_IPG_ROOT_CLK>,
> > > +                     <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
> > > +                     <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
> > > +            clock-names = "pclk", "wrap", "phy";
> > > +            clock-frequency = <166000000>;
> > > +
> > > +            power-domains = <&pgc_mipi_phy>;
> > > +            phy-supply = <&reg_1p0d>;
> > > +            resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
> > > +            reset-names = "mrst";
> > > +            fsl,csis-hs-settle = <3>;
> > > +
> > > +            ports {
> > > +                    #address-cells = <1>;
> > > +                    #size-cells = <0>;
> > > +
> > > +                    port@0 {
> > > +                            reg = <0>;
> > > +
> > > +                            mipi_from_sensor: endpoint {
> > > +                                    remote-endpoint = <&ov2680_to_mipi>;
> > > +                                    data-lanes = <1>;
> > > +                            };
> > > +                    };
> > > +
> > > +                    port@1 {
> > > +                            reg = <1>;
> > > +
> > > +                            mipi_vc0_to_csi_mux: endpoint {
> > > +                                    remote-endpoint = <&csi_mux_from_mipi_vc0>;
> > > +                            };
> > > +                    };
> > > +            };
> > > +    };
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index b7f7f14cd85b..9da67222b0c7 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -10773,8 +10773,8 @@ L:	linux-media@vger.kernel.org
> > >  S:	Maintained
> > >  T:	git git://linuxtv.org/media_tree.git
> > >  F:	Documentation/admin-guide/media/imx7.rst
> > > -F:	Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> > >  F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> > > +F:	Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> > >  F:	drivers/staging/media/imx/imx7-media-csi.c
> > >  F:	drivers/staging/media/imx/imx7-mipi-csis.c
> > >
> > > --
> > > 2.28.0
> > >

Re: [PATCH v2 3/3] dt-bindings: imx7-mipi-csi2: convert bindings to yaml

[Rui Miguel Silva]

Hi Jacopo,
On Fri, Oct 16, 2020 at 08:02:13PM +0200, Jacopo Mondi wrote:
> Hi Rui,
> 
> On Fri, Oct 16, 2020 at 03:51:09PM +0100, Rui Miguel Silva wrote:
> > Hi again Jacopo,
> > On Thu, Oct 15, 2020 at 05:52:51PM +0200, Jacopo Mondi wrote:
> > > Hi Rui,
> > >
> > > On Wed, Oct 14, 2020 at 03:27:59PM +0100, Rui Miguel Silva wrote:
> > > > Convert imx7 mipi csi2 bindings documentation to yaml schema, remove
> > > > the textual document and update MAINTAINERS entry.
> > > >
> > > > Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
> > > > ---
> > > >  .../bindings/media/imx7-mipi-csi2.txt         |  90 ---------
> > > >  .../bindings/media/nxp,imx7-mipi-csi2.yaml    | 181 ++++++++++++++++++
> > > >  MAINTAINERS                                   |   2 +-
> > > >  3 files changed, 182 insertions(+), 91 deletions(-)
> > > >  delete mode 100644 Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> > > >  create mode 100644 Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt b/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> > > > deleted file mode 100644
> > > > index 71fd74ed3ec8..000000000000
> > > > --- a/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> > > > +++ /dev/null
> > > > @@ -1,90 +0,0 @@
> > > > -Freescale i.MX7 Mipi CSI2
> > > > -=========================
> > > > -
> > > > -mipi_csi2 node
> > > > ---------------
> > > > -
> > > > -This is the device node for the MIPI CSI-2 receiver core in i.MX7 SoC. It is
> > > > -compatible with previous version of Samsung D-phy.
> > > > -
> > > > -Required properties:
> > > > -
> > > > -- compatible    : "fsl,imx7-mipi-csi2";
> > > > -- reg           : base address and length of the register set for the device;
> > > > -- interrupts    : should contain MIPI CSIS interrupt;
> > > > -- clocks        : list of clock specifiers, see
> > > > -        Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
> > > > -- clock-names   : must contain "pclk", "wrap" and "phy" entries, matching
> > > > -                  entries in the clock property;
> > > > -- power-domains : a phandle to the power domain, see
> > > > -          Documentation/devicetree/bindings/power/power_domain.txt for details.
> > > > -- reset-names   : should include following entry "mrst";
> > > > -- resets        : a list of phandle, should contain reset entry of
> > > > -                  reset-names;
> > > > -- phy-supply    : from the generic phy bindings, a phandle to a regulator that
> > > > -	          provides power to MIPI CSIS core;
> > > > -
> > > > -Optional properties:
> > > > -
> > > > -- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
> > > > -		    value when this property is not specified is 166 MHz;
> > > > -- fsl,csis-hs-settle : differential receiver (HS-RX) settle time;
> > > > -
> > > > -The device node should contain two 'port' child nodes with one child 'endpoint'
> > > > -node, according to the bindings defined in:
> > > > - Documentation/devicetree/bindings/ media/video-interfaces.txt.
> > > > - The following are properties specific to those nodes.
> > > > -
> > > > -port node
> > > > ----------
> > > > -
> > > > -- reg		  : (required) can take the values 0 or 1, where 0 shall be
> > > > -                     related to the sink port and port 1 shall be the source
> > > > -                     one;
> > > > -
> > > > -endpoint node
> > > > --------------
> > > > -
> > > > -- data-lanes    : (required) an array specifying active physical MIPI-CSI2
> > > > -		    data input lanes and their mapping to logical lanes; this
> > > > -                    shall only be applied to port 0 (sink port), the array's
> > > > -                    content is unused only its length is meaningful,
> > > > -                    in this case the maximum length supported is 2;
> > > > -
> > > > -example:
> > > > -
> > > > -        mipi_csi: mipi-csi@30750000 {
> > > > -                #address-cells = <1>;
> > > > -                #size-cells = <0>;
> > > > -
> > > > -                compatible = "fsl,imx7-mipi-csi2";
> > > > -                reg = <0x30750000 0x10000>;
> > > > -                interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> > > > -                clocks = <&clks IMX7D_IPG_ROOT_CLK>,
> > > > -                                <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
> > > > -                                <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
> > > > -                clock-names = "pclk", "wrap", "phy";
> > > > -                clock-frequency = <166000000>;
> > > > -                power-domains = <&pgc_mipi_phy>;
> > > > -                phy-supply = <&reg_1p0d>;
> > > > -                resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
> > > > -                reset-names = "mrst";
> > > > -                fsl,csis-hs-settle = <3>;
> > > > -
> > > > -                port@0 {
> > > > -                        reg = <0>;
> > > > -
> > > > -                        mipi_from_sensor: endpoint {
> > > > -                                remote-endpoint = <&ov2680_to_mipi>;
> > > > -                                data-lanes = <1>;
> > > > -                        };
> > > > -                };
> > > > -
> > > > -                port@1 {
> > > > -                        reg = <1>;
> > > > -
> > > > -                        mipi_vc0_to_csi_mux: endpoint {
> > > > -                                remote-endpoint = <&csi_mux_from_mipi_vc0>;
> > > > -                        };
> > > > -                };
> > > > -        };
> > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> > > > new file mode 100644
> > > > index 000000000000..0438b28232ed
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> > > > @@ -0,0 +1,181 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only
> > >
> > > Same question about dual licensing
> > >
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/media/nxp,imx7-mipi-csi2.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: NXP i.MX7 Mipi CSI2
> > > > +
> > > > +maintainers:
> > > > +  - Rui Miguel Silva <rmfrfs@gmail.com>
> > > > +
> > > > +description: |
> > > > +  this is the device node for the mipi csi-2 receiver core in i.mx7 soc. it is
> > > > +  compatible with previous version of samsung d-phy.
> > >
> > > nit: missing a few captial letters (beginning and after a full stop).
> >
> > will fix along all document.
> >
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    items:
> > > > +      - enum:
> > > > +          - fsl,imx7-mipi-csi2
> > >
> > > Should enum with a single item be expressed as a const ?
> >
> > agree.
> >
> > >
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  interrupts:
> > > > +    maxItems: 1
> > > > +
> > > > +  clocks:
> > > > +    minItems: 3
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: pclk
> > > > +      - const: wrap
> > > > +      - const: phy
> > > > +
> > > > +  power-domains:
> > > > +    maxItems: 1
> > > > +
> > > > +  phy-supply:
> > > > +    description:
> > > > +      Phandle to a regulator that provides power to the PHY. This
> > > > +      regulator will be managed during the PHY power on/off sequence.
> > > > +
> > > > +  resets:
> > > > +    maxItems: 1
> > > > +
> > > > +  reset-names:
> > > > +    const: mrst
> > > > +
> > > > +  clock-frequency:
> > >
> > > This was here already, but shouldn't this property be assigned in the
> > > clock provider and you should have here a reference to it ? Otherwise,
> > > is this really a custom property hijacking a standard property name ?
> > >
> > > I see it is mentioned in the example-schema.yaml, so it's probably
> > > correct.
> >
> > I think limits and default can be assigned here.
> >
> > >
> > > > +    description:
> > > > +      The IP main (system bus) clock frequency in Hertz
> > > > +    default: 166000000
> > > > +
> > > > +  fsl,csis-hs-settle:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    description:
> > > > +      differential receiver (HS-RX) settle time
> > >
> > > You have used capital letters at the beginning of documentation blocks
> > > before.
> > >
> > > > +
> > > > +  ports:
> > > > +    type: object
> > > > +    description:
> > > > +      A node containing input and output port nodes with endpoint definitions
> > > > +      as documented in
> > > > +      Documentation/devicetree/bindings/media/video-interfaces.txt
> > > > +
> > > > +    properties:
> > > > +      '#address-cells':
> > > > +        const: 1
> > > > +
> > > > +      '#size-cells':
> > > > +        const: 0
> > > > +
> > > > +      port@0:
> > > > +        type: object
> > > > +        description:
> > > > +          Input port node, single endpoint describing the CSI-2 transmitter.
> > > > +
> > > > +        properties:
> > > > +          reg:
> > > > +            const: 0
> > > > +
> > > > +          endpoint:
> > > > +            type: object
> > > > +
> > > > +            properties:
> > > > +              data-lanes:
> > > > +                maxItems: 1
> > >
> > > How many data lanes does this receiver supports ? I see in the txt
> > > bindingsd:
> > >     data-lanes    : (required) an array specifying active physical MIPI-CSI2
> > >                     data input lanes and their mapping to logical lanes; this
> > >                     shall only be applied to port 0 (sink port), the array's
> > >                     content is unused only its length is meaningful,
> > >                     in this case the maximum length supported is 2;
> > >
> > > I'm not sure I fully get this. Does this mean up to 2 data lanes are
> > > supported ?
> >
> > yeah, it supports up to 2 data lanes. Will fix
> >
> 
> This one might get nasty then. For reference, this is what I've been
> suggested to use by Laurent and Rob in the imx214 conversion (not yet
> landed afaict) to express that <1 2> and <1 2 3 4> where allowed:
> 
> +        properties:
> +          data-lanes:
> +            $ref: /schemas/types.yaml#/definitions/uint32-array
> +            description: See ../video-interfaces.txt
> +            anyOf:
> +              - items:
> +                - const: 1
> +                - const: 2
> +              - items:
> +                - const: 1
> +                - const: 2
> +                - const: 3
> +                - const: 4
> 
> Hope it helps.

Sure it helped, thanks for the hint.

------
Cheers,
     Rui

> 
> Thanks
>   j
> 
> > >
> > > > +
> > > > +              remote-endpoint: true
> > > > +
> > > > +            required:
> > > > +              - data-lanes
> > > > +              - remote-endpoint
> > > > +
> > > > +            additionalProperties: false
> > > > +
> > > > +        additionalProperties: false
> > > > +
> > > > +      port@1:
> > > > +        type: object
> > > > +        description:
> > > > +          Output port node,
> > >
> > > Rougue ',' at the end. You probably want a full stop or
> > > > +
> > > > +        properties:
> > > > +          reg:
> > > > +            const: 1
> > > > +
> > > > +          endpoint:
> > > > +              type: object
> > > > +
> > > > +              properties:
> > > > +                remote-endpoint: true
> > > > +
> > > > +              required:
> > > > +                - remote-endpoint
> > > > +
> > > > +              additionalProperties: false
> > >
> > > As no other endpoint property is specified, the whole 'endpoint' block
> > > can probably be omitted ?
> > >
> > > > +
> > > > +        additionalProperties: false
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - interrupts
> > > > +  - clocks
> > > > +  - clock-names
> > > > +  - power-domains
> > > > +  - resets
> > > > +  - reset-names
> > > > +  - ports
> > >
> > > phy-supply was listed as required in the txt bindings
> >
> > good catch.
> >
> > >
> > > > +
> > > > +unevaluatedProperties: false
> > >
> > > additionalProperties: false
> > >
> >
> > Thanks again for your review.
> >
> > Cheers,
> >    Rui
> >
> > > Thanks
> > >   j
> > >
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    #include <dt-bindings/clock/imx7d-clock.h>
> > > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > > +    #include <dt-bindings/interrupt-controller/irq.h>
> > > > +    #include <dt-bindings/reset/imx7-reset.h>
> > > > +
> > > > +    mipi_csi: mipi-csi@30750000 {
> > > > +            compatible = "fsl,imx7-mipi-csi2";
> > > > +            reg = <0x30750000 0x10000>;
> > > > +            interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> > > > +
> > > > +            clocks = <&clks IMX7D_IPG_ROOT_CLK>,
> > > > +                     <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
> > > > +                     <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
> > > > +            clock-names = "pclk", "wrap", "phy";
> > > > +            clock-frequency = <166000000>;
> > > > +
> > > > +            power-domains = <&pgc_mipi_phy>;
> > > > +            phy-supply = <&reg_1p0d>;
> > > > +            resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
> > > > +            reset-names = "mrst";
> > > > +            fsl,csis-hs-settle = <3>;
> > > > +
> > > > +            ports {
> > > > +                    #address-cells = <1>;
> > > > +                    #size-cells = <0>;
> > > > +
> > > > +                    port@0 {
> > > > +                            reg = <0>;
> > > > +
> > > > +                            mipi_from_sensor: endpoint {
> > > > +                                    remote-endpoint = <&ov2680_to_mipi>;
> > > > +                                    data-lanes = <1>;
> > > > +                            };
> > > > +                    };
> > > > +
> > > > +                    port@1 {
> > > > +                            reg = <1>;
> > > > +
> > > > +                            mipi_vc0_to_csi_mux: endpoint {
> > > > +                                    remote-endpoint = <&csi_mux_from_mipi_vc0>;
> > > > +                            };
> > > > +                    };
> > > > +            };
> > > > +    };
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index b7f7f14cd85b..9da67222b0c7 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -10773,8 +10773,8 @@ L:	linux-media@vger.kernel.org
> > > >  S:	Maintained
> > > >  T:	git git://linuxtv.org/media_tree.git
> > > >  F:	Documentation/admin-guide/media/imx7.rst
> > > > -F:	Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
> > > >  F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> > > > +F:	Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> > > >  F:	drivers/staging/media/imx/imx7-media-csi.c
> > > >  F:	drivers/staging/media/imx/imx7-mipi-csis.c
> > > >
> > > > --
> > > > 2.28.0
> > > >

Re: [PATCH v2 3/3] dt-bindings: imx7-mipi-csi2: convert bindings to yaml

[Rob Herring]

On Wed, 14 Oct 2020 15:27:59 +0100, Rui Miguel Silva wrote:
> Convert imx7 mipi csi2 bindings documentation to yaml schema, remove
> the textual document and update MAINTAINERS entry.
> 
> Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
> ---
>  .../bindings/media/imx7-mipi-csi2.txt         |  90 ---------
>  .../bindings/media/nxp,imx7-mipi-csi2.yaml    | 181 ++++++++++++++++++
>  MAINTAINERS                                   |   2 +-
>  3 files changed, 182 insertions(+), 91 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
>  create mode 100644 Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

./Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml:111:15: [warning] wrong indentation: expected 12 but found 14 (indentation)


See https://patchwork.ozlabs.org/patch/1382165

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.