Document bindings for the Synopsys DesignWare HDMI RX Controller.
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
---
.../bindings/media/snps,dw-hdmi-rx.yaml | 128 ++++++++++++++++++
1 file changed, 128 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
diff --git a/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
new file mode 100644
index 000000000000..a70d96b548ee
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
@@ -0,0 +1,128 @@
+# SPDX-License-Identifier: (GPL-3.0 OR BSD-2-Clause)
+# Device Tree bindings for Synopsys DesignWare HDMI RX Controller
+
+---
+$id: http://devicetree.org/schemas/media/snps,dw-hdmi-rx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare HDMI RX Controller
+
+maintainers:
+ - Shreeya Patel <shreeya.patel@collabora.com>
+
+properties:
+ compatible:
+ items:
+ - const: rockchip,rk3588-hdmirx-ctrler
+ - const: snps,dw-hdmi-rx
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 3
+
+ interrupt-names:
+ items:
+ - const: cec
+ - const: hdmi
+ - const: dma
+
+ clocks:
+ maxItems: 7
+
+ clock-names:
+ items:
+ - const: aclk
+ - const: audio
+ - const: cr_para
+ - const: pclk
+ - const: ref
+ - const: hclk_s_hdmirx
+ - const: hclk_vo1
+
+ power-domains:
+ maxItems: 1
+
+ resets:
+ maxItems: 4
+
+ reset-names:
+ items:
+ - const: rst_a
+ - const: rst_p
+ - const: rst_ref
+ - const: rst_biu
+
+ pinctrl-names:
+ const: default
+
+ memory-region:
+ maxItems: 1
+
+ hdmirx-5v-detection-gpios:
+ description: GPIO specifier for 5V detection.
+ maxItems: 1
+
+ rockchip,grf:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ The phandle of the syscon node for the GRF register.
+
+ rockchip,vo1_grf:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ The phandle of the syscon node for the VO1 GRF register.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - interrupt-names
+ - clocks
+ - clock-names
+ - power-domains
+ - resets
+ - pinctrl-0
+ - pinctrl-names
+ - hdmirx-5v-detection-gpios
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/rockchip,rk3588-cru.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/power/rk3588-power.h>
+ #include <dt-bindings/reset/rockchip,rk3588-cru.h>
+ hdmirx_ctrler: hdmirx-controller@fdee0000 {
+ compatible = "rockchip,rk3588-hdmirx-ctrler", "snps,dw-hdmi-rx";
+ reg = <0x0 0xfdee0000 0x0 0x6000>;
+ interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH 0>;
+ interrupt-names = "cec", "hdmi", "dma";
+ clocks = <&cru ACLK_HDMIRX>,
+ <&cru CLK_HDMIRX_AUD>,
+ <&cru CLK_CR_PARA>,
+ <&cru PCLK_HDMIRX>,
+ <&cru CLK_HDMIRX_REF>,
+ <&cru PCLK_S_HDMIRX>,
+ <&cru HCLK_VO1>;
+ clock-names = "aclk",
+ "audio",
+ "cr_para",
+ "pclk",
+ "ref",
+ "hclk_s_hdmirx",
+ "hclk_vo1";
+ power-domains = <&power RK3588_PD_VO1>;
+ resets = <&cru SRST_A_HDMIRX>, <&cru SRST_P_HDMIRX>,
+ <&cru SRST_HDMIRX_REF>, <&cru SRST_A_HDMIRX_BIU>;
+ reset-names = "rst_a", "rst_p", "rst_ref", "rst_biu";
+ pinctrl-0 = <&hdmim1_rx_cec &hdmim1_rx_hpdin &hdmim1_rx_scl &hdmim1_rx_sda &hdmirx_5v_detection>;
+ pinctrl-names = "default";
+ memory-region = <&hdmirx_cma>;
+ hdmirx-5v-detection-gpios = <&gpio1 RK_PC6 GPIO_ACTIVE_LOW>;
+ };
--
2.39.2On Fri, 16 Feb 2024 15:19:20 +0530, Shreeya Patel wrote: > Document bindings for the Synopsys DesignWare HDMI RX Controller. > > Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> > --- > .../bindings/media/snps,dw-hdmi-rx.yaml | 128 ++++++++++++++++++ > 1 file changed, 128 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: Error: Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.example.dts:57.47-48 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.example.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1428: dt_binding_check] Error 2 make: *** [Makefile:240: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240216094922.257674-3-shreeya.patel@collabora.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 16/02/2024 10:49, Shreeya Patel wrote:
> Document bindings for the Synopsys DesignWare HDMI RX Controller.
>
> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
A nit, subject: drop second/last, redundant "bindings for". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> ---
> .../bindings/media/snps,dw-hdmi-rx.yaml | 128 ++++++++++++++++++
> 1 file changed, 128 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
> new file mode 100644
> index 000000000000..a70d96b548ee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
> @@ -0,0 +1,128 @@
> +# SPDX-License-Identifier: (GPL-3.0 OR BSD-2-Clause)
Use license checkpatch tells you.
> +# Device Tree bindings for Synopsys DesignWare HDMI RX Controller
> +
> +---
> +$id: http://devicetree.org/schemas/media/snps,dw-hdmi-rx.yaml#
Why this is a media, not display? Does RX means input? Lack of hardware
description does not help?
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DesignWare HDMI RX Controller
> +
> +maintainers:
> + - Shreeya Patel <shreeya.patel@collabora.com>
> +
description:
> +properties:
> + compatible:
> + items:
> + - const: rockchip,rk3588-hdmirx-ctrler
> + - const: snps,dw-hdmi-rx
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 3
> +
> + interrupt-names:
> + items:
> + - const: cec
> + - const: hdmi
> + - const: dma
> +
> + clocks:
> + maxItems: 7
> +
> + clock-names:
> + items:
> + - const: aclk
> + - const: audio
> + - const: cr_para
> + - const: pclk
> + - const: ref
> + - const: hclk_s_hdmirx
> + - const: hclk_vo1
> +
> + power-domains:
> + maxItems: 1
> +
> + resets:
> + maxItems: 4
> +
> + reset-names:
> + items:
> + - const: rst_a
> + - const: rst_p
> + - const: rst_ref
> + - const: rst_biu
Drop rest_ prefix
> +
> + pinctrl-names:
> + const: default
Drop
> +
> + memory-region:
> + maxItems: 1
> +
> + hdmirx-5v-detection-gpios:
> + description: GPIO specifier for 5V detection.
Detection of what? Isn't this HPD?
> + maxItems: 1
> +
> + rockchip,grf:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + The phandle of the syscon node for the GRF register.
Instead describe what for. Basically 80% of your description is
redundant and only "GRF register" brings some information.
> +
> + rockchip,vo1_grf:
No underscores.
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + The phandle of the syscon node for the VO1 GRF register.
Same problem.
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - interrupt-names
> + - clocks
> + - clock-names
> + - power-domains
> + - resets
> + - pinctrl-0
> + - pinctrl-names
Why? Drop.
> + - hdmirx-5v-detection-gpios
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/power/rk3588-power.h>
> + #include <dt-bindings/reset/rockchip,rk3588-cru.h>
> + hdmirx_ctrler: hdmirx-controller@fdee0000 {
What is hdmirx-controller? Isn't this just hdmi@?
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "rockchip,rk3588-hdmirx-ctrler", "snps,dw-hdmi-rx";
> + reg = <0x0 0xfdee0000 0x0 0x6000>;
> + interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH 0>;
Best regards,
Krzysztof
On Friday, February 16, 2024 15:31 IST, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> On 16/02/2024 10:49, Shreeya Patel wrote:
> > Document bindings for the Synopsys DesignWare HDMI RX Controller.
> >
> > Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
>
> A nit, subject: drop second/last, redundant "bindings for". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
>
> > ---
> > .../bindings/media/snps,dw-hdmi-rx.yaml | 128 ++++++++++++++++++
> > 1 file changed, 128 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
> > new file mode 100644
> > index 000000000000..a70d96b548ee
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
> > @@ -0,0 +1,128 @@
> > +# SPDX-License-Identifier: (GPL-3.0 OR BSD-2-Clause)
>
> Use license checkpatch tells you.
>
> > +# Device Tree bindings for Synopsys DesignWare HDMI RX Controller
> > +
> > +---
> > +$id: http://devicetree.org/schemas/media/snps,dw-hdmi-rx.yaml#
>
> Why this is a media, not display? Does RX means input? Lack of hardware
> description does not help?
>
Yes, RX means input and this binding doc is for the HDMI INPUT controller.
I'll add some description in v2
>
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Synopsys DesignWare HDMI RX Controller
> > +
> > +maintainers:
> > + - Shreeya Patel <shreeya.patel@collabora.com>
> > +
>
> description:
>
> > +properties:
> > + compatible:
> > + items:
> > + - const: rockchip,rk3588-hdmirx-ctrler
> > + - const: snps,dw-hdmi-rx
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 3
> > +
> > + interrupt-names:
> > + items:
> > + - const: cec
> > + - const: hdmi
> > + - const: dma
> > +
> > + clocks:
> > + maxItems: 7
> > +
> > + clock-names:
> > + items:
> > + - const: aclk
> > + - const: audio
> > + - const: cr_para
> > + - const: pclk
> > + - const: ref
> > + - const: hclk_s_hdmirx
> > + - const: hclk_vo1
> > +
> > + power-domains:
> > + maxItems: 1
> > +
> > + resets:
> > + maxItems: 4
> > +
> > + reset-names:
> > + items:
> > + - const: rst_a
> > + - const: rst_p
> > + - const: rst_ref
> > + - const: rst_biu
>
> Drop rest_ prefix
>
> > +
> > + pinctrl-names:
> > + const: default
>
> Drop
>
> > +
> > + memory-region:
> > + maxItems: 1
> > +
> > + hdmirx-5v-detection-gpios:
> > + description: GPIO specifier for 5V detection.
>
> Detection of what? Isn't this HPD?
>
> > + maxItems: 1
> > +
> > + rockchip,grf:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description:
> > + The phandle of the syscon node for the GRF register.
>
> Instead describe what for. Basically 80% of your description is
> redundant and only "GRF register" brings some information.
>
>
> > +
> > + rockchip,vo1_grf:
>
> No underscores.
>
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description:
> > + The phandle of the syscon node for the VO1 GRF register.
>
> Same problem.
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - interrupt-names
> > + - clocks
> > + - clock-names
> > + - power-domains
> > + - resets
> > + - pinctrl-0
> > + - pinctrl-names
>
> Why? Drop.
>
> > + - hdmirx-5v-detection-gpios
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + #include <dt-bindings/power/rk3588-power.h>
> > + #include <dt-bindings/reset/rockchip,rk3588-cru.h>
> > + hdmirx_ctrler: hdmirx-controller@fdee0000 {
>
> What is hdmirx-controller? Isn't this just hdmi@?
>
Writing just hdmi would imply hdmi output I think so that name
will not be appropriate here.
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
This documentation doesn't have any generic name for HDMI INPUT
but maybe we can use the name hdmi-receiver like some other existing
binding has it here :-
Documentation/devicetree/bindings/media/i2c/tda1997x.txt
Thanks,
Shreeya Patel
>
> > + compatible = "rockchip,rk3588-hdmirx-ctrler", "snps,dw-hdmi-rx";
> > + reg = <0x0 0xfdee0000 0x0 0x6000>;
> > + interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH 0>,
> > + <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH 0>,
> > + <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH 0>;
>
>
>
> Best regards,
> Krzysztof
>
> _______________________________________________
> Kernel mailing list -- kernel@mailman.collabora.com
> To unsubscribe send an email to kernel-leave@mailman.collabora.com
> This list is managed by https://mailman.collabora.com
On 21/02/2024 21:55, Shreeya Patel wrote:
>>> + - hdmirx-5v-detection-gpios
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/clock/rockchip,rk3588-cru.h>
>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>> + #include <dt-bindings/power/rk3588-power.h>
>>> + #include <dt-bindings/reset/rockchip,rk3588-cru.h>
>>> + hdmirx_ctrler: hdmirx-controller@fdee0000 {
>>
>> What is hdmirx-controller? Isn't this just hdmi@?
>>
>
> Writing just hdmi would imply hdmi output I think so that name
> will not be appropriate here.
>
>> Node names should be generic. See also an explanation and list of
>> examples (not exhaustive) in DT specification:
>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>>
>
> This documentation doesn't have any generic name for HDMI INPUT
> but maybe we can use the name hdmi-receiver like some other existing
> binding has it here :-
> Documentation/devicetree/bindings/media/i2c/tda1997x.txt
Yes, it is fine. You did not respond to other comments, so I assume you
agree with them.
Best regards,
Krzysztof
© 2016 - 2026 Red Hat, Inc.