Re: [PATCH v3 2/9] dt-bindings: media: nxp: Add Wave6 video codec device
From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2025-08-29 13:57:29
Also in:
linux-devicetree, linux-media, lkml
On 29/08/2025 10:46, Nas Chung wrote:
Add documents for the Wave6 video codec on NXP i.MX SoCs.
Pretty incomplete commit msg. Nothing explaining hardware, nothing documenting resolution of previous discussions (where is all this chip&media?). ...
+ +properties: + compatible: + enum: + - nxp,imx95-vpu + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + power-domains: + maxItems: 1 + + memory-region: + maxItems: 1 + + sram: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle of the SRAM memory region node. + + "#cooling-cells": + const: 2 + + "#address-cells": + const: 2 + + "#size-cells": + const: 2 + + ranges: true + +patternProperties: + "^video-core@[0-9a-f]+$": + type: object
Missing description.
+ additionalProperties: false + + properties: + compatible: + enum: + - nxp,imx95-vpu-core
Why do you need here compatible? Can this child be anything else? Can it be re-used? Is it actually a separate block? Your example suggests that the only distinctive resource are the interrupt and address space and that's on the edge of calling it a separate device. There is some tendency to call such "pseudo-cores" a separate devices in case of video codec bindings and experience shows these are usually fake. It's not the same as DP or HDMI sub-block of display pipeline. That's why you should come here with strong argument what separate piece of hardware this is.
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ power-domains:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ required:
+ - compatible
+ - reg
+ - clocks
+ - power-domains
+ - interrupts
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - power-domains
+ - memory-region
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/nxp,imx95-clock.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ vpu: video-codec@4c4c0000 {Unused label, drop
+ compatible = "nxp,imx95-vpu";
+ reg = <0x0 0x4c4c0000 0x0 0x10000>;
+ clocks = <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>;
+ power-domains = <&scmi_perf 10>;
+ memory-region = <&vpu_boot>;
+ sram = <&sram1>;
+ #cooling-cells = <2>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ vpucore0: video-core@4c480000 {None of these labels are used, drop.
+ compatible = "nxp,imx95-vpu-core";
+ reg = <0x0 0x4c480000 0x0 0x10000>;
+ clocks = <&scmi_clk 115>;
+ power-domains = <&scmi_devpd 21>;
+ interrupts = <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ vpucore1: video-core@4c490000 {
+ compatible = "nxp,imx95-vpu-core";
+ reg = <0x0 0x4c490000 0x0 0x10000>;
+ clocks = <&scmi_clk 115>;
+ power-domains = <&scmi_devpd 21>;
+ interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
+ };
+Best regards, Krzysztof