[PATCH 0/3] dt-bindings: display: bridge: nxp,tda998x: Json-schema conversion and fixes

[PATCH 0/3] dt-bindings: display: bridge: nxp,tda998x: Json-schema conversion and fixes

[Geert Uytterhoeven]

	Hi all,

This patch series converts the NXP TDA998x HDMI transmitter Device Tree
binding documentation to json-schema, after a few customary fixes.

Thanks for your comments!

Geert Uytterhoeven (3):
  ARM: dts: am335x: Fix TDA998x ports addressing
  [RFC] arm64: dts: renesas: cat874: Drop bogus clocks property
  [RFC] dt-bindings: display: bridge: nxp,tda998x: Convert to
    json-schema

 .../bindings/display/bridge/nxp,tda998x.yaml  | 108 ++++++++++++++++++
 .../bindings/display/bridge/tda998x.txt       |  54 ---------
 arch/arm/boot/dts/am335x-boneblack-hdmi.dtsi  |   7 +-
 arch/arm/boot/dts/am335x-myirtech-myd.dts     |   7 +-
 .../boot/dts/renesas/r8a774c0-cat874.dts      |   1 -
 5 files changed, 120 insertions(+), 57 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/nxp,tda998x.yaml
 delete mode 100644 Documentation/devicetree/bindings/display/bridge/tda998x.txt

-- 
2.25.1

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

[PATCH 1/3] ARM: dts: am335x: Fix TDA998x ports addressing

[Geert Uytterhoeven]

Fix addressing in the NXP TDA998x HDMI transmitters' subnodes:
  - Add missing #{address,size}-cells properties to ports capsule,
  - Add missing reg properties to port child nodes,
  - Drop bogus unit addresses from endpoint grandchildren nodes.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/am335x-boneblack-hdmi.dtsi | 7 ++++++-
 arch/arm/boot/dts/am335x-myirtech-myd.dts    | 7 ++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-boneblack-hdmi.dtsi b/arch/arm/boot/dts/am335x-boneblack-hdmi.dtsi
index 7cfddada934861bc..486f24deb875c688 100644
--- a/arch/arm/boot/dts/am335x-boneblack-hdmi.dtsi
+++ b/arch/arm/boot/dts/am335x-boneblack-hdmi.dtsi
@@ -85,8 +85,13 @@ tda19988: tda19988@70 {
 		audio-ports = <	TDA998x_I2S	0x03>;
 
 		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
 			port@0 {
-				hdmi_0: endpoint@0 {
+				reg = <0>;
+
+				hdmi_0: endpoint {
 					remote-endpoint = <&lcdc_0>;
 				};
 			};
diff --git a/arch/arm/boot/dts/am335x-myirtech-myd.dts b/arch/arm/boot/dts/am335x-myirtech-myd.dts
index 1479fd95dec2441c..4120bfb351bd9e69 100644
--- a/arch/arm/boot/dts/am335x-myirtech-myd.dts
+++ b/arch/arm/boot/dts/am335x-myirtech-myd.dts
@@ -161,8 +161,13 @@ tda9988: tda9988@70 {
 		#sound-dai-cells = <0>;
 
 		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
 			port@0 {
-				hdmi_0: endpoint@0 {
+				reg = <0>;
+
+				hdmi_0: endpoint {
 					remote-endpoint = <&lcdc_0>;
 				};
 			};
-- 
2.25.1

[PATCH 2/3] [RFC] arm64: dts: renesas: cat874: Drop bogus clocks property

[Geert Uytterhoeven]

The NXP TDA998x HDMI transmitter Device Tree binding documentation does
not mention a clocks property.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Is this property just missing from the bindings?
The driver doesn't seem to use it.
---
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index 4e72e4f2bab055a5..c6fb684814bc290e 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -239,7 +239,6 @@ tda19988: tda19988@70 {
 
 		#sound-dai-cells = <0>;
 		audio-ports = <TDA998x_I2S 0x03>;
-		clocks = <&rcar_sound 1>;
 
 		ports {
 			#address-cells = <1>;
-- 
2.25.1

[PATCH 3/3] [RFC] dt-bindings: display: bridge: nxp,tda998x: Convert to json-schema

[Geert Uytterhoeven]

Convert the NXP TDA998x HDMI transmitter Device Tree binding
documentation to json-schema.

Add missing "#sound-dai-cells" property.
Add ports hierarchy, as an alternative to port.
Drop pinctrl properties, as they do not belong here.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
RFC as I do not know:
  1. The correct value(s) for '#sound-dai-cells' (the example used 2,
     which was IMHO wrong, while all actual users use 0),
  2. The meaning of the various ports subnodes.
---
 .../bindings/display/bridge/nxp,tda998x.yaml  | 108 ++++++++++++++++++
 .../bindings/display/bridge/tda998x.txt       |  54 ---------
 2 files changed, 108 insertions(+), 54 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/nxp,tda998x.yaml
 delete mode 100644 Documentation/devicetree/bindings/display/bridge/tda998x.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/nxp,tda998x.yaml b/Documentation/devicetree/bindings/display/bridge/nxp,tda998x.yaml
new file mode 100644
index 0000000000000000..87c64edcf5d5617d
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/nxp,tda998x.yaml
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/nxp,tda998x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP TDA998x HDMI transmitter
+
+maintainers:
+  - Russell King <linux@armlinux.org.uk>
+
+properties:
+  compatible:
+    const: nxp,tda998x
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  video-ports:
+    default: 0x230145
+    description:
+      24 bits value which defines how the video controller output is wired to
+      the TDA998x input.
+
+  audio-ports:
+    description:
+      Array of 8-bit values, 2 values per DAI (Documentation/sound/soc/dai.rst).
+      The implementation allows one or two DAIs.
+      If two DAIs are defined, they must be of different type.
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    items:
+      minItems: 1
+      maxItems: 2
+      items:
+        - description: |
+            The first value defines the DAI type: TDA998x_SPDIF or TDA998x_I2S
+            (see include/dt-bindings/display/tda998x.h).
+        - description:
+            The second value defines the tda998x AP_ENA reg content when the
+            DAI in question is used.
+
+  '#sound-dai-cells':
+    enum: [ 0, 1 ]
+
+  nxp,calib-gpios:
+    maxItems: 1
+    description:
+      Calibration GPIO, which must correspond with the gpio used for the
+      TDA998x interrupt pin.
+
+  port:
+    $ref: /schemas/graph.yaml#/properties/port
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        type: object
+        description: FIXME
+
+      port@1:
+        type: object
+        description: FIXME
+
+required:
+  - compatible
+  - reg
+
+oneOf:
+  - required:
+      - port
+  - required:
+      - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/display/tda998x.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        tda998x: hdmi-encoder@70 {
+            compatible = "nxp,tda998x";
+            reg = <0x70>;
+            interrupt-parent = <&gpio0>;
+            interrupts = <27 IRQ_TYPE_EDGE_FALLING>;
+            video-ports = <0x230145>;
+
+            #sound-dai-cells = <1>;
+                         /* DAI-format / AP_ENA reg value */
+            audio-ports = <TDA998x_SPDIF 0x04>,
+                          <TDA998x_I2S 0x03>;
+
+            port {
+                tda998x_in: endpoint {
+                    remote-endpoint = <&lcdc_0>;
+                };
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/display/bridge/tda998x.txt b/Documentation/devicetree/bindings/display/bridge/tda998x.txt
deleted file mode 100644
index f5a02f61dd36f1c6..0000000000000000
--- a/Documentation/devicetree/bindings/display/bridge/tda998x.txt
+++ /dev/null
@@ -1,54 +0,0 @@
-Device-Tree bindings for the NXP TDA998x HDMI transmitter
-
-Required properties;
-  - compatible: must be "nxp,tda998x"
-
-  - reg: I2C address
-
-Required node:
-  - port: Input port node with endpoint definition, as described
-        in Documentation/devicetree/bindings/graph.txt
-
-Optional properties:
-  - interrupts: interrupt number and trigger type
-	default: polling
-
-  - pinctrl-0: pin control group to be used for
-	screen plug/unplug interrupt.
-
-  - pinctrl-names: must contain a "default" entry.
-
-  - video-ports: 24 bits value which defines how the video controller
-	output is wired to the TDA998x input - default: <0x230145>
-
-  - audio-ports: array of 8-bit values, 2 values per one DAI[1].
-	The first value defines the DAI type: TDA998x_SPDIF or TDA998x_I2S[2].
-	The second value defines the tda998x AP_ENA reg content when the DAI
-	in question is used. The implementation allows one or two DAIs. If two
-	DAIs are defined, they must be of different type.
-
-  - nxp,calib-gpios: calibration GPIO, which must correspond with the
-	gpio used for the TDA998x interrupt pin.
-
-[1] Documentation/sound/soc/dai.rst
-[2] include/dt-bindings/display/tda998x.h
-
-Example:
-
-#include <dt-bindings/display/tda998x.h>
-
-	tda998x: hdmi-encoder {
-		compatible = "nxp,tda998x";
-		reg = <0x70>;
-		interrupt-parent = <&gpio0>;
-		interrupts = <27 2>;		/* falling edge */
-		pinctrl-0 = <&pmx_camera>;
-		pinctrl-names = "default";
-		video-ports = <0x230145>;
-
-		#sound-dai-cells = <2>;
-			     /*	DAI-format	AP_ENA reg value */
-		audio-ports = <	TDA998x_SPDIF	0x04
-				TDA998x_I2S	0x03>;
-
-	};
-- 
2.25.1

Re: [PATCH 3/3] [RFC] dt-bindings: display: bridge: nxp,tda998x: Convert to json-schema

[Rob Herring]

On Thu, Oct 21, 2021 at 03:18:53PM +0200, Geert Uytterhoeven wrote:
> Convert the NXP TDA998x HDMI transmitter Device Tree binding
> documentation to json-schema.
> 
> Add missing "#sound-dai-cells" property.
> Add ports hierarchy, as an alternative to port.
> Drop pinctrl properties, as they do not belong here.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> RFC as I do not know:
>   1. The correct value(s) for '#sound-dai-cells' (the example used 2,
>      which was IMHO wrong, while all actual users use 0),
>   2. The meaning of the various ports subnodes.
> ---
>  .../bindings/display/bridge/nxp,tda998x.yaml  | 108 ++++++++++++++++++
>  .../bindings/display/bridge/tda998x.txt       |  54 ---------
>  2 files changed, 108 insertions(+), 54 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/nxp,tda998x.yaml
>  delete mode 100644 Documentation/devicetree/bindings/display/bridge/tda998x.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/nxp,tda998x.yaml b/Documentation/devicetree/bindings/display/bridge/nxp,tda998x.yaml
> new file mode 100644
> index 0000000000000000..87c64edcf5d5617d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/nxp,tda998x.yaml
> @@ -0,0 +1,108 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/nxp,tda998x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP TDA998x HDMI transmitter
> +
> +maintainers:
> +  - Russell King <linux@armlinux.org.uk>
> +
> +properties:
> +  compatible:
> +    const: nxp,tda998x
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  video-ports:
> +    default: 0x230145
> +    description:
> +      24 bits value which defines how the video controller output is wired to
> +      the TDA998x input.

maximum: 0xffffff

(at least...)

> +
> +  audio-ports:
> +    description:
> +      Array of 8-bit values, 2 values per DAI (Documentation/sound/soc/dai.rst).
> +      The implementation allows one or two DAIs.
> +      If two DAIs are defined, they must be of different type.
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    items:
> +      minItems: 1
> +      maxItems: 2
> +      items:
> +        - description: |
> +            The first value defines the DAI type: TDA998x_SPDIF or TDA998x_I2S
> +            (see include/dt-bindings/display/tda998x.h).
> +        - description:
> +            The second value defines the tda998x AP_ENA reg content when the
> +            DAI in question is used.
> +
> +  '#sound-dai-cells':
> +    enum: [ 0, 1 ]
> +
> +  nxp,calib-gpios:
> +    maxItems: 1
> +    description:
> +      Calibration GPIO, which must correspond with the gpio used for the
> +      TDA998x interrupt pin.
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/properties/port
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        type: object
> +        description: FIXME

Looks like the input from the example

> +
> +      port@1:
> +        type: object
> +        description: FIXME

Presumably the output to connector or another bridge.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +oneOf:
> +  - required:
> +      - port
> +  - required:
> +      - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/display/tda998x.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        tda998x: hdmi-encoder@70 {
> +            compatible = "nxp,tda998x";
> +            reg = <0x70>;
> +            interrupt-parent = <&gpio0>;
> +            interrupts = <27 IRQ_TYPE_EDGE_FALLING>;
> +            video-ports = <0x230145>;
> +
> +            #sound-dai-cells = <1>;
> +                         /* DAI-format / AP_ENA reg value */
> +            audio-ports = <TDA998x_SPDIF 0x04>,
> +                          <TDA998x_I2S 0x03>;
> +
> +            port {
> +                tda998x_in: endpoint {
> +                    remote-endpoint = <&lcdc_0>;
> +                };
> +            };
> +        };
> +    };

Re: [PATCH 3/3] [RFC] dt-bindings: display: bridge: nxp,tda998x: Convert to json-schema

[Russell King (Oracle)]

On Thu, Oct 28, 2021 at 08:04:48PM -0500, Rob Herring wrote:
> On Thu, Oct 21, 2021 at 03:18:53PM +0200, Geert Uytterhoeven wrote:
> > +    properties:
> > +      port@0:
> > +        type: object
> > +        description: FIXME
> 
> Looks like the input from the example
> 
> > +
> > +      port@1:
> > +        type: object
> > +        description: FIXME
> 
> Presumably the output to connector or another bridge.

This is changing the binding. The original had:

Required node:
  - port: Input port node with endpoint definition, as described
        in Documentation/devicetree/bindings/graph.txt

The above change appears to require that tda998x now has two ports.
This goes against current usage in DT and the example.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 3/3] [RFC] dt-bindings: display: bridge: nxp,tda998x: Convert to json-schema

[Geert Uytterhoeven]

Hi Russell,

Thanks for your comments!

On Fri, Oct 29, 2021 at 10:08 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> On Thu, Oct 28, 2021 at 08:04:48PM -0500, Rob Herring wrote:
> > On Thu, Oct 21, 2021 at 03:18:53PM +0200, Geert Uytterhoeven wrote:
> > > +    properties:
> > > +      port@0:
> > > +        type: object
> > > +        description: FIXME
> >
> > Looks like the input from the example
> >
> > > +
> > > +      port@1:
> > > +        type: object
> > > +        description: FIXME
> >
> > Presumably the output to connector or another bridge.
>
> This is changing the binding. The original had:
>
> Required node:
>   - port: Input port node with endpoint definition, as described
>         in Documentation/devicetree/bindings/graph.txt

Indeed, cfr. "Add ports hierarchy, as an alternative to port." in the
patch description. Some users use port, other use ports with one or
two port subnodes.

> The above change appears to require that tda998x now has two ports.

No, you can still use port:

+oneOf:
+  - required:
+      - port
+  - required:
+      - ports

When using ports, no further requirements are set, but perhaps port@0
should be made required in that case?

> This goes against current usage in DT and the example.

The original example didn't even have the original required input
port node, so it was incomplete.

arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts has two port subnodes.
Is that wrong?

Thanks!

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: [PATCH 3/3] [RFC] dt-bindings: display: bridge: nxp,tda998x: Convert to json-schema

[Russell King (Oracle)]

On Fri, Oct 29, 2021 at 10:28:22AM +0200, Geert Uytterhoeven wrote:
> Hi Russell,
> 
> Thanks for your comments!
> 
> No, you can still use port:
> 
> +oneOf:
> +  - required:
> +      - port
> +  - required:
> +      - ports
> 
> When using ports, no further requirements are set, but perhaps port@0
> should be made required in that case?

Maybe I don't understand the binding description then, but to me it
looks like you require both port@0 and port@1.

The reality of the driver is that it makes almost no use of the graph
itself, except via drm_of_find_possible_crtcs() to find the connected
CRTCs. If it is connected to an I2S source, then it probably needs a
port specification for that. If someone wants to describe the HDMI
connector (which I don't see any point in) then they likely need a
port specification for that too. However, the driver itself doesn't
care about any of those.

So, describing the port nodes makes no sense.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 3/3] [RFC] dt-bindings: display: bridge: nxp,tda998x: Convert to json-schema

[Geert Uytterhoeven]

Hi Russell,

On Fri, Oct 29, 2021 at 11:33 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> On Fri, Oct 29, 2021 at 10:28:22AM +0200, Geert Uytterhoeven wrote:
> > No, you can still use port:
> >
> > +oneOf:
> > +  - required:
> > +      - port
> > +  - required:
> > +      - ports
> >
> > When using ports, no further requirements are set, but perhaps port@0
> > should be made required in that case?
>
> Maybe I don't understand the binding description then, but to me it
> looks like you require both port@0 and port@1.

"make dtbs_check" disagrees.

> The reality of the driver is that it makes almost no use of the graph
> itself, except via drm_of_find_possible_crtcs() to find the connected
> CRTCs. If it is connected to an I2S source, then it probably needs a
> port specification for that. If someone wants to describe the HDMI
> connector (which I don't see any point in) then they likely need a

I can't comment on the point of describing the HDMI connector.

> port specification for that too. However, the driver itself doesn't
> care about any of those.

DT describes hardware, not software limitations.

> So, describing the port nodes makes no sense.

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: [PATCH 3/3] [RFC] dt-bindings: display: bridge: nxp,tda998x: Convert to json-schema

[Russell King (Oracle)]

On Fri, Oct 29, 2021 at 11:40:26AM +0200, Geert Uytterhoeven wrote:
> Hi Russell,
> 
> On Fri, Oct 29, 2021 at 11:33 AM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> > On Fri, Oct 29, 2021 at 10:28:22AM +0200, Geert Uytterhoeven wrote:
> > > No, you can still use port:
> > >
> > > +oneOf:
> > > +  - required:
> > > +      - port
> > > +  - required:
> > > +      - ports
> > >
> > > When using ports, no further requirements are set, but perhaps port@0
> > > should be made required in that case?
> >
> > Maybe I don't understand the binding description then, but to me it
> > looks like you require both port@0 and port@1.
> 
> "make dtbs_check" disagrees.
> 
> > The reality of the driver is that it makes almost no use of the graph
> > itself, except via drm_of_find_possible_crtcs() to find the connected
> > CRTCs. If it is connected to an I2S source, then it probably needs a
> > port specification for that. If someone wants to describe the HDMI
> > connector (which I don't see any point in) then they likely need a
> 
> I can't comment on the point of describing the HDMI connector.
> 
> > port specification for that too. However, the driver itself doesn't
> > care about any of those.
> 
> DT describes hardware, not software limitations.

Sigh. There's no point discussing this further, my replies seem to be
interpreted out of context.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!