[RFT PATCH 0/5] Convert adi,adv7511.txt DT bindings to yaml

[RFT PATCH 0/5] Convert adi,adv7511.txt DT bindings to yaml

[Ricardo Cañuelo]

Hi,

This series convert the adi,adv7511.txt DT bindings to json-schema. As a
result of the conversion some dts files needed to be updated.

The changes to the dts files are of two types:

  - Reordering of the I2C slave addresses list of the ADV75xx node. The
    addresses in the 'reg' property and the matching names in
    'reg-names' for an I2C slave don't need to be in any particular
    order, but the DT schema defines these properties as a cell array
    and a string array respectively, which are ordered, so the
    definitions in the dts files must match the order in the binding.

  - Filling the minimum binding requirements. Most of the time this
    means creating a 'ports' node in the boards that don't define
    them. Note, however, that the purpose of this is simply to make the
    definition compliant with the binding. I didn't define any endpoints
    for the ports.

About the binding conversion:

  - The dependencies between properties are properly modelled but the
    result might look cluttered. If you find it hard to read or
    maintain, another option is to split the binding in two: one for the
    ADV7511/11w/13 and another one for the ADV7533/35.

Patches 1/5 to 4/5 contain the dts changes. Patch 5/5 contains the
binding conversion.

NOTE: the bindings have been tested with:

  make dt_binding_check ARCH=<arch> DT_SCHEMA_FILES=<...adi,adv7511.yaml>
  make dtbs_check ARCH=<arch> DT_SCHEMA_FILES=<...adi,adv7511.yaml>

for <arch> = arm and arm64. dts changes haven't been tested in hardware.

Kind regards,
Ricardo

Ricardo Cañuelo (5):
  arm64: dts: draak: Reorder hdmi-encoder@39 reg and reg-names
    properties
  ARM: dts: wheat: reorder reg and reg-names properties in hdmi bridges
  ARM: dts: zynq: add port definitions to hdmi-tx@39
  arm64: dts: hisilicon: hikey960: add missing hdmi bridge properties
  dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml

 .../bindings/display/bridge/adi,adv7511.txt   | 143 ---------
 .../bindings/display/bridge/adi,adv7511.yaml  | 295 ++++++++++++++++++
 arch/arm/boot/dts/r8a7792-wheat.dts           |   8 +-
 arch/arm/boot/dts/zynq-zc702.dts              |  10 +
 arch/arm/boot/dts/zynq-zc706.dts              |  10 +
 .../boot/dts/hisilicon/hi3660-hikey960.dts    |  11 +
 .../arm64/boot/dts/renesas/r8a77995-draak.dts |   4 +-
 7 files changed, 332 insertions(+), 149 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
 create mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml

-- 
2.18.0

[RFT PATCH 1/5] arm64: dts: draak: Reorder hdmi-encoder@39 reg and reg-names properties

[Ricardo Cañuelo]

Reorder the I2C slave addresses of the hdmi-encoder@39 node to make it
compliant with the adi,adv7511w DT binding.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index 67634cb01d6b..28ed852bd417 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -272,8 +272,8 @@
 
 	hdmi-encoder@39 {
 		compatible = "adi,adv7511w";
-		reg = <0x39>, <0x3f>, <0x38>, <0x3c>;
-		reg-names = "main", "edid", "packet", "cec";
+		reg = <0x39>, <0x3f>, <0x3c>, <0x38>;
+		reg-names = "main", "edid", "cec", "packet";
 		interrupt-parent = <&gpio1>;
 		interrupts = <28 IRQ_TYPE_LEVEL_LOW>;
 
-- 
2.18.0

[RFT PATCH 2/5] ARM: dts: wheat: reorder reg and reg-names properties in hdmi bridges

[Ricardo Cañuelo]

Reorder the I2C slave addresses of hdmi@3d and hdmi@39 to make them
compliant with the adi,adv7513 DT binding.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 arch/arm/boot/dts/r8a7792-wheat.dts | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7792-wheat.dts b/arch/arm/boot/dts/r8a7792-wheat.dts
index bd2a63bdab3d..f3c7072fe03f 100644
--- a/arch/arm/boot/dts/r8a7792-wheat.dts
+++ b/arch/arm/boot/dts/r8a7792-wheat.dts
@@ -249,8 +249,8 @@
 	 */
 	hdmi@3d {
 		compatible = "adi,adv7513";
-		reg = <0x3d>, <0x2d>, <0x4d>, <0x5d>;
-		reg-names = "main", "cec", "edid", "packet";
+		reg = <0x3d>, <0x4d>, <0x2d>, <0x5d>;
+		reg-names = "main", "edid", "cec", "packet";
 
 		adi,input-depth = <8>;
 		adi,input-colorspace = "rgb";
@@ -280,8 +280,8 @@
 
 	hdmi@39 {
 		compatible = "adi,adv7513";
-		reg = <0x39>, <0x29>, <0x49>, <0x59>;
-		reg-names = "main", "cec", "edid", "packet";
+		reg = <0x39>, <0x49>, <0x29>, <0x59>;
+		reg-names = "main", "edid", "cec", "packet";
 
 		adi,input-depth = <8>;
 		adi,input-colorspace = "rgb";
-- 
2.18.0

[RFT PATCH 3/5] ARM: dts: zynq: add port definitions to hdmi-tx@39

[Ricardo Cañuelo]

Define a 'ports' node for 'adv7511: hdmi-tx@39' to make it compliant with
the adi,adv7511 DT binding.

This fills the minimum requirements to meet the binding requirements,
remote endpoints are not defined.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 arch/arm/boot/dts/zynq-zc702.dts | 10 ++++++++++
 arch/arm/boot/dts/zynq-zc706.dts | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/zynq-zc702.dts b/arch/arm/boot/dts/zynq-zc702.dts
index 27cd6cb52f1b..79fd236edded 100644
--- a/arch/arm/boot/dts/zynq-zc702.dts
+++ b/arch/arm/boot/dts/zynq-zc702.dts
@@ -135,6 +135,16 @@
 				adi,input-clock = "1x";
 				adi,input-style = <3>;
 				adi,input-justification = "right";
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					port@0 {
+						reg = <0>;
+					};
+					port@1 {
+						reg = <1>;
+					};
+				};
 			};
 		};
 
diff --git a/arch/arm/boot/dts/zynq-zc706.dts b/arch/arm/boot/dts/zynq-zc706.dts
index 77943c16d33f..99fa51ba6e93 100644
--- a/arch/arm/boot/dts/zynq-zc706.dts
+++ b/arch/arm/boot/dts/zynq-zc706.dts
@@ -93,6 +93,16 @@
 				adi,input-clock = "1x";
 				adi,input-style = <3>;
 				adi,input-justification = "evenly";
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					port@0 {
+						reg = <0>;
+					};
+					port@1 {
+						reg = <1>;
+					};
+				};
 			};
 		};
 
-- 
2.18.0

[RFT PATCH 4/5] arm64: dts: hisilicon: hikey960: add missing hdmi bridge properties

[Ricardo Cañuelo]

Define a 'ports' node for 'adv7533: adv7533@39' and the 'adi,dsi-lanes'
property to make it compliant with the adi,adv7533 DT binding.

This fills the requirements to meet the binding requirements,
remote endpoints are not defined.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
index e035cf195b19..8c4bfbaf3a80 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
@@ -530,6 +530,17 @@
 		status = "ok";
 		compatible = "adi,adv7533";
 		reg = <0x39>;
+		adi,dsi-lanes = <4>;
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			port@0 {
+				reg = <0>;
+			};
+			port@1 {
+				reg = <1>;
+			};
+		};
 	};
 };
 
-- 
2.18.0

[RFT PATCH 5/5] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml

[Ricardo Cañuelo]

Convert the ADV7511/11w/13/33/35 DT bindings to json-schema.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 .../bindings/display/bridge/adi,adv7511.txt   | 143 ---------
 .../bindings/display/bridge/adi,adv7511.yaml  | 293 ++++++++++++++++++
 2 files changed, 293 insertions(+), 143 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
 create mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
deleted file mode 100644
index 659523f538bf..000000000000
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ /dev/null
@@ -1,143 +0,0 @@
-Analog Devices ADV7511(W)/13/33/35 HDMI Encoders
-------------------------------------------------
-
-The ADV7511, ADV7511W, ADV7513, ADV7533 and ADV7535 are HDMI audio and video
-transmitters compatible with HDMI 1.4 and DVI 1.0. They support color space
-conversion, S/PDIF, CEC and HDCP. ADV7533/5 supports the DSI interface for input
-pixels, while the others support RGB interface.
-
-Required properties:
-
-- compatible: Should be one of:
-		"adi,adv7511"
-		"adi,adv7511w"
-		"adi,adv7513"
-		"adi,adv7533"
-		"adi,adv7535"
-
-- reg: I2C slave addresses
-  The ADV7511 internal registers are split into four pages exposed through
-  different I2C addresses, creating four register maps. Each map has it own
-  I2C address and acts as a standard slave device on the I2C bus. The main
-  address is mandatory, others are optional and revert to defaults if not
-  specified.
-
-
-The ADV7511 supports a large number of input data formats that differ by their
-color depth, color format, clock mode, bit justification and random
-arrangement of components on the data bus. The combination of the following
-properties describe the input and map directly to the video input tables of the
-ADV7511 datasheet that document all the supported combinations.
-
-- adi,input-depth: Number of bits per color component at the input (8, 10 or
-  12).
-- adi,input-colorspace: The input color space, one of "rgb", "yuv422" or
-  "yuv444".
-- adi,input-clock: The input clock type, one of "1x" (one clock cycle per
-  pixel), "2x" (two clock cycles per pixel), "ddr" (one clock cycle per pixel,
-  data driven on both edges).
-
-The following input format properties are required except in "rgb 1x" and
-"yuv444 1x" modes, in which case they must not be specified.
-
-- adi,input-style: The input components arrangement variant (1, 2 or 3), as
-  listed in the input format tables in the datasheet.
-- adi,input-justification: The input bit justification ("left", "evenly",
-  "right").
-
-- avdd-supply: A 1.8V supply that powers up the AVDD pin on the chip.
-- dvdd-supply: A 1.8V supply that powers up the DVDD pin on the chip.
-- pvdd-supply: A 1.8V supply that powers up the PVDD pin on the chip.
-- dvdd-3v-supply: A 3.3V supply that powers up the pin called DVDD_3V
-  on the chip.
-- bgvdd-supply: A 1.8V supply that powers up the BGVDD pin. This is
-  needed only for ADV7511.
-
-The following properties are required for ADV7533 and ADV7535:
-
-- adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It should
-  be one of 1, 2, 3 or 4.
-- a2vdd-supply: 1.8V supply that powers up the A2VDD pin on the chip.
-- v3p3-supply: A 3.3V supply that powers up the V3P3 pin on the chip.
-- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be
-  either 1.2V or 1.8V for ADV7533 but only 1.8V for ADV7535.
-
-Optional properties:
-
-- interrupts: Specifier for the ADV7511 interrupt
-- pd-gpios: Specifier for the GPIO connected to the power down signal
-
-- adi,clock-delay: Video data clock delay relative to the pixel clock, in ps
-  (-1200 ps .. 1600 ps). Defaults to no delay.
-- adi,embedded-sync: The input uses synchronization signals embedded in the
-  data stream (similar to BT.656). Defaults to separate H/V synchronization
-  signals.
-- adi,disable-timing-generator: Only for ADV7533 and ADV7535. Disables the
-  internal timing generator. The chip will rely on the sync signals in the
-  DSI data lanes, rather than generate its own timings for HDMI output.
-- clocks: from common clock binding: reference to the CEC clock.
-- clock-names: from common clock binding: must be "cec".
-- reg-names : Names of maps with programmable addresses.
-	It can contain any map needing a non-default address.
-	Possible maps names are : "main", "edid", "cec", "packet"
-
-Required nodes:
-
-The ADV7511 has two video ports. Their connections are modelled using the OF
-graph bindings specified in Documentation/devicetree/bindings/graph.txt.
-
-- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533/5, the
-  remote endpoint phandle should be a reference to a valid mipi_dsi_host device
-  node.
-- Video port 1 for the HDMI output
-- Audio port 2 for the HDMI audio input
-
-
-Example
--------
-
-	adv7511w: hdmi@39 {
-		compatible = "adi,adv7511w";
-		/*
-		 * The EDID page will be accessible on address 0x66 on the I2C
-		 * bus. All other maps continue to use their default addresses.
-		 */
-		reg = <0x39>, <0x66>;
-		reg-names = "main", "edid";
-		interrupt-parent = <&gpio3>;
-		interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
-		clocks = <&cec_clock>;
-		clock-names = "cec";
-
-		adi,input-depth = <8>;
-		adi,input-colorspace = "rgb";
-		adi,input-clock = "1x";
-		adi,input-style = <1>;
-		adi,input-justification = "evenly";
-
-		ports {
-			#address-cells = <1>;
-			#size-cells = <0>;
-
-			port@0 {
-				reg = <0>;
-				adv7511w_in: endpoint {
-					remote-endpoint = <&dpi_out>;
-				};
-			};
-
-			port@1 {
-				reg = <1>;
-				adv7511_out: endpoint {
-					remote-endpoint = <&hdmi_connector_in>;
-				};
-			};
-
-			port@2 {
-				reg = <2>;
-				codec_endpoint: endpoint {
-					remote-endpoint = <&i2s0_cpu_endpoint>;
-				};
-			};
-		};
-	};
diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml
new file mode 100644
index 000000000000..0654c3a35281
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml
@@ -0,0 +1,293 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/adi,adv7511.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADV7511(W)/13/33/35 HDMI Encoders
+
+maintainers:
+  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+
+description: |
+  The ADV7511, ADV7511W, ADV7513, ADV7533 and ADV7535 are HDMI audio and
+  video transmitters compatible with HDMI 1.4 and DVI 1.0. They support
+  color space conversion, S/PDIF, CEC and HDCP. ADV7533/5 supports the
+  DSI interface for input pixels, while the others support RGB
+  interface.
+
+properties:
+  compatible:
+    enum:
+      - adi,adv7511
+      - adi,adv7511w
+      - adi,adv7513
+      - adi,adv7533
+      - adi,adv7535
+
+  reg:
+    description: |
+      I2C slave addresses.
+
+      The ADV7511 internal registers are split into four pages exposed
+      through different I2C addresses, creating four register maps. Each
+      map has it own I2C address and acts as a standard slave device on
+      the I2C bus. The main address is mandatory, others are optional
+      and revert to defaults if not specified.
+    minItems: 1
+    maxItems: 4
+
+  clocks:
+    description: Reference to the CEC clock.
+    maxItems: 1
+
+  clock-names:
+    const: cec
+
+  reg-names:
+    description:
+      Names of maps with programmable addresses. It can contain any map
+      needing a non-default address.
+    minItems: 1
+    items:
+      - const: main
+      - const: edid
+      - const: cec
+      - const: packet
+
+  interrupts:
+    maxItems: 1
+
+  pd-gpios:
+    description: GPIO connected to the power down signal.
+    maxItems: 1
+
+  avdd-supply:
+    description: A 1.8V supply that powers up the AVDD pin.
+    maxItems: 1
+
+  dvdd-supply:
+    description: A 1.8V supply that powers up the DVDD pin.
+    maxItems: 1
+
+  pvdd-supply:
+    description: A 1.8V supply that powers up the PVDD pin.
+    maxItems: 1
+
+  ports:
+    description:
+      The ADV7511 has two video ports and one audio port. This node
+      models their connections as documented in
+      Documentation/devicetree/bindings/media/video-interfaces.txt
+      Documentation/devicetree/bindings/graph.txt
+    type: object
+    properties:
+      port@0:
+        description:
+          Video port for the RGB, YUV or DSI input. In the case of
+          ADV7533/5, the remote endpoint phandle should be a reference
+          to a valid mipi_dsi_host_device.
+        type: object
+
+      port@1:
+        description: Video port for the HDMI output.
+        type: object
+
+      port@2:
+        description: Audio port for the HDMI output.
+        type: object
+
+
+allOf:
+  # ADV7511/11w/13-specific parameters
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,adv7511
+              - adi,adv7511w
+              - adi,adv7513
+
+    then:
+      properties:
+        dvdd-3v-supply:
+          description: A 3.3V supply that powers up the DVDD_3V pin.
+          maxItems: 1
+
+        bgvdd-supply:
+          description: A 1.8V supply that powers up the BGVDD pin.
+          maxItems: 1
+
+        adi,input-depth:
+          description: Number of bits per color component at the input.
+          allOf:
+            - $ref: /schemas/types.yaml#/definitions/uint32
+            - enum: [ 8, 10, 12 ]
+
+        adi,input-colorspace:
+          description: Input color space.
+          allOf:
+            - $ref: /schemas/types.yaml#/definitions/string
+            - enum: [ rgb, yuv422, yuv444 ]
+
+        adi,input-clock:
+          description: |
+            Input clock type.
+              "1x": one clock cycle per pixel
+              "2x": two clock cycles per pixel
+              "dd": one clock cycle per pixel, data driven on both edges
+          allOf:
+            - $ref: /schemas/types.yaml#/definitions/string
+            - enum: [ 1x, 2x, dd ]
+
+        adi,clock-delay:
+          description:
+            Video data clock delay relative to the pixel clock, in ps
+            (-1200ps .. 1600 ps).
+          allOf:
+            - $ref: /schemas/types.yaml#/definitions/uint32
+            - default: 0
+
+        adi,embedded-sync:
+          description:
+            The input uses synchronization signals embedded in the data
+            stream (similar to BT.656). Defaults to 0 (separate H/V
+            synchronization signals).
+          allOf:
+            - $ref: /schemas/types.yaml#/definitions/uint32
+            - enum: [ 0, 1 ]
+            - default: 0
+
+      required:
+        - adi,input-depth
+        - adi,input-colorspace
+        - adi,input-clock
+
+  # adi,input-style and adi,input-justification are required except in
+  # "rgb 1x" and "yuv444 1x" modes.
+  - if:
+      not:
+        properties:
+          adi,input-colorspace:
+            contains:
+              enum: [ rgb, yuv444 ]
+          adi,input-clock:
+            contains:
+              const: 1x
+
+    then:
+      properties:
+        adi,input-style:
+          description:
+            Input components arrangement variant as listed in the input
+            format tables in the datasheet.
+          allOf:
+            - $ref: /schemas/types.yaml#/definitions/uint32
+            - enum: [ 1, 2, 3 ]
+
+        adi,input-justification:
+          description: Input bit justification.
+          allOf:
+            - $ref: /schemas/types.yaml#/definitions/string
+            - enum: [ left, evenly, right ]
+
+      required:
+        - adi,input-style
+        - adi,input-justification
+
+  # ADV7533/35-specific parameters
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,adv7533
+              - adi,adv7535
+
+    then:
+      properties:
+        adi,disable-timing-generator:
+          description:
+            Disables the interal timing generator. The chip will rely on
+            the sync signals in the DSI data lanes, rather than generate
+            its own timings for HDMI output.
+          type: boolean
+
+        adi,dsi-lanes:
+          description: Number of DSI data lanes connected to the DSI host.
+          allOf:
+            - $ref: /schemas/types.yaml#/definitions/uint32
+            - enum: [ 1, 2, 3, 4 ]
+
+        a2vdd-supply:
+          description: A 1.8V supply that powers up the A2VDD pin.
+          maxItems: 1
+
+        v3p3-supply:
+          description: A 3.3V supply that powers up the V3P3 pin.
+          maxItems: 1
+
+        v1p2-supply:
+          description:
+            A supply that powers up the V1P2 pin. It can be either 1.2V
+            or 1.8V for ADV7533 but only 1.8V for ADV7535.
+          maxItems: 1
+
+      required:
+        - adi,dsi-lanes
+
+required:
+  - compatible
+  - reg
+  - ports
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    adv7511w: hdmi@39 {
+        compatible = "adi,adv7511w";
+        /*
+         * The EDID page will be accessible on address 0x66 on the I2C
+         * bus. All other maps continue to use their default addresses.
+         */
+        reg = <0x39>, <0x66>;
+        reg-names = "main", "edid";
+        interrupt-parent = <&gpio3>;
+        interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
+        clocks = <&cec_clock>;
+        clock-names = "cec";
+
+        adi,input-depth = <8>;
+        adi,input-colorspace = "rgb";
+        adi,input-clock = "1x";
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                adv7511w_in: endpoint {
+                    remote-endpoint = <&dpi_out>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+                adv7511_out: endpoint {
+                    remote-endpoint = <&hdmi_connector_in>;
+                };
+            };
+
+            port@2 {
+                reg = <2>;
+                codec_endpoint: endpoint {
+                    remote-endpoint = <&i2s0_cpu_endpoint>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.18.0

Re: [RFT PATCH 5/5] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml

[Rob Herring]

On Fri, May 01, 2020 at 10:32:27AM +0200, Ricardo Cañuelo wrote:
> Convert the ADV7511/11w/13/33/35 DT bindings to json-schema.
> 
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  .../bindings/display/bridge/adi,adv7511.txt   | 143 ---------
>  .../bindings/display/bridge/adi,adv7511.yaml  | 293 ++++++++++++++++++
>  2 files changed, 293 insertions(+), 143 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml


> diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml
> new file mode 100644
> index 000000000000..0654c3a35281
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml
> @@ -0,0 +1,293 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/adi,adv7511.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADV7511(W)/13/33/35 HDMI Encoders
> +
> +maintainers:
> +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +
> +description: |
> +  The ADV7511, ADV7511W, ADV7513, ADV7533 and ADV7535 are HDMI audio and
> +  video transmitters compatible with HDMI 1.4 and DVI 1.0. They support
> +  color space conversion, S/PDIF, CEC and HDCP. ADV7533/5 supports the
> +  DSI interface for input pixels, while the others support RGB
> +  interface.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,adv7511
> +      - adi,adv7511w
> +      - adi,adv7513
> +      - adi,adv7533
> +      - adi,adv7535
> +
> +  reg:
> +    description: |
> +      I2C slave addresses.
> +
> +      The ADV7511 internal registers are split into four pages exposed
> +      through different I2C addresses, creating four register maps. Each
> +      map has it own I2C address and acts as a standard slave device on
> +      the I2C bus. The main address is mandatory, others are optional
> +      and revert to defaults if not specified.
> +    minItems: 1
> +    maxItems: 4
> +
> +  clocks:
> +    description: Reference to the CEC clock.
> +    maxItems: 1
> +
> +  clock-names:
> +    const: cec
> +
> +  reg-names:
> +    description:
> +      Names of maps with programmable addresses. It can contain any map
> +      needing a non-default address.
> +    minItems: 1
> +    items:
> +      - const: main
> +      - const: edid
> +      - const: cec
> +      - const: packet
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  pd-gpios:
> +    description: GPIO connected to the power down signal.
> +    maxItems: 1
> +
> +  avdd-supply:
> +    description: A 1.8V supply that powers up the AVDD pin.
> +    maxItems: 1

Drop. Supplies are always a single item.

> +
> +  dvdd-supply:
> +    description: A 1.8V supply that powers up the DVDD pin.
> +    maxItems: 1
> +
> +  pvdd-supply:
> +    description: A 1.8V supply that powers up the PVDD pin.
> +    maxItems: 1
> +
> +  ports:
> +    description:
> +      The ADV7511 has two video ports and one audio port. This node
> +      models their connections as documented in
> +      Documentation/devicetree/bindings/media/video-interfaces.txt
> +      Documentation/devicetree/bindings/graph.txt
> +    type: object
> +    properties:
> +      port@0:
> +        description:
> +          Video port for the RGB, YUV or DSI input. In the case of
> +          ADV7533/5, the remote endpoint phandle should be a reference
> +          to a valid mipi_dsi_host_device.
> +        type: object
> +
> +      port@1:
> +        description: Video port for the HDMI output.
> +        type: object
> +
> +      port@2:
> +        description: Audio port for the HDMI output.
> +        type: object
> +
> +
> +allOf:
> +  # ADV7511/11w/13-specific parameters
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,adv7511
> +              - adi,adv7511w
> +              - adi,adv7513
> +
> +    then:
> +      properties:
> +        dvdd-3v-supply:
> +          description: A 3.3V supply that powers up the DVDD_3V pin.
> +          maxItems: 1
> +
> +        bgvdd-supply:
> +          description: A 1.8V supply that powers up the BGVDD pin.
> +          maxItems: 1
> +
> +        adi,input-depth:
> +          description: Number of bits per color component at the input.
> +          allOf:
> +            - $ref: /schemas/types.yaml#/definitions/uint32
> +            - enum: [ 8, 10, 12 ]
> +
> +        adi,input-colorspace:
> +          description: Input color space.
> +          allOf:
> +            - $ref: /schemas/types.yaml#/definitions/string
> +            - enum: [ rgb, yuv422, yuv444 ]
> +
> +        adi,input-clock:
> +          description: |
> +            Input clock type.
> +              "1x": one clock cycle per pixel
> +              "2x": two clock cycles per pixel
> +              "dd": one clock cycle per pixel, data driven on both edges
> +          allOf:
> +            - $ref: /schemas/types.yaml#/definitions/string
> +            - enum: [ 1x, 2x, dd ]
> +
> +        adi,clock-delay:
> +          description:
> +            Video data clock delay relative to the pixel clock, in ps
> +            (-1200ps .. 1600 ps).
> +          allOf:
> +            - $ref: /schemas/types.yaml#/definitions/uint32
> +            - default: 0
> +
> +        adi,embedded-sync:
> +          description:
> +            The input uses synchronization signals embedded in the data
> +            stream (similar to BT.656). Defaults to 0 (separate H/V
> +            synchronization signals).
> +          allOf:
> +            - $ref: /schemas/types.yaml#/definitions/uint32
> +            - enum: [ 0, 1 ]
> +            - default: 0
> +
> +      required:
> +        - adi,input-depth
> +        - adi,input-colorspace
> +        - adi,input-clock
> +
> +  # adi,input-style and adi,input-justification are required except in
> +  # "rgb 1x" and "yuv444 1x" modes.
> +  - if:
> +      not:
> +        properties:
> +          adi,input-colorspace:
> +            contains:
> +              enum: [ rgb, yuv444 ]
> +          adi,input-clock:
> +            contains:
> +              const: 1x

I believe this will be true (before the not) if the properties are not 
present. You need 'required' if that's not what you want.

> +
> +    then:
> +      properties:
> +        adi,input-style:
> +          description:
> +            Input components arrangement variant as listed in the input
> +            format tables in the datasheet.
> +          allOf:
> +            - $ref: /schemas/types.yaml#/definitions/uint32
> +            - enum: [ 1, 2, 3 ]
> +
> +        adi,input-justification:
> +          description: Input bit justification.
> +          allOf:
> +            - $ref: /schemas/types.yaml#/definitions/string
> +            - enum: [ left, evenly, right ]
> +
> +      required:
> +        - adi,input-style
> +        - adi,input-justification
> +
> +  # ADV7533/35-specific parameters
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,adv7533
> +              - adi,adv7535
> +
> +    then:
> +      properties:
> +        adi,disable-timing-generator:
> +          description:
> +            Disables the interal timing generator. The chip will rely on
> +            the sync signals in the DSI data lanes, rather than generate
> +            its own timings for HDMI output.
> +          type: boolean
> +
> +        adi,dsi-lanes:
> +          description: Number of DSI data lanes connected to the DSI host.
> +          allOf:
> +            - $ref: /schemas/types.yaml#/definitions/uint32
> +            - enum: [ 1, 2, 3, 4 ]
> +
> +        a2vdd-supply:
> +          description: A 1.8V supply that powers up the A2VDD pin.
> +          maxItems: 1
> +
> +        v3p3-supply:
> +          description: A 3.3V supply that powers up the V3P3 pin.
> +          maxItems: 1
> +
> +        v1p2-supply:
> +          description:
> +            A supply that powers up the V1P2 pin. It can be either 1.2V
> +            or 1.8V for ADV7533 but only 1.8V for ADV7535.
> +          maxItems: 1
> +
> +      required:
> +        - adi,dsi-lanes
> +
> +required:
> +  - compatible
> +  - reg
> +  - ports
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    adv7511w: hdmi@39 {
> +        compatible = "adi,adv7511w";
> +        /*
> +         * The EDID page will be accessible on address 0x66 on the I2C
> +         * bus. All other maps continue to use their default addresses.
> +         */
> +        reg = <0x39>, <0x66>;
> +        reg-names = "main", "edid";
> +        interrupt-parent = <&gpio3>;
> +        interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
> +        clocks = <&cec_clock>;
> +        clock-names = "cec";
> +
> +        adi,input-depth = <8>;
> +        adi,input-colorspace = "rgb";
> +        adi,input-clock = "1x";
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +                adv7511w_in: endpoint {
> +                    remote-endpoint = <&dpi_out>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +                adv7511_out: endpoint {
> +                    remote-endpoint = <&hdmi_connector_in>;
> +                };
> +            };
> +
> +            port@2 {
> +                reg = <2>;
> +                codec_endpoint: endpoint {
> +                    remote-endpoint = <&i2s0_cpu_endpoint>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> -- 
> 2.18.0
> 

Re: [RFT PATCH 5/5] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml

[Ricardo Cañuelo]

Hi Rob, thanks for taking the time to review the patch. Some comments
below:

On mar 05-05-2020 13:56:07, Rob Herring wrote:
> > +  # adi,input-style and adi,input-justification are required except in
> > +  # "rgb 1x" and "yuv444 1x" modes.
> > +  - if:
> > +      not:
> > +        properties:
> > +          adi,input-colorspace:
> > +            contains:
> > +              enum: [ rgb, yuv444 ]
> > +          adi,input-clock:
> > +            contains:
> > +              const: 1x
> 
> I believe this will be true (before the not) if the properties are not 
> present. You need 'required' if that's not what you want.

I'm not sure I understand what you mean, but dt_binding_check doesn't
say anything about adi,input-style and adi,input-justification being
required when adi,input-colorspace and adi,input-clock are not present.

I think I covered every possible case wrt those properties when running
dt_binding_check and I got the results I was looking for:

- When compatible is either "adi,adv7533" or "adi,adv7535",
  adi,input-colorspace and adi,input-clock aren't required

- For any of the other compatible strings, adi,input-colorspace and
  adi,input-clock are required.

- When adi,input-colorspace and adi,input-clock are defined and they are
  different than "rgb 1x" or "yuv444 1x", adi,input-style and
  adi,input-justification are required.

There's an issue I can't figure out, though. adi,input-colorspace and
adi,input-clock are defined only for devices other than "adi,adv7533"
and "adi,adv7535", but a DT for one of these devices can use those
properties and the binding check won't complain. Moreover, it will check
the above condition even if it doesn't make sense for them (ie. it may
complain that adi,input-style and adi,input-justification are required
even if they aren't defined for "adi,adv7533" and "adi,adv7535").

I think it's a minor issue, but do you know if there's a way to model
that? Are properties always unconditionally defined?

Cheers,
Ricardo

Re: [RFT PATCH 5/5] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml

[Rob Herring]

On Wed, May 6, 2020 at 8:11 AM Ricardo Cañuelo
<ricardo.canuelo@collabora.com> wrote:
>
> Hi Rob, thanks for taking the time to review the patch. Some comments
> below:
>
> On mar 05-05-2020 13:56:07, Rob Herring wrote:
> > > +  # adi,input-style and adi,input-justification are required except in
> > > +  # "rgb 1x" and "yuv444 1x" modes.
> > > +  - if:
> > > +      not:
> > > +        properties:
> > > +          adi,input-colorspace:
> > > +            contains:
> > > +              enum: [ rgb, yuv444 ]
> > > +          adi,input-clock:
> > > +            contains:
> > > +              const: 1x
> >
> > I believe this will be true (before the not) if the properties are not
> > present. You need 'required' if that's not what you want.
>
> I'm not sure I understand what you mean, but dt_binding_check doesn't
> say anything about adi,input-style and adi,input-justification being
> required when adi,input-colorspace and adi,input-clock are not present.
>
> I think I covered every possible case wrt those properties when running
> dt_binding_check and I got the results I was looking for:
>
> - When compatible is either "adi,adv7533" or "adi,adv7535",
>   adi,input-colorspace and adi,input-clock aren't required
>
> - For any of the other compatible strings, adi,input-colorspace and
>   adi,input-clock are required.
>
> - When adi,input-colorspace and adi,input-clock are defined and they are
>   different than "rgb 1x" or "yuv444 1x", adi,input-style and
>   adi,input-justification are required.
>
> There's an issue I can't figure out, though. adi,input-colorspace and
> adi,input-clock are defined only for devices other than "adi,adv7533"
> and "adi,adv7535", but a DT for one of these devices can use those
> properties and the binding check won't complain. Moreover, it will check
> the above condition even if it doesn't make sense for them (ie. it may
> complain that adi,input-style and adi,input-justification are required
> even if they aren't defined for "adi,adv7533" and "adi,adv7535").
>
> I think it's a minor issue, but do you know if there's a way to model
> that? Are properties always unconditionally defined?

I think you want something like this:

if:
  properties:
    compatible:
      contains:
        enum:
          - adi,adv7533
          - adi,adv7535
then:
  not:
    properties:
      adi,input-style: false
      adi,input-justification: false

Which means the schema fails if either property is present.

It may also just be easier to split this schema into 2. It's a
judgement call on how much is shared vs. how much if/then/else logic
there is.

Rob

Re: [RFT PATCH 5/5] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml

[Geert Uytterhoeven]

Hi Ricardo,

On Fri, May 1, 2020 at 10:33 AM Ricardo Cañuelo
<ricardo.canuelo@collabora.com> wrote:
> Convert the ADV7511/11w/13/33/35 DT bindings to json-schema.
>
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml
> @@ -0,0 +1,293 @@

> +  reg-names:
> +    description:
> +      Names of maps with programmable addresses. It can contain any map
> +      needing a non-default address.
> +    minItems: 1
> +    items:
> +      - const: main
> +      - const: edid
> +      - const: cec
> +      - const: packet

Can't you avoid the need for patches
[RFT PATCH 1/5] arm64: dts: draak: Reorder hdmi-encoder@39 reg and
reg-names properties
[RFT PATCH 2/5] ARM: dts: wheat: reorder reg and reg-names properties
in hdmi bridges

by using

    items:
      enum:
        - main
        - edid
        - cec
        - packet

instead?

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFT PATCH 5/5] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml

[Ricardo Cañuelo]

Hi Geert,

Thanks for reviewing the patches. Some comments below,

On mié 06-05-2020 09:44:19, Geert Uytterhoeven wrote: 
> Can't you avoid the need for patches
> [RFT PATCH 1/5] arm64: dts: draak: Reorder hdmi-encoder@39 reg and
> reg-names properties
> [RFT PATCH 2/5] ARM: dts: wheat: reorder reg and reg-names properties
> in hdmi bridges
> 
> by using
> 
>     items:
>       enum:
>         - main
>         - edid
>         - cec
>         - packet
> 
> instead?

Not really, because that defines a scalar property that can take any of
those values (if I'm not mistaken), and the core schema enforces that
reg-names must be an array.

I think the closest I can get to what you mean would be something like
this:

    items:
      - enum:
        - main
        - edid
        - cec
        - packet
      - enum:
        - main
        - edid
        - cec
        - packet
      - enum:
        - main
        - edid
        - cec
        - packet
      - enum:
        - main
        - edid
        - cec
        - packet

But then that wouldn't prevent anyone from defining duplicate reg-names
(eg. "main", "cec", edid", "cec"), which is even worse IMO.

Cheers,
Ricardo

Re: [RFT PATCH 5/5] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml

[Laurent Pinchart]

On Wed, May 06, 2020 at 10:23:32AM +0200, Ricardo Cañuelo wrote:
> Hi Geert,
> 
> Thanks for reviewing the patches. Some comments below,
> 
> On mié 06-05-2020 09:44:19, Geert Uytterhoeven wrote: 
> > Can't you avoid the need for patches
> > [RFT PATCH 1/5] arm64: dts: draak: Reorder hdmi-encoder@39 reg and
> > reg-names properties
> > [RFT PATCH 2/5] ARM: dts: wheat: reorder reg and reg-names properties
> > in hdmi bridges
> > 
> > by using
> > 
> >     items:
> >       enum:
> >         - main
> >         - edid
> >         - cec
> >         - packet
> > 
> > instead?
> 
> Not really, because that defines a scalar property that can take any of
> those values (if I'm not mistaken), and the core schema enforces that
> reg-names must be an array.
> 
> I think the closest I can get to what you mean would be something like
> this:
> 
>     items:
>       - enum:
>         - main
>         - edid
>         - cec
>         - packet
>       - enum:
>         - main
>         - edid
>         - cec
>         - packet
>       - enum:
>         - main
>         - edid
>         - cec
>         - packet
>       - enum:
>         - main
>         - edid
>         - cec
>         - packet
> 
> But then that wouldn't prevent anyone from defining duplicate reg-names
> (eg. "main", "cec", edid", "cec"), which is even worse IMO.

The direction DT bindings are taking is to enfore a particular order. It
will cause DT validation errors for old device trees, but it won't break
backward compatibility as the order won't be enforced at runtime, so I
think that's fine. Tidying up the existing DT sources to use a
consistent order seems best to me.

-- 
Regards,

Laurent Pinchart

Re: [RFT PATCH 5/5] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml

[Rob Herring]

On Wed, May 6, 2020 at 5:57 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Wed, May 06, 2020 at 10:23:32AM +0200, Ricardo Cañuelo wrote:
> > Hi Geert,
> >
> > Thanks for reviewing the patches. Some comments below,
> >
> > On mié 06-05-2020 09:44:19, Geert Uytterhoeven wrote:
> > > Can't you avoid the need for patches
> > > [RFT PATCH 1/5] arm64: dts: draak: Reorder hdmi-encoder@39 reg and
> > > reg-names properties
> > > [RFT PATCH 2/5] ARM: dts: wheat: reorder reg and reg-names properties
> > > in hdmi bridges
> > >
> > > by using
> > >
> > >     items:
> > >       enum:
> > >         - main
> > >         - edid
> > >         - cec
> > >         - packet
> > >
> > > instead?
> >
> > Not really, because that defines a scalar property that can take any of
> > those values (if I'm not mistaken), and the core schema enforces that
> > reg-names must be an array.

No, 'items' as a schema rather than a list applies to every element in an array.

> >
> > I think the closest I can get to what you mean would be something like
> > this:
> >
> >     items:
> >       - enum:
> >         - main
> >         - edid
> >         - cec
> >         - packet
> >       - enum:
> >         - main
> >         - edid
> >         - cec
> >         - packet
> >       - enum:
> >         - main
> >         - edid
> >         - cec
> >         - packet
> >       - enum:
> >         - main
> >         - edid
> >         - cec
> >         - packet
> >
> > But then that wouldn't prevent anyone from defining duplicate reg-names
> > (eg. "main", "cec", edid", "cec"), which is even worse IMO.
>
> The direction DT bindings are taking is to enfore a particular order. It
> will cause DT validation errors for old device trees, but it won't break
> backward compatibility as the order won't be enforced at runtime, so I
> think that's fine. Tidying up the existing DT sources to use a
> consistent order seems best to me.

Yes.

It's not a new direction though. The order was always supposed to be
defined, it's just enforceable now.

Rob